On Sat, May 26, 2012 at 8:01 AM, Grant Likely [off-list ref] wrote:

On Fri, 25 May 2012 21:36:16 +0800, Dong Aisheng [off-list ref] wrote:

quoted

From: Dong Aisheng <redacted>

Signed-off-by: Dong Aisheng <redacted>

Nack. ?Several problems here. ?First and foremost there isn't any
description of *why* this change is needed or when it was removed.

Sorry i sent it too fast to eager to see people's comments on the main
issue of patch 6.
It should have description on what you pointed out.
It's needed by the patch 6 in this series that it wants to find gpiochip from
node then calculate the gpio number from device tree in a standard way.

Any patch beyond the most trivial change needs to have a commit
message the describes it.

Second, of_node_to_gpiochip() is no longer a valid API because gpiolib
now allows multiple gpiochips to use the same device tree node. ?See
commit 3d0f7cf0f "gpio: Adjust of_xlate API to support multiple GPIO
chips" to see a description of why this was changed.

I noticed that of_node_to_gpiochip() is removed by that patch.
But i did not realized that this API was broken before.
After looking a bit more on the sample code of an banked gpio,
https://lkml.org/lkml/2012/5/18/51
it seems besides the gpio node, we also depend on controller specific
gpio .of_xlate function
to find the correct gpiochip in the same bank for the same node.
So it's correct that of_node_to_gpiochip() is broken now.

I may change it if we finally still find this API is needed.

Thanks for the review.

Regards
Dong Aisheng

quoted

---
?drivers/gpio/gpiolib-of.c | ? 11 +++++++++++
?include/linux/of_gpio.h ? | ? ?5 +++++
?2 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 8389d4a..b8010a9 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c

@@ -234,3 +234,14 @@ void of_gpiochip_remove(struct gpio_chip *chip)

? ? ? if (chip->of_node)
? ? ? ? ? ? ? of_node_put(chip->of_node);
?}
+
+/* Private function for resolving node pointer to gpio_chip */
+static int of_gpiochip_is_match(struct gpio_chip *chip, void *data)
+{
+ ? ? return chip->of_node == data;
+}
+
+struct gpio_chip *of_node_to_gpiochip(struct device_node *np)
+{
+ ? ? return gpiochip_find(np, of_gpiochip_is_match);
+}

diff --git a/include/linux/of_gpio.h b/include/linux/of_gpio.h
index c454f57..880783b 100644
--- a/include/linux/of_gpio.h
+++ b/include/linux/of_gpio.h

@@ -61,6 +61,7 @@ extern void of_gpiochip_remove(struct gpio_chip *gc);

?extern int of_gpio_simple_xlate(struct gpio_chip *gc,
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? const struct of_phandle_args *gpiospec,
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? u32 *flags);
+extern struct gpio_chip *of_node_to_gpiochip(struct device_node *np);

?#else /* CONFIG_OF_GPIO */

@@ -84,6 +85,10 @@ static inline int of_gpio_simple_xlate(struct gpio_chip *gc,

? ? ? return -ENOSYS;
?}

+static struct gpio_chip *of_node_to_gpiochip(struct device_node *np)
+{
+ ? ? return NULL;
+}
?static inline void of_gpiochip_add(struct gpio_chip *gc) { }
?static inline void of_gpiochip_remove(struct gpio_chip *gc) { }

--
1.7.0.4

--
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at ?http://vger.kernel.org/majordomo-info.html
Please read the FAQ at ?http://www.tux.org/lkml/