linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 1/4] serial: mctrl_gpio: Avoid probe failures in case of missing gpiolib
@ 2019-08-02 10:04 Schrempf Frieder
  2019-08-02 10:04 ` [PATCH v3 2/4] serial: mctrl_gpio: Add a NULL check to mctrl_gpio_to_gpiod() Schrempf Frieder
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Schrempf Frieder @ 2019-08-02 10:04 UTC (permalink / raw)
  To: u.kleine-koenig, shawnguo, s.hauer, kernel, festevam, linux-imx,
	Greg Kroah-Hartman
  Cc: linux-arm-kernel, geert+renesas, Schrempf Frieder, Jiri Slaby,
	linux-serial, linux-kernel

From: Frieder Schrempf <frieder.schrempf@kontron.de>

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.

Reviewed-by: Fabio Estevam <festevam@gmail.com>
Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
Changes in v3
=============
* Move the changes in mctrl_gpio_to_gpiod() to a separate patch
* Reorder tags

Changes in v2
=============
* Move the sh_sci changes to a separate patch
* Add Fabio's R-b tag
---
 drivers/tty/serial/serial_mctrl_gpio.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/serial_mctrl_gpio.h b/drivers/tty/serial/serial_mctrl_gpio.h
index b7d3cca48ede..1b2ff503b2c2 100644
--- a/drivers/tty/serial/serial_mctrl_gpio.h
+++ b/drivers/tty/serial/serial_mctrl_gpio.h
@@ -114,19 +114,19 @@ static inline
 struct gpio_desc *mctrl_gpio_to_gpiod(struct mctrl_gpios *gpios,
 				      enum mctrl_gpio_idx gidx)
 {
-	return ERR_PTR(-ENOSYS);
+	return NULL;
 }
 
 static inline
 struct mctrl_gpios *mctrl_gpio_init(struct uart_port *port, unsigned int idx)
 {
-	return ERR_PTR(-ENOSYS);
+	return NULL;
 }
 
 static inline
 struct mctrl_gpios *mctrl_gpio_init_noauto(struct device *dev, unsigned int idx)
 {
-	return ERR_PTR(-ENOSYS);
+	return NULL;
 }
 
 static inline
-- 
2.17.1

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

* [PATCH v3 2/4] serial: mctrl_gpio: Add a NULL check to mctrl_gpio_to_gpiod()
  2019-08-02 10:04 [PATCH v3 1/4] serial: mctrl_gpio: Avoid probe failures in case of missing gpiolib Schrempf Frieder
@ 2019-08-02 10:04 ` Schrempf Frieder
  2019-08-02 12:12   ` Uwe Kleine-König
  2019-08-02 10:04 ` [PATCH v3 3/4] serial: sh-sci: Don't check for mctrl_gpio_init() returning -ENOSYS Schrempf Frieder
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: Schrempf Frieder @ 2019-08-02 10:04 UTC (permalink / raw)
  To: u.kleine-koenig, shawnguo, s.hauer, kernel, festevam, linux-imx,
	Greg Kroah-Hartman
  Cc: linux-arm-kernel, geert+renesas, Schrempf Frieder, Jiri Slaby,
	linux-serial, linux-kernel

From: Frieder Schrempf <frieder.schrempf@kontron.de>

As it is allowed to use the mctrl_gpio_* functions before
initialization (as the 8250 driver does according to 434be0ae7aa7),
it seems appropriate to have a NULL check in all of the functions.
Otherwise the mctrl_gpio_to_gpiod() function is prone to be used
in a context that can lead to a NULL pointer dereference.

Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
Changes in v3
=============
* Move the changes in mctrl_gpio_to_gpiod() to a separate patch
---
 drivers/tty/serial/serial_mctrl_gpio.c | 3 +++
 1 file changed, 3 insertions(+)

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;
+
 	return gpios->gpio[gidx];
 }
 EXPORT_SYMBOL_GPL(mctrl_gpio_to_gpiod);
-- 
2.17.1

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

* [PATCH v3 3/4] serial: sh-sci: Don't check for mctrl_gpio_init() returning -ENOSYS
  2019-08-02 10:04 [PATCH v3 1/4] serial: mctrl_gpio: Avoid probe failures in case of missing gpiolib Schrempf Frieder
  2019-08-02 10:04 ` [PATCH v3 2/4] serial: mctrl_gpio: Add a NULL check to mctrl_gpio_to_gpiod() Schrempf Frieder
@ 2019-08-02 10:04 ` Schrempf Frieder
  2019-08-02 12:13   ` Uwe Kleine-König
  2019-08-06  8:09   ` Geert Uytterhoeven
  2019-08-02 10:04 ` [PATCH v3 4/4] serial: 8250: " Schrempf Frieder
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 15+ messages in thread
From: Schrempf Frieder @ 2019-08-02 10:04 UTC (permalink / raw)
  To: u.kleine-koenig, shawnguo, s.hauer, kernel, festevam, linux-imx,
	Greg Kroah-Hartman
  Cc: linux-arm-kernel, geert+renesas, Schrempf Frieder, Jiri Slaby,
	linux-serial, linux-kernel

From: Frieder Schrempf <frieder.schrempf@kontron.de>

Now that the mctrl_gpio code returns NULL instead of ERR_PTR(-ENOSYS)
if CONFIG_GPIOLIB is disabled, we can safely remove this check.

Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
Changes in v3
=============
* Adjust the commit message and subject line

Changes in v2
=============
* Move the sh_sci changes to a separate patch
---
 drivers/tty/serial/sh-sci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index d18c680aa64b..249325b65ee0 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -3287,7 +3287,7 @@ static int sci_probe_single(struct platform_device *dev,
 		return ret;
 
 	sciport->gpios = mctrl_gpio_init(&sciport->port, 0);
-	if (IS_ERR(sciport->gpios) && PTR_ERR(sciport->gpios) != -ENOSYS)
+	if (IS_ERR(sciport->gpios))
 		return PTR_ERR(sciport->gpios);
 
 	if (sciport->has_rtscts) {
-- 
2.17.1

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

* [PATCH v3 4/4] serial: 8250: Don't check for mctrl_gpio_init() returning -ENOSYS
  2019-08-02 10:04 [PATCH v3 1/4] serial: mctrl_gpio: Avoid probe failures in case of missing gpiolib Schrempf Frieder
  2019-08-02 10:04 ` [PATCH v3 2/4] serial: mctrl_gpio: Add a NULL check to mctrl_gpio_to_gpiod() Schrempf Frieder
  2019-08-02 10:04 ` [PATCH v3 3/4] serial: sh-sci: Don't check for mctrl_gpio_init() returning -ENOSYS Schrempf Frieder
@ 2019-08-02 10:04 ` Schrempf Frieder
  2019-08-02 12:15   ` Uwe Kleine-König
  2019-08-02 12:08 ` [PATCH v3 1/4] serial: mctrl_gpio: Avoid probe failures in case of missing gpiolib Uwe Kleine-König
  2019-08-06  8:09 ` Geert Uytterhoeven
  4 siblings, 1 reply; 15+ messages in thread
From: Schrempf Frieder @ 2019-08-02 10:04 UTC (permalink / raw)
  To: u.kleine-koenig, shawnguo, s.hauer, kernel, festevam, linux-imx
  Cc: linux-arm-kernel, geert+renesas, Schrempf Frieder,
	Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-kernel

From: Frieder Schrempf <frieder.schrempf@kontron.de>

Now that the mctrl_gpio code returns NULL instead of ERR_PTR(-ENOSYS)
if CONFIG_GPIOLIB is disabled, we can safely remove this check.

Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
Changes in v3
=============
* Adjust the commit message and subject line

Changes in v2
=============
* Add a patch for the 8250 driver
---
 drivers/tty/serial/8250/8250_core.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index df3bcc0b2d74..e682390ce0de 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -1026,10 +1026,8 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
 		if (!has_acpi_companion(uart->port.dev)) {
 			gpios = mctrl_gpio_init(&uart->port, 0);
 			if (IS_ERR(gpios)) {
-				if (PTR_ERR(gpios) != -ENOSYS) {
-					ret = PTR_ERR(gpios);
-					goto out_unlock;
-				}
+				ret = PTR_ERR(gpios);
+				goto out_unlock;
 			} else {
 				uart->gpios = gpios;
 			}
-- 
2.17.1

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

* Re: [PATCH v3 1/4] serial: mctrl_gpio: Avoid probe failures in case of missing gpiolib
  2019-08-02 10:04 [PATCH v3 1/4] serial: mctrl_gpio: Avoid probe failures in case of missing gpiolib Schrempf Frieder
                   ` (2 preceding siblings ...)
  2019-08-02 10:04 ` [PATCH v3 4/4] serial: 8250: " Schrempf Frieder
@ 2019-08-02 12:08 ` Uwe Kleine-König
  2019-08-06  8:09 ` Geert Uytterhoeven
  4 siblings, 0 replies; 15+ messages in thread
From: Uwe Kleine-König @ 2019-08-02 12:08 UTC (permalink / raw)
  To: Schrempf Frieder
  Cc: shawnguo, s.hauer, kernel, festevam, linux-imx,
	Greg Kroah-Hartman, geert+renesas, linux-kernel, linux-serial,
	Jiri Slaby, linux-arm-kernel

On Fri, Aug 02, 2019 at 10:04:09AM +0000, Schrempf Frieder wrote:
> From: Frieder Schrempf <frieder.schrempf@kontron.de>
> 
> 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.
> 
> Reviewed-by: Fabio Estevam <festevam@gmail.com>
> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>

Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Thanks
Uwe

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

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

* Re: [PATCH v3 2/4] serial: mctrl_gpio: Add a NULL check to mctrl_gpio_to_gpiod()
  2019-08-02 10:04 ` [PATCH v3 2/4] serial: mctrl_gpio: Add a NULL check to mctrl_gpio_to_gpiod() Schrempf Frieder
@ 2019-08-02 12:12   ` Uwe Kleine-König
  2019-08-05  9:01     ` Schrempf Frieder
  0 siblings, 1 reply; 15+ messages in thread
From: Uwe Kleine-König @ 2019-08-02 12:12 UTC (permalink / raw)
  To: Schrempf Frieder
  Cc: shawnguo, s.hauer, kernel, festevam, linux-imx,
	Greg Kroah-Hartman, geert+renesas, linux-kernel, linux-serial,
	Jiri Slaby, linux-arm-kernel

On Fri, Aug 02, 2019 at 10:04:10AM +0000, Schrempf Frieder wrote:
> From: Frieder Schrempf <frieder.schrempf@kontron.de>
> 
> As it is allowed to use the mctrl_gpio_* functions before
> initialization (as the 8250 driver does according to 434be0ae7aa7),

Actually I was surprised some time ago that 8250 used serial_mctrl
without first initializing it and expecting it to work. I didn't look in
detail, but I wouldn't go so far to call this "allowed". The commit
itself calls it "workaround" which seems a better match.

> it seems appropriate to have a NULL check in all of the functions.
> Otherwise the mctrl_gpio_to_gpiod() function is prone to be used
> in a context that can lead to a NULL pointer dereference.
> 
> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>

Best regards
Uwe

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

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

* Re: [PATCH v3 3/4] serial: sh-sci: Don't check for mctrl_gpio_init() returning -ENOSYS
  2019-08-02 10:04 ` [PATCH v3 3/4] serial: sh-sci: Don't check for mctrl_gpio_init() returning -ENOSYS Schrempf Frieder
@ 2019-08-02 12:13   ` Uwe Kleine-König
  2019-08-06  8:09   ` Geert Uytterhoeven
  1 sibling, 0 replies; 15+ messages in thread
From: Uwe Kleine-König @ 2019-08-02 12:13 UTC (permalink / raw)
  To: Schrempf Frieder
  Cc: shawnguo, s.hauer, kernel, festevam, linux-imx,
	Greg Kroah-Hartman, geert+renesas, linux-kernel, linux-serial,
	Jiri Slaby, linux-arm-kernel

On Fri, Aug 02, 2019 at 10:04:10AM +0000, Schrempf Frieder wrote:
> From: Frieder Schrempf <frieder.schrempf@kontron.de>
> 
> Now that the mctrl_gpio code returns NULL instead of ERR_PTR(-ENOSYS)
> if CONFIG_GPIOLIB is disabled, we can safely remove this check.
> 
> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>

Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Thanks
Uwe

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

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

* Re: [PATCH v3 4/4] serial: 8250: Don't check for mctrl_gpio_init() returning -ENOSYS
  2019-08-02 10:04 ` [PATCH v3 4/4] serial: 8250: " Schrempf Frieder
@ 2019-08-02 12:15   ` Uwe Kleine-König
  2019-08-02 12:26     ` Greg Kroah-Hartman
  0 siblings, 1 reply; 15+ messages in thread
From: Uwe Kleine-König @ 2019-08-02 12:15 UTC (permalink / raw)
  To: Schrempf Frieder
  Cc: shawnguo, s.hauer, kernel, festevam, linux-imx, geert+renesas,
	Greg Kroah-Hartman, linux-kernel, linux-serial, Jiri Slaby,
	linux-arm-kernel

On Fri, Aug 02, 2019 at 10:04:11AM +0000, Schrempf Frieder wrote:
> From: Frieder Schrempf <frieder.schrempf@kontron.de>
> 
> Now that the mctrl_gpio code returns NULL instead of ERR_PTR(-ENOSYS)
> if CONFIG_GPIOLIB is disabled, we can safely remove this check.
> 
> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>

Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

@greg: This patch doesn't depend on patch 2; ditto for patch 3. So only
taking patches 1, 3 and 4 should be fine. This way Frieder's v4 only
have to care for patch 2. (Assuming noone objects to 1, 3 and 4 of
course.)

Best regards
Uwe

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

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

* Re: [PATCH v3 4/4] serial: 8250: Don't check for mctrl_gpio_init() returning -ENOSYS
  2019-08-02 12:15   ` Uwe Kleine-König
@ 2019-08-02 12:26     ` Greg Kroah-Hartman
  2019-08-06  7:44       ` Uwe Kleine-König
  0 siblings, 1 reply; 15+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-02 12:26 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Schrempf Frieder, shawnguo, s.hauer, kernel, festevam, linux-imx,
	geert+renesas, linux-kernel, linux-serial, Jiri Slaby,
	linux-arm-kernel

On Fri, Aug 02, 2019 at 02:15:55PM +0200, Uwe Kleine-König wrote:
> On Fri, Aug 02, 2019 at 10:04:11AM +0000, Schrempf Frieder wrote:
> > From: Frieder Schrempf <frieder.schrempf@kontron.de>
> > 
> > Now that the mctrl_gpio code returns NULL instead of ERR_PTR(-ENOSYS)
> > if CONFIG_GPIOLIB is disabled, we can safely remove this check.
> > 
> > Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
> 
> Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> 
> @greg: This patch doesn't depend on patch 2; ditto for patch 3. So only
> taking patches 1, 3 and 4 should be fine. This way Frieder's v4 only
> have to care for patch 2. (Assuming noone objects to 1, 3 and 4 of
> course.)

Sounds good, I'll do that, thanks.

greg k-h

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

* Re: [PATCH v3 2/4] serial: mctrl_gpio: Add a NULL check to mctrl_gpio_to_gpiod()
  2019-08-02 12:12   ` Uwe Kleine-König
@ 2019-08-05  9:01     ` Schrempf Frieder
  2019-08-06  7:45       ` Uwe Kleine-König
  0 siblings, 1 reply; 15+ messages in thread
From: Schrempf Frieder @ 2019-08-05  9:01 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: shawnguo, s.hauer, kernel, festevam, linux-imx,
	Greg Kroah-Hartman, geert+renesas, linux-kernel, linux-serial,
	Jiri Slaby, linux-arm-kernel

On 02.08.19 14:12, Uwe Kleine-König wrote:
> On Fri, Aug 02, 2019 at 10:04:10AM +0000, Schrempf Frieder wrote:
>> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>>
>> As it is allowed to use the mctrl_gpio_* functions before
>> initialization (as the 8250 driver does according to 434be0ae7aa7),
> 
> Actually I was surprised some time ago that 8250 used serial_mctrl
> without first initializing it and expecting it to work. I didn't look in
> detail, but I wouldn't go so far to call this "allowed". The commit
> itself calls it "workaround" which seems a better match.

Ok, but if this is considered to be a workaround and as the 8250 driver 
does not use mctrl_gpio_to_gpiod(), we should maybe just drop this patch 
instead of encouraging others to use mctrl_gpio before initialization.

I'm really not sure what's best, so depending on what you will propose, 
I'll send a new version of this patch with adjusted commit message or not.

By the way, Uwe and Fabio: Thanks for your reviews!

> 
>> it seems appropriate to have a NULL check in all of the functions.
>> Otherwise the mctrl_gpio_to_gpiod() function is prone to be used
>> in a context that can lead to a NULL pointer dereference.
>>
>> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
> 
> Best regards
> Uwe
> 

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

* Re: [PATCH v3 4/4] serial: 8250: Don't check for mctrl_gpio_init() returning -ENOSYS
  2019-08-02 12:26     ` Greg Kroah-Hartman
@ 2019-08-06  7:44       ` Uwe Kleine-König
  0 siblings, 0 replies; 15+ messages in thread
From: Uwe Kleine-König @ 2019-08-06  7:44 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Schrempf Frieder, shawnguo, s.hauer, kernel, festevam, linux-imx,
	geert+renesas, linux-kernel, linux-serial, Jiri Slaby,
	linux-arm-kernel

Hey Greg

On Fri, Aug 02, 2019 at 02:26:23PM +0200, Greg Kroah-Hartman wrote:
> On Fri, Aug 02, 2019 at 02:15:55PM +0200, Uwe Kleine-König wrote:
> > On Fri, Aug 02, 2019 at 10:04:11AM +0000, Schrempf Frieder wrote:
> > > From: Frieder Schrempf <frieder.schrempf@kontron.de>
> > > 
> > > Now that the mctrl_gpio code returns NULL instead of ERR_PTR(-ENOSYS)
> > > if CONFIG_GPIOLIB is disabled, we can safely remove this check.
> > > 
> > > Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
> > 
> > Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > 
> > @greg: This patch doesn't depend on patch 2; ditto for patch 3. So only
> > taking patches 1, 3 and 4 should be fine. This way Frieder's v4 only
> > have to care for patch 2. (Assuming noone objects to 1, 3 and 4 of
> > course.)
> 
> Sounds good, I'll do that, thanks.

again you somehow managed to mangle my name :-|

$ git log -3 8f0df898b27926e443d13770adfd828cc0f50148 | grep Uwe
    Acked-by: Uwe Kleine-Knig <u.kleine-koenig@pengutronix.de>
    Acked-by: Uwe Kleine-Knig <u.kleine-koenig@pengutronix.de>
    Reviewed-by: Uwe Kleine-Knig <u.kleine-koenig@pengutronix.de>

in all three instances the ö is missing.

Uwe

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

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

* Re: [PATCH v3 2/4] serial: mctrl_gpio: Add a NULL check to mctrl_gpio_to_gpiod()
  2019-08-05  9:01     ` Schrempf Frieder
@ 2019-08-06  7:45       ` Uwe Kleine-König
  2019-08-06  8:02         ` Schrempf Frieder
  0 siblings, 1 reply; 15+ messages in thread
From: Uwe Kleine-König @ 2019-08-06  7:45 UTC (permalink / raw)
  To: Schrempf Frieder
  Cc: shawnguo, s.hauer, kernel, festevam, linux-imx,
	Greg Kroah-Hartman, geert+renesas, linux-kernel, linux-serial,
	Jiri Slaby, linux-arm-kernel

Hello Frieder,

On Mon, Aug 05, 2019 at 09:01:39AM +0000, Schrempf Frieder wrote:
> On 02.08.19 14:12, Uwe Kleine-König wrote:
> > On Fri, Aug 02, 2019 at 10:04:10AM +0000, Schrempf Frieder wrote:
> >> From: Frieder Schrempf <frieder.schrempf@kontron.de>
> >>
> >> As it is allowed to use the mctrl_gpio_* functions before
> >> initialization (as the 8250 driver does according to 434be0ae7aa7),
> > 
> > Actually I was surprised some time ago that 8250 used serial_mctrl
> > without first initializing it and expecting it to work. I didn't look in
> > detail, but I wouldn't go so far to call this "allowed". The commit
> > itself calls it "workaround" which seems a better match.
> 
> Ok, but if this is considered to be a workaround and as the 8250 driver 
> does not use mctrl_gpio_to_gpiod(), we should maybe just drop this patch 
> instead of encouraging others to use mctrl_gpio before initialization.
> 
> I'm really not sure what's best, so depending on what you will propose, 
> I'll send a new version of this patch with adjusted commit message or not.

I wouldn't encourage usage of mctrl-gpio before it's initialized. So I
suggest to drop this patch.

Best regards
Uwe

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

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

* Re: [PATCH v3 2/4] serial: mctrl_gpio: Add a NULL check to mctrl_gpio_to_gpiod()
  2019-08-06  7:45       ` Uwe Kleine-König
@ 2019-08-06  8:02         ` Schrempf Frieder
  0 siblings, 0 replies; 15+ messages in thread
From: Schrempf Frieder @ 2019-08-06  8:02 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: shawnguo, s.hauer, kernel, festevam, linux-imx,
	Greg Kroah-Hartman, geert+renesas, linux-kernel, linux-serial,
	Jiri Slaby, linux-arm-kernel

On 06.08.19 09:45, Uwe Kleine-König wrote:
> Hello Frieder,
> 
> On Mon, Aug 05, 2019 at 09:01:39AM +0000, Schrempf Frieder wrote:
>> On 02.08.19 14:12, Uwe Kleine-König wrote:
>>> On Fri, Aug 02, 2019 at 10:04:10AM +0000, Schrempf Frieder wrote:
>>>> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>>>>
>>>> As it is allowed to use the mctrl_gpio_* functions before
>>>> initialization (as the 8250 driver does according to 434be0ae7aa7),
>>>
>>> Actually I was surprised some time ago that 8250 used serial_mctrl
>>> without first initializing it and expecting it to work. I didn't look in
>>> detail, but I wouldn't go so far to call this "allowed". The commit
>>> itself calls it "workaround" which seems a better match.
>>
>> Ok, but if this is considered to be a workaround and as the 8250 driver
>> does not use mctrl_gpio_to_gpiod(), we should maybe just drop this patch
>> instead of encouraging others to use mctrl_gpio before initialization.
>>
>> I'm really not sure what's best, so depending on what you will propose,
>> I'll send a new version of this patch with adjusted commit message or not.
> 
> I wouldn't encourage usage of mctrl-gpio before it's initialized. So I
> suggest to drop this patch.

Ok, thanks.

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

* Re: [PATCH v3 1/4] serial: mctrl_gpio: Avoid probe failures in case of missing gpiolib
  2019-08-02 10:04 [PATCH v3 1/4] serial: mctrl_gpio: Avoid probe failures in case of missing gpiolib Schrempf Frieder
                   ` (3 preceding siblings ...)
  2019-08-02 12:08 ` [PATCH v3 1/4] serial: mctrl_gpio: Avoid probe failures in case of missing gpiolib Uwe Kleine-König
@ 2019-08-06  8:09 ` Geert Uytterhoeven
  4 siblings, 0 replies; 15+ messages in thread
From: Geert Uytterhoeven @ 2019-08-06  8:09 UTC (permalink / raw)
  To: Schrempf Frieder
  Cc: u.kleine-koenig, shawnguo, s.hauer, kernel, festevam, linux-imx,
	Greg Kroah-Hartman, linux-arm-kernel, geert+renesas, Jiri Slaby,
	linux-serial, linux-kernel

On Fri, Aug 2, 2019 at 12:04 PM Schrempf Frieder
<frieder.schrempf@kontron.de> wrote:
> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>
> 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.
>
> Reviewed-by: Fabio Estevam <festevam@gmail.com>
> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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] 15+ messages in thread

* Re: [PATCH v3 3/4] serial: sh-sci: Don't check for mctrl_gpio_init() returning -ENOSYS
  2019-08-02 10:04 ` [PATCH v3 3/4] serial: sh-sci: Don't check for mctrl_gpio_init() returning -ENOSYS Schrempf Frieder
  2019-08-02 12:13   ` Uwe Kleine-König
@ 2019-08-06  8:09   ` Geert Uytterhoeven
  1 sibling, 0 replies; 15+ messages in thread
From: Geert Uytterhoeven @ 2019-08-06  8:09 UTC (permalink / raw)
  To: Schrempf Frieder
  Cc: u.kleine-koenig, shawnguo, s.hauer, kernel, festevam, linux-imx,
	Greg Kroah-Hartman, linux-arm-kernel, geert+renesas, Jiri Slaby,
	linux-serial, linux-kernel

On Fri, Aug 2, 2019 at 12:04 PM Schrempf Frieder
<frieder.schrempf@kontron.de> wrote:
> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>
> Now that the mctrl_gpio code returns NULL instead of ERR_PTR(-ENOSYS)
> if CONFIG_GPIOLIB is disabled, we can safely remove this check.
>
> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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] 15+ messages in thread

end of thread, other threads:[~2019-08-06  8:10 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-02 10:04 [PATCH v3 1/4] serial: mctrl_gpio: Avoid probe failures in case of missing gpiolib Schrempf Frieder
2019-08-02 10:04 ` [PATCH v3 2/4] serial: mctrl_gpio: Add a NULL check to mctrl_gpio_to_gpiod() Schrempf Frieder
2019-08-02 12:12   ` Uwe Kleine-König
2019-08-05  9:01     ` Schrempf Frieder
2019-08-06  7:45       ` Uwe Kleine-König
2019-08-06  8:02         ` Schrempf Frieder
2019-08-02 10:04 ` [PATCH v3 3/4] serial: sh-sci: Don't check for mctrl_gpio_init() returning -ENOSYS Schrempf Frieder
2019-08-02 12:13   ` Uwe Kleine-König
2019-08-06  8:09   ` Geert Uytterhoeven
2019-08-02 10:04 ` [PATCH v3 4/4] serial: 8250: " Schrempf Frieder
2019-08-02 12:15   ` Uwe Kleine-König
2019-08-02 12:26     ` Greg Kroah-Hartman
2019-08-06  7:44       ` Uwe Kleine-König
2019-08-02 12:08 ` [PATCH v3 1/4] serial: mctrl_gpio: Avoid probe failures in case of missing gpiolib Uwe Kleine-König
2019-08-06  8:09 ` 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).