[U-Boot] [PATCH] imx: serial: Wait for ongoing transmission to finish before serial reset

[U-Boot] [PATCH] imx: serial: Wait for ongoing transmission to finish before serial reset

[Lukasz Majewski]

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))
+		;
+
 	writel(0, &base->cr1);
 	writel(0, &base->cr2);
 
-- 
2.1.4

[U-Boot] [PATCH] imx: serial: Wait for ongoing transmission to finish before serial reset

[Stefano Babic]

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


-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH] imx: serial: Wait for ongoing transmission to finish before serial reset

[Lothar Waßmann]

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

[U-Boot] [PATCH] imx: serial: Wait for ongoing transmission to finish before serial reset

[Łukasz Majewski]

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
> 


-- 
Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de

[U-Boot] [PATCH] imx: serial: Wait for ongoing transmission to finish before serial reset

[Simon Glass]

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

[U-Boot] [PATCH] imx: serial: Wait for ongoing transmission to finish before serial reset

[Łukasz Majewski]

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
> 


-- 
Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de

[U-Boot] [PATCH] imx: serial: Wait for ongoing transmission to finish before serial reset

[Simon Glass]

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