Re: Boolean properties

[David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>]

On Mon, Dec 05, 2011 at 04:29:22PM -0800, Simon Glass wrote:
> Hi,
> 
> As I understand it, boolean properties in the device tree are defined
> as follows:
> 
> - if the property is present, then it is true
> - if the property is absent, then it is false

That's the common convention.  Although an individual binding could
define something else, and I think some already do define a
boolean~=int approach instead (e.g. the ibm,dfp cpu property).  In
some cases that's anticipating future expansion to values other than 0
and 1.

> The fdtget and fdtput utilities deal with reading and changing these.
> At present fdtget returns the value of the property and is not
> suitable for boolean properties. fdtput allows setting of a property
> value but not removing it.
> 
> To deal with boolean properties these utilities need to change. One
> option is to use something like:
> 
> $ fdtget -t f /path/to/node property
> (prints 'true' or 'false')
> $ fdtput -t f /path/to/node property false/true
> $ fdtput -t f /path/to/node property 0/1
> (deletes the property for false and 0; adds it with no value for true and 1)
> 
> (Unfortunately the 'b' type is taken for byte at present, so it is
> unclear to be so far what letter we should use for 'f' in the -t flag)

Hrm.  Rather than treating this as a boolean "type", I think it would
be less ambiguous to instead add options to explicitly delete a
property (either e.g. fdtput -d or a new fdtdel), and to create an
empty valued property.

> But first I would like to suggest that we support boolean properties
> with a value, i.e. change to:
> 
> - if the property is absent, then it is false
> - if the property is present, then it is true
>     - unless the property has a 0 value, iwc it is false

So the first question here is who/what exactly constitutes "we" here -
kernel, u-boot, dtc?  Using the above logic sounds like a good idea
for fdt readers on the "be liberal in what you accept" principle.  But
it's something I'd like to see Mitch Bradley and/or Segher comment on
- they'd have a better idea of whether there's existing practice that
might break with that.

> There are three reasons:
> 1. it is very confusing for the poor user to set a property value to 0
> and have it come back as 'true' from a function that checks these
> sorts of things. This is particularly true since fdt properties do not
> have types, or even type hints.  The function I have in mind is
> something like this (modified to fit the discussion):
> 
> /**
>  * 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 (regardless of any property data/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
>  */
> int fdtdec_get_bool(const void *blob, int node, const char *prop_name);
> 
> An alternative might be to print an error (and refuse to return a
> value) when a boolean property has a value:
> 
>  * @return 1 if the properly is present; 0 if it isn't present, -1 if
> it has any data (and is therefore illegal as a boolean property)

Kind of a pain for C.

> 2. it allows us to adjust the boolean value without requiring a change
> in the size of the device tree blob (faster, easier)
> 
> 3. Related to 2 it allows us to put template values in the fdt which
> can then be *optionally* updated in the build system or in U-Boot to
> the required value before passing to the kernel.
> 
> I presume this discussion has happened before - can anyone please
> point me to the thread?
> 
> Regards,
> Simon
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson