* [PATCH 1/3] of: add functions to count number of elements in a property
2014-02-11 23:59 [PATCH 0/3] of: functions to count number of elements and convert regulators Heiko Stübner
@ 2014-02-12 0:00 ` Heiko Stübner
2014-02-12 15:48 ` Mark Brown
2014-02-12 0:01 ` [PATCH 2/3] regulator: gpio-regulator: do not open-code counting and access of dt array elements Heiko Stübner
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Heiko Stübner @ 2014-02-12 0:00 UTC (permalink / raw)
To: grant.likely-QSEj5FYQhm4dnm+yROfE0A
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Liam Girdwood, Mark Brown
From: Heiko Stuebner <heiko.stuebner-HCpLIkUQxWGakBO8gow8eQ@public.gmane.org>
The need to know the number of array elements in a property is
a common pattern. To prevent duplication of open-coded implementations
add a helper static function that also centralises strict sanity
checking and DTB format details, as well as a set of wrapper functions
for u8, u16, u32 and u64.
Suggested-by: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
Signed-off-by: Heiko Stuebner <heiko.stuebner-HCpLIkUQxWGakBO8gow8eQ@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Acked-by: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
drivers/of/base.c | 32 ++++++++++++++++++++++
include/linux/of.h | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 108 insertions(+)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 239e3da..bed63c2 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -886,6 +886,38 @@ struct device_node *of_find_node_by_phandle(phandle handle)
EXPORT_SYMBOL(of_find_node_by_phandle);
/**
+ * of_property_count_elems_of_size - Count the number of elements in a property
+ *
+ * @np: device node from which the property value is to be read.
+ * @propname: name of the property to be searched.
+ * @elem_size: size of the individual element
+ *
+ * Search for a property in a device node and count the number of elements of
+ * size elem_size in it. Returns number of elements on sucess, -EINVAL if the
+ * property does not exist or its length does not match a multiple of elem_size
+ * and -ENODATA if the property does not have a value.
+ */
+int of_property_count_elems_of_size(const struct device_node *np,
+ const char *propname, int elem_size)
+{
+ struct property *prop = of_find_property(np, propname, NULL);
+
+ if (!prop)
+ return -EINVAL;
+ if (!prop->value)
+ return -ENODATA;
+
+ if (prop->length % elem_size != 0) {
+ pr_err("size of %s in node %s is not a multiple of %d\n",
+ propname, np->full_name, elem_size);
+ return -EINVAL;
+ }
+
+ return prop->length / elem_size;
+}
+EXPORT_SYMBOL_GPL(of_property_count_elems_of_size);
+
+/**
* of_find_property_value_of_size
*
* @np: device node from which the property value is to be read.
diff --git a/include/linux/of.h b/include/linux/of.h
index b3d0f6d..8cc9cef 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -218,6 +218,8 @@ 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_count_elems_of_size(const struct device_node *np,
+ const char *propname, int elem_size);
extern int of_property_read_u32_index(const struct device_node *np,
const char *propname,
u32 index, u32 *out_value);
@@ -410,6 +412,12 @@ static inline struct device_node *of_find_compatible_node(
return NULL;
}
+static inline int of_property_count_elems_of_size(const struct device_node *np,
+ const char *propname, int elem_size)
+{
+ return -ENOSYS;
+}
+
static inline int of_property_read_u32_index(const struct device_node *np,
const char *propname, u32 index, u32 *out_value)
{
@@ -556,6 +564,74 @@ static inline struct device_node *of_find_matching_node(
}
/**
+ * of_property_count_u8_elems - Count the number of u8 elements in a property
+ *
+ * @np: device node from which the property value is to be read.
+ * @propname: name of the property to be searched.
+ *
+ * Search for a property in a device node and count the number of u8 elements
+ * in it. Returns number of elements on sucess, -EINVAL if the property does
+ * not exist or its length does not match a multiple of u8 and -ENODATA if the
+ * property does not have a value.
+ */
+static inline int of_property_count_u8_elems(const struct device_node *np,
+ const char *propname)
+{
+ return of_property_count_elems_of_size(np, propname, sizeof(u8));
+}
+
+/**
+ * of_property_count_u16_elems - Count the number of u16 elements in a property
+ *
+ * @np: device node from which the property value is to be read.
+ * @propname: name of the property to be searched.
+ *
+ * Search for a property in a device node and count the number of u16 elements
+ * in it. Returns number of elements on sucess, -EINVAL if the property does
+ * not exist or its length does not match a multiple of u16 and -ENODATA if the
+ * property does not have a value.
+ */
+static inline int of_property_count_u16_elems(const struct device_node *np,
+ const char *propname)
+{
+ return of_property_count_elems_of_size(np, propname, sizeof(u16));
+}
+
+/**
+ * of_property_count_u32_elems - Count the number of u32 elements in a property
+ *
+ * @np: device node from which the property value is to be read.
+ * @propname: name of the property to be searched.
+ *
+ * Search for a property in a device node and count the number of u32 elements
+ * in it. Returns number of elements on sucess, -EINVAL if the property does
+ * not exist or its length does not match a multiple of u32 and -ENODATA if the
+ * property does not have a value.
+ */
+static inline int of_property_count_u32_elems(const struct device_node *np,
+ const char *propname)
+{
+ return of_property_count_elems_of_size(np, propname, sizeof(u32));
+}
+
+/**
+ * of_property_count_u64_elems - Count the number of u64 elements in a property
+ *
+ * @np: device node from which the property value is to be read.
+ * @propname: name of the property to be searched.
+ *
+ * Search for a property in a device node and count the number of u64 elements
+ * in it. Returns number of elements on sucess, -EINVAL if the property does
+ * not exist or its length does not match a multiple of u64 and -ENODATA if the
+ * property does not have a value.
+ */
+static inline int of_property_count_u64_elems(const struct device_node *np,
+ const char *propname)
+{
+ return of_property_count_elems_of_size(np, propname, sizeof(u64));
+}
+
+/**
* of_property_read_bool - Findfrom a property
* @np: device node from which the property value is to be read.
* @propname: name of the property to be searched.
--
1.7.10.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
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 1/3] of: add functions to count number of elements in a property
2014-02-12 0:00 ` [PATCH 1/3] of: add functions to count number of elements in a property Heiko Stübner
@ 2014-02-12 15:48 ` Mark Brown
0 siblings, 0 replies; 8+ messages in thread
From: Mark Brown @ 2014-02-12 15:48 UTC (permalink / raw)
To: Heiko Stübner
Cc: grant.likely, robh+dt, devicetree, linux-kernel, Liam Girdwood
[-- Attachment #1: Type: text/plain, Size: 454 bytes --]
On Wed, Feb 12, 2014 at 01:00:34AM +0100, Heiko Stübner wrote:
> From: Heiko Stuebner <heiko.stuebner@bqreaders.com>
>
> The need to know the number of array elements in a property is
> a common pattern. To prevent duplication of open-coded implementations
> add a helper static function that also centralises strict sanity
> checking and DTB format details, as well as a set of wrapper functions
> for u8, u16, u32 and u64.
Applied, thanks.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/3] regulator: gpio-regulator: do not open-code counting and access of dt array elements
2014-02-11 23:59 [PATCH 0/3] of: functions to count number of elements and convert regulators Heiko Stübner
2014-02-12 0:00 ` [PATCH 1/3] of: add functions to count number of elements in a property Heiko Stübner
@ 2014-02-12 0:01 ` Heiko Stübner
2014-02-12 15:51 ` Mark Brown
2014-02-12 0:01 ` [PATCH 3/3] regulator: ti-abb-regulator: " Heiko Stübner
2014-02-12 9:25 ` [PATCH 0/3] of: functions to count number of elements and convert regulators Mark Rutland
3 siblings, 1 reply; 8+ messages in thread
From: Heiko Stübner @ 2014-02-12 0:01 UTC (permalink / raw)
To: grant.likely-QSEj5FYQhm4dnm+yROfE0A
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Liam Girdwood, Mark Brown
From: Heiko Stuebner <heiko.stuebner-HCpLIkUQxWGakBO8gow8eQ@public.gmane.org>
Open coding the counting of elements in a dt-property is abstracted by the newly
introduced of_property_count_uXX_elems functions. Additionally the raw iteration
over the states element exposes the endian conversion and dtb-format details,
which according to Mark Rutland "would be nice to limit [...] to of_ helper
functions".
Thus change gpio-regulator to use the helper for element counting and
of_property_read_u32_index for retrieval of individual values.
This makes it possible to remove the raw access to the states property entirely.
Signed-off-by: Heiko Stuebner <heiko.stuebner-HCpLIkUQxWGakBO8gow8eQ@public.gmane.org>
---
drivers/regulator/gpio-regulator.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/regulator/gpio-regulator.c b/drivers/regulator/gpio-regulator.c
index ac3a8c7..500aa3b 100644
--- a/drivers/regulator/gpio-regulator.c
+++ b/drivers/regulator/gpio-regulator.c
@@ -136,7 +136,6 @@ static struct gpio_regulator_config *
of_get_gpio_regulator_config(struct device *dev, struct device_node *np)
{
struct gpio_regulator_config *config;
- struct property *prop;
const char *regtype;
int proplen, gpio, i;
int ret;
@@ -191,14 +190,12 @@ of_get_gpio_regulator_config(struct device *dev, struct device_node *np)
}
/* Fetch states. */
- prop = of_find_property(np, "states", NULL);
- if (!prop) {
+ proplen = of_property_count_u32_elems(np, "states");
+ if (proplen < 0) {
dev_err(dev, "No 'states' property found\n");
return ERR_PTR(-EINVAL);
}
- proplen = prop->length / sizeof(int);
-
config->states = devm_kzalloc(dev,
sizeof(struct gpio_regulator_state)
* (proplen / 2),
@@ -207,10 +204,10 @@ of_get_gpio_regulator_config(struct device *dev, struct device_node *np)
return ERR_PTR(-ENOMEM);
for (i = 0; i < proplen / 2; i++) {
- config->states[i].value =
- be32_to_cpup((int *)prop->value + (i * 2));
- config->states[i].gpios =
- be32_to_cpup((int *)prop->value + (i * 2 + 1));
+ of_property_read_u32_index(np, "states", i * 2,
+ &config->states[i].value);
+ of_property_read_u32_index(np, "states", i * 2 + 1,
+ &config->states[i].gpios);
}
config->nr_states = i;
--
1.7.10.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
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 2/3] regulator: gpio-regulator: do not open-code counting and access of dt array elements
2014-02-12 0:01 ` [PATCH 2/3] regulator: gpio-regulator: do not open-code counting and access of dt array elements Heiko Stübner
@ 2014-02-12 15:51 ` Mark Brown
0 siblings, 0 replies; 8+ messages in thread
From: Mark Brown @ 2014-02-12 15:51 UTC (permalink / raw)
To: Heiko Stübner
Cc: grant.likely, robh+dt, devicetree, linux-kernel, Liam Girdwood
[-- Attachment #1: Type: text/plain, Size: 311 bytes --]
On Wed, Feb 12, 2014 at 01:01:08AM +0100, Heiko Stübner wrote:
> From: Heiko Stuebner <heiko.stuebner@bqreaders.com>
>
> Open coding the counting of elements in a dt-property is abstracted by the newly
> introduced of_property_count_uXX_elems functions. Additionally the raw iteration
Applied, thanks.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 3/3] regulator: ti-abb-regulator: do not open-code counting and access of dt array elements
2014-02-11 23:59 [PATCH 0/3] of: functions to count number of elements and convert regulators Heiko Stübner
2014-02-12 0:00 ` [PATCH 1/3] of: add functions to count number of elements in a property Heiko Stübner
2014-02-12 0:01 ` [PATCH 2/3] regulator: gpio-regulator: do not open-code counting and access of dt array elements Heiko Stübner
@ 2014-02-12 0:01 ` Heiko Stübner
2014-02-12 15:50 ` Mark Brown
2014-02-12 9:25 ` [PATCH 0/3] of: functions to count number of elements and convert regulators Mark Rutland
3 siblings, 1 reply; 8+ messages in thread
From: Heiko Stübner @ 2014-02-12 0:01 UTC (permalink / raw)
To: grant.likely-QSEj5FYQhm4dnm+yROfE0A
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Liam Girdwood, Mark Brown
From: Heiko Stuebner <heiko.stuebner-HCpLIkUQxWGakBO8gow8eQ@public.gmane.org>
Open coding the counting of elements in a dt-property is abstracted by the newly
introduced of_property_count_uXX_elems functions. Additionally the raw iteration
over the states element exposes the endian conversion and dtb-format details,
which according to Mark Rutland "would be nice to limit [...] to of_ helper
functions".
Thus change ti-abb-regulator to use the helper for element counting and
of_property_read_u32_index for retrieval of individual values.
This makes it possible to remove the raw access to the property entirely.
Signed-off-by: Heiko Stuebner <heiko.stuebner-HCpLIkUQxWGakBO8gow8eQ@public.gmane.org>
---
drivers/regulator/ti-abb-regulator.c | 43 ++++++++++++++++------------------
1 file changed, 20 insertions(+), 23 deletions(-)
diff --git a/drivers/regulator/ti-abb-regulator.c b/drivers/regulator/ti-abb-regulator.c
index a97d0c5..804c83a 100644
--- a/drivers/regulator/ti-abb-regulator.c
+++ b/drivers/regulator/ti-abb-regulator.c
@@ -507,32 +507,24 @@ static int ti_abb_init_table(struct device *dev, struct ti_abb *abb,
struct regulator_init_data *rinit_data)
{
struct ti_abb_info *info;
- const struct property *prop;
- const __be32 *abb_info;
const u32 num_values = 6;
char *pname = "ti,abb_info";
- u32 num_entries, i;
+ u32 i;
unsigned int *volt_table;
- int min_uV = INT_MAX, max_uV = 0;
+ int num_entries, min_uV = INT_MAX, max_uV = 0;
struct regulation_constraints *c = &rinit_data->constraints;
- prop = of_find_property(dev->of_node, pname, NULL);
- if (!prop) {
- dev_err(dev, "No '%s' property?\n", pname);
- return -ENODEV;
- }
-
- if (!prop->value) {
- dev_err(dev, "Empty '%s' property?\n", pname);
- return -ENODATA;
- }
-
/*
* Each abb_info is a set of n-tuple, where n is num_values, consisting
* of voltage and a set of detection logic for ABB information for that
* voltage to apply.
*/
- num_entries = prop->length / sizeof(u32);
+ num_entries = of_property_count_u32_elems(dev->of_node, pname);
+ if (num_entries < 0) {
+ dev_err(dev, "No '%s' property?\n", pname);
+ return -ENODEV;
+ }
+
if (!num_entries || (num_entries % num_values)) {
dev_err(dev, "All '%s' list entries need %d vals\n", pname,
num_values);
@@ -561,18 +553,23 @@ static int ti_abb_init_table(struct device *dev, struct ti_abb *abb,
/* We do not know where the OPP voltage is at the moment */
abb->current_info_idx = -EINVAL;
- abb_info = prop->value;
for (i = 0; i < num_entries; i++, info++, volt_table++) {
u32 efuse_offset, rbb_mask, fbb_mask, vset_mask;
u32 efuse_val;
/* NOTE: num_values should equal to entries picked up here */
- *volt_table = be32_to_cpup(abb_info++);
- info->opp_sel = be32_to_cpup(abb_info++);
- efuse_offset = be32_to_cpup(abb_info++);
- rbb_mask = be32_to_cpup(abb_info++);
- fbb_mask = be32_to_cpup(abb_info++);
- vset_mask = be32_to_cpup(abb_info++);
+ of_property_read_u32_index(dev->of_node, pname, i * num_values,
+ volt_table);
+ of_property_read_u32_index(dev->of_node, pname,
+ i * num_values + 1, &info->opp_sel);
+ of_property_read_u32_index(dev->of_node, pname,
+ i * num_values + 2, &efuse_offset);
+ of_property_read_u32_index(dev->of_node, pname,
+ i * num_values + 3, &rbb_mask);
+ of_property_read_u32_index(dev->of_node, pname,
+ i * num_values + 4, &fbb_mask);
+ of_property_read_u32_index(dev->of_node, pname,
+ i * num_values + 5, &vset_mask);
dev_dbg(dev,
"[%d]v=%d ABB=%d ef=0x%x rbb=0x%x fbb=0x%x vset=0x%x\n",
--
1.7.10.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
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 3/3] regulator: ti-abb-regulator: do not open-code counting and access of dt array elements
2014-02-12 0:01 ` [PATCH 3/3] regulator: ti-abb-regulator: " Heiko Stübner
@ 2014-02-12 15:50 ` Mark Brown
0 siblings, 0 replies; 8+ messages in thread
From: Mark Brown @ 2014-02-12 15:50 UTC (permalink / raw)
To: Heiko Stübner
Cc: grant.likely-QSEj5FYQhm4dnm+yROfE0A,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Liam Girdwood
[-- Attachment #1: Type: text/plain, Size: 512 bytes --]
On Wed, Feb 12, 2014 at 01:01:42AM +0100, Heiko Stübner wrote:
> From: Heiko Stuebner <heiko.stuebner-HCpLIkUQxWGakBO8gow8eQ@public.gmane.org>
>
> Open coding the counting of elements in a dt-property is abstracted by the newly
> introduced of_property_count_uXX_elems functions. Additionally the raw iteration
> over the states element exposes the endian conversion and dtb-format details,
> which according to Mark Rutland "would be nice to limit [...] to of_ helper
> functions".
Applied, thanks.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/3] of: functions to count number of elements and convert regulators
2014-02-11 23:59 [PATCH 0/3] of: functions to count number of elements and convert regulators Heiko Stübner
` (2 preceding siblings ...)
2014-02-12 0:01 ` [PATCH 3/3] regulator: ti-abb-regulator: " Heiko Stübner
@ 2014-02-12 9:25 ` Mark Rutland
3 siblings, 0 replies; 8+ messages in thread
From: Mark Rutland @ 2014-02-12 9:25 UTC (permalink / raw)
To: Heiko Stübner
Cc: grant.likely@linaro.org, robh+dt@kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
Liam Girdwood, Mark Brown
Hi Heiko,
On Tue, Feb 11, 2014 at 11:59:25PM +0000, Heiko Stübner wrote:
> In a different thread [0] Mark Rutland suggested that drivers should not
> repeatedly open-code the counting of array elements in a property as well
> as handling the format and endianes of the DTB, as these should be limited
> to the of_ helper functions.
>
> Therefore the first patch introduces a set of helper functions for counting
> the number of u8,...,u64 elements in a property.
>
> The second and third patch convert the two regulator drivers that use this
> pattern to instead use both of_property_count_u32_elemens as well as
> of_property_read_u32_index.
>
> gpio-regulator change tested on a s3c2416-based device, ti-abb-regulator
> compile-tested only.
Cheers for this. All the patches look fine to me, so for the series:
Acked-by: Mark Rutland <mark.rutland@arm.com>
Thanks,
Mark.
^ permalink raw reply [flat|nested] 8+ messages in thread