diff mbox

[v2,2/4] gpio: add parameter to allow the use named gpios

Message ID 1421876028-22799-3-git-send-email-o.schinagl@ultimaker.com
State New
Headers show

Commit Message

Olliver Schinagl Jan. 21, 2015, 9:33 p.m. UTC 
From: Olliver Schinagl <oliver@schinagl.nl>

The gpio binding document says that new code should always use named
gpios. Patch 40b73183 added support to parse a list of gpios from child
nodes, but does not make it possible to use named gpios. This patch adds
the con_id property and implements it is done in gpiolib.c, where the
old-style of using unnamed gpios still works.

Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
---
 drivers/gpio/devres.c                     | 18 +++++++++++++++++-
 drivers/input/keyboard/gpio_keys_polled.c |  2 +-
 drivers/leds/leds-gpio.c                  |  2 +-
 include/linux/gpio/consumer.h             |  1 +
 4 files changed, 20 insertions(+), 3 deletions(-)

Comments

Linus Walleij Jan. 30, 2015, 1:46 p.m. UTC | #1 
On Wed, Jan 21, 2015 at 10:33 PM, Olliver Schinagl
<o.schinagl@ultimaker.com> wrote:

> From: Olliver Schinagl <oliver@schinagl.nl>
>
> The gpio binding document says that new code should always use named
> gpios. Patch 40b73183 added support to parse a list of gpios from child
> nodes, but does not make it possible to use named gpios. This patch adds
> the con_id property and implements it is done in gpiolib.c, where the
> old-style of using unnamed gpios still works.
>
> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
> ---
>  drivers/gpio/devres.c                     | 18 +++++++++++++++++-
>  drivers/input/keyboard/gpio_keys_polled.c |  2 +-
>  drivers/leds/leds-gpio.c                  |  2 +-
>  include/linux/gpio/consumer.h             |  1 +

Alexandre: does this match your vision of how it should work, i.e. ACK?

Bryan/Dmitry: can you ACK the oneliners in your subsystems?

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bryan Wu Jan. 30, 2015, 7:12 p.m. UTC | #2 
On Fri, Jan 30, 2015 at 5:46 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Wed, Jan 21, 2015 at 10:33 PM, Olliver Schinagl
> <o.schinagl@ultimaker.com> wrote:
>
>> From: Olliver Schinagl <oliver@schinagl.nl>
>>
>> The gpio binding document says that new code should always use named
>> gpios. Patch 40b73183 added support to parse a list of gpios from child
>> nodes, but does not make it possible to use named gpios. This patch adds
>> the con_id property and implements it is done in gpiolib.c, where the
>> old-style of using unnamed gpios still works.
>>
>> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
>> ---
>>  drivers/gpio/devres.c                     | 18 +++++++++++++++++-
>>  drivers/input/keyboard/gpio_keys_polled.c |  2 +-
>>  drivers/leds/leds-gpio.c                  |  2 +-
>>  include/linux/gpio/consumer.h             |  1 +
>
> Alexandre: does this match your vision of how it should work, i.e. ACK?
>
> Bryan/Dmitry: can you ACK the oneliners in your subsystems?

Sure, please take my Ack
Acked-by: Bryan Wu <cooloney@gmail.com>

>
> Yours,
> Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dmitry Torokhov Jan. 30, 2015, 10:16 p.m. UTC | #3 
On Fri, Jan 30, 2015 at 11:12:53AM -0800, Bryan Wu wrote:
> On Fri, Jan 30, 2015 at 5:46 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> > On Wed, Jan 21, 2015 at 10:33 PM, Olliver Schinagl
> > <o.schinagl@ultimaker.com> wrote:
> >
> >> From: Olliver Schinagl <oliver@schinagl.nl>
> >>
> >> The gpio binding document says that new code should always use named
> >> gpios. Patch 40b73183 added support to parse a list of gpios from child
> >> nodes, but does not make it possible to use named gpios. This patch adds
> >> the con_id property and implements it is done in gpiolib.c, where the
> >> old-style of using unnamed gpios still works.
> >>
> >> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
> >> ---
> >>  drivers/gpio/devres.c                     | 18 +++++++++++++++++-
> >>  drivers/input/keyboard/gpio_keys_polled.c |  2 +-
> >>  drivers/leds/leds-gpio.c                  |  2 +-
> >>  include/linux/gpio/consumer.h             |  1 +
> >
> > Alexandre: does this match your vision of how it should work, i.e. ACK?
> >
> > Bryan/Dmitry: can you ACK the oneliners in your subsystems?
> 
> Sure, please take my Ack
> Acked-by: Bryan Wu <cooloney@gmail.com>

Mine as well:

Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Thanks.
Dmitry Torokhov Jan. 30, 2015, 10:22 p.m. UTC | #4 
On Fri, Jan 30, 2015 at 02:16:00PM -0800, Dmitry Torokhov wrote:
> On Fri, Jan 30, 2015 at 11:12:53AM -0800, Bryan Wu wrote:
> > On Fri, Jan 30, 2015 at 5:46 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> > > On Wed, Jan 21, 2015 at 10:33 PM, Olliver Schinagl
> > > <o.schinagl@ultimaker.com> wrote:
> > >
> > >> From: Olliver Schinagl <oliver@schinagl.nl>
> > >>
> > >> The gpio binding document says that new code should always use named
> > >> gpios. Patch 40b73183 added support to parse a list of gpios from child
> > >> nodes, but does not make it possible to use named gpios. This patch adds
> > >> the con_id property and implements it is done in gpiolib.c, where the
> > >> old-style of using unnamed gpios still works.
> > >>
> > >> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
> > >> ---
> > >>  drivers/gpio/devres.c                     | 18 +++++++++++++++++-
> > >>  drivers/input/keyboard/gpio_keys_polled.c |  2 +-
> > >>  drivers/leds/leds-gpio.c                  |  2 +-
> > >>  include/linux/gpio/consumer.h             |  1 +
> > >
> > > Alexandre: does this match your vision of how it should work, i.e. ACK?
> > >
> > > Bryan/Dmitry: can you ACK the oneliners in your subsystems?
> > 
> > Sure, please take my Ack
> > Acked-by: Bryan Wu <cooloney@gmail.com>
> 
> Mine as well:
> 
> Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Forgot to mention: the ack is for this patch only; the patch #4 is
NAKed because:

1. The logic of handling old and new name AFAICS is broken and
2. gpio_keys_polled-gpios as name is plain ugly.

Thanks.
Alexandre Courbot Feb. 9, 2015, 5:24 a.m. UTC | #5 
On Fri, Jan 30, 2015 at 10:46 PM, Linus Walleij
<linus.walleij@linaro.org> wrote:
> On Wed, Jan 21, 2015 at 10:33 PM, Olliver Schinagl
> <o.schinagl@ultimaker.com> wrote:
>
>> From: Olliver Schinagl <oliver@schinagl.nl>
>>
>> The gpio binding document says that new code should always use named
>> gpios. Patch 40b73183 added support to parse a list of gpios from child
>> nodes, but does not make it possible to use named gpios. This patch adds
>> the con_id property and implements it is done in gpiolib.c, where the
>> old-style of using unnamed gpios still works.
>>
>> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
>> ---
>>  drivers/gpio/devres.c                     | 18 +++++++++++++++++-
>>  drivers/input/keyboard/gpio_keys_polled.c |  2 +-
>>  drivers/leds/leds-gpio.c                  |  2 +-
>>  include/linux/gpio/consumer.h             |  1 +
>
> Alexandre: does this match your vision of how it should work, i.e. ACK?

Pretty much, yes - as I mentioned in the previous versions there may
be shortcomings for ACPI, but we need a refactor of the whole thing -
nothing that this patch should address by itself.

So this patch:

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij March 4, 2015, 1 p.m. UTC | #6 
On Wed, Jan 21, 2015 at 10:33 PM, Olliver Schinagl
<o.schinagl@ultimaker.com> wrote:

> From: Olliver Schinagl <oliver@schinagl.nl>
>
> The gpio binding document says that new code should always use named
> gpios. Patch 40b73183 added support to parse a list of gpios from child
> nodes, but does not make it possible to use named gpios. This patch adds
> the con_id property and implements it is done in gpiolib.c, where the
> old-style of using unnamed gpios still works.
>
> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>

Patch applied with the ACKs and tags.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/gpio/devres.c b/drivers/gpio/devres.c
index 13dbd3d..12c2082 100644
--- a/drivers/gpio/devres.c
+++ b/drivers/gpio/devres.c
@@ -111,23 +111,39 @@  EXPORT_SYMBOL(__devm_gpiod_get_index);
 /**
  * devm_get_gpiod_from_child - get a GPIO descriptor from a device's child node
  * @dev:	GPIO consumer
+ * @con_id:	function within the GPIO consumer
  * @child:	firmware node (child of @dev)
  *
  * GPIO descriptors returned from this function are automatically disposed on
  * driver detach.
  */
 struct gpio_desc *devm_get_gpiod_from_child(struct device *dev,
+					    const char *con_id,
 					    struct fwnode_handle *child)
 {
+	static const char const *suffixes[] = { "gpios", "gpio" };
+	char prop_name[32]; /* 32 is max size of property name */
 	struct gpio_desc **dr;
 	struct gpio_desc *desc;
+	unsigned int i;
 
 	dr = devres_alloc(devm_gpiod_release, sizeof(struct gpio_desc *),
 			  GFP_KERNEL);
 	if (!dr)
 		return ERR_PTR(-ENOMEM);
 
-	desc = fwnode_get_named_gpiod(child, "gpios");
+	for (i = 0; i < ARRAY_SIZE(suffixes); i++) {
+		if (con_id)
+			snprintf(prop_name, sizeof(prop_name), "%s-%s",
+					    con_id, suffixes[i]);
+		else
+			snprintf(prop_name, sizeof(prop_name), "%s",
+							       suffixes[i]);
+
+		desc = fwnode_get_named_gpiod(child, prop_name);
+		if (!IS_ERR(desc) || (PTR_ERR(desc) == -EPROBE_DEFER))
+			break;
+	}
 	if (IS_ERR(desc)) {
 		devres_free(dr);
 		return desc;
diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
index 90df4df..097d721 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -125,7 +125,7 @@  static struct gpio_keys_platform_data *gpio_keys_polled_get_devtree_pdata(struct
 	device_for_each_child_node(dev, child) {
 		struct gpio_desc *desc;
 
-		desc = devm_get_gpiod_from_child(dev, child);
+		desc = devm_get_gpiod_from_child(dev, NULL, child);
 		if (IS_ERR(desc)) {
 			error = PTR_ERR(desc);
 			if (error != -EPROBE_DEFER)
diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index 7ea1ea42..8e5af69 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -184,7 +184,7 @@  static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev)
 		struct gpio_led led = {};
 		const char *state = NULL;
 
-		led.gpiod = devm_get_gpiod_from_child(dev, child);
+		led.gpiod = devm_get_gpiod_from_child(dev, NULL, child);
 		if (IS_ERR(led.gpiod)) {
 			fwnode_handle_put(child);
 			goto err;
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 45afc2d..ed20759 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -110,6 +110,7 @@  struct fwnode_handle;
 struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode,
 					 const char *propname);
 struct gpio_desc *devm_get_gpiod_from_child(struct device *dev,
+					    const char *con_id,
 					    struct fwnode_handle *child);
 #else /* CONFIG_GPIOLIB */