Re: [PATCH 1/1] of: introduce helper to manage boolean

Hi Scott,

On Tue, Jul 10, 2012 at 11:23 PM, Scott Wood [off-list ref]wrote:

On 07/10/2012 07:10 AM, Simon Glass wrote:

quoted

Hi,

On Tue, Mar 13, 2012 at 5:16 AM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org
<mailto:grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>> wrote:

    On Tue, 13 Mar 2012 04:17:39 +0100, Jean-Christophe PLAGNIOL-VILLARD
    <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org <mailto:plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>> wrote:
    > On 14:39 Mon 12 Mar     , Scott Wood wrote:
    > > On 03/09/2012 10:36 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
    > > >>>> Ugh.  so any value other than 1 returns false?  I think
    that will surprise
    > > >>>> most people.
    > > >>>>
    > > >>>> I don't like this api or binding.  If it is a bool
    property, then why isn't
    > > >>>> simply testing for the property existance sufficient?
    > > >>> no if you want to disable it
    > > >>>
    > > >>> if a bool is define in the dtsi and want to disable it int
    the dts
    > > >>>
    > > >>> if you we can do the the invert
    > > >>>
    > > >>> if !0 => true
    > > >>>
    > > >>> is-ok;                  => true
    > > >>> is-ok = <val != 0>;     => true
    > > >>> is-ok = <0>;            => false
    > > >>
    > > >> This is a failure of the dtc tool, not the binding.
     Accepting this binding
    > > >> means we have to live with it for a very long time.  It needs
    to be fixed
    > > >> in dtc instead so that properties can be deleted instead of
    only modified.
    > > > I understand your idea but today if you put and value in the
    property it's true.
    > > >
    > > > So is-ok = <0>; is true also which is illogical as in any
    language a boolean is
    > > > true (1) or false (0). When I read the property I will
    understand false not true
    > >
    > > You could say similar things about is-ok = "no" or is-ok = "" or
    is-ok =
    > > "I'd rather you didn't"... it's expected that violating the
    binding may
    > > produce illogical results.
    > today is most of the binding people use a number whe the want to
    be able to
    > delete it and it's the same in most of the promgramming language

    It isn't yet a big pain point, so there isn't time pressure here.
     Fixing the tool
    is the better solution, and since the kernel carries a copy of the
    tool we don't
    need to worry about adding a feature that isn't available by the dtc
    packaged by
    a distribution.

    Fixing the tool is the correct thing to do.


I just wanted to pick up on this thread. Now in the kernel, absence of a
property indicates false, and presence indicates true. This is a kernel
convention, nothing to do with the dtb format.

It's an aspect of the device binding, and was used by Open Firmware
bindings before Linux came up with flat device trees.

OK. When I asked about the boolean property on the devicetree-discuss I
understood that no one really cared, and it was a matter for Linux.

Also note that even non-boolean properties can mean something different
when absent.  Sometimes this is a default value; sometimes, like with
"ranges", it's something that can't be expressed with any value (empty
ranges means all translations go straight through; no ranges means no
translation is possible).

That's fine, but I'm not sure I understand why that relates to boolean
properties, which currently mean always the same thing (false) when absent.
I don't think there is any intent to change that.

quoted

I think it is useful to
support a boolean with a non-null value which can be 0, meaning true as
Jean-Christophe says.

My reasons are:

1. dtc does not have a way to delete a property and it isn't clear what
syntax could be used there.

Surely some syntax can be created for this.  E.g. /delete-prop/ foo;

Yes, in fact I saw a patch after sending this email. So for normal values
to change the value we do

   prop = <23>;

but for booleans we must do EITHER:

   bool;

or

   /delete-prop/ bool;

depending on whether we want the make it true or false? Ick.

quoted

It also seems a little brittle to delete a
property in one place and define it in another (ordering? confusion when
people can't work out why it has gone?). So not only can dtc not do
this, but I'm not sure that it should.

I don't see how it's worse than defining it one place and then
redefining it elsewhere.

It seems worse to me, see above. Also if we end up with symbols it will be
impossible to do something like:

   bool-property = <WIBBLE_VALUE>;

You will have to do:

if WIBBLE_VALUE == 0
  /delete-prop/ bool-property;
#else
  bool-property;
#endif

or whatever, or maybe I got that around the wrong way.... Not nice IMO.

(any comments on point 2?)

quoted

3. Discoverability: it is nice to be able to see the possible options,
even if disabled.

This assumes the possible options were known in advance, or that you
don't maintain compatibility with old device trees when a new option is
added.

You can still add the option with a zero value - or maybe I misunderstand
what you mean.

quoted

In particular, if a boolean value is true, and you
later decide with fdtput to turn it off, you effectively remove all
trace of it. This could be confusing for those who are looking for
available options.

There should still be a "trace of it" in the binding document.

Yes true, that's the saving grace.

I'm not even saying that we ever use

   bool-prop = <1>;

Clearly that is redundant. I'm just agreeing with Jean-Chrstophe that it
would be nice and clean to do something like:

   bool-prop = <0>;

and it will make it false.

Regards,
Simon

-Scott