Re: [Xen-devel] [RFC 15/24] xen/arm: Use dt_device_match to avoid multiple if conditions

[Julien Grall <julien.grall@xxxxxxxxxx>]

On 08/22/2013 02:30 PM, Ian Campbell wrote:
> On Fri, 2013-08-16 at 22:05 +0100, Julien Grall wrote:
>> There is some place in Xen ARM code where multiple if conditions is used
>> check the presence of a node or find a node.
>> These pieces of code can be replace by an array and using proper device tree
>> helpers.
>>
>> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
>> ---
>>  xen/arch/arm/domain_build.c |   12 +++++++++---
>>  xen/arch/arm/time.c         |   11 ++++++++---
>>  2 files changed, 17 insertions(+), 6 deletions(-)
>>
>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>> index a324c3b..604ec1c 100644
>> --- a/xen/arch/arm/domain_build.c
>> +++ b/xen/arch/arm/domain_build.c
>> @@ -435,6 +435,14 @@ static int map_device(struct domain *d, const struct 
>> dt_device_node *dev)
>>      return 0;
>>  }
>>  
>> +static const struct dt_device_match skip_matches[] __initconst =
>> +{
>> +    DT_MATCH_COMPATIBLE("xen,xen"),
>> +    DT_MATCH_TYPE("memory"),
>> +    DT_MATCH_PATH("/chosen"),
>> +    { /* sentinel */ },
>> +};
> 
> Can this not be within the scope of the only function which uses it?

Indeed. I will update it for the next patch series.

> 
> If not then perhaps it can be given a more meaningful name.
> dom0_skip_matches?
> 
>> +
>>  static int handle_node(struct domain *d, const struct dt_device_node *np)
>>  {
>>      const struct dt_device_node *child;
>> @@ -443,9 +451,7 @@ static int handle_node(struct domain *d, const struct 
>> dt_device_node *np)
>>      DPRINT("handle %s\n", dt_node_full_name(np));
>>  
>>      /* Skip theses nodes and the sub-nodes */
>> -    if ( dt_device_is_compatible(np, "xen,xen") ||
>> -         dt_device_type_is_equal(np, "memory") ||
>> -         !strcmp("/chosen", dt_node_full_name(np)) )
>> +    if ( dt_match_node(skip_matches, np ) )
>>          return 0;
>>  
>>      if ( dt_device_used_by(np) != DOMID_XEN )
>> diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
>> index b5864ef..05e58ba 100644
>> --- a/xen/arch/arm/time.c
>> +++ b/xen/arch/arm/time.c
>> @@ -98,6 +98,13 @@ static uint32_t calibrate_timer(void)
>>  }
>>  */
>>  
>> +static const struct dt_device_match timer_ids[] __initdata =
>> +{
>> +    DT_MATCH_COMPATIBLE("arm,armv7-timer"),
>> +    DT_MATCH_COMPATIBLE("arm,armv8-timer"),
>> +    { /* sentinel */ },
>> +};
>> +
>>  /* Set up the timer on the boot CPU */
>>  int __init init_xen_time(void)
>>  {
>> @@ -105,9 +112,7 @@ int __init init_xen_time(void)
>>      int res;
>>      unsigned int i;
>>  
>> -    dev = dt_find_compatible_node(NULL, NULL, "arm,armv7-timer");
>> -    if ( !dev )
>> -        dev = dt_find_compatible_node(NULL, NULL, "arm,armv8-timer");
>> +    dev = dt_find_matching_node(NULL, timer_ids);
>>      if ( !dev )
>>          panic("Unable to find a compatible timer in the device tree\n");
>>  
> 
> 


-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel