2020-05-25 19:41:06

by Vladimir Oltean

[permalink] [raw]
Subject: [PATCH] serial: 8250: probe all 16550A variants by default

From: Vladimir Oltean <[email protected]>

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 <[email protected]>
---
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


2020-05-25 21:34:33

by Josh Triplett

[permalink] [raw]
Subject: Re: [PATCH] serial: 8250: probe all 16550A variants by default

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
>

2020-05-25 21:50:46

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH] serial: 8250: probe all 16550A variants by default

Hi Josh,

On Mon, 25 May 2020 at 20:28, Josh Triplett <[email protected]> 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

2020-05-25 21:55:29

by Josh Triplett

[permalink] [raw]
Subject: Re: [PATCH] serial: 8250: probe all 16550A variants by default

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 <[email protected]> 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

2020-05-26 08:38:27

by Josh Triplett

[permalink] [raw]
Subject: Re: [PATCH] serial: 8250: probe all 16550A variants by default

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

2020-05-26 11:29:26

by Maxim Kochetkov

[permalink] [raw]
Subject: Re: [PATCH] serial: 8250: probe all 16550A variants by default

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.