[U-Boot] [PATCH 1/3] dm: serial: Do not panic if no serial ports are found

[Simon Glass]

Hi Hans,

On 8 July 2015 at 08:16, Hans de Goede <hdegoede at redhat.com> wrote:
> Hi,
>
>
> On 08-07-15 16:08, Simon Glass wrote:
>>
>> +Masahiro
>>
>> Hi Hans,
>>
>> On 8 July 2015 at 05:56, Hans de Goede <hdegoede at redhat.com> wrote:
>>>
>>> Hi Simon,
>>>
>>>
>>> On 07-07-15 09:00, Hans de Goede wrote:
>>>>
>>>>
>>>> Hi,
>>>>
>>>> On 06-07-15 18:39, Simon Glass wrote:
>>>>>
>>>>>
>>>>> Hi Hans,
>>>>>
>>>>> On 5 July 2015 at 12:56, Hans de Goede <hdegoede at redhat.com> wrote:
>>>>>>
>>>>>>
>>>>>> Some boards simply do not have any serial ports. Also no one will see
>>>>>> the
>>>>>> panic message as there is no where to print it if no serial port is
>>>>>> found
>>>>>> (and other stdout options are not yet set up at this point).
>>>>>>
>>>>>
>>>>> It is visible (or will be when some patches land) if you have a debug
>>>>> UART set up.
>>>>
>>>>
>>>>
>>>> Ok.
>>>>
>>>>>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>>>>>> ---
>>>>>>    drivers/serial/serial-uclass.c | 8 +++-----
>>>>>>    1 file changed, 3 insertions(+), 5 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/serial/serial-uclass.c
>>>>>> b/drivers/serial/serial-uclass.c
>>>>>> index 815fec3..f036499 100644
>>>>>> --- a/drivers/serial/serial-uclass.c
>>>>>> +++ b/drivers/serial/serial-uclass.c
>>>>>> @@ -27,7 +27,7 @@ static const unsigned long baudrate_table[] =
>>>>>> CONFIG_SYS_BAUDRATE_TABLE;
>>>>>>    #error "Serial is required before relocation - define
>>>>>> CONFIG_SYS_MALLOC_F_LEN to make this work"
>>>>>>    #endif
>>>>>>
>>>>>> -static void serial_find_console_or_panic(void)
>>>>>> +static void serial_find_console(void)
>>>>>>    {
>>>>>>           struct udevice *dev;
>>>>>>           int node;
>>>>>> @@ -77,14 +77,12 @@ static void serial_find_console_or_panic(void)
>>>>>>                   }
>>>>>>    #undef INDEX
>>>>>>           }
>>>>>> -
>>>>>> -       panic_str("No serial driver found");
>>>>>>    }
>>>>>>
>>>>>>    /* Called prior to relocation */
>>>>>>    int serial_init(void)
>>>>>>    {
>>>>>> -       serial_find_console_or_panic();
>>>>>> +       serial_find_console();
>>>>>>           gd->flags |= GD_FLG_SERIAL_READY;
>>>>>>
>>>>>>           return 0;
>>>>>> @@ -93,7 +91,7 @@ int serial_init(void)
>>>>>>    /* Called after relocation */
>>>>>>    void serial_initialize(void)
>>>>>>    {
>>>>>> -       serial_find_console_or_panic();
>>>>>> +       serial_find_console();
>>>>>>    }
>>>>>>
>>>>>>    static void _serial_putc(struct udevice *dev, char ch)
>>>>>
>>>>>
>>>>>
>>>>> How is this handled before driver model?
>>>>
>>>>
>>>>
>>>> It was not, the boards involved all use the A13 SoC which is the
>>>> same die as the A10s but then in a different (cheaper) package
>>>> with way less pins. As such uart0 is not routed to the outside
>>>> on the A13. When not setting CONFIG_DM_SERIAL we are simply using
>>>> uart0 as serial console, and that ends at the bonding pads at the
>>>> edge of the die. doing things that way is not really useful,
>>>> but there is a serial console and u-boot is happy.
>>>>
>>>> In the non devicetree world this sort of was a natural hack, we
>>>> simply left CONSOLE_INDEX undefined for these boards, which causes
>>>> it to default to 1 / uart0 and things just work. I had never given
>>>> this much thought until moving to devicetree.
>>>>
>>>> As you can hopefully understand I do not want to do the samething
>>>> with DM_SERIAL builds as that would require hacking up the dts
>>>> to add a node for hardware which is effectively disabled for the
>>>> A13 package of the sun5i die.
>>>>
>>>>   > It is possible to mark a
>>>>>
>>>>>
>>>>> device disabled in device tree - perhaps we should have a way to see
>>>>> that the console is present (i.e. there is an alias) but it is
>>>>> disabled?
>>>>
>>>>
>>>>
>>>> That seems rather ugly, I really do not want to add an alias to
>>>> a non enabled device, that just feels wrong on all levels.
>>>>
>>>>> Normally a serial console is required, so I'd like to preserve this
>>>>> behaviour for most boards.
>>>>
>>>>
>>>>
>>>> Ok, I actually expected as much, this was actually my second solution
>>>> for the problem at hand, but as it was the simplest solution I decided
>>>> to submit this version first. My first / original solution is to add a
>>>> CONFIG_DM_SERIAL_NO_PANIC Kconfig option which can then be set on boards
>>>> which have no serial port.
>>>>
>>>> Or probably better a CONFIG_DM_SERIAL_NO_SERIAL_CONSOLE option ?
>>>>
>>>> This option would disable the panic / disable probing all together while
>>>> we would keep using DM_SERIAL on these boards, because:
>>>>
>>>> 1) Consistency I really want to use DM_FOO everywhere for sunxi
>>>> 2) The non dm serial code cannot handle not having a serial port either,
>>>>      and fixing it looks like it is going to be a bit harder (from a
>>>> quick
>>>>      glance).
>>>
>>>
>>>
>>> You probably just have not gotten around to this yet, but in case it is
>>> not clear, the above solution (adding a
>>> CONFIG_DM_SERIAL_NO_SERIAL_CONSOLE
>>> Kconfig option) is a suggestion on how to fix. I'll happily implement
>>> this
>>> if people like it, but atm I'm waiting for feedback on the suggestion.
>>>
>>
>> I've been thinking about it. Perhaps an option to allow U-Boot to run
>> without a console is best. But instead of CONFIG_DM_SERIAL_NO_PANIC,
>> perhaps we could have CONFIG_REQUIRE_CONSOLE which is normally y.
>
>
> That works for me, although I would prefer CONFIG_REQUIRE_SERIAL_CONSOLE,
> the boards in question do have a console in the form of a cfb console.

SGTM thanks Hans.

Regards,
Simon