Boolean properties

Boolean properties

[Simon Glass]

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

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)

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

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)

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

Re: Boolean properties

[David Gibson]

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

Re: Boolean properties

[Simon Glass]

Hi David,

On Tue, Dec 6, 2011 at 3:12 PM, David Gibson
<david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> 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.

OK I didn't know that.

>
>> 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.

That was my first thought, but I thought it would be painful since
callers would need to do something like:

if [ $value -eq 0 ]
   fdtput -d /path/to/node property
else
   fdtput /path/to/node property <some tag that means nothing>
    (or maybe    fdtput -z /path/to/node property)
fi

so better IMO to provide boolean support in the tool. But it does
illustrate that presence/absence is not always ideal.

>
>> 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.

I mean device tree users (kernel, u-boot) rather than dtc (which I
suspect would be unaffected).

>
>> 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.

I don't like it either. But we currently have a situation where a zero
value is considered true, and this can be confusing. I would prefer to
expand the meaning.

>
>> 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

Re: Boolean properties

[Mitch Bradley]

On 12/7/2011 7:12 AM, David Gibson wrote:
> 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.

Since the producer and consumer of a named property must agree on the 
interpretation of its value, you can do whatever you want with a 
newly-named property without breaking anything else - although you might 
not be able to use existing encoding/decoding methods if you make up 
something new.

The use case for absence/presence boolean properties was as follows:

You develop a device and describe it via a set of properties.  Some time 
later, someone develops a newer version of that device that has a new 
feature that was not contemplated in the original design.  You need to 
declare that the feature exists, but the device node for the older 
version has no property relating to that hitherto-unknown feature.  So 
you invent a new property, with the understanding that the absence of 
that property indicates that the feature is not present.

That scheme does not preclude a "name"=0/1 formulation, with the 
understanding that absence implies 0, but it's unclear to me that the 
0/1 value adds much.  Either way, for your system to work, you have to 
get the right information in the device node.

The =0/1 formulation could in principle be used for 3 states, with 
absence meaning either "don't know" or "error".  To my way of thinking, 
that just makes things more complicated, putting the burden on the 
programmer to somehow handle a vague condition.  I prefer "get it right 
and be done with it".



>
>> 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
>>
>

Re: Boolean properties

[David Gibson]

On Tue, Dec 06, 2011 at 04:01:20PM -0800, Simon Glass wrote:
> Hi David,
> 
> On Tue, Dec 6, 2011 at 3:12 PM, David Gibson
> <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> > 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.
> 
> OK I didn't know that.
> 
> >
> >> 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.
> 
> That was my first thought, but I thought it would be painful since
> callers would need to do something like:
> 
> if [ $value -eq 0 ]
>    fdtput -d /path/to/node property
> else
>    fdtput /path/to/node property <some tag that means nothing>
>     (or maybe    fdtput -z /path/to/node property)
> fi
> 
> so better IMO to provide boolean support in the tool. But it does
> illustrate that presence/absence is not always ideal.

Hrm.  I think that's a lesser evil than having a data "type" that can
actually completely delete the 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.
> 
> I mean device tree users (kernel, u-boot) rather than dtc (which I
> suspect would be unaffected).

Well, like I say, I have no in principle objection to using this as
the usual boolean test, but actually changing it is obviously
something you'd need to convince the maintainers of the relevant
projects.

> >> 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.
> 
> I don't like it either. But we currently have a situation where a zero
> value is considered true, and this can be confusing. I would prefer to
> expand the meaning.

Actually, on further thought, I don't mind the above so much, because
even if you use it without checking for the error value, you'll at
least get the right behaviour for the unambiguous cases.

> >> 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?

-- 
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

Re: Boolean properties

[Simon Glass]

Hi Mitch,

On Wed, Dec 7, 2011 at 12:04 AM, Mitch Bradley <wmb-D5eQfiDGL7eakBO8gow8eQ@public.gmane.org> wrote:
> On 12/7/2011 7:12 AM, David Gibson wrote:
>>
>> 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.
>
>
> Since the producer and consumer of a named property must agree on the
> interpretation of its value, you can do whatever you want with a newly-named
> property without breaking anything else - although you might not be able to
> use existing encoding/decoding methods if you make up something new.
>
> The use case for absence/presence boolean properties was as follows:
>
> You develop a device and describe it via a set of properties.  Some time
> later, someone develops a newer version of that device that has a new
> feature that was not contemplated in the original design.  You need to
> declare that the feature exists, but the device node for the older version
> has no property relating to that hitherto-unknown feature.  So you invent a
> new property, with the understanding that the absence of that property
> indicates that the feature is not present.
>
> That scheme does not preclude a "name"=0/1 formulation, with the
> understanding that absence implies 0, but it's unclear to me that the 0/1
> value adds much.  Either way, for your system to work, you have to get the
> right information in the device node.

Yes that's right. Can you please look at my three reasons below and
comment on those?

>
> The =0/1 formulation could in principle be used for 3 states, with absence
> meaning either "don't know" or "error".  To my way of thinking, that just
> makes things more complicated, putting the burden on the programmer to
> somehow handle a vague condition.  I prefer "get it right and be done with
> it".

Absence means 0 and I don't propose to change that, nor introduce a
vague condition. My suggest is purely that we allow boolean properties
to have value which is not ignored. Please see below for my reasons,
which may or may not be persuasive:

>
>
>
>
>>
>>> 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
>>>
>>
>

Re: Boolean properties

[Simon Glass]

Hi David,

On Thu, Dec 8, 2011 at 5:15 PM, David Gibson
<david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> On Tue, Dec 06, 2011 at 04:01:20PM -0800, Simon Glass wrote:
>> Hi David,
>>
>> On Tue, Dec 6, 2011 at 3:12 PM, David Gibson
>> <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
>> > 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.
>>
>> OK I didn't know that.
>>
>> >
>> >> 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.
>>
>> That was my first thought, but I thought it would be painful since
>> callers would need to do something like:
>>
>> if [ $value -eq 0 ]
>>    fdtput -d /path/to/node property
>> else
>>    fdtput /path/to/node property <some tag that means nothing>
>>     (or maybe    fdtput -z /path/to/node property)
>> fi
>>
>> so better IMO to provide boolean support in the tool. But it does
>> illustrate that presence/absence is not always ideal.
>
> Hrm.  I think that's a lesser evil than having a data "type" that can
> actually completely delete the property.

Well isn't that the nature of the boolean property. In a way that is
exactly my point, that to change a boolean property to zero you have
to delete it. This is not something that fdtput would mandate, merely
make easier for the user. I agree it is odd, and really that is my
point.

>
>> >> 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.
>>
>> I mean device tree users (kernel, u-boot) rather than dtc (which I
>> suspect would be unaffected).
>
> Well, like I say, I have no in principle objection to using this as
> the usual boolean test, but actually changing it is obviously
> something you'd need to convince the maintainers of the relevant
> projects.

OK will see about 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.
>>
>> I don't like it either. But we currently have a situation where a zero
>> value is considered true, and this can be confusing. I would prefer to
>> expand the meaning.
>
> Actually, on further thought, I don't mind the above so much, because
> even if you use it without checking for the error value, you'll at
> least get the right behaviour for the unambiguous cases.

Yes that's right. Of course you will get the 'right' behaviour when
the value is 0 (i.e. the boolean will be considered true).

>
>> >> 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?
>
> --
> 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

Regards,
Simon

Re: Boolean properties

[Mitch Bradley]

I don't have a strong feeling either way - I was just reporting the 
historical thinking.  I agree that it's easier to change a value than to 
delete a property, but in the grand scheme of things, it's not a huge 
deal, just a minor convenience.


On 12/9/2011 9:33 AM, Simon Glass wrote:
> Hi Mitch,
>
> On Wed, Dec 7, 2011 at 12:04 AM, Mitch Bradley<wmb-D5eQfiDGL7eakBO8gow8eQ@public.gmane.org>  wrote:
>> On 12/7/2011 7:12 AM, David Gibson wrote:
>>>
>>> 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.
>>
>>
>> Since the producer and consumer of a named property must agree on the
>> interpretation of its value, you can do whatever you want with a newly-named
>> property without breaking anything else - although you might not be able to
>> use existing encoding/decoding methods if you make up something new.
>>
>> The use case for absence/presence boolean properties was as follows:
>>
>> You develop a device and describe it via a set of properties.  Some time
>> later, someone develops a newer version of that device that has a new
>> feature that was not contemplated in the original design.  You need to
>> declare that the feature exists, but the device node for the older version
>> has no property relating to that hitherto-unknown feature.  So you invent a
>> new property, with the understanding that the absence of that property
>> indicates that the feature is not present.
>>
>> That scheme does not preclude a "name"=0/1 formulation, with the
>> understanding that absence implies 0, but it's unclear to me that the 0/1
>> value adds much.  Either way, for your system to work, you have to get the
>> right information in the device node.
>
> Yes that's right. Can you please look at my three reasons below and
> comment on those?
>
>>
>> The =0/1 formulation could in principle be used for 3 states, with absence
>> meaning either "don't know" or "error".  To my way of thinking, that just
>> makes things more complicated, putting the burden on the programmer to
>> somehow handle a vague condition.  I prefer "get it right and be done with
>> it".
>
> Absence means 0 and I don't propose to change that, nor introduce a
> vague condition. My suggest is purely that we allow boolean properties
> to have value which is not ignored. Please see below for my reasons,
> which may or may not be persuasive:
>
>>
>>
>>
>>
>>>
>>>> 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
>>>>
>>>
>>
>
>