linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/5] of: Add helper for counting phandle refernces
@ 2013-02-12 23:06 Grant Likely
  2013-02-12 23:06 ` [PATCH v3 1/5] of/selftest: Fix GPIOs selftest to cover the 7th case Grant Likely
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Grant Likely @ 2013-02-12 23:06 UTC (permalink / raw)
  To: linux-kernel, devicetree-discuss

Well, this started as a single patch to get rid of horrible brute-force
counting of phandles and ended with fixes to the selftest code and
cleanups to the existing phandle parser before actually getting to the
new feature.

The selftest code is still a little fiddly to execute, but at least it
is consistent now. Sometime soon I'll link the selftest data directly
into the kernel so nothing special has to be done to run it, but I think
the DT grafting support needs to go in first.


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

* [PATCH v3 1/5] of/selftest: Fix GPIOs selftest to cover the 7th case
  2013-02-12 23:06 [PATCH v3 0/5] of: Add helper for counting phandle refernces Grant Likely
@ 2013-02-12 23:06 ` Grant Likely
  2013-02-12 23:06 ` [PATCH v3 2/5] of/selftest: Use selftest() macro throughout Grant Likely
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Grant Likely @ 2013-02-12 23:06 UTC (permalink / raw)
  To: linux-kernel, devicetree-discuss; +Cc: Grant Likely, Rob Herring

The of_gpio_named_count() self test doesn't hit the out-of-range
condition even though it is coded. Fix the bug by increasing the for
loop range by one.

Reported-by: Andreas Larsson <andreas@gaisler.com>
Cc: Rob Herring <rob.herring@calxeda.com>
Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---
 drivers/of/selftest.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/of/selftest.c b/drivers/of/selftest.c
index f24ffd7..9f62eb5 100644
--- a/drivers/of/selftest.c
+++ b/drivers/of/selftest.c
@@ -35,7 +35,7 @@ static void __init of_selftest_parse_phandle_with_args(void)
 		return;
 	}
 
-	for (i = 0; i < 7; i++) {
+	for (i = 0; i < 8; i++) {
 		bool passed = true;
 		rc = of_parse_phandle_with_args(np, "phandle-list",
 						"#phandle-cells", i, &args);
-- 
1.7.10.4


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

* [PATCH v3 2/5] of/selftest: Use selftest() macro throughout
  2013-02-12 23:06 [PATCH v3 0/5] of: Add helper for counting phandle refernces Grant Likely
  2013-02-12 23:06 ` [PATCH v3 1/5] of/selftest: Fix GPIOs selftest to cover the 7th case Grant Likely
@ 2013-02-12 23:06 ` Grant Likely
  2013-02-12 23:06 ` [PATCH v3 3/5] of/base: Clean up exit paths for of_parse_phandle_with_args() Grant Likely
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Grant Likely @ 2013-02-12 23:06 UTC (permalink / raw)
  To: linux-kernel, devicetree-discuss; +Cc: Grant Likely, Rob Herring

Some of the selftests are open-coded. Others use the selftest() macro
defined in drivers/of/selftest.c. The macro makes for cleaner selftest
code, so refactor the of_parse_phandle_with_args() tests to use it.

Cc: Rob Herring <rob.herring@calxeda.com>
Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---
 drivers/of/selftest.c |   37 ++++++++++++++-----------------------
 1 file changed, 14 insertions(+), 23 deletions(-)

diff --git a/drivers/of/selftest.c b/drivers/of/selftest.c
index 9f62eb5..01b4174 100644
--- a/drivers/of/selftest.c
+++ b/drivers/of/selftest.c
@@ -2,7 +2,7 @@
  * Self tests for device tree subsystem
  */
 
-#define pr_fmt(fmt) "### %s(): " fmt, __func__
+#define pr_fmt(fmt) "### dt-test ### " fmt
 
 #include <linux/clk.h>
 #include <linux/err.h>
@@ -16,19 +16,20 @@
 
 static bool selftest_passed = true;
 #define selftest(result, fmt, ...) { \
-	selftest_passed &= (result); \
-	if (!(result)) \
+	if (!(result)) { \
 		pr_err("FAIL %s:%i " fmt, __FILE__, __LINE__, ##__VA_ARGS__); \
+		selftest_passed = false; \
+	} else { \
+		pr_info("pass %s:%i\n", __FILE__, __LINE__); \
+	} \
 }
 
 static void __init of_selftest_parse_phandle_with_args(void)
 {
 	struct device_node *np;
 	struct of_phandle_args args;
-	int rc, i;
-	bool passed_all = true;
+	int i, rc;
 
-	pr_info("start\n");
 	np = of_find_node_by_path("/testcase-data/phandle-tests/consumer-a");
 	if (!np) {
 		pr_err("missing testcase data\n");
@@ -79,45 +80,35 @@ static void __init of_selftest_parse_phandle_with_args(void)
 			passed &= (args.args[0] == (i + 1));
 			break;
 		case 7:
-			passed &= (rc == -EINVAL);
+			passed &= (rc == -ENOENT);
 			break;
 		default:
 			passed = false;
 		}
 
-		if (!passed) {
-			int j;
-			pr_err("index %i - data error on node %s rc=%i regs=[",
-				i, args.np->full_name, rc);
-			for (j = 0; j < args.args_count; j++)
-				printk(" %i", args.args[j]);
-			printk(" ]\n");
-
-			passed_all = false;
-		}
+		selftest(passed, "index %i - data error on node %s rc=%i\n",
+			 i, args.np->full_name, rc);
 	}
 
 	/* Check for missing list property */
 	rc = of_parse_phandle_with_args(np, "phandle-list-missing",
 					"#phandle-cells", 0, &args);
-	passed_all &= (rc == -EINVAL);
+	selftest(rc == -ENOENT, "expected:%i got:%i\n", -ENOENT, rc);
 
 	/* Check for missing cells property */
 	rc = of_parse_phandle_with_args(np, "phandle-list",
 					"#phandle-cells-missing", 0, &args);
-	passed_all &= (rc == -EINVAL);
+	selftest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc);
 
 	/* Check for bad phandle in list */
 	rc = of_parse_phandle_with_args(np, "phandle-list-bad-phandle",
 					"#phandle-cells", 0, &args);
-	passed_all &= (rc == -EINVAL);
+	selftest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc);
 
 	/* Check for incorrectly formed argument list */
 	rc = of_parse_phandle_with_args(np, "phandle-list-bad-args",
 					"#phandle-cells", 1, &args);
-	passed_all &= (rc == -EINVAL);
-
-	pr_info("end - %s\n", passed_all ? "PASS" : "FAIL");
+	selftest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc);
 }
 
 static void __init of_selftest_property_match_string(void)
-- 
1.7.10.4


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

* [PATCH v3 3/5] of/base: Clean up exit paths for of_parse_phandle_with_args()
  2013-02-12 23:06 [PATCH v3 0/5] of: Add helper for counting phandle refernces Grant Likely
  2013-02-12 23:06 ` [PATCH v3 1/5] of/selftest: Fix GPIOs selftest to cover the 7th case Grant Likely
  2013-02-12 23:06 ` [PATCH v3 2/5] of/selftest: Use selftest() macro throughout Grant Likely
@ 2013-02-12 23:06 ` Grant Likely
  2013-02-12 23:06 ` [PATCH v3 4/5] of: Create function for counting number of phandles in a property Grant Likely
  2013-02-12 23:06 ` [PATCH v3 5/5] gpio: Make of_count_named_gpios() use new of_count_phandle_with_args() Grant Likely
  4 siblings, 0 replies; 8+ messages in thread
From: Grant Likely @ 2013-02-12 23:06 UTC (permalink / raw)
  To: linux-kernel, devicetree-discuss; +Cc: Grant Likely, Rob Herring

Some of the exit paths were not correctly releasing the node. Fix it by
creating an 'err' label for collecting the error paths and releasing the
node.

Cc: Rob Herring <rob.herring@calxeda.com>
Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---
 drivers/of/base.c |   26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index e2d44e0..c4538ab 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1087,7 +1087,7 @@ int of_parse_phandle_with_args(const struct device_node *np, const char *list_na
 				struct of_phandle_args *out_args)
 {
 	const __be32 *list, *list_end;
-	int size, cur_index = 0;
+	int rc = 0, size, cur_index = 0;
 	uint32_t count = 0;
 	struct device_node *node = NULL;
 	phandle phandle;
@@ -1100,6 +1100,7 @@ int of_parse_phandle_with_args(const struct device_node *np, const char *list_na
 
 	/* Loop over the phandles until all the requested entry is found */
 	while (list < list_end) {
+		rc = -EINVAL;
 		count = 0;
 
 		/*
@@ -1116,13 +1117,13 @@ int of_parse_phandle_with_args(const struct device_node *np, const char *list_na
 			if (!node) {
 				pr_err("%s: could not find phandle\n",
 					 np->full_name);
-				break;
+				goto err;
 			}
 			if (of_property_read_u32(node, cells_name, &count)) {
 				pr_err("%s: could not get %s for %s\n",
 					 np->full_name, cells_name,
 					 node->full_name);
-				break;
+				goto err;
 			}
 
 			/*
@@ -1132,7 +1133,7 @@ int of_parse_phandle_with_args(const struct device_node *np, const char *list_na
 			if (list + count > list_end) {
 				pr_err("%s: arguments longer than property\n",
 					 np->full_name);
-				break;
+				goto err;
 			}
 		}
 
@@ -1142,9 +1143,10 @@ int of_parse_phandle_with_args(const struct device_node *np, const char *list_na
 		 * index matches, then fill the out_args structure and return,
 		 * or return -ENOENT for an empty entry.
 		 */
+		rc = -ENOENT;
 		if (cur_index == index) {
 			if (!phandle)
-				return -ENOENT;
+				goto err;
 
 			if (out_args) {
 				int i;
@@ -1155,6 +1157,10 @@ int of_parse_phandle_with_args(const struct device_node *np, const char *list_na
 				for (i = 0; i < count; i++)
 					out_args->args[i] = be32_to_cpup(list++);
 			}
+
+			/* Found it! return success */
+			if (node)
+				of_node_put(node);
 			return 0;
 		}
 
@@ -1164,10 +1170,16 @@ int of_parse_phandle_with_args(const struct device_node *np, const char *list_na
 		cur_index++;
 	}
 
-	/* Loop exited without finding a valid entry; return an error */
+	/*
+	 * Unlock node before returning result; will be one of:
+	 * -ENOENT : index is for empty phandle
+	 * -EINVAL : parsing error on data
+	 */
+	rc = -ENOENT;
+ err:
 	if (node)
 		of_node_put(node);
-	return -EINVAL;
+	return rc;
 }
 EXPORT_SYMBOL(of_parse_phandle_with_args);
 
-- 
1.7.10.4


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

* [PATCH v3 4/5] of: Create function for counting number of phandles in a property
  2013-02-12 23:06 [PATCH v3 0/5] of: Add helper for counting phandle refernces Grant Likely
                   ` (2 preceding siblings ...)
  2013-02-12 23:06 ` [PATCH v3 3/5] of/base: Clean up exit paths for of_parse_phandle_with_args() Grant Likely
@ 2013-02-12 23:06 ` Grant Likely
  2013-02-13  9:57   ` Andreas Larsson
  2013-02-12 23:06 ` [PATCH v3 5/5] gpio: Make of_count_named_gpios() use new of_count_phandle_with_args() Grant Likely
  4 siblings, 1 reply; 8+ messages in thread
From: Grant Likely @ 2013-02-12 23:06 UTC (permalink / raw)
  To: linux-kernel, devicetree-discuss
  Cc: Grant Likely, Linus Walleij, Rob Herring, Andreas Larsson

This patch creates of_count_phandle_with_args(), a new function for
counting the number of phandle+argument tuples in a given property. This
is better than the existing method of parsing each phandle individually
until parsing fails which is a horribly slow way to do the count.

Tested on ARM using the selftest code.

v3: - Rebased on top of selftest code cleanup patch
v2: - fix bug where of_parse_phandle_with_args() could behave like _count_.
    - made of_gpio_named_count() into a static inline regardless of CONFIG_OF_GPIO

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Andreas Larsson <andreas@gaisler.com>
---
 drivers/of/base.c     |   41 +++++++++++++++++++++++++++++++++++++----
 drivers/of/selftest.c |   15 +++++++++++++++
 include/linux/of.h    |    9 +++++++++
 3 files changed, 61 insertions(+), 4 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index c4538ab..f21794c 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1082,9 +1082,10 @@ EXPORT_SYMBOL(of_parse_phandle);
  * To get a device_node of the `node2' node you may call this:
  * of_parse_phandle_with_args(node3, "list", "#list-cells", 1, &args);
  */
-int of_parse_phandle_with_args(const struct device_node *np, const char *list_name,
-				const char *cells_name, int index,
-				struct of_phandle_args *out_args)
+static int __of_parse_phandle_with_args(const struct device_node *np,
+					const char *list_name,
+					const char *cells_name, int index,
+					struct of_phandle_args *out_args)
 {
 	const __be32 *list, *list_end;
 	int rc = 0, size, cur_index = 0;
@@ -1174,15 +1175,47 @@ int of_parse_phandle_with_args(const struct device_node *np, const char *list_na
 	 * Unlock node before returning result; will be one of:
 	 * -ENOENT : index is for empty phandle
 	 * -EINVAL : parsing error on data
+	 * [1..n]  : Number of phandle (count mode; when index = -1)
 	 */
-	rc = -ENOENT;
+	rc = index < 0 ? cur_index : -ENOENT;
  err:
 	if (node)
 		of_node_put(node);
 	return rc;
 }
+
+int of_parse_phandle_with_args(const struct device_node *np, const char *list_name,
+				const char *cells_name, int index,
+				struct of_phandle_args *out_args)
+{
+	if (index < 0)
+		return -EINVAL;
+	return __of_parse_phandle_with_args(np, list_name, cells_name, index, out_args);
+}
 EXPORT_SYMBOL(of_parse_phandle_with_args);
 
+/**
+ * of_count_phandle_with_args() - Find the number of phandles references in a property
+ * @np:		pointer to a device tree node containing a list
+ * @list_name:	property name that contains a list
+ * @cells_name:	property name that specifies phandles' arguments count
+ *
+ * Returns the number of phandle + argument tuples within a property. It
+ * is a typical pattern to encode a list of phandle and variable
+ * arguments into a single property. The number of arguments is encoded
+ * by a property in the phandle-target node. For example, a gpios
+ * property would contain a list of GPIO specifies consisting of a
+ * phandle and 1 or more arguments. The number of arguments are
+ * determined by the #gpio-cells property in the node pointed to by the
+ * phandle.
+ */
+int of_count_phandle_with_args(const struct device_node *np, const char *list_name,
+				const char *cells_name)
+{
+	return __of_parse_phandle_with_args(np, list_name, cells_name, -1, NULL);
+}
+EXPORT_SYMBOL(of_count_phandle_with_args);
+
 #if defined(CONFIG_OF_DYNAMIC)
 static int of_property_notify(int action, struct device_node *np,
 			      struct property *prop)
diff --git a/drivers/of/selftest.c b/drivers/of/selftest.c
index 01b4174..0eb5c38 100644
--- a/drivers/of/selftest.c
+++ b/drivers/of/selftest.c
@@ -36,6 +36,9 @@ static void __init of_selftest_parse_phandle_with_args(void)
 		return;
 	}
 
+	rc = of_count_phandle_with_args(np, "phandle-list", "#phandle-cells");
+	selftest(rc == 7, "of_count_phandle_with_args() returned %i, expected 7\n", rc);
+
 	for (i = 0; i < 8; i++) {
 		bool passed = true;
 		rc = of_parse_phandle_with_args(np, "phandle-list",
@@ -94,21 +97,33 @@ static void __init of_selftest_parse_phandle_with_args(void)
 	rc = of_parse_phandle_with_args(np, "phandle-list-missing",
 					"#phandle-cells", 0, &args);
 	selftest(rc == -ENOENT, "expected:%i got:%i\n", -ENOENT, rc);
+	rc = of_count_phandle_with_args(np, "phandle-list-missing",
+					"#phandle-cells");
+	selftest(rc == -ENOENT, "expected:%i got:%i\n", -ENOENT, rc);
 
 	/* Check for missing cells property */
 	rc = of_parse_phandle_with_args(np, "phandle-list",
 					"#phandle-cells-missing", 0, &args);
 	selftest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc);
+	rc = of_count_phandle_with_args(np, "phandle-list",
+					"#phandle-cells-missing");
+	selftest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc);
 
 	/* Check for bad phandle in list */
 	rc = of_parse_phandle_with_args(np, "phandle-list-bad-phandle",
 					"#phandle-cells", 0, &args);
 	selftest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc);
+	rc = of_count_phandle_with_args(np, "phandle-list-bad-phandle",
+					"#phandle-cells");
+	selftest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc);
 
 	/* Check for incorrectly formed argument list */
 	rc = of_parse_phandle_with_args(np, "phandle-list-bad-args",
 					"#phandle-cells", 1, &args);
 	selftest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc);
+	rc = of_count_phandle_with_args(np, "phandle-list-bad-args",
+					"#phandle-cells");
+	selftest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc);
 }
 
 static void __init of_selftest_property_match_string(void)
diff --git a/include/linux/of.h b/include/linux/of.h
index b9e1b91..a0f1292 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -277,6 +277,8 @@ extern struct device_node *of_parse_phandle(const struct device_node *np,
 extern int of_parse_phandle_with_args(const struct device_node *np,
 	const char *list_name, const char *cells_name, int index,
 	struct of_phandle_args *out_args);
+extern int of_count_phandle_with_args(const struct device_node *np,
+	const char *list_name, const char *cells_name);
 
 extern void of_alias_scan(void * (*dt_alloc)(u64 size, u64 align));
 extern int of_alias_get_id(struct device_node *np, const char *stem);
@@ -467,6 +469,13 @@ static inline int of_parse_phandle_with_args(struct device_node *np,
 	return -ENOSYS;
 }
 
+static inline int of_count_phandle_with_args(struct device_node *np,
+					     const char *list_name,
+					     const char *cells_name)
+{
+	return -ENOSYS;
+}
+
 static inline int of_alias_get_id(struct device_node *np, const char *stem)
 {
 	return -ENOSYS;
-- 
1.7.10.4


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

* [PATCH v3 5/5] gpio: Make of_count_named_gpios() use new of_count_phandle_with_args()
  2013-02-12 23:06 [PATCH v3 0/5] of: Add helper for counting phandle refernces Grant Likely
                   ` (3 preceding siblings ...)
  2013-02-12 23:06 ` [PATCH v3 4/5] of: Create function for counting number of phandles in a property Grant Likely
@ 2013-02-12 23:06 ` Grant Likely
  2013-02-13 10:00   ` Andreas Larsson
  4 siblings, 1 reply; 8+ messages in thread
From: Grant Likely @ 2013-02-12 23:06 UTC (permalink / raw)
  To: linux-kernel, devicetree-discuss
  Cc: Grant Likely, Linus Walleij, Rob Herring, Andreas Larsson

This patch replaces the horribly coded of_count_named_gpios() with a
call to of_count_phandle_with_args() which is far more efficient. This
also changes the return value of of_gpio_count() & of_gpio_named_count()
from 'unsigned int' to 'int' so that it can return an error code. All
the users of that function are fixed up to correctly handle a negative
return value.

v2: Split GPIO portion into a separate patch

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Andreas Larsson <andreas@gaisler.com>
---
 drivers/gpio/gpiolib-of.c              |   35 ----------------------------
 drivers/hwmon/gpio-fan.c               |    4 ++--
 drivers/input/keyboard/matrix_keypad.c |    8 +++----
 drivers/net/phy/mdio-mux-gpio.c        |    4 ++--
 drivers/spi/spi-fsl-spi.c              |    4 ++--
 drivers/spi/spi-oc-tiny.c              |    8 +++----
 drivers/spi/spi-ppc4xx.c               |    4 ++--
 drivers/spi/spi.c                      |    5 ++--
 include/linux/of_gpio.h                |   40 ++++++++++++++++++--------------
 9 files changed, 41 insertions(+), 71 deletions(-)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index d542a14..dd8a212 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -89,41 +89,6 @@ int of_get_named_gpio_flags(struct device_node *np, const char *propname,
 EXPORT_SYMBOL(of_get_named_gpio_flags);
 
 /**
- * of_gpio_named_count - Count GPIOs for a device
- * @np:		device node to count GPIOs for
- * @propname:	property name containing gpio specifier(s)
- *
- * The function returns the count of GPIOs specified for a node.
- *
- * Note that the empty GPIO specifiers counts too. For example,
- *
- * gpios = <0
- *          &pio1 1 2
- *          0
- *          &pio2 3 4>;
- *
- * defines four GPIOs (so this function will return 4), two of which
- * are not specified.
- */
-unsigned int of_gpio_named_count(struct device_node *np, const char* propname)
-{
-	unsigned int cnt = 0;
-
-	do {
-		int ret;
-
-		ret = of_parse_phandle_with_args(np, propname, "#gpio-cells",
-						 cnt, NULL);
-		/* A hole in the gpios = <> counts anyway. */
-		if (ret < 0 && ret != -EEXIST)
-			break;
-	} while (++cnt);
-
-	return cnt;
-}
-EXPORT_SYMBOL(of_gpio_named_count);
-
-/**
  * of_gpio_simple_xlate - translate gpio_spec to the GPIO number and flags
  * @gc:		pointer to the gpio_chip structure
  * @np:		device node of the GPIO chip
diff --git a/drivers/hwmon/gpio-fan.c b/drivers/hwmon/gpio-fan.c
index 4e04c12..3978194 100644
--- a/drivers/hwmon/gpio-fan.c
+++ b/drivers/hwmon/gpio-fan.c
@@ -422,7 +422,7 @@ static int gpio_fan_get_of_pdata(struct device *dev,
 
 	/* Fill GPIO pin array */
 	pdata->num_ctrl = of_gpio_count(node);
-	if (!pdata->num_ctrl) {
+	if (pdata->num_ctrl <= 0) {
 		dev_err(dev, "gpios DT property empty / missing");
 		return -ENODEV;
 	}
@@ -477,7 +477,7 @@ static int gpio_fan_get_of_pdata(struct device *dev,
 	pdata->speed = speed;
 
 	/* Alarm GPIO if one exists */
-	if (of_gpio_named_count(node, "alarm-gpios")) {
+	if (of_gpio_named_count(node, "alarm-gpios") > 0) {
 		struct gpio_fan_alarm *alarm;
 		int val;
 		enum of_gpio_flags flags;
diff --git a/drivers/input/keyboard/matrix_keypad.c b/drivers/input/keyboard/matrix_keypad.c
index f4ff0dd..71d7719 100644
--- a/drivers/input/keyboard/matrix_keypad.c
+++ b/drivers/input/keyboard/matrix_keypad.c
@@ -403,7 +403,7 @@ matrix_keypad_parse_dt(struct device *dev)
 	struct matrix_keypad_platform_data *pdata;
 	struct device_node *np = dev->of_node;
 	unsigned int *gpios;
-	int i;
+	int i, nrow, ncol;
 
 	if (!np) {
 		dev_err(dev, "device lacks DT data\n");
@@ -416,9 +416,9 @@ matrix_keypad_parse_dt(struct device *dev)
 		return ERR_PTR(-ENOMEM);
 	}
 
-	pdata->num_row_gpios = of_gpio_named_count(np, "row-gpios");
-	pdata->num_col_gpios = of_gpio_named_count(np, "col-gpios");
-	if (!pdata->num_row_gpios || !pdata->num_col_gpios) {
+	pdata->num_row_gpios = nrow = of_gpio_named_count(np, "row-gpios");
+	pdata->num_col_gpios = ncol = of_gpio_named_count(np, "col-gpios");
+	if (nrow <= 0 || ncol <= 0) {
 		dev_err(dev, "number of keypad rows/columns not specified\n");
 		return ERR_PTR(-EINVAL);
 	}
diff --git a/drivers/net/phy/mdio-mux-gpio.c b/drivers/net/phy/mdio-mux-gpio.c
index 0c9accb..e91d7d7 100644
--- a/drivers/net/phy/mdio-mux-gpio.c
+++ b/drivers/net/phy/mdio-mux-gpio.c
@@ -53,7 +53,7 @@ static int mdio_mux_gpio_probe(struct platform_device *pdev)
 {
 	enum of_gpio_flags f;
 	struct mdio_mux_gpio_state *s;
-	unsigned int num_gpios;
+	int num_gpios;
 	unsigned int n;
 	int r;
 
@@ -61,7 +61,7 @@ static int mdio_mux_gpio_probe(struct platform_device *pdev)
 		return -ENODEV;
 
 	num_gpios = of_gpio_count(pdev->dev.of_node);
-	if (num_gpios == 0 || num_gpios > MDIO_MUX_GPIO_MAX_BITS)
+	if (num_gpios <= 0 || num_gpios > MDIO_MUX_GPIO_MAX_BITS)
 		return -ENODEV;
 
 	s = devm_kzalloc(&pdev->dev, sizeof(*s), GFP_KERNEL);
diff --git a/drivers/spi/spi-fsl-spi.c b/drivers/spi/spi-fsl-spi.c
index 1a7f635..086a9ee 100644
--- a/drivers/spi/spi-fsl-spi.c
+++ b/drivers/spi/spi-fsl-spi.c
@@ -947,12 +947,12 @@ static int of_fsl_spi_get_chipselects(struct device *dev)
 	struct device_node *np = dev->of_node;
 	struct fsl_spi_platform_data *pdata = dev->platform_data;
 	struct mpc8xxx_spi_probe_info *pinfo = to_of_pinfo(pdata);
-	unsigned int ngpios;
+	int ngpios;
 	int i = 0;
 	int ret;
 
 	ngpios = of_gpio_count(np);
-	if (!ngpios) {
+	if (ngpios <= 0) {
 		/*
 		 * SPI w/o chip-select line. One SPI device is still permitted
 		 * though.
diff --git a/drivers/spi/spi-oc-tiny.c b/drivers/spi/spi-oc-tiny.c
index 432e66e..cb2e284 100644
--- a/drivers/spi/spi-oc-tiny.c
+++ b/drivers/spi/spi-oc-tiny.c
@@ -54,7 +54,7 @@ struct tiny_spi {
 	unsigned int txc, rxc;
 	const u8 *txp;
 	u8 *rxp;
-	unsigned int gpio_cs_count;
+	int gpio_cs_count;
 	int *gpio_cs;
 };
 
@@ -74,7 +74,7 @@ static void tiny_spi_chipselect(struct spi_device *spi, int is_active)
 {
 	struct tiny_spi *hw = tiny_spi_to_hw(spi);
 
-	if (hw->gpio_cs_count) {
+	if (hw->gpio_cs_count > 0) {
 		gpio_set_value(hw->gpio_cs[spi->chip_select],
 			(spi->mode & SPI_CS_HIGH) ? is_active : !is_active);
 	}
@@ -254,7 +254,7 @@ static int tiny_spi_of_probe(struct platform_device *pdev)
 	if (!np)
 		return 0;
 	hw->gpio_cs_count = of_gpio_count(np);
-	if (hw->gpio_cs_count) {
+	if (hw->gpio_cs_count > 0) {
 		hw->gpio_cs = devm_kzalloc(&pdev->dev,
 				hw->gpio_cs_count * sizeof(unsigned int),
 				GFP_KERNEL);
@@ -352,7 +352,7 @@ static int tiny_spi_probe(struct platform_device *pdev)
 			goto exit_gpio;
 		gpio_direction_output(hw->gpio_cs[i], 1);
 	}
-	hw->bitbang.master->num_chipselect = max(1U, hw->gpio_cs_count);
+	hw->bitbang.master->num_chipselect = max(1, hw->gpio_cs_count);
 
 	/* register our spi controller */
 	err = spi_bitbang_start(&hw->bitbang);
diff --git a/drivers/spi/spi-ppc4xx.c b/drivers/spi/spi-ppc4xx.c
index 7a85f22..af3e6e7 100644
--- a/drivers/spi/spi-ppc4xx.c
+++ b/drivers/spi/spi-ppc4xx.c
@@ -419,7 +419,7 @@ static int __init spi_ppc4xx_of_probe(struct platform_device *op)
 	 * This includes both "null" gpio's and real ones.
 	 */
 	num_gpios = of_gpio_count(np);
-	if (num_gpios) {
+	if (num_gpios > 0) {
 		int i;
 
 		hw->gpios = kzalloc(sizeof(int) * num_gpios, GFP_KERNEL);
@@ -471,7 +471,7 @@ static int __init spi_ppc4xx_of_probe(struct platform_device *op)
 		SPI_CPHA | SPI_CPOL | SPI_CS_HIGH | SPI_LSB_FIRST;
 
 	/* this many pins in all GPIO controllers */
-	bbp->master->num_chipselect = num_gpios;
+	bbp->master->num_chipselect = num_gpios > 0 ? num_gpios : 0;
 
 	/* Get the clock for the OPB */
 	opbnp = of_find_compatible_node(NULL, NULL, "ibm,opb");
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 19ee901..21c4748 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1059,15 +1059,14 @@ EXPORT_SYMBOL_GPL(spi_alloc_master);
 #ifdef CONFIG_OF
 static int of_spi_register_master(struct spi_master *master)
 {
-	u16 nb;
-	int i, *cs;
+	int nb, i, *cs;
 	struct device_node *np = master->dev.of_node;
 
 	if (!np)
 		return 0;
 
 	nb = of_gpio_named_count(np, "cs-gpios");
-	master->num_chipselect = max(nb, master->num_chipselect);
+	master->num_chipselect = max(nb, (int)master->num_chipselect);
 
 	if (nb < 1)
 		return 0;
diff --git a/include/linux/of_gpio.h b/include/linux/of_gpio.h
index c454f57..a83dc6f 100644
--- a/include/linux/of_gpio.h
+++ b/include/linux/of_gpio.h
@@ -50,9 +50,6 @@ static inline struct of_mm_gpio_chip *to_of_mm_gpio_chip(struct gpio_chip *gc)
 extern int of_get_named_gpio_flags(struct device_node *np,
 		const char *list_name, int index, enum of_gpio_flags *flags);
 
-extern unsigned int of_gpio_named_count(struct device_node *np,
-					const char* propname);
-
 extern int of_mm_gpiochip_add(struct device_node *np,
 			      struct of_mm_gpio_chip *mm_gc);
 
@@ -71,12 +68,6 @@ static inline int of_get_named_gpio_flags(struct device_node *np,
 	return -ENOSYS;
 }
 
-static inline unsigned int of_gpio_named_count(struct device_node *np,
-					const char* propname)
-{
-	return 0;
-}
-
 static inline int of_gpio_simple_xlate(struct gpio_chip *gc,
 				       const struct of_phandle_args *gpiospec,
 				       u32 *flags)
@@ -90,22 +81,37 @@ static inline void of_gpiochip_remove(struct gpio_chip *gc) { }
 #endif /* CONFIG_OF_GPIO */
 
 /**
- * of_gpio_count - Count GPIOs for a device
+ * of_gpio_named_count() - Count GPIOs for a device
  * @np:		device node to count GPIOs for
+ * @propname:	property name containing gpio specifier(s)
  *
  * The function returns the count of GPIOs specified for a node.
+ * Note that the empty GPIO specifiers count too. Returns either
+ *   Number of gpios defined in property,
+ *   -EINVAL for an incorrectly formed gpios property, or
+ *   -ENOENT for a missing gpios property
  *
- * Note that the empty GPIO specifiers counts too. For example,
- *
+ * Example:
  * gpios = <0
- *          &pio1 1 2
+ *          &gpio1 1 2
  *          0
- *          &pio2 3 4>;
+ *          &gpio2 3 4>;
+ *
+ * The above example defines four GPIOs, two of which are not specified.
+ * This function will return '4'
+ */
+static inline int of_gpio_named_count(struct device_node *np, const char* propname)
+{
+	return of_count_phandle_with_args(np, propname, "#gpio-cells");
+}
+
+/**
+ * of_gpio_count() - Count GPIOs for a device
+ * @np:		device node to count GPIOs for
  *
- * defines four GPIOs (so this function will return 4), two of which
- * are not specified.
+ * Same as of_gpio_named_count, but hard coded to use the 'gpios' property
  */
-static inline unsigned int of_gpio_count(struct device_node *np)
+static inline int of_gpio_count(struct device_node *np)
 {
 	return of_gpio_named_count(np, "gpios");
 }
-- 
1.7.10.4


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

* Re: [PATCH v3 4/5] of: Create function for counting number of phandles in a property
  2013-02-12 23:06 ` [PATCH v3 4/5] of: Create function for counting number of phandles in a property Grant Likely
@ 2013-02-13  9:57   ` Andreas Larsson
  0 siblings, 0 replies; 8+ messages in thread
From: Andreas Larsson @ 2013-02-13  9:57 UTC (permalink / raw)
  To: Grant Likely; +Cc: linux-kernel, devicetree-discuss, Linus Walleij, Rob Herring

On 2013-02-13 00:06, Grant Likely wrote:
> This patch creates of_count_phandle_with_args(), a new function for
> counting the number of phandle+argument tuples in a given property. This
> is better than the existing method of parsing each phandle individually
> until parsing fails which is a horribly slow way to do the count.
>
> Tested on ARM using the selftest code.
>
> v3: - Rebased on top of selftest code cleanup patch
> v2: - fix bug where of_parse_phandle_with_args() could behave like _count_.
>      - made of_gpio_named_count() into a static inline regardless of CONFIG_OF_GPIO
>
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: Andreas Larsson <andreas@gaisler.com>

Tested-by: Andreas Larsson <andreas@gaisler.com>

Cheers,
Andreas Larsson


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

* Re: [PATCH v3 5/5] gpio: Make of_count_named_gpios() use new of_count_phandle_with_args()
  2013-02-12 23:06 ` [PATCH v3 5/5] gpio: Make of_count_named_gpios() use new of_count_phandle_with_args() Grant Likely
@ 2013-02-13 10:00   ` Andreas Larsson
  0 siblings, 0 replies; 8+ messages in thread
From: Andreas Larsson @ 2013-02-13 10:00 UTC (permalink / raw)
  To: Grant Likely; +Cc: linux-kernel, devicetree-discuss, Linus Walleij, Rob Herring

On 2013-02-13 00:06, Grant Likely wrote:
> This patch replaces the horribly coded of_count_named_gpios() with a
> call to of_count_phandle_with_args() which is far more efficient. This
> also changes the return value of of_gpio_count() & of_gpio_named_count()
> from 'unsigned int' to 'int' so that it can return an error code. All
> the users of that function are fixed up to correctly handle a negative
> return value.
>
> v2: Split GPIO portion into a separate patch
>
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: Andreas Larsson <andreas@gaisler.com>

For gpiolib-of.c, of_gpio.h and spi.c:
Tested-by: Andreas Larsson <andreas@gaisler.com>

Cheers,
Andreas Larsson


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

end of thread, other threads:[~2013-02-13 10:00 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-12 23:06 [PATCH v3 0/5] of: Add helper for counting phandle refernces Grant Likely
2013-02-12 23:06 ` [PATCH v3 1/5] of/selftest: Fix GPIOs selftest to cover the 7th case Grant Likely
2013-02-12 23:06 ` [PATCH v3 2/5] of/selftest: Use selftest() macro throughout Grant Likely
2013-02-12 23:06 ` [PATCH v3 3/5] of/base: Clean up exit paths for of_parse_phandle_with_args() Grant Likely
2013-02-12 23:06 ` [PATCH v3 4/5] of: Create function for counting number of phandles in a property Grant Likely
2013-02-13  9:57   ` Andreas Larsson
2013-02-12 23:06 ` [PATCH v3 5/5] gpio: Make of_count_named_gpios() use new of_count_phandle_with_args() Grant Likely
2013-02-13 10:00   ` Andreas Larsson

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