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 <balbi@xxxxxx>
> ---
> 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 majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
