linux-kernel - Re: [PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR

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, 23 Mar 2017 08:44:41 -0700
From:   Dmitry Torokhov <dmitry.torokhov@...il.com>
To:     Linus Walleij <linus.walleij@...aro.org>
Cc:     Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        Richard Genoud <richard.genoud@...il.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Nicolas Ferre <nicolas.ferre@...rochip.com>,
        Boris Brezillon <boris.brezillon@...e-electrons.com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-serial@...r.kernel.org" <linux-serial@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Janusz Uzycki <j.uzycki@...roma.com.pl>,
        "linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>
Subject: Re: [PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR_NULL
 calls

On Thu, Mar 23, 2017 at 07:43:25AM -0700, Dmitry Torokhov wrote:
> On Thu, Mar 23, 2017 at 02:41:53PM +0100, Linus Walleij wrote:
> > On Thu, Mar 23, 2017 at 1:34 PM, Uwe Kleine-König
> > <u.kleine-koenig@...gutronix.de> wrote:
> > 
> > > Maybe we can make gpiod_get_optional look like this:
> > >
> > >         if (!dev->of_node && isnt_a_acpi_device(dev) && !IS_ENABLED(GPIOLIB))
> > >                 return NULL;
> > >         else
> > >                 return -ENOSYS;
> > >
> > > I don't know how isnt_a_acpi_device looks like, probably it involves
> > > CONFIG_ACPI and/or dev->acpi_node.
> > >
> > > This should be safe and still comfortable for legacy platforms, isn't it?
> > 
> > I like the looks of this.
> > 
> > Can we revert Dmitry's patch and apply something like this instead?
> > 
> > Dmitry, how do you feel about this?
> 
> I frankly do not see the point. It still makes driver code more complex
> for no good reason. I also think that not having optional GPIO is not an
> error, so returning value from error space is not correct. NULL is value
> from another space altogether.
> 
> Uwe seems to be concerned about case that I find extremely unlikely. We
> are talking about a system that does not have GPIO support and behaves
> just fine, with the exception that it actually has (physically) a
> *single* GPIO, and that GPIO happens to be optional in a single driver,
> but in this particular system is actually needed (but that need
> manifests in a non-obvious way). And we have system integrator that has
> no idea what they are doing (no schematic, etc).
> 
> I think that if there is one optional GPIO there will be mandatiry GPIOs
> in such system as well and selection of GPIOLIB will be forced early on
> in board bringup.

One more thing: if we keep reporting -ENOSYS in case of !CONFIG_GPIOLIB,
then most of the non platform-sepcific drivers will eventually gain code
silently coping with this -ENOSYS:

	data->gpiod = gpiod_getptional(...);
	if (IS_ERR(data->gpiod)) {
		error = PTR_ERR(data->gpiod);
		if (error != -ENOSYS)
			return error;

		data->gpiod = NULL; /* This GPIO _is_ optional */
	}

which will negate Uwe's claim that it will help debugging issues.

Thanks.

-- 
Dmitry