[PATCH v4 0/2] serial_mxc: fixing reception

[PATCH v4 0/2] serial_mxc: fixing reception

[Johannes Schneider]

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 v4:
- add 'reviewd-by'

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(-)

-- 
2.25.1

[PATCH v4 1/2] serial: mxc: enable the RX pipeline

[Johannes Schneider]

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 v4:
- add 'reviewd-by'

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);
 }
 
-- 
2.25.1

[PATCH v4 2/2] serial: mxc: have putc use the TXFIFO

[Johannes Schneider]

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@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);
-- 
2.25.1

Re: [PATCH v4 1/2] serial: mxc: enable the RX pipeline

[Fabio Estevam]

Hi Johannes,

On 06/09/2022 09:06, Johannes Schneider wrote:

> +
> +	/* 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);
> +

Nitpick: the recommended style for multi-line comments is:


/*
  * 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.
  */

Reviewed-by: Fabio Estevam <festevam@denx.de>

Re: [PATCH v4 2/2] serial: mxc: have putc use the TXFIFO

[Fabio Estevam]

On 06/09/2022 09:06, 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@nxp.com

Reviewed-by: Fabio Estevam <festevam@denx.de>