linux-kernel - Re: [PATCH] serial: imx: Avoid probe failure in case of missing gpiolib

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 1 Aug 2019 11:55:29 +0200
From:   Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>
To:     Schrempf Frieder <frieder.schrempf@...tron.de>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Shawn Guo <shawnguo@...nel.org>,
        Sascha Hauer <s.hauer@...gutronix.de>,
        Pengutronix Kernel Team <kernel@...gutronix.de>,
        Fabio Estevam <festevam@...il.com>,
        NXP Linux Team <linux-imx@....com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-serial@...r.kernel.org" <linux-serial@...r.kernel.org>,
        Jiri Slaby <jslaby@...e.com>
Subject: Re: [PATCH] serial: imx: Avoid probe failure in case of missing
 gpiolib

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@...tron.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/  |