linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] dt: add helper function to read u8 & u16 variables & arrays
@ 2012-10-12 18:01 Viresh Kumar
  2012-10-15  7:56 ` Shevchenko, Andriy
  2012-10-25  7:03 ` Viresh Kumar
  0 siblings, 2 replies; 12+ messages in thread
From: Viresh Kumar @ 2012-10-12 18:01 UTC (permalink / raw)
  To: rob.herring, grant.likely
  Cc: spear-devel, devicetree-discuss, linux-kernel, andriy.shevchenko,
	Viresh Kumar

This adds following helper routines:
- of_property_read_u8_array()
- of_property_read_u16_array()
- of_property_read_u8()
- of_property_read_u16()

First two actually share most of the code with of_property_read_u32_array(), so
the common part is taken out into a macro, which can be used by all three
*_array() routines.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/of/base.c  | 73 +++++++++++++++++++++++++++++++++++++++++++-----------
 include/linux/of.h | 30 ++++++++++++++++++++++
 2 files changed, 89 insertions(+), 14 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 38892a9..446fe7f 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -670,6 +670,64 @@ struct device_node *of_find_node_by_phandle(phandle handle)
 }
 EXPORT_SYMBOL(of_find_node_by_phandle);
 
+#define of_property_read_array(_np, _pname, _out, _sz, _type)		\
+	struct property *_prop = of_find_property(_np, _pname, NULL);	\
+	const __be32 *_val;						\
+									\
+	if (!_prop)							\
+		return -EINVAL;						\
+	if (!_prop->value)						\
+		return -ENODATA;					\
+	if ((_sz * sizeof(*_out)) > _prop->length)			\
+		return -EOVERFLOW;					\
+									\
+	_val = _prop->value;						\
+	while (_sz--)							\
+		*_out++ = (_type)be32_to_cpup(_val++);			\
+	return 0;
+
+/**
+ * of_property_read_u8_array - Find and read an array of u8 from a property.
+ *
+ * @np:		device node from which the property value is to be read.
+ * @propname:	name of the property to be searched.
+ * @out_value:	pointer to return value, modified only if return value is 0.
+ *
+ * Search for a property in a device node and read 8-bit value(s) from
+ * it. Returns 0 on success, -EINVAL if the property does not exist,
+ * -ENODATA if property does not have a value, and -EOVERFLOW if the
+ * property data isn't large enough.
+ *
+ * The out_value is modified only if a valid u8 value can be decoded.
+ */
+int of_property_read_u8_array(const struct device_node *np,
+			const char *propname, u8 *out_values, size_t sz)
+{
+	of_property_read_array(np, propname, out_values, sz, u8);
+}
+EXPORT_SYMBOL_GPL(of_property_read_u8_array);
+
+/**
+ * of_property_read_u16_array - Find and read an array of u16 from a property.
+ *
+ * @np:		device node from which the property value is to be read.
+ * @propname:	name of the property to be searched.
+ * @out_value:	pointer to return value, modified only if return value is 0.
+ *
+ * Search for a property in a device node and read 16-bit value(s) from
+ * it. Returns 0 on success, -EINVAL if the property does not exist,
+ * -ENODATA if property does not have a value, and -EOVERFLOW if the
+ * property data isn't large enough.
+ *
+ * The out_value is modified only if a valid u16 value can be decoded.
+ */
+int of_property_read_u16_array(const struct device_node *np,
+			const char *propname, u16 *out_values, size_t sz)
+{
+	of_property_read_array(np, propname, out_values, sz, u16);
+}
+EXPORT_SYMBOL_GPL(of_property_read_u16_array);
+
 /**
  * of_property_read_u32_array - Find and read an array of 32 bit integers
  * from a property.
@@ -689,20 +747,7 @@ int of_property_read_u32_array(const struct device_node *np,
 			       const char *propname, u32 *out_values,
 			       size_t sz)
 {
-	struct property *prop = of_find_property(np, propname, NULL);
-	const __be32 *val;
-
-	if (!prop)
-		return -EINVAL;
-	if (!prop->value)
-		return -ENODATA;
-	if ((sz * sizeof(*out_values)) > prop->length)
-		return -EOVERFLOW;
-
-	val = prop->value;
-	while (sz--)
-		*out_values++ = be32_to_cpup(val++);
-	return 0;
+	of_property_read_array(np, propname, out_values, sz, u32);
 }
 EXPORT_SYMBOL_GPL(of_property_read_u32_array);
 
diff --git a/include/linux/of.h b/include/linux/of.h
index 72843b7..e2d9b40 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -223,6 +223,10 @@ extern struct device_node *of_find_node_with_property(
 extern struct property *of_find_property(const struct device_node *np,
 					 const char *name,
 					 int *lenp);
+extern int of_property_read_u8_array(const struct device_node *np,
+			const char *propname, u8 *out_values, size_t sz);
+extern int of_property_read_u16_array(const struct device_node *np,
+			const char *propname, u16 *out_values, size_t sz);
 extern int of_property_read_u32_array(const struct device_node *np,
 				      const char *propname,
 				      u32 *out_values,
@@ -357,6 +361,18 @@ static inline struct device_node *of_find_compatible_node(
 	return NULL;
 }
 
+static inline int of_property_read_u8_array(const struct device_node *np,
+			const char *propname, u8 *out_values, size_t sz)
+{
+	return -ENOSYS;
+}
+
+static inline int of_property_read_u16_array(const struct device_node *np,
+			const char *propname, u16 *out_values, size_t sz)
+{
+	return -ENOSYS;
+}
+
 static inline int of_property_read_u32_array(const struct device_node *np,
 					     const char *propname,
 					     u32 *out_values, size_t sz)
@@ -463,6 +479,20 @@ static inline bool of_property_read_bool(const struct device_node *np,
 	return prop ? true : false;
 }
 
+static inline int of_property_read_u8(const struct device_node *np,
+				       const char *propname,
+				       u8 *out_value)
+{
+	return of_property_read_u8_array(np, propname, out_value, 1);
+}
+
+static inline int of_property_read_u16(const struct device_node *np,
+				       const char *propname,
+				       u16 *out_value)
+{
+	return of_property_read_u16_array(np, propname, out_value, 1);
+}
+
 static inline int of_property_read_u32(const struct device_node *np,
 				       const char *propname,
 				       u32 *out_value)
-- 
1.7.12.rc2.18.g61b472e


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] dt: add helper function to read u8 & u16 variables & arrays
  2012-10-12 18:01 [PATCH] dt: add helper function to read u8 & u16 variables & arrays Viresh Kumar
@ 2012-10-15  7:56 ` Shevchenko, Andriy
  2012-10-15  8:06   ` Viresh Kumar
  2012-10-25  7:03 ` Viresh Kumar
  1 sibling, 1 reply; 12+ messages in thread
From: Shevchenko, Andriy @ 2012-10-15  7:56 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: rob.herring, grant.likely, spear-devel, devicetree-discuss, linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 6718 bytes --]

On Fri, 2012-10-12 at 23:31 +0530, Viresh Kumar wrote: 
> This adds following helper routines:
> - of_property_read_u8_array()
> - of_property_read_u16_array()
> - of_property_read_u8()
> - of_property_read_u16()
> 
> First two actually share most of the code with of_property_read_u32_array(), so
> the common part is taken out into a macro, which can be used by all three
> *_array() routines.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/of/base.c  | 73 +++++++++++++++++++++++++++++++++++++++++++-----------
>  include/linux/of.h | 30 ++++++++++++++++++++++
>  2 files changed, 89 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 38892a9..446fe7f 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -670,6 +670,64 @@ struct device_node *of_find_node_by_phandle(phandle handle)
>  }
>  EXPORT_SYMBOL(of_find_node_by_phandle);
>  
> +#define of_property_read_array(_np, _pname, _out, _sz, _type)		\
> +	struct property *_prop = of_find_property(_np, _pname, NULL);	\
> +	const __be32 *_val;						\
> +									\
> +	if (!_prop)							\
> +		return -EINVAL;						\
> +	if (!_prop->value)						\
> +		return -ENODATA;					\
> +	if ((_sz * sizeof(*_out)) > _prop->length)			\
> +		return -EOVERFLOW;					\
> +									\
> +	_val = _prop->value;						\
> +	while (_sz--)							\
> +		*_out++ = (_type)be32_to_cpup(_val++);			\
How about 
*_out++ = (typeof(*_out))...
?

> +	return 0;
> +
> +/**
> + * of_property_read_u8_array - Find and read an array of u8 from a property.
> + *
> + * @np:		device node from which the property value is to be read.
> + * @propname:	name of the property to be searched.
> + * @out_value:	pointer to return value, modified only if return value is 0.
> + *
> + * Search for a property in a device node and read 8-bit value(s) from
> + * it. Returns 0 on success, -EINVAL if the property does not exist,
> + * -ENODATA if property does not have a value, and -EOVERFLOW if the
> + * property data isn't large enough.
> + *
> + * The out_value is modified only if a valid u8 value can be decoded.
> + */
> +int of_property_read_u8_array(const struct device_node *np,
> +			const char *propname, u8 *out_values, size_t sz)
> +{
> +	of_property_read_array(np, propname, out_values, sz, u8);
> +}
> +EXPORT_SYMBOL_GPL(of_property_read_u8_array);
> +
> +/**
> + * of_property_read_u16_array - Find and read an array of u16 from a property.
> + *
> + * @np:		device node from which the property value is to be read.
> + * @propname:	name of the property to be searched.
> + * @out_value:	pointer to return value, modified only if return value is 0.
> + *
> + * Search for a property in a device node and read 16-bit value(s) from
> + * it. Returns 0 on success, -EINVAL if the property does not exist,
> + * -ENODATA if property does not have a value, and -EOVERFLOW if the
> + * property data isn't large enough.
> + *
> + * The out_value is modified only if a valid u16 value can be decoded.
> + */
> +int of_property_read_u16_array(const struct device_node *np,
> +			const char *propname, u16 *out_values, size_t sz)
> +{
> +	of_property_read_array(np, propname, out_values, sz, u16);
> +}
> +EXPORT_SYMBOL_GPL(of_property_read_u16_array);
> +
>  /**
>   * of_property_read_u32_array - Find and read an array of 32 bit integers
>   * from a property.
> @@ -689,20 +747,7 @@ int of_property_read_u32_array(const struct device_node *np,
>  			       const char *propname, u32 *out_values,
>  			       size_t sz)
>  {
> -	struct property *prop = of_find_property(np, propname, NULL);
> -	const __be32 *val;
> -
> -	if (!prop)
> -		return -EINVAL;
> -	if (!prop->value)
> -		return -ENODATA;
> -	if ((sz * sizeof(*out_values)) > prop->length)
> -		return -EOVERFLOW;
> -
> -	val = prop->value;
> -	while (sz--)
> -		*out_values++ = be32_to_cpup(val++);
> -	return 0;
> +	of_property_read_array(np, propname, out_values, sz, u32);
>  }
>  EXPORT_SYMBOL_GPL(of_property_read_u32_array);
>  
> diff --git a/include/linux/of.h b/include/linux/of.h
> index 72843b7..e2d9b40 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -223,6 +223,10 @@ extern struct device_node *of_find_node_with_property(
>  extern struct property *of_find_property(const struct device_node *np,
>  					 const char *name,
>  					 int *lenp);
> +extern int of_property_read_u8_array(const struct device_node *np,
> +			const char *propname, u8 *out_values, size_t sz);
> +extern int of_property_read_u16_array(const struct device_node *np,
> +			const char *propname, u16 *out_values, size_t sz);
>  extern int of_property_read_u32_array(const struct device_node *np,
>  				      const char *propname,
>  				      u32 *out_values,
> @@ -357,6 +361,18 @@ static inline struct device_node *of_find_compatible_node(
>  	return NULL;
>  }
>  
> +static inline int of_property_read_u8_array(const struct device_node *np,
> +			const char *propname, u8 *out_values, size_t sz)
> +{
> +	return -ENOSYS;
> +}
> +
> +static inline int of_property_read_u16_array(const struct device_node *np,
> +			const char *propname, u16 *out_values, size_t sz)
> +{
> +	return -ENOSYS;
> +}
> +
>  static inline int of_property_read_u32_array(const struct device_node *np,
>  					     const char *propname,
>  					     u32 *out_values, size_t sz)
> @@ -463,6 +479,20 @@ static inline bool of_property_read_bool(const struct device_node *np,
>  	return prop ? true : false;
>  }
>  
> +static inline int of_property_read_u8(const struct device_node *np,
> +				       const char *propname,
> +				       u8 *out_value)
> +{
> +	return of_property_read_u8_array(np, propname, out_value, 1);
> +}
> +
> +static inline int of_property_read_u16(const struct device_node *np,
> +				       const char *propname,
> +				       u16 *out_value)
> +{
> +	return of_property_read_u16_array(np, propname, out_value, 1);
> +}
> +
>  static inline int of_property_read_u32(const struct device_node *np,
>  				       const char *propname,
>  				       u32 *out_value)

-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] dt: add helper function to read u8 & u16 variables & arrays
  2012-10-15  7:56 ` Shevchenko, Andriy
@ 2012-10-15  8:06   ` Viresh Kumar
  0 siblings, 0 replies; 12+ messages in thread
From: Viresh Kumar @ 2012-10-15  8:06 UTC (permalink / raw)
  To: Shevchenko, Andriy
  Cc: rob.herring, grant.likely, spear-devel, devicetree-discuss, linux-kernel

On 15 October 2012 13:26, Shevchenko, Andriy
<andriy.shevchenko@intel.com> wrote:
> On Fri, 2012-10-12 at 23:31 +0530, Viresh Kumar wrote:

>> +     while (_sz--)                                                   \
>> +             *_out++ = (_type)be32_to_cpup(_val++);                  \
> How about
> *_out++ = (typeof(*_out))...
> ?

:)


diff --git a/drivers/of/base.c b/drivers/of/base.c
index c5a09c2..039e178 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -670,7 +670,7 @@ struct device_node *of_find_node_by_phandle(phandle handle)
 }
 EXPORT_SYMBOL(of_find_node_by_phandle);

-#define of_property_read_array(_np, _pname, _out, _sz, _type)          \
+#define of_property_read_array(_np, _pname, _out, _sz)                 \
        struct property *_prop = of_find_property(_np, _pname, NULL);   \
        const __be32 *_val;                                             \
                                                                        \
@@ -683,7 +683,7 @@ EXPORT_SYMBOL(of_find_node_by_phandle);
                                                                        \
        _val = _prop->value;                                            \
        while (_sz--)                                                   \
-               *_out++ = (_type)be32_to_cpup(_val++);                  \
+               *_out++ = (typeof(*_out))be32_to_cpup(_val++);          \
        return 0;

 /**
@@ -703,7 +703,7 @@ EXPORT_SYMBOL(of_find_node_by_phandle);
 int of_property_read_u8_array(const struct device_node *np,
                        const char *propname, u8 *out_values, size_t sz)
 {
-       of_property_read_array(np, propname, out_values, sz, u8);
+       of_property_read_array(np, propname, out_values, sz);
 }
 EXPORT_SYMBOL_GPL(of_property_read_u8_array);

@@ -724,7 +724,7 @@ EXPORT_SYMBOL_GPL(of_property_read_u8_array);
 int of_property_read_u16_array(const struct device_node *np,
                        const char *propname, u16 *out_values, size_t sz)
 {
-       of_property_read_array(np, propname, out_values, sz, u16);
+       of_property_read_array(np, propname, out_values, sz);
 }
 EXPORT_SYMBOL_GPL(of_property_read_u16_array);

@@ -747,7 +747,7 @@ int of_property_read_u32_array(const struct device_node *np,
                               const char *propname, u32 *out_values,
                               size_t sz)
 {
-       of_property_read_array(np, propname, out_values, sz, u32);
+       of_property_read_array(np, propname, out_values, sz);
 }
 EXPORT_SYMBOL_GPL(of_property_read_u32_array);

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] dt: add helper function to read u8 & u16 variables & arrays
  2012-10-12 18:01 [PATCH] dt: add helper function to read u8 & u16 variables & arrays Viresh Kumar
  2012-10-15  7:56 ` Shevchenko, Andriy
@ 2012-10-25  7:03 ` Viresh Kumar
  2012-10-25 13:19   ` Rob Herring
  1 sibling, 1 reply; 12+ messages in thread
From: Viresh Kumar @ 2012-10-25  7:03 UTC (permalink / raw)
  To: rob.herring, grant.likely
  Cc: spear-devel, devicetree-discuss, linux-kernel, andriy.shevchenko,
	Viresh Kumar

On 12 October 2012 23:31, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> This adds following helper routines:
> - of_property_read_u8_array()
> - of_property_read_u16_array()
> - of_property_read_u8()
> - of_property_read_u16()
>
> First two actually share most of the code with of_property_read_u32_array(), so
> the common part is taken out into a macro, which can be used by all three
> *_array() routines.

Hi Rob,

Any comment here?

--
viresh

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] dt: add helper function to read u8 & u16 variables & arrays
  2012-10-25  7:03 ` Viresh Kumar
@ 2012-10-25 13:19   ` Rob Herring
  2012-10-25 14:18     ` Viresh Kumar
  0 siblings, 1 reply; 12+ messages in thread
From: Rob Herring @ 2012-10-25 13:19 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: grant.likely, spear-devel, devicetree-discuss, linux-kernel,
	andriy.shevchenko

On 10/25/2012 02:03 AM, Viresh Kumar wrote:
> On 12 October 2012 23:31, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>> This adds following helper routines:
>> - of_property_read_u8_array()
>> - of_property_read_u16_array()
>> - of_property_read_u8()
>> - of_property_read_u16()
>>
>> First two actually share most of the code with of_property_read_u32_array(), so
>> the common part is taken out into a macro, which can be used by all three
>> *_array() routines.
> 
> Hi Rob,
> 
> Any comment here?

For some reason, this does not show up on the list either in my mail or
mail list archives, but it is in patchwork. Can you resend it please.

The main question I have is be32_to_cpup() the right thing to do. I
would expect byte arrays to not need endian conversion, but I haven't
looked at how '/bits/ x' data is stored.

Rob

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] dt: add helper function to read u8 & u16 variables & arrays
  2012-10-25 13:19   ` Rob Herring
@ 2012-10-25 14:18     ` Viresh Kumar
  2012-10-26  4:17       ` Viresh Kumar
  0 siblings, 1 reply; 12+ messages in thread
From: Viresh Kumar @ 2012-10-25 14:18 UTC (permalink / raw)
  To: Rob Herring
  Cc: grant.likely, spear-devel, devicetree-discuss, linux-kernel,
	andriy.shevchenko

On 25 October 2012 18:49, Rob Herring <robherring2@gmail.com> wrote:
> For some reason, this does not show up on the list either in my mail or
> mail list archives, but it is in patchwork. Can you resend it please.

Strange. I kept you in --to field. I know people added in cc are
sometimes removed
by the list when they have selected "avoid duplicate mails" option for
their list.

> The main question I have is be32_to_cpup() the right thing to do. I
> would expect byte arrays to not need endian conversion, but I haven't
> looked at how '/bits/ x' data is stored.

The problem i see here is:

The data passed via DT comes as Little Endian in the kernel.

For a little endian system, byte zero will contain the data and so
(u8) val

look to be the correct thing.

For a big endian system, byte 3 will contain data as it is swapped by
be32_to_cpup.
So (u8) val would return value stored by byte 0 instead. ??

Does my logic look correct to you??

--
viresh

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] dt: add helper function to read u8 & u16 variables & arrays
  2012-10-25 14:18     ` Viresh Kumar
@ 2012-10-26  4:17       ` Viresh Kumar
  0 siblings, 0 replies; 12+ messages in thread
From: Viresh Kumar @ 2012-10-26  4:17 UTC (permalink / raw)
  To: Rob Herring
  Cc: grant.likely, spear-devel, devicetree-discuss, linux-kernel,
	andriy.shevchenko

On 25 October 2012 19:48, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> The problem i see here is:
>
> The data passed via DT comes as Little Endian in the kernel.
>
> For a little endian system, byte zero will contain the data and so
> (u8) val
>
> look to be the correct thing.
>
> For a big endian system, byte 3 will contain data as it is swapped by
> be32_to_cpup.
> So (u8) val would return value stored by byte 0 instead. ??

I feel above explanation was wrong. I didn't had a big endian system
to test this but this is what i derived theoretically. Consider following
sequence of commands:

u32 x = 0x01;              //This will store off-0: 1, off-3:0 for LE system
                                   // and will store off-0: 0, off-3:1
for BE system
u8 y = (u8) x;              // For any architecture type, i.e. big or
little this must store
                                   // 1 in y. This is ANCI C semantic
and should be architecture
                                   // independent.

Which would mean, type cast will give off-0 on LE and off-3 on BE systems. Don't
confuse this with getting values using pointers, as we try to get data
out of specific
locations in those cases.

So, my initial code seems to be doing the right thing. be32_to_cpup()
will move the
LSB to off-3 and that is what we will get during the cast.

I will resend my original code to you.

--
viresh

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] dt: add helper function to read u8 & u16 variables & arrays
  2012-11-20  4:45 Viresh Kumar
  2012-11-20  8:21 ` Shevchenko, Andriy
  2012-11-20 17:21 ` Stephen Warren
@ 2012-11-21  4:24 ` Rob Herring
  2 siblings, 0 replies; 12+ messages in thread
From: Rob Herring @ 2012-11-21  4:24 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: rob.herring, grant.likely, linaro-dev, andriy.shevchenko,
	swarren, patches, devicetree-discuss, spear-devel, linux-kernel

On 11/19/2012 10:45 PM, Viresh Kumar wrote:
> This adds following helper routines:
> - of_property_read_u8_array()
> - of_property_read_u16_array()
> - of_property_read_u8()
> - of_property_read_u16()
> 
> This expects arrays from DT to be passed as:
> - u8 array:
> 	property = /bits/ 8 <0x50 0x60 0x70>;
> - u16 array:
> 	property = /bits/ 16 <0x5000 0x6000 0x7000>;
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

Applied.

Rob

> ---
> V2->V3:
> - Expect u8 & u16 arrays to be passed using: /bits/ 8 or 16
> - remove common macro, as not much common now :(
> - Tested on ARM platform.
> 
>  drivers/of/base.c  | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/of.h | 30 +++++++++++++++++++++
>  2 files changed, 107 insertions(+)
> 
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index af3b22a..f564e31 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -671,12 +671,89 @@ struct device_node *of_find_node_by_phandle(phandle handle)
>  EXPORT_SYMBOL(of_find_node_by_phandle);
>  
>  /**
> + * of_property_read_u8_array - Find and read an array of u8 from a property.
> + *
> + * @np:		device node from which the property value is to be read.
> + * @propname:	name of the property to be searched.
> + * @out_value:	pointer to return value, modified only if return value is 0.
> + * @sz:		number of array elements to read
> + *
> + * Search for a property in a device node and read 8-bit value(s) from
> + * it. Returns 0 on success, -EINVAL if the property does not exist,
> + * -ENODATA if property does not have a value, and -EOVERFLOW if the
> + * property data isn't large enough.
> + *
> + * dts entry of array should be like:
> + *	property = /bits/ 8 <0x50 0x60 0x70>;
> + *
> + * The out_value is modified only if a valid u8 value can be decoded.
> + */
> +int of_property_read_u8_array(const struct device_node *np,
> +			const char *propname, u8 *out_values, size_t sz)
> +{
> +	struct property *prop = of_find_property(np, propname, NULL);
> +	const u8 *val;
> +
> +	if (!prop)
> +		return -EINVAL;
> +	if (!prop->value)
> +		return -ENODATA;
> +	if ((sz * sizeof(*out_values)) > prop->length)
> +		return -EOVERFLOW;
> +
> +	val = prop->value;
> +	while (sz--)
> +		*out_values++ = *val++;
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(of_property_read_u8_array);
> +
> +/**
> + * of_property_read_u16_array - Find and read an array of u16 from a property.
> + *
> + * @np:		device node from which the property value is to be read.
> + * @propname:	name of the property to be searched.
> + * @out_value:	pointer to return value, modified only if return value is 0.
> + * @sz:		number of array elements to read
> + *
> + * Search for a property in a device node and read 16-bit value(s) from
> + * it. Returns 0 on success, -EINVAL if the property does not exist,
> + * -ENODATA if property does not have a value, and -EOVERFLOW if the
> + * property data isn't large enough.
> + *
> + * dts entry of array should be like:
> + *	property = /bits/ 16 <0x5000 0x6000 0x7000>;
> + *
> + * The out_value is modified only if a valid u16 value can be decoded.
> + */
> +int of_property_read_u16_array(const struct device_node *np,
> +			const char *propname, u16 *out_values, size_t sz)
> +{
> +	struct property *prop = of_find_property(np, propname, NULL);
> +	const __be16 *val;
> +
> +	if (!prop)
> +		return -EINVAL;
> +	if (!prop->value)
> +		return -ENODATA;
> +	if ((sz * sizeof(*out_values)) > prop->length)
> +		return -EOVERFLOW;
> +
> +	val = prop->value;
> +	while (sz--)
> +		*out_values++ = be16_to_cpup(val++);
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(of_property_read_u16_array);
> +
> +/**
>   * of_property_read_u32_array - Find and read an array of 32 bit integers
>   * from a property.
>   *
>   * @np:		device node from which the property value is to be read.
>   * @propname:	name of the property to be searched.
>   * @out_value:	pointer to return value, modified only if return value is 0.
> + * @sz:		number of array elements to read
>   *
>   * Search for a property in a device node and read 32-bit value(s) from
>   * it. Returns 0 on success, -EINVAL if the property does not exist,
> diff --git a/include/linux/of.h b/include/linux/of.h
> index b4e50d5..bfdc130 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -223,6 +223,10 @@ extern struct device_node *of_find_node_with_property(
>  extern struct property *of_find_property(const struct device_node *np,
>  					 const char *name,
>  					 int *lenp);
> +extern int of_property_read_u8_array(const struct device_node *np,
> +			const char *propname, u8 *out_values, size_t sz);
> +extern int of_property_read_u16_array(const struct device_node *np,
> +			const char *propname, u16 *out_values, size_t sz);
>  extern int of_property_read_u32_array(const struct device_node *np,
>  				      const char *propname,
>  				      u32 *out_values,
> @@ -364,6 +368,18 @@ static inline struct device_node *of_find_compatible_node(
>  	return NULL;
>  }
>  
> +static inline int of_property_read_u8_array(const struct device_node *np,
> +			const char *propname, u8 *out_values, size_t sz)
> +{
> +	return -ENOSYS;
> +}
> +
> +static inline int of_property_read_u16_array(const struct device_node *np,
> +			const char *propname, u16 *out_values, size_t sz)
> +{
> +	return -ENOSYS;
> +}
> +
>  static inline int of_property_read_u32_array(const struct device_node *np,
>  					     const char *propname,
>  					     u32 *out_values, size_t sz)
> @@ -470,6 +486,20 @@ static inline bool of_property_read_bool(const struct device_node *np,
>  	return prop ? true : false;
>  }
>  
> +static inline int of_property_read_u8(const struct device_node *np,
> +				       const char *propname,
> +				       u8 *out_value)
> +{
> +	return of_property_read_u8_array(np, propname, out_value, 1);
> +}
> +
> +static inline int of_property_read_u16(const struct device_node *np,
> +				       const char *propname,
> +				       u16 *out_value)
> +{
> +	return of_property_read_u16_array(np, propname, out_value, 1);
> +}
> +
>  static inline int of_property_read_u32(const struct device_node *np,
>  				       const char *propname,
>  				       u32 *out_value)
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] dt: add helper function to read u8 & u16 variables & arrays
  2012-11-20  4:45 Viresh Kumar
  2012-11-20  8:21 ` Shevchenko, Andriy
@ 2012-11-20 17:21 ` Stephen Warren
  2012-11-21  4:24 ` Rob Herring
  2 siblings, 0 replies; 12+ messages in thread
From: Stephen Warren @ 2012-11-20 17:21 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: rob.herring, grant.likely, spear-devel, devicetree-discuss,
	linux-kernel, andriy.shevchenko, linaro-dev, patches

On 11/19/2012 09:45 PM, Viresh Kumar wrote:
> This adds following helper routines:
> - of_property_read_u8_array()
> - of_property_read_u16_array()
> - of_property_read_u8()
> - of_property_read_u16()
> 
> This expects arrays from DT to be passed as:
> - u8 array:
> 	property = /bits/ 8 <0x50 0x60 0x70>;
> - u16 array:
> 	property = /bits/ 16 <0x5000 0x6000 0x7000>;

Reviewed-by: Stephen Warren <swarren@nvidia.com>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] dt: add helper function to read u8 & u16 variables & arrays
  2012-11-20  8:21 ` Shevchenko, Andriy
@ 2012-11-20  8:25   ` Viresh Kumar
  0 siblings, 0 replies; 12+ messages in thread
From: Viresh Kumar @ 2012-11-20  8:25 UTC (permalink / raw)
  To: Shevchenko, Andriy
  Cc: rob.herring, grant.likely, spear-devel, devicetree-discuss,
	linux-kernel, linaro-dev, patches, swarren

On 20 November 2012 13:51, Shevchenko, Andriy
<andriy.shevchenko@intel.com> wrote:
> You could at least create macro to do a precheck if you want to.
>
> Like
> #define CHECK_PROP(prop, sz, out)
>
> {
> if (!prop)
>                 return -EINVAL;
>         if (!prop->value)
>                 return -ENODATA;
>         if ((sz * sizeof(*out)) > prop->length)
>                 return -EOVERFLOW;
> }

Hi Andriy,

Initially i started with a macro for the complete routine, but later as types
started to play important role in it, i have to split it to routines.

I thought of this idea to do prop check in a macro, but then i thought
it might cover a bigger range of files and so thought of doing that separately.
For, simplicity left it this time.

--
viresh

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] dt: add helper function to read u8 & u16 variables & arrays
  2012-11-20  4:45 Viresh Kumar
@ 2012-11-20  8:21 ` Shevchenko, Andriy
  2012-11-20  8:25   ` Viresh Kumar
  2012-11-20 17:21 ` Stephen Warren
  2012-11-21  4:24 ` Rob Herring
  2 siblings, 1 reply; 12+ messages in thread
From: Shevchenko, Andriy @ 2012-11-20  8:21 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: rob.herring, grant.likely, spear-devel, devicetree-discuss,
	linux-kernel, linaro-dev, patches, swarren

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 4972 bytes --]

On Tue, 2012-11-20 at 10:15 +0530, Viresh Kumar wrote: 
> This adds following helper routines:
> - of_property_read_u8_array()
> - of_property_read_u16_array()
> - of_property_read_u8()
> - of_property_read_u16()
> 
> This expects arrays from DT to be passed as:
> - u8 array:
> 	property = /bits/ 8 <0x50 0x60 0x70>;
> - u16 array:
> 	property = /bits/ 16 <0x5000 0x6000 0x7000>;
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> V2->V3:
> - Expect u8 & u16 arrays to be passed using: /bits/ 8 or 16
> - remove common macro, as not much common now :(

You could at least create macro to do a precheck if you want to.

Like 
#define CHECK_PROP(prop, sz, out)

{
if (!prop) 
		return -EINVAL;
	if (!prop->value)
		return -ENODATA;
	if ((sz * sizeof(*out)) > prop->length)
		return -EOVERFLOW;
}

Of course you can do even robust macro to cover types, but I don't like
ugly macros (as I can't see how it could be shaped nicely).

> - Tested on ARM platform.
> 

> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -671,12 +671,89 @@ struct device_node *of_find_node_by_phandle(phandle handle)
>  EXPORT_SYMBOL(of_find_node_by_phandle);
>  
>  /**
> + * of_property_read_u8_array - Find and read an array of u8 from a property.
> + *
> + * @np:		device node from which the property value is to be read.
> + * @propname:	name of the property to be searched.
> + * @out_value:	pointer to return value, modified only if return value is 0.
> + * @sz:		number of array elements to read
> + *
> + * Search for a property in a device node and read 8-bit value(s) from
> + * it. Returns 0 on success, -EINVAL if the property does not exist,
> + * -ENODATA if property does not have a value, and -EOVERFLOW if the
> + * property data isn't large enough.
> + *
> + * dts entry of array should be like:
> + *	property = /bits/ 8 <0x50 0x60 0x70>;
> + *
> + * The out_value is modified only if a valid u8 value can be decoded.
> + */
> +int of_property_read_u8_array(const struct device_node *np,
> +			const char *propname, u8 *out_values, size_t sz)
> +{
> +	struct property *prop = of_find_property(np, propname, NULL);
> +	const u8 *val;
> +
> +	if (!prop)
> +		return -EINVAL;
> +	if (!prop->value)
> +		return -ENODATA;
> +	if ((sz * sizeof(*out_values)) > prop->length)
> +		return -EOVERFLOW;
> +
> +	val = prop->value;
> +	while (sz--)
> +		*out_values++ = *val++;
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(of_property_read_u8_array);
> +
> +/**
> + * of_property_read_u16_array - Find and read an array of u16 from a property.
> + *
> + * @np:		device node from which the property value is to be read.
> + * @propname:	name of the property to be searched.
> + * @out_value:	pointer to return value, modified only if return value is 0.
> + * @sz:		number of array elements to read
> + *
> + * Search for a property in a device node and read 16-bit value(s) from
> + * it. Returns 0 on success, -EINVAL if the property does not exist,
> + * -ENODATA if property does not have a value, and -EOVERFLOW if the
> + * property data isn't large enough.
> + *
> + * dts entry of array should be like:
> + *	property = /bits/ 16 <0x5000 0x6000 0x7000>;
> + *
> + * The out_value is modified only if a valid u16 value can be decoded.
> + */
> +int of_property_read_u16_array(const struct device_node *np,
> +			const char *propname, u16 *out_values, size_t sz)
> +{
> +	struct property *prop = of_find_property(np, propname, NULL);
> +	const __be16 *val;
> +
> +	if (!prop)
> +		return -EINVAL;
> +	if (!prop->value)
> +		return -ENODATA;
> +	if ((sz * sizeof(*out_values)) > prop->length)
> +		return -EOVERFLOW;
> +
> +	val = prop->value;
> +	while (sz--)
> +		*out_values++ = be16_to_cpup(val++);
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(of_property_read_u16_array);
> +
> +/**
>   * of_property_read_u32_array - Find and read an array of 32 bit integers
>   * from a property.
>   *
>   * @np:		device node from which the property value is to be read.
>   * @propname:	name of the property to be searched.
>   * @out_value:	pointer to return value, modified only if return value is 0.
> + * @sz:		number of array elements to read
>   *
>   * Search for a property in a device node and read 32-bit value(s) from
>   * it. Returns 0 on success, -EINVAL if the property does not exist,

-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] dt: add helper function to read u8 & u16 variables & arrays
@ 2012-11-20  4:45 Viresh Kumar
  2012-11-20  8:21 ` Shevchenko, Andriy
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Viresh Kumar @ 2012-11-20  4:45 UTC (permalink / raw)
  To: rob.herring, grant.likely
  Cc: spear-devel, devicetree-discuss, linux-kernel, andriy.shevchenko,
	linaro-dev, patches, swarren, Viresh Kumar

This adds following helper routines:
- of_property_read_u8_array()
- of_property_read_u16_array()
- of_property_read_u8()
- of_property_read_u16()

This expects arrays from DT to be passed as:
- u8 array:
	property = /bits/ 8 <0x50 0x60 0x70>;
- u16 array:
	property = /bits/ 16 <0x5000 0x6000 0x7000>;

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
V2->V3:
- Expect u8 & u16 arrays to be passed using: /bits/ 8 or 16
- remove common macro, as not much common now :(
- Tested on ARM platform.

 drivers/of/base.c  | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/of.h | 30 +++++++++++++++++++++
 2 files changed, 107 insertions(+)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index af3b22a..f564e31 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -671,12 +671,89 @@ struct device_node *of_find_node_by_phandle(phandle handle)
 EXPORT_SYMBOL(of_find_node_by_phandle);
 
 /**
+ * of_property_read_u8_array - Find and read an array of u8 from a property.
+ *
+ * @np:		device node from which the property value is to be read.
+ * @propname:	name of the property to be searched.
+ * @out_value:	pointer to return value, modified only if return value is 0.
+ * @sz:		number of array elements to read
+ *
+ * Search for a property in a device node and read 8-bit value(s) from
+ * it. Returns 0 on success, -EINVAL if the property does not exist,
+ * -ENODATA if property does not have a value, and -EOVERFLOW if the
+ * property data isn't large enough.
+ *
+ * dts entry of array should be like:
+ *	property = /bits/ 8 <0x50 0x60 0x70>;
+ *
+ * The out_value is modified only if a valid u8 value can be decoded.
+ */
+int of_property_read_u8_array(const struct device_node *np,
+			const char *propname, u8 *out_values, size_t sz)
+{
+	struct property *prop = of_find_property(np, propname, NULL);
+	const u8 *val;
+
+	if (!prop)
+		return -EINVAL;
+	if (!prop->value)
+		return -ENODATA;
+	if ((sz * sizeof(*out_values)) > prop->length)
+		return -EOVERFLOW;
+
+	val = prop->value;
+	while (sz--)
+		*out_values++ = *val++;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_property_read_u8_array);
+
+/**
+ * of_property_read_u16_array - Find and read an array of u16 from a property.
+ *
+ * @np:		device node from which the property value is to be read.
+ * @propname:	name of the property to be searched.
+ * @out_value:	pointer to return value, modified only if return value is 0.
+ * @sz:		number of array elements to read
+ *
+ * Search for a property in a device node and read 16-bit value(s) from
+ * it. Returns 0 on success, -EINVAL if the property does not exist,
+ * -ENODATA if property does not have a value, and -EOVERFLOW if the
+ * property data isn't large enough.
+ *
+ * dts entry of array should be like:
+ *	property = /bits/ 16 <0x5000 0x6000 0x7000>;
+ *
+ * The out_value is modified only if a valid u16 value can be decoded.
+ */
+int of_property_read_u16_array(const struct device_node *np,
+			const char *propname, u16 *out_values, size_t sz)
+{
+	struct property *prop = of_find_property(np, propname, NULL);
+	const __be16 *val;
+
+	if (!prop)
+		return -EINVAL;
+	if (!prop->value)
+		return -ENODATA;
+	if ((sz * sizeof(*out_values)) > prop->length)
+		return -EOVERFLOW;
+
+	val = prop->value;
+	while (sz--)
+		*out_values++ = be16_to_cpup(val++);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_property_read_u16_array);
+
+/**
  * of_property_read_u32_array - Find and read an array of 32 bit integers
  * from a property.
  *
  * @np:		device node from which the property value is to be read.
  * @propname:	name of the property to be searched.
  * @out_value:	pointer to return value, modified only if return value is 0.
+ * @sz:		number of array elements to read
  *
  * Search for a property in a device node and read 32-bit value(s) from
  * it. Returns 0 on success, -EINVAL if the property does not exist,
diff --git a/include/linux/of.h b/include/linux/of.h
index b4e50d5..bfdc130 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -223,6 +223,10 @@ extern struct device_node *of_find_node_with_property(
 extern struct property *of_find_property(const struct device_node *np,
 					 const char *name,
 					 int *lenp);
+extern int of_property_read_u8_array(const struct device_node *np,
+			const char *propname, u8 *out_values, size_t sz);
+extern int of_property_read_u16_array(const struct device_node *np,
+			const char *propname, u16 *out_values, size_t sz);
 extern int of_property_read_u32_array(const struct device_node *np,
 				      const char *propname,
 				      u32 *out_values,
@@ -364,6 +368,18 @@ static inline struct device_node *of_find_compatible_node(
 	return NULL;
 }
 
+static inline int of_property_read_u8_array(const struct device_node *np,
+			const char *propname, u8 *out_values, size_t sz)
+{
+	return -ENOSYS;
+}
+
+static inline int of_property_read_u16_array(const struct device_node *np,
+			const char *propname, u16 *out_values, size_t sz)
+{
+	return -ENOSYS;
+}
+
 static inline int of_property_read_u32_array(const struct device_node *np,
 					     const char *propname,
 					     u32 *out_values, size_t sz)
@@ -470,6 +486,20 @@ static inline bool of_property_read_bool(const struct device_node *np,
 	return prop ? true : false;
 }
 
+static inline int of_property_read_u8(const struct device_node *np,
+				       const char *propname,
+				       u8 *out_value)
+{
+	return of_property_read_u8_array(np, propname, out_value, 1);
+}
+
+static inline int of_property_read_u16(const struct device_node *np,
+				       const char *propname,
+				       u16 *out_value)
+{
+	return of_property_read_u16_array(np, propname, out_value, 1);
+}
+
 static inline int of_property_read_u32(const struct device_node *np,
 				       const char *propname,
 				       u32 *out_value)
-- 
1.7.12.rc2.18.g61b472e



^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2012-11-21  4:24 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-12 18:01 [PATCH] dt: add helper function to read u8 & u16 variables & arrays Viresh Kumar
2012-10-15  7:56 ` Shevchenko, Andriy
2012-10-15  8:06   ` Viresh Kumar
2012-10-25  7:03 ` Viresh Kumar
2012-10-25 13:19   ` Rob Herring
2012-10-25 14:18     ` Viresh Kumar
2012-10-26  4:17       ` Viresh Kumar
2012-11-20  4:45 Viresh Kumar
2012-11-20  8:21 ` Shevchenko, Andriy
2012-11-20  8:25   ` Viresh Kumar
2012-11-20 17:21 ` Stephen Warren
2012-11-21  4:24 ` Rob Herring

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