[PATCH] of: introduces helper to manage optional property

[PATCH] of: introduces helper to manage optional property

[Xiubo Li]

Gets to know whether one optional property is present or not in one
device node. Returns true if the property is present, false otherwise.

There is one similar helper of_property_read_bool(), but it means that
the property is one boolen type. For many drivers, they need to know
whether one optional property is present or not firstly. Though many
other of_XX helper can do this before deal with it's real work, there
maybe other errors the driver need to deal with, and the errors maybe
the same with the property absent error. And this helper can make the
code briefer,more readable and easier to deal with.

Signed-off-by: Xiubo Li <Li.Xiubo-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
 include/linux/of.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/include/linux/of.h b/include/linux/of.h
index 276c546..0bd8cb9 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -601,6 +601,23 @@ static inline int of_property_read_u32(const struct device_node *np,
 	return of_property_read_u32_array(np, propname, out_value, 1);
 }
 
+/**
+ * of_property_optional - Find one optional property
+ * @np:		device node from which the property to be searched.
+ * @propname:	name of the optional property to be searched for.
+ *
+ * Get to know whether one optional property is present or not in a
+ * device node.
+ * Returns true if the property is present, false otherwise.
+ */
+static inline bool of_property_optional(const struct device_node *np,
+					 const char *propname)
+{
+	struct property *prop = of_find_property(np, propname, NULL);
+
+	return prop ? true : false;
+}
+
 #define of_property_for_each_u32(np, propname, prop, p, u)	\
 	for (prop = of_find_property(np, propname, NULL),	\
 		p = of_prop_next_u32(prop, NULL, &u);		\
-- 
1.8.4


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] of: introduces helper to manage optional property

[Gerhard Sittig]

On Thu, Dec 26, 2013 at 13:18 +0800, Xiubo Li wrote:
> 
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -601,6 +601,23 @@ static inline int of_property_read_u32(const struct device_node *np,
>  	return of_property_read_u32_array(np, propname, out_value, 1);
>  }
>  
> +/**
> + * of_property_optional - Find one optional property
> + * @np:		device node from which the property to be searched.
> + * @propname:	name of the optional property to be searched for.
> + *
> + * Get to know whether one optional property is present or not in a
> + * device node.
> + * Returns true if the property is present, false otherwise.
> + */
> +static inline bool of_property_optional(const struct device_node *np,
> +					 const char *propname)
> +{
> +	struct property *prop = of_find_property(np, propname, NULL);
> +
> +	return prop ? true : false;
> +}
> +

Exactly how is this different from of_property_read_bool()?
AFAICS it's identical!  You may be trying to solve the wrong
problem.

And I'm afraid that the routine's name you picked is even more
confusing.  The call site ("if (of_property_optional() ...") will
no longer reflect that it's evaluating a boolean condition.
Being an optional property and being boolean are two completely
different things.


virtually yours
Gerhard Sittig
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office-ynQEQJNshbs@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] of: introduces helper to manage optional property

[Mark Rutland]

On Thu, Dec 26, 2013 at 05:18:35AM +0000, Xiubo Li wrote:
> Gets to know whether one optional property is present or not in one
> device node. Returns true if the property is present, false otherwise.
> 
> There is one similar helper of_property_read_bool(), but it means that
> the property is one boolen type. For many drivers, they need to know
> whether one optional property is present or not firstly. Though many
> other of_XX helper can do this before deal with it's real work, there
> maybe other errors the driver need to deal with, and the errors maybe
> the same with the property absent error. And this helper can make the
> code briefer,more readable and easier to deal with.
> 
> Signed-off-by: Xiubo Li <Li.Xiubo-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> ---
>  include/linux/of.h | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/include/linux/of.h b/include/linux/of.h
> index 276c546..0bd8cb9 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -601,6 +601,23 @@ static inline int of_property_read_u32(const struct device_node *np,
>  	return of_property_read_u32_array(np, propname, out_value, 1);
>  }
>  
> +/**
> + * of_property_optional - Find one optional property

If we really do need something like this, it would be better named
of_property_present (and can alias of_property_read_bool internally).

Is there a particular case this would make easier? Do you have a real
example?

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] of: introduces helper to manage optional property

[Li.Xiubo-KZfg59tc24xl57MIdRCFDg]

> > @@ -601,6 +601,23 @@ static inline int of_property_read_u32(const struct
> device_node *np,
> >  	return of_property_read_u32_array(np, propname, out_value, 1);
> >  }
> >
> > +/**
> > + * of_property_optional - Find one optional property
> > + * @np:		device node from which the property to be searched.
> > + * @propname:	name of the optional property to be searched for.
> > + *
> > + * Get to know whether one optional property is present or not in a
> > + * device node.
> > + * Returns true if the property is present, false otherwise.
> > + */
> > +static inline bool of_property_optional(const struct device_node *np,
> > +					 const char *propname)
> > +{
> > +	struct property *prop = of_find_property(np, propname, NULL);
> > +
> > +	return prop ? true : false;
> > +}
> > +
> 
> Exactly how is this different from of_property_read_bool()?
> AFAICS it's identical!  You may be trying to solve the wrong
> problem.
> 
> And I'm afraid that the routine's name you picked is even more
> confusing.  The call site ("if (of_property_optional() ...") will
> no longer reflect that it's evaluating a boolean condition.
> Being an optional property and being boolean are two completely
> different things.
> 
> 

Yes, That's right.

Thanks,
Best Regards,



--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] of: introduces helper to manage optional property

[Grant Likely]

On Mon, 6 Jan 2014 16:26:09 +0000, Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> wrote:
> On Thu, Dec 26, 2013 at 05:18:35AM +0000, Xiubo Li wrote:
> > Gets to know whether one optional property is present or not in one
> > device node. Returns true if the property is present, false otherwise.
> > 
> > There is one similar helper of_property_read_bool(), but it means that
> > the property is one boolen type. For many drivers, they need to know
> > whether one optional property is present or not firstly. Though many
> > other of_XX helper can do this before deal with it's real work, there
> > maybe other errors the driver need to deal with, and the errors maybe
> > the same with the property absent error. And this helper can make the
> > code briefer,more readable and easier to deal with.
> > 
> > Signed-off-by: Xiubo Li <Li.Xiubo-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > ---
> >  include/linux/of.h | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> > 
> > diff --git a/include/linux/of.h b/include/linux/of.h
> > index 276c546..0bd8cb9 100644
> > --- a/include/linux/of.h
> > +++ b/include/linux/of.h
> > @@ -601,6 +601,23 @@ static inline int of_property_read_u32(const struct device_node *np,
> >  	return of_property_read_u32_array(np, propname, out_value, 1);
> >  }
> >  
> > +/**
> > + * of_property_optional - Find one optional property
> 
> If we really do need something like this, it would be better named
> of_property_present (and can alias of_property_read_bool internally).

I would rather not add an alias or new function for this is possible
given that existing functions (of_find_property and
of_property_read_bool provide the same behavour)

g.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html