[PATCH v3 0/2] serial_mxc: fixing reception

while writing to a imx-serial device is probably thoroughly tested - and obviosly works for the debug-serial - using a serial driver to read data received over the serial interface does not work reliably.
the patches in this series address issues found during the implementation of a custom uboot-command to query a coprocessor, connected to an imx8mm over uart4, for mainboard-identification strings
Changes in v3: - more verbose commit messages
Changes in v2: - manually fix 'to' and 'cc' - fix comment delimiter
Johannes Schneider (2): serial: mxc: enable the RX pipeline serial: mxc: have putc use the TXFIFO
drivers/serial/serial_mxc.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)

on imx8(mm) the RXDMUXSEL needs to be set for data going over the wire (as observable on a connected 'scope) to actually make it into the RXFIFO
the reference manual is not overly clear about this, and only mentiones that "UCR3_RXDMUXSEL should always be set." - and since the CR3 register reverts to its reset values after setting the baudrate, setting this bit is done during '_mxc_serial_setbgr'
Signed-off-by: Johannes Schneider johannes.schneider@leica-geosystems.com
---
Changes in v3: - more verbose commit messages
Changes in v2: - manually fix 'to' and 'cc' - fix comment delimiter
drivers/serial/serial_mxc.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c index 70a0e5e919..fa344c8b8f 100644 --- a/drivers/serial/serial_mxc.c +++ b/drivers/serial/serial_mxc.c @@ -61,6 +61,11 @@ #define UCR3_AWAKEN (1<<4) /* Async wake interrupt enable */ #define UCR3_REF25 (1<<3) /* Ref freq 25 MHz */ #define UCR3_REF30 (1<<2) /* Ref Freq 30 MHz */ + +/* imx8 names these bitsfields instead: */ +#define UCR3_DTRDEN BIT(3) /* bit not used in this chip */ +#define UCR3_RXDMUXSEL BIT(2) /* RXD muxed input selected; 'should always be set' */ + #define UCR3_INVT (1<<1) /* Inverted Infrared transmission */ #define UCR3_BPEN (1<<0) /* Preset registers enable */ #define UCR4_CTSTL_32 (32<<10) /* CTS trigger level (32 chars) */ @@ -176,6 +181,13 @@ static void _mxc_serial_setbrg(struct mxc_uart *base, unsigned long clk,
writel(UCR2_WS | UCR2_IRTS | UCR2_RXEN | UCR2_TXEN | UCR2_SRST, &base->cr2); + + /* setting the baudrate triggers a reset, returning cr3 to its + * reset value but UCR3_RXDMUXSEL "should always be set." + * according to the imx8 reference-manual + */ + writel(readl(&base->cr3) | UCR3_RXDMUXSEL, &base->cr3); + writel(UCR1_UARTEN, &base->cr1); }

On 9/6/2022 1:39 PM, Johannes Schneider wrote:
on imx8(mm) the RXDMUXSEL needs to be set for data going over the wire (as observable on a connected 'scope) to actually make it into the RXFIFO
the reference manual is not overly clear about this, and only mentiones that "UCR3_RXDMUXSEL should always be set." - and since the CR3 register reverts to its reset values after setting the baudrate, setting this bit is done during '_mxc_serial_setbgr'
Signed-off-by: Johannes Schneider johannes.schneider@leica-geosystems.com
Reviewed-by: Peng Fan peng.fan@nxp.com
Changes in v3:
- more verbose commit messages
Changes in v2:
manually fix 'to' and 'cc'
fix comment delimiter
drivers/serial/serial_mxc.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c index 70a0e5e919..fa344c8b8f 100644 --- a/drivers/serial/serial_mxc.c +++ b/drivers/serial/serial_mxc.c @@ -61,6 +61,11 @@ #define UCR3_AWAKEN (1<<4) /* Async wake interrupt enable */ #define UCR3_REF25 (1<<3) /* Ref freq 25 MHz */ #define UCR3_REF30 (1<<2) /* Ref Freq 30 MHz */
+/* imx8 names these bitsfields instead: */ +#define UCR3_DTRDEN BIT(3) /* bit not used in this chip */ +#define UCR3_RXDMUXSEL BIT(2) /* RXD muxed input selected; 'should always be set' */
- #define UCR3_INVT (1<<1) /* Inverted Infrared transmission */ #define UCR3_BPEN (1<<0) /* Preset registers enable */ #define UCR4_CTSTL_32 (32<<10) /* CTS trigger level (32 chars) */
@@ -176,6 +181,13 @@ static void _mxc_serial_setbrg(struct mxc_uart *base, unsigned long clk,
writel(UCR2_WS | UCR2_IRTS | UCR2_RXEN | UCR2_TXEN | UCR2_SRST, &base->cr2);
- /* setting the baudrate triggers a reset, returning cr3 to its
* reset value but UCR3_RXDMUXSEL "should always be set."
* according to the imx8 reference-manual
*/
- writel(readl(&base->cr3) | UCR3_RXDMUXSEL, &base->cr3);
- writel(UCR1_UARTEN, &base->cr1); }

only waiting for TXEMPTY leads to corrupted messages going over the wire - which is fixed by making use of the FIFO
this change is following the linux kernel uart driver (drivers/tty/serial/imx.c), which also checks UTS_TXFULL instead of UTS_TXEMPTY
Signed-off-by: Johannes Schneider johannes.schneider@leica-geosystems.com Reviewed-by: Peng Fan peng.fan@oss.nxp.com ---
(no changes since v1)
drivers/serial/serial_mxc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c index fa344c8b8f..0520c7929a 100644 --- a/drivers/serial/serial_mxc.c +++ b/drivers/serial/serial_mxc.c @@ -310,7 +310,7 @@ static int mxc_serial_putc(struct udevice *dev, const char ch) struct mxc_serial_plat *plat = dev_get_plat(dev); struct mxc_uart *const uart = plat->reg;
- if (!(readl(&uart->ts) & UTS_TXEMPTY)) + if (readl(&uart->ts) & UTS_TXFULL) return -EAGAIN;
writel(ch, &uart->txd);

On 9/6/2022 1:39 PM, Johannes Schneider wrote:
only waiting for TXEMPTY leads to corrupted messages going over the wire - which is fixed by making use of the FIFO
this change is following the linux kernel uart driver (drivers/tty/serial/imx.c), which also checks UTS_TXFULL instead of UTS_TXEMPTY
Signed-off-by: Johannes Schneider johannes.schneider@leica-geosystems.com Reviewed-by: Peng Fan peng.fan@oss.nxp.com
Reviewed-by: Peng Fan peng.fan@nxp.com
(no changes since v1)
drivers/serial/serial_mxc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c index fa344c8b8f..0520c7929a 100644 --- a/drivers/serial/serial_mxc.c +++ b/drivers/serial/serial_mxc.c @@ -310,7 +310,7 @@ static int mxc_serial_putc(struct udevice *dev, const char ch) struct mxc_serial_plat *plat = dev_get_plat(dev); struct mxc_uart *const uart = plat->reg;
- if (!(readl(&uart->ts) & UTS_TXEMPTY))
if (readl(&uart->ts) & UTS_TXFULL) return -EAGAIN;
writel(ch, &uart->txd);
participants (2)
-
Johannes Schneider
-
Peng Fan