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]

Message-ID: <CACRpkdYBesafyhp0=W-cnOncYJ-1eNHZ4E_nv_f0AkWrhUk41Q@mail.gmail.com>
Date:   Fri, 24 Mar 2017 09:58:28 +0100
From:   Linus Walleij <linus.walleij@...aro.org>
To:     Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>
Cc:     Dmitry Torokhov <dmitry.torokhov@...il.com>,
        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 8:10 PM, Uwe Kleine-König
<u.kleine-koenig@...gutronix.de> wrote:
> On Thu, Mar 23, 2017 at 08:44:41AM -0700, Dmitry Torokhov wrote:
>> 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
>
> Note that this code is in the gpio header, and not in driver code. So
> the driver just does
>
>         gpiod = gpiod_get_optional(...)
>         if (IS_ERR(gpiod))
>                 return PTR_ERR(gpiod);
>
> (as it is supposed to do now). I think that's nice.

It does look nice. Compare this to what we must do for optional regulators:

        st->reg = devm_regulator_get_optional(&spi->dev, "vref");
        if (IS_ERR(st->reg)) {
                /* Any other error indicates that the regulator does exist */
                if (PTR_ERR(st->reg) != -ENODEV)
                        return PTR_ERR(st->reg);
                /* Use internal reference */
        }

So for optional regulators we get -ENODEV if we don't have it,
and then proceed on an alternate path, such as using an internal
reference voltage.

However the fact that regulators and GPIO optionals are already
handled differently is creating "cognitive dissonance" or what I
should call it.

Yours,
Linus Walleij