[RFC] With 8250 Designware UART, if writes to the LCR failed the kernel will hung up

[RFC] With 8250 Designware UART, if writes to the LCR failed the kernel will hung up

[Zhang Zhen]

Hi,

	I'm testing 4.0-rc1 kernel on my board with 8250 Designware UART.(ARM Cortex-a15 single core).
	
	I found if serial is busy and writes to the LCR failed after tried 1000 times.
	The kernel will hung up.

	The system boot success after changed from:

 95 static void dw8250_serial_out(struct uart_port *p, int offset, int value)
 96 {
 97         struct dw8250_data *d = p->private_data;
 98
...
...
112                         writeb(value, p->membase + (UART_LCR << p->regshift));
113                 }
114                 dev_err(p->dev, "Couldn't set LCR to %d\n", value);
115         }
116 }

	to:

 95 static void dw8250_serial_out(struct uart_port *p, int offset, int value)
 96 {
 97         struct dw8250_data *d = p->private_data;
 98
...
...
112                         writeb(value, p->membase + (UART_LCR << p->regshift));
113                 }
114                 dev_info(p->dev, "Couldn't set LCR to %d\n", value);            //changed here
115         }
116 }

	The reason is serial8250_console_write can't get port->lock because serial8250_do_set_termios has
	got port->lock.
	So i think here we should change from dev_err to dev_info ?
	Any suggestions are welcome.

Best regards!

	

	

Re: [RFC] With 8250 Designware UART, if writes to the LCR failed the kernel will hung up

[Peter Hurley]

Hi Zhang,

On 03/06/2015 04:11 AM, Zhang Zhen wrote:
> Hi,
> 
> 	I'm testing 4.0-rc1 kernel on my board with 8250 Designware UART.(ARM Cortex-a15 single core).
> 	
> 	I found if serial is busy and writes to the LCR failed after tried 1000 times.
> 	The kernel will hung up.
> 
> 	The system boot success after changed from:
> 
>  95 static void dw8250_serial_out(struct uart_port *p, int offset, int value)
>  96 {
>  97         struct dw8250_data *d = p->private_data;
>  98
> ...
> ...
> 112                         writeb(value, p->membase + (UART_LCR << p->regshift));
> 113                 }
> 114                 dev_err(p->dev, "Couldn't set LCR to %d\n", value);
> 115         }
> 116 }
> 
> 	to:
> 
>  95 static void dw8250_serial_out(struct uart_port *p, int offset, int value)
>  96 {
>  97         struct dw8250_data *d = p->private_data;
>  98
> ...
> ...
> 112                         writeb(value, p->membase + (UART_LCR << p->regshift));
> 113                 }
> 114                 dev_info(p->dev, "Couldn't set LCR to %d\n", value);            //changed here
> 115         }
> 116 }
> 
> 	The reason is serial8250_console_write can't get port->lock because serial8250_do_set_termios has
> 	got port->lock.
> 	So i think here we should change from dev_err to dev_info ?

That's not really going to help because this will still hang if the
console_loglevel is set to < KERN_INFO.

> 	Any suggestions are welcome.

Check that the port is not the uart_console() before logging the error,
like;

	if (!uart_console(p))
		dev_err(p->dev, "Couldn't .....");

Use a global flag to note the error and check it from other contexts.
Plus, find out why you can't write LCR there.

Also, consider re-designing how the 8250_dw driver implements that
"feature".

Regards,
Peter Hurley

Re: [RFC] With 8250 Designware UART, if writes to the LCR failed the kernel will hung up

[Tim Kryger]

You only hit the silicon bug if you bombard the uart with characters
and simultaneously request a baud rate or framing change.

I'm not sure why you would do either to the uart console.  Is it
possible your host machine is doing something weird?

If you have the leverage, remind the SoC vendor to upgrade the serial
block.  Synopsys fixed this a long time ago.

-Tim

On Fri, Mar 6, 2015 at 8:50 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
> Hi Zhang,
>
> On 03/06/2015 04:11 AM, Zhang Zhen wrote:
>> Hi,
>>
>>       I'm testing 4.0-rc1 kernel on my board with 8250 Designware UART.(ARM Cortex-a15 single core).
>>
>>       I found if serial is busy and writes to the LCR failed after tried 1000 times.
>>       The kernel will hung up.
>>
>>       The system boot success after changed from:
>>
>>  95 static void dw8250_serial_out(struct uart_port *p, int offset, int value)
>>  96 {
>>  97         struct dw8250_data *d = p->private_data;
>>  98
>> ...
>> ...
>> 112                         writeb(value, p->membase + (UART_LCR << p->regshift));
>> 113                 }
>> 114                 dev_err(p->dev, "Couldn't set LCR to %d\n", value);
>> 115         }
>> 116 }
>>
>>       to:
>>
>>  95 static void dw8250_serial_out(struct uart_port *p, int offset, int value)
>>  96 {
>>  97         struct dw8250_data *d = p->private_data;
>>  98
>> ...
>> ...
>> 112                         writeb(value, p->membase + (UART_LCR << p->regshift));
>> 113                 }
>> 114                 dev_info(p->dev, "Couldn't set LCR to %d\n", value);            //changed here
>> 115         }
>> 116 }
>>
>>       The reason is serial8250_console_write can't get port->lock because serial8250_do_set_termios has
>>       got port->lock.
>>       So i think here we should change from dev_err to dev_info ?
>
> That's not really going to help because this will still hang if the
> console_loglevel is set to < KERN_INFO.
>
>>       Any suggestions are welcome.
>
> Check that the port is not the uart_console() before logging the error,
> like;
>
>         if (!uart_console(p))
>                 dev_err(p->dev, "Couldn't .....");
>
> Use a global flag to note the error and check it from other contexts.
> Plus, find out why you can't write LCR there.
>
> Also, consider re-designing how the 8250_dw driver implements that
> "feature".
>
> Regards,
> Peter Hurley
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC] With 8250 Designware UART, if writes to the LCR failed the kernel will hung up

[long.wanglong]

On 2015/3/7 11:01, Tim Kryger wrote:
> You only hit the silicon bug if you bombard the uart with characters
> and simultaneously request a baud rate or framing change.
> 
> I'm not sure why you would do either to the uart console.  Is it
> possible your host machine is doing something weird?
> 
> If you have the leverage, remind the SoC vendor to upgrade the serial
> block.  Synopsys fixed this a long time ago.
> 
> -Tim

Hi, Tim

thanks for you reply. if we bombard the uart with characters and simultaneously
request a baud rate change, we hit the silicon bug.

Our scenario is to do stability testing（at the system booting, we bombard the uart
with characters）, With this version(v3.0.6) Synopsys Serial,  the expect are
print out the message(Couldn't set LCR to xxxx) and the kernel will not hung.

Maybe the next release of the board we will upgrade the serial block to the new version.
but the issue is that how we circumvent this problem in kernel?

do you have any ideas? thanks.

Best Regards
Wang Long


> 
> On Fri, Mar 6, 2015 at 8:50 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
>> Hi Zhang,
>>
>> On 03/06/2015 04:11 AM, Zhang Zhen wrote:
>>> Hi,
>>>
>>>       I'm testing 4.0-rc1 kernel on my board with 8250 Designware UART.(ARM Cortex-a15 single core).
>>>
>>>       I found if serial is busy and writes to the LCR failed after tried 1000 times.
>>>       The kernel will hung up.
>>>
>>>       The system boot success after changed from:
>>>
>>>  95 static void dw8250_serial_out(struct uart_port *p, int offset, int value)
>>>  96 {
>>>  97         struct dw8250_data *d = p->private_data;
>>>  98
>>> ...
>>> ...
>>> 112                         writeb(value, p->membase + (UART_LCR << p->regshift));
>>> 113                 }
>>> 114                 dev_err(p->dev, "Couldn't set LCR to %d\n", value);
>>> 115         }
>>> 116 }
>>>
>>>       to:
>>>
>>>  95 static void dw8250_serial_out(struct uart_port *p, int offset, int value)
>>>  96 {
>>>  97         struct dw8250_data *d = p->private_data;
>>>  98
>>> ...
>>> ...
>>> 112                         writeb(value, p->membase + (UART_LCR << p->regshift));
>>> 113                 }
>>> 114                 dev_info(p->dev, "Couldn't set LCR to %d\n", value);            //changed here
>>> 115         }
>>> 116 }
>>>
>>>       The reason is serial8250_console_write can't get port->lock because serial8250_do_set_termios has
>>>       got port->lock.
>>>       So i think here we should change from dev_err to dev_info ?
>>
>> That's not really going to help because this will still hang if the
>> console_loglevel is set to < KERN_INFO.
>>
>>>       Any suggestions are welcome.
>>
>> Check that the port is not the uart_console() before logging the error,
>> like;
>>
>>         if (!uart_console(p))
>>                 dev_err(p->dev, "Couldn't .....");
>>
>> Use a global flag to note the error and check it from other contexts.
>> Plus, find out why you can't write LCR there.
>>
>> Also, consider re-designing how the 8250_dw driver implements that
>> "feature".
>>
>> Regards,
>> Peter Hurley
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/
> 
> .
> 

Re: [RFC] With 8250 Designware UART, if writes to the LCR failed the kernel will hung up

[Alan Cox]

> Maybe the next release of the board we will upgrade the serial block to the new version.
> but the issue is that how we circumvent this problem in kernel?

What is the official vendor workaround ?

Alan

Re: [RFC] With 8250 Designware UART, if writes to the LCR failed the kernel will hung up

[Tim Kryger]

On Mon, Mar 9, 2015 at 6:32 AM, Alan Cox <alan@linux.intel.com> wrote:
>> Maybe the next release of the board we will upgrade the serial block to the new version.
>> but the issue is that how we circumvent this problem in kernel?
>
> What is the official vendor workaround ?

They introduced a UART_16550_COMPATIBLE option for the IP which should
be selected.

If configured, writes to the LCR are allowed even if it is "busy" so
there is no need to retry.

More importantly there is no possibility of failing to write the LCR
after numerous retries.

-Tim

Re: [RFC] With 8250 Designware UART, if writes to the LCR failed the kernel will hung up

[Alan Cox]

On Mon, 2015-03-09 at 07:36 -0700, Tim Kryger wrote:
> On Mon, Mar 9, 2015 at 6:32 AM, Alan Cox <alan@linux.intel.com> wrote:
> >> Maybe the next release of the board we will upgrade the serial block to the new version.
> >> but the issue is that how we circumvent this problem in kernel?
> >
> > What is the official vendor workaround ?
> 
> They introduced a UART_16550_COMPATIBLE option for the IP which should
> be selected.
> 
> If configured, writes to the LCR are allowed even if it is "busy" so
> there is no need to retry.
> 
> More importantly there is no possibility of failing to write the LCR
> after numerous retries.


Ah no - I meant what is their official software workaround for existing
parts with the bug ? Presumably they have an errata document that
discusses this and the correct methods they recommend to avoid the
hang ?

Alan

Re: [RFC] With 8250 Designware UART, if writes to the LCR failed the kernel will hung up

[Tim Kryger]

On Mon, Mar 9, 2015 at 8:05 AM, Alan Cox <alan@linux.intel.com> wrote:

> Ah no - I meant what is their official software workaround for existing
> parts with the bug ? Presumably they have an errata document that
> discusses this and the correct methods they recommend to avoid the
> hang ?

As far as I know, the only advice they provided was rather naive.

The documentation I saw suggested stashing a copy of the LCR and then
rewriting it when the special LCR write failed interrupt was raised.

That approach was not workable as the LCR might be written while the
interrupt is masked causing the sequence of register writes to occur
in an order other than what was desired.

Additionally, when the LCR needed to be re-written but the UART stayed
busy, the interrupt would keep firing and the driver would starve out
everything else on the CPU.

The current workaround of clearing fifos and retrying a fixed number
of times isn't ideal but I'm not sure what else can be done given the
way this hardware works.

Additional background is in c49436b657d0a56a6ad90d14a7c3041add7cf64d

-Tim

Re: [RFC] With 8250 Designware UART, if writes to the LCR failed the kernel will hung up

[Zhang Zhen]

On 2015/3/10 10:47, Tim Kryger wrote:
> On Mon, Mar 9, 2015 at 8:05 AM, Alan Cox <alan@linux.intel.com> wrote:
> 
>> Ah no - I meant what is their official software workaround for existing
>> parts with the bug ? Presumably they have an errata document that
>> discusses this and the correct methods they recommend to avoid the
>> hang ?
> 
> As far as I know, the only advice they provided was rather naive.
> 
> The documentation I saw suggested stashing a copy of the LCR and then
> rewriting it when the special LCR write failed interrupt was raised.
> 
> That approach was not workable as the LCR might be written while the
> interrupt is masked causing the sequence of register writes to occur
> in an order other than what was desired.
> 
> Additionally, when the LCR needed to be re-written but the UART stayed
> busy, the interrupt would keep firing and the driver would starve out
> everything else on the CPU.
> 
> The current workaround of clearing fifos and retrying a fixed number
> of times isn't ideal but I'm not sure what else can be done given the
> way this hardware works.
> 
> Additional background is in c49436b657d0a56a6ad90d14a7c3041add7cf64d
> 
Hi,

If writes to the LCR failed and the dev_err in dw8250_serial_out lead to
the system hung up.
The root cause is that serial8250_console_write try to get port->lock which
has been got by serial8250_do_set_termios.

"Writes to the LCR are used to change the baud rate, parity, stop bit,
or data length so the data that may be lost is likely not important."
The dev_err in dw8250_serial_out must be needed? We can delete dev_err directly ?

Best regards!
> -Tim
> 
> .
> 

Re: [RFC] With 8250 Designware UART, if writes to the LCR failed the kernel will hung up

[Peter Hurley]

On 03/09/2015 10:47 PM, Tim Kryger wrote:
> The current workaround of clearing fifos and retrying a fixed number
> of times isn't ideal but I'm not sure what else can be done given the
> way this hardware works.

But hanging the machine is not an acceptable outcome.

Since the hang stems from the printk while already holding the port->lock,
until I can fix the console write, I think it's best to comment out
the dev_err() with a FIXME note.

The 8250 console write already checks port->sysrq to allow printk from
within the driver so this solution just needs to be generalized.
And there are debug uses of printk() from within the driver that are just
as broken, so this isn't the only use case. (I want to do something
similar for the au_serial-* i/o accessors too).

Regards,
Peter Hurley

Re: [RFC] With 8250 Designware UART, if writes to the LCR failed the kernel will hung up

[Zhang Zhen]

On 2015/3/10 21:25, Peter Hurley wrote:
> On 03/09/2015 10:47 PM, Tim Kryger wrote:
>> The current workaround of clearing fifos and retrying a fixed number
>> of times isn't ideal but I'm not sure what else can be done given the
>> way this hardware works.
> 
> But hanging the machine is not an acceptable outcome.
> 
> Since the hang stems from the printk while already holding the port->lock,
> until I can fix the console write, I think it's best to comment out
> the dev_err() with a FIXME note.

Yeah，i agree.
> 
> The 8250 console write already checks port->sysrq to allow printk from
> within the driver so this solution just needs to be generalized.
> And there are debug uses of printk() from within the driver that are just
> as broken, so this isn't the only use case. (I want to do something
> similar for the au_serial-* i/o accessors too).
> 
> Regards,
> Peter Hurley
> 
> 
> 
> .
> 

Re: [RFC] With 8250 Designware UART, if writes to the LCR failed the kernel will hung up

[Andy Shevchenko]

On Tue, Mar 10, 2015 at 4:47 AM, Tim Kryger <tim.kryger@gmail.com> wrote:
> On Mon, Mar 9, 2015 at 8:05 AM, Alan Cox <alan@linux.intel.com> wrote:
>
>> Ah no - I meant what is their official software workaround for existing
>> parts with the bug ? Presumably they have an errata document that
>> discusses this and the correct methods they recommend to avoid the
>> hang ?
>
> As far as I know, the only advice they provided was rather naive.
>
> The documentation I saw suggested stashing a copy of the LCR and then
> rewriting it when the special LCR write failed interrupt was raised.
>
> That approach was not workable as the LCR might be written while the
> interrupt is masked causing the sequence of register writes to occur
> in an order other than what was desired.
>
> Additionally, when the LCR needed to be re-written but the UART stayed
> busy, the interrupt would keep firing and the driver would starve out
> everything else on the CPU.
>
> The current workaround of clearing fifos and retrying a fixed number
> of times isn't ideal but I'm not sure what else can be done given the
> way this hardware works.
>
> Additional background is in c49436b657d0a56a6ad90d14a7c3041add7cf64d

I hit the similar problem as Zhang described quite long ago when I
tried to write to the port chosen as console.
In my case seems the same (DW) IP is enumerated via PCI. Do we need to
tweak PCI cases as well?


-- 
With Best Regards,
Andy Shevchenko

Re: [RFC] With 8250 Designware UART, if writes to the LCR failed the kernel will hung up

[Peter Hurley]

Hi Andy,

On 03/13/2015 11:36 AM, Andy Shevchenko wrote:
> On Tue, Mar 10, 2015 at 4:47 AM, Tim Kryger <tim.kryger@gmail.com> wrote:
>> On Mon, Mar 9, 2015 at 8:05 AM, Alan Cox <alan@linux.intel.com> wrote:
>>
>>> Ah no - I meant what is their official software workaround for existing
>>> parts with the bug ? Presumably they have an errata document that
>>> discusses this and the correct methods they recommend to avoid the
>>> hang ?
>>
>> As far as I know, the only advice they provided was rather naive.
>>
>> The documentation I saw suggested stashing a copy of the LCR and then
>> rewriting it when the special LCR write failed interrupt was raised.
>>
>> That approach was not workable as the LCR might be written while the
>> interrupt is masked causing the sequence of register writes to occur
>> in an order other than what was desired.
>>
>> Additionally, when the LCR needed to be re-written but the UART stayed
>> busy, the interrupt would keep firing and the driver would starve out
>> everything else on the CPU.
>>
>> The current workaround of clearing fifos and retrying a fixed number
>> of times isn't ideal but I'm not sure what else can be done given the
>> way this hardware works.
>>
>> Additional background is in c49436b657d0a56a6ad90d14a7c3041add7cf64d
> 
> I hit the similar problem as Zhang described quite long ago when I
> tried to write to the port chosen as console.
> In my case seems the same (DW) IP is enumerated via PCI. Do we need to
> tweak PCI cases as well?

Currently, 8250_dw only supports platform devices.

To support your use-case, the 8250_dw i/o accessors would need to be
set for that hardware. Otherwise, even if you get this hardware past
the PCI probe, it will lockup anytime the LCR is written while data is
still in the RX fifo.

Regards,
Peter Hurley