linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] gpio: Fix VLA removal fallout
@ 2018-09-05  9:05 Geert Uytterhoeven
  2018-09-05  9:05 ` [PATCH 1/3] Documentation: gpio: Fix return type of gpiod_set_raw_array_value*() Geert Uytterhoeven
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Geert Uytterhoeven @ 2018-09-05  9:05 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Laura Abbott, Jonathan Corbet, linux-gpio, linux-doc,
	linux-kernel, Geert Uytterhoeven

	Hi Linus,

This patch series fixes various (mostly harmless) issues introduced by
commit 3027743f83f867d8 ("gpio: Remove VLA from gpiolib").

As per the "one patch should fix one issue"-policy, this series contains 3
patches, although they all have the same Fixes: tag.

W.r.t. propagating errors: while gpiod_set_array_value_complex() and its
callers can now return an error code, this is currently limited to -ENOMEM.
Actual failures setting a GPIO output value cannot be propagated, as
gpio_chip.set() still returns void.  Do you want to change that?
E.g. gen_74x164_set_value() can fail.

Feel free to fold patches if deemed appropriate.

Thanks!

Geert Uytterhoeven (3):
  Documentation: gpio: Fix return type of gpiod_set_raw_array_value*()
  gpio: Propagate errors from gpiod_set_array_value_complex()
  gpio: Restore indentation of continued lines

 Documentation/driver-api/gpio/consumer.rst | 24 +++++++--------
 drivers/gpio/gpiolib.c                     | 34 +++++++++++-----------
 drivers/gpio/gpiolib.h                     |  6 ++--
 include/linux/gpio/consumer.h              | 31 ++++++++++----------
 4 files changed, 48 insertions(+), 47 deletions(-)

-- 
2.17.1

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

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

* [PATCH 1/3] Documentation: gpio: Fix return type of gpiod_set_raw_array_value*()
  2018-09-05  9:05 [PATCH 0/3] gpio: Fix VLA removal fallout Geert Uytterhoeven
@ 2018-09-05  9:05 ` Geert Uytterhoeven
  2018-09-05  9:05 ` [PATCH 2/3] gpio: Propagate errors from gpiod_set_array_value_complex() Geert Uytterhoeven
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Geert Uytterhoeven @ 2018-09-05  9:05 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Laura Abbott, Jonathan Corbet, linux-gpio, linux-doc,
	linux-kernel, Geert Uytterhoeven

The return type of gpiod_set_raw_array_value() and
gpiod_set_raw_array_value_cansleep() was changed from void to int, but
the doc update was forgotten.

Fixes: 3027743f83f867d8 ("gpio: Remove VLA from gpiolib")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 Documentation/driver-api/gpio/consumer.rst | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/Documentation/driver-api/gpio/consumer.rst b/Documentation/driver-api/gpio/consumer.rst
index aa03f389d41d6a79..a4e340c1859439ad 100644
--- a/Documentation/driver-api/gpio/consumer.rst
+++ b/Documentation/driver-api/gpio/consumer.rst
@@ -337,15 +337,15 @@ The following functions get or set the values of an array of GPIOs::
 	void gpiod_set_array_value(unsigned int array_size,
 				   struct gpio_desc **desc_array,
 				   int *value_array)
-	void gpiod_set_raw_array_value(unsigned int array_size,
-				       struct gpio_desc **desc_array,
-				       int *value_array)
+	int gpiod_set_raw_array_value(unsigned int array_size,
+				      struct gpio_desc **desc_array,
+				      int *value_array)
 	void gpiod_set_array_value_cansleep(unsigned int array_size,
 					    struct gpio_desc **desc_array,
 					    int *value_array)
-	void gpiod_set_raw_array_value_cansleep(unsigned int array_size,
-						struct gpio_desc **desc_array,
-						int *value_array)
+	int gpiod_set_raw_array_value_cansleep(unsigned int array_size,
+					       struct gpio_desc **desc_array,
+					       int *value_array)
 
 The array can be an arbitrary set of GPIOs. The functions will try to access
 GPIOs belonging to the same bank or chip simultaneously if supported by the
-- 
2.17.1


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

* [PATCH 2/3] gpio: Propagate errors from gpiod_set_array_value_complex()
  2018-09-05  9:05 [PATCH 0/3] gpio: Fix VLA removal fallout Geert Uytterhoeven
  2018-09-05  9:05 ` [PATCH 1/3] Documentation: gpio: Fix return type of gpiod_set_raw_array_value*() Geert Uytterhoeven
@ 2018-09-05  9:05 ` Geert Uytterhoeven
  2018-09-05  9:05 ` [PATCH 3/3] gpio: Restore indentation of continued lines Geert Uytterhoeven
  2018-09-06 13:01 ` [PATCH 0/3] gpio: Fix VLA removal fallout Linus Walleij
  3 siblings, 0 replies; 6+ messages in thread
From: Geert Uytterhoeven @ 2018-09-05  9:05 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Laura Abbott, Jonathan Corbet, linux-gpio, linux-doc,
	linux-kernel, Geert Uytterhoeven

Internal helper function gpiod_set_array_value_complex() was changed to
return an error value, but not all gpiolib callers were updated to
propagate the new error up.

Fixes: 3027743f83f867d8 ("gpio: Remove VLA from gpiolib")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 Documentation/driver-api/gpio/consumer.rst | 12 ++++++------
 drivers/gpio/gpiolib.c                     | 22 +++++++++++-----------
 include/linux/gpio/consumer.h              | 20 +++++++++++---------
 3 files changed, 28 insertions(+), 26 deletions(-)

diff --git a/Documentation/driver-api/gpio/consumer.rst b/Documentation/driver-api/gpio/consumer.rst
index a4e340c1859439ad..c7302c7e0f858995 100644
--- a/Documentation/driver-api/gpio/consumer.rst
+++ b/Documentation/driver-api/gpio/consumer.rst
@@ -334,15 +334,15 @@ The following functions get or set the values of an array of GPIOs::
 					   struct gpio_desc **desc_array,
 					   int *value_array);
 
-	void gpiod_set_array_value(unsigned int array_size,
-				   struct gpio_desc **desc_array,
-				   int *value_array)
+	int gpiod_set_array_value(unsigned int array_size,
+				  struct gpio_desc **desc_array,
+				  int *value_array)
 	int gpiod_set_raw_array_value(unsigned int array_size,
 				      struct gpio_desc **desc_array,
 				      int *value_array)
-	void gpiod_set_array_value_cansleep(unsigned int array_size,
-					    struct gpio_desc **desc_array,
-					    int *value_array)
+	int gpiod_set_array_value_cansleep(unsigned int array_size,
+					   struct gpio_desc **desc_array,
+					   int *value_array)
 	int gpiod_set_raw_array_value_cansleep(unsigned int array_size,
 					       struct gpio_desc **desc_array,
 					       int *value_array)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index e8f8a199939350a9..b8486e204b9e8c69 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -3184,13 +3184,13 @@ EXPORT_SYMBOL_GPL(gpiod_set_raw_array_value);
  * This function should be called from contexts where we cannot sleep, and will
  * complain if the GPIO chip functions potentially sleep.
  */
-void gpiod_set_array_value(unsigned int array_size,
-			   struct gpio_desc **desc_array, int *value_array)
+int gpiod_set_array_value(unsigned int array_size,
+			  struct gpio_desc **desc_array, int *value_array)
 {
 	if (!desc_array)
-		return;
-	gpiod_set_array_value_complex(false, false, array_size, desc_array,
-				      value_array);
+		return -EINVAL;
+	return gpiod_set_array_value_complex(false, false, array_size,
+					     desc_array, value_array);
 }
 EXPORT_SYMBOL_GPL(gpiod_set_array_value);
 
@@ -3542,15 +3542,15 @@ void gpiod_add_lookup_tables(struct gpiod_lookup_table **tables, size_t n)
  *
  * This function is to be called from contexts that can sleep.
  */
-void gpiod_set_array_value_cansleep(unsigned int array_size,
-				    struct gpio_desc **desc_array,
-				    int *value_array)
+int gpiod_set_array_value_cansleep(unsigned int array_size,
+				   struct gpio_desc **desc_array,
+				   int *value_array)
 {
 	might_sleep_if(extra_checks);
 	if (!desc_array)
-		return;
-	gpiod_set_array_value_complex(false, true, array_size, desc_array,
-				      value_array);
+		return -EINVAL;
+	return gpiod_set_array_value_complex(false, true, array_size,
+					     desc_array, value_array);
 }
 EXPORT_SYMBOL_GPL(gpiod_set_array_value_cansleep);
 
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 21ddbe4400308ee5..54ca52f67e8cf38b 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -106,8 +106,8 @@ int gpiod_get_value(const struct gpio_desc *desc);
 int gpiod_get_array_value(unsigned int array_size,
 			  struct gpio_desc **desc_array, int *value_array);
 void gpiod_set_value(struct gpio_desc *desc, int value);
-void gpiod_set_array_value(unsigned int array_size,
-			   struct gpio_desc **desc_array, int *value_array);
+int gpiod_set_array_value(unsigned int array_size,
+			  struct gpio_desc **desc_array, int *value_array);
 int gpiod_get_raw_value(const struct gpio_desc *desc);
 int gpiod_get_raw_array_value(unsigned int array_size,
 			      struct gpio_desc **desc_array,
@@ -123,9 +123,9 @@ int gpiod_get_array_value_cansleep(unsigned int array_size,
 				   struct gpio_desc **desc_array,
 				   int *value_array);
 void gpiod_set_value_cansleep(struct gpio_desc *desc, int value);
-void gpiod_set_array_value_cansleep(unsigned int array_size,
-				    struct gpio_desc **desc_array,
-				    int *value_array);
+int gpiod_set_array_value_cansleep(unsigned int array_size,
+				   struct gpio_desc **desc_array,
+				   int *value_array);
 int gpiod_get_raw_value_cansleep(const struct gpio_desc *desc);
 int gpiod_get_raw_array_value_cansleep(unsigned int array_size,
 				       struct gpio_desc **desc_array,
@@ -341,12 +341,13 @@ static inline void gpiod_set_value(struct gpio_desc *desc, int value)
 	/* GPIO can never have been requested */
 	WARN_ON(1);
 }
-static inline void gpiod_set_array_value(unsigned int array_size,
-					 struct gpio_desc **desc_array,
-					 int *value_array)
+static inline int gpiod_set_array_value(unsigned int array_size,
+					struct gpio_desc **desc_array,
+					int *value_array)
 {
 	/* GPIO can never have been requested */
 	WARN_ON(1);
+	return 0;
 }
 static inline int gpiod_get_raw_value(const struct gpio_desc *desc)
 {
@@ -395,12 +396,13 @@ static inline void gpiod_set_value_cansleep(struct gpio_desc *desc, int value)
 	/* GPIO can never have been requested */
 	WARN_ON(1);
 }
-static inline void gpiod_set_array_value_cansleep(unsigned int array_size,
+static inline int gpiod_set_array_value_cansleep(unsigned int array_size,
 					    struct gpio_desc **desc_array,
 					    int *value_array)
 {
 	/* GPIO can never have been requested */
 	WARN_ON(1);
+	return 0;
 }
 static inline int gpiod_get_raw_value_cansleep(const struct gpio_desc *desc)
 {
-- 
2.17.1


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

* [PATCH 3/3] gpio: Restore indentation of continued lines
  2018-09-05  9:05 [PATCH 0/3] gpio: Fix VLA removal fallout Geert Uytterhoeven
  2018-09-05  9:05 ` [PATCH 1/3] Documentation: gpio: Fix return type of gpiod_set_raw_array_value*() Geert Uytterhoeven
  2018-09-05  9:05 ` [PATCH 2/3] gpio: Propagate errors from gpiod_set_array_value_complex() Geert Uytterhoeven
@ 2018-09-05  9:05 ` Geert Uytterhoeven
  2018-09-06 13:01 ` [PATCH 0/3] gpio: Fix VLA removal fallout Linus Walleij
  3 siblings, 0 replies; 6+ messages in thread
From: Geert Uytterhoeven @ 2018-09-05  9:05 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Laura Abbott, Jonathan Corbet, linux-gpio, linux-doc,
	linux-kernel, Geert Uytterhoeven

Fixes: 3027743f83f867d8 ("gpio: Remove VLA from gpiolib")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/gpio/gpiolib.c        | 12 ++++++------
 drivers/gpio/gpiolib.h        |  6 +++---
 include/linux/gpio/consumer.h | 11 +++++------
 3 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index b8486e204b9e8c69..e95aac19ab6a0a2a 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -3025,9 +3025,9 @@ static void gpio_chip_set_multiple(struct gpio_chip *chip,
 }
 
 int gpiod_set_array_value_complex(bool raw, bool can_sleep,
-				   unsigned int array_size,
-				   struct gpio_desc **desc_array,
-				   int *value_array)
+				  unsigned int array_size,
+				  struct gpio_desc **desc_array,
+				  int *value_array)
 {
 	int i = 0;
 
@@ -3163,7 +3163,7 @@ EXPORT_SYMBOL_GPL(gpiod_set_value);
  * complain if the GPIO chip functions potentially sleep.
  */
 int gpiod_set_raw_array_value(unsigned int array_size,
-			 struct gpio_desc **desc_array, int *value_array)
+			      struct gpio_desc **desc_array, int *value_array)
 {
 	if (!desc_array)
 		return -EINVAL;
@@ -3503,8 +3503,8 @@ EXPORT_SYMBOL_GPL(gpiod_set_value_cansleep);
  * This function is to be called from contexts that can sleep.
  */
 int gpiod_set_raw_array_value_cansleep(unsigned int array_size,
-					struct gpio_desc **desc_array,
-					int *value_array)
+				       struct gpio_desc **desc_array,
+				       int *value_array)
 {
 	might_sleep_if(extra_checks);
 	if (!desc_array)
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index a7e49fef73d41de2..5fc5aa9f84b33fc4 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -189,9 +189,9 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep,
 				  struct gpio_desc **desc_array,
 				  int *value_array);
 int gpiod_set_array_value_complex(bool raw, bool can_sleep,
-				   unsigned int array_size,
-				   struct gpio_desc **desc_array,
-				   int *value_array);
+				  unsigned int array_size,
+				  struct gpio_desc **desc_array,
+				  int *value_array);
 
 /* This is just passed between gpiolib and devres */
 struct gpio_desc *gpiod_get_from_of_node(struct device_node *node,
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 54ca52f67e8cf38b..e60e2085d48ce995 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -114,8 +114,7 @@ int gpiod_get_raw_array_value(unsigned int array_size,
 			      int *value_array);
 void gpiod_set_raw_value(struct gpio_desc *desc, int value);
 int gpiod_set_raw_array_value(unsigned int array_size,
-			       struct gpio_desc **desc_array,
-			       int *value_array);
+			      struct gpio_desc **desc_array, int *value_array);
 
 /* Value get/set from sleeping context */
 int gpiod_get_value_cansleep(const struct gpio_desc *desc);
@@ -132,8 +131,8 @@ int gpiod_get_raw_array_value_cansleep(unsigned int array_size,
 				       int *value_array);
 void gpiod_set_raw_value_cansleep(struct gpio_desc *desc, int value);
 int gpiod_set_raw_array_value_cansleep(unsigned int array_size,
-					struct gpio_desc **desc_array,
-					int *value_array);
+				       struct gpio_desc **desc_array,
+				       int *value_array);
 
 int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce);
 int gpiod_set_transitory(struct gpio_desc *desc, bool transitory);
@@ -369,8 +368,8 @@ static inline void gpiod_set_raw_value(struct gpio_desc *desc, int value)
 	WARN_ON(1);
 }
 static inline int gpiod_set_raw_array_value(unsigned int array_size,
-					     struct gpio_desc **desc_array,
-					     int *value_array)
+					    struct gpio_desc **desc_array,
+					    int *value_array)
 {
 	/* GPIO can never have been requested */
 	WARN_ON(1);
-- 
2.17.1


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

* Re: [PATCH 0/3] gpio: Fix VLA removal fallout
  2018-09-05  9:05 [PATCH 0/3] gpio: Fix VLA removal fallout Geert Uytterhoeven
                   ` (2 preceding siblings ...)
  2018-09-05  9:05 ` [PATCH 3/3] gpio: Restore indentation of continued lines Geert Uytterhoeven
@ 2018-09-06 13:01 ` Linus Walleij
  2018-09-06 13:13   ` Geert Uytterhoeven
  3 siblings, 1 reply; 6+ messages in thread
From: Linus Walleij @ 2018-09-06 13:01 UTC (permalink / raw)
  To: Geert Uytterhoeven, Janusz Krzysztofik
  Cc: Laura Abbott, Jonathan Corbet, open list:GPIO SUBSYSTEM,
	linux-doc, linux-kernel

Hi Geert,

Thanks for the patches!

On Wed, Sep 5, 2018 at 11:23 AM Geert Uytterhoeven
<geert+renesas@glider.be> wrote:

> This patch series fixes various (mostly harmless) issues introduced by
> commit 3027743f83f867d8 ("gpio: Remove VLA from gpiolib").
>
> As per the "one patch should fix one issue"-policy, this series contains 3
> patches, although they all have the same Fixes: tag.
>
> W.r.t. propagating errors: while gpiod_set_array_value_complex() and its
> callers can now return an error code, this is currently limited to -ENOMEM.
> Actual failures setting a GPIO output value cannot be propagated, as
> gpio_chip.set() still returns void.  Do you want to change that?
> E.g. gen_74x164_set_value() can fail.
>
> Feel free to fold patches if deemed appropriate.

What I want to know is if these patches drive a truck through Janusz patch
set augmenting the array functions that I definately also plan to merge for
this kernel cycle.

Issues should be fixed of course, but if some of them already disappear
if I apply Janusz patches, I'd rather postpone ... is it going to be hard
to redo the cleanups on top of his patches?

Yours,
Linus Walleij

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

* Re: [PATCH 0/3] gpio: Fix VLA removal fallout
  2018-09-06 13:01 ` [PATCH 0/3] gpio: Fix VLA removal fallout Linus Walleij
@ 2018-09-06 13:13   ` Geert Uytterhoeven
  0 siblings, 0 replies; 6+ messages in thread
From: Geert Uytterhoeven @ 2018-09-06 13:13 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Geert Uytterhoeven, Janusz Krzysztofik, Laura Abbott,
	Jonathan Corbet, open list:GPIO SUBSYSTEM,
	open list:DOCUMENTATION, Linux Kernel Mailing List

Hi Linus,

On Thu, Sep 6, 2018 at 3:01 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> On Wed, Sep 5, 2018 at 11:23 AM Geert Uytterhoeven
> <geert+renesas@glider.be> wrote:
> > This patch series fixes various (mostly harmless) issues introduced by
> > commit 3027743f83f867d8 ("gpio: Remove VLA from gpiolib").
> >
> > As per the "one patch should fix one issue"-policy, this series contains 3
> > patches, although they all have the same Fixes: tag.
> >
> > W.r.t. propagating errors: while gpiod_set_array_value_complex() and its
> > callers can now return an error code, this is currently limited to -ENOMEM.
> > Actual failures setting a GPIO output value cannot be propagated, as
> > gpio_chip.set() still returns void.  Do you want to change that?
> > E.g. gen_74x164_set_value() can fail.
> >
> > Feel free to fold patches if deemed appropriate.
>
> What I want to know is if these patches drive a truck through Janusz patch
> set augmenting the array functions that I definately also plan to merge for
> this kernel cycle.
>
> Issues should be fixed of course, but if some of them already disappear
> if I apply Janusz patches, I'd rather postpone ... is it going to be hard
> to redo the cleanups on top of his patches?

I can respin afterwards. Just a few changed lines of context.

One question is if the stable team plans to backport the VLA removal (and
my fixes) or not...

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

end of thread, other threads:[~2018-09-06 13:13 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-05  9:05 [PATCH 0/3] gpio: Fix VLA removal fallout Geert Uytterhoeven
2018-09-05  9:05 ` [PATCH 1/3] Documentation: gpio: Fix return type of gpiod_set_raw_array_value*() Geert Uytterhoeven
2018-09-05  9:05 ` [PATCH 2/3] gpio: Propagate errors from gpiod_set_array_value_complex() Geert Uytterhoeven
2018-09-05  9:05 ` [PATCH 3/3] gpio: Restore indentation of continued lines Geert Uytterhoeven
2018-09-06 13:01 ` [PATCH 0/3] gpio: Fix VLA removal fallout Linus Walleij
2018-09-06 13:13   ` Geert Uytterhoeven

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).