Re: [PATCH] serial: 8250: Default SERIAL_OF_PLATFORM to SERIAL_8250

[Guenter Roeck]

On Wed, Nov 14, 2018 at 07:56:47PM -0800, Florian Fainelli wrote:
> 
> 
> On November 14, 2018 5:11:25 PM PST, Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
> >On Thu, Nov 01, 2018 at 11:26:06AM -0700, Florian Fainelli wrote:
> >> It is way too easy to miss enabling SERIAL_OF_PLATFORM which would
> >> result in the inability for the kernel to have a valid console
> >device,
> >> which can be seen with:
> >> 
> >> Warning: unable to open an initial console.
> >> 
> >> and then:
> >> 
> >> Run /init as init process
> >> Kernel panic - not syncing: Attempted to kill init!
> >exitcode=0x00000100
> >> 
> >> Since SERIAL_OF_PLATFORM already depends on SERIAL_8250 && OF there
> >> really is no drawback to defaulting this config to the value of
> >> SERIAL_8250.
> >> 
> >> Signed-off-by: Florian Fainelli <f.fainelli@xxxxxxxxx>
> >> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> >
> >This patch results in situations where CONFIG_SERIAL_OF_PLATFORM is now
> >defined where it was not previously. Example mpc85xx_defconfig. This in
> >turn results in boot failures for those configurations, with an error
> >message of
> >
> >of_serial: probe of e0004500.serial failed with error -22
> >
> >which wasn't seen before.
> 
> Do you know which Device Tree is being used here? The most obvious thing that could be done is to add a !PPC condition but this might be missing other platforms doing their own 8250 registration yet being OF aware (sparc?).

This is a qemu boot test. No, I don't know what exactly is happening,
except that this (emulated) system obviously does not expect
CONFIG_SERIAL_OF_PLATFORM to be enabled and, afaik, the devicetree
is generated internally by qemu. I would have thought that just enabling
a configuration by default out of the blue might be considered problematic
by itself, but maybe I am wrong.

> 
> >
> >Not sure if replacing a potential problem with a real one is really an
> >improvement.`
> 
> That comment is not particularly helpful though I have an appreciation for when a change breaks things in unexpected ways and how frustrating that can be.

What is really frustrating (and let me think about dropping all those
boot tests) is that one ends up having to argue if the problem is real
or only applies to a presumably or possibly wrong qemu emulation, and
that one ends up having to discuss the validity of the test case. 

Since this is "only" an emulation and thus not a "real" system,
please feel free to ignore this report. I'll just drop all boot tests
using this configuration once the patch hits mainline.

Guenter