Skip to contentSkip to conversation

of: functions to count number of elements and convert regulators

PATCH Applied Heiko Stübner 3 participants 8 messages Feb 11, 2014 – Feb 12, 2014

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover of: functions to count number of elements and convert regulators

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.

[0] https://lkml.org/lkml/2014/1/16/172

Heiko Stuebner (3):
of: add functions to count number of elements in a property
regulator: gpio-regulator: do not open-code counting and access of dt array elements
regulator: ti-abb-regulator: do not open-code counting and access of dt array elements

 drivers/of/base.c                    |   32 ++++++++++++++
 drivers/regulator/gpio-regulator.c   |   15 +++----
 drivers/regulator/ti-abb-regulator.c |   43 +++++++++----------
 include/linux/of.h                   |   76 ++++++++++++++++++++++++++++++++++
 4 files changed, 134 insertions(+), 32 deletions(-)
Signature · 1 line
1.7.10.4
#1 lore raw

PATCH 1/3 of: add functions to count number of elements in a property

From: Heiko Stuebner <[email protected]>

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.

---
 drivers/of/base.c  |   32 ++++++++++++++++++++++
 include/linux/of.h |   76 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 108 insertions(+)
drivers/of/base.c +32 -0
index 239e3da..bed63c2 100644
@@ -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.
include/linux/of.h +76 -0

94 lines not on this page · raw message

Signature · 7 lines
1.7.10.4


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
maintainer Applied
#2 lore raw

Re: [PATCH 1/3] of: add functions to count number of elements in a property

Quoted 7 lines · Heiko Stübner

From: Heiko Stuebner <[email protected]>

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.

#3 lore raw

PATCH 2/3 regulator: gpio-regulator: do not open-code counting and access of dt array elements

In reply to Heiko Stübner

From: Heiko Stuebner <[email protected]>

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.

---
 drivers/regulator/gpio-regulator.c |   15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)
drivers/regulator/gpio-regulator.c +6 -9
index ac3a8c7..500aa3b 100644
@@ -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;
 
Signature · 7 lines
1.7.10.4


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
maintainer Applied
#4 lore raw

Re: [PATCH 2/3] regulator: gpio-regulator: do not open-code counting and access of dt array elements

Quoted 4 lines · Heiko Stübner

From: Heiko Stuebner <[email protected]>

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.

#5 lore raw

PATCH 3/3 regulator: ti-abb-regulator: do not open-code counting and access of dt array elements

In reply to Heiko Stübner

From: Heiko Stuebner <[email protected]>

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.

---
 drivers/regulator/ti-abb-regulator.c |   43 ++++++++++++++++------------------
 1 file changed, 20 insertions(+), 23 deletions(-)
drivers/regulator/ti-abb-regulator.c +20 -23
index a97d0c5..804c83a 100644
@@ -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",
Signature · 7 lines
1.7.10.4


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
maintainer Applied
#6 lore raw

Re: [PATCH 3/3] regulator: ti-abb-regulator: do not open-code counting and access of dt array elements

Quoted 7 lines · Heiko Stübner

From: Heiko Stuebner <[email protected]>

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.

Acked-by on the v1 series
#7 lore raw
In reply to Heiko Stübner

Hi Heiko,

Quoted 8 lines · Heiko Stübner

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.

Quoted 6 lines

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:

Thanks,
Mark.