[PATCH 3/3] OF: base: add empty property value pointer
sebastian.hesselbarth at gmail.com
Sun Jun 23 16:11:39 EDT 2013
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 at gmail.com>
>> Cc: barebox at 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).
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.
More information about the barebox