Re: [PATCH 1/1] of: introduce helper to manage boolean
From: Simon Glass <hidden>
Date: 2012-07-12 05:27:50
Also in:
linux-arm-kernel
Hi Scott, On Wed, Jul 11, 2012 at 1:11 AM, Scott Wood [off-list ref] wrote:
On 07/10/2012 05:53 PM, Simon Glass wrote:quoted
Hi Scott, On Tue, Jul 10, 2012 at 11:23 PM, Scott Wood <scottwood-KZfg59tc24xl57MIdRCFDg@public.gmane.org <mailto:scottwood-KZfg59tc24xl57MIdRCFDg@public.gmane.org>> 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.
OK I see. We don't have that option for booleans, since absence will always mean false.
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.Doesn't seem that bad to me except in the case you show below where you're trying to do it programmatically.quoted
It seems worse to me, see above. Also if we end up with symbols it will*> be impossible to do something like:quoted
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.
Fair enough. Jean-Christophe's patch certainly changes existing behaviour, and without that it might be confusing - we don't want two functions for reading boolean properties. The only plus is that (perhaps) no one expects "bool-prop = <0>" to mean true, so it might be safe.
quoted
(any comments on point 2?)It's basically the same as the above, right?
Yes, and you commented above.
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.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.
Yes agreed. The point of this is just to make it easier (I think) to make a boolean value false either in the source, or later using fdtput. So what do we need to do? Take a vote? Revisit Jean-Christophe's patch?
-Scott
Regards, Simon