| Message ID | 1507022205-11877-1-git-send-email-lukma@denx.de |
|---|---|
| State | Superseded |
| Delegated to: | Stefano Babic |
| Headers | show |
| Series | [U-Boot] imx: serial: Wait for ongoing transmission to finish before serial reset | expand |
On 03/10/2017 11:16, Lukasz Majewski wrote: > It may happen that the MXC serial IP block is performing some ongoing > transmission (started at e.g. board_init()) when the "initr_serial" is > called. > > As a result the serial port IP block is reset, so transmitted data is > corrupted: > > I2C: ready > DRAM: 1 GiB > jSS('HH��SL_SDHC: 04 rev 0x0 > > This patch prevents from this situation, by waiting for transmission > complete bit set (UART Status Register 2 (UARTx_USR2), bit TXDC): > > I2C: ready > DRAM: 1 GiB > ID: unit type 0x4 rev 0x0 > > Signed-off-by: Lukasz Majewski <lukma@denx.de> > --- > > drivers/serial/serial_mxc.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c > index cce80a8..ef4eb12 100644 > --- a/drivers/serial/serial_mxc.c > +++ b/drivers/serial/serial_mxc.c > @@ -141,6 +141,13 @@ struct mxc_uart { > > static void _mxc_serial_init(struct mxc_uart *base) > { > + /* > + * Wait for any ongoing transmission to finish - for example > + * from pre-relocation enabled UART > + */ > + while (!(readl(&base->sr2) & USR2_TXDC)) > + ; Yes, the same is in kernel, too. Reviewed-by: Stefano Babic <sbabic@denx.de> Best regards, Stefano Babic
Hi, On Tue, 3 Oct 2017 11:16:45 +0200 Lukasz Majewski wrote: > It may happen that the MXC serial IP block is performing some ongoing > transmission (started at e.g. board_init()) when the "initr_serial" is > called. > > As a result the serial port IP block is reset, so transmitted data is > corrupted: > > I2C: ready > DRAM: 1 GiB > jSS('HH��SL_SDHC: 04 rev 0x0 > > This patch prevents from this situation, by waiting for transmission > complete bit set (UART Status Register 2 (UARTx_USR2), bit TXDC): > > I2C: ready > DRAM: 1 GiB > ID: unit type 0x4 rev 0x0 > > Signed-off-by: Lukasz Majewski <lukma@denx.de> > --- > > drivers/serial/serial_mxc.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c > index cce80a8..ef4eb12 100644 > --- a/drivers/serial/serial_mxc.c > +++ b/drivers/serial/serial_mxc.c > @@ -141,6 +141,13 @@ struct mxc_uart { > > static void _mxc_serial_init(struct mxc_uart *base) > { > + /* > + * Wait for any ongoing transmission to finish - for example > + * from pre-relocation enabled UART > + */ > + while (!(readl(&base->sr2) & USR2_TXDC)) > + ; > + > Loops that poll for HW activated bits should always have a timeout. Hardware will definitely break some day and deliver unexpected results. Software should cope with this as best as it can! Lothar Waßmann
HI Lothar, > Hi, > > On Tue, 3 Oct 2017 11:16:45 +0200 Lukasz Majewski wrote: >> It may happen that the MXC serial IP block is performing some ongoing >> transmission (started at e.g. board_init()) when the "initr_serial" is >> called. >> >> As a result the serial port IP block is reset, so transmitted data is >> corrupted: >> >> I2C: ready >> DRAM: 1 GiB >> jSS('HH��SL_SDHC: 04 rev 0x0 >> >> This patch prevents from this situation, by waiting for transmission >> complete bit set (UART Status Register 2 (UARTx_USR2), bit TXDC): >> >> I2C: ready >> DRAM: 1 GiB >> ID: unit type 0x4 rev 0x0 >> >> Signed-off-by: Lukasz Majewski <lukma@denx.de> >> --- >> >> drivers/serial/serial_mxc.c | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c >> index cce80a8..ef4eb12 100644 >> --- a/drivers/serial/serial_mxc.c >> +++ b/drivers/serial/serial_mxc.c >> @@ -141,6 +141,13 @@ struct mxc_uart { >> >> static void _mxc_serial_init(struct mxc_uart *base) >> { >> + /* >> + * Wait for any ongoing transmission to finish - for example >> + * from pre-relocation enabled UART >> + */ >> + while (!(readl(&base->sr2) & USR2_TXDC)) >> + ; >> + >> > Loops that poll for HW activated bits should always have a timeout. > Hardware will definitely break some day and deliver unexpected results. > Software should cope with this as best as it can! In principle yes. Please find below rationale for this patch: 1. According to imx6q this bit shows emptiness of TxFIFO and Shift register [1]. It seems like a purely HW controlled bit. 2. Having timeout here would require first initialization of timer. However, this code is also used in a very early console initialization. 3. In Linux kernel [2] the same check is performed with: while (!(readl(sport->port.membase + USR2) & USR2_TXDC)) barrier(); [1] - Transmitter Complete. Indicates that the transmit buffer (TxFIFO) and Shift Register is empty; therefore the transmission is complete. TXDC is cleared automatically when data is written to the TxFIFO. [2] - http://elixir.free-electrons.com/linux/v4.2/source/drivers/tty/serial/imx.c#L1379 > > > Lothar Waßmann >
Hi Lukasz, On 3 October 2017 at 03:16, Lukasz Majewski <lukma@denx.de> wrote: > It may happen that the MXC serial IP block is performing some ongoing > transmission (started at e.g. board_init()) when the "initr_serial" is > called. > > As a result the serial port IP block is reset, so transmitted data is > corrupted: > > I2C: ready > DRAM: 1 GiB > jSS('HH��SL_SDHC: 04 rev 0x0 > > This patch prevents from this situation, by waiting for transmission > complete bit set (UART Status Register 2 (UARTx_USR2), bit TXDC): > > I2C: ready > DRAM: 1 GiB > ID: unit type 0x4 rev 0x0 > > Signed-off-by: Lukasz Majewski <lukma@denx.de> > --- > > drivers/serial/serial_mxc.c | 7 +++++++ > 1 file changed, 7 insertions(+) Is it possible to use driver model to do this in a generic way for all serial drivers? The pending() method allows you to check if there are any characters in the output FIFO. Regards, Simon
Hi Simon, > Hi Lukasz, > > On 3 October 2017 at 03:16, Lukasz Majewski <lukma@denx.de> wrote: >> It may happen that the MXC serial IP block is performing some ongoing >> transmission (started at e.g. board_init()) when the "initr_serial" is >> called. >> >> As a result the serial port IP block is reset, so transmitted data is >> corrupted: >> >> I2C: ready >> DRAM: 1 GiB >> jSS('HH��SL_SDHC: 04 rev 0x0 >> >> This patch prevents from this situation, by waiting for transmission >> complete bit set (UART Status Register 2 (UARTx_USR2), bit TXDC): >> >> I2C: ready >> DRAM: 1 GiB >> ID: unit type 0x4 rev 0x0 >> >> Signed-off-by: Lukasz Majewski <lukma@denx.de> >> --- >> >> drivers/serial/serial_mxc.c | 7 +++++++ >> 1 file changed, 7 insertions(+) > > Is it possible to use driver model to do this in a generic way for all > serial drivers? The pending() method allows you to check if there are > any characters in the output FIFO. Please correct me if I'm wrong. Do you mean to define pre_init() callback in serial-uclass.c file, which would utilize ->pending callback if available? In that way we would have the generic facility for above check available on all platforms. > > Regards, > Simon >
Hi Lucasz, On 9 October 2017 at 12:46, Łukasz Majewski <lukma@denx.de> wrote: > Hi Simon, > >> Hi Lukasz, >> >> On 3 October 2017 at 03:16, Lukasz Majewski <lukma@denx.de> wrote: >>> >>> It may happen that the MXC serial IP block is performing some ongoing >>> transmission (started at e.g. board_init()) when the "initr_serial" is >>> called. >>> >>> As a result the serial port IP block is reset, so transmitted data is >>> corrupted: >>> >>> I2C: ready >>> DRAM: 1 GiB >>> jSS('HH��SL_SDHC: 04 rev 0x0 >>> >>> This patch prevents from this situation, by waiting for transmission >>> complete bit set (UART Status Register 2 (UARTx_USR2), bit TXDC): >>> >>> I2C: ready >>> DRAM: 1 GiB >>> ID: unit type 0x4 rev 0x0 >>> >>> Signed-off-by: Lukasz Majewski <lukma@denx.de> >>> --- >>> >>> drivers/serial/serial_mxc.c | 7 +++++++ >>> 1 file changed, 7 insertions(+) >> >> >> Is it possible to use driver model to do this in a generic way for all >> serial drivers? The pending() method allows you to check if there are >> any characters in the output FIFO. > > > Please correct me if I'm wrong. > > Do you mean to define pre_init() callback in serial-uclass.c file, which > would utilize ->pending callback if available? > > In that way we would have the generic facility for above check available on > all platforms. Yes that is what I am hoping. Regards, Simon
diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c index cce80a8..ef4eb12 100644 --- a/drivers/serial/serial_mxc.c +++ b/drivers/serial/serial_mxc.c @@ -141,6 +141,13 @@ struct mxc_uart { static void _mxc_serial_init(struct mxc_uart *base) { + /* + * Wait for any ongoing transmission to finish - for example + * from pre-relocation enabled UART + */ + while (!(readl(&base->sr2) & USR2_TXDC)) + ; + writel(0, &base->cr1); writel(0, &base->cr2);
It may happen that the MXC serial IP block is performing some ongoing transmission (started at e.g. board_init()) when the "initr_serial" is called. As a result the serial port IP block is reset, so transmitted data is corrupted: I2C: ready DRAM: 1 GiB jSS('HH��SL_SDHC: 04 rev 0x0 This patch prevents from this situation, by waiting for transmission complete bit set (UART Status Register 2 (UARTx_USR2), bit TXDC): I2C: ready DRAM: 1 GiB ID: unit type 0x4 rev 0x0 Signed-off-by: Lukasz Majewski <lukma@denx.de> --- drivers/serial/serial_mxc.c | 7 +++++++ 1 file changed, 7 insertions(+)