[U-Boot] [PATCH 02/14] fdt: Add functions to access phandles, arrays and bools

[Simon Glass]

Hi Stephen,

On Fri, Dec 2, 2011 at 7:55 AM, Stephen Warren <swarren at nvidia.com> wrote:
> On 12/01/2011 06:01 PM, Simon Glass wrote:
>> Hi Stephen,
>>
>> On Mon, Nov 28, 2011 at 10:41 AM, Stephen Warren <swarren at nvidia.com> wrote:
>>> On 11/23/2011 08:54 PM, Simon Glass wrote:
>>>> Add a function to lookup a property which is a phandle in a node, and
>>>> another to read a fixed-length integer array from an fdt property.
>>>> Also add a function to read boolean properties.
>>>>
>>>> Signed-off-by: Simon Glass <sjg at chromium.org>
>
>>>> +/**
>>>> + * Look up a boolean property in a node and return it.
>>>> + *
>>>> + * A boolean properly is true if present in the device tree and false if not
>>>> + * present, or present with a 0 value.
>>>> + *
>>>> + * @param blob       FDT blob
>>>> + * @param node       node to examine
>>>> + * @param prop_name  name of property to find
>>>> + * @return 1 if the properly is present; 0 if it isn't present or is 0
>>>> + */
>>>> +int fdtdec_get_bool(const void *blob, int node, const char *prop_name);
>>>
>>> Does U-Boot allow use of the "bool" type here?
>>
>> Which bool type? It is returning an int.
>
> I was asking if the return type could be changed to bool.

Sorry I misunderstood -  no U-Boot does not have a bool type yet.

>
>>>> +int fdtdec_get_bool(const void *blob, int node, const char *prop_name)
>>>> +{
>>>> +     const s32 *cell;
>>>> +     int len;
>>>> +
>>>> +     debug("%s: %s\n", __func__, prop_name);
>>>> +     cell = fdt_getprop(blob, node, prop_name, &len);
>>>> +     if (!cell)
>>>> +             return 0;
>>>> +     if (len >= sizeof(u32) && *cell == 0)
>>>> +             return 0;
>>>> +
>>>> +     return 1;
>>>> +}
>>>
>>> In the kernel, I believe that property existence is all that's usually
>>> checked. Is that wrong? Did the definition of a boolean property's value
>>> in the function description above come from the specification? If a
>>> property had a length of 0/1/2/3 with a zero value, it seems very odd to
>>> treat that as true.
>>
>> It is useful to be able to set the value to 0 or 1 (with fdtget/put),
>> rather than remove or add the property. A value with a length of less
>> than one cell is considered illegal here.
>>
>> The basic idea is that the presence of the property means that it is
>> 'true'. If it happens to have a value, then we allow that to specify
>> 'false' if it is zero.
>
> Well, it's more up to standard device tree practice, not me. I've
> certainly sent patches that used a property with 0/1 value as a bool
> and received review feedback from DT experts that DT represents bools
> as present/absent properties, both with no value, so I assume zero
> length.

Yes that is the preferred way. However, it is useful to be able to
define a meaning when the property does have a value. The value may
come into the fdt later after compilation. At that point it is easier
to change a value rather than make something present/absent. For one
thing it can be done without adjust the size of the fdt blob.

Perhaps the solution is to adjust fdtput to support boolean values
explicitly (in that it would add or remove the property based on a
command-line 0/1 value).

Still I think it is useful to support a zero value meaning false when
it is provided.

Regards,
Simon

>
> --
> nvpublic
>