* [PATCH] serial: imx: Avoid probe failure in case of missing gpiolib
@ 2019-08-01 8:18 Schrempf Frieder
2019-08-01 8:48 ` Uwe Kleine-König
0 siblings, 1 reply; 6+ messages in thread
From: Schrempf Frieder @ 2019-08-01 8:18 UTC (permalink / raw)
To: Greg Kroah-Hartman, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team
Cc: Schrempf Frieder, Jiri Slaby, linux-serial, linux-arm-kernel,
linux-kernel
From: Frieder Schrempf <frieder.schrempf@kontron.de>
If CONFIG_GPIOLIB is not enabled, mctrl_gpio_init() will return
-ENOSYS and cause the probing of the imx UART to fail. As the
GPIOs are optional, we should continue probing in this case.
Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
drivers/tty/serial/imx.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 10db3e54ac9e..51714498dacf 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -2237,7 +2237,9 @@ static int imx_uart_probe(struct platform_device *pdev)
timer_setup(&sport->timer, imx_uart_timeout, 0);
sport->gpios = mctrl_gpio_init(&sport->port, 0);
- if (IS_ERR(sport->gpios))
+ if (PTR_ERR(sport->gpios) == -ENOSYS)
+ sport->gpios = NULL;
+ else if (IS_ERR(sport->gpios))
return PTR_ERR(sport->gpios);
sport->clk_ipg = devm_clk_get(&pdev->dev, "ipg");
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] serial: imx: Avoid probe failure in case of missing gpiolib
2019-08-01 8:18 [PATCH] serial: imx: Avoid probe failure in case of missing gpiolib Schrempf Frieder
@ 2019-08-01 8:48 ` Uwe Kleine-König
2019-08-01 9:28 ` Schrempf Frieder
0 siblings, 1 reply; 6+ messages in thread
From: Uwe Kleine-König @ 2019-08-01 8:48 UTC (permalink / raw)
To: Schrempf Frieder
Cc: Greg Kroah-Hartman, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
linux-arm-kernel, linux-kernel, linux-serial, Jiri Slaby
On Thu, Aug 01, 2019 at 08:18:05AM +0000, Schrempf Frieder wrote:
> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>
> If CONFIG_GPIOLIB is not enabled, mctrl_gpio_init() will return
> -ENOSYS and cause the probing of the imx UART to fail. As the
> GPIOs are optional, we should continue probing in this case.
Is this really still the case? On which version did you hit this
problem?
I would expect that is gone with
d99482673f950817b30caf3fcdfb31179b050ce1 if not earlier.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] serial: imx: Avoid probe failure in case of missing gpiolib
2019-08-01 8:48 ` Uwe Kleine-König
@ 2019-08-01 9:28 ` Schrempf Frieder
2019-08-01 9:55 ` Uwe Kleine-König
0 siblings, 1 reply; 6+ messages in thread
From: Schrempf Frieder @ 2019-08-01 9:28 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Greg Kroah-Hartman, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
linux-arm-kernel, linux-kernel, linux-serial, Jiri Slaby
Hi Uwe,
On 01.08.19 10:48, Uwe Kleine-König wrote:
> On Thu, Aug 01, 2019 at 08:18:05AM +0000, Schrempf Frieder wrote:
>> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>>
>> If CONFIG_GPIOLIB is not enabled, mctrl_gpio_init() will return
>> -ENOSYS and cause the probing of the imx UART to fail. As the
>> GPIOs are optional, we should continue probing in this case.
>
> Is this really still the case? On which version did you hit this
> problem?
Yes, I think it is. I used v5.2.5, that already has d99482673f95.
>
> I would expect that is gone with
> d99482673f950817b30caf3fcdfb31179b050ce1 if not earlier.
I think this is a different problem. If CONFIG_GPIOLIB is disabled,
mctrl_gpio_init() returns -ENOSYS unconditionally here: [1]. The
existing patch (d99482673f95) seems to handle the case when
CONFIG_GPIOLIB is enabled, but no or not all GPIOs are given in the dtb.
The sh-sci.c driver has a similar check to skip this case: [2].
Regards,
Frieder
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/tty/serial/serial_mctrl_gpio.h#n121
[2]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/tty/serial/sh-sci.c#n3290
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] serial: imx: Avoid probe failure in case of missing gpiolib
2019-08-01 9:28 ` Schrempf Frieder
@ 2019-08-01 9:55 ` Uwe Kleine-König
2019-08-01 10:59 ` Schrempf Frieder
0 siblings, 1 reply; 6+ messages in thread
From: Uwe Kleine-König @ 2019-08-01 9:55 UTC (permalink / raw)
To: Schrempf Frieder
Cc: Greg Kroah-Hartman, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
linux-arm-kernel, linux-kernel, linux-serial, Jiri Slaby
On Thu, Aug 01, 2019 at 09:28:33AM +0000, Schrempf Frieder wrote:
> Hi Uwe,
>
> On 01.08.19 10:48, Uwe Kleine-König wrote:
> > On Thu, Aug 01, 2019 at 08:18:05AM +0000, Schrempf Frieder wrote:
> >> From: Frieder Schrempf <frieder.schrempf@kontron.de>
> >>
> >> If CONFIG_GPIOLIB is not enabled, mctrl_gpio_init() will return
> >> -ENOSYS and cause the probing of the imx UART to fail. As the
> >> GPIOs are optional, we should continue probing in this case.
> >
> > Is this really still the case? On which version did you hit this
> > problem?
>
> Yes, I think it is. I used v5.2.5, that already has d99482673f95.
>
> >
> > I would expect that is gone with
> > d99482673f950817b30caf3fcdfb31179b050ce1 if not earlier.
>
> I think this is a different problem. If CONFIG_GPIOLIB is disabled,
> mctrl_gpio_init() returns -ENOSYS unconditionally here: [1]. The
> existing patch (d99482673f95) seems to handle the case when
> CONFIG_GPIOLIB is enabled, but no or not all GPIOs are given in the dtb.
Ah, I see.
I don't think we should handle this on a per-driver basis. So my
suggestion is to drop the dummy implementation for mctrl_gpio if GPIOLIB
is disabled. Then the behaviour should be consistant with the gpio stuff
returning NULL in this case. (Or alternatively adapt the dummy
implementation to shortcut and behave identically.)
(Having said that I don't like gpiolib's behaviour of returning NULL for
the optional calls if it's disabled, but having mctrl_gpio behave
differently is worse.)
> The sh-sci.c driver has a similar check to skip this case: [2].
This should than be dropped, too.
Best regards
Uwe
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/tty/serial/serial_mctrl_gpio.h#n121
> [2]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/tty/serial/sh-sci.c#n3290
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] serial: imx: Avoid probe failure in case of missing gpiolib
2019-08-01 9:55 ` Uwe Kleine-König
@ 2019-08-01 10:59 ` Schrempf Frieder
2019-08-01 11:50 ` Uwe Kleine-König
0 siblings, 1 reply; 6+ messages in thread
From: Schrempf Frieder @ 2019-08-01 10:59 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Greg Kroah-Hartman, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
linux-arm-kernel, linux-kernel, linux-serial, Jiri Slaby
On 01.08.19 11:55, Uwe Kleine-König wrote:
> On Thu, Aug 01, 2019 at 09:28:33AM +0000, Schrempf Frieder wrote:
>> Hi Uwe,
>>
>> On 01.08.19 10:48, Uwe Kleine-König wrote:
>>> On Thu, Aug 01, 2019 at 08:18:05AM +0000, Schrempf Frieder wrote:
>>>> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>>>>
>>>> If CONFIG_GPIOLIB is not enabled, mctrl_gpio_init() will return
>>>> -ENOSYS and cause the probing of the imx UART to fail. As the
>>>> GPIOs are optional, we should continue probing in this case.
>>>
>>> Is this really still the case? On which version did you hit this
>>> problem?
>>
>> Yes, I think it is. I used v5.2.5, that already has d99482673f95.
>>
>>>
>>> I would expect that is gone with
>>> d99482673f950817b30caf3fcdfb31179b050ce1 if not earlier.
>>
>> I think this is a different problem. If CONFIG_GPIOLIB is disabled,
>> mctrl_gpio_init() returns -ENOSYS unconditionally here: [1]. The
>> existing patch (d99482673f95) seems to handle the case when
>> CONFIG_GPIOLIB is enabled, but no or not all GPIOs are given in the dtb.
>
> Ah, I see.
>
> I don't think we should handle this on a per-driver basis. So my
> suggestion is to drop the dummy implementation for mctrl_gpio if GPIOLIB
> is disabled. Then the behaviour should be consistant with the gpio stuff
> returning NULL in this case. (Or alternatively adapt the dummy
> implementation to shortcut and behave identically.)
I get your point, but it seems a bit strange to go down all the way from
the driver to mctrl_gpio_init_noauto() and into the loop just to return
an empty struct mctrl_gpios to the driver that will be looped over again
on each call of mctrl_gpio_*().
So I would rather go with a variation of your second proposal and keep
the dummy implementation, but let it return NULL instead of an error
pointer, as all the mctrl_gpio_*() functions already seem to have a
check for gpios == NULL.
What do you think?
>
> (Having said that I don't like gpiolib's behaviour of returning NULL for
> the optional calls if it's disabled, but having mctrl_gpio behave
> differently is worse.)
>
>> The sh-sci.c driver has a similar check to skip this case: [2].
>
> This should than be dropped, too.
>
> Best regards
> Uwe
>
>> [1]
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/tty/serial/serial_mctrl_gpio.h#n121
>> [2]
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/tty/serial/sh-sci.c#n3290
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] serial: imx: Avoid probe failure in case of missing gpiolib
2019-08-01 10:59 ` Schrempf Frieder
@ 2019-08-01 11:50 ` Uwe Kleine-König
0 siblings, 0 replies; 6+ messages in thread
From: Uwe Kleine-König @ 2019-08-01 11:50 UTC (permalink / raw)
To: Schrempf Frieder
Cc: Greg Kroah-Hartman, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
linux-arm-kernel, linux-kernel, linux-serial, Jiri Slaby
Hello,
On Thu, Aug 01, 2019 at 10:59:54AM +0000, Schrempf Frieder wrote:
> So I would rather go with a variation of your second proposal and keep
> the dummy implementation, but let it return NULL instead of an error
> pointer, as all the mctrl_gpio_*() functions already seem to have a
> check for gpios == NULL.
>
> What do you think?
I'll gladly review a patch.
Best regads
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-08-01 11:50 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-01 8:18 [PATCH] serial: imx: Avoid probe failure in case of missing gpiolib Schrempf Frieder
2019-08-01 8:48 ` Uwe Kleine-König
2019-08-01 9:28 ` Schrempf Frieder
2019-08-01 9:55 ` Uwe Kleine-König
2019-08-01 10:59 ` Schrempf Frieder
2019-08-01 11:50 ` Uwe Kleine-König
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).