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

Simon Glass
2015-08-31T23:12:27Z

Hi Michal,

On 31 August 2015 at 08:08, Michal Simek <monstr@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@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@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