linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: scottwood@freescale.com (Scott Wood)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/1] of: introduce helper to manage boolean
Date: Tue, 10 Jul 2012 18:11:30 -0500	[thread overview]
Message-ID: <4FFCB6A2.50805@freescale.com> (raw)
In-Reply-To: <CAPnjgZ1D0hCZobFENTPN8Vkz04x5bXa9RviJuVcgBt66BL961Q@mail.gmail.com>

On 07/10/2012 05:53 PM, Simon Glass wrote:
> Hi Scott,
> 
> On Tue, Jul 10, 2012 at 11:23 PM, Scott Wood <scottwood@freescale.com
> <mailto:scottwood@freescale.com>> wrote:
> 
>     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.

The point was this isn't the only scenario where the absence of a
property means something, and where you might want to delete the property.

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

Doesn't seem that bad to me except in the case you show below where
you're trying to do it programmatically.

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

Yeah, that's unpleasant.

I don't hugely object to a new boolean type for use in new bindings,
where <0> or absence can both be used to indicate false, as long as it's
clearly documented.  I'm hesitant to start doing this on existing
bindings, though, and in any case would like to see support for
property/node deletion in dtc.

> (any comments on point 2?)

It's basically the same as the above, right?

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

We normally want to work with existing device trees (which could be
partially produced by firmware, so changing can be unpleasant), so the
absence of the property is still going to be a possibility.  Plus
enumerating every possibility, no matter how rarely used, in every node
of a certain type seems excessively verbose.  It's not like these are
user configuration knobs.

-Scott

  reply	other threads:[~2012-07-10 23:11 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-07  4:13 [PATCH 1/1] of: introduce helper to manage boolean Jean-Christophe PLAGNIOL-VILLARD
2012-02-07 15:13 ` Rob Herring
2012-02-07 16:14   ` Jean-Christophe PLAGNIOL-VILLARD
2012-03-09  1:44 ` Grant Likely
2012-03-09 10:04   ` Jean-Christophe PLAGNIOL-VILLARD
2012-03-09 16:26     ` Grant Likely
2012-03-09 16:36       ` Jean-Christophe PLAGNIOL-VILLARD
2012-03-12 19:39         ` Scott Wood
2012-03-13  3:17           ` Jean-Christophe PLAGNIOL-VILLARD
2012-03-13  4:16             ` Grant Likely
2012-03-13  7:03               ` Jean-Christophe PLAGNIOL-VILLARD
2012-07-10 12:10               ` Simon Glass
2012-07-10 21:23                 ` Scott Wood
2012-07-10 22:53                   ` Simon Glass
2012-07-10 23:11                     ` Scott Wood [this message]
2012-07-12  5:27                       ` Simon Glass

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4FFCB6A2.50805@freescale.com \
    --to=scottwood@freescale.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).