[Patch v2 0/2] gpio: add GPIO hogging mechanism

[Patch v2 0/2] gpio: add GPIO hogging mechanism

[Benoit Parrot]

This patch set re-introduces the gpio hogging concept first 
presented by Boris Brezillion.
This patch set provides a way to initally configure specific GPIO
when the gpio controller is probed.

The actual DT scanning to collect the GPIO specific data is performed
as part of the gpiochip_add().

The purpose of this is to allows specific GPIOs to be configured
without any driver specific code.
This particularly useful because board design are getting
increasingly complex and given SoC pins can now have upward
of 10 mux values a lot of connections are now dependent on
external IO muxes to switch various modes and combination.

Specific drivers should not necessarily need to be aware of
what accounts to a specific board implementation. This board level
"description" should be best kept as part of the dts file.

Changes since v1:
 * Split the devicetree bindings documentation in its own patch.
 * Refactor the gpio-hog mechanism as private functions meant to
   be to invoked from of_gpiochip_add().

Benoit Parrot (2):
  gpio: add GPIO hogging mechanism
  gpio: Document GPIO hogging mechanism

 Documentation/devicetree/bindings/gpio/gpio.txt |  25 ++++
 drivers/gpio/gpiolib-of.c                       | 188 ++++++++++++++++++++++++
 2 files changed, 213 insertions(+)

-- 
1.8.5.1

[Patch v2 1/2] gpio: add GPIO hogging mechanism

[Benoit Parrot]

Based on Boris Brezillion's work this is a reworked patch
of his initial GPIO hogging mechanism.
This patch provides a way to initally configure specific GPIO
when the gpio controller is probed.

The actual DT scanning to collect the GPIO specific data is performed
as part of the gpiochip_add().

The purpose of this is to allows specific GPIOs to be configured
without any driver specific code.
This particularly useful because board design are getting
increasingly complex and given SoC pins can now have upward
of 10 mux values a lot of connections are now dependent on
external IO muxes to switch various modes and combination.

Specific drivers should not necessarily need to be aware of
what accounts to a specific board implementation. This board level
"description" should be best kept as part of the dts file.

Signed-off-by: Benoit Parrot <bparrot@ti.com>
---
Changes since v1:
 * Refactor the gpio-hog mechanism as private functions meant to
   be to invoked from of_gpiochip_add().

 drivers/gpio/gpiolib-of.c | 188 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 188 insertions(+)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 604dbe6..3caed76 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -22,6 +22,7 @@
 #include <linux/of_gpio.h>
 #include <linux/pinctrl/pinctrl.h>
 #include <linux/slab.h>
+#include <linux/gpio/machine.h>
 
 #include "gpiolib.h"
 
@@ -111,6 +112,184 @@ int of_get_named_gpio_flags(struct device_node *np, const char *list_name,
 EXPORT_SYMBOL(of_get_named_gpio_flags);
 
 /**
+ * of_get_gpio_hog() - Get a GPIO hog descriptor, names and flags for GPIO API
+ * @np:		device node to get GPIO from
+ * @name:	GPIO line name
+ * @flags:	a flags pointer to fill in
+ *
+ * Returns GPIO descriptor to use with Linux GPIO API, or one of the errno
+ * value on the error condition.
+ */
+
+static struct gpio_desc *of_get_gpio_hog(struct device_node *np,
+				  const char **name,
+				  enum gpio_lookup_flags *lflags,
+				  enum gpiod_flags *dflags)
+{
+	struct device_node *chip_np;
+	enum of_gpio_flags xlate_flags;
+	struct gpio_desc *desc;
+	struct gg_data gg_data = {
+		.flags = &xlate_flags,
+		.out_gpio = NULL,
+	};
+	u32 tmp;
+	int i, in, outlo, outhi;
+	int ret;
+
+	if (!np)
+		return ERR_PTR(-EINVAL);
+
+	chip_np = np->parent;
+	if (!chip_np) {
+		desc = ERR_PTR(-EINVAL);
+		goto out;
+	}
+
+	if (!lflags || !dflags) {
+		desc = ERR_PTR(-EINVAL);
+		goto out;
+	}
+
+	*lflags = 0;
+	*dflags = 0;
+	in = 0;
+	outlo = 0;
+	outhi = 0;
+
+	ret = of_property_read_u32(chip_np, "#gpio-cells", &tmp);
+	if (ret) {
+		desc = ERR_PTR(ret);
+		goto out;
+	}
+
+	if (tmp > MAX_PHANDLE_ARGS) {
+		desc = ERR_PTR(-EINVAL);
+		goto out;
+	}
+
+	gg_data.gpiospec.args_count = tmp;
+	gg_data.gpiospec.np = chip_np;
+	for (i = 0; i < tmp; i++) {
+		ret = of_property_read_u32(np, "gpios",
+					   &gg_data.gpiospec.args[i]);
+		if (ret) {
+			desc = ERR_PTR(ret);
+			goto out;
+		}
+	}
+
+	gpiochip_find(&gg_data, of_gpiochip_find_and_xlate);
+	if (!gg_data.out_gpio) {
+		if (np->parent == np)
+			desc = ERR_PTR(-ENXIO);
+		else
+			desc = ERR_PTR(-EPROBE_DEFER);
+		goto out;
+	}
+
+	if (xlate_flags & OF_GPIO_ACTIVE_LOW)
+		*lflags |= GPIOF_ACTIVE_LOW;
+
+	if (of_property_read_bool(np, "input")) {
+		*dflags |= GPIOD_IN;
+		in = 1;
+	}
+	if (of_property_read_bool(np, "output-low")) {
+		*dflags |= GPIOD_OUT_LOW;
+		outlo = 1;
+	}
+	if (of_property_read_bool(np, "output-high")) {
+		*dflags |= GPIOD_OUT_HIGH;
+		outhi = 1;
+	}
+	if ((in + outlo + outhi) > 1) {
+		pr_warn("%s: GPIO %s: more than one direction/value selected, assuming: %s.\n",
+			 __func__, np->name,
+			 (outhi)?"output-high":(outlo)?"output-low":"input");
+	}
+
+	if (of_property_read_bool(np, "open-drain-line"))
+		*lflags |= GPIO_OPEN_DRAIN;
+	if (of_property_read_bool(np, "open-source-line"))
+		*lflags |= GPIO_OPEN_SOURCE;
+	if (name && of_property_read_string(np, "line-name", name))
+		*name = np->name;
+
+	desc = gg_data.out_gpio;
+
+out:
+	return desc;
+}
+
+
+/**
+ * do_gpio_hog - Given node is a GPIO hog configuration, handle it
+ * @np:		device node to get GPIO from
+ *
+ * This is only used by of_gpiochip_add to request/set GPIO initial
+ * configuration.
+ */
+static int do_gpio_hog(struct device_node *np)
+{
+	struct gpio_desc *desc = NULL;
+	int err;
+	const char *name;
+	enum gpio_lookup_flags lflags;
+	enum gpiod_flags dflags;
+
+	desc = of_get_gpio_hog(np, &name, &lflags, &dflags);
+	if (!desc)
+		return -ENOTSUPP;
+	else if (IS_ERR(desc))
+		return PTR_ERR(desc);
+
+	err = gpiod_request(desc, name);
+	if (err < 0)
+		return err;
+
+	if (lflags & GPIO_ACTIVE_LOW)
+		set_bit(FLAG_ACTIVE_LOW, &desc->flags);
+	if (lflags & GPIO_OPEN_DRAIN)
+		set_bit(FLAG_OPEN_DRAIN, &desc->flags);
+	if (lflags & GPIO_OPEN_SOURCE)
+		set_bit(FLAG_OPEN_SOURCE, &desc->flags);
+
+	/* No particular flag request, not really hogging then... */
+	if (!(dflags & GPIOD_FLAGS_BIT_DIR_SET)) {
+		pr_warn("%s: GPIO %s: no hogging direction specified, bailing out\n",
+			 __func__, name);
+		err = -EINVAL;
+		goto free_gpio;
+	}
+
+	/* Process flags */
+	if (dflags & GPIOD_FLAGS_BIT_DIR_OUT)
+		err = gpiod_direction_output(desc,
+					     dflags & GPIOD_FLAGS_BIT_DIR_VAL);
+	else
+		err = gpiod_direction_input(desc);
+
+	if (err < 0) {
+		pr_warn("%s: GPIO %s setting direction/value failed\n",
+			 __func__, name);
+		goto free_gpio;
+	}
+
+	pr_debug("%s: gpio-hog: GPIO:%d (%s) as %s%s\n", __func__,
+		desc_to_gpio(desc), name,
+		(dflags&GPIOD_FLAGS_BIT_DIR_OUT) ? "output" : "input",
+		(dflags&GPIOD_FLAGS_BIT_DIR_OUT) ?
+			(dflags&GPIOD_FLAGS_BIT_DIR_VAL) ? "/high" : "/low":"");
+
+	return 0;
+
+free_gpio:
+	gpiod_put(desc);
+	return err;
+}
+
+/**
  * 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
@@ -289,6 +468,8 @@ static void of_gpiochip_add_pin_range(struct gpio_chip *chip) {}
 
 void of_gpiochip_add(struct gpio_chip *chip)
 {
+	struct device_node *np;
+
 	if ((!chip->of_node) && (chip->dev))
 		chip->of_node = chip->dev->of_node;
 
@@ -302,6 +483,13 @@ void of_gpiochip_add(struct gpio_chip *chip)
 
 	of_gpiochip_add_pin_range(chip);
 	of_node_get(chip->of_node);
+
+	for_each_child_of_node(chip->dev->of_node, np) {
+		if (!of_property_read_bool(np, "gpio-hog"))
+			continue;
+		/* Hog this GPIO */
+		do_gpio_hog(np);
+	}
 }
 
 void of_gpiochip_remove(struct gpio_chip *chip)
-- 
1.8.5.1

Re: [Patch v2 1/2] gpio: add GPIO hogging mechanism

[Alexandre Courbot]

On Fri, Nov 21, 2014 at 8:54 AM, Benoit Parrot <bparrot@ti.com> wrote:
> Based on Boris Brezillion's work this is a reworked patch
> of his initial GPIO hogging mechanism.
> This patch provides a way to initally configure specific GPIO
> when the gpio controller is probed.
>
> The actual DT scanning to collect the GPIO specific data is performed
> as part of the gpiochip_add().
>
> The purpose of this is to allows specific GPIOs to be configured
> without any driver specific code.
> This particularly useful because board design are getting
> increasingly complex and given SoC pins can now have upward
> of 10 mux values a lot of connections are now dependent on
> external IO muxes to switch various modes and combination.
>
> Specific drivers should not necessarily need to be aware of
> what accounts to a specific board implementation. This board level
> "description" should be best kept as part of the dts file.
>
> Signed-off-by: Benoit Parrot <bparrot@ti.com>
> ---
> Changes since v1:
>  * Refactor the gpio-hog mechanism as private functions meant to
>    be to invoked from of_gpiochip_add().
>
>  drivers/gpio/gpiolib-of.c | 188 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 188 insertions(+)
>
> diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
> index 604dbe6..3caed76 100644
> --- a/drivers/gpio/gpiolib-of.c
> +++ b/drivers/gpio/gpiolib-of.c
> @@ -22,6 +22,7 @@
>  #include <linux/of_gpio.h>
>  #include <linux/pinctrl/pinctrl.h>
>  #include <linux/slab.h>
> +#include <linux/gpio/machine.h>
>
>  #include "gpiolib.h"
>
> @@ -111,6 +112,184 @@ int of_get_named_gpio_flags(struct device_node *np, const char *list_name,
>  EXPORT_SYMBOL(of_get_named_gpio_flags);
>
>  /**
> + * of_get_gpio_hog() - Get a GPIO hog descriptor, names and flags for GPIO API
> + * @np:                device node to get GPIO from
> + * @name:      GPIO line name
> + * @flags:     a flags pointer to fill in
> + *
> + * Returns GPIO descriptor to use with Linux GPIO API, or one of the errno
> + * value on the error condition.
> + */
> +
> +static struct gpio_desc *of_get_gpio_hog(struct device_node *np,
> +                                 const char **name,
> +                                 enum gpio_lookup_flags *lflags,
> +                                 enum gpiod_flags *dflags)
> +{
> +       struct device_node *chip_np;
> +       enum of_gpio_flags xlate_flags;
> +       struct gpio_desc *desc;
> +       struct gg_data gg_data = {
> +               .flags = &xlate_flags,
> +               .out_gpio = NULL,
> +       };
> +       u32 tmp;
> +       int i, in, outlo, outhi;
> +       int ret;
> +
> +       if (!np)
> +               return ERR_PTR(-EINVAL);

This function is being called from a perfectly mastered context in
this file, so maybe we can avoid this check.

> +
> +       chip_np = np->parent;
> +       if (!chip_np) {
> +               desc = ERR_PTR(-EINVAL);
> +               goto out;
> +       }
> +
> +       if (!lflags || !dflags) {
> +               desc = ERR_PTR(-EINVAL);
> +               goto out;
> +       }

Same for this one.

> +
> +       *lflags = 0;
> +       *dflags = 0;
> +       in = 0;
> +       outlo = 0;
> +       outhi = 0;
> +
> +       ret = of_property_read_u32(chip_np, "#gpio-cells", &tmp);
> +       if (ret) {
> +               desc = ERR_PTR(ret);
> +               goto out;

Please use "return ERR_PTR(ret);" directly, since you do absolutely no
cleanup in out:. Same remark everywhere it applies.

> +       }
> +
> +       if (tmp > MAX_PHANDLE_ARGS) {
> +               desc = ERR_PTR(-EINVAL);
> +               goto out;
> +       }
> +
> +       gg_data.gpiospec.args_count = tmp;
> +       gg_data.gpiospec.np = chip_np;
> +       for (i = 0; i < tmp; i++) {
> +               ret = of_property_read_u32(np, "gpios",
> +                                          &gg_data.gpiospec.args[i]);
> +               if (ret) {
> +                       desc = ERR_PTR(ret);
> +                       goto out;
> +               }
> +       }
> +
> +       gpiochip_find(&gg_data, of_gpiochip_find_and_xlate);

This seems to work but only supports one GPIO per hog node. It would
be nice to be able to specify several GPIOs to which the same settings
need to be applied.

> +       if (!gg_data.out_gpio) {
> +               if (np->parent == np)
> +                       desc = ERR_PTR(-ENXIO);
> +               else
> +                       desc = ERR_PTR(-EPROBE_DEFER);
> +               goto out;
> +       }
> +
> +       if (xlate_flags & OF_GPIO_ACTIVE_LOW)
> +               *lflags |= GPIOF_ACTIVE_LOW;
> +
> +       if (of_property_read_bool(np, "input")) {
> +               *dflags |= GPIOD_IN;
> +               in = 1;
> +       }
> +       if (of_property_read_bool(np, "output-low")) {
> +               *dflags |= GPIOD_OUT_LOW;
> +               outlo = 1;
> +       }
> +       if (of_property_read_bool(np, "output-high")) {
> +               *dflags |= GPIOD_OUT_HIGH;
> +               outhi = 1;
> +       }

I thought we agreed that this should be a "direction =
input|output-low|output-high" property?

> +       if ((in + outlo + outhi) > 1) {
> +               pr_warn("%s: GPIO %s: more than one direction/value selected, assuming: %s.\n",
> +                        __func__, np->name,
> +                        (outhi)?"output-high":(outlo)?"output-low":"input");
> +       }
> +
> +       if (of_property_read_bool(np, "open-drain-line"))
> +               *lflags |= GPIO_OPEN_DRAIN;
> +       if (of_property_read_bool(np, "open-source-line"))
> +               *lflags |= GPIO_OPEN_SOURCE;

These properties are not documented in your DT bindings.

> +       if (name && of_property_read_string(np, "line-name", name))
> +               *name = np->name;
> +
> +       desc = gg_data.out_gpio;
> +
> +out:
> +       return desc;
> +}
> +
> +
> +/**
> + * do_gpio_hog - Given node is a GPIO hog configuration, handle it
> + * @np:                device node to get GPIO from
> + *
> + * This is only used by of_gpiochip_add to request/set GPIO initial
> + * configuration.
> + */
> +static int do_gpio_hog(struct device_node *np)
> +{
> +       struct gpio_desc *desc = NULL;
> +       int err;
> +       const char *name;
> +       enum gpio_lookup_flags lflags;
> +       enum gpiod_flags dflags;
> +
> +       desc = of_get_gpio_hog(np, &name, &lflags, &dflags);
> +       if (!desc)
> +               return -ENOTSUPP;
> +       else if (IS_ERR(desc))
> +               return PTR_ERR(desc);
> +
> +       err = gpiod_request(desc, name);

Using this function means that a GPIO chip module cannot be unloaded
if it uses GPIO hogs. Is it the intended behavior? If not, please use
gpiochip_request_own_desc() instead, and make sure to call
gpiochip_free_own_desc() for each hog when the driver is unloaded.

> +       if (err < 0)
> +               return err;
> +
> +       if (lflags & GPIO_ACTIVE_LOW)
> +               set_bit(FLAG_ACTIVE_LOW, &desc->flags);
> +       if (lflags & GPIO_OPEN_DRAIN)
> +               set_bit(FLAG_OPEN_DRAIN, &desc->flags);
> +       if (lflags & GPIO_OPEN_SOURCE)
> +               set_bit(FLAG_OPEN_SOURCE, &desc->flags);
> +
> +       /* No particular flag request, not really hogging then... */
> +       if (!(dflags & GPIOD_FLAGS_BIT_DIR_SET)) {
> +               pr_warn("%s: GPIO %s: no hogging direction specified, bailing out\n",
> +                        __func__, name);
> +               err = -EINVAL;
> +               goto free_gpio;
> +       }
> +
> +       /* Process flags */
> +       if (dflags & GPIOD_FLAGS_BIT_DIR_OUT)
> +               err = gpiod_direction_output(desc,
> +                                            dflags & GPIOD_FLAGS_BIT_DIR_VAL);
> +       else
> +               err = gpiod_direction_input(desc);
> +
> +       if (err < 0) {
> +               pr_warn("%s: GPIO %s setting direction/value failed\n",
> +                        __func__, name);
> +               goto free_gpio;
> +       }

I would suggest to factorize this code that is similar to the one
found in __gpiod_get_index(). Do all the DT parsing in a function that
just returns a descriptor and the

> +
> +       pr_debug("%s: gpio-hog: GPIO:%d (%s) as %s%s\n", __func__,
> +               desc_to_gpio(desc), name,
> +               (dflags&GPIOD_FLAGS_BIT_DIR_OUT) ? "output" : "input",
> +               (dflags&GPIOD_FLAGS_BIT_DIR_OUT) ?
> +                       (dflags&GPIOD_FLAGS_BIT_DIR_VAL) ? "/high" : "/low":"");
> +
> +       return 0;
> +
> +free_gpio:
> +       gpiod_put(desc);
> +       return err;
> +}

I think most of this function should be moved into gpiolib.c. Add a
function there that takes the descriptor, lflags and dflags and that
sets the GPIO up, possibly adding it to a list of hogs so we can unset
the hogs when unloading the module. That way we have a hogging
mechanism that is not dependent on device tree and can be used by
other GPIO binders, such as ACPI.

There is still some work needed but it looks much better than the
first version. Please make sure to also keep Maxime, Pantelis and Jiri
in the loop since they expressed interest in your first version.

Re: [Patch v2 1/2] gpio: add GPIO hogging mechanism

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1494 bytes --]

Hi,

On Fri, Nov 28, 2014 at 04:30:01PM +0900, Alexandre Courbot wrote:
> > +/**
> > + * do_gpio_hog - Given node is a GPIO hog configuration, handle it
> > + * @np:                device node to get GPIO from
> > + *
> > + * This is only used by of_gpiochip_add to request/set GPIO initial
> > + * configuration.
> > + */
> > +static int do_gpio_hog(struct device_node *np)
> > +{
> > +       struct gpio_desc *desc = NULL;
> > +       int err;
> > +       const char *name;
> > +       enum gpio_lookup_flags lflags;
> > +       enum gpiod_flags dflags;
> > +
> > +       desc = of_get_gpio_hog(np, &name, &lflags, &dflags);
> > +       if (!desc)
> > +               return -ENOTSUPP;
> > +       else if (IS_ERR(desc))
> > +               return PTR_ERR(desc);
> > +
> > +       err = gpiod_request(desc, name);
> 
> Using this function means that a GPIO chip module cannot be unloaded
> if it uses GPIO hogs. Is it the intended behavior? If not, please use
> gpiochip_request_own_desc() instead, and make sure to call
> gpiochip_free_own_desc() for each hog when the driver is unloaded.

The only thing I'd like to have would be that the request here would
be non-exclusive, so that a later driver would still be allowed later
on to request that GPIO later on and manage it itself (ideally using
the usual gpiod_request function).

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Patch v2 1/2] gpio: add GPIO hogging mechanism

[Benoit Parrot]

Maxime Ripard <maxime.ripard@free-electrons.com> wrote on Mon [2014-Dec-01 17:36:39 +0100]:
> Hi,
> 
> On Fri, Nov 28, 2014 at 04:30:01PM +0900, Alexandre Courbot wrote:
> > > +/**
> > > + * do_gpio_hog - Given node is a GPIO hog configuration, handle it
> > > + * @np:                device node to get GPIO from
> > > + *
> > > + * This is only used by of_gpiochip_add to request/set GPIO initial
> > > + * configuration.
> > > + */
> > > +static int do_gpio_hog(struct device_node *np)
> > > +{
> > > +       struct gpio_desc *desc = NULL;
> > > +       int err;
> > > +       const char *name;
> > > +       enum gpio_lookup_flags lflags;
> > > +       enum gpiod_flags dflags;
> > > +
> > > +       desc = of_get_gpio_hog(np, &name, &lflags, &dflags);
> > > +       if (!desc)
> > > +               return -ENOTSUPP;
> > > +       else if (IS_ERR(desc))
> > > +               return PTR_ERR(desc);
> > > +
> > > +       err = gpiod_request(desc, name);
> > 
> > Using this function means that a GPIO chip module cannot be unloaded
> > if it uses GPIO hogs. Is it the intended behavior? If not, please use
> > gpiochip_request_own_desc() instead, and make sure to call
> > gpiochip_free_own_desc() for each hog when the driver is unloaded.
> 
> The only thing I'd like to have would be that the request here would
> be non-exclusive, so that a later driver would still be allowed later
> on to request that GPIO later on and manage it itself (ideally using
> the usual gpiod_request function).

I'll let Linus chime in on this.
But the premise for the hogging mechanism is to have a mechanism to set GPIOs
which do not need to be requested by any other entity.
If a driver needs access to a specific GPIO then it should use the existing
gpiolib API to request it and set it up.

> 
> Maxime
> 
> -- 
> Maxime Ripard, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com

Regards,
Benoit

Re: [Patch v2 1/2] gpio: add GPIO hogging mechanism

[Alexandre Courbot]

On Tue, Dec 2, 2014 at 1:36 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi,
>
> On Fri, Nov 28, 2014 at 04:30:01PM +0900, Alexandre Courbot wrote:
>> > +/**
>> > + * do_gpio_hog - Given node is a GPIO hog configuration, handle it
>> > + * @np:                device node to get GPIO from
>> > + *
>> > + * This is only used by of_gpiochip_add to request/set GPIO initial
>> > + * configuration.
>> > + */
>> > +static int do_gpio_hog(struct device_node *np)
>> > +{
>> > +       struct gpio_desc *desc = NULL;
>> > +       int err;
>> > +       const char *name;
>> > +       enum gpio_lookup_flags lflags;
>> > +       enum gpiod_flags dflags;
>> > +
>> > +       desc = of_get_gpio_hog(np, &name, &lflags, &dflags);
>> > +       if (!desc)
>> > +               return -ENOTSUPP;
>> > +       else if (IS_ERR(desc))
>> > +               return PTR_ERR(desc);
>> > +
>> > +       err = gpiod_request(desc, name);
>>
>> Using this function means that a GPIO chip module cannot be unloaded
>> if it uses GPIO hogs. Is it the intended behavior? If not, please use
>> gpiochip_request_own_desc() instead, and make sure to call
>> gpiochip_free_own_desc() for each hog when the driver is unloaded.
>
> The only thing I'd like to have would be that the request here would
> be non-exclusive, so that a later driver would still be allowed later
> on to request that GPIO later on and manage it itself (ideally using
> the usual gpiod_request function).

Actually we have a plan (and I have some code too) to allow multiple
consumers per GPIO. Although like Benoit I wonder why you would want
to hog a GPIO and then request it properly later. Also, that probably
means we should abandon the hog since it actively drives the line and
would interfere with the late requested. How to do that correctly is
not really clear to me.

Re: [Patch v2 1/2] gpio: add GPIO hogging mechanism

[Linus Walleij]

On Tue, Dec 2, 2014 at 3:13 PM, Alexandre Courbot <gnurou@gmail.com> wrote:
> On Tue, Dec 2, 2014 at 1:36 AM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:

>> The only thing I'd like to have would be that the request here would
>> be non-exclusive, so that a later driver would still be allowed later
>> on to request that GPIO later on and manage it itself (ideally using
>> the usual gpiod_request function).
>
> Actually we have a plan (and I have some code too) to allow multiple
> consumers per GPIO. Although like Benoit I wonder why you would want
> to hog a GPIO and then request it properly later. Also, that probably
> means we should abandon the hog since it actively drives the line and
> would interfere with the late requested. How to do that correctly is
> not really clear to me.

I don't get the usecase. A hogged GPIO is per definition hogged.
This sounds more like "initial settings" or something, which is another
usecase altogether.

Yours,
Linus Walleij

Re: [Patch v2 1/2] gpio: add GPIO hogging mechanism

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2111 bytes --]

On Tue, Dec 02, 2014 at 03:29:46PM +0100, Linus Walleij wrote:
> On Tue, Dec 2, 2014 at 3:13 PM, Alexandre Courbot <gnurou@gmail.com> wrote:
> > On Tue, Dec 2, 2014 at 1:36 AM, Maxime Ripard
> > <maxime.ripard@free-electrons.com> wrote:
> 
> >> The only thing I'd like to have would be that the request here would
> >> be non-exclusive, so that a later driver would still be allowed later
> >> on to request that GPIO later on and manage it itself (ideally using
> >> the usual gpiod_request function).
> >
> > Actually we have a plan (and I have some code too) to allow multiple
> > consumers per GPIO. Although like Benoit I wonder why you would want
> > to hog a GPIO and then request it properly later. Also, that probably
> > means we should abandon the hog since it actively drives the line and
> > would interfere with the late requested. How to do that correctly is
> > not really clear to me.
> 
> I don't get the usecase. A hogged GPIO is per definition hogged.
> This sounds more like "initial settings" or something, which is another
> usecase altogether.

We do have one board where we have a pin (let's say GPIO14 of the bank
A) that enables a regulator that will provide VCC the bank B.

Now, both banks are handled by the same driver, but in order to have a
working output on the bank B, we do need to set GPIO14 as soon as
we're probed.

Just relying on the usual deferred probing introduces a circular
dependency between the gpio-regulator that needs to grab its GPIO from
a driver not there yet, and the gpio driver that needs to enable its
gpio-regulator.

GPIO hogging needs to be the ideal solution for that, since we can
just enforce the GPIO14 value as the driver is probed, which provides
the guarantee that any driver using the bank B will actually drive the
GPIO it might use.

However, an exclusive request will prevent any representation of this
as a regulator, which sounds a bit weird, since it really is just
that.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Patch v2 1/2] gpio: add GPIO hogging mechanism

[Alexandre Courbot]

On Wed, Dec 3, 2014 at 1:12 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Tue, Dec 02, 2014 at 03:29:46PM +0100, Linus Walleij wrote:
>> On Tue, Dec 2, 2014 at 3:13 PM, Alexandre Courbot <gnurou@gmail.com> wrote:
>> > On Tue, Dec 2, 2014 at 1:36 AM, Maxime Ripard
>> > <maxime.ripard@free-electrons.com> wrote:
>>
>> >> The only thing I'd like to have would be that the request here would
>> >> be non-exclusive, so that a later driver would still be allowed later
>> >> on to request that GPIO later on and manage it itself (ideally using
>> >> the usual gpiod_request function).
>> >
>> > Actually we have a plan (and I have some code too) to allow multiple
>> > consumers per GPIO. Although like Benoit I wonder why you would want
>> > to hog a GPIO and then request it properly later. Also, that probably
>> > means we should abandon the hog since it actively drives the line and
>> > would interfere with the late requested. How to do that correctly is
>> > not really clear to me.
>>
>> I don't get the usecase. A hogged GPIO is per definition hogged.
>> This sounds more like "initial settings" or something, which is another
>> usecase altogether.
>
> We do have one board where we have a pin (let's say GPIO14 of the bank
> A) that enables a regulator that will provide VCC the bank B.
>
> Now, both banks are handled by the same driver, but in order to have a
> working output on the bank B, we do need to set GPIO14 as soon as
> we're probed.
>
> Just relying on the usual deferred probing introduces a circular
> dependency between the gpio-regulator that needs to grab its GPIO from
> a driver not there yet, and the gpio driver that needs to enable its
> gpio-regulator.

I don't get it. According to what you said, the following order should
go through IIUC:

1) bank A is probed, gpio 14 is available
2) gpio-regulator is probed, acquires GPIO 14, regulator for Bank B is available
3) bank B is probed, grabs its regulator and turn it on, probes.

What am I missing?

>
> GPIO hogging needs to be the ideal solution for that, since we can
> just enforce the GPIO14 value as the driver is probed, which provides
> the guarantee that any driver using the bank B will actually drive the
> GPIO it might use.

At this point I start wondering if such initial setup should not be
the job of the bootloader? GPIO hogging ought to be simple and
definitive, adding the possibility to have it just as an initial value
would considerably complexify it. E.g. when is the gpio chip driver
supposed to release the hogged descriptor in such a case?

Note that if the multiple GPIO consumer feature we are planning goes
through, you should be able to use both hogging *and* a regulator on
the same GPIO and achieve what you want. The expectation of multiple
consumers is that the board designers know what they are doing, and
this case would certainly fit (chip hogs the line and doesn't touch
the value after that, letting the regulator control it without any
conflict afterwards), although it would of course be better to solve
the issue through regular probing...

Re: [Patch v2 1/2] gpio: add GPIO hogging mechanism

[Pantelis Antoniou]

Hi Alexandre,

I tried to stay away while things are being fleshed out but…

> On Dec 4, 2014, at 16:15 , Alexandre Courbot <gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> 
> On Wed, Dec 3, 2014 at 1:12 AM, Maxime Ripard
> <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>> On Tue, Dec 02, 2014 at 03:29:46PM +0100, Linus Walleij wrote:
>>> On Tue, Dec 2, 2014 at 3:13 PM, Alexandre Courbot <gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>> On Tue, Dec 2, 2014 at 1:36 AM, Maxime Ripard
>>>> <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>>> 
>>>>> The only thing I'd like to have would be that the request here would
>>>>> be non-exclusive, so that a later driver would still be allowed later
>>>>> on to request that GPIO later on and manage it itself (ideally using
>>>>> the usual gpiod_request function).
>>>> 
>>>> Actually we have a plan (and I have some code too) to allow multiple
>>>> consumers per GPIO. Although like Benoit I wonder why you would want
>>>> to hog a GPIO and then request it properly later. Also, that probably
>>>> means we should abandon the hog since it actively drives the line and
>>>> would interfere with the late requested. How to do that correctly is
>>>> not really clear to me.
>>> 
>>> I don't get the usecase. A hogged GPIO is per definition hogged.
>>> This sounds more like "initial settings" or something, which is another
>>> usecase altogether.
>> 
>> We do have one board where we have a pin (let's say GPIO14 of the bank
>> A) that enables a regulator that will provide VCC the bank B.
>> 
>> Now, both banks are handled by the same driver, but in order to have a
>> working output on the bank B, we do need to set GPIO14 as soon as
>> we're probed.
>> 
>> Just relying on the usual deferred probing introduces a circular
>> dependency between the gpio-regulator that needs to grab its GPIO from
>> a driver not there yet, and the gpio driver that needs to enable its
>> gpio-regulator.
> 
> I don't get it. According to what you said, the following order should
> go through IIUC:
> 
> 1) bank A is probed, gpio 14 is available
> 2) gpio-regulator is probed, acquires GPIO 14, regulator for Bank B is available
> 3) bank B is probed, grabs its regulator and turn it on, probes.
> 
> What am I missing?
> 
>> 
>> GPIO hogging needs to be the ideal solution for that, since we can
>> just enforce the GPIO14 value as the driver is probed, which provides
>> the guarantee that any driver using the bank B will actually drive the
>> GPIO it might use.
> 
> At this point I start wondering if such initial setup should not be
> the job of the bootloader? GPIO hogging ought to be simple and
> definitive, adding the possibility to have it just as an initial value
> would considerably complexify it. E.g. when is the gpio chip driver
> supposed to release the hogged descriptor in such a case?
> 

Do not count on the bootloader setting up anything. The trend is
for the bootloader to setup the minimal environment to load your kernel
and jump to it.

http://www.denx.de/wiki/pub/U-Boot/MiniSummitELCE2013/2013-ELCE-U-Boot-Falcon-Boot.pdf


> Note that if the multiple GPIO consumer feature we are planning goes
> through, you should be able to use both hogging *and* a regulator on
> the same GPIO and achieve what you want. The expectation of multiple
> consumers is that the board designers know what they are doing, and
> this case would certainly fit (chip hogs the line and doesn't touch
> the value after that, letting the regulator control it without any
> conflict afterwards), although it would of course be better to solve
> the issue through regular probing...


That’s why I was advocating a simple probing driver for all this.
Figure out a way for this driver to be probed first would be an easier
solution that what’s going on here.

Regards

— Pantelis


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

Re: [Patch v2 1/2] gpio: add GPIO hogging mechanism

[Alexandre Courbot]

On Thu, Dec 4, 2014 at 11:27 PM, Pantelis Antoniou
<panto@antoniou-consulting.com> wrote:
> Hi Alexandre,
>
> I tried to stay away while things are being fleshed out but…
>
>> On Dec 4, 2014, at 16:15 , Alexandre Courbot <gnurou@gmail.com> wrote:
>>
>> On Wed, Dec 3, 2014 at 1:12 AM, Maxime Ripard
>> <maxime.ripard@free-electrons.com> wrote:
>>> On Tue, Dec 02, 2014 at 03:29:46PM +0100, Linus Walleij wrote:
>>>> On Tue, Dec 2, 2014 at 3:13 PM, Alexandre Courbot <gnurou@gmail.com> wrote:
>>>>> On Tue, Dec 2, 2014 at 1:36 AM, Maxime Ripard
>>>>> <maxime.ripard@free-electrons.com> wrote:
>>>>
>>>>>> The only thing I'd like to have would be that the request here would
>>>>>> be non-exclusive, so that a later driver would still be allowed later
>>>>>> on to request that GPIO later on and manage it itself (ideally using
>>>>>> the usual gpiod_request function).
>>>>>
>>>>> Actually we have a plan (and I have some code too) to allow multiple
>>>>> consumers per GPIO. Although like Benoit I wonder why you would want
>>>>> to hog a GPIO and then request it properly later. Also, that probably
>>>>> means we should abandon the hog since it actively drives the line and
>>>>> would interfere with the late requested. How to do that correctly is
>>>>> not really clear to me.
>>>>
>>>> I don't get the usecase. A hogged GPIO is per definition hogged.
>>>> This sounds more like "initial settings" or something, which is another
>>>> usecase altogether.
>>>
>>> We do have one board where we have a pin (let's say GPIO14 of the bank
>>> A) that enables a regulator that will provide VCC the bank B.
>>>
>>> Now, both banks are handled by the same driver, but in order to have a
>>> working output on the bank B, we do need to set GPIO14 as soon as
>>> we're probed.
>>>
>>> Just relying on the usual deferred probing introduces a circular
>>> dependency between the gpio-regulator that needs to grab its GPIO from
>>> a driver not there yet, and the gpio driver that needs to enable its
>>> gpio-regulator.
>>
>> I don't get it. According to what you said, the following order should
>> go through IIUC:
>>
>> 1) bank A is probed, gpio 14 is available
>> 2) gpio-regulator is probed, acquires GPIO 14, regulator for Bank B is available
>> 3) bank B is probed, grabs its regulator and turn it on, probes.
>>
>> What am I missing?
>>
>>>
>>> GPIO hogging needs to be the ideal solution for that, since we can
>>> just enforce the GPIO14 value as the driver is probed, which provides
>>> the guarantee that any driver using the bank B will actually drive the
>>> GPIO it might use.
>>
>> At this point I start wondering if such initial setup should not be
>> the job of the bootloader? GPIO hogging ought to be simple and
>> definitive, adding the possibility to have it just as an initial value
>> would considerably complexify it. E.g. when is the gpio chip driver
>> supposed to release the hogged descriptor in such a case?
>>
>
> Do not count on the bootloader setting up anything. The trend is
> for the bootloader to setup the minimal environment to load your kernel
> and jump to it.
>
> http://www.denx.de/wiki/pub/U-Boot/MiniSummitELCE2013/2013-ELCE-U-Boot-Falcon-Boot.pdf

Just wondering. :)

But yeah, there are some use-cases (such as this one or
Linux-as-a-bootloader) for which this would not play nicely.

>
>
>> Note that if the multiple GPIO consumer feature we are planning goes
>> through, you should be able to use both hogging *and* a regulator on
>> the same GPIO and achieve what you want. The expectation of multiple
>> consumers is that the board designers know what they are doing, and
>> this case would certainly fit (chip hogs the line and doesn't touch
>> the value after that, letting the regulator control it without any
>> conflict afterwards), although it would of course be better to solve
>> the issue through regular probing...
>
>
> That’s why I was advocating a simple probing driver for all this.
> Figure out a way for this driver to be probed first would be an easier
> solution that what’s going on here.

Do you mean, a driver whose sole job is to probe other drivers in the
right order? :/

Re: [Patch v2 1/2] gpio: add GPIO hogging mechanism

[Pantelis Antoniou]

Hi Alexandre,

> On Dec 4, 2014, at 16:41 , Alexandre Courbot <gnurou@gmail.com> wrote:
> 
> On Thu, Dec 4, 2014 at 11:27 PM, Pantelis Antoniou
> <panto@antoniou-consulting.com> wrote:
>> Hi Alexandre,
>> 
>> I tried to stay away while things are being fleshed out but…
>> 
>>> On Dec 4, 2014, at 16:15 , Alexandre Courbot <gnurou@gmail.com> wrote:
>>> 
>>> On Wed, Dec 3, 2014 at 1:12 AM, Maxime Ripard
>>> <maxime.ripard@free-electrons.com> wrote:
>>>> On Tue, Dec 02, 2014 at 03:29:46PM +0100, Linus Walleij wrote:
>>>>> On Tue, Dec 2, 2014 at 3:13 PM, Alexandre Courbot <gnurou@gmail.com> wrote:
>>>>>> On Tue, Dec 2, 2014 at 1:36 AM, Maxime Ripard
>>>>>> <maxime.ripard@free-electrons.com> wrote:
>>>>> 
>>>>>>> The only thing I'd like to have would be that the request here would
>>>>>>> be non-exclusive, so that a later driver would still be allowed later
>>>>>>> on to request that GPIO later on and manage it itself (ideally using
>>>>>>> the usual gpiod_request function).
>>>>>> 
>>>>>> Actually we have a plan (and I have some code too) to allow multiple
>>>>>> consumers per GPIO. Although like Benoit I wonder why you would want
>>>>>> to hog a GPIO and then request it properly later. Also, that probably
>>>>>> means we should abandon the hog since it actively drives the line and
>>>>>> would interfere with the late requested. How to do that correctly is
>>>>>> not really clear to me.
>>>>> 
>>>>> I don't get the usecase. A hogged GPIO is per definition hogged.
>>>>> This sounds more like "initial settings" or something, which is another
>>>>> usecase altogether.
>>>> 
>>>> We do have one board where we have a pin (let's say GPIO14 of the bank
>>>> A) that enables a regulator that will provide VCC the bank B.
>>>> 
>>>> Now, both banks are handled by the same driver, but in order to have a
>>>> working output on the bank B, we do need to set GPIO14 as soon as
>>>> we're probed.
>>>> 
>>>> Just relying on the usual deferred probing introduces a circular
>>>> dependency between the gpio-regulator that needs to grab its GPIO from
>>>> a driver not there yet, and the gpio driver that needs to enable its
>>>> gpio-regulator.
>>> 
>>> I don't get it. According to what you said, the following order should
>>> go through IIUC:
>>> 
>>> 1) bank A is probed, gpio 14 is available
>>> 2) gpio-regulator is probed, acquires GPIO 14, regulator for Bank B is available
>>> 3) bank B is probed, grabs its regulator and turn it on, probes.
>>> 
>>> What am I missing?
>>> 
>>>> 
>>>> GPIO hogging needs to be the ideal solution for that, since we can
>>>> just enforce the GPIO14 value as the driver is probed, which provides
>>>> the guarantee that any driver using the bank B will actually drive the
>>>> GPIO it might use.
>>> 
>>> At this point I start wondering if such initial setup should not be
>>> the job of the bootloader? GPIO hogging ought to be simple and
>>> definitive, adding the possibility to have it just as an initial value
>>> would considerably complexify it. E.g. when is the gpio chip driver
>>> supposed to release the hogged descriptor in such a case?
>>> 
>> 
>> Do not count on the bootloader setting up anything. The trend is
>> for the bootloader to setup the minimal environment to load your kernel
>> and jump to it.
>> 
>> http://www.denx.de/wiki/pub/U-Boot/MiniSummitELCE2013/2013-ELCE-U-Boot-Falcon-Boot.pdf
> 
> Just wondering. :)
> 
> But yeah, there are some use-cases (such as this one or
> Linux-as-a-bootloader) for which this would not play nicely.
> 
>> 
>> 
>>> Note that if the multiple GPIO consumer feature we are planning goes
>>> through, you should be able to use both hogging *and* a regulator on
>>> the same GPIO and achieve what you want. The expectation of multiple
>>> consumers is that the board designers know what they are doing, and
>>> this case would certainly fit (chip hogs the line and doesn't touch
>>> the value after that, letting the regulator control it without any
>>> conflict afterwards), although it would of course be better to solve
>>> the issue through regular probing...
>> 
>> 
>> That’s why I was advocating a simple probing driver for all this.
>> Figure out a way for this driver to be probed first would be an easier
>> solution that what’s going on here.
> 
> Do you mean, a driver whose sole job is to probe other drivers in the
> right order? :/

$DEITY no :)

I mean instead of having the gpio hog in the gpio adapter driver, have
a gpio-hog driver, that’s using an undisclosed method to make sure that
it’s the first one to be probed afterwards.

We can probably do it using DT.

Or maybe using the proposed gpio hog method doesn’t cover every case.

I don’t know, that’s why we’re spending time talking about this :)

Regards

— pantelis

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

Re: [Patch v2 1/2] gpio: add GPIO hogging mechanism

[Alexandre Courbot]

On Thu, Dec 4, 2014 at 11:47 PM, Pantelis Antoniou
<panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org> wrote:
> Hi Alexandre,
>
>> On Dec 4, 2014, at 16:41 , Alexandre Courbot <gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>
>> On Thu, Dec 4, 2014 at 11:27 PM, Pantelis Antoniou
>> <panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org> wrote:
>>> Hi Alexandre,
>>>
>>> I tried to stay away while things are being fleshed out but…
>>>
>>>> On Dec 4, 2014, at 16:15 , Alexandre Courbot <gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>>
>>>> On Wed, Dec 3, 2014 at 1:12 AM, Maxime Ripard
>>>> <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>>>>> On Tue, Dec 02, 2014 at 03:29:46PM +0100, Linus Walleij wrote:
>>>>>> On Tue, Dec 2, 2014 at 3:13 PM, Alexandre Courbot <gnurou@gmail.com> wrote:
>>>>>>> On Tue, Dec 2, 2014 at 1:36 AM, Maxime Ripard
>>>>>>> <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>>>>>>
>>>>>>>> The only thing I'd like to have would be that the request here would
>>>>>>>> be non-exclusive, so that a later driver would still be allowed later
>>>>>>>> on to request that GPIO later on and manage it itself (ideally using
>>>>>>>> the usual gpiod_request function).
>>>>>>>
>>>>>>> Actually we have a plan (and I have some code too) to allow multiple
>>>>>>> consumers per GPIO. Although like Benoit I wonder why you would want
>>>>>>> to hog a GPIO and then request it properly later. Also, that probably
>>>>>>> means we should abandon the hog since it actively drives the line and
>>>>>>> would interfere with the late requested. How to do that correctly is
>>>>>>> not really clear to me.
>>>>>>
>>>>>> I don't get the usecase. A hogged GPIO is per definition hogged.
>>>>>> This sounds more like "initial settings" or something, which is another
>>>>>> usecase altogether.
>>>>>
>>>>> We do have one board where we have a pin (let's say GPIO14 of the bank
>>>>> A) that enables a regulator that will provide VCC the bank B.
>>>>>
>>>>> Now, both banks are handled by the same driver, but in order to have a
>>>>> working output on the bank B, we do need to set GPIO14 as soon as
>>>>> we're probed.
>>>>>
>>>>> Just relying on the usual deferred probing introduces a circular
>>>>> dependency between the gpio-regulator that needs to grab its GPIO from
>>>>> a driver not there yet, and the gpio driver that needs to enable its
>>>>> gpio-regulator.
>>>>
>>>> I don't get it. According to what you said, the following order should
>>>> go through IIUC:
>>>>
>>>> 1) bank A is probed, gpio 14 is available
>>>> 2) gpio-regulator is probed, acquires GPIO 14, regulator for Bank B is available
>>>> 3) bank B is probed, grabs its regulator and turn it on, probes.
>>>>
>>>> What am I missing?
>>>>
>>>>>
>>>>> GPIO hogging needs to be the ideal solution for that, since we can
>>>>> just enforce the GPIO14 value as the driver is probed, which provides
>>>>> the guarantee that any driver using the bank B will actually drive the
>>>>> GPIO it might use.
>>>>
>>>> At this point I start wondering if such initial setup should not be
>>>> the job of the bootloader? GPIO hogging ought to be simple and
>>>> definitive, adding the possibility to have it just as an initial value
>>>> would considerably complexify it. E.g. when is the gpio chip driver
>>>> supposed to release the hogged descriptor in such a case?
>>>>
>>>
>>> Do not count on the bootloader setting up anything. The trend is
>>> for the bootloader to setup the minimal environment to load your kernel
>>> and jump to it.
>>>
>>> http://www.denx.de/wiki/pub/U-Boot/MiniSummitELCE2013/2013-ELCE-U-Boot-Falcon-Boot.pdf
>>
>> Just wondering. :)
>>
>> But yeah, there are some use-cases (such as this one or
>> Linux-as-a-bootloader) for which this would not play nicely.
>>
>>>
>>>
>>>> Note that if the multiple GPIO consumer feature we are planning goes
>>>> through, you should be able to use both hogging *and* a regulator on
>>>> the same GPIO and achieve what you want. The expectation of multiple
>>>> consumers is that the board designers know what they are doing, and
>>>> this case would certainly fit (chip hogs the line and doesn't touch
>>>> the value after that, letting the regulator control it without any
>>>> conflict afterwards), although it would of course be better to solve
>>>> the issue through regular probing...
>>>
>>>
>>> That’s why I was advocating a simple probing driver for all this.
>>> Figure out a way for this driver to be probed first would be an easier
>>> solution that what’s going on here.
>>
>> Do you mean, a driver whose sole job is to probe other drivers in the
>> right order? :/
>
> $DEITY no :)
>
> I mean instead of having the gpio hog in the gpio adapter driver, have
> a gpio-hog driver, that’s using an undisclosed method to make sure that
> it’s the first one to be probed afterwards.

IIUC that would not solve this particular issue - here the GPIO
controller is both a provider and (indirect) consumer of a GPIO for
itself. If the hog is in a separate node, if would have to be probed
from inside the probe() function of the GPIO controller to do the job,
which would be the same effect as having the hogs directly under the
controller node, only with more hassle.

Again, IIUC. >_<
--
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

Re: [Patch v2 1/2] gpio: add GPIO hogging mechanism

[Pantelis Antoniou]

Hi Alexandre,

> On Dec 4, 2014, at 16:58 , Alexandre Courbot <gnurou@gmail.com> wrote:
> 
> On Thu, Dec 4, 2014 at 11:47 PM, Pantelis Antoniou
> <panto@antoniou-consulting.com> wrote:
>> Hi Alexandre,
>> 
>>> On Dec 4, 2014, at 16:41 , Alexandre Courbot <gnurou@gmail.com> wrote:
>>> 
>>> On Thu, Dec 4, 2014 at 11:27 PM, Pantelis Antoniou
>>> <panto@antoniou-consulting.com> wrote:
>>>> Hi Alexandre,
>>>> 
>>>> I tried to stay away while things are being fleshed out but…
>>>> 
>>>>> On Dec 4, 2014, at 16:15 , Alexandre Courbot <gnurou@gmail.com> wrote:
>>>>> 
>>>>> On Wed, Dec 3, 2014 at 1:12 AM, Maxime Ripard
>>>>> <maxime.ripard@free-electrons.com> wrote:
>>>>>> On Tue, Dec 02, 2014 at 03:29:46PM +0100, Linus Walleij wrote:
>>>>>>> On Tue, Dec 2, 2014 at 3:13 PM, Alexandre Courbot <gnurou@gmail.com> wrote:
>>>>>>>> On Tue, Dec 2, 2014 at 1:36 AM, Maxime Ripard
>>>>>>>> <maxime.ripard@free-electrons.com> wrote:
>>>>>>> 
>>>>>>>>> The only thing I'd like to have would be that the request here would
>>>>>>>>> be non-exclusive, so that a later driver would still be allowed later
>>>>>>>>> on to request that GPIO later on and manage it itself (ideally using
>>>>>>>>> the usual gpiod_request function).
>>>>>>>> 
>>>>>>>> Actually we have a plan (and I have some code too) to allow multiple
>>>>>>>> consumers per GPIO. Although like Benoit I wonder why you would want
>>>>>>>> to hog a GPIO and then request it properly later. Also, that probably
>>>>>>>> means we should abandon the hog since it actively drives the line and
>>>>>>>> would interfere with the late requested. How to do that correctly is
>>>>>>>> not really clear to me.
>>>>>>> 
>>>>>>> I don't get the usecase. A hogged GPIO is per definition hogged.
>>>>>>> This sounds more like "initial settings" or something, which is another
>>>>>>> usecase altogether.
>>>>>> 
>>>>>> We do have one board where we have a pin (let's say GPIO14 of the bank
>>>>>> A) that enables a regulator that will provide VCC the bank B.
>>>>>> 
>>>>>> Now, both banks are handled by the same driver, but in order to have a
>>>>>> working output on the bank B, we do need to set GPIO14 as soon as
>>>>>> we're probed.
>>>>>> 
>>>>>> Just relying on the usual deferred probing introduces a circular
>>>>>> dependency between the gpio-regulator that needs to grab its GPIO from
>>>>>> a driver not there yet, and the gpio driver that needs to enable its
>>>>>> gpio-regulator.
>>>>> 
>>>>> I don't get it. According to what you said, the following order should
>>>>> go through IIUC:
>>>>> 
>>>>> 1) bank A is probed, gpio 14 is available
>>>>> 2) gpio-regulator is probed, acquires GPIO 14, regulator for Bank B is available
>>>>> 3) bank B is probed, grabs its regulator and turn it on, probes.
>>>>> 
>>>>> What am I missing?
>>>>> 
>>>>>> 
>>>>>> GPIO hogging needs to be the ideal solution for that, since we can
>>>>>> just enforce the GPIO14 value as the driver is probed, which provides
>>>>>> the guarantee that any driver using the bank B will actually drive the
>>>>>> GPIO it might use.
>>>>> 
>>>>> At this point I start wondering if such initial setup should not be
>>>>> the job of the bootloader? GPIO hogging ought to be simple and
>>>>> definitive, adding the possibility to have it just as an initial value
>>>>> would considerably complexify it. E.g. when is the gpio chip driver
>>>>> supposed to release the hogged descriptor in such a case?
>>>>> 
>>>> 
>>>> Do not count on the bootloader setting up anything. The trend is
>>>> for the bootloader to setup the minimal environment to load your kernel
>>>> and jump to it.
>>>> 
>>>> http://www.denx.de/wiki/pub/U-Boot/MiniSummitELCE2013/2013-ELCE-U-Boot-Falcon-Boot.pdf
>>> 
>>> Just wondering. :)
>>> 
>>> But yeah, there are some use-cases (such as this one or
>>> Linux-as-a-bootloader) for which this would not play nicely.
>>> 
>>>> 
>>>> 
>>>>> Note that if the multiple GPIO consumer feature we are planning goes
>>>>> through, you should be able to use both hogging *and* a regulator on
>>>>> the same GPIO and achieve what you want. The expectation of multiple
>>>>> consumers is that the board designers know what they are doing, and
>>>>> this case would certainly fit (chip hogs the line and doesn't touch
>>>>> the value after that, letting the regulator control it without any
>>>>> conflict afterwards), although it would of course be better to solve
>>>>> the issue through regular probing...
>>>> 
>>>> 
>>>> That’s why I was advocating a simple probing driver for all this.
>>>> Figure out a way for this driver to be probed first would be an easier
>>>> solution that what’s going on here.
>>> 
>>> Do you mean, a driver whose sole job is to probe other drivers in the
>>> right order? :/
>> 
>> $DEITY no :)
>> 
>> I mean instead of having the gpio hog in the gpio adapter driver, have
>> a gpio-hog driver, that’s using an undisclosed method to make sure that
>> it’s the first one to be probed afterwards.
> 
> IIUC that would not solve this particular issue - here the GPIO
> controller is both a provider and (indirect) consumer of a GPIO for
> itself. If the hog is in a separate node, if would have to be probed
> from inside the probe() function of the GPIO controller to do the job,
> which would be the same effect as having the hogs directly under the
> controller node, only with more hassle.
> 
> Again, IIUC. >_<

If you had a way to specify the order of probing that would work no?
You don’t have to do it in the gpio-controller, you can do it in the
platform bus probe.

Regards

— Pantelis

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

Re: [Patch v2 1/2] gpio: add GPIO hogging mechanism

[Alexandre Courbot]

On Fri, Dec 5, 2014 at 12:02 AM, Pantelis Antoniou
<panto@antoniou-consulting.com> wrote:
> Hi Alexandre,
>
>> On Dec 4, 2014, at 16:58 , Alexandre Courbot <gnurou@gmail.com> wrote:
>>
>> On Thu, Dec 4, 2014 at 11:47 PM, Pantelis Antoniou
>> <panto@antoniou-consulting.com> wrote:
>>> Hi Alexandre,
>>>
>>>> On Dec 4, 2014, at 16:41 , Alexandre Courbot <gnurou@gmail.com> wrote:
>>>>
>>>> On Thu, Dec 4, 2014 at 11:27 PM, Pantelis Antoniou
>>>> <panto@antoniou-consulting.com> wrote:
>>>>> Hi Alexandre,
>>>>>
>>>>> I tried to stay away while things are being fleshed out but…
>>>>>
>>>>>> On Dec 4, 2014, at 16:15 , Alexandre Courbot <gnurou@gmail.com> wrote:
>>>>>>
>>>>>> On Wed, Dec 3, 2014 at 1:12 AM, Maxime Ripard
>>>>>> <maxime.ripard@free-electrons.com> wrote:
>>>>>>> On Tue, Dec 02, 2014 at 03:29:46PM +0100, Linus Walleij wrote:
>>>>>>>> On Tue, Dec 2, 2014 at 3:13 PM, Alexandre Courbot <gnurou@gmail.com> wrote:
>>>>>>>>> On Tue, Dec 2, 2014 at 1:36 AM, Maxime Ripard
>>>>>>>>> <maxime.ripard@free-electrons.com> wrote:
>>>>>>>>
>>>>>>>>>> The only thing I'd like to have would be that the request here would
>>>>>>>>>> be non-exclusive, so that a later driver would still be allowed later
>>>>>>>>>> on to request that GPIO later on and manage it itself (ideally using
>>>>>>>>>> the usual gpiod_request function).
>>>>>>>>>
>>>>>>>>> Actually we have a plan (and I have some code too) to allow multiple
>>>>>>>>> consumers per GPIO. Although like Benoit I wonder why you would want
>>>>>>>>> to hog a GPIO and then request it properly later. Also, that probably
>>>>>>>>> means we should abandon the hog since it actively drives the line and
>>>>>>>>> would interfere with the late requested. How to do that correctly is
>>>>>>>>> not really clear to me.
>>>>>>>>
>>>>>>>> I don't get the usecase. A hogged GPIO is per definition hogged.
>>>>>>>> This sounds more like "initial settings" or something, which is another
>>>>>>>> usecase altogether.
>>>>>>>
>>>>>>> We do have one board where we have a pin (let's say GPIO14 of the bank
>>>>>>> A) that enables a regulator that will provide VCC the bank B.
>>>>>>>
>>>>>>> Now, both banks are handled by the same driver, but in order to have a
>>>>>>> working output on the bank B, we do need to set GPIO14 as soon as
>>>>>>> we're probed.
>>>>>>>
>>>>>>> Just relying on the usual deferred probing introduces a circular
>>>>>>> dependency between the gpio-regulator that needs to grab its GPIO from
>>>>>>> a driver not there yet, and the gpio driver that needs to enable its
>>>>>>> gpio-regulator.
>>>>>>
>>>>>> I don't get it. According to what you said, the following order should
>>>>>> go through IIUC:
>>>>>>
>>>>>> 1) bank A is probed, gpio 14 is available
>>>>>> 2) gpio-regulator is probed, acquires GPIO 14, regulator for Bank B is available
>>>>>> 3) bank B is probed, grabs its regulator and turn it on, probes.
>>>>>>
>>>>>> What am I missing?
>>>>>>
>>>>>>>
>>>>>>> GPIO hogging needs to be the ideal solution for that, since we can
>>>>>>> just enforce the GPIO14 value as the driver is probed, which provides
>>>>>>> the guarantee that any driver using the bank B will actually drive the
>>>>>>> GPIO it might use.
>>>>>>
>>>>>> At this point I start wondering if such initial setup should not be
>>>>>> the job of the bootloader? GPIO hogging ought to be simple and
>>>>>> definitive, adding the possibility to have it just as an initial value
>>>>>> would considerably complexify it. E.g. when is the gpio chip driver
>>>>>> supposed to release the hogged descriptor in such a case?
>>>>>>
>>>>>
>>>>> Do not count on the bootloader setting up anything. The trend is
>>>>> for the bootloader to setup the minimal environment to load your kernel
>>>>> and jump to it.
>>>>>
>>>>> http://www.denx.de/wiki/pub/U-Boot/MiniSummitELCE2013/2013-ELCE-U-Boot-Falcon-Boot.pdf
>>>>
>>>> Just wondering. :)
>>>>
>>>> But yeah, there are some use-cases (such as this one or
>>>> Linux-as-a-bootloader) for which this would not play nicely.
>>>>
>>>>>
>>>>>
>>>>>> Note that if the multiple GPIO consumer feature we are planning goes
>>>>>> through, you should be able to use both hogging *and* a regulator on
>>>>>> the same GPIO and achieve what you want. The expectation of multiple
>>>>>> consumers is that the board designers know what they are doing, and
>>>>>> this case would certainly fit (chip hogs the line and doesn't touch
>>>>>> the value after that, letting the regulator control it without any
>>>>>> conflict afterwards), although it would of course be better to solve
>>>>>> the issue through regular probing...
>>>>>
>>>>>
>>>>> That’s why I was advocating a simple probing driver for all this.
>>>>> Figure out a way for this driver to be probed first would be an easier
>>>>> solution that what’s going on here.
>>>>
>>>> Do you mean, a driver whose sole job is to probe other drivers in the
>>>> right order? :/
>>>
>>> $DEITY no :)
>>>
>>> I mean instead of having the gpio hog in the gpio adapter driver, have
>>> a gpio-hog driver, that’s using an undisclosed method to make sure that
>>> it’s the first one to be probed afterwards.
>>
>> IIUC that would not solve this particular issue - here the GPIO
>> controller is both a provider and (indirect) consumer of a GPIO for
>> itself. If the hog is in a separate node, if would have to be probed
>> from inside the probe() function of the GPIO controller to do the job,
>> which would be the same effect as having the hogs directly under the
>> controller node, only with more hassle.
>>
>> Again, IIUC. >_<
>
> If you had a way to specify the order of probing that would work no?
> You don’t have to do it in the gpio-controller, you can do it in the
> platform bus probe.

A probe order that works would be the following:

1) bank A is probed, gpio 14 is available
2) gpio-regulator is probed, acquires GPIO 14, regulator for Bank B is available
3) bank B is probed, grabs its regulator and turn it on, probes.

The problem is that in the present case, 1) and 3) are the same
operation because both banks are the same device.

You could probably solve this by making bank A and bank B separate
devices, then even EPROBE_DEFER would allow you to probe everything in
the right order. But since the DT bindings are (supposedly) already
published this is likely not an option (?). And logically speaking,
these banks form one device anyway.

Re: [Patch v2 1/2] gpio: add GPIO hogging mechanism

[Pantelis Antoniou]

Hi Alexandre,

> On Dec 4, 2014, at 17:10 , Alexandre Courbot <gnurou@gmail.com> wrote:
> 
> On Fri, Dec 5, 2014 at 12:02 AM, Pantelis Antoniou
> <panto@antoniou-consulting.com> wrote:
>> Hi Alexandre,
>> 
>>> On Dec 4, 2014, at 16:58 , Alexandre Courbot <gnurou@gmail.com> wrote:
>>> 
>>> On Thu, Dec 4, 2014 at 11:47 PM, Pantelis Antoniou
>>> <panto@antoniou-consulting.com> wrote:
>>>> Hi Alexandre,
>>>> 
>>>>> On Dec 4, 2014, at 16:41 , Alexandre Courbot <gnurou@gmail.com> wrote:
>>>>> 
>>>>> On Thu, Dec 4, 2014 at 11:27 PM, Pantelis Antoniou
>>>>> <panto@antoniou-consulting.com> wrote:
>>>>>> Hi Alexandre,
>>>>>> 
>>>>>> I tried to stay away while things are being fleshed out but…
>>>>>> 
>>>>>>> On Dec 4, 2014, at 16:15 , Alexandre Courbot <gnurou@gmail.com> wrote:
>>>>>>> 
>>>>>>> On Wed, Dec 3, 2014 at 1:12 AM, Maxime Ripard
>>>>>>> <maxime.ripard@free-electrons.com> wrote:
>>>>>>>> On Tue, Dec 02, 2014 at 03:29:46PM +0100, Linus Walleij wrote:
>>>>>>>>> On Tue, Dec 2, 2014 at 3:13 PM, Alexandre Courbot <gnurou@gmail.com> wrote:
>>>>>>>>>> On Tue, Dec 2, 2014 at 1:36 AM, Maxime Ripard
>>>>>>>>>> <maxime.ripard@free-electrons.com> wrote:
>>>>>>>>> 
>>>>>>>>>>> The only thing I'd like to have would be that the request here would
>>>>>>>>>>> be non-exclusive, so that a later driver would still be allowed later
>>>>>>>>>>> on to request that GPIO later on and manage it itself (ideally using
>>>>>>>>>>> the usual gpiod_request function).
>>>>>>>>>> 
>>>>>>>>>> Actually we have a plan (and I have some code too) to allow multiple
>>>>>>>>>> consumers per GPIO. Although like Benoit I wonder why you would want
>>>>>>>>>> to hog a GPIO and then request it properly later. Also, that probably
>>>>>>>>>> means we should abandon the hog since it actively drives the line and
>>>>>>>>>> would interfere with the late requested. How to do that correctly is
>>>>>>>>>> not really clear to me.
>>>>>>>>> 
>>>>>>>>> I don't get the usecase. A hogged GPIO is per definition hogged.
>>>>>>>>> This sounds more like "initial settings" or something, which is another
>>>>>>>>> usecase altogether.
>>>>>>>> 
>>>>>>>> We do have one board where we have a pin (let's say GPIO14 of the bank
>>>>>>>> A) that enables a regulator that will provide VCC the bank B.
>>>>>>>> 
>>>>>>>> Now, both banks are handled by the same driver, but in order to have a
>>>>>>>> working output on the bank B, we do need to set GPIO14 as soon as
>>>>>>>> we're probed.
>>>>>>>> 
>>>>>>>> Just relying on the usual deferred probing introduces a circular
>>>>>>>> dependency between the gpio-regulator that needs to grab its GPIO from
>>>>>>>> a driver not there yet, and the gpio driver that needs to enable its
>>>>>>>> gpio-regulator.
>>>>>>> 
>>>>>>> I don't get it. According to what you said, the following order should
>>>>>>> go through IIUC:
>>>>>>> 
>>>>>>> 1) bank A is probed, gpio 14 is available
>>>>>>> 2) gpio-regulator is probed, acquires GPIO 14, regulator for Bank B is available
>>>>>>> 3) bank B is probed, grabs its regulator and turn it on, probes.
>>>>>>> 
>>>>>>> What am I missing?
>>>>>>> 
>>>>>>>> 
>>>>>>>> GPIO hogging needs to be the ideal solution for that, since we can
>>>>>>>> just enforce the GPIO14 value as the driver is probed, which provides
>>>>>>>> the guarantee that any driver using the bank B will actually drive the
>>>>>>>> GPIO it might use.
>>>>>>> 
>>>>>>> At this point I start wondering if such initial setup should not be
>>>>>>> the job of the bootloader? GPIO hogging ought to be simple and
>>>>>>> definitive, adding the possibility to have it just as an initial value
>>>>>>> would considerably complexify it. E.g. when is the gpio chip driver
>>>>>>> supposed to release the hogged descriptor in such a case?
>>>>>>> 
>>>>>> 
>>>>>> Do not count on the bootloader setting up anything. The trend is
>>>>>> for the bootloader to setup the minimal environment to load your kernel
>>>>>> and jump to it.
>>>>>> 
>>>>>> http://www.denx.de/wiki/pub/U-Boot/MiniSummitELCE2013/2013-ELCE-U-Boot-Falcon-Boot.pdf
>>>>> 
>>>>> Just wondering. :)
>>>>> 
>>>>> But yeah, there are some use-cases (such as this one or
>>>>> Linux-as-a-bootloader) for which this would not play nicely.
>>>>> 
>>>>>> 
>>>>>> 
>>>>>>> Note that if the multiple GPIO consumer feature we are planning goes
>>>>>>> through, you should be able to use both hogging *and* a regulator on
>>>>>>> the same GPIO and achieve what you want. The expectation of multiple
>>>>>>> consumers is that the board designers know what they are doing, and
>>>>>>> this case would certainly fit (chip hogs the line and doesn't touch
>>>>>>> the value after that, letting the regulator control it without any
>>>>>>> conflict afterwards), although it would of course be better to solve
>>>>>>> the issue through regular probing...
>>>>>> 
>>>>>> 
>>>>>> That’s why I was advocating a simple probing driver for all this.
>>>>>> Figure out a way for this driver to be probed first would be an easier
>>>>>> solution that what’s going on here.
>>>>> 
>>>>> Do you mean, a driver whose sole job is to probe other drivers in the
>>>>> right order? :/
>>>> 
>>>> $DEITY no :)
>>>> 
>>>> I mean instead of having the gpio hog in the gpio adapter driver, have
>>>> a gpio-hog driver, that’s using an undisclosed method to make sure that
>>>> it’s the first one to be probed afterwards.
>>> 
>>> IIUC that would not solve this particular issue - here the GPIO
>>> controller is both a provider and (indirect) consumer of a GPIO for
>>> itself. If the hog is in a separate node, if would have to be probed
>>> from inside the probe() function of the GPIO controller to do the job,
>>> which would be the same effect as having the hogs directly under the
>>> controller node, only with more hassle.
>>> 
>>> Again, IIUC. >_<
>> 
>> If you had a way to specify the order of probing that would work no?
>> You don’t have to do it in the gpio-controller, you can do it in the
>> platform bus probe.
> 
> A probe order that works would be the following:
> 
> 1) bank A is probed, gpio 14 is available
> 2) gpio-regulator is probed, acquires GPIO 14, regulator for Bank B is available
> 3) bank B is probed, grabs its regulator and turn it on, probes.
> 
> The problem is that in the present case, 1) and 3) are the same
> operation because both banks are the same device.
> 
> You could probably solve this by making bank A and bank B separate
> devices, then even EPROBE_DEFER would allow you to probe everything in
> the right order. But since the DT bindings are (supposedly) already
> published this is likely not an option (?). And logically speaking,
> these banks form one device anyway.

Let’s put that in a pseudo DT definition.

BANK_A: bank_a {
	compatible = “banka,gpio-controller”;
};

GPIO_REGULATOR: gpio-regulator {
	compatible = “gpio-regulator”;
	gpio = <&BANK_A 14>;
};

BANK_B: bank_b {
	compatible = “bankb,gpio-controller”;
	power-supply = <&GPIO_REGULATOR>;
};

The correct probe order is bank_a, gpio-regulator, bank_b correct?

It is possible to modify DTC to follow phandle references and spit out
a depends node that the platform bus probe can use.

For instance, 

	__depends__ {
		gpio-regulator = <&BANK_A>;
		bank_b = <&GPIO_REGULATOR>;
	};

Would that work for your case?

Regards

— Pantelis

Re: [Patch v2 1/2] gpio: add GPIO hogging mechanism

[Alexandre Courbot]

On Fri, Dec 5, 2014 at 12:22 AM, Pantelis Antoniou
<panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org> wrote:
> Hi Alexandre,
>
>> On Dec 4, 2014, at 17:10 , Alexandre Courbot <gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>
>> On Fri, Dec 5, 2014 at 12:02 AM, Pantelis Antoniou
>> <panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org> wrote:
>>> Hi Alexandre,
>>>
>>>> On Dec 4, 2014, at 16:58 , Alexandre Courbot <gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>>
>>>> On Thu, Dec 4, 2014 at 11:47 PM, Pantelis Antoniou
>>>> <panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org> wrote:
>>>>> Hi Alexandre,
>>>>>
>>>>>> On Dec 4, 2014, at 16:41 , Alexandre Courbot <gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>>>>
>>>>>> On Thu, Dec 4, 2014 at 11:27 PM, Pantelis Antoniou
>>>>>> <panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org> wrote:
>>>>>>> Hi Alexandre,
>>>>>>>
>>>>>>> I tried to stay away while things are being fleshed out but…
>>>>>>>
>>>>>>>> On Dec 4, 2014, at 16:15 , Alexandre Courbot <gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>>>>>>
>>>>>>>> On Wed, Dec 3, 2014 at 1:12 AM, Maxime Ripard
>>>>>>>> <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>>>>>>>>> On Tue, Dec 02, 2014 at 03:29:46PM +0100, Linus Walleij wrote:
>>>>>>>>>> On Tue, Dec 2, 2014 at 3:13 PM, Alexandre Courbot <gnurou@gmail.com> wrote:
>>>>>>>>>>> On Tue, Dec 2, 2014 at 1:36 AM, Maxime Ripard
>>>>>>>>>>> <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>>>>>>>>>>
>>>>>>>>>>>> The only thing I'd like to have would be that the request here would
>>>>>>>>>>>> be non-exclusive, so that a later driver would still be allowed later
>>>>>>>>>>>> on to request that GPIO later on and manage it itself (ideally using
>>>>>>>>>>>> the usual gpiod_request function).
>>>>>>>>>>>
>>>>>>>>>>> Actually we have a plan (and I have some code too) to allow multiple
>>>>>>>>>>> consumers per GPIO. Although like Benoit I wonder why you would want
>>>>>>>>>>> to hog a GPIO and then request it properly later. Also, that probably
>>>>>>>>>>> means we should abandon the hog since it actively drives the line and
>>>>>>>>>>> would interfere with the late requested. How to do that correctly is
>>>>>>>>>>> not really clear to me.
>>>>>>>>>>
>>>>>>>>>> I don't get the usecase. A hogged GPIO is per definition hogged.
>>>>>>>>>> This sounds more like "initial settings" or something, which is another
>>>>>>>>>> usecase altogether.
>>>>>>>>>
>>>>>>>>> We do have one board where we have a pin (let's say GPIO14 of the bank
>>>>>>>>> A) that enables a regulator that will provide VCC the bank B.
>>>>>>>>>
>>>>>>>>> Now, both banks are handled by the same driver, but in order to have a
>>>>>>>>> working output on the bank B, we do need to set GPIO14 as soon as
>>>>>>>>> we're probed.
>>>>>>>>>
>>>>>>>>> Just relying on the usual deferred probing introduces a circular
>>>>>>>>> dependency between the gpio-regulator that needs to grab its GPIO from
>>>>>>>>> a driver not there yet, and the gpio driver that needs to enable its
>>>>>>>>> gpio-regulator.
>>>>>>>>
>>>>>>>> I don't get it. According to what you said, the following order should
>>>>>>>> go through IIUC:
>>>>>>>>
>>>>>>>> 1) bank A is probed, gpio 14 is available
>>>>>>>> 2) gpio-regulator is probed, acquires GPIO 14, regulator for Bank B is available
>>>>>>>> 3) bank B is probed, grabs its regulator and turn it on, probes.
>>>>>>>>
>>>>>>>> What am I missing?
>>>>>>>>
>>>>>>>>>
>>>>>>>>> GPIO hogging needs to be the ideal solution for that, since we can
>>>>>>>>> just enforce the GPIO14 value as the driver is probed, which provides
>>>>>>>>> the guarantee that any driver using the bank B will actually drive the
>>>>>>>>> GPIO it might use.
>>>>>>>>
>>>>>>>> At this point I start wondering if such initial setup should not be
>>>>>>>> the job of the bootloader? GPIO hogging ought to be simple and
>>>>>>>> definitive, adding the possibility to have it just as an initial value
>>>>>>>> would considerably complexify it. E.g. when is the gpio chip driver
>>>>>>>> supposed to release the hogged descriptor in such a case?
>>>>>>>>
>>>>>>>
>>>>>>> Do not count on the bootloader setting up anything. The trend is
>>>>>>> for the bootloader to setup the minimal environment to load your kernel
>>>>>>> and jump to it.
>>>>>>>
>>>>>>> http://www.denx.de/wiki/pub/U-Boot/MiniSummitELCE2013/2013-ELCE-U-Boot-Falcon-Boot.pdf
>>>>>>
>>>>>> Just wondering. :)
>>>>>>
>>>>>> But yeah, there are some use-cases (such as this one or
>>>>>> Linux-as-a-bootloader) for which this would not play nicely.
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>> Note that if the multiple GPIO consumer feature we are planning goes
>>>>>>>> through, you should be able to use both hogging *and* a regulator on
>>>>>>>> the same GPIO and achieve what you want. The expectation of multiple
>>>>>>>> consumers is that the board designers know what they are doing, and
>>>>>>>> this case would certainly fit (chip hogs the line and doesn't touch
>>>>>>>> the value after that, letting the regulator control it without any
>>>>>>>> conflict afterwards), although it would of course be better to solve
>>>>>>>> the issue through regular probing...
>>>>>>>
>>>>>>>
>>>>>>> That’s why I was advocating a simple probing driver for all this.
>>>>>>> Figure out a way for this driver to be probed first would be an easier
>>>>>>> solution that what’s going on here.
>>>>>>
>>>>>> Do you mean, a driver whose sole job is to probe other drivers in the
>>>>>> right order? :/
>>>>>
>>>>> $DEITY no :)
>>>>>
>>>>> I mean instead of having the gpio hog in the gpio adapter driver, have
>>>>> a gpio-hog driver, that’s using an undisclosed method to make sure that
>>>>> it’s the first one to be probed afterwards.
>>>>
>>>> IIUC that would not solve this particular issue - here the GPIO
>>>> controller is both a provider and (indirect) consumer of a GPIO for
>>>> itself. If the hog is in a separate node, if would have to be probed
>>>> from inside the probe() function of the GPIO controller to do the job,
>>>> which would be the same effect as having the hogs directly under the
>>>> controller node, only with more hassle.
>>>>
>>>> Again, IIUC. >_<
>>>
>>> If you had a way to specify the order of probing that would work no?
>>> You don’t have to do it in the gpio-controller, you can do it in the
>>> platform bus probe.
>>
>> A probe order that works would be the following:
>>
>> 1) bank A is probed, gpio 14 is available
>> 2) gpio-regulator is probed, acquires GPIO 14, regulator for Bank B is available
>> 3) bank B is probed, grabs its regulator and turn it on, probes.
>>
>> The problem is that in the present case, 1) and 3) are the same
>> operation because both banks are the same device.
>>
>> You could probably solve this by making bank A and bank B separate
>> devices, then even EPROBE_DEFER would allow you to probe everything in
>> the right order. But since the DT bindings are (supposedly) already
>> published this is likely not an option (?). And logically speaking,
>> these banks form one device anyway.
>
> Let’s put that in a pseudo DT definition.
>
> BANK_A: bank_a {
>         compatible = “banka,gpio-controller”;
> };
>
> GPIO_REGULATOR: gpio-regulator {
>         compatible = “gpio-regulator”;
>         gpio = <&BANK_A 14>;
> };
>
> BANK_B: bank_b {
>         compatible = “bankb,gpio-controller”;
>         power-supply = <&GPIO_REGULATOR>;
> };
>
> The correct probe order is bank_a, gpio-regulator, bank_b correct?

Right. And actually this would work without any DT annotation, altough
it might take a few more loops to get to the right order.

But as Maxime confirmed later, his device is more like this:

/* Contains both banks A and B */
gpio: gpio {
        compatible = "foo,gpio-controller";
        bankb-supply = <&GPIO_REGULATOR>'
};

GPIO_REGULATOR: gpio-regulator {
        compatible = “gpio-regulator”;
        gpio = <&gpio 14>;
};

And here there is nothing we can do, I'm afraid, without splitting the
banks into their own nodes first.
--
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

Re: [Patch v2 1/2] gpio: add GPIO hogging mechanism

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2547 bytes --]

Hi,

On Thu, Dec 04, 2014 at 11:15:38PM +0900, Alexandre Courbot wrote:
> On Wed, Dec 3, 2014 at 1:12 AM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > On Tue, Dec 02, 2014 at 03:29:46PM +0100, Linus Walleij wrote:
> >> On Tue, Dec 2, 2014 at 3:13 PM, Alexandre Courbot <gnurou@gmail.com> wrote:
> >> > On Tue, Dec 2, 2014 at 1:36 AM, Maxime Ripard
> >> > <maxime.ripard@free-electrons.com> wrote:
> >>
> >> >> The only thing I'd like to have would be that the request here would
> >> >> be non-exclusive, so that a later driver would still be allowed later
> >> >> on to request that GPIO later on and manage it itself (ideally using
> >> >> the usual gpiod_request function).
> >> >
> >> > Actually we have a plan (and I have some code too) to allow multiple
> >> > consumers per GPIO. Although like Benoit I wonder why you would want
> >> > to hog a GPIO and then request it properly later. Also, that probably
> >> > means we should abandon the hog since it actively drives the line and
> >> > would interfere with the late requested. How to do that correctly is
> >> > not really clear to me.
> >>
> >> I don't get the usecase. A hogged GPIO is per definition hogged.
> >> This sounds more like "initial settings" or something, which is another
> >> usecase altogether.
> >
> > We do have one board where we have a pin (let's say GPIO14 of the bank
> > A) that enables a regulator that will provide VCC the bank B.
> >
> > Now, both banks are handled by the same driver, but in order to have a
> > working output on the bank B, we do need to set GPIO14 as soon as
> > we're probed.
> >
> > Just relying on the usual deferred probing introduces a circular
> > dependency between the gpio-regulator that needs to grab its GPIO from
> > a driver not there yet, and the gpio driver that needs to enable its
> > gpio-regulator.
> 
> I don't get it. According to what you said, the following order should
> go through IIUC:
> 
> 1) bank A is probed, gpio 14 is available
> 2) gpio-regulator is probed, acquires GPIO 14, regulator for Bank B is available
> 3) bank B is probed, grabs its regulator and turn it on, probes.
> 
> What am I missing?

It would be true if bank A and B were exposed through different
drivers (or at least different instances of the same driver), which is
not the case.

In our case, banks A and B are handled by the same instance.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Patch v2 1/2] gpio: add GPIO hogging mechanism

[Alexandre Courbot]

On Thu, Dec 4, 2014 at 11:27 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi,
>
> On Thu, Dec 04, 2014 at 11:15:38PM +0900, Alexandre Courbot wrote:
>> On Wed, Dec 3, 2014 at 1:12 AM, Maxime Ripard
>> <maxime.ripard@free-electrons.com> wrote:
>> > On Tue, Dec 02, 2014 at 03:29:46PM +0100, Linus Walleij wrote:
>> >> On Tue, Dec 2, 2014 at 3:13 PM, Alexandre Courbot <gnurou@gmail.com> wrote:
>> >> > On Tue, Dec 2, 2014 at 1:36 AM, Maxime Ripard
>> >> > <maxime.ripard@free-electrons.com> wrote:
>> >>
>> >> >> The only thing I'd like to have would be that the request here would
>> >> >> be non-exclusive, so that a later driver would still be allowed later
>> >> >> on to request that GPIO later on and manage it itself (ideally using
>> >> >> the usual gpiod_request function).
>> >> >
>> >> > Actually we have a plan (and I have some code too) to allow multiple
>> >> > consumers per GPIO. Although like Benoit I wonder why you would want
>> >> > to hog a GPIO and then request it properly later. Also, that probably
>> >> > means we should abandon the hog since it actively drives the line and
>> >> > would interfere with the late requested. How to do that correctly is
>> >> > not really clear to me.
>> >>
>> >> I don't get the usecase. A hogged GPIO is per definition hogged.
>> >> This sounds more like "initial settings" or something, which is another
>> >> usecase altogether.
>> >
>> > We do have one board where we have a pin (let's say GPIO14 of the bank
>> > A) that enables a regulator that will provide VCC the bank B.
>> >
>> > Now, both banks are handled by the same driver, but in order to have a
>> > working output on the bank B, we do need to set GPIO14 as soon as
>> > we're probed.
>> >
>> > Just relying on the usual deferred probing introduces a circular
>> > dependency between the gpio-regulator that needs to grab its GPIO from
>> > a driver not there yet, and the gpio driver that needs to enable its
>> > gpio-regulator.
>>
>> I don't get it. According to what you said, the following order should
>> go through IIUC:
>>
>> 1) bank A is probed, gpio 14 is available
>> 2) gpio-regulator is probed, acquires GPIO 14, regulator for Bank B is available
>> 3) bank B is probed, grabs its regulator and turn it on, probes.
>>
>> What am I missing?
>
> It would be true if bank A and B were exposed through different
> drivers (or at least different instances of the same driver), which is
> not the case.
>
> In our case, banks A and B are handled by the same instance.

Ok, so both banks A and B are part of the same device/DT node. Now I
think I understand the issue. You need to hog the pin so that bank B
will work right after the device is probed.

But you will still have the problem that the regulator device will
*not* be available when your device is probed, so you cannot call
regulator_get() for bank B anyway. I guess your only choice is to hog
that pin and leave it active ad vitam eternam.

Re: [Patch v2 1/2] gpio: add GPIO hogging mechanism

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 3399 bytes --]

On Thu, Dec 04, 2014 at 11:49:19PM +0900, Alexandre Courbot wrote:
> On Thu, Dec 4, 2014 at 11:27 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > Hi,
> >
> > On Thu, Dec 04, 2014 at 11:15:38PM +0900, Alexandre Courbot wrote:
> >> On Wed, Dec 3, 2014 at 1:12 AM, Maxime Ripard
> >> <maxime.ripard@free-electrons.com> wrote:
> >> > On Tue, Dec 02, 2014 at 03:29:46PM +0100, Linus Walleij wrote:
> >> >> On Tue, Dec 2, 2014 at 3:13 PM, Alexandre Courbot <gnurou@gmail.com> wrote:
> >> >> > On Tue, Dec 2, 2014 at 1:36 AM, Maxime Ripard
> >> >> > <maxime.ripard@free-electrons.com> wrote:
> >> >>
> >> >> >> The only thing I'd like to have would be that the request here would
> >> >> >> be non-exclusive, so that a later driver would still be allowed later
> >> >> >> on to request that GPIO later on and manage it itself (ideally using
> >> >> >> the usual gpiod_request function).
> >> >> >
> >> >> > Actually we have a plan (and I have some code too) to allow multiple
> >> >> > consumers per GPIO. Although like Benoit I wonder why you would want
> >> >> > to hog a GPIO and then request it properly later. Also, that probably
> >> >> > means we should abandon the hog since it actively drives the line and
> >> >> > would interfere with the late requested. How to do that correctly is
> >> >> > not really clear to me.
> >> >>
> >> >> I don't get the usecase. A hogged GPIO is per definition hogged.
> >> >> This sounds more like "initial settings" or something, which is another
> >> >> usecase altogether.
> >> >
> >> > We do have one board where we have a pin (let's say GPIO14 of the bank
> >> > A) that enables a regulator that will provide VCC the bank B.
> >> >
> >> > Now, both banks are handled by the same driver, but in order to have a
> >> > working output on the bank B, we do need to set GPIO14 as soon as
> >> > we're probed.
> >> >
> >> > Just relying on the usual deferred probing introduces a circular
> >> > dependency between the gpio-regulator that needs to grab its GPIO from
> >> > a driver not there yet, and the gpio driver that needs to enable its
> >> > gpio-regulator.
> >>
> >> I don't get it. According to what you said, the following order should
> >> go through IIUC:
> >>
> >> 1) bank A is probed, gpio 14 is available
> >> 2) gpio-regulator is probed, acquires GPIO 14, regulator for Bank B is available
> >> 3) bank B is probed, grabs its regulator and turn it on, probes.
> >>
> >> What am I missing?
> >
> > It would be true if bank A and B were exposed through different
> > drivers (or at least different instances of the same driver), which is
> > not the case.
> >
> > In our case, banks A and B are handled by the same instance.
> 
> Ok, so both banks A and B are part of the same device/DT node. Now I
> think I understand the issue. You need to hog the pin so that bank B
> will work right after the device is probed.

Exactly.

> But you will still have the problem that the regulator device will
> *not* be available when your device is probed, so you cannot call
> regulator_get() for bank B anyway. I guess your only choice is to hog
> that pin and leave it active ad vitam eternam.

Hmmm, indeed.

I'll stop boring you with this then :)

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Patch v2 1/2] gpio: add GPIO hogging mechanism

[Alexandre Courbot]

On Fri, Dec 5, 2014 at 7:24 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Thu, Dec 04, 2014 at 11:49:19PM +0900, Alexandre Courbot wrote:
>> On Thu, Dec 4, 2014 at 11:27 PM, Maxime Ripard
>> <maxime.ripard@free-electrons.com> wrote:
>> > Hi,
>> >
>> > On Thu, Dec 04, 2014 at 11:15:38PM +0900, Alexandre Courbot wrote:
>> >> On Wed, Dec 3, 2014 at 1:12 AM, Maxime Ripard
>> >> <maxime.ripard@free-electrons.com> wrote:
>> >> > On Tue, Dec 02, 2014 at 03:29:46PM +0100, Linus Walleij wrote:
>> >> >> On Tue, Dec 2, 2014 at 3:13 PM, Alexandre Courbot <gnurou@gmail.com> wrote:
>> >> >> > On Tue, Dec 2, 2014 at 1:36 AM, Maxime Ripard
>> >> >> > <maxime.ripard@free-electrons.com> wrote:
>> >> >>
>> >> >> >> The only thing I'd like to have would be that the request here would
>> >> >> >> be non-exclusive, so that a later driver would still be allowed later
>> >> >> >> on to request that GPIO later on and manage it itself (ideally using
>> >> >> >> the usual gpiod_request function).
>> >> >> >
>> >> >> > Actually we have a plan (and I have some code too) to allow multiple
>> >> >> > consumers per GPIO. Although like Benoit I wonder why you would want
>> >> >> > to hog a GPIO and then request it properly later. Also, that probably
>> >> >> > means we should abandon the hog since it actively drives the line and
>> >> >> > would interfere with the late requested. How to do that correctly is
>> >> >> > not really clear to me.
>> >> >>
>> >> >> I don't get the usecase. A hogged GPIO is per definition hogged.
>> >> >> This sounds more like "initial settings" or something, which is another
>> >> >> usecase altogether.
>> >> >
>> >> > We do have one board where we have a pin (let's say GPIO14 of the bank
>> >> > A) that enables a regulator that will provide VCC the bank B.
>> >> >
>> >> > Now, both banks are handled by the same driver, but in order to have a
>> >> > working output on the bank B, we do need to set GPIO14 as soon as
>> >> > we're probed.
>> >> >
>> >> > Just relying on the usual deferred probing introduces a circular
>> >> > dependency between the gpio-regulator that needs to grab its GPIO from
>> >> > a driver not there yet, and the gpio driver that needs to enable its
>> >> > gpio-regulator.
>> >>
>> >> I don't get it. According to what you said, the following order should
>> >> go through IIUC:
>> >>
>> >> 1) bank A is probed, gpio 14 is available
>> >> 2) gpio-regulator is probed, acquires GPIO 14, regulator for Bank B is available
>> >> 3) bank B is probed, grabs its regulator and turn it on, probes.
>> >>
>> >> What am I missing?
>> >
>> > It would be true if bank A and B were exposed through different
>> > drivers (or at least different instances of the same driver), which is
>> > not the case.
>> >
>> > In our case, banks A and B are handled by the same instance.
>>
>> Ok, so both banks A and B are part of the same device/DT node. Now I
>> think I understand the issue. You need to hog the pin so that bank B
>> will work right after the device is probed.
>
> Exactly.
>
>> But you will still have the problem that the regulator device will
>> *not* be available when your device is probed, so you cannot call
>> regulator_get() for bank B anyway. I guess your only choice is to hog
>> that pin and leave it active ad vitam eternam.
>
> Hmmm, indeed.
>
> I'll stop boring you with this then :)

Please *keep* bothering us with any doubt you may have until they are
all cleared and you are sure this feature will be useful to you.
Especially since we are designing DT bindings that will have to be
carried over forever. We really want to get them right, and need input
of potential users for that.

Having a few design arguments is a small thing compared to the hassle
of having to work with unadapted features and bindings.

Re: [Patch v2 1/2] gpio: add GPIO hogging mechanism

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 4938 bytes --]

On Sat, Dec 06, 2014 at 09:08:36PM +0900, Alexandre Courbot wrote:
> On Fri, Dec 5, 2014 at 7:24 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > On Thu, Dec 04, 2014 at 11:49:19PM +0900, Alexandre Courbot wrote:
> >> On Thu, Dec 4, 2014 at 11:27 PM, Maxime Ripard
> >> <maxime.ripard@free-electrons.com> wrote:
> >> > Hi,
> >> >
> >> > On Thu, Dec 04, 2014 at 11:15:38PM +0900, Alexandre Courbot wrote:
> >> >> On Wed, Dec 3, 2014 at 1:12 AM, Maxime Ripard
> >> >> <maxime.ripard@free-electrons.com> wrote:
> >> >> > On Tue, Dec 02, 2014 at 03:29:46PM +0100, Linus Walleij wrote:
> >> >> >> On Tue, Dec 2, 2014 at 3:13 PM, Alexandre Courbot <gnurou@gmail.com> wrote:
> >> >> >> > On Tue, Dec 2, 2014 at 1:36 AM, Maxime Ripard
> >> >> >> > <maxime.ripard@free-electrons.com> wrote:
> >> >> >>
> >> >> >> >> The only thing I'd like to have would be that the request here would
> >> >> >> >> be non-exclusive, so that a later driver would still be allowed later
> >> >> >> >> on to request that GPIO later on and manage it itself (ideally using
> >> >> >> >> the usual gpiod_request function).
> >> >> >> >
> >> >> >> > Actually we have a plan (and I have some code too) to allow multiple
> >> >> >> > consumers per GPIO. Although like Benoit I wonder why you would want
> >> >> >> > to hog a GPIO and then request it properly later. Also, that probably
> >> >> >> > means we should abandon the hog since it actively drives the line and
> >> >> >> > would interfere with the late requested. How to do that correctly is
> >> >> >> > not really clear to me.
> >> >> >>
> >> >> >> I don't get the usecase. A hogged GPIO is per definition hogged.
> >> >> >> This sounds more like "initial settings" or something, which is another
> >> >> >> usecase altogether.
> >> >> >
> >> >> > We do have one board where we have a pin (let's say GPIO14 of the bank
> >> >> > A) that enables a regulator that will provide VCC the bank B.
> >> >> >
> >> >> > Now, both banks are handled by the same driver, but in order to have a
> >> >> > working output on the bank B, we do need to set GPIO14 as soon as
> >> >> > we're probed.
> >> >> >
> >> >> > Just relying on the usual deferred probing introduces a circular
> >> >> > dependency between the gpio-regulator that needs to grab its GPIO from
> >> >> > a driver not there yet, and the gpio driver that needs to enable its
> >> >> > gpio-regulator.
> >> >>
> >> >> I don't get it. According to what you said, the following order should
> >> >> go through IIUC:
> >> >>
> >> >> 1) bank A is probed, gpio 14 is available
> >> >> 2) gpio-regulator is probed, acquires GPIO 14, regulator for Bank B is available
> >> >> 3) bank B is probed, grabs its regulator and turn it on, probes.
> >> >>
> >> >> What am I missing?
> >> >
> >> > It would be true if bank A and B were exposed through different
> >> > drivers (or at least different instances of the same driver), which is
> >> > not the case.
> >> >
> >> > In our case, banks A and B are handled by the same instance.
> >>
> >> Ok, so both banks A and B are part of the same device/DT node. Now I
> >> think I understand the issue. You need to hog the pin so that bank B
> >> will work right after the device is probed.
> >
> > Exactly.
> >
> >> But you will still have the problem that the regulator device will
> >> *not* be available when your device is probed, so you cannot call
> >> regulator_get() for bank B anyway. I guess your only choice is to hog
> >> that pin and leave it active ad vitam eternam.
> >
> > Hmmm, indeed.
> >
> > I'll stop boring you with this then :)
> 
> Please *keep* bothering us with any doubt you may have until they are
> all cleared and you are sure this feature will be useful to you.
> Especially since we are designing DT bindings that will have to be
> carried over forever. We really want to get them right, and need input
> of potential users for that.
> 
> Having a few design arguments is a small thing compared to the hassle
> of having to work with unadapted features and bindings.

Ok. What I had in mind when I first thought about it was to set GPIO
as hogged through the GPIO flags, and then have a dumb GPIO driver
like Pantelis was suggesting.

I don't know if hogged would still be the right term, but we could
have that flag that would just allow the value to be set through
gpio_request init value, and deny/cache any subsequent change through
gpio_set_value.

gpio_request with this flag would never return EPROBE_DEFER, and just
cache the value to be set for when the driver comes in.

We could enforce driver-less GPIOs through that dumb driver, and we
would still be able to break weird dependency chains that end up in
circle.

That's just a thought though.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Patch v2 1/2] gpio: add GPIO hogging mechanism

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1910 bytes --]

Hi again,

It looks like I missed some part of it.

On Thu, Dec 04, 2014 at 11:15:38PM +0900, Alexandre Courbot wrote:
> > GPIO hogging needs to be the ideal solution for that, since we can
> > just enforce the GPIO14 value as the driver is probed, which provides
> > the guarantee that any driver using the bank B will actually drive the
> > GPIO it might use.
> 
> At this point I start wondering if such initial setup should not be
> the job of the bootloader? GPIO hogging ought to be simple and
> definitive, adding the possibility to have it just as an initial value
> would considerably complexify it. E.g. when is the gpio chip driver
> supposed to release the hogged descriptor in such a case?

Relying on the bootloader for such trivial (and critical) things may
not work at all. You don't always have the option to replace it,
either because you physically can't, or just because you don't have
any alternative.

I agree that in general this is something that should go in the
bootloader, but we should have a way to deal with the case where it's
not done.

> Note that if the multiple GPIO consumer feature we are planning goes
> through, you should be able to use both hogging *and* a regulator on
> the same GPIO and achieve what you want. The expectation of multiple
> consumers is that the board designers know what they are doing, and
> this case would certainly fit (chip hogs the line and doesn't touch
> the value after that, letting the regulator control it without any
> conflict afterwards), although it would of course be better to solve
> the issue through regular probing...

If such an effort is on-going, then I'm totally fine waiting for it
and leaving that outside the hogging mechanism. As long as it works,
I'm happy.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Patch v2 1/2] gpio: add GPIO hogging mechanism

[Alexandre Courbot]

On Thu, Dec 4, 2014 at 11:52 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi again,
>
> It looks like I missed some part of it.
>
> On Thu, Dec 04, 2014 at 11:15:38PM +0900, Alexandre Courbot wrote:
>> > GPIO hogging needs to be the ideal solution for that, since we can
>> > just enforce the GPIO14 value as the driver is probed, which provides
>> > the guarantee that any driver using the bank B will actually drive the
>> > GPIO it might use.
>>
>> At this point I start wondering if such initial setup should not be
>> the job of the bootloader? GPIO hogging ought to be simple and
>> definitive, adding the possibility to have it just as an initial value
>> would considerably complexify it. E.g. when is the gpio chip driver
>> supposed to release the hogged descriptor in such a case?
>
> Relying on the bootloader for such trivial (and critical) things may
> not work at all. You don't always have the option to replace it,
> either because you physically can't, or just because you don't have
> any alternative.
>
> I agree that in general this is something that should go in the
> bootloader, but we should have a way to deal with the case where it's
> not done.
>
>> Note that if the multiple GPIO consumer feature we are planning goes
>> through, you should be able to use both hogging *and* a regulator on
>> the same GPIO and achieve what you want. The expectation of multiple
>> consumers is that the board designers know what they are doing, and
>> this case would certainly fit (chip hogs the line and doesn't touch
>> the value after that, letting the regulator control it without any
>> conflict afterwards), although it would of course be better to solve
>> the issue through regular probing...
>
> If such an effort is on-going, then I'm totally fine waiting for it
> and leaving that outside the hogging mechanism. As long as it works,
> I'm happy.

Ok. I just want to wait until the next -rc1 to make sure that the
large GPIO array removal patch (on which the multiple consumers
feature depend) did not break anything important, then I will submit
it.

Re: [Patch v2 1/2] gpio: add GPIO hogging mechanism

[Linus Walleij]

On Tue, Dec 2, 2014 at 5:12 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Tue, Dec 02, 2014 at 03:29:46PM +0100, Linus Walleij wrote:
>> On Tue, Dec 2, 2014 at 3:13 PM, Alexandre Courbot <gnurou@gmail.com> wrote:
>> > On Tue, Dec 2, 2014 at 1:36 AM, Maxime Ripard
>> > <maxime.ripard@free-electrons.com> wrote:
>>
>> >> The only thing I'd like to have would be that the request here would
>> >> be non-exclusive, so that a later driver would still be allowed later
>> >> on to request that GPIO later on and manage it itself (ideally using
>> >> the usual gpiod_request function).
>> >
>> > Actually we have a plan (and I have some code too) to allow multiple
>> > consumers per GPIO. Although like Benoit I wonder why you would want
>> > to hog a GPIO and then request it properly later. Also, that probably
>> > means we should abandon the hog since it actively drives the line and
>> > would interfere with the late requested. How to do that correctly is
>> > not really clear to me.
>>
>> I don't get the usecase. A hogged GPIO is per definition hogged.
>> This sounds more like "initial settings" or something, which is another
>> usecase altogether.
>
> We do have one board where we have a pin (let's say GPIO14 of the bank
> A) that enables a regulator that will provide VCC the bank B.
>
> Now, both banks are handled by the same driver, but in order to have a
> working output on the bank B, we do need to set GPIO14 as soon as
> we're probed.
>
> Just relying on the usual deferred probing introduces a circular
> dependency between the gpio-regulator that needs to grab its GPIO from
> a driver not there yet, and the gpio driver that needs to enable its
> gpio-regulator.
>
> GPIO hogging needs to be the ideal solution for that, since we can
> just enforce the GPIO14 value as the driver is probed, which provides
> the guarantee that any driver using the bank B will actually drive the
> GPIO it might use.
>
> However, an exclusive request will prevent any representation of this
> as a regulator, which sounds a bit weird, since it really is just
> that.

Well that's elegant...

I think it's a hog in this case though, not a GPIO regulator,
definately not both. But let's check Mark's opinion on this.

Yours,
Linus Walleij

Re: [Patch v2 1/2] gpio: add GPIO hogging mechanism

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 573 bytes --]

On Mon, Jan 12, 2015 at 10:40:04AM +0100, Linus Walleij wrote:
> On Tue, Dec 2, 2014 at 5:12 PM, Maxime Ripard

> > We do have one board where we have a pin (let's say GPIO14 of the bank
> > A) that enables a regulator that will provide VCC the bank B.

> I think it's a hog in this case though, not a GPIO regulator,
> definately not both. But let's check Mark's opinion on this.

I'm lacking some context here but my first thought is that if VCC might
be supplied by some other thing we need to use a regulator since
otherwise we can't substitute in the other regulator.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [Patch v2 1/2] gpio: add GPIO hogging mechanism

[Benoit Parrot]

Alexandre Courbot <gnurou@gmail.com> wrote on Fri [2014-Nov-28 16:30:01 +0900]:
> On Fri, Nov 21, 2014 at 8:54 AM, Benoit Parrot <bparrot@ti.com> wrote:
> > Based on Boris Brezillion's work this is a reworked patch
> > of his initial GPIO hogging mechanism.
> > This patch provides a way to initally configure specific GPIO
> > when the gpio controller is probed.
> >
> > The actual DT scanning to collect the GPIO specific data is performed
> > as part of the gpiochip_add().
> >
> > The purpose of this is to allows specific GPIOs to be configured
> > without any driver specific code.
> > This particularly useful because board design are getting
> > increasingly complex and given SoC pins can now have upward
> > of 10 mux values a lot of connections are now dependent on
> > external IO muxes to switch various modes and combination.
> >
> > Specific drivers should not necessarily need to be aware of
> > what accounts to a specific board implementation. This board level
> > "description" should be best kept as part of the dts file.
> >
> > Signed-off-by: Benoit Parrot <bparrot@ti.com>
> > ---
> > Changes since v1:
> >  * Refactor the gpio-hog mechanism as private functions meant to
> >    be to invoked from of_gpiochip_add().
> >
> >  drivers/gpio/gpiolib-of.c | 188 ++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 188 insertions(+)
> >
> > diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
> > index 604dbe6..3caed76 100644
> > --- a/drivers/gpio/gpiolib-of.c
> > +++ b/drivers/gpio/gpiolib-of.c
> > @@ -22,6 +22,7 @@
> >  #include <linux/of_gpio.h>
> >  #include <linux/pinctrl/pinctrl.h>
> >  #include <linux/slab.h>
> > +#include <linux/gpio/machine.h>
> >
> >  #include "gpiolib.h"
> >
> > @@ -111,6 +112,184 @@ int of_get_named_gpio_flags(struct device_node *np, const char *list_name,
> >  EXPORT_SYMBOL(of_get_named_gpio_flags);
> >
> >  /**
> > + * of_get_gpio_hog() - Get a GPIO hog descriptor, names and flags for GPIO API
> > + * @np:                device node to get GPIO from
> > + * @name:      GPIO line name
> > + * @flags:     a flags pointer to fill in
> > + *
> > + * Returns GPIO descriptor to use with Linux GPIO API, or one of the errno
> > + * value on the error condition.
> > + */
> > +
> > +static struct gpio_desc *of_get_gpio_hog(struct device_node *np,
> > +                                 const char **name,
> > +                                 enum gpio_lookup_flags *lflags,
> > +                                 enum gpiod_flags *dflags)
> > +{
> > +       struct device_node *chip_np;
> > +       enum of_gpio_flags xlate_flags;
> > +       struct gpio_desc *desc;
> > +       struct gg_data gg_data = {
> > +               .flags = &xlate_flags,
> > +               .out_gpio = NULL,
> > +       };
> > +       u32 tmp;
> > +       int i, in, outlo, outhi;
> > +       int ret;
> > +
> > +       if (!np)
> > +               return ERR_PTR(-EINVAL);
> 
> This function is being called from a perfectly mastered context in
> this file, so maybe we can avoid this check.

You are correct, I will change this.
> 
> > +
> > +       chip_np = np->parent;
> > +       if (!chip_np) {
> > +               desc = ERR_PTR(-EINVAL);
> > +               goto out;
> > +       }
> > +
> > +       if (!lflags || !dflags) {
> > +               desc = ERR_PTR(-EINVAL);
> > +               goto out;
> > +       }
> 
> Same for this one.
Agreed.
> 
> > +
> > +       *lflags = 0;
> > +       *dflags = 0;
> > +       in = 0;
> > +       outlo = 0;
> > +       outhi = 0;
> > +
> > +       ret = of_property_read_u32(chip_np, "#gpio-cells", &tmp);
> > +       if (ret) {
> > +               desc = ERR_PTR(ret);
> > +               goto out;
> 
> Please use "return ERR_PTR(ret);" directly, since you do absolutely no
> cleanup in out:. Same remark everywhere it applies.

Agreed.

> 
> > +       }
> > +
> > +       if (tmp > MAX_PHANDLE_ARGS) {
> > +               desc = ERR_PTR(-EINVAL);
> > +               goto out;
> > +       }
> > +
> > +       gg_data.gpiospec.args_count = tmp;
> > +       gg_data.gpiospec.np = chip_np;
> > +       for (i = 0; i < tmp; i++) {
> > +               ret = of_property_read_u32(np, "gpios",
> > +                                          &gg_data.gpiospec.args[i]);
> > +               if (ret) {
> > +                       desc = ERR_PTR(ret);
> > +                       goto out;
> > +               }
> > +       }
> > +
> > +       gpiochip_find(&gg_data, of_gpiochip_find_and_xlate);
> 
> This seems to work but only supports one GPIO per hog node. It would
> be nice to be able to specify several GPIOs to which the same settings
> need to be applied.

This is on purpose following Linus Walleij's comment.

> 
> > +       if (!gg_data.out_gpio) {
> > +               if (np->parent == np)
> > +                       desc = ERR_PTR(-ENXIO);
> > +               else
> > +                       desc = ERR_PTR(-EPROBE_DEFER);
> > +               goto out;
> > +       }
> > +
> > +       if (xlate_flags & OF_GPIO_ACTIVE_LOW)
> > +               *lflags |= GPIOF_ACTIVE_LOW;
> > +
> > +       if (of_property_read_bool(np, "input")) {
> > +               *dflags |= GPIOD_IN;
> > +               in = 1;
> > +       }
> > +       if (of_property_read_bool(np, "output-low")) {
> > +               *dflags |= GPIOD_OUT_LOW;
> > +               outlo = 1;
> > +       }
> > +       if (of_property_read_bool(np, "output-high")) {
> > +               *dflags |= GPIOD_OUT_HIGH;
> > +               outhi = 1;
> > +       }
> 
> I thought we agreed that this should be a "direction =
> input|output-low|output-high" property?

Yes, miss that my bad.
I'll change it, so it will only look for a single "direction" node, and hopefully simplify this mess.

> 
> > +       if ((in + outlo + outhi) > 1) {
> > +               pr_warn("%s: GPIO %s: more than one direction/value selected, assuming: %s.\n",
> > +                        __func__, np->name,
> > +                        (outhi)?"output-high":(outlo)?"output-low":"input");
> > +       }
> > +
> > +       if (of_property_read_bool(np, "open-drain-line"))
> > +               *lflags |= GPIO_OPEN_DRAIN;
> > +       if (of_property_read_bool(np, "open-source-line"))
> > +               *lflags |= GPIO_OPEN_SOURCE;
> 
> These properties are not documented in your DT bindings.

Yeah I'll remove those as they might as well be covered under the "xlate_flags" instead
similar to GPIO_ACTIVE_LOW or whenever it gets propagated.


> 
> > +       if (name && of_property_read_string(np, "line-name", name))
> > +               *name = np->name;
> > +
> > +       desc = gg_data.out_gpio;
> > +
> > +out:
> > +       return desc;
> > +}
> > +
> > +
> > +/**
> > + * do_gpio_hog - Given node is a GPIO hog configuration, handle it
> > + * @np:                device node to get GPIO from
> > + *
> > + * This is only used by of_gpiochip_add to request/set GPIO initial
> > + * configuration.
> > + */
> > +static int do_gpio_hog(struct device_node *np)
> > +{
> > +       struct gpio_desc *desc = NULL;
> > +       int err;
> > +       const char *name;
> > +       enum gpio_lookup_flags lflags;
> > +       enum gpiod_flags dflags;
> > +
> > +       desc = of_get_gpio_hog(np, &name, &lflags, &dflags);
> > +       if (!desc)
> > +               return -ENOTSUPP;
> > +       else if (IS_ERR(desc))
> > +               return PTR_ERR(desc);
> > +
> > +       err = gpiod_request(desc, name);
> 
> Using this function means that a GPIO chip module cannot be unloaded
> if it uses GPIO hogs. Is it the intended behavior? If not, please use
> gpiochip_request_own_desc() instead, and make sure to call
> gpiochip_free_own_desc() for each hog when the driver is unloaded.

So I guess we could add a undo_gpio_hog() function and hook it up under of_gpiochip_remove().
Now instead of maintaining a seperate structure just to keep track of hogged descriptor,
would it be acceptable to add a new "gpio_desc.flags" value in gpiolib.h says:

   #define FLAG_GPIO_IS_HOGGED 10

And key on that at removal time instead of creating a list and having to maintain that?

> 
> > +       if (err < 0)
> > +               return err;
> > +
> > +       if (lflags & GPIO_ACTIVE_LOW)
> > +               set_bit(FLAG_ACTIVE_LOW, &desc->flags);
> > +       if (lflags & GPIO_OPEN_DRAIN)
> > +               set_bit(FLAG_OPEN_DRAIN, &desc->flags);
> > +       if (lflags & GPIO_OPEN_SOURCE)
> > +               set_bit(FLAG_OPEN_SOURCE, &desc->flags);
> > +
> > +       /* No particular flag request, not really hogging then... */
> > +       if (!(dflags & GPIOD_FLAGS_BIT_DIR_SET)) {
> > +               pr_warn("%s: GPIO %s: no hogging direction specified, bailing out\n",
> > +                        __func__, name);
> > +               err = -EINVAL;
> > +               goto free_gpio;
> > +       }
> > +
> > +       /* Process flags */
> > +       if (dflags & GPIOD_FLAGS_BIT_DIR_OUT)
> > +               err = gpiod_direction_output(desc,
> > +                                            dflags & GPIOD_FLAGS_BIT_DIR_VAL);
> > +       else
> > +               err = gpiod_direction_input(desc);
> > +
> > +       if (err < 0) {
> > +               pr_warn("%s: GPIO %s setting direction/value failed\n",
> > +                        __func__, name);
> > +               goto free_gpio;
> > +       }
> 
> I would suggest to factorize this code that is similar to the one
> found in __gpiod_get_index(). Do all the DT parsing in a function that
> just returns a descriptor and the

I would tend to agree.
But as Linus suggested I was trying to contain the changes to gpiolib_of.c only.
> 
> > +
> > +       pr_debug("%s: gpio-hog: GPIO:%d (%s) as %s%s\n", __func__,
> > +               desc_to_gpio(desc), name,
> > +               (dflags&GPIOD_FLAGS_BIT_DIR_OUT) ? "output" : "input",
> > +               (dflags&GPIOD_FLAGS_BIT_DIR_OUT) ?
> > +                       (dflags&GPIOD_FLAGS_BIT_DIR_VAL) ? "/high" : "/low":"");
> > +
> > +       return 0;
> > +
> > +free_gpio:
> > +       gpiod_put(desc);
> > +       return err;
> > +}
> 
> I think most of this function should be moved into gpiolib.c. Add a
> function there that takes the descriptor, lflags and dflags and that
> sets the GPIO up, possibly adding it to a list of hogs so we can unset
> the hogs when unloading the module. That way we have a hogging
> mechanism that is not dependent on device tree and can be used by
> other GPIO binders, such as ACPI.
>

That's fine with as long as everyone agrees.
 
> There is still some work needed but it looks much better than the
> first version. Please make sure to also keep Maxime, Pantelis and Jiri
> in the loop since they expressed interest in your first version.

Regards,
Benoit

Re: [Patch v2 1/2] gpio: add GPIO hogging mechanism

[Alexandre Courbot]

On Tue, Dec 2, 2014 at 9:22 AM, Benoit Parrot <bparrot-l0cyMroinI0@public.gmane.org> wrote:
>> > +       }
>> > +
>> > +       if (tmp > MAX_PHANDLE_ARGS) {
>> > +               desc = ERR_PTR(-EINVAL);
>> > +               goto out;
>> > +       }
>> > +
>> > +       gg_data.gpiospec.args_count = tmp;
>> > +       gg_data.gpiospec.np = chip_np;
>> > +       for (i = 0; i < tmp; i++) {
>> > +               ret = of_property_read_u32(np, "gpios",
>> > +                                          &gg_data.gpiospec.args[i]);
>> > +               if (ret) {
>> > +                       desc = ERR_PTR(ret);
>> > +                       goto out;
>> > +               }
>> > +       }
>> > +
>> > +       gpiochip_find(&gg_data, of_gpiochip_find_and_xlate);
>>
>> This seems to work but only supports one GPIO per hog node. It would
>> be nice to be able to specify several GPIOs to which the same settings
>> need to be applied.
>
> This is on purpose following Linus Walleij's comment.

Could you point me to his comment? My bad for not remembering what he
said, but I'd like to understand why.

>> Using this function means that a GPIO chip module cannot be unloaded
>> if it uses GPIO hogs. Is it the intended behavior? If not, please use
>> gpiochip_request_own_desc() instead, and make sure to call
>> gpiochip_free_own_desc() for each hog when the driver is unloaded.
>
> So I guess we could add a undo_gpio_hog() function and hook it up under of_gpiochip_remove().
> Now instead of maintaining a seperate structure just to keep track of hogged descriptor,
> would it be acceptable to add a new "gpio_desc.flags" value in gpiolib.h says:
>
>    #define FLAG_GPIO_IS_HOGGED 10
>
> And key on that at removal time instead of creating a list and having to maintain that?

Definitely, that would be even better I think.

>> I would suggest to factorize this code that is similar to the one
>> found in __gpiod_get_index(). Do all the DT parsing in a function that
>> just returns a descriptor and the
>
> I would tend to agree.
> But as Linus suggested I was trying to contain the changes to gpiolib_of.c only.

If we add a FLAG_GPIO_IS_HOGGED and undo the hogs when the chip is
unloaded, I would say that this becomes a gpiolib feature. Moving it
here would also allow non-DT GPIO providers to implement hogs (it
should be particularly easy to implement for platform data). Linus, do
you agree?
--
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

Re: [Patch v2 1/2] gpio: add GPIO hogging mechanism

[Linus Walleij]

On Tue, Dec 2, 2014 at 3:10 PM, Alexandre Courbot <gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Tue, Dec 2, 2014 at 9:22 AM, Benoit Parrot <bparrot-l0cyMroinI0@public.gmane.org> wrote:
>>> > +       }
>>> > +
>>> > +       if (tmp > MAX_PHANDLE_ARGS) {
>>> > +               desc = ERR_PTR(-EINVAL);
>>> > +               goto out;
>>> > +       }
>>> > +
>>> > +       gg_data.gpiospec.args_count = tmp;
>>> > +       gg_data.gpiospec.np = chip_np;
>>> > +       for (i = 0; i < tmp; i++) {
>>> > +               ret = of_property_read_u32(np, "gpios",
>>> > +                                          &gg_data.gpiospec.args[i]);
>>> > +               if (ret) {
>>> > +                       desc = ERR_PTR(ret);
>>> > +                       goto out;
>>> > +               }
>>> > +       }
>>> > +
>>> > +       gpiochip_find(&gg_data, of_gpiochip_find_and_xlate);
>>>
>>> This seems to work but only supports one GPIO per hog node. It would
>>> be nice to be able to specify several GPIOs to which the same settings
>>> need to be applied.
>>
>> This is on purpose following Linus Walleij's comment.
>
> Could you point me to his comment? My bad for not remembering what he
> said, but I'd like to understand why.

Said in previous message I think: either one-per-node or lists directly
in the gpiochip node.

>>> Using this function means that a GPIO chip module cannot be unloaded
>>> if it uses GPIO hogs. Is it the intended behavior? If not, please use
>>> gpiochip_request_own_desc() instead, and make sure to call
>>> gpiochip_free_own_desc() for each hog when the driver is unloaded.
>>
>> So I guess we could add a undo_gpio_hog() function and hook it up under of_gpiochip_remove().
>> Now instead of maintaining a seperate structure just to keep track of hogged descriptor,
>> would it be acceptable to add a new "gpio_desc.flags" value in gpiolib.h says:
>>
>>    #define FLAG_GPIO_IS_HOGGED 10
>>
>> And key on that at removal time instead of creating a list and having to maintain that?
>
> Definitely, that would be even better I think.

Clever. Go for this.

>>> I would suggest to factorize this code that is similar to the one
>>> found in __gpiod_get_index(). Do all the DT parsing in a function that
>>> just returns a descriptor and the
>>
>> I would tend to agree.
>> But as Linus suggested I was trying to contain the changes to gpiolib_of.c only.
>
> If we add a FLAG_GPIO_IS_HOGGED and undo the hogs when the chip is
> unloaded, I would say that this becomes a gpiolib feature. Moving it
> here would also allow non-DT GPIO providers to implement hogs (it
> should be particularly easy to implement for platform data). Linus, do
> you agree?

Yes, this is better and leaves the door open for other users.

Yours,
Linus Walleij
--
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

Re: [Patch v2 1/2] gpio: add GPIO hogging mechanism

[Linus Walleij]

On Tue, Dec 2, 2014 at 1:22 AM, Benoit Parrot <bparrot-l0cyMroinI0@public.gmane.org> wrote:
> Alexandre Courbot <gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote on Fri [2014-Nov-28 16:30:01 +0900]:
>> On Fri, Nov 21, 2014 at 8:54 AM, Benoit Parrot <bparrot-l0cyMroinI0@public.gmane.org> wrote:

>> > +       }
>> > +
>> > +       if (tmp > MAX_PHANDLE_ARGS) {
>> > +               desc = ERR_PTR(-EINVAL);
>> > +               goto out;
>> > +       }
>> > +
>> > +       gg_data.gpiospec.args_count = tmp;
>> > +       gg_data.gpiospec.np = chip_np;
>> > +       for (i = 0; i < tmp; i++) {
>> > +               ret = of_property_read_u32(np, "gpios",
>> > +                                          &gg_data.gpiospec.args[i]);
>> > +               if (ret) {
>> > +                       desc = ERR_PTR(ret);
>> > +                       goto out;
>> > +               }
>> > +       }
>> > +
>> > +       gpiochip_find(&gg_data, of_gpiochip_find_and_xlate);
>>
>> This seems to work but only supports one GPIO per hog node. It would
>> be nice to be able to specify several GPIOs to which the same settings
>> need to be applied.
>
> This is on purpose following Linus Walleij's comment.

Yes, I think either we have separate nodes for each hogged line *OR*
we just put a list of hogs under the gpiochip, no special node at all.

The one-node-per-hog pattern has the upside of being usable
to also name the hogs. (Exporting them is dubious however!
I would add a special type of node for that.)

>> > +       if (err < 0)
>> > +               return err;
>> > +
>> > +       if (lflags & GPIO_ACTIVE_LOW)
>> > +               set_bit(FLAG_ACTIVE_LOW, &desc->flags);
>> > +       if (lflags & GPIO_OPEN_DRAIN)
>> > +               set_bit(FLAG_OPEN_DRAIN, &desc->flags);
>> > +       if (lflags & GPIO_OPEN_SOURCE)
>> > +               set_bit(FLAG_OPEN_SOURCE, &desc->flags);
>> > +
>> > +       /* No particular flag request, not really hogging then... */
>> > +       if (!(dflags & GPIOD_FLAGS_BIT_DIR_SET)) {
>> > +               pr_warn("%s: GPIO %s: no hogging direction specified, bailing out\n",
>> > +                        __func__, name);
>> > +               err = -EINVAL;
>> > +               goto free_gpio;
>> > +       }
>> > +
>> > +       /* Process flags */
>> > +       if (dflags & GPIOD_FLAGS_BIT_DIR_OUT)
>> > +               err = gpiod_direction_output(desc,
>> > +                                            dflags & GPIOD_FLAGS_BIT_DIR_VAL);
>> > +       else
>> > +               err = gpiod_direction_input(desc);
>> > +
>> > +       if (err < 0) {
>> > +               pr_warn("%s: GPIO %s setting direction/value failed\n",
>> > +                        __func__, name);
>> > +               goto free_gpio;
>> > +       }
>>
>> I would suggest to factorize this code that is similar to the one
>> found in __gpiod_get_index(). Do all the DT parsing in a function that
>> just returns a descriptor and the
>
> I would tend to agree.
> But as Linus suggested I was trying to contain the changes to gpiolib_of.c only.

Yes I prefer we begin by supporting it in OF and then generalize it later
if more users (board files, ACPI) appear.

Not a big deal but I want to avoid big design up front unless it's
easy and a few alterations.
http://c2.com/cgi/wiki?BigDesignUpFront

Yours,
Linus Walleij
--
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

[Patch v2 2/2] gpio: Document GPIO hogging mechanism

[Benoit Parrot]

Add GPIO hogging documentation to gpio.txt

Signed-off-by: Benoit Parrot <bparrot@ti.com>
---
Changes since v1:
 * Split the devicetree bindings documentation in its own patch.

 Documentation/devicetree/bindings/gpio/gpio.txt | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/Documentation/devicetree/bindings/gpio/gpio.txt b/Documentation/devicetree/bindings/gpio/gpio.txt
index 3fb8f53..82755e2 100644
--- a/Documentation/devicetree/bindings/gpio/gpio.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio.txt
@@ -103,6 +103,24 @@ Every GPIO controller node must contain both an empty "gpio-controller"
 property, and a #gpio-cells integer property, which indicates the number of
 cells in a gpio-specifier.
 
+The GPIO chip may contain GPIO hog definitions. GPIO hogging is a mechanism
+providing automatic GPIO request and configuration as part of the
+gpio-controller's driver probe function.
+
+Each GPIO hog definition is represented as a child node of the GPIO controller.
+Required properties:
+- gpio-hog: a property specifying that this child node represent a gpio-hog.
+- gpios: store the gpio information (id, flags, ...). Shall contain the
+  number of cells specified in its parent node (GPIO controller node).
+- input: a property specifying to set the GPIO direction as input.
+- output-high: a property specifying to set the GPIO direction to output with
+  the value high.
+- output-low: a property specifying to set the GPIO direction to output with
+  the value low.
+
+Optional properties:
+- line-name: the GPIO label name. If not present the node name is used.
+
 Example of two SOC GPIO banks defined as gpio-controller nodes:
 
 	qe_pio_a: gpio-controller@1400 {
@@ -110,6 +128,13 @@ Example of two SOC GPIO banks defined as gpio-controller nodes:
 		reg = <0x1400 0x18>;
 		gpio-controller;
 		#gpio-cells = <2>;
+
+		line_b: line_b {
+			gpio-hog;
+			gpios = <6 0>;
+			output-low;
+			line-name = "foo-bar-gpio";
+		};
 	};
 
 	qe_pio_e: gpio-controller@1460 {
-- 
1.8.5.1

Re: [Patch v2 2/2] gpio: Document GPIO hogging mechanism

[Alexandre Courbot]

On Fri, Nov 21, 2014 at 8:54 AM, Benoit Parrot <bparrot-l0cyMroinI0@public.gmane.org> wrote:
> Add GPIO hogging documentation to gpio.txt
>
> Signed-off-by: Benoit Parrot <bparrot-l0cyMroinI0@public.gmane.org>
> ---
> Changes since v1:
>  * Split the devicetree bindings documentation in its own patch.
>
>  Documentation/devicetree/bindings/gpio/gpio.txt | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/gpio/gpio.txt b/Documentation/devicetree/bindings/gpio/gpio.txt
> index 3fb8f53..82755e2 100644
> --- a/Documentation/devicetree/bindings/gpio/gpio.txt
> +++ b/Documentation/devicetree/bindings/gpio/gpio.txt
> @@ -103,6 +103,24 @@ Every GPIO controller node must contain both an empty "gpio-controller"
>  property, and a #gpio-cells integer property, which indicates the number of
>  cells in a gpio-specifier.
>
> +The GPIO chip may contain GPIO hog definitions. GPIO hogging is a mechanism
> +providing automatic GPIO request and configuration as part of the
> +gpio-controller's driver probe function.
> +
> +Each GPIO hog definition is represented as a child node of the GPIO controller.
> +Required properties:
> +- gpio-hog: a property specifying that this child node represent a gpio-hog.
> +- gpios: store the gpio information (id, flags, ...). Shall contain the
> +  number of cells specified in its parent node (GPIO controller node).
> +- input: a property specifying to set the GPIO direction as input.
> +- output-high: a property specifying to set the GPIO direction to output with
> +  the value high.
> +- output-low: a property specifying to set the GPIO direction to output with
> +  the value low.
> +
> +Optional properties:
> +- line-name: the GPIO label name. If not present the node name is used.
> +
>  Example of two SOC GPIO banks defined as gpio-controller nodes:
>
>         qe_pio_a: gpio-controller@1400 {
> @@ -110,6 +128,13 @@ Example of two SOC GPIO banks defined as gpio-controller nodes:
>                 reg = <0x1400 0x18>;
>                 gpio-controller;
>                 #gpio-cells = <2>;
> +
> +               line_b: line_b {

Mmm what is the label used for? Can this node ever be referenced from
somewhere else?
--
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

Re: [Patch v2 2/2] gpio: Document GPIO hogging mechanism

[Benoit Parrot]

Alexandre Courbot <gnurou@gmail.com> wrote on Fri [2014-Nov-28 16:31:19 +0900]:
> On Fri, Nov 21, 2014 at 8:54 AM, Benoit Parrot <bparrot@ti.com> wrote:
> > Add GPIO hogging documentation to gpio.txt
> >
> > Signed-off-by: Benoit Parrot <bparrot@ti.com>
> > ---
> > Changes since v1:
> >  * Split the devicetree bindings documentation in its own patch.
> >
> >  Documentation/devicetree/bindings/gpio/gpio.txt | 25 +++++++++++++++++++++++++
> >  1 file changed, 25 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/gpio/gpio.txt b/Documentation/devicetree/bindings/gpio/gpio.txt
> > index 3fb8f53..82755e2 100644
> > --- a/Documentation/devicetree/bindings/gpio/gpio.txt
> > +++ b/Documentation/devicetree/bindings/gpio/gpio.txt
> > @@ -103,6 +103,24 @@ Every GPIO controller node must contain both an empty "gpio-controller"
> >  property, and a #gpio-cells integer property, which indicates the number of
> >  cells in a gpio-specifier.
> >
> > +The GPIO chip may contain GPIO hog definitions. GPIO hogging is a mechanism
> > +providing automatic GPIO request and configuration as part of the
> > +gpio-controller's driver probe function.
> > +
> > +Each GPIO hog definition is represented as a child node of the GPIO controller.
> > +Required properties:
> > +- gpio-hog: a property specifying that this child node represent a gpio-hog.
> > +- gpios: store the gpio information (id, flags, ...). Shall contain the
> > +  number of cells specified in its parent node (GPIO controller node).
> > +- input: a property specifying to set the GPIO direction as input.
> > +- output-high: a property specifying to set the GPIO direction to output with
> > +  the value high.
> > +- output-low: a property specifying to set the GPIO direction to output with
> > +  the value low.
> > +
> > +Optional properties:
> > +- line-name: the GPIO label name. If not present the node name is used.
> > +
> >  Example of two SOC GPIO banks defined as gpio-controller nodes:
> >
> >         qe_pio_a: gpio-controller@1400 {
> > @@ -110,6 +128,13 @@ Example of two SOC GPIO banks defined as gpio-controller nodes:
> >                 reg = <0x1400 0x18>;
> >                 gpio-controller;
> >                 #gpio-cells = <2>;
> > +
> > +               line_b: line_b {
> 
> Mmm what is the label used for? Can this node ever be referenced from
> somewhere else?

It's not used for anything else as far as I know other than as the line-name to be assigned to the gpio being hogged.
I guess you agree with Linus and should make the line-name mandatory and remove the label altogether?

I was trying to keep the verbosity to a minimum so as to have the possibilty to keep everything on a single line when possible.

Benoit

Re: [Patch v2 2/2] gpio: Document GPIO hogging mechanism

[Alexandre Courbot]

On Tue, Dec 2, 2014 at 7:57 AM, Benoit Parrot <bparrot@ti.com> wrote:
> Alexandre Courbot <gnurou@gmail.com> wrote on Fri [2014-Nov-28 16:31:19 +0900]:
>> On Fri, Nov 21, 2014 at 8:54 AM, Benoit Parrot <bparrot@ti.com> wrote:
>> > Add GPIO hogging documentation to gpio.txt
>> >
>> > Signed-off-by: Benoit Parrot <bparrot@ti.com>
>> > ---
>> > Changes since v1:
>> >  * Split the devicetree bindings documentation in its own patch.
>> >
>> >  Documentation/devicetree/bindings/gpio/gpio.txt | 25 +++++++++++++++++++++++++
>> >  1 file changed, 25 insertions(+)
>> >
>> > diff --git a/Documentation/devicetree/bindings/gpio/gpio.txt b/Documentation/devicetree/bindings/gpio/gpio.txt
>> > index 3fb8f53..82755e2 100644
>> > --- a/Documentation/devicetree/bindings/gpio/gpio.txt
>> > +++ b/Documentation/devicetree/bindings/gpio/gpio.txt
>> > @@ -103,6 +103,24 @@ Every GPIO controller node must contain both an empty "gpio-controller"
>> >  property, and a #gpio-cells integer property, which indicates the number of
>> >  cells in a gpio-specifier.
>> >
>> > +The GPIO chip may contain GPIO hog definitions. GPIO hogging is a mechanism
>> > +providing automatic GPIO request and configuration as part of the
>> > +gpio-controller's driver probe function.
>> > +
>> > +Each GPIO hog definition is represented as a child node of the GPIO controller.
>> > +Required properties:
>> > +- gpio-hog: a property specifying that this child node represent a gpio-hog.
>> > +- gpios: store the gpio information (id, flags, ...). Shall contain the
>> > +  number of cells specified in its parent node (GPIO controller node).
>> > +- input: a property specifying to set the GPIO direction as input.
>> > +- output-high: a property specifying to set the GPIO direction to output with
>> > +  the value high.
>> > +- output-low: a property specifying to set the GPIO direction to output with
>> > +  the value low.
>> > +
>> > +Optional properties:
>> > +- line-name: the GPIO label name. If not present the node name is used.
>> > +
>> >  Example of two SOC GPIO banks defined as gpio-controller nodes:
>> >
>> >         qe_pio_a: gpio-controller@1400 {
>> > @@ -110,6 +128,13 @@ Example of two SOC GPIO banks defined as gpio-controller nodes:
>> >                 reg = <0x1400 0x18>;
>> >                 gpio-controller;
>> >                 #gpio-cells = <2>;
>> > +
>> > +               line_b: line_b {
>>
>> Mmm what is the label used for? Can this node ever be referenced from
>> somewhere else?
>
> It's not used for anything else as far as I know other than as the line-name to be assigned to the gpio being hogged.
> I guess you agree with Linus and should make the line-name mandatory and remove the label altogether?
>
> I was trying to keep the verbosity to a minimum so as to have the possibilty to keep everything on a single line when possible.

It's just that when you see a label, you expect it to be referenced
somewhere, which is obviously not the case here. Just having

line_b {

would work just as well, wouldn't it?