[U-Boot] [PATCH 04/13] dm: serial: Deal with stdout-path with an alias

[Simon Glass]

Hi Michal,

On 31 August 2015 at 08:08, Michal Simek <monstr at monstr.eu> wrote:
> On 08/31/2015 03:54 PM, Simon Glass wrote:
>> Hi Michal,
>>
>> On 31 August 2015 at 05:13, Michal Simek <monstr at monstr.eu> wrote:
>>> On 08/29/2015 05:10 PM, Simon Glass wrote:
>>>> Sometimes stdout-path contains a UART alias along with speed, etc. For
>>>> example:
>>>>
>>>>       stdout-path = "serial0:115200n8";
>>>>
>>>> Add support for decoding this.
>>>>
>>>> Signed-off-by: Simon Glass <sjg at chromium.org>
>>>> ---
>>>>
>>>>  drivers/serial/serial-uclass.c | 30 +++++++++++++++++++++++++-----
>>>>  1 file changed, 25 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
>>>> index 55011cc..842f78b 100644
>>>> --- a/drivers/serial/serial-uclass.c
>>>> +++ b/drivers/serial/serial-uclass.c
>>>> @@ -29,14 +29,34 @@ static const unsigned long baudrate_table[] = CONFIG_SYS_BAUDRATE_TABLE;
>>>>
>>>>  static void serial_find_console_or_panic(void)
>>>>  {
>>>> +     const void *blob = gd->fdt_blob;
>>>
>>> This is one change and should be separated from the rest.
>>
>> Yes I should do that.
>>
>>>
>>>>       struct udevice *dev;
>>>>       int node;
>>>>
>>>> -     if (CONFIG_IS_ENABLED(OF_CONTROL) && gd->fdt_blob) {
>>>> +     if (CONFIG_IS_ENABLED(OF_CONTROL) && blob) {
>>>>               /* Check for a chosen console */
>>>> -             node = fdtdec_get_chosen_node(gd->fdt_blob, "stdout-path");
>>>> +             node = fdtdec_get_chosen_node(blob, "stdout-path");
>>>> +             if (node < 0) {
>>>> +                     const char *str, *p, *name;
>>>> +
>>>> +                     /*
>>>> +                      * Deal with things like
>>>> +                      *      stdout-path = "serial0:115200n8";
>>>> +                      *
>>>> +                      * We need to look up the alias and then follow it to
>>>> +                      * the correct node.
>>>> +                      */
>>>> +                     str = fdtdec_get_chosen_prop(blob, "stdout-path");
>>>> +                     if (str) {
>>>> +                             p = strchr(str, ':');
>>>> +                             name = fdt_get_alias_namelen(blob, str,
>>>> +                                             p ? p - str : strlen(str));
>>>> +                             if (name)
>>>> +                                     node = fdt_path_offset(blob, name);
>>>> +                     }
>>>> +             }
>>>
>>> And this is second.
>>>
>>>>               if (node < 0)
>>>
>>> You have if (node < 0) above too which looks pretty odd.
>>
>> OK, but that's what I want to check...
>
> I believe you but then I would expect you have just one
> if (node < 0) in the code not two.

OK I will add a comment.

Regards,
Simon