Re: [PATCH v2] serial: 8250_dw: Improve unwritable LCR workaround
On Tuesday 10 December 2013 09:11:55 Ezequiel Garcia wrote:
> Hi Tim, James:
>
> On Mon, Dec 09, 2013 at 04:42:23PM -0800, Tim Kryger wrote:
> > On Fri, Dec 6, 2013 at 4:59 PM, James Hogan
wrote:
> [..]
>
> > diff --git a/drivers/tty/serial/8250/8250_dw.c
> > b/drivers/tty/serial/8250/8250_dw.c
> > index 4658e3e..5f096c7 100644
> > --- a/drivers/tty/serial/8250/8250_dw.c
> > +++ b/drivers/tty/serial/8250/8250_dw.c
> > @@ -96,7 +96,8 @@ static void dw8250_serial_out(struct uart_port *p,
> > int offset, int value)
> >
> > if (offset == UART_LCR) {
> >
> > int tries = 1000;
> > while (tries--) {
> >
> > - if (value == p->serial_in(p, UART_LCR))
> > + unsigned int lcr = p->serial_in(p, UART_LCR);
> > + if ((value & ~UART_LCR_SPAR) == (lcr &
> > ~UART_LCR_SPAR))>
> > return;
> >
> > dw8250_force_idle(p);
> > writeb(value, p->membase + (UART_LCR <<
> > p->regshift));
> >
> > @@ -132,7 +133,8 @@ static void dw8250_serial_out32(struct uart_port
> > *p, int offset, int value)
> >
> > if (offset == UART_LCR) {
> >
> > int tries = 1000;
> > while (tries--) {
> >
> > - if (value == p->serial_in(p, UART_LCR))
> > + unsigned int lcr = p->serial_in(p, UART_LCR);
> > + if ((value & ~UART_LCR_SPAR) == (lcr &
> > ~UART_LCR_SPAR))>
> > return;
> >
> > dw8250_force_idle(p);
> > writel(value, p->membase + (UART_LCR <<
> > p->regshift));
> >
> > Would you mind posting this for proper review so we can get the fix in?
>
> Applying this change fixes all the problems I reported (just did a quick
> test on the board I have currently on my desk).
>
> Nice work!
Thanks for testing it.
I've posted Tim's version for proper review with both your tested-by.
Cheers
James
signature.asc
Description: This is a digitally signed message part.
Re: [PATCH v2] serial: 8250_dw: Improve unwritable LCR workaround
Hi Tim, James:
On Mon, Dec 09, 2013 at 04:42:23PM -0800, Tim Kryger wrote:
> On Fri, Dec 6, 2013 at 4:59 PM, James Hogan wrote:
>
[..]
> diff --git a/drivers/tty/serial/8250/8250_dw.c
> b/drivers/tty/serial/8250/8250_dw.c
> index 4658e3e..5f096c7 100644
> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c
> @@ -96,7 +96,8 @@ static void dw8250_serial_out(struct uart_port *p,
> int offset, int value)
> if (offset == UART_LCR) {
> int tries = 1000;
> while (tries--) {
> - if (value == p->serial_in(p, UART_LCR))
> + unsigned int lcr = p->serial_in(p, UART_LCR);
> + if ((value & ~UART_LCR_SPAR) == (lcr &
> ~UART_LCR_SPAR))
> return;
> dw8250_force_idle(p);
> writeb(value, p->membase + (UART_LCR << p->regshift));
> @@ -132,7 +133,8 @@ static void dw8250_serial_out32(struct uart_port
> *p, int offset, int value)
> if (offset == UART_LCR) {
> int tries = 1000;
> while (tries--) {
> - if (value == p->serial_in(p, UART_LCR))
> + unsigned int lcr = p->serial_in(p, UART_LCR);
> + if ((value & ~UART_LCR_SPAR) == (lcr &
> ~UART_LCR_SPAR))
> return;
> dw8250_force_idle(p);
> writel(value, p->membase + (UART_LCR << p->regshift));
>
>
> Would you mind posting this for proper review so we can get the fix in?
>
Applying this change fixes all the problems I reported (just did a quick test on
the board I have currently on my desk).
Nice work!
--
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@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: [PATCH v2] serial: 8250_dw: Improve unwritable LCR workaround
On Tuesday 10 December 2013 09:11:55 Ezequiel Garcia wrote:
Hi Tim, James:
On Mon, Dec 09, 2013 at 04:42:23PM -0800, Tim Kryger wrote:
On Fri, Dec 6, 2013 at 4:59 PM, James Hogan james.ho...@imgtec.com
wrote:
[..]
diff --git a/drivers/tty/serial/8250/8250_dw.c
b/drivers/tty/serial/8250/8250_dw.c
index 4658e3e..5f096c7 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -96,7 +96,8 @@ static void dw8250_serial_out(struct uart_port *p,
int offset, int value)
if (offset == UART_LCR) {
int tries = 1000;
while (tries--) {
- if (value == p-serial_in(p, UART_LCR))
+ unsigned int lcr = p-serial_in(p, UART_LCR);
+ if ((value ~UART_LCR_SPAR) == (lcr
~UART_LCR_SPAR))
return;
dw8250_force_idle(p);
writeb(value, p-membase + (UART_LCR
p-regshift));
@@ -132,7 +133,8 @@ static void dw8250_serial_out32(struct uart_port
*p, int offset, int value)
if (offset == UART_LCR) {
int tries = 1000;
while (tries--) {
- if (value == p-serial_in(p, UART_LCR))
+ unsigned int lcr = p-serial_in(p, UART_LCR);
+ if ((value ~UART_LCR_SPAR) == (lcr
~UART_LCR_SPAR))
return;
dw8250_force_idle(p);
writel(value, p-membase + (UART_LCR
p-regshift));
Would you mind posting this for proper review so we can get the fix in?
Applying this change fixes all the problems I reported (just did a quick
test on the board I have currently on my desk).
Nice work!
Thanks for testing it.
I've posted Tim's version for proper review with both your tested-by.
Cheers
James
signature.asc
Description: This is a digitally signed message part.
Re: [PATCH v2] serial: 8250_dw: Improve unwritable LCR workaround
Hi Tim, James:
On Mon, Dec 09, 2013 at 04:42:23PM -0800, Tim Kryger wrote:
On Fri, Dec 6, 2013 at 4:59 PM, James Hogan james.ho...@imgtec.com wrote:
[..]
diff --git a/drivers/tty/serial/8250/8250_dw.c
b/drivers/tty/serial/8250/8250_dw.c
index 4658e3e..5f096c7 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -96,7 +96,8 @@ static void dw8250_serial_out(struct uart_port *p,
int offset, int value)
if (offset == UART_LCR) {
int tries = 1000;
while (tries--) {
- if (value == p-serial_in(p, UART_LCR))
+ unsigned int lcr = p-serial_in(p, UART_LCR);
+ if ((value ~UART_LCR_SPAR) == (lcr
~UART_LCR_SPAR))
return;
dw8250_force_idle(p);
writeb(value, p-membase + (UART_LCR p-regshift));
@@ -132,7 +133,8 @@ static void dw8250_serial_out32(struct uart_port
*p, int offset, int value)
if (offset == UART_LCR) {
int tries = 1000;
while (tries--) {
- if (value == p-serial_in(p, UART_LCR))
+ unsigned int lcr = p-serial_in(p, UART_LCR);
+ if ((value ~UART_LCR_SPAR) == (lcr
~UART_LCR_SPAR))
return;
dw8250_force_idle(p);
writel(value, p-membase + (UART_LCR p-regshift));
Would you mind posting this for proper review so we can get the fix in?
Applying this change fixes all the problems I reported (just did a quick test on
the board I have currently on my desk).
Nice work!
--
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@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: [PATCH v2] serial: 8250_dw: Improve unwritable LCR workaround
On Fri, Dec 6, 2013 at 4:59 PM, James Hogan wrote:
> It appears to work with ~0x20 too, and the workaround isn't getting hit (only
> tested boot and logging in - nothing fancy). I think having the printks in
> this code with the console directed at the serial must have caused
> resursion/busy problems somehow.
James,
I tested tested the code you proposed (cleaned up to avoid magic
numbers) on my hardware and it works fine.
diff --git a/drivers/tty/serial/8250/8250_dw.c
b/drivers/tty/serial/8250/8250_dw.c
index 4658e3e..5f096c7 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -96,7 +96,8 @@ static void dw8250_serial_out(struct uart_port *p,
int offset, int value)
if (offset == UART_LCR) {
int tries = 1000;
while (tries--) {
- if (value == p->serial_in(p, UART_LCR))
+ unsigned int lcr = p->serial_in(p, UART_LCR);
+ if ((value & ~UART_LCR_SPAR) == (lcr & ~UART_LCR_SPAR))
return;
dw8250_force_idle(p);
writeb(value, p->membase + (UART_LCR << p->regshift));
@@ -132,7 +133,8 @@ static void dw8250_serial_out32(struct uart_port
*p, int offset, int value)
if (offset == UART_LCR) {
int tries = 1000;
while (tries--) {
- if (value == p->serial_in(p, UART_LCR))
+ unsigned int lcr = p->serial_in(p, UART_LCR);
+ if ((value & ~UART_LCR_SPAR) == (lcr & ~UART_LCR_SPAR))
return;
dw8250_force_idle(p);
writel(value, p->membase + (UART_LCR << p->regshift));
Would you mind posting this for proper review so we can get the fix in?
Thanks,
Tim Kryger
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@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: [PATCH v2] serial: 8250_dw: Improve unwritable LCR workaround
On Fri, Dec 6, 2013 at 4:59 PM, James Hogan james.ho...@imgtec.com wrote:
It appears to work with ~0x20 too, and the workaround isn't getting hit (only
tested boot and logging in - nothing fancy). I think having the printks in
this code with the console directed at the serial must have caused
resursion/busy problems somehow.
James,
I tested tested the code you proposed (cleaned up to avoid magic
numbers) on my hardware and it works fine.
diff --git a/drivers/tty/serial/8250/8250_dw.c
b/drivers/tty/serial/8250/8250_dw.c
index 4658e3e..5f096c7 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -96,7 +96,8 @@ static void dw8250_serial_out(struct uart_port *p,
int offset, int value)
if (offset == UART_LCR) {
int tries = 1000;
while (tries--) {
- if (value == p-serial_in(p, UART_LCR))
+ unsigned int lcr = p-serial_in(p, UART_LCR);
+ if ((value ~UART_LCR_SPAR) == (lcr ~UART_LCR_SPAR))
return;
dw8250_force_idle(p);
writeb(value, p-membase + (UART_LCR p-regshift));
@@ -132,7 +133,8 @@ static void dw8250_serial_out32(struct uart_port
*p, int offset, int value)
if (offset == UART_LCR) {
int tries = 1000;
while (tries--) {
- if (value == p-serial_in(p, UART_LCR))
+ unsigned int lcr = p-serial_in(p, UART_LCR);
+ if ((value ~UART_LCR_SPAR) == (lcr ~UART_LCR_SPAR))
return;
dw8250_force_idle(p);
writel(value, p-membase + (UART_LCR p-regshift));
Would you mind posting this for proper review so we can get the fix in?
Thanks,
Tim Kryger
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@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: [PATCH v2] serial: 8250_dw: Improve unwritable LCR workaround
On Friday 06 December 2013 16:31:38 Tim Kryger wrote:
> On Fri, Dec 6, 2013 at 3:51 PM, James Hogan wrote:
> > On Friday 06 December 2013 23:29:02 James Hogan wrote:
> >> So it looks like the LCR does always change immediately for me in this
> >> case
> >> (obviously it hasn't hit the BUSY case), but not all the bits can be
> >> written. In particular bit 5 and bit 7 at the least. If I do this (sorry
> >> for whitespace munging):
> >>
> >> diff --git a/drivers/tty/serial/8250/8250_dw.c
> >> b/drivers/tty/serial/8250/8250_dw.c
> >> index 4658e3e..722d448 100644
> >> --- a/drivers/tty/serial/8250/8250_dw.c
> >> +++ b/drivers/tty/serial/8250/8250_dw.c
> >> @@ -96,7 +96,7 @@ static void dw8250_serial_out(struct uart_port *p, int
> >> offset, int value)
> >>
> >> if (offset == UART_LCR) {
> >>
> >> int tries = 1000;
> >> while (tries--) {
> >>
> >> - if (value == p->serial_in(p, UART_LCR))
> >> + if (value & ~0xa0 == p->serial_in(p, UART_LCR) &
> >> ~0xa0)>>
> >> return;
> >>
> >> dw8250_force_idle(p);
> >> writeb(value, p->membase + (UART_LCR <<
> >> p->regshift));
> >>
> >> @@ -132,7 +132,7 @@ static void dw8250_serial_out32(struct uart_port *p,
> >> int offset, int value)
> >>
> >> if (offset == UART_LCR) {
> >>
> >> int tries = 1000;
> >> while (tries--) {
> >>
> >> - if (value == p->serial_in(p, UART_LCR))
> >> + if (value & ~0xa0 == p->serial_in(p, UART_LCR) &
> >> ~0xa0)>
> > My appologies, that should have had some more brackets (I should have
> > retested after cleaning up my debugging). I.e.
> > + if ((value & ~0xa0) == (p->serial_in(p, UART_LCR)
> > & ~0xa0))
> >
> > Cheers
> > James
>
> James,
>
> Thanks for the information. This is really helpful.
>
> You are right about bit 5 being a problem. Its behavior differs
> between IP versions 3.00a and 3.14c per the docs.
>
> As for bit 7, when it doesn't change this indicates the UART was busy
> and we need to do the workaround.
>
> Would you mind testing it again using a mask of ~0x20?
It appears to work with ~0x20 too, and the workaround isn't getting hit (only
tested boot and logging in - nothing fancy). I think having the printks in
this code with the console directed at the serial must have caused
resursion/busy problems somehow.
Cheers
James
signature.asc
Description: This is a digitally signed message part.
Re: [PATCH v2] serial: 8250_dw: Improve unwritable LCR workaround
On Fri, Dec 6, 2013 at 3:51 PM, James Hogan wrote:
> On Friday 06 December 2013 23:29:02 James Hogan wrote:
>> So it looks like the LCR does always change immediately for me in this case
>> (obviously it hasn't hit the BUSY case), but not all the bits can be
>> written. In particular bit 5 and bit 7 at the least. If I do this (sorry
>> for whitespace munging):
>>
>> diff --git a/drivers/tty/serial/8250/8250_dw.c
>> b/drivers/tty/serial/8250/8250_dw.c
>> index 4658e3e..722d448 100644
>> --- a/drivers/tty/serial/8250/8250_dw.c
>> +++ b/drivers/tty/serial/8250/8250_dw.c
>> @@ -96,7 +96,7 @@ static void dw8250_serial_out(struct uart_port *p, int
>> offset, int value)
>> if (offset == UART_LCR) {
>> int tries = 1000;
>> while (tries--) {
>> - if (value == p->serial_in(p, UART_LCR))
>> + if (value & ~0xa0 == p->serial_in(p, UART_LCR) & ~0xa0)
>> return;
>> dw8250_force_idle(p);
>> writeb(value, p->membase + (UART_LCR << p->regshift));
>> @@ -132,7 +132,7 @@ static void dw8250_serial_out32(struct uart_port *p, int
>> offset, int value)
>> if (offset == UART_LCR) {
>> int tries = 1000;
>> while (tries--) {
>> - if (value == p->serial_in(p, UART_LCR))
>> + if (value & ~0xa0 == p->serial_in(p, UART_LCR) & ~0xa0)
>
> My appologies, that should have had some more brackets (I should have retested
> after cleaning up my debugging). I.e.
> + if ((value & ~0xa0) == (p->serial_in(p, UART_LCR) &
> ~0xa0))
>
> Cheers
> James
James,
Thanks for the information. This is really helpful.
You are right about bit 5 being a problem. Its behavior differs
between IP versions 3.00a and 3.14c per the docs.
As for bit 7, when it doesn't change this indicates the UART was busy
and we need to do the workaround.
Would you mind testing it again using a mask of ~0x20?
Thanks,
Tim
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@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: [PATCH v2] serial: 8250_dw: Improve unwritable LCR workaround
On Friday 06 December 2013 23:29:02 James Hogan wrote:
> So it looks like the LCR does always change immediately for me in this case
> (obviously it hasn't hit the BUSY case), but not all the bits can be
> written. In particular bit 5 and bit 7 at the least. If I do this (sorry
> for whitespace munging):
>
> diff --git a/drivers/tty/serial/8250/8250_dw.c
> b/drivers/tty/serial/8250/8250_dw.c
> index 4658e3e..722d448 100644
> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c
> @@ -96,7 +96,7 @@ static void dw8250_serial_out(struct uart_port *p, int
> offset, int value)
> if (offset == UART_LCR) {
> int tries = 1000;
> while (tries--) {
> - if (value == p->serial_in(p, UART_LCR))
> + if (value & ~0xa0 == p->serial_in(p, UART_LCR) & ~0xa0)
> return;
> dw8250_force_idle(p);
> writeb(value, p->membase + (UART_LCR << p->regshift));
> @@ -132,7 +132,7 @@ static void dw8250_serial_out32(struct uart_port *p, int
> offset, int value)
> if (offset == UART_LCR) {
> int tries = 1000;
> while (tries--) {
> - if (value == p->serial_in(p, UART_LCR))
> + if (value & ~0xa0 == p->serial_in(p, UART_LCR) & ~0xa0)
My appologies, that should have had some more brackets (I should have retested
after cleaning up my debugging). I.e.
+ if ((value & ~0xa0) == (p->serial_in(p, UART_LCR) &
~0xa0))
Cheers
James
signature.asc
Description: This is a digitally signed message part.
Re: [PATCH v2] serial: 8250_dw: Improve unwritable LCR workaround
On Tuesday 26 November 2013 15:36:00 Ezequiel Garcia wrote:
> Hello,
>
> On Tue, Oct 01, 2013 at 10:18:08AM -0700, Tim Kryger wrote:
> > When configured with UART_16550_COMPATIBLE=NO or in versions prior to
> > the introduction of this option, the Designware UART will ignore writes
> > to the LCR if the UART is busy. The current workaround saves a copy of
> > the last written LCR and re-writes it in the ISR for a special interrupt
> > that is raised when a write was ignored.
> >
> > Unfortunately, interrupts are typically disabled prior to performing a
> > sequence of register writes that include the LCR so the point at which
> > the retry occurs is too late. An example is serial8250_do_set_termios()
> > where an ignored LCR write results in the baud divisor not being set and
> > instead a garbage character is sent out the transmitter.
> >
> > Furthermore, since serial_port_out() offers no way to indicate failure,
> > a serious effort must be made to ensure that the LCR is actually updated
> > before returning back to the caller. This is difficult, however, as a
> > UART that was busy during the first attempt is likely to still be busy
> > when a subsequent attempt is made unless some extra action is taken.
> >
> > This updated workaround reads back the LCR after each write to confirm
> > that the new value was accepted by the hardware. Should the hardware
> > ignore a write, the TX/RX FIFOs are cleared and the receive buffer read
> > before attempting to rewrite the LCR out of the hope that doing so will
> > force the UART into an idle state. While this may seem unnecessarily
> > aggressive, 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. Admittedly, this is far from ideal but it seems to be the
> > best that can be done given the hardware limitations.
> >
> > Lastly, the revised workaround doesn't touch the LCR in the ISR, so it
> > avoids the possibility of a "serial8250: too much work for irq" lock up.
> > This problem is rare in real situations but can be reproduced easily by
> > wiring up two UARTs and running the following commands.
> >
> > # stty -F /dev/ttyS1 echo
> > # stty -F /dev/ttyS2 echo
> > # cat /dev/ttyS1 &
> > [1] 375
> > # echo asdf > /dev/ttyS1
> > asdf
> >
> > [ 27.70] serial8250: too much work for irq96
> > [ 27.70] serial8250: too much work for irq96
> > [ 27.71] serial8250: too much work for irq96
> > [ 27.71] serial8250: too much work for irq96
> > [ 27.72] serial8250: too much work for irq96
> > [ 27.72] serial8250: too much work for irq96
> > [ 27.73] serial8250: too much work for irq96
> > [ 27.73] serial8250: too much work for irq96
> > [ 27.74] serial8250: too much work for irq96
> >
> > Signed-off-by: Tim Kryger
> > Reviewed-by: Matt Porter
> > Reviewed-by: Markus Mayer
> > ---
> >
> > Changes in v2:
> > - Rebased on tty-next
> > - Updated commit messsage to mention UART_16550_COMPATIBLE
> > - Removed potentially unnecessary read of LSR and MSR
> > - Only attempt workaround when LCR write is ignored
> >
> > drivers/tty/serial/8250/8250_dw.c | 41
> > ++- 1 file changed, 32
> > insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/tty/serial/8250/8250_dw.c
> > b/drivers/tty/serial/8250/8250_dw.c index d04a037..4658e3e 100644
> > --- a/drivers/tty/serial/8250/8250_dw.c
> > +++ b/drivers/tty/serial/8250/8250_dw.c
> > @@ -57,7 +57,6 @@
> >
> > struct dw8250_data {
> >
> > u8 usr_reg;
> >
> > - int last_lcr;
> >
> > int last_mcr;
> > int line;
> > struct clk *clk;
> >
> > @@ -77,17 +76,33 @@ static inline int dw8250_modify_msr(struct uart_port
> > *p, int offset, int value)>
> > return value;
> >
> > }
> >
> > +static void dw8250_force_idle(struct uart_port *p)
> > +{
> > + serial8250_clear_and_reinit_fifos(container_of
> > + (p, struct uart_8250_port, port));
> > + (void)p->serial_in(p, UART_RX);
> > +}
> > +
> >
> > static void dw8250_serial_out(struct uart_port *p, int offset, int value)
> > {
> >
> > struct dw8250_data *d = p->private_data;
> >
> > - if (offset == UART_LCR)
> > - d->last_lcr = value;
> > -
> >
> > if (offset == UART_MCR)
> >
> > d->last_mcr = value;
> >
> > writeb(value, p->membase + (offset << p->regshift));
> >
> > +
> > + /* Make sure LCR write wasn't ignored */
> > + if (offset == UART_LCR) {
> > + int tries = 1000;
> > + while (tries--) {
> > + if (value == p->serial_in(p, UART_LCR))
> > + return;
> > + dw8250_force_idle(p);
> > + writeb(value, p->membase + (UART_LCR <<
Re: [PATCH v2] serial: 8250_dw: Improve unwritable LCR workaround
On Tuesday 26 November 2013 15:36:00 Ezequiel Garcia wrote:
Hello,
On Tue, Oct 01, 2013 at 10:18:08AM -0700, Tim Kryger wrote:
When configured with UART_16550_COMPATIBLE=NO or in versions prior to
the introduction of this option, the Designware UART will ignore writes
to the LCR if the UART is busy. The current workaround saves a copy of
the last written LCR and re-writes it in the ISR for a special interrupt
that is raised when a write was ignored.
Unfortunately, interrupts are typically disabled prior to performing a
sequence of register writes that include the LCR so the point at which
the retry occurs is too late. An example is serial8250_do_set_termios()
where an ignored LCR write results in the baud divisor not being set and
instead a garbage character is sent out the transmitter.
Furthermore, since serial_port_out() offers no way to indicate failure,
a serious effort must be made to ensure that the LCR is actually updated
before returning back to the caller. This is difficult, however, as a
UART that was busy during the first attempt is likely to still be busy
when a subsequent attempt is made unless some extra action is taken.
This updated workaround reads back the LCR after each write to confirm
that the new value was accepted by the hardware. Should the hardware
ignore a write, the TX/RX FIFOs are cleared and the receive buffer read
before attempting to rewrite the LCR out of the hope that doing so will
force the UART into an idle state. While this may seem unnecessarily
aggressive, 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. Admittedly, this is far from ideal but it seems to be the
best that can be done given the hardware limitations.
Lastly, the revised workaround doesn't touch the LCR in the ISR, so it
avoids the possibility of a serial8250: too much work for irq lock up.
This problem is rare in real situations but can be reproduced easily by
wiring up two UARTs and running the following commands.
# stty -F /dev/ttyS1 echo
# stty -F /dev/ttyS2 echo
# cat /dev/ttyS1
[1] 375
# echo asdf /dev/ttyS1
asdf
[ 27.70] serial8250: too much work for irq96
[ 27.70] serial8250: too much work for irq96
[ 27.71] serial8250: too much work for irq96
[ 27.71] serial8250: too much work for irq96
[ 27.72] serial8250: too much work for irq96
[ 27.72] serial8250: too much work for irq96
[ 27.73] serial8250: too much work for irq96
[ 27.73] serial8250: too much work for irq96
[ 27.74] serial8250: too much work for irq96
Signed-off-by: Tim Kryger tim.kry...@linaro.org
Reviewed-by: Matt Porter matt.por...@linaro.org
Reviewed-by: Markus Mayer markus.ma...@linaro.org
---
Changes in v2:
- Rebased on tty-next
- Updated commit messsage to mention UART_16550_COMPATIBLE
- Removed potentially unnecessary read of LSR and MSR
- Only attempt workaround when LCR write is ignored
drivers/tty/serial/8250/8250_dw.c | 41
++- 1 file changed, 32
insertions(+), 9 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_dw.c
b/drivers/tty/serial/8250/8250_dw.c index d04a037..4658e3e 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -57,7 +57,6 @@
struct dw8250_data {
u8 usr_reg;
- int last_lcr;
int last_mcr;
int line;
struct clk *clk;
@@ -77,17 +76,33 @@ static inline int dw8250_modify_msr(struct uart_port
*p, int offset, int value)
return value;
}
+static void dw8250_force_idle(struct uart_port *p)
+{
+ serial8250_clear_and_reinit_fifos(container_of
+ (p, struct uart_8250_port, port));
+ (void)p-serial_in(p, UART_RX);
+}
+
static void dw8250_serial_out(struct uart_port *p, int offset, int value)
{
struct dw8250_data *d = p-private_data;
- if (offset == UART_LCR)
- d-last_lcr = value;
-
if (offset == UART_MCR)
d-last_mcr = value;
writeb(value, p-membase + (offset p-regshift));
+
+ /* Make sure LCR write wasn't ignored */
+ if (offset == UART_LCR) {
+ int tries = 1000;
+ while (tries--) {
+ if (value == p-serial_in(p, UART_LCR))
+ return;
+ dw8250_force_idle(p);
+ writeb(value, p-membase + (UART_LCR p-regshift));
+ }
+ dev_err(p-dev, Couldn't set LCR to %d\n, value);
+ }
}
static unsigned int dw8250_serial_in(struct uart_port *p, int offset)
@@ -108,13
Re: [PATCH v2] serial: 8250_dw: Improve unwritable LCR workaround
On Friday 06 December 2013 23:29:02 James Hogan wrote:
So it looks like the LCR does always change immediately for me in this case
(obviously it hasn't hit the BUSY case), but not all the bits can be
written. In particular bit 5 and bit 7 at the least. If I do this (sorry
for whitespace munging):
diff --git a/drivers/tty/serial/8250/8250_dw.c
b/drivers/tty/serial/8250/8250_dw.c
index 4658e3e..722d448 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -96,7 +96,7 @@ static void dw8250_serial_out(struct uart_port *p, int
offset, int value)
if (offset == UART_LCR) {
int tries = 1000;
while (tries--) {
- if (value == p-serial_in(p, UART_LCR))
+ if (value ~0xa0 == p-serial_in(p, UART_LCR) ~0xa0)
return;
dw8250_force_idle(p);
writeb(value, p-membase + (UART_LCR p-regshift));
@@ -132,7 +132,7 @@ static void dw8250_serial_out32(struct uart_port *p, int
offset, int value)
if (offset == UART_LCR) {
int tries = 1000;
while (tries--) {
- if (value == p-serial_in(p, UART_LCR))
+ if (value ~0xa0 == p-serial_in(p, UART_LCR) ~0xa0)
My appologies, that should have had some more brackets (I should have retested
after cleaning up my debugging). I.e.
+ if ((value ~0xa0) == (p-serial_in(p, UART_LCR)
~0xa0))
Cheers
James
signature.asc
Description: This is a digitally signed message part.
Re: [PATCH v2] serial: 8250_dw: Improve unwritable LCR workaround
On Fri, Dec 6, 2013 at 3:51 PM, James Hogan james.ho...@imgtec.com wrote:
On Friday 06 December 2013 23:29:02 James Hogan wrote:
So it looks like the LCR does always change immediately for me in this case
(obviously it hasn't hit the BUSY case), but not all the bits can be
written. In particular bit 5 and bit 7 at the least. If I do this (sorry
for whitespace munging):
diff --git a/drivers/tty/serial/8250/8250_dw.c
b/drivers/tty/serial/8250/8250_dw.c
index 4658e3e..722d448 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -96,7 +96,7 @@ static void dw8250_serial_out(struct uart_port *p, int
offset, int value)
if (offset == UART_LCR) {
int tries = 1000;
while (tries--) {
- if (value == p-serial_in(p, UART_LCR))
+ if (value ~0xa0 == p-serial_in(p, UART_LCR) ~0xa0)
return;
dw8250_force_idle(p);
writeb(value, p-membase + (UART_LCR p-regshift));
@@ -132,7 +132,7 @@ static void dw8250_serial_out32(struct uart_port *p, int
offset, int value)
if (offset == UART_LCR) {
int tries = 1000;
while (tries--) {
- if (value == p-serial_in(p, UART_LCR))
+ if (value ~0xa0 == p-serial_in(p, UART_LCR) ~0xa0)
My appologies, that should have had some more brackets (I should have retested
after cleaning up my debugging). I.e.
+ if ((value ~0xa0) == (p-serial_in(p, UART_LCR)
~0xa0))
Cheers
James
James,
Thanks for the information. This is really helpful.
You are right about bit 5 being a problem. Its behavior differs
between IP versions 3.00a and 3.14c per the docs.
As for bit 7, when it doesn't change this indicates the UART was busy
and we need to do the workaround.
Would you mind testing it again using a mask of ~0x20?
Thanks,
Tim
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@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: [PATCH v2] serial: 8250_dw: Improve unwritable LCR workaround
On Friday 06 December 2013 16:31:38 Tim Kryger wrote:
On Fri, Dec 6, 2013 at 3:51 PM, James Hogan james.ho...@imgtec.com wrote:
On Friday 06 December 2013 23:29:02 James Hogan wrote:
So it looks like the LCR does always change immediately for me in this
case
(obviously it hasn't hit the BUSY case), but not all the bits can be
written. In particular bit 5 and bit 7 at the least. If I do this (sorry
for whitespace munging):
diff --git a/drivers/tty/serial/8250/8250_dw.c
b/drivers/tty/serial/8250/8250_dw.c
index 4658e3e..722d448 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -96,7 +96,7 @@ static void dw8250_serial_out(struct uart_port *p, int
offset, int value)
if (offset == UART_LCR) {
int tries = 1000;
while (tries--) {
- if (value == p-serial_in(p, UART_LCR))
+ if (value ~0xa0 == p-serial_in(p, UART_LCR)
~0xa0)
return;
dw8250_force_idle(p);
writeb(value, p-membase + (UART_LCR
p-regshift));
@@ -132,7 +132,7 @@ static void dw8250_serial_out32(struct uart_port *p,
int offset, int value)
if (offset == UART_LCR) {
int tries = 1000;
while (tries--) {
- if (value == p-serial_in(p, UART_LCR))
+ if (value ~0xa0 == p-serial_in(p, UART_LCR)
~0xa0)
My appologies, that should have had some more brackets (I should have
retested after cleaning up my debugging). I.e.
+ if ((value ~0xa0) == (p-serial_in(p, UART_LCR)
~0xa0))
Cheers
James
James,
Thanks for the information. This is really helpful.
You are right about bit 5 being a problem. Its behavior differs
between IP versions 3.00a and 3.14c per the docs.
As for bit 7, when it doesn't change this indicates the UART was busy
and we need to do the workaround.
Would you mind testing it again using a mask of ~0x20?
It appears to work with ~0x20 too, and the workaround isn't getting hit (only
tested boot and logging in - nothing fancy). I think having the printks in
this code with the console directed at the serial must have caused
resursion/busy problems somehow.
Cheers
James
signature.asc
Description: This is a digitally signed message part.
Re: [PATCH v2] serial: 8250_dw: Improve unwritable LCR workaround
On Wed, Dec 4, 2013 at 5:01 AM, Ezequiel Garcia wrote: > On Thu, Nov 28, 2013 at 04:53:37PM -0300, Ezequiel Garcia wrote: >> On Thu, Nov 28, 2013 at 04:47:20PM -0300, Ezequiel Garcia wrote: >> > Changing the console port by setting "console=ttyS1,115200" gives this: >> > >> > [..] >> > Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled >> > dw-apb-uart d0012000.serial: Couldn't set LCR to 191 >> > dw-apb-uart d0012000.serial: Couldn't set LCR to 191 >> > dw-apb-uart d0012000.serial: Couldn't set LCR to 224 >> > dw-apb-uart d0012000.serial: Couldn't set LCR to 224 >> > d0012000.serial: ttyS0 at MMIO 0xd0012000 (irq = 17, base_baud = 15625000) >> > is a 16550A >> > dw-apb-uart d0012100.serial: Couldn't set LCR to 191 >> > dw-apb-uart d0012100.serial: Couldn't set LCR to 191 >> > dw-apb-uart d0012100.serial: Couldn't set LCR to 224 >> > dw-apb-uart d0012100.serial: Couldn't set LCR to 224 >> > d0012100.serial: ttyS1 at MMIO 0xd0012100 (irq = 18, base_baud = 15625000) >> > is a 16550A >> > console [ttyS1] enabled >> > dw-apb-uart d0012200.serial: Couldn't set LCR to 191 >> > dw-apb-uart d0012200.serial: Couldn't set LCR to 191 >> > dw-apb-uart d0012200.serial: Couldn't set LCR to 224 >> > dw-apb-uart d0012200.serial: Couldn't set LCR to 224 >> > d0012200.serial: ttyS2 at MMIO 0xd0012200 (irq = 31, base_baud = 15625000) >> > is a 16550A >> > dw-apb-uart d0012300.serial: Couldn't set LCR to 191 >> > dw-apb-uart d0012300.serial: Couldn't set LCR to 191 >> > dw-apb-uart d0012300.serial: Couldn't set LCR to 224 >> > dw-apb-uart d0012300.serial: Couldn't set LCR to 224 >> > d0012300.serial: ttyS3 at MMIO 0xd0012300 (irq = 32, base_baud = 15625000) >> > is a 16550A >> > >> > So we get the "Couldn't set" message in all four ports. >> > >> > Tim: Any ideas? >> >> And another thing: the weird output on the console looks related to the >> early boot console. If I enable 'earlyprintk' on ttyS0 but set the console >> on ttyS1, this is what I get on ttyS0: >> >> bootconsole [earlycon0] enabled >> [..] >> Kernel command line: earlyprintk console=ttyS1,115200 root=/dev/nfs rw >> nfsroot=192.168.0.45:/opt/buildrootfs,v3, >> ip=192.168.0.159:192.168.0.45:192.168.0.1:255.255.255.0:develboard:eth0:on >> rootwait >> [..] >> Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled >> �Ɋ��ɁɆ� >> Welcome to Buildroot >> buildroot login: >> >> Hope this helps you understand what's going on... > > Gentle ping? > > Any ideas about those weird characters? If there was just one weird character, I would say it was an indication that hardware rejected a write to LCR and then software wrote the lower 8 bits of the baud into DLL which happens to live at the same address offset as RBR. However, there are a bunch of them here so it is less clear. It would be really helpful to get any extra information that you can about the Synopsys IP in your SoC. If it configured with UART_ADD_ENCODED_PARAMS = 1, there should be a UART configuration ID register at offset 0xF4. Could you try reading that back? This register has information about the FIFO size and a few more things. Also, I am curious what LCR value the hardware reports when the driver fails update it. Perhaps you can amend the error message to include p->serial_in(p, UART_LCR) too? -Tim -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@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: [PATCH v2] serial: 8250_dw: Improve unwritable LCR workaround
On Thu, Nov 28, 2013 at 04:53:37PM -0300, Ezequiel Garcia wrote: > On Thu, Nov 28, 2013 at 04:47:20PM -0300, Ezequiel Garcia wrote: > > Hi Thomas, Tim: > > > > On Thu, Nov 28, 2013 at 09:30:34AM +0100, Thomas Petazzoni wrote: > > > Dear Ezequiel Garcia, > > > > > > On Wed, 27 Nov 2013 15:54:49 -0300, Ezequiel Garcia wrote: > > > > > > > > An external device may be keeping the UART busy and preventing LCR > > > > > from being written. > > > > > > > > > > What device is attached to ttyS1? > > > > > > > > There's no device attached at ttyS1. I've just tested this in another > > > > box and it seems the same error is obtained on each unused port: > > > > > > Are you sure about this? I suppose you're testing on the Armada XP GP > > > board, and this board has a 4 ports FTDI chip, and according to the > > > board schematics the four UARTs are all connected to the FTDI chip. So > > > from the SoC perspective, ttyS1 is connected to something, as far as I > > > can understand. Or maybe you also tested Armada XP DB ? > > > > > > > Yeah, sorry about that. I missed the FTDI chip. As Thomas says the XP GP > > board I'm testing this on, has its four UARTs connected to a FTDI chip. > > > > Changing the console port by setting "console=ttyS1,115200" gives this: > > > > [..] > > Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled > > dw-apb-uart d0012000.serial: Couldn't set LCR to 191 > > dw-apb-uart d0012000.serial: Couldn't set LCR to 191 > > dw-apb-uart d0012000.serial: Couldn't set LCR to 224 > > dw-apb-uart d0012000.serial: Couldn't set LCR to 224 > > d0012000.serial: ttyS0 at MMIO 0xd0012000 (irq = 17, base_baud = 15625000) > > is a 16550A > > dw-apb-uart d0012100.serial: Couldn't set LCR to 191 > > dw-apb-uart d0012100.serial: Couldn't set LCR to 191 > > dw-apb-uart d0012100.serial: Couldn't set LCR to 224 > > dw-apb-uart d0012100.serial: Couldn't set LCR to 224 > > d0012100.serial: ttyS1 at MMIO 0xd0012100 (irq = 18, base_baud = 15625000) > > is a 16550A > > console [ttyS1] enabled > > dw-apb-uart d0012200.serial: Couldn't set LCR to 191 > > dw-apb-uart d0012200.serial: Couldn't set LCR to 191 > > dw-apb-uart d0012200.serial: Couldn't set LCR to 224 > > dw-apb-uart d0012200.serial: Couldn't set LCR to 224 > > d0012200.serial: ttyS2 at MMIO 0xd0012200 (irq = 31, base_baud = 15625000) > > is a 16550A > > dw-apb-uart d0012300.serial: Couldn't set LCR to 191 > > dw-apb-uart d0012300.serial: Couldn't set LCR to 191 > > dw-apb-uart d0012300.serial: Couldn't set LCR to 224 > > dw-apb-uart d0012300.serial: Couldn't set LCR to 224 > > d0012300.serial: ttyS3 at MMIO 0xd0012300 (irq = 32, base_baud = 15625000) > > is a 16550A > > > > So we get the "Couldn't set" message in all four ports. > > > > Tim: Any ideas? > > And another thing: the weird output on the console looks related to the > early boot console. If I enable 'earlyprintk' on ttyS0 but set the console > on ttyS1, this is what I get on ttyS0: > > bootconsole [earlycon0] enabled > [..] > Kernel command line: earlyprintk console=ttyS1,115200 root=/dev/nfs rw > nfsroot=192.168.0.45:/opt/buildrootfs,v3, > ip=192.168.0.159:192.168.0.45:192.168.0.1:255.255.255.0:develboard:eth0:on > rootwait > [..] > Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled > �Ɋ��ɁɆ� > Welcome to Buildroot > buildroot login: > > Hope this helps you understand what's going on... Gentle ping? Any ideas about those weird characters? Thanks! -- Ezequiel García, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@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: [PATCH v2] serial: 8250_dw: Improve unwritable LCR workaround
On Thu, Nov 28, 2013 at 04:53:37PM -0300, Ezequiel Garcia wrote: On Thu, Nov 28, 2013 at 04:47:20PM -0300, Ezequiel Garcia wrote: Hi Thomas, Tim: On Thu, Nov 28, 2013 at 09:30:34AM +0100, Thomas Petazzoni wrote: Dear Ezequiel Garcia, On Wed, 27 Nov 2013 15:54:49 -0300, Ezequiel Garcia wrote: An external device may be keeping the UART busy and preventing LCR from being written. What device is attached to ttyS1? There's no device attached at ttyS1. I've just tested this in another box and it seems the same error is obtained on each unused port: Are you sure about this? I suppose you're testing on the Armada XP GP board, and this board has a 4 ports FTDI chip, and according to the board schematics the four UARTs are all connected to the FTDI chip. So from the SoC perspective, ttyS1 is connected to something, as far as I can understand. Or maybe you also tested Armada XP DB ? Yeah, sorry about that. I missed the FTDI chip. As Thomas says the XP GP board I'm testing this on, has its four UARTs connected to a FTDI chip. Changing the console port by setting console=ttyS1,115200 gives this: [..] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled dw-apb-uart d0012000.serial: Couldn't set LCR to 191 dw-apb-uart d0012000.serial: Couldn't set LCR to 191 dw-apb-uart d0012000.serial: Couldn't set LCR to 224 dw-apb-uart d0012000.serial: Couldn't set LCR to 224 d0012000.serial: ttyS0 at MMIO 0xd0012000 (irq = 17, base_baud = 15625000) is a 16550A dw-apb-uart d0012100.serial: Couldn't set LCR to 191 dw-apb-uart d0012100.serial: Couldn't set LCR to 191 dw-apb-uart d0012100.serial: Couldn't set LCR to 224 dw-apb-uart d0012100.serial: Couldn't set LCR to 224 d0012100.serial: ttyS1 at MMIO 0xd0012100 (irq = 18, base_baud = 15625000) is a 16550A console [ttyS1] enabled dw-apb-uart d0012200.serial: Couldn't set LCR to 191 dw-apb-uart d0012200.serial: Couldn't set LCR to 191 dw-apb-uart d0012200.serial: Couldn't set LCR to 224 dw-apb-uart d0012200.serial: Couldn't set LCR to 224 d0012200.serial: ttyS2 at MMIO 0xd0012200 (irq = 31, base_baud = 15625000) is a 16550A dw-apb-uart d0012300.serial: Couldn't set LCR to 191 dw-apb-uart d0012300.serial: Couldn't set LCR to 191 dw-apb-uart d0012300.serial: Couldn't set LCR to 224 dw-apb-uart d0012300.serial: Couldn't set LCR to 224 d0012300.serial: ttyS3 at MMIO 0xd0012300 (irq = 32, base_baud = 15625000) is a 16550A So we get the Couldn't set message in all four ports. Tim: Any ideas? And another thing: the weird output on the console looks related to the early boot console. If I enable 'earlyprintk' on ttyS0 but set the console on ttyS1, this is what I get on ttyS0: bootconsole [earlycon0] enabled [..] Kernel command line: earlyprintk console=ttyS1,115200 root=/dev/nfs rw nfsroot=192.168.0.45:/opt/buildrootfs,v3, ip=192.168.0.159:192.168.0.45:192.168.0.1:255.255.255.0:develboard:eth0:on rootwait [..] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled �Ɋ��ɁɆ� Welcome to Buildroot buildroot login: Hope this helps you understand what's going on... Gentle ping? Any ideas about those weird characters? Thanks! -- Ezequiel García, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@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: [PATCH v2] serial: 8250_dw: Improve unwritable LCR workaround
On Wed, Dec 4, 2013 at 5:01 AM, Ezequiel Garcia ezequiel.gar...@free-electrons.com wrote: On Thu, Nov 28, 2013 at 04:53:37PM -0300, Ezequiel Garcia wrote: On Thu, Nov 28, 2013 at 04:47:20PM -0300, Ezequiel Garcia wrote: Changing the console port by setting console=ttyS1,115200 gives this: [..] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled dw-apb-uart d0012000.serial: Couldn't set LCR to 191 dw-apb-uart d0012000.serial: Couldn't set LCR to 191 dw-apb-uart d0012000.serial: Couldn't set LCR to 224 dw-apb-uart d0012000.serial: Couldn't set LCR to 224 d0012000.serial: ttyS0 at MMIO 0xd0012000 (irq = 17, base_baud = 15625000) is a 16550A dw-apb-uart d0012100.serial: Couldn't set LCR to 191 dw-apb-uart d0012100.serial: Couldn't set LCR to 191 dw-apb-uart d0012100.serial: Couldn't set LCR to 224 dw-apb-uart d0012100.serial: Couldn't set LCR to 224 d0012100.serial: ttyS1 at MMIO 0xd0012100 (irq = 18, base_baud = 15625000) is a 16550A console [ttyS1] enabled dw-apb-uart d0012200.serial: Couldn't set LCR to 191 dw-apb-uart d0012200.serial: Couldn't set LCR to 191 dw-apb-uart d0012200.serial: Couldn't set LCR to 224 dw-apb-uart d0012200.serial: Couldn't set LCR to 224 d0012200.serial: ttyS2 at MMIO 0xd0012200 (irq = 31, base_baud = 15625000) is a 16550A dw-apb-uart d0012300.serial: Couldn't set LCR to 191 dw-apb-uart d0012300.serial: Couldn't set LCR to 191 dw-apb-uart d0012300.serial: Couldn't set LCR to 224 dw-apb-uart d0012300.serial: Couldn't set LCR to 224 d0012300.serial: ttyS3 at MMIO 0xd0012300 (irq = 32, base_baud = 15625000) is a 16550A So we get the Couldn't set message in all four ports. Tim: Any ideas? And another thing: the weird output on the console looks related to the early boot console. If I enable 'earlyprintk' on ttyS0 but set the console on ttyS1, this is what I get on ttyS0: bootconsole [earlycon0] enabled [..] Kernel command line: earlyprintk console=ttyS1,115200 root=/dev/nfs rw nfsroot=192.168.0.45:/opt/buildrootfs,v3, ip=192.168.0.159:192.168.0.45:192.168.0.1:255.255.255.0:develboard:eth0:on rootwait [..] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled �Ɋ��ɁɆ� Welcome to Buildroot buildroot login: Hope this helps you understand what's going on... Gentle ping? Any ideas about those weird characters? If there was just one weird character, I would say it was an indication that hardware rejected a write to LCR and then software wrote the lower 8 bits of the baud into DLL which happens to live at the same address offset as RBR. However, there are a bunch of them here so it is less clear. It would be really helpful to get any extra information that you can about the Synopsys IP in your SoC. If it configured with UART_ADD_ENCODED_PARAMS = 1, there should be a UART configuration ID register at offset 0xF4. Could you try reading that back? This register has information about the FIFO size and a few more things. Also, I am curious what LCR value the hardware reports when the driver fails update it. Perhaps you can amend the error message to include p-serial_in(p, UART_LCR) too? -Tim -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@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: [PATCH v2] serial: 8250_dw: Improve unwritable LCR workaround
On Thu, Nov 28, 2013 at 04:47:20PM -0300, Ezequiel Garcia wrote: > Hi Thomas, Tim: > > On Thu, Nov 28, 2013 at 09:30:34AM +0100, Thomas Petazzoni wrote: > > Dear Ezequiel Garcia, > > > > On Wed, 27 Nov 2013 15:54:49 -0300, Ezequiel Garcia wrote: > > > > > > An external device may be keeping the UART busy and preventing LCR > > > > from being written. > > > > > > > > What device is attached to ttyS1? > > > > > > There's no device attached at ttyS1. I've just tested this in another > > > box and it seems the same error is obtained on each unused port: > > > > Are you sure about this? I suppose you're testing on the Armada XP GP > > board, and this board has a 4 ports FTDI chip, and according to the > > board schematics the four UARTs are all connected to the FTDI chip. So > > from the SoC perspective, ttyS1 is connected to something, as far as I > > can understand. Or maybe you also tested Armada XP DB ? > > > > Yeah, sorry about that. I missed the FTDI chip. As Thomas says the XP GP > board I'm testing this on, has its four UARTs connected to a FTDI chip. > > Changing the console port by setting "console=ttyS1,115200" gives this: > > [..] > Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled > dw-apb-uart d0012000.serial: Couldn't set LCR to 191 > dw-apb-uart d0012000.serial: Couldn't set LCR to 191 > dw-apb-uart d0012000.serial: Couldn't set LCR to 224 > dw-apb-uart d0012000.serial: Couldn't set LCR to 224 > d0012000.serial: ttyS0 at MMIO 0xd0012000 (irq = 17, base_baud = 15625000) is > a 16550A > dw-apb-uart d0012100.serial: Couldn't set LCR to 191 > dw-apb-uart d0012100.serial: Couldn't set LCR to 191 > dw-apb-uart d0012100.serial: Couldn't set LCR to 224 > dw-apb-uart d0012100.serial: Couldn't set LCR to 224 > d0012100.serial: ttyS1 at MMIO 0xd0012100 (irq = 18, base_baud = 15625000) is > a 16550A > console [ttyS1] enabled > dw-apb-uart d0012200.serial: Couldn't set LCR to 191 > dw-apb-uart d0012200.serial: Couldn't set LCR to 191 > dw-apb-uart d0012200.serial: Couldn't set LCR to 224 > dw-apb-uart d0012200.serial: Couldn't set LCR to 224 > d0012200.serial: ttyS2 at MMIO 0xd0012200 (irq = 31, base_baud = 15625000) is > a 16550A > dw-apb-uart d0012300.serial: Couldn't set LCR to 191 > dw-apb-uart d0012300.serial: Couldn't set LCR to 191 > dw-apb-uart d0012300.serial: Couldn't set LCR to 224 > dw-apb-uart d0012300.serial: Couldn't set LCR to 224 > d0012300.serial: ttyS3 at MMIO 0xd0012300 (irq = 32, base_baud = 15625000) is > a 16550A > > So we get the "Couldn't set" message in all four ports. > > Tim: Any ideas? And another thing: the weird output on the console looks related to the early boot console. If I enable 'earlyprintk' on ttyS0 but set the console on ttyS1, this is what I get on ttyS0: bootconsole [earlycon0] enabled [..] Kernel command line: earlyprintk console=ttyS1,115200 root=/dev/nfs rw nfsroot=192.168.0.45:/opt/buildrootfs,v3, ip=192.168.0.159:192.168.0.45:192.168.0.1:255.255.255.0:develboard:eth0:on rootwait [..] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled �Ɋ��ɁɆ� Welcome to Buildroot buildroot login: Hope this helps you understand what's going on... -- Ezequiel García, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@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: [PATCH v2] serial: 8250_dw: Improve unwritable LCR workaround
Hi Thomas, Tim: On Thu, Nov 28, 2013 at 09:30:34AM +0100, Thomas Petazzoni wrote: > Dear Ezequiel Garcia, > > On Wed, 27 Nov 2013 15:54:49 -0300, Ezequiel Garcia wrote: > > > > An external device may be keeping the UART busy and preventing LCR > > > from being written. > > > > > > What device is attached to ttyS1? > > > > There's no device attached at ttyS1. I've just tested this in another > > box and it seems the same error is obtained on each unused port: > > Are you sure about this? I suppose you're testing on the Armada XP GP > board, and this board has a 4 ports FTDI chip, and according to the > board schematics the four UARTs are all connected to the FTDI chip. So > from the SoC perspective, ttyS1 is connected to something, as far as I > can understand. Or maybe you also tested Armada XP DB ? > Yeah, sorry about that. I missed the FTDI chip. As Thomas says the XP GP board I'm testing this on, has its four UARTs connected to a FTDI chip. Changing the console port by setting "console=ttyS1,115200" gives this: [..] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled dw-apb-uart d0012000.serial: Couldn't set LCR to 191 dw-apb-uart d0012000.serial: Couldn't set LCR to 191 dw-apb-uart d0012000.serial: Couldn't set LCR to 224 dw-apb-uart d0012000.serial: Couldn't set LCR to 224 d0012000.serial: ttyS0 at MMIO 0xd0012000 (irq = 17, base_baud = 15625000) is a 16550A dw-apb-uart d0012100.serial: Couldn't set LCR to 191 dw-apb-uart d0012100.serial: Couldn't set LCR to 191 dw-apb-uart d0012100.serial: Couldn't set LCR to 224 dw-apb-uart d0012100.serial: Couldn't set LCR to 224 d0012100.serial: ttyS1 at MMIO 0xd0012100 (irq = 18, base_baud = 15625000) is a 16550A console [ttyS1] enabled dw-apb-uart d0012200.serial: Couldn't set LCR to 191 dw-apb-uart d0012200.serial: Couldn't set LCR to 191 dw-apb-uart d0012200.serial: Couldn't set LCR to 224 dw-apb-uart d0012200.serial: Couldn't set LCR to 224 d0012200.serial: ttyS2 at MMIO 0xd0012200 (irq = 31, base_baud = 15625000) is a 16550A dw-apb-uart d0012300.serial: Couldn't set LCR to 191 dw-apb-uart d0012300.serial: Couldn't set LCR to 191 dw-apb-uart d0012300.serial: Couldn't set LCR to 224 dw-apb-uart d0012300.serial: Couldn't set LCR to 224 d0012300.serial: ttyS3 at MMIO 0xd0012300 (irq = 32, base_baud = 15625000) is a 16550A So we get the "Couldn't set" message in all four ports. Tim: Any ideas? -- Ezequiel García, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@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: [PATCH v2] serial: 8250_dw: Improve unwritable LCR workaround
Dear Ezequiel Garcia, On Wed, 27 Nov 2013 15:54:49 -0300, Ezequiel Garcia wrote: > > An external device may be keeping the UART busy and preventing LCR > > from being written. > > > > What device is attached to ttyS1? > > There's no device attached at ttyS1. I've just tested this in another > box and it seems the same error is obtained on each unused port: Are you sure about this? I suppose you're testing on the Armada XP GP board, and this board has a 4 ports FTDI chip, and according to the board schematics the four UARTs are all connected to the FTDI chip. So from the SoC perspective, ttyS1 is connected to something, as far as I can understand. Or maybe you also tested Armada XP DB ? Best regards, Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@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: [PATCH v2] serial: 8250_dw: Improve unwritable LCR workaround
Dear Ezequiel Garcia, On Wed, 27 Nov 2013 15:54:49 -0300, Ezequiel Garcia wrote: An external device may be keeping the UART busy and preventing LCR from being written. What device is attached to ttyS1? There's no device attached at ttyS1. I've just tested this in another box and it seems the same error is obtained on each unused port: Are you sure about this? I suppose you're testing on the Armada XP GP board, and this board has a 4 ports FTDI chip, and according to the board schematics the four UARTs are all connected to the FTDI chip. So from the SoC perspective, ttyS1 is connected to something, as far as I can understand. Or maybe you also tested Armada XP DB ? Best regards, Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@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: [PATCH v2] serial: 8250_dw: Improve unwritable LCR workaround
Hi Thomas, Tim: On Thu, Nov 28, 2013 at 09:30:34AM +0100, Thomas Petazzoni wrote: Dear Ezequiel Garcia, On Wed, 27 Nov 2013 15:54:49 -0300, Ezequiel Garcia wrote: An external device may be keeping the UART busy and preventing LCR from being written. What device is attached to ttyS1? There's no device attached at ttyS1. I've just tested this in another box and it seems the same error is obtained on each unused port: Are you sure about this? I suppose you're testing on the Armada XP GP board, and this board has a 4 ports FTDI chip, and according to the board schematics the four UARTs are all connected to the FTDI chip. So from the SoC perspective, ttyS1 is connected to something, as far as I can understand. Or maybe you also tested Armada XP DB ? Yeah, sorry about that. I missed the FTDI chip. As Thomas says the XP GP board I'm testing this on, has its four UARTs connected to a FTDI chip. Changing the console port by setting console=ttyS1,115200 gives this: [..] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled dw-apb-uart d0012000.serial: Couldn't set LCR to 191 dw-apb-uart d0012000.serial: Couldn't set LCR to 191 dw-apb-uart d0012000.serial: Couldn't set LCR to 224 dw-apb-uart d0012000.serial: Couldn't set LCR to 224 d0012000.serial: ttyS0 at MMIO 0xd0012000 (irq = 17, base_baud = 15625000) is a 16550A dw-apb-uart d0012100.serial: Couldn't set LCR to 191 dw-apb-uart d0012100.serial: Couldn't set LCR to 191 dw-apb-uart d0012100.serial: Couldn't set LCR to 224 dw-apb-uart d0012100.serial: Couldn't set LCR to 224 d0012100.serial: ttyS1 at MMIO 0xd0012100 (irq = 18, base_baud = 15625000) is a 16550A console [ttyS1] enabled dw-apb-uart d0012200.serial: Couldn't set LCR to 191 dw-apb-uart d0012200.serial: Couldn't set LCR to 191 dw-apb-uart d0012200.serial: Couldn't set LCR to 224 dw-apb-uart d0012200.serial: Couldn't set LCR to 224 d0012200.serial: ttyS2 at MMIO 0xd0012200 (irq = 31, base_baud = 15625000) is a 16550A dw-apb-uart d0012300.serial: Couldn't set LCR to 191 dw-apb-uart d0012300.serial: Couldn't set LCR to 191 dw-apb-uart d0012300.serial: Couldn't set LCR to 224 dw-apb-uart d0012300.serial: Couldn't set LCR to 224 d0012300.serial: ttyS3 at MMIO 0xd0012300 (irq = 32, base_baud = 15625000) is a 16550A So we get the Couldn't set message in all four ports. Tim: Any ideas? -- Ezequiel García, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@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: [PATCH v2] serial: 8250_dw: Improve unwritable LCR workaround
On Thu, Nov 28, 2013 at 04:47:20PM -0300, Ezequiel Garcia wrote: Hi Thomas, Tim: On Thu, Nov 28, 2013 at 09:30:34AM +0100, Thomas Petazzoni wrote: Dear Ezequiel Garcia, On Wed, 27 Nov 2013 15:54:49 -0300, Ezequiel Garcia wrote: An external device may be keeping the UART busy and preventing LCR from being written. What device is attached to ttyS1? There's no device attached at ttyS1. I've just tested this in another box and it seems the same error is obtained on each unused port: Are you sure about this? I suppose you're testing on the Armada XP GP board, and this board has a 4 ports FTDI chip, and according to the board schematics the four UARTs are all connected to the FTDI chip. So from the SoC perspective, ttyS1 is connected to something, as far as I can understand. Or maybe you also tested Armada XP DB ? Yeah, sorry about that. I missed the FTDI chip. As Thomas says the XP GP board I'm testing this on, has its four UARTs connected to a FTDI chip. Changing the console port by setting console=ttyS1,115200 gives this: [..] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled dw-apb-uart d0012000.serial: Couldn't set LCR to 191 dw-apb-uart d0012000.serial: Couldn't set LCR to 191 dw-apb-uart d0012000.serial: Couldn't set LCR to 224 dw-apb-uart d0012000.serial: Couldn't set LCR to 224 d0012000.serial: ttyS0 at MMIO 0xd0012000 (irq = 17, base_baud = 15625000) is a 16550A dw-apb-uart d0012100.serial: Couldn't set LCR to 191 dw-apb-uart d0012100.serial: Couldn't set LCR to 191 dw-apb-uart d0012100.serial: Couldn't set LCR to 224 dw-apb-uart d0012100.serial: Couldn't set LCR to 224 d0012100.serial: ttyS1 at MMIO 0xd0012100 (irq = 18, base_baud = 15625000) is a 16550A console [ttyS1] enabled dw-apb-uart d0012200.serial: Couldn't set LCR to 191 dw-apb-uart d0012200.serial: Couldn't set LCR to 191 dw-apb-uart d0012200.serial: Couldn't set LCR to 224 dw-apb-uart d0012200.serial: Couldn't set LCR to 224 d0012200.serial: ttyS2 at MMIO 0xd0012200 (irq = 31, base_baud = 15625000) is a 16550A dw-apb-uart d0012300.serial: Couldn't set LCR to 191 dw-apb-uart d0012300.serial: Couldn't set LCR to 191 dw-apb-uart d0012300.serial: Couldn't set LCR to 224 dw-apb-uart d0012300.serial: Couldn't set LCR to 224 d0012300.serial: ttyS3 at MMIO 0xd0012300 (irq = 32, base_baud = 15625000) is a 16550A So we get the Couldn't set message in all four ports. Tim: Any ideas? And another thing: the weird output on the console looks related to the early boot console. If I enable 'earlyprintk' on ttyS0 but set the console on ttyS1, this is what I get on ttyS0: bootconsole [earlycon0] enabled [..] Kernel command line: earlyprintk console=ttyS1,115200 root=/dev/nfs rw nfsroot=192.168.0.45:/opt/buildrootfs,v3, ip=192.168.0.159:192.168.0.45:192.168.0.1:255.255.255.0:develboard:eth0:on rootwait [..] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled �Ɋ��ɁɆ� Welcome to Buildroot buildroot login: Hope this helps you understand what's going on... -- Ezequiel García, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@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: [PATCH v2] serial: 8250_dw: Improve unwritable LCR workaround
On Wed, Nov 27, 2013 at 10:54 AM, Ezequiel Garcia wrote: > On Tue, Nov 26, 2013 at 03:03:03PM -0800, Tim Kryger wrote: >> An external device may be keeping the UART busy and preventing LCR >> from being written. >> >> What device is attached to ttyS1? > > There's no device attached at ttyS1. I've just tested this in another > box and it seems the same error is obtained on each unused port: Do you know if your UART pins have pull up resistors? If you can, try attaching something to an unused port. -Tim -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@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: [PATCH v2] serial: 8250_dw: Improve unwritable LCR workaround
On Tue, Nov 26, 2013 at 03:03:03PM -0800, Tim Kryger wrote: > On Tue, Nov 26, 2013 at 10:36 AM, Ezequiel Garcia > wrote: > > > Since v3.13-rc1, this commit seems to have introduced some oddities on > > some of our boards. See this log snippet: > > > > Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled > > R�console [ttyS0] enabled > > console [ttyS0] enabled > > bootconsole [earlycon0] disabled > > bootconsole [earlycon0] disabled > > dw-apb-uart d0012100.serial: Couldn't set LCR to 191 > > dw-apb-uart d0012100.serial: Couldn't set LCR to 191 > > dw-apb-uart d0012100.serial: Couldn't set LCR to 224 > > dw-apb-uart d0012100.serial: Couldn't set LCR to 224 > > d0012100.serial: ttyS1 at MMIO 0xd0012100 (irq = 18, base_baud = 15625000) > > is a 16550A > > > > This behavior appear in at least Armada 370 and Armada XP boxes. > > > > I confirm reverting this commit fixes the issue and things get back to > > normal. > > Here's the complete kernel log: sprunge.us/gMdL > > > > Ideas? > > Hi Ezequiel, > > An external device may be keeping the UART busy and preventing LCR > from being written. > > What device is attached to ttyS1? There's no device attached at ttyS1. I've just tested this in another box and it seems the same error is obtained on each unused port: [...] dw-apb-uart d0012100.serial: Couldn't set LCR to 191 dw-apb-uart d0012100.serial: Couldn't set LCR to 191 dw-apb-uart d0012100.serial: Couldn't set LCR to 224 dw-apb-uart d0012100.serial: Couldn't set LCR to 224 d0012100.serial: ttyS1 at MMIO 0xd0012100 (irq = 18, base_baud = 15625000) is a 16550A dw-apb-uart d0012200.serial: Couldn't set LCR to 191 dw-apb-uart d0012200.serial: Couldn't set LCR to 191 dw-apb-uart d0012200.serial: Couldn't set LCR to 224 dw-apb-uart d0012200.serial: Couldn't set LCR to 224 d0012200.serial: ttyS2 at MMIO 0xd0012200 (irq = 31, base_baud = 15625000) is a 16550A dw-apb-uart d0012300.serial: Couldn't set LCR to 191 dw-apb-uart d0012300.serial: Couldn't set LCR to 191 dw-apb-uart d0012300.serial: Couldn't set LCR to 224 dw-apb-uart d0012300.serial: Couldn't set LCR to 224 d0012300.serial: ttyS3 at MMIO 0xd0012300 (irq = 32, base_baud = 15625000) is a 16550A In this board, I only have ttyS0 (console) connected. > Also, do you know the version of the Synopsys IP? > If built with ADDITIONAL_FEATURES=YES, the version can be read from > the hardware: > > # busybox devmem 0xd00121f8 32 > No, I don't know this IP version and ADDITIONAL_FEATURES seems not built. Thanks for taking a look at this! -- Ezequiel García, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@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: [PATCH v2] serial: 8250_dw: Improve unwritable LCR workaround
On Tue, Nov 26, 2013 at 03:03:03PM -0800, Tim Kryger wrote: On Tue, Nov 26, 2013 at 10:36 AM, Ezequiel Garcia ezequiel.gar...@free-electrons.com wrote: Since v3.13-rc1, this commit seems to have introduced some oddities on some of our boards. See this log snippet: Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled R�console [ttyS0] enabled console [ttyS0] enabled bootconsole [earlycon0] disabled bootconsole [earlycon0] disabled dw-apb-uart d0012100.serial: Couldn't set LCR to 191 dw-apb-uart d0012100.serial: Couldn't set LCR to 191 dw-apb-uart d0012100.serial: Couldn't set LCR to 224 dw-apb-uart d0012100.serial: Couldn't set LCR to 224 d0012100.serial: ttyS1 at MMIO 0xd0012100 (irq = 18, base_baud = 15625000) is a 16550A This behavior appear in at least Armada 370 and Armada XP boxes. I confirm reverting this commit fixes the issue and things get back to normal. Here's the complete kernel log: sprunge.us/gMdL Ideas? Hi Ezequiel, An external device may be keeping the UART busy and preventing LCR from being written. What device is attached to ttyS1? There's no device attached at ttyS1. I've just tested this in another box and it seems the same error is obtained on each unused port: [...] dw-apb-uart d0012100.serial: Couldn't set LCR to 191 dw-apb-uart d0012100.serial: Couldn't set LCR to 191 dw-apb-uart d0012100.serial: Couldn't set LCR to 224 dw-apb-uart d0012100.serial: Couldn't set LCR to 224 d0012100.serial: ttyS1 at MMIO 0xd0012100 (irq = 18, base_baud = 15625000) is a 16550A dw-apb-uart d0012200.serial: Couldn't set LCR to 191 dw-apb-uart d0012200.serial: Couldn't set LCR to 191 dw-apb-uart d0012200.serial: Couldn't set LCR to 224 dw-apb-uart d0012200.serial: Couldn't set LCR to 224 d0012200.serial: ttyS2 at MMIO 0xd0012200 (irq = 31, base_baud = 15625000) is a 16550A dw-apb-uart d0012300.serial: Couldn't set LCR to 191 dw-apb-uart d0012300.serial: Couldn't set LCR to 191 dw-apb-uart d0012300.serial: Couldn't set LCR to 224 dw-apb-uart d0012300.serial: Couldn't set LCR to 224 d0012300.serial: ttyS3 at MMIO 0xd0012300 (irq = 32, base_baud = 15625000) is a 16550A In this board, I only have ttyS0 (console) connected. Also, do you know the version of the Synopsys IP? If built with ADDITIONAL_FEATURES=YES, the version can be read from the hardware: # busybox devmem 0xd00121f8 32 No, I don't know this IP version and ADDITIONAL_FEATURES seems not built. Thanks for taking a look at this! -- Ezequiel García, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@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: [PATCH v2] serial: 8250_dw: Improve unwritable LCR workaround
On Wed, Nov 27, 2013 at 10:54 AM, Ezequiel Garcia ezequiel.gar...@free-electrons.com wrote: On Tue, Nov 26, 2013 at 03:03:03PM -0800, Tim Kryger wrote: An external device may be keeping the UART busy and preventing LCR from being written. What device is attached to ttyS1? There's no device attached at ttyS1. I've just tested this in another box and it seems the same error is obtained on each unused port: Do you know if your UART pins have pull up resistors? If you can, try attaching something to an unused port. -Tim -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@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: [PATCH v2] serial: 8250_dw: Improve unwritable LCR workaround
On Tue, Nov 26, 2013 at 10:36 AM, Ezequiel Garcia wrote: > Since v3.13-rc1, this commit seems to have introduced some oddities on > some of our boards. See this log snippet: > > Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled > R�console [ttyS0] enabled > console [ttyS0] enabled > bootconsole [earlycon0] disabled > bootconsole [earlycon0] disabled > dw-apb-uart d0012100.serial: Couldn't set LCR to 191 > dw-apb-uart d0012100.serial: Couldn't set LCR to 191 > dw-apb-uart d0012100.serial: Couldn't set LCR to 224 > dw-apb-uart d0012100.serial: Couldn't set LCR to 224 > d0012100.serial: ttyS1 at MMIO 0xd0012100 (irq = 18, base_baud = 15625000) is > a 16550A > > This behavior appear in at least Armada 370 and Armada XP boxes. > > I confirm reverting this commit fixes the issue and things get back to normal. > Here's the complete kernel log: sprunge.us/gMdL > > Ideas? Hi Ezequiel, An external device may be keeping the UART busy and preventing LCR from being written. What device is attached to ttyS1? Also, do you know the version of the Synopsys IP? If built with ADDITIONAL_FEATURES=YES, the version can be read from the hardware: # busybox devmem 0xd00121f8 32 -Tim -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@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: [PATCH v2] serial: 8250_dw: Improve unwritable LCR workaround
Hello,
On Tue, Oct 01, 2013 at 10:18:08AM -0700, Tim Kryger wrote:
> When configured with UART_16550_COMPATIBLE=NO or in versions prior to
> the introduction of this option, the Designware UART will ignore writes
> to the LCR if the UART is busy. The current workaround saves a copy of
> the last written LCR and re-writes it in the ISR for a special interrupt
> that is raised when a write was ignored.
>
> Unfortunately, interrupts are typically disabled prior to performing a
> sequence of register writes that include the LCR so the point at which
> the retry occurs is too late. An example is serial8250_do_set_termios()
> where an ignored LCR write results in the baud divisor not being set and
> instead a garbage character is sent out the transmitter.
>
> Furthermore, since serial_port_out() offers no way to indicate failure,
> a serious effort must be made to ensure that the LCR is actually updated
> before returning back to the caller. This is difficult, however, as a
> UART that was busy during the first attempt is likely to still be busy
> when a subsequent attempt is made unless some extra action is taken.
>
> This updated workaround reads back the LCR after each write to confirm
> that the new value was accepted by the hardware. Should the hardware
> ignore a write, the TX/RX FIFOs are cleared and the receive buffer read
> before attempting to rewrite the LCR out of the hope that doing so will
> force the UART into an idle state. While this may seem unnecessarily
> aggressive, 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. Admittedly, this is far from ideal but it seems to be the
> best that can be done given the hardware limitations.
>
> Lastly, the revised workaround doesn't touch the LCR in the ISR, so it
> avoids the possibility of a "serial8250: too much work for irq" lock up.
> This problem is rare in real situations but can be reproduced easily by
> wiring up two UARTs and running the following commands.
>
> # stty -F /dev/ttyS1 echo
> # stty -F /dev/ttyS2 echo
> # cat /dev/ttyS1 &
> [1] 375
> # echo asdf > /dev/ttyS1
> asdf
>
> [ 27.70] serial8250: too much work for irq96
> [ 27.70] serial8250: too much work for irq96
> [ 27.71] serial8250: too much work for irq96
> [ 27.71] serial8250: too much work for irq96
> [ 27.72] serial8250: too much work for irq96
> [ 27.72] serial8250: too much work for irq96
> [ 27.73] serial8250: too much work for irq96
> [ 27.73] serial8250: too much work for irq96
> [ 27.74] serial8250: too much work for irq96
>
> Signed-off-by: Tim Kryger
> Reviewed-by: Matt Porter
> Reviewed-by: Markus Mayer
> ---
>
> Changes in v2:
> - Rebased on tty-next
> - Updated commit messsage to mention UART_16550_COMPATIBLE
> - Removed potentially unnecessary read of LSR and MSR
> - Only attempt workaround when LCR write is ignored
>
> drivers/tty/serial/8250/8250_dw.c | 41
> ++-
> 1 file changed, 32 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_dw.c
> b/drivers/tty/serial/8250/8250_dw.c
> index d04a037..4658e3e 100644
> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c
> @@ -57,7 +57,6 @@
>
> struct dw8250_data {
> u8 usr_reg;
> - int last_lcr;
> int last_mcr;
> int line;
> struct clk *clk;
> @@ -77,17 +76,33 @@ static inline int dw8250_modify_msr(struct uart_port *p,
> int offset, int value)
> return value;
> }
>
> +static void dw8250_force_idle(struct uart_port *p)
> +{
> + serial8250_clear_and_reinit_fifos(container_of
> + (p, struct uart_8250_port, port));
> + (void)p->serial_in(p, UART_RX);
> +}
> +
> static void dw8250_serial_out(struct uart_port *p, int offset, int value)
> {
> struct dw8250_data *d = p->private_data;
>
> - if (offset == UART_LCR)
> - d->last_lcr = value;
> -
> if (offset == UART_MCR)
> d->last_mcr = value;
>
> writeb(value, p->membase + (offset << p->regshift));
> +
> + /* Make sure LCR write wasn't ignored */
> + if (offset == UART_LCR) {
> + int tries = 1000;
> + while (tries--) {
> + if (value == p->serial_in(p, UART_LCR))
> + return;
> + dw8250_force_idle(p);
> + writeb(value, p->membase + (UART_LCR << p->regshift));
> + }
> + dev_err(p->dev, "Couldn't set LCR to %d\n", value);
> + }
> }
>
> static unsigned int dw8250_serial_in(struct uart_port *p, int offset)
> @@ -108,13 +123,22 @@ static void dw8250_serial_out32(struct uart_port *p,
> int offset, int value)
> {
>
Re: [PATCH v2] serial: 8250_dw: Improve unwritable LCR workaround
Hello,
On Tue, Oct 01, 2013 at 10:18:08AM -0700, Tim Kryger wrote:
When configured with UART_16550_COMPATIBLE=NO or in versions prior to
the introduction of this option, the Designware UART will ignore writes
to the LCR if the UART is busy. The current workaround saves a copy of
the last written LCR and re-writes it in the ISR for a special interrupt
that is raised when a write was ignored.
Unfortunately, interrupts are typically disabled prior to performing a
sequence of register writes that include the LCR so the point at which
the retry occurs is too late. An example is serial8250_do_set_termios()
where an ignored LCR write results in the baud divisor not being set and
instead a garbage character is sent out the transmitter.
Furthermore, since serial_port_out() offers no way to indicate failure,
a serious effort must be made to ensure that the LCR is actually updated
before returning back to the caller. This is difficult, however, as a
UART that was busy during the first attempt is likely to still be busy
when a subsequent attempt is made unless some extra action is taken.
This updated workaround reads back the LCR after each write to confirm
that the new value was accepted by the hardware. Should the hardware
ignore a write, the TX/RX FIFOs are cleared and the receive buffer read
before attempting to rewrite the LCR out of the hope that doing so will
force the UART into an idle state. While this may seem unnecessarily
aggressive, 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. Admittedly, this is far from ideal but it seems to be the
best that can be done given the hardware limitations.
Lastly, the revised workaround doesn't touch the LCR in the ISR, so it
avoids the possibility of a serial8250: too much work for irq lock up.
This problem is rare in real situations but can be reproduced easily by
wiring up two UARTs and running the following commands.
# stty -F /dev/ttyS1 echo
# stty -F /dev/ttyS2 echo
# cat /dev/ttyS1
[1] 375
# echo asdf /dev/ttyS1
asdf
[ 27.70] serial8250: too much work for irq96
[ 27.70] serial8250: too much work for irq96
[ 27.71] serial8250: too much work for irq96
[ 27.71] serial8250: too much work for irq96
[ 27.72] serial8250: too much work for irq96
[ 27.72] serial8250: too much work for irq96
[ 27.73] serial8250: too much work for irq96
[ 27.73] serial8250: too much work for irq96
[ 27.74] serial8250: too much work for irq96
Signed-off-by: Tim Kryger tim.kry...@linaro.org
Reviewed-by: Matt Porter matt.por...@linaro.org
Reviewed-by: Markus Mayer markus.ma...@linaro.org
---
Changes in v2:
- Rebased on tty-next
- Updated commit messsage to mention UART_16550_COMPATIBLE
- Removed potentially unnecessary read of LSR and MSR
- Only attempt workaround when LCR write is ignored
drivers/tty/serial/8250/8250_dw.c | 41
++-
1 file changed, 32 insertions(+), 9 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_dw.c
b/drivers/tty/serial/8250/8250_dw.c
index d04a037..4658e3e 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -57,7 +57,6 @@
struct dw8250_data {
u8 usr_reg;
- int last_lcr;
int last_mcr;
int line;
struct clk *clk;
@@ -77,17 +76,33 @@ static inline int dw8250_modify_msr(struct uart_port *p,
int offset, int value)
return value;
}
+static void dw8250_force_idle(struct uart_port *p)
+{
+ serial8250_clear_and_reinit_fifos(container_of
+ (p, struct uart_8250_port, port));
+ (void)p-serial_in(p, UART_RX);
+}
+
static void dw8250_serial_out(struct uart_port *p, int offset, int value)
{
struct dw8250_data *d = p-private_data;
- if (offset == UART_LCR)
- d-last_lcr = value;
-
if (offset == UART_MCR)
d-last_mcr = value;
writeb(value, p-membase + (offset p-regshift));
+
+ /* Make sure LCR write wasn't ignored */
+ if (offset == UART_LCR) {
+ int tries = 1000;
+ while (tries--) {
+ if (value == p-serial_in(p, UART_LCR))
+ return;
+ dw8250_force_idle(p);
+ writeb(value, p-membase + (UART_LCR p-regshift));
+ }
+ dev_err(p-dev, Couldn't set LCR to %d\n, value);
+ }
}
static unsigned int dw8250_serial_in(struct uart_port *p, int offset)
@@ -108,13 +123,22 @@ static void dw8250_serial_out32(struct uart_port *p,
int offset, int value)
{
struct dw8250_data *d = p-private_data;
- if (offset ==
Re: [PATCH v2] serial: 8250_dw: Improve unwritable LCR workaround
On Tue, Nov 26, 2013 at 10:36 AM, Ezequiel Garcia ezequiel.gar...@free-electrons.com wrote: Since v3.13-rc1, this commit seems to have introduced some oddities on some of our boards. See this log snippet: Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled R�console [ttyS0] enabled console [ttyS0] enabled bootconsole [earlycon0] disabled bootconsole [earlycon0] disabled dw-apb-uart d0012100.serial: Couldn't set LCR to 191 dw-apb-uart d0012100.serial: Couldn't set LCR to 191 dw-apb-uart d0012100.serial: Couldn't set LCR to 224 dw-apb-uart d0012100.serial: Couldn't set LCR to 224 d0012100.serial: ttyS1 at MMIO 0xd0012100 (irq = 18, base_baud = 15625000) is a 16550A This behavior appear in at least Armada 370 and Armada XP boxes. I confirm reverting this commit fixes the issue and things get back to normal. Here's the complete kernel log: sprunge.us/gMdL Ideas? Hi Ezequiel, An external device may be keeping the UART busy and preventing LCR from being written. What device is attached to ttyS1? Also, do you know the version of the Synopsys IP? If built with ADDITIONAL_FEATURES=YES, the version can be read from the hardware: # busybox devmem 0xd00121f8 32 -Tim -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@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: [PATCH v2] serial: 8250_dw: Improve unwritable LCR workaround
Hi, On Tue, Oct 01, 2013 at 10:18:08AM -0700, Tim Kryger wrote: > When configured with UART_16550_COMPATIBLE=NO or in versions prior to > the introduction of this option, the Designware UART will ignore writes > to the LCR if the UART is busy. The current workaround saves a copy of > the last written LCR and re-writes it in the ISR for a special interrupt > that is raised when a write was ignored. > > Unfortunately, interrupts are typically disabled prior to performing a > sequence of register writes that include the LCR so the point at which > the retry occurs is too late. An example is serial8250_do_set_termios() > where an ignored LCR write results in the baud divisor not being set and > instead a garbage character is sent out the transmitter. > > Furthermore, since serial_port_out() offers no way to indicate failure, > a serious effort must be made to ensure that the LCR is actually updated > before returning back to the caller. This is difficult, however, as a > UART that was busy during the first attempt is likely to still be busy > when a subsequent attempt is made unless some extra action is taken. > > This updated workaround reads back the LCR after each write to confirm > that the new value was accepted by the hardware. Should the hardware > ignore a write, the TX/RX FIFOs are cleared and the receive buffer read > before attempting to rewrite the LCR out of the hope that doing so will > force the UART into an idle state. While this may seem unnecessarily > aggressive, 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. Admittedly, this is far from ideal but it seems to be the > best that can be done given the hardware limitations. > > Lastly, the revised workaround doesn't touch the LCR in the ISR, so it > avoids the possibility of a "serial8250: too much work for irq" lock up. > This problem is rare in real situations but can be reproduced easily by > wiring up two UARTs and running the following commands. > > # stty -F /dev/ttyS1 echo > # stty -F /dev/ttyS2 echo > # cat /dev/ttyS1 & > [1] 375 > # echo asdf > /dev/ttyS1 > asdf > > [ 27.70] serial8250: too much work for irq96 > [ 27.70] serial8250: too much work for irq96 > [ 27.71] serial8250: too much work for irq96 > [ 27.71] serial8250: too much work for irq96 > [ 27.72] serial8250: too much work for irq96 > [ 27.72] serial8250: too much work for irq96 > [ 27.73] serial8250: too much work for irq96 > [ 27.73] serial8250: too much work for irq96 > [ 27.74] serial8250: too much work for irq96 > > Signed-off-by: Tim Kryger > Reviewed-by: Matt Porter > Reviewed-by: Markus Mayer > --- > > Changes in v2: > - Rebased on tty-next > - Updated commit messsage to mention UART_16550_COMPATIBLE > - Removed potentially unnecessary read of LSR and MSR > - Only attempt workaround when LCR write is ignored I'm OK with this. Reviewed-by: Heikki Krogerus Br, -- heikki -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@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: [PATCH v2] serial: 8250_dw: Improve unwritable LCR workaround
Hi, On Tue, Oct 01, 2013 at 10:18:08AM -0700, Tim Kryger wrote: When configured with UART_16550_COMPATIBLE=NO or in versions prior to the introduction of this option, the Designware UART will ignore writes to the LCR if the UART is busy. The current workaround saves a copy of the last written LCR and re-writes it in the ISR for a special interrupt that is raised when a write was ignored. Unfortunately, interrupts are typically disabled prior to performing a sequence of register writes that include the LCR so the point at which the retry occurs is too late. An example is serial8250_do_set_termios() where an ignored LCR write results in the baud divisor not being set and instead a garbage character is sent out the transmitter. Furthermore, since serial_port_out() offers no way to indicate failure, a serious effort must be made to ensure that the LCR is actually updated before returning back to the caller. This is difficult, however, as a UART that was busy during the first attempt is likely to still be busy when a subsequent attempt is made unless some extra action is taken. This updated workaround reads back the LCR after each write to confirm that the new value was accepted by the hardware. Should the hardware ignore a write, the TX/RX FIFOs are cleared and the receive buffer read before attempting to rewrite the LCR out of the hope that doing so will force the UART into an idle state. While this may seem unnecessarily aggressive, 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. Admittedly, this is far from ideal but it seems to be the best that can be done given the hardware limitations. Lastly, the revised workaround doesn't touch the LCR in the ISR, so it avoids the possibility of a serial8250: too much work for irq lock up. This problem is rare in real situations but can be reproduced easily by wiring up two UARTs and running the following commands. # stty -F /dev/ttyS1 echo # stty -F /dev/ttyS2 echo # cat /dev/ttyS1 [1] 375 # echo asdf /dev/ttyS1 asdf [ 27.70] serial8250: too much work for irq96 [ 27.70] serial8250: too much work for irq96 [ 27.71] serial8250: too much work for irq96 [ 27.71] serial8250: too much work for irq96 [ 27.72] serial8250: too much work for irq96 [ 27.72] serial8250: too much work for irq96 [ 27.73] serial8250: too much work for irq96 [ 27.73] serial8250: too much work for irq96 [ 27.74] serial8250: too much work for irq96 Signed-off-by: Tim Kryger tim.kry...@linaro.org Reviewed-by: Matt Porter matt.por...@linaro.org Reviewed-by: Markus Mayer markus.ma...@linaro.org --- Changes in v2: - Rebased on tty-next - Updated commit messsage to mention UART_16550_COMPATIBLE - Removed potentially unnecessary read of LSR and MSR - Only attempt workaround when LCR write is ignored I'm OK with this. Reviewed-by: Heikki Krogerus heikki.kroge...@linux.intel.com Br, -- heikki -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2] serial: 8250_dw: Improve unwritable LCR workaround
When configured with UART_16550_COMPATIBLE=NO or in versions prior to
the introduction of this option, the Designware UART will ignore writes
to the LCR if the UART is busy. The current workaround saves a copy of
the last written LCR and re-writes it in the ISR for a special interrupt
that is raised when a write was ignored.
Unfortunately, interrupts are typically disabled prior to performing a
sequence of register writes that include the LCR so the point at which
the retry occurs is too late. An example is serial8250_do_set_termios()
where an ignored LCR write results in the baud divisor not being set and
instead a garbage character is sent out the transmitter.
Furthermore, since serial_port_out() offers no way to indicate failure,
a serious effort must be made to ensure that the LCR is actually updated
before returning back to the caller. This is difficult, however, as a
UART that was busy during the first attempt is likely to still be busy
when a subsequent attempt is made unless some extra action is taken.
This updated workaround reads back the LCR after each write to confirm
that the new value was accepted by the hardware. Should the hardware
ignore a write, the TX/RX FIFOs are cleared and the receive buffer read
before attempting to rewrite the LCR out of the hope that doing so will
force the UART into an idle state. While this may seem unnecessarily
aggressive, 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. Admittedly, this is far from ideal but it seems to be the
best that can be done given the hardware limitations.
Lastly, the revised workaround doesn't touch the LCR in the ISR, so it
avoids the possibility of a "serial8250: too much work for irq" lock up.
This problem is rare in real situations but can be reproduced easily by
wiring up two UARTs and running the following commands.
# stty -F /dev/ttyS1 echo
# stty -F /dev/ttyS2 echo
# cat /dev/ttyS1 &
[1] 375
# echo asdf > /dev/ttyS1
asdf
[ 27.70] serial8250: too much work for irq96
[ 27.70] serial8250: too much work for irq96
[ 27.71] serial8250: too much work for irq96
[ 27.71] serial8250: too much work for irq96
[ 27.72] serial8250: too much work for irq96
[ 27.72] serial8250: too much work for irq96
[ 27.73] serial8250: too much work for irq96
[ 27.73] serial8250: too much work for irq96
[ 27.74] serial8250: too much work for irq96
Signed-off-by: Tim Kryger
Reviewed-by: Matt Porter
Reviewed-by: Markus Mayer
---
Changes in v2:
- Rebased on tty-next
- Updated commit messsage to mention UART_16550_COMPATIBLE
- Removed potentially unnecessary read of LSR and MSR
- Only attempt workaround when LCR write is ignored
drivers/tty/serial/8250/8250_dw.c | 41 ++-
1 file changed, 32 insertions(+), 9 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_dw.c
b/drivers/tty/serial/8250/8250_dw.c
index d04a037..4658e3e 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -57,7 +57,6 @@
struct dw8250_data {
u8 usr_reg;
- int last_lcr;
int last_mcr;
int line;
struct clk *clk;
@@ -77,17 +76,33 @@ static inline int dw8250_modify_msr(struct uart_port *p,
int offset, int value)
return value;
}
+static void dw8250_force_idle(struct uart_port *p)
+{
+ serial8250_clear_and_reinit_fifos(container_of
+ (p, struct uart_8250_port, port));
+ (void)p->serial_in(p, UART_RX);
+}
+
static void dw8250_serial_out(struct uart_port *p, int offset, int value)
{
struct dw8250_data *d = p->private_data;
- if (offset == UART_LCR)
- d->last_lcr = value;
-
if (offset == UART_MCR)
d->last_mcr = value;
writeb(value, p->membase + (offset << p->regshift));
+
+ /* Make sure LCR write wasn't ignored */
+ if (offset == UART_LCR) {
+ int tries = 1000;
+ while (tries--) {
+ if (value == p->serial_in(p, UART_LCR))
+ return;
+ dw8250_force_idle(p);
+ writeb(value, p->membase + (UART_LCR << p->regshift));
+ }
+ dev_err(p->dev, "Couldn't set LCR to %d\n", value);
+ }
}
static unsigned int dw8250_serial_in(struct uart_port *p, int offset)
@@ -108,13 +123,22 @@ static void dw8250_serial_out32(struct uart_port *p, int
offset, int value)
{
struct dw8250_data *d = p->private_data;
- if (offset == UART_LCR)
- d->last_lcr = value;
-
if (offset == UART_MCR)
d->last_mcr = value;
writel(value, p->membase + (offset << p->regshift));
+
+
[PATCH v2] serial: 8250_dw: Improve unwritable LCR workaround
When configured with UART_16550_COMPATIBLE=NO or in versions prior to
the introduction of this option, the Designware UART will ignore writes
to the LCR if the UART is busy. The current workaround saves a copy of
the last written LCR and re-writes it in the ISR for a special interrupt
that is raised when a write was ignored.
Unfortunately, interrupts are typically disabled prior to performing a
sequence of register writes that include the LCR so the point at which
the retry occurs is too late. An example is serial8250_do_set_termios()
where an ignored LCR write results in the baud divisor not being set and
instead a garbage character is sent out the transmitter.
Furthermore, since serial_port_out() offers no way to indicate failure,
a serious effort must be made to ensure that the LCR is actually updated
before returning back to the caller. This is difficult, however, as a
UART that was busy during the first attempt is likely to still be busy
when a subsequent attempt is made unless some extra action is taken.
This updated workaround reads back the LCR after each write to confirm
that the new value was accepted by the hardware. Should the hardware
ignore a write, the TX/RX FIFOs are cleared and the receive buffer read
before attempting to rewrite the LCR out of the hope that doing so will
force the UART into an idle state. While this may seem unnecessarily
aggressive, 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. Admittedly, this is far from ideal but it seems to be the
best that can be done given the hardware limitations.
Lastly, the revised workaround doesn't touch the LCR in the ISR, so it
avoids the possibility of a serial8250: too much work for irq lock up.
This problem is rare in real situations but can be reproduced easily by
wiring up two UARTs and running the following commands.
# stty -F /dev/ttyS1 echo
# stty -F /dev/ttyS2 echo
# cat /dev/ttyS1
[1] 375
# echo asdf /dev/ttyS1
asdf
[ 27.70] serial8250: too much work for irq96
[ 27.70] serial8250: too much work for irq96
[ 27.71] serial8250: too much work for irq96
[ 27.71] serial8250: too much work for irq96
[ 27.72] serial8250: too much work for irq96
[ 27.72] serial8250: too much work for irq96
[ 27.73] serial8250: too much work for irq96
[ 27.73] serial8250: too much work for irq96
[ 27.74] serial8250: too much work for irq96
Signed-off-by: Tim Kryger tim.kry...@linaro.org
Reviewed-by: Matt Porter matt.por...@linaro.org
Reviewed-by: Markus Mayer markus.ma...@linaro.org
---
Changes in v2:
- Rebased on tty-next
- Updated commit messsage to mention UART_16550_COMPATIBLE
- Removed potentially unnecessary read of LSR and MSR
- Only attempt workaround when LCR write is ignored
drivers/tty/serial/8250/8250_dw.c | 41 ++-
1 file changed, 32 insertions(+), 9 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_dw.c
b/drivers/tty/serial/8250/8250_dw.c
index d04a037..4658e3e 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -57,7 +57,6 @@
struct dw8250_data {
u8 usr_reg;
- int last_lcr;
int last_mcr;
int line;
struct clk *clk;
@@ -77,17 +76,33 @@ static inline int dw8250_modify_msr(struct uart_port *p,
int offset, int value)
return value;
}
+static void dw8250_force_idle(struct uart_port *p)
+{
+ serial8250_clear_and_reinit_fifos(container_of
+ (p, struct uart_8250_port, port));
+ (void)p-serial_in(p, UART_RX);
+}
+
static void dw8250_serial_out(struct uart_port *p, int offset, int value)
{
struct dw8250_data *d = p-private_data;
- if (offset == UART_LCR)
- d-last_lcr = value;
-
if (offset == UART_MCR)
d-last_mcr = value;
writeb(value, p-membase + (offset p-regshift));
+
+ /* Make sure LCR write wasn't ignored */
+ if (offset == UART_LCR) {
+ int tries = 1000;
+ while (tries--) {
+ if (value == p-serial_in(p, UART_LCR))
+ return;
+ dw8250_force_idle(p);
+ writeb(value, p-membase + (UART_LCR p-regshift));
+ }
+ dev_err(p-dev, Couldn't set LCR to %d\n, value);
+ }
}
static unsigned int dw8250_serial_in(struct uart_port *p, int offset)
@@ -108,13 +123,22 @@ static void dw8250_serial_out32(struct uart_port *p, int
offset, int value)
{
struct dw8250_data *d = p-private_data;
- if (offset == UART_LCR)
- d-last_lcr = value;
-
if (offset == UART_MCR)
d-last_mcr = value;
writel(value,
