Xen devel - Aug 2013 - Re: [PATCH V1 17/29] xen/arm: Mark each device used by Xen as disabled in DOM0 FDT

Hi Julien
> On some board, there is no alias to the UART. To avoid modification in
> the device tree, dt-uart should also search device by path.
> 
> To distinguish an alias from a path, dt-uart will check the first
character.
> If it''s a / then it''s path, otherwise it''s an
alias.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> Acked-by: Ian Cambell <ian.campbell@xxxxxxxxxx>
> ---
>  xen/drivers/char/dt-uart.c |   16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/drivers/char/dt-uart.c b/xen/drivers/char/dt-uart.c
> index 93bb0f5..d7204fb 100644
> --- a/xen/drivers/char/dt-uart.c
> +++ b/xen/drivers/char/dt-uart.c
> @@ -26,9 +26,10 @@
>  
>  /*
>   * Configure UART port with a string:
> - * alias,options
> + * path,options
>   *
> - * @alias: alias used in the device tree for the UART
> + * @path: full path used in the device tree for the UART. If the path
> + * doesn''t start with ''/'', we assuming that
it''s an alias.
>   * @options: UART speficic options (see in each UART driver)
>   */
>  static char __initdata opt_dtuart[30] = "";
> @@ -38,7 +39,7 @@ void __init dt_uart_init(void)
>  {
>      struct dt_device_node *dev;
>      int ret;
> -    const char *devalias = opt_dtuart;
> +    const char *devpath = opt_dtuart;
>      char *options;
>  
>      if ( !console_has("dtuart") || !strcmp(opt_dtuart,
"") )
> @@ -53,12 +54,15 @@ void __init dt_uart_init(void)
>      else
>          options = "";
>  
> -    early_printk("Looking for UART console %s\n", devalias);
> -    dev = dt_find_node_by_alias(devalias);
> +    early_printk("Looking for UART console %s\n", devpath);
> +    if ( *devpath == ''/'' )
> +        dev = dt_find_node_by_path(devpath);
> +    else
> +        dev = dt_find_node_by_alias(devpath);
if ''/'' start an alias, it should be
+    if ( *devpath == ''/'' )
+        dev = dt_find_node_by_alias(devpath);
+    else
+        dev = dt_find_node_by_path(devpath);




bamvor
>  
>      if ( !dev )
>      {
> -        early_printk("Unable to find device \"%s\"\n",
devalias);
> +        early_printk("Unable to find device \"%s\"\n",
devpath);
>          return;
>      }
>  
> -- 
> 1.7.10.4

On Wed, 2013-08-28 at 19:19 -0600, Bamvor Jian Zhang wrote:
> > -    early_printk("Looking for UART console %s\n",
devalias);
> > -    dev = dt_find_node_by_alias(devalias);
> > +    early_printk("Looking for UART console %s\n", devpath);
> > +    if ( *devpath == ''/'' )
> > +        dev = dt_find_node_by_path(devpath);
> > +    else
> > +        dev = dt_find_node_by_alias(devpath);
> if ''/'' start an alias, it should be
I think /  is the start of a path not an alias, so the current code is
correct.
> +    if ( *devpath == ''/'' )
> +        dev = dt_find_node_by_alias(devpath);
> +    else
> +        dev = dt_find_node_by_path(devpath);
> 
Ian.

On 08/29/2013 02:19 AM, Bamvor Jian Zhang wrote:
> Hi Julien
Hi Bamvor,
>> On some board, there is no alias to the UART. To avoid modification in
>> the device tree, dt-uart should also search device by path.
>>
>> To distinguish an alias from a path, dt-uart will check the first
character.
>> If it''s a / then it''s path, otherwise it''s
an alias.
>>
>> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
>> Acked-by: Ian Cambell <ian.campbell@xxxxxxxxxx>
>> ---
>>  xen/drivers/char/dt-uart.c |   16 ++++++++++------
>>  1 file changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/xen/drivers/char/dt-uart.c b/xen/drivers/char/dt-uart.c
>> index 93bb0f5..d7204fb 100644
>> --- a/xen/drivers/char/dt-uart.c
>> +++ b/xen/drivers/char/dt-uart.c
>> @@ -26,9 +26,10 @@
>>  
>>  /*
>>   * Configure UART port with a string:
>> - * alias,options
>> + * path,options
>>   *
>> - * @alias: alias used in the device tree for the UART
>> + * @path: full path used in the device tree for the UART. If the path
>> + * doesn''t start with ''/'', we assuming that
it''s an alias.
>>   * @options: UART speficic options (see in each UART driver)
>>   */
>>  static char __initdata opt_dtuart[30] = "";
>> @@ -38,7 +39,7 @@ void __init dt_uart_init(void)
>>  {
>>      struct dt_device_node *dev;
>>      int ret;
>> -    const char *devalias = opt_dtuart;
>> +    const char *devpath = opt_dtuart;
>>      char *options;
>>  
>>      if ( !console_has("dtuart") || !strcmp(opt_dtuart,
"") )
>> @@ -53,12 +54,15 @@ void __init dt_uart_init(void)
>>      else
>>          options = "";
>>  
>> -    early_printk("Looking for UART console %s\n", devalias);
>> -    dev = dt_find_node_by_alias(devalias);
>> +    early_printk("Looking for UART console %s\n", devpath);
>> +    if ( *devpath == ''/'' )
>> +        dev = dt_find_node_by_path(devpath);
>> +    else
>> +        dev = dt_find_node_by_alias(devpath);
> if ''/'' start an alias, it should be
From the ePAR
(http://www.power.org/resources/downloads/Power_ePAPR_APPROVED_v1.0.pdf)
section 3.3: an alias name must match the following pattern: [0-9a-z-]+
So we can''t find ''/'' in the alias.
> +    if ( *devpath == ''/'' )
> +        dev = dt_find_node_by_alias(devpath);
> +    else
> +        dev = dt_find_node_by_path(devpath);
A path always start by ''/''. With your solution, it
won''t be possible to
get the UART by path.

-- 
Julien Grall