* [PATCH] serial: 8250: probe all 16550A variants by default
@ 2020-05-25 13:02 Vladimir Oltean
2020-05-25 13:40 ` Andy Shevchenko
2020-05-25 17:28 ` Josh Triplett
0 siblings, 2 replies; 7+ messages in thread
From: Vladimir Oltean @ 2020-05-25 13:02 UTC (permalink / raw)
To: gregkh, linux-serial
Cc: jslaby, andriy.shevchenko, lukas, heikki.krogerus, vigneshr,
linux-kernel, fido_max, radu-andrei.bulie
From: Vladimir Oltean <vladimir.oltean@nxp.com>
On NXP T1040, the UART is typically detected as 16550A_FSL64. After said
patch, it gets detected as plain 16550A and the Linux console is
completely garbled and missing characters.
So clearly, introducing the SERIAL_8250_16550A_VARIANTS config option
has broken many existing users because it has changed the default
behavior. Restore that by adding a 'default y' to this option. Users who
care about 20 ms shorter boot time can always disable it, but stop
wasting many debugging hours for people who don't care all that much.
Fixes: dc56ecb81a0a ("serial: 8250: Support disabling mdelay-filled probes of 16550A variants")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/tty/serial/8250/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index af0688156dd0..89c7ecb55619 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -63,6 +63,7 @@ config SERIAL_8250_PNP
config SERIAL_8250_16550A_VARIANTS
bool "Support for variants of the 16550A serial port"
depends on SERIAL_8250
+ default y
help
The 8250 driver can probe for many variants of the venerable 16550A
serial port. Doing so takes additional time at boot.
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] serial: 8250: probe all 16550A variants by default
2020-05-25 13:02 [PATCH] serial: 8250: probe all 16550A variants by default Vladimir Oltean
@ 2020-05-25 13:40 ` Andy Shevchenko
2020-05-25 17:28 ` Josh Triplett
1 sibling, 0 replies; 7+ messages in thread
From: Andy Shevchenko @ 2020-05-25 13:40 UTC (permalink / raw)
To: Vladimir Oltean
Cc: gregkh, linux-serial, jslaby, lukas, heikki.krogerus, vigneshr,
linux-kernel, fido_max, radu-andrei.bulie
On Mon, May 25, 2020 at 04:02:38PM +0300, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
>
> On NXP T1040, the UART is typically detected as 16550A_FSL64. After said
> patch, it gets detected as plain 16550A and the Linux console is
> completely garbled and missing characters.
>
> So clearly, introducing the SERIAL_8250_16550A_VARIANTS config option
> has broken many existing users because it has changed the default
> behavior. Restore that by adding a 'default y' to this option. Users who
> care about 20 ms shorter boot time can always disable it, but stop
> wasting many debugging hours for people who don't care all that much.
While this sounds like a good enough fix of the regression, the proper one
seems to have a separate quirk driver which avoids auto probe and uses
compatible strings or whatever that board can supply. 8250_port historically
has a burden of old and buggy hardware, that's why the above mentioned patch
exposed that.
That said, I have no objections, but had to note about possible approaches.
> Fixes: dc56ecb81a0a ("serial: 8250: Support disabling mdelay-filled probes of 16550A variants")
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> drivers/tty/serial/8250/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
> index af0688156dd0..89c7ecb55619 100644
> --- a/drivers/tty/serial/8250/Kconfig
> +++ b/drivers/tty/serial/8250/Kconfig
> @@ -63,6 +63,7 @@ config SERIAL_8250_PNP
> config SERIAL_8250_16550A_VARIANTS
> bool "Support for variants of the 16550A serial port"
> depends on SERIAL_8250
> + default y
> help
> The 8250 driver can probe for many variants of the venerable 16550A
> serial port. Doing so takes additional time at boot.
> --
> 2.25.1
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] serial: 8250: probe all 16550A variants by default
2020-05-25 13:02 [PATCH] serial: 8250: probe all 16550A variants by default Vladimir Oltean
2020-05-25 13:40 ` Andy Shevchenko
@ 2020-05-25 17:28 ` Josh Triplett
2020-05-25 18:52 ` Vladimir Oltean
2020-05-26 7:05 ` Maxim Kochetkov
1 sibling, 2 replies; 7+ messages in thread
From: Josh Triplett @ 2020-05-25 17:28 UTC (permalink / raw)
To: Vladimir Oltean
Cc: gregkh, linux-serial, jslaby, andriy.shevchenko, lukas,
heikki.krogerus, vigneshr, linux-kernel, fido_max,
radu-andrei.bulie
On Mon, May 25, 2020 at 04:02:38PM +0300, Vladimir Oltean wrote:
> On NXP T1040, the UART is typically detected as 16550A_FSL64. After said
> patch, it gets detected as plain 16550A and the Linux console is
> completely garbled and missing characters.
Interesting that there's *new* powerpc hardware that needs these
variants. I based the patch on the fact that, on x86 at least, hardware
using these variants hasn't been made for a long time.
In the hopes of preserving at least part of the benefit of the patch,
could you please change it to `default y if !X86_64`?
> drivers/tty/serial/8250/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
> index af0688156dd0..89c7ecb55619 100644
> --- a/drivers/tty/serial/8250/Kconfig
> +++ b/drivers/tty/serial/8250/Kconfig
> @@ -63,6 +63,7 @@ config SERIAL_8250_PNP
> config SERIAL_8250_16550A_VARIANTS
> bool "Support for variants of the 16550A serial port"
> depends on SERIAL_8250
> + default y
> help
> The 8250 driver can probe for many variants of the venerable 16550A
> serial port. Doing so takes additional time at boot.
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] serial: 8250: probe all 16550A variants by default
2020-05-25 17:28 ` Josh Triplett
@ 2020-05-25 18:52 ` Vladimir Oltean
2020-05-25 20:48 ` Josh Triplett
2020-05-26 7:05 ` Maxim Kochetkov
1 sibling, 1 reply; 7+ messages in thread
From: Vladimir Oltean @ 2020-05-25 18:52 UTC (permalink / raw)
To: Josh Triplett
Cc: Greg Kroah-Hartman, linux-serial, jslaby, andriy.shevchenko,
lukas, heikki.krogerus, vigneshr, lkml, fido_max,
radu-andrei.bulie
Hi Josh,
On Mon, 25 May 2020 at 20:28, Josh Triplett <josh@joshtriplett.org> wrote:
>
> On Mon, May 25, 2020 at 04:02:38PM +0300, Vladimir Oltean wrote:
> > On NXP T1040, the UART is typically detected as 16550A_FSL64. After said
> > patch, it gets detected as plain 16550A and the Linux console is
> > completely garbled and missing characters.
>
> Interesting that there's *new* powerpc hardware that needs these
> variants. I based the patch on the fact that, on x86 at least, hardware
> using these variants hasn't been made for a long time.
>
> In the hopes of preserving at least part of the benefit of the patch,
> could you please change it to `default y if !X86_64`?
>
Why don't you add CONFIG_SERIAL_8250_16550A_VARIANTS=n in x86_64_defconfig?
> > drivers/tty/serial/8250/Kconfig | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
> > index af0688156dd0..89c7ecb55619 100644
> > --- a/drivers/tty/serial/8250/Kconfig
> > +++ b/drivers/tty/serial/8250/Kconfig
> > @@ -63,6 +63,7 @@ config SERIAL_8250_PNP
> > config SERIAL_8250_16550A_VARIANTS
> > bool "Support for variants of the 16550A serial port"
> > depends on SERIAL_8250
> > + default y
> > help
> > The 8250 driver can probe for many variants of the venerable 16550A
> > serial port. Doing so takes additional time at boot.
> > --
> > 2.25.1
> >
Thanks,
-Vladimir
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] serial: 8250: probe all 16550A variants by default
2020-05-25 18:52 ` Vladimir Oltean
@ 2020-05-25 20:48 ` Josh Triplett
0 siblings, 0 replies; 7+ messages in thread
From: Josh Triplett @ 2020-05-25 20:48 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Greg Kroah-Hartman, linux-serial, jslaby, andriy.shevchenko,
lukas, heikki.krogerus, vigneshr, lkml, fido_max,
radu-andrei.bulie
On Mon, May 25, 2020 at 09:52:54PM +0300, Vladimir Oltean wrote:
> Hi Josh,
>
> On Mon, 25 May 2020 at 20:28, Josh Triplett <josh@joshtriplett.org> wrote:
> >
> > On Mon, May 25, 2020 at 04:02:38PM +0300, Vladimir Oltean wrote:
> > > On NXP T1040, the UART is typically detected as 16550A_FSL64. After said
> > > patch, it gets detected as plain 16550A and the Linux console is
> > > completely garbled and missing characters.
> >
> > Interesting that there's *new* powerpc hardware that needs these
> > variants. I based the patch on the fact that, on x86 at least, hardware
> > using these variants hasn't been made for a long time.
> >
> > In the hopes of preserving at least part of the benefit of the patch,
> > could you please change it to `default y if !X86_64`?
> >
>
> Why don't you add CONFIG_SERIAL_8250_16550A_VARIANTS=n in x86_64_defconfig?
In general, it seems preferable to me when the defconfig files differ
less from the defaults encoded in Kconfig.
You're proposing a change to the default; could you please include one
or the other additional change in your patch to preserve the behavior on
x86_64?
> > > drivers/tty/serial/8250/Kconfig | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
> > > index af0688156dd0..89c7ecb55619 100644
> > > --- a/drivers/tty/serial/8250/Kconfig
> > > +++ b/drivers/tty/serial/8250/Kconfig
> > > @@ -63,6 +63,7 @@ config SERIAL_8250_PNP
> > > config SERIAL_8250_16550A_VARIANTS
> > > bool "Support for variants of the 16550A serial port"
> > > depends on SERIAL_8250
> > > + default y
> > > help
> > > The 8250 driver can probe for many variants of the venerable 16550A
> > > serial port. Doing so takes additional time at boot.
> > > --
> > > 2.25.1
> > >
>
> Thanks,
> -Vladimir
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] serial: 8250: probe all 16550A variants by default
2020-05-25 17:28 ` Josh Triplett
2020-05-25 18:52 ` Vladimir Oltean
@ 2020-05-26 7:05 ` Maxim Kochetkov
2020-05-26 8:35 ` Josh Triplett
1 sibling, 1 reply; 7+ messages in thread
From: Maxim Kochetkov @ 2020-05-26 7:05 UTC (permalink / raw)
To: Josh Triplett, Vladimir Oltean
Cc: gregkh, linux-serial, jslaby, andriy.shevchenko, lukas,
heikki.krogerus, vigneshr, linux-kernel, radu-andrei.bulie
This change breaks all my devices: OMAP-L138 (davinci based), LS1021A,
T1040, Marvell (kirkwood2 based). Only enabling VARIANTS on all my
devices fix the issue.
25.05.2020 20:28, Josh Triplett wrote:
> On Mon, May 25, 2020 at 04:02:38PM +0300, Vladimir Oltean wrote:
>> On NXP T1040, the UART is typically detected as 16550A_FSL64. After said
>> patch, it gets detected as plain 16550A and the Linux console is
>> completely garbled and missing characters.
> Interesting that there's*new* powerpc hardware that needs these
> variants. I based the patch on the fact that, on x86 at least, hardware
> using these variants hasn't been made for a long time.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] serial: 8250: probe all 16550A variants by default
2020-05-26 7:05 ` Maxim Kochetkov
@ 2020-05-26 8:35 ` Josh Triplett
0 siblings, 0 replies; 7+ messages in thread
From: Josh Triplett @ 2020-05-26 8:35 UTC (permalink / raw)
To: Maxim Kochetkov
Cc: Vladimir Oltean, gregkh, linux-serial, jslaby, andriy.shevchenko,
lukas, heikki.krogerus, vigneshr, linux-kernel,
radu-andrei.bulie
On Tue, May 26, 2020 at 10:05:25AM +0300, Maxim Kochetkov wrote:
> This change breaks all my devices: OMAP-L138 (davinci based), LS1021A,
> T1040, Marvell (kirkwood2 based). Only enabling VARIANTS on all my devices
> fix the issue.
Preparing a patch right now, with the appropriate Fixes tag so it should
end up on any kernel that has the original patch.
- Josh Triplett
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-05-26 8:35 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-25 13:02 [PATCH] serial: 8250: probe all 16550A variants by default Vladimir Oltean
2020-05-25 13:40 ` Andy Shevchenko
2020-05-25 17:28 ` Josh Triplett
2020-05-25 18:52 ` Vladimir Oltean
2020-05-25 20:48 ` Josh Triplett
2020-05-26 7:05 ` Maxim Kochetkov
2020-05-26 8:35 ` Josh Triplett
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).