On Thu, Feb 05, 2015 at 06:01:06PM +0000, Felipe Balbi wrote: > allowing values to boolean flags lets us setup > defaults on DTSI which can get disabled later > at board DTS if, for whatever reason, board can't > use that default. > > One such example is DM81xx EVM where we can't use > MUSB's multipoint feature even though SoC supports > it. Something at the board level prevents us from > using the feature. > > Instead of removing "multipoint;" from DTSI and > adding it to all board DTS just so we can remove > it from our quirky board seems like overkill when > we could just add: > > multipoint = <0>; > > to that quirky board's DTS. > > Note that the description here is but one example > and it's likely many others have faced something > similar. >
While I appreciate that adding and removing properties in this way is painful, I think that this must be dealt with at DTB compile-time rather than kernel run-time. There are codebases other than Linux which parse DTs, and not all drivers call of_property_read_bool to parse boolean properties, an awful lot still just check of_find_property. Additionally, some bindings _explicitly_ state boolean properties are empty and have no value, which this extension would break. I think that this patch only adds to the inconsistency we currently have, and given that, I would rather not have this extension to of_property_read_bool. Arguably of_proeprty_read_bool should warn if it encounters a non-empty property. Thanks, Mark. > Signed-off-by: Felipe Balbi <[email protected]> > --- > include/linux/of.h | 22 ++++++++++++++++++++-- > 1 file changed, 20 insertions(+), 2 deletions(-) > > diff --git a/include/linux/of.h b/include/linux/of.h > index 76c055b20fef..c5ee9320f237 100644 > --- a/include/linux/of.h > +++ b/include/linux/of.h > @@ -792,14 +792,32 @@ static inline int of_property_read_u32(const struct > device_node *np, > * @propname: name of the property to be searched. > * > * Search for a property in a device node. > - * Returns true if the property exist false otherwise. > + * Returns true if the property exist and has a value greater than zero, > + * fals otherwise. > */ > static inline bool of_property_read_bool(const struct device_node *np, > const char *propname) > { > struct property *prop = of_find_property(np, propname, NULL); > + int rc; > + u32 val; > > - return prop ? true : false; > + if (!prop) > + return false; > + > + rc = of_property_read_u32(np, propname, &val); > + > + /* > + * if property doesn't have a value, or prop->length == 0 and > + * we overflow, treat it as simple value-less flag. > + */ > + if (rc == -ENODATA || rc == -EOVERFLOW) > + return true; > + if (WARN(rc < 0, "failed to read '%s' value -> %d\n", > + propname, rc)) > + return false; > + > + return !!val; > } > > static inline int of_property_read_s32(const struct device_node *np, > -- > 2.3.0-rc1 > > -- > To unsubscribe from this list: send the line "unsubscribe devicetree" in > the body of a message to [email protected] > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
