Thread (8 messages) 8 messages, 2 authors, 2019-08-02

Re: [PATCH v2 1/3] serial: mctrl_gpio: Avoid probe failures in case of missing gpiolib

From: Schrempf Frieder <hidden>
Date: 2019-08-02 07:56:58
Also in: linux-serial, lkml

On 01.08.19 22:33, Uwe Kleine-König wrote:
On Thu, Aug 01, 2019 at 06:45:21PM +0000, Schrempf Frieder wrote:
quoted
From: Frieder Schrempf <redacted>

If CONFIG_GPIOLIB is not enabled, mctrl_gpio_init() and
mctrl_gpio_init_noauto() will currently return an error pointer with
-ENOSYS. As the mctrl GPIOs are usually optional, drivers need to
check for this condition to allow continue probing.

To avoid the need for this check in each driver, we return NULL
instead, as all the mctrl_gpio_*() functions are skipped anyway.
We also adapt mctrl_gpio_to_gpiod() to be in line with this change.

Signed-off-by: Frieder Schrempf <redacted>
Reviewed-by: Fabio Estevam <festevam@gmail.com>
nitpick: put your S-o-b last.
Ok.
quoted
---
Changes in v2
=============
* Move the sh_sci changes to a separate patch
* Add a patch for the 8250 driver
* Add Fabio's R-b tag
---
  drivers/tty/serial/serial_mctrl_gpio.c | 3 +++
  drivers/tty/serial/serial_mctrl_gpio.h | 6 +++---
  2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
index 2b400189be91..54c43e02e375 100644
--- a/drivers/tty/serial/serial_mctrl_gpio.c
+++ b/drivers/tty/serial/serial_mctrl_gpio.c
@@ -61,6 +61,9 @@ EXPORT_SYMBOL_GPL(mctrl_gpio_set);
  struct gpio_desc *mctrl_gpio_to_gpiod(struct mctrl_gpios *gpios,
  				      enum mctrl_gpio_idx gidx)
  {
+	if (gpios == NULL)
+		return NULL;
+
I wonder why you need this. If GPIOLIB is off this code isn't active and
with GPIOLIB calling mctrl_gpio_to_gpiod with a gpios == NULL is a bug
that IMHO should not be silently ignored.

Am I missing something (again)?
No, you're right. My thoughts were, that if the mctrl_gpio functions are 
allowed to be passed a NULL pointer in general, they all should have a 
NULL check, even if in the current context (GPIOLIB disabled) this code 
is not active. Apparently there are other cases when a NULL pointer is 
passed, see [1]. So you can't really consider gpios == NULL to be a bug 
as this seems to be allowed in general.

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?id=434be0ae7aa746f481c3a22139e472dbc3f4f817
quoted
  	return gpios->gpio[gidx];
  }
Best regards
Uwe
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help