Re: [RFC for-4.5 05/12] xen/dts: Add dt_parse_phandle_with_args and dt_parse_phandle

[Julien Grall <julien.grall@linaro.org>]

Hello Ian,

On 02/19/2014 12:34 PM, Ian Campbell wrote:
> On Fri, 2014-02-07 at 17:43 +0000, Julien Grall wrote:
>> Code adapted from linux drivers/of/base.c (commit ef42c58).
> 
> On that basis I only took a cursory glance through that monster
> function.
> 
>>
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>> diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
>> index 7c075d9..d429e60 100644
>> --- a/xen/include/xen/device_tree.h
>> +++ b/xen/include/xen/device_tree.h
>> @@ -112,6 +112,13 @@ struct dt_device_node {
>>  
>>  };
>>  
>> +#define MAX_PHANDLE_ARGS 16
>> +struct dt_phandle_args {
>> +    struct dt_device_node *np;
>> +    int args_count;
>> +    uint32_t args[MAX_PHANDLE_ARGS];
>> +};
>> +
>>  /**
>>   * IRQ line type.
>>   *
>> @@ -621,6 +628,53 @@ void dt_set_range(__be32 **cellp, const struct dt_device_node *np,
>>  void dt_get_range(const __be32 **cellp, const struct dt_device_node *np,
>>                    u64 *address, u64 *size);
>>  
>> +/**
>> + * dt_parse_phandle - Resolve a phandle property to a device_node pointer
>> + * @np: Pointer to device node holding phandle property
>> + * @phandle_name: Name of property holding a phandle value
>> + * @index: For properties holding a table of phandles, this is the index into
>> + *         the table
> 
> Otherwise it is -1 or something else?

Even with one element, the property contains a table of phandles. So,
the first index is always 0.

> 
>> + *
>> + * Returns the device_node pointer.
>> + */
>> +struct dt_device_node *dt_parse_phandle(const struct dt_device_node *np,
>> +				                        const char *phandle_name,
> 
> Stray hard tabs?

Oops, I forgot to check the hard tabs on this function. I will fix it.

>> +                                        int index);
>> +
>> +/**
>> + * dt_parse_phandle_with_args() - Find a node pointed by phandle in a list
>> + * @np:	pointer to a device tree node containing a list
>> + * @list_name: property name that contains a list
>> + * @cells_name: property name that specifies phandles' arguments count
>> + * @index: index of a phandle to parse out
>> + * @out_args: optional pointer to output arguments structure (will be filled)
>> + *
>> + * This function is useful to parse lists of phandles and their arguments.
>> + * Returns 0 on success and fills out_args, on error returns appropriate
>> + * errno value.
>> + *
>> + * Example:
>> + *
>> + * phandle1: node1 {
>> + * 	#list-cells = <2>;
>> + * }
>> + *
>> + * phandle2: node2 {
>> + * 	#list-cells = <1>;
>> + * }
>> + *
>> + * node3 {
>> + * 	list = <&phandle1 1 2 &phandle2 3>;
>> + * }
>> + *
>> + * To get a device_node of the `node2' node you may call this:
>> + * dt_parse_phandle_with_args(node3, "list", "#list-cells", 1, &args);
> 
> Wow, what an exciting function!
> 
> How do I decide the correct size for out_args?

out_args is a structure dt_phandle_args which contains the node and an
array of arguments.

The function is designed to parse one phandle by one phandle. So
out_args is either NULL (we don't want to get the info) or contains a
pointer to the structure to fill.

The description of out_args seems clear to me. Does it miss something?

Cheers,

-- 
Julien Grall