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

[Florian Fainelli]

On 11/15/18 9:25 AM, Guenter Roeck wrote:
> On Thu, Nov 15, 2018 at 09:19:14AM -0800, Florian Fainelli wrote:
>>
>>
>> On 11/14/2018 9:38 PM, Guenter Roeck wrote:
>>> 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?).
>>>>
>>>>>
>>>>> 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.
>>>
>>> Actally, never mind. I dropped the test cases. Sorry for the noise.
>>
>> Why? The tests are useful, if I gave you an impression that I was just
>> going to walk away from this issue and not look at it, then that is not
>> happening. What I was objecting to is your qualification of the issue,
>> this is unfortunately not a potential/latent problem, it happens more
>> often than not.
>>
> 
> I ended up adding a flag to my builders which remove the offending
> configuration for affected platforms/configurations, so this is no longer
> an issue for my build tests.

OK, would you mind testing this below? It seems to me that 8250_of.c is
incompatible with arch/powerpc/kernel/legacy_serial.c and that is what
is causing the issue here.

diff --git a/drivers/tty/serial/8250/Kconfig
b/drivers/tty/serial/8250/Kconfig
index d7737dca0e48..21cb14cbd34a 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -483,7 +483,7 @@ config SERIAL_8250_PXA

 config SERIAL_OF_PLATFORM
        tristate "Devicetree based probing for 8250 ports"
-       depends on SERIAL_8250 && OF
+       depends on SERIAL_8250 && OF && !(PPC && PPC_UDBG_16550)
        default SERIAL_8250
        help
          This option is used for all 8250 compatible serial ports that
-- 
Florian