2012-11-20 04:45:29

by Viresh Kumar

[permalink] [raw]
Subject: [PATCH] dt: add helper function to read u8 & u16 variables & arrays

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 <[email protected]>
---
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


2012-11-20 08:21:24

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH] dt: add helper function to read u8 & u16 variables & arrays

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 <[email protected]>
> ---
> 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 <[email protected]>
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?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2012-11-20 08:25:07

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH] dt: add helper function to read u8 & u16 variables & arrays

On 20 November 2012 13:51, Shevchenko, Andriy
<[email protected]> 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

2012-11-20 17:21:45

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH] dt: add helper function to read u8 & u16 variables & arrays

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 <[email protected]>

2012-11-21 04:24:11

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH] dt: add helper function to read u8 & u16 variables & arrays

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 <[email protected]>

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