Re: [PATCH v2 02/17] fdt: Add functions to access phandles, arrays and bools

[Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>]

Hi Stephen,

On Mon, Dec 5, 2011 at 1:59 PM, Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> On 12/02/2011 07:11 PM, Simon Glass wrote:
>> Add a function to look up 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, although there is no
>> actual boolean type in U-Boot.
>
>> +/**
>> + * 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)
>> +{
>> +     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;
>> +}
>
> I'm still slightly worried that this implementation may interpret the
> FDT differently to the kernel. At least the code I've written for
> boolean properties /only/ looks at the presence of the property, and
> never at the contents even if there is one.
>
> That said, the ePAPR specification does only say that boolean properties
> /might/ have an empty value, thus implying that a non-zero length is
> permissible. so, perhaps this is fine.

OK

>
> Actually no, I'm more than worried now I think it through. You'd argued
> for being able to write 0/1 to the property so that U-Boot could modify
> the value in-place without having to add/remove nodes to/from the FDT
> image, but rather just change their content. However, if this modified
> FDT is then passed to the kernel (rather than some other fresh FDT),
> then it's imperative that U-Boot and the kernel represent boolean
> properties in the exact same way. Hence, we either can't have U-Boot use
> this edit-in-place optimization, or U-Boot needs some "cleanup" of the
> FDT image before passing it to the kernel. However, the latter is
> impossible, since by then since the boot-the-kernel code has no idea
> whether a property is a boolean or not, and hence implementing such a
> cleanup is impossible.

Hang on - this fdt cannot be passed to the kernel! It is for U-Boot's
use, for it's own configuration. There is no mechanism to fix up
U-Boot's internal fdt and pass it to the kernel. U-Boot scripts can't
even find its address!

That side of U-Boot is a complete different use of fdt and I have been
careful (so far) to keep them apart.

Regards,
Simon

>
> --
> nvpublic