Re: [PATCH 3/3] OF: base: add empty property value pointer

[Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>]

On 06/23/2013 08:33 PM, Sascha Hauer wrote:
> On Fri, Jun 21, 2013 at 05:15:18PM +0200, Sebastian Hesselbarth wrote:
>> Since property values can be empty, we need property value pointer to
>> be non-NULL to distinguish those properties from non-existing properties.
>> This adds a static u32 to which empty properties set their value pointer.
>> Also, the value memory is only freed, if property length is not zero.
>>
>> Signed-off-by: Sebastian Hesselbarth<sebastian.hesselbarth@gmail.com>
>> ---
>> Cc: barebox@lists.infradead.org
>> ---
>>   drivers/of/base.c |    6 +++++-
>>   1 files changed, 5 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>> index 7926d5d..a100a17 100644
>> --- a/drivers/of/base.c
>> +++ b/drivers/of/base.c
>> @@ -1501,6 +1501,8 @@ struct device_node *of_new_node(struct device_node *parent, const char *name)
>>   	return node;
>>   }
>>
>> +static u32 empty_prop_value;
>> +
>>   struct property *of_new_property(struct device_node *node, const char *name,
>>   		const void *data, int len)
>>   {
>> @@ -1515,6 +1517,7 @@ struct property *of_new_property(struct device_node *node, const char *name,
>>   		goto bail_out;
>>
>>   	prop->length = len;
>> +	prop->value =&empty_prop_value;
>
> This at least breaks of_set_property() and of_free(). Both unconditionally
> do a free(pp->value).

Sascha,

this is patch 3/3, the two functions above are taken care of patch 2/3.

> Why do we need this anyway? We can always call of_find_property() to see
> if a property exists.

Actually, I was preparing to import drivers/of/address.c from linux.
of_translate_one uses of_get_property on "ranges". This returns
the property's value pointer instead of the property itself, which
is NULL for an empty ranges property. Now that you point it out,
using of_find_property is also an option.

Nevertheless, patches 1 (with your comments applied) and 2 seem
sensible to me. Patch 3 is an option to keep it in sync with Linux
OF API behavior but is optional.

Sebastian

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox