[PATCH 0/3] gpiolib: introduce gpio initializer

[PATCH 0/3] gpiolib: introduce gpio initializer

[Uwe Kleine-König]

Hello,

the target of this series is to allow gpios to be initialized but
without the side effect that the gpios are unusable then (as is the case
with gpio-hogs).

This can for example be used to prevent debug pins from floating when
unused or keep a chip in reset until a matching driver is available.

I already tried to fix the binding, no feedback so far.
See http://patchwork.ozlabs.org/patch/817695/ .

Best regards
Uwe

Uwe Kleine-König (3):
  gpiolib: reorder functions
  gpiolib: introduce helper functions working on gpio_info structs
  gpiolib: introduce gpio initializer

 drivers/gpio/gpiolib.c | 272 +++++++++++++++++++++++++++++++------------------
 1 file changed, 173 insertions(+), 99 deletions(-)

-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 1/3] gpiolib: reorder functions

[Uwe Kleine-König]

gpio_request_one() and gpio_request_array() make use of
gpio_direction_*, so it is natural to define the former them further low
in the file. It doesn't matter for the compiler here as all functions
are declared in a header, but the next commit adds a few more functions
that need this reordering to no need additional forward declarations.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/gpio/gpiolib.c | 152 ++++++++++++++++++++++++-------------------------
 1 file changed, 76 insertions(+), 76 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index b83a27de7d21..b868986dd449 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -110,82 +110,6 @@ void gpio_free(unsigned gpio)
 	gi->label = NULL;
 }
 
-/**
- * gpio_request_one - request a single GPIO with initial configuration
- * @gpio:	the GPIO number
- * @flags:	GPIO configuration as specified by GPIOF_*
- * @label:	a literal description string of this GPIO
- */
-int gpio_request_one(unsigned gpio, unsigned long flags, const char *label)
-{
-	int err;
-	struct gpio_info *gi = gpio_to_desc(gpio);
-
-	/*
-	 * Not all of the flags below are mulit-bit, but, for the sake
-	 * of consistency, the code is written as if all of them were.
-	 */
-	const bool active_low  = (flags & GPIOF_ACTIVE_LOW) == GPIOF_ACTIVE_LOW;
-	const bool dir_in      = (flags & GPIOF_DIR_IN) == GPIOF_DIR_IN;
-	const bool logical     = (flags & GPIOF_LOGICAL) == GPIOF_LOGICAL;
-	const bool init_active = (flags & GPIOF_INIT_ACTIVE) == GPIOF_INIT_ACTIVE;
-	const bool init_high   = (flags & GPIOF_INIT_HIGH) == GPIOF_INIT_HIGH;
-
-	err = gpio_request(gpio, label);
-	if (err)
-		return err;
-
-	gi->active_low = active_low;
-
-	if (dir_in)
-		err = gpio_direction_input(gpio);
-	else if (logical)
-		err = gpio_direction_active(gpio, init_active);
-	else
-		err = gpio_direction_output(gpio, init_high);
-
-	if (err)
-		gpio_free(gpio);
-
-	return err;
-}
-EXPORT_SYMBOL_GPL(gpio_request_one);
-
-/**
- * gpio_request_array - request multiple GPIOs in a single call
- * @array:	array of the 'struct gpio'
- * @num:	how many GPIOs in the array
- */
-int gpio_request_array(const struct gpio *array, size_t num)
-{
-	int i, err;
-
-	for (i = 0; i < num; i++, array++) {
-		err = gpio_request_one(array->gpio, array->flags, array->label);
-		if (err)
-			goto err_free;
-	}
-	return 0;
-
-err_free:
-	while (i--)
-		gpio_free((--array)->gpio);
-	return err;
-}
-EXPORT_SYMBOL_GPL(gpio_request_array);
-
-/**
- * gpio_free_array - release multiple GPIOs in a single call
- * @array:	array of the 'struct gpio'
- * @num:	how many GPIOs in the array
- */
-void gpio_free_array(const struct gpio *array, size_t num)
-{
-	while (num--)
-		gpio_free((array++)->gpio);
-}
-EXPORT_SYMBOL_GPL(gpio_free_array);
-
 void gpio_set_value(unsigned gpio, int value)
 {
 	struct gpio_info *gi = gpio_to_desc(gpio);
@@ -289,6 +213,82 @@ int gpio_direction_input(unsigned gpio)
 }
 EXPORT_SYMBOL(gpio_direction_input);
 
+/**
+ * gpio_request_one - request a single GPIO with initial configuration
+ * @gpio:	the GPIO number
+ * @flags:	GPIO configuration as specified by GPIOF_*
+ * @label:	a literal description string of this GPIO
+ */
+int gpio_request_one(unsigned gpio, unsigned long flags, const char *label)
+{
+	int err;
+	struct gpio_info *gi = gpio_to_desc(gpio);
+
+	/*
+	 * Not all of the flags below are mulit-bit, but, for the sake
+	 * of consistency, the code is written as if all of them were.
+	 */
+	const bool active_low  = (flags & GPIOF_ACTIVE_LOW) == GPIOF_ACTIVE_LOW;
+	const bool dir_in      = (flags & GPIOF_DIR_IN) == GPIOF_DIR_IN;
+	const bool logical     = (flags & GPIOF_LOGICAL) == GPIOF_LOGICAL;
+	const bool init_active = (flags & GPIOF_INIT_ACTIVE) == GPIOF_INIT_ACTIVE;
+	const bool init_high   = (flags & GPIOF_INIT_HIGH) == GPIOF_INIT_HIGH;
+
+	err = gpio_request(gpio, label);
+	if (err)
+		return err;
+
+	gi->active_low = active_low;
+
+	if (dir_in)
+		err = gpio_direction_input(gpio);
+	else if (logical)
+		err = gpio_direction_active(gpio, init_active);
+	else
+		err = gpio_direction_output(gpio, init_high);
+
+	if (err)
+		gpio_free(gpio);
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(gpio_request_one);
+
+/**
+ * gpio_request_array - request multiple GPIOs in a single call
+ * @array:	array of the 'struct gpio'
+ * @num:	how many GPIOs in the array
+ */
+int gpio_request_array(const struct gpio *array, size_t num)
+{
+	int i, err;
+
+	for (i = 0; i < num; i++, array++) {
+		err = gpio_request_one(array->gpio, array->flags, array->label);
+		if (err)
+			goto err_free;
+	}
+	return 0;
+
+err_free:
+	while (i--)
+		gpio_free((--array)->gpio);
+	return err;
+}
+EXPORT_SYMBOL_GPL(gpio_request_array);
+
+/**
+ * gpio_free_array - release multiple GPIOs in a single call
+ * @array:	array of the 'struct gpio'
+ * @num:	how many GPIOs in the array
+ */
+void gpio_free_array(const struct gpio *array, size_t num)
+{
+	while (num--)
+		gpio_free((array++)->gpio);
+}
+EXPORT_SYMBOL_GPL(gpio_free_array);
+
 static int gpiochip_find_base(int start, int ngpio)
 {
 	int i;
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/3] gpiolib: introduce helper functions working on gpio_info structs

[Uwe Kleine-König]

gpioinfo_*() assume their gpio_info pointer parameter to be valid and
don't ensure the gpio to be requested.

This drops several checks for being requested and allows further
extensions to work with unrequested gpios.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/gpio/gpiolib.c | 135 +++++++++++++++++++++++++++++++++++--------------
 1 file changed, 96 insertions(+), 39 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index b868986dd449..de543fbb1321 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -47,6 +47,11 @@ static struct gpio_info *gpio_to_desc(unsigned gpio)
 	return NULL;
 }
 
+static unsigned gpioinfo_chip_offset(struct gpio_info *gi)
+{
+	return (gi - gpio_desc) - gi->chip->base;
+}
+
 static int gpio_adjust_value(struct gpio_info *gi,
 			     int value)
 {
@@ -56,16 +61,10 @@ static int gpio_adjust_value(struct gpio_info *gi,
 	return !!value ^ gi->active_low;
 }
 
-int gpio_request(unsigned gpio, const char *label)
+static int gpioinfo_request(struct gpio_info *gi, const char *label)
 {
-	struct gpio_info *gi = gpio_to_desc(gpio);
 	int ret;
 
-	if (!gi) {
-		ret = -ENODEV;
-		goto done;
-	}
-
 	if (gi->requested) {
 		ret = -EBUSY;
 		goto done;
@@ -74,7 +73,8 @@ int gpio_request(unsigned gpio, const char *label)
 	ret = 0;
 
 	if (gi->chip->ops->request) {
-		ret = gi->chip->ops->request(gi->chip, gpio - gi->chip->base);
+		ret = gi->chip->ops->request(gi->chip,
+					     gpioinfo_chip_offset(gi));
 		if (ret)
 			goto done;
 	}
@@ -86,23 +86,31 @@ int gpio_request(unsigned gpio, const char *label)
 done:
 	if (ret)
 		pr_err("_gpio_request: gpio-%d (%s) status %d\n",
-			 gpio, label ? : "?", ret);
+		       gi - gpio_desc, label ? : "?", ret);
 
 	return ret;
 }
 
-void gpio_free(unsigned gpio)
+int gpio_request(unsigned gpio, const char *label)
 {
 	struct gpio_info *gi = gpio_to_desc(gpio);
 
-	if (!gi)
-		return;
+	if (!gi) {
+		pr_err("_gpio_request: gpio-%d (%s) status %d\n",
+			 gpio, label ? : "?", -ENODEV);
+		return -ENODEV;
+	}
+
+	return gpioinfo_request(gi, label);
+}
 
+static void gpioinfo_free(struct gpio_info *gi)
+{
 	if (!gi->requested)
 		return;
 
 	if (gi->chip->ops->free)
-		gi->chip->ops->free(gi->chip, gpio - gi->chip->base);
+		gi->chip->ops->free(gi->chip, gpioinfo_chip_offset(gi));
 
 	gi->requested = false;
 	gi->active_low = false;
@@ -110,6 +118,22 @@ void gpio_free(unsigned gpio)
 	gi->label = NULL;
 }
 
+void gpio_free(unsigned gpio)
+{
+	struct gpio_info *gi = gpio_to_desc(gpio);
+
+	if (!gi)
+		return;
+
+	gpioinfo_free(gi);
+}
+
+static void gpioinfo_set_value(struct gpio_info *gi, int value)
+{
+	if (gi->chip->ops->set)
+		gi->chip->ops->set(gi->chip, gpioinfo_chip_offset(gi), value);
+}
+
 void gpio_set_value(unsigned gpio, int value)
 {
 	struct gpio_info *gi = gpio_to_desc(gpio);
@@ -120,8 +144,7 @@ void gpio_set_value(unsigned gpio, int value)
 	if (gpio_ensure_requested(gi, gpio))
 		return;
 
-	if (gi->chip->ops->set)
-		gi->chip->ops->set(gi->chip, gpio - gi->chip->base, value);
+	gpioinfo_set_value(gi, value);
 }
 EXPORT_SYMBOL(gpio_set_value);
 
@@ -136,6 +159,14 @@ void gpio_set_active(unsigned gpio, bool value)
 }
 EXPORT_SYMBOL(gpio_set_active);
 
+static int gpioinfo_get_value(struct gpio_info *gi)
+{
+	if (!gi->chip->ops->get)
+		return -ENOSYS;
+
+	return gi->chip->ops->get(gi->chip, gpioinfo_chip_offset(gi));
+}
+
 int gpio_get_value(unsigned gpio)
 {
 	struct gpio_info *gi = gpio_to_desc(gpio);
@@ -148,9 +179,7 @@ int gpio_get_value(unsigned gpio)
 	if (ret)
 		return ret;
 
-	if (!gi->chip->ops->get)
-		return -ENOSYS;
-	return gi->chip->ops->get(gi->chip, gpio - gi->chip->base);
+	return gpioinfo_get_value(gi);
 }
 EXPORT_SYMBOL(gpio_get_value);
 
@@ -165,6 +194,15 @@ int gpio_is_active(unsigned gpio)
 }
 EXPORT_SYMBOL(gpio_is_active);
 
+static int gpioinfo_direction_output(struct gpio_info *gi, int value)
+{
+	if (!gi->chip->ops->direction_output)
+		return -ENOSYS;
+
+	return gi->chip->ops->direction_output(gi->chip,
+					       gpioinfo_chip_offset(gi), value);
+}
+
 int gpio_direction_output(unsigned gpio, int value)
 {
 	struct gpio_info *gi = gpio_to_desc(gpio);
@@ -177,13 +215,15 @@ int gpio_direction_output(unsigned gpio, int value)
 	if (ret)
 		return ret;
 
-	if (!gi->chip->ops->direction_output)
-		return -ENOSYS;
-	return gi->chip->ops->direction_output(gi->chip, gpio - gi->chip->base,
-					       value);
+	return gpioinfo_direction_output(gi, value);
 }
 EXPORT_SYMBOL(gpio_direction_output);
 
+static int gpioinfo_direction_active(struct gpio_info *gi, bool value)
+{
+	return gpioinfo_direction_output(gi, gpio_adjust_value(gi, value));
+}
+
 int gpio_direction_active(unsigned gpio, bool value)
 {
 	struct gpio_info *gi = gpio_to_desc(gpio);
@@ -191,10 +231,19 @@ int gpio_direction_active(unsigned gpio, bool value)
 	if (!gi)
 		return -ENODEV;
 
-	return gpio_direction_output(gpio, gpio_adjust_value(gi, value));
+	return gpioinfo_direction_active(gi, value);
 }
 EXPORT_SYMBOL(gpio_direction_active);
 
+static int gpioinfo_direction_input(struct gpio_info *gi)
+{
+	if (!gi->chip->ops->direction_input)
+		return -ENOSYS;
+
+	return gi->chip->ops->direction_input(gi->chip,
+					      gpioinfo_chip_offset(gi));
+}
+
 int gpio_direction_input(unsigned gpio)
 {
 	struct gpio_info *gi = gpio_to_desc(gpio);
@@ -207,22 +256,14 @@ int gpio_direction_input(unsigned gpio)
 	if (ret)
 		return ret;
 
-	if (!gi->chip->ops->direction_input)
-		return -ENOSYS;
-	return gi->chip->ops->direction_input(gi->chip, gpio - gi->chip->base);
+	return gpioinfo_direction_input(gi);
 }
 EXPORT_SYMBOL(gpio_direction_input);
 
-/**
- * gpio_request_one - request a single GPIO with initial configuration
- * @gpio:	the GPIO number
- * @flags:	GPIO configuration as specified by GPIOF_*
- * @label:	a literal description string of this GPIO
- */
-int gpio_request_one(unsigned gpio, unsigned long flags, const char *label)
+static int gpioinfo_request_one(struct gpio_info *gi, unsigned long flags,
+				const char *label)
 {
 	int err;
-	struct gpio_info *gi = gpio_to_desc(gpio);
 
 	/*
 	 * Not all of the flags below are mulit-bit, but, for the sake
@@ -234,24 +275,40 @@ int gpio_request_one(unsigned gpio, unsigned long flags, const char *label)
 	const bool init_active = (flags & GPIOF_INIT_ACTIVE) == GPIOF_INIT_ACTIVE;
 	const bool init_high   = (flags & GPIOF_INIT_HIGH) == GPIOF_INIT_HIGH;
 
-	err = gpio_request(gpio, label);
+	err = gpioinfo_request(gi, label);
 	if (err)
 		return err;
 
 	gi->active_low = active_low;
 
 	if (dir_in)
-		err = gpio_direction_input(gpio);
+		err = gpioinfo_direction_input(gi);
 	else if (logical)
-		err = gpio_direction_active(gpio, init_active);
+		err = gpioinfo_direction_active(gi, init_active);
 	else
-		err = gpio_direction_output(gpio, init_high);
+		err = gpioinfo_direction_output(gi, init_high);
 
 	if (err)
-		gpio_free(gpio);
+		gpioinfo_free(gi);
 
 	return err;
 }
+
+/**
+ * gpio_request_one - request a single GPIO with initial configuration
+ * @gpio:	the GPIO number
+ * @flags:	GPIO configuration as specified by GPIOF_*
+ * @label:	a literal description string of this GPIO
+ */
+int gpio_request_one(unsigned gpio, unsigned long flags, const char *label)
+{
+	struct gpio_info *gi = gpio_to_desc(gpio);
+
+	if (!gi)
+		return -ENODEV;
+
+	return gpioinfo_request_one(gi, flags, label);
+}
 EXPORT_SYMBOL_GPL(gpio_request_one);
 
 /**
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 3/3] gpiolib: introduce gpio initializer

[Uwe Kleine-König]

To define the initial state of a given gpio you can add a gpio-init node
to a gpio controller. This doesn't prevent a later consumer to request
that pin and only defines a save initial state.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/gpio/gpiolib.c | 43 ++++++++++++++++++++++++++++++-------------
 1 file changed, 30 insertions(+), 13 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index de543fbb1321..4aeba5193726 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -260,13 +260,12 @@ int gpio_direction_input(unsigned gpio)
 }
 EXPORT_SYMBOL(gpio_direction_input);
 
-static int gpioinfo_request_one(struct gpio_info *gi, unsigned long flags,
-				const char *label)
+static int gpioinfo_configure(struct gpio_info *gi, unsigned long flags)
 {
 	int err;
 
 	/*
-	 * Not all of the flags below are mulit-bit, but, for the sake
+	 * Not all of the flags below are multi-bit, but, for the sake
 	 * of consistency, the code is written as if all of them were.
 	 */
 	const bool active_low  = (flags & GPIOF_ACTIVE_LOW) == GPIOF_ACTIVE_LOW;
@@ -275,10 +274,6 @@ static int gpioinfo_request_one(struct gpio_info *gi, unsigned long flags,
 	const bool init_active = (flags & GPIOF_INIT_ACTIVE) == GPIOF_INIT_ACTIVE;
 	const bool init_high   = (flags & GPIOF_INIT_HIGH) == GPIOF_INIT_HIGH;
 
-	err = gpioinfo_request(gi, label);
-	if (err)
-		return err;
-
 	gi->active_low = active_low;
 
 	if (dir_in)
@@ -288,6 +283,19 @@ static int gpioinfo_request_one(struct gpio_info *gi, unsigned long flags,
 	else
 		err = gpioinfo_direction_output(gi, init_high);
 
+	return err;
+}
+
+static int gpioinfo_request_one(struct gpio_info *gi, unsigned long flags,
+				const char *label)
+{
+	int err;
+
+	err = gpioinfo_request(gi, label);
+	if (err)
+		return err;
+
+	err = gpioinfo_configure(gi, flags);
 	if (err)
 		gpioinfo_free(gi);
 
@@ -375,14 +383,15 @@ static int gpiochip_find_base(int start, int ngpio)
 	return base;
 }
 
-static int of_hog_gpio(struct device_node *np, struct gpio_chip *chip,
-		       unsigned int idx)
+static int of_hogninit_gpio(struct device_node *np, struct gpio_chip *chip,
+			    unsigned int idx, bool request)
 {
 	struct device_node *chip_np = chip->dev->device_node;
 	unsigned long flags = 0;
 	u32 gpio_cells, gpio_num, gpio_flags;
 	int ret, gpio;
 	const char *name = NULL;
+	struct gpio_info *gi;
 
 	ret = of_property_read_u32(chip_np, "#gpio-cells", &gpio_cells);
 	if (ret)
@@ -417,6 +426,7 @@ static int of_hog_gpio(struct device_node *np, struct gpio_chip *chip,
 		return ret;
 	}
 
+	gi = gpio_to_desc(gpio);
 
 	/*
 	 * Note that, in order to be compatible with Linux, the code
@@ -439,9 +449,13 @@ static int of_hog_gpio(struct device_node *np, struct gpio_chip *chip,
 	else
 		return -EINVAL;
 
-	of_property_read_string(np, "line-name", &name);
+	if (request) {
+		of_property_read_string(np, "line-name", &name);
 
-	return gpio_request_one(gpio, flags, name);
+		return gpioinfo_request_one(gi, flags, name);
+	} else {
+		return gpioinfo_configure(gi, flags);
+	}
 }
 
 static int of_gpiochip_scan_hogs(struct gpio_chip *chip)
@@ -453,12 +467,15 @@ static int of_gpiochip_scan_hogs(struct gpio_chip *chip)
 		return 0;
 
 	for_each_available_child_of_node(chip->dev->device_node, np) {
-		if (!of_property_read_bool(np, "gpio-hog"))
+		bool hog = of_property_read_bool(np, "gpio-hog");
+		bool init = of_property_read_bool(np, "gpio-init");
+
+		if (!hog && !init)
 			continue;
 
 		for (ret = 0, i = 0;
 		     !ret;
-		     ret = of_hog_gpio(np, chip, i), i++)
+		     ret = of_hogninit_gpio(np, chip, i, hog), i++)
 			;
 		/*
 		 * We ignore -EOVERFLOW because it serves us as an
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 0/3] gpiolib: introduce gpio initializer

[Sascha Hauer]

Hi,

On Tue, Sep 26, 2017 at 11:26:43PM +0200, Uwe Kleine-König wrote:
> Hello,
> 
> the target of this series is to allow gpios to be initialized but
> without the side effect that the gpios are unusable then (as is the case
> with gpio-hogs).
> 
> This can for example be used to prevent debug pins from floating when
> unused or keep a chip in reset until a matching driver is available.
> 
> I already tried to fix the binding, no feedback so far.
> See http://patchwork.ozlabs.org/patch/817695/ .

I'm delaying this for some time in the hope that upstream settles to
some solution.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 0/3] gpiolib: introduce gpio initializer

[Uwe Kleine-König]

Hello Sascha,

On Fri, Oct 06, 2017 at 06:29:00AM +0200, Sascha Hauer wrote:
> On Tue, Sep 26, 2017 at 11:26:43PM +0200, Uwe Kleine-König wrote:
> > the target of this series is to allow gpios to be initialized but
> > without the side effect that the gpios are unusable then (as is the case
> > with gpio-hogs).
> > 
> > This can for example be used to prevent debug pins from floating when
> > unused or keep a chip in reset until a matching driver is available.
> > 
> > I already tried to fix the binding, no feedback so far.
> > See http://patchwork.ozlabs.org/patch/817695/ .
> 
> I'm delaying this for some time in the hope that upstream settles to
> some solution.

IMHO patches 1 and 2 can already go in. They are clean up and
optimisation that are IMOH worth to have even without the last patch.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox