linux-kernel - Re: [PATCH 1/3] gpio - Add EXAR XRA1403 SPI GPIO expander driver

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:   Fri, 7 Apr 2017 12:07:26 +0200
From:   Linus Walleij <linus.walleij@...aro.org>
To:     "Han, Nandor (GE Healthcare)" <nandor.han@...com>
Cc:     Alexandre Courbot <gnurou@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        "linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Malinen, Semi (GE Healthcare)" <semi.malinen@...com>
Subject: Re: [PATCH 1/3] gpio - Add EXAR XRA1403 SPI GPIO expander driver

On Wed, Apr 5, 2017 at 3:24 PM, Han, Nandor (GE Healthcare)
<nandor.han@...com> wrote:
> [Me]
>> > +       /* bring the chip out of reset */
>> > +       reset_gpio = gpiod_get_optional(&spi->dev, "reset", GPIOD_OUT_LOW);
>> > +       if (IS_ERR(reset_gpio))
>> > +               dev_warn(&spi->dev, "could not get reset-gpios\n");
>> > +       else if (reset_gpio)
>> > +               gpiod_put(reset_gpio);
>>
>> I don't think you should put it, other than in the remove()
>> function and in that case you need to have it in the
>> state container.
>
> Can you please be more explicit here.
>
> Currently I'm trying to bring the device out from reset in case reset GPIO is provided.
> I don't see how this could be done in remove()  :)

If you issue gpiod_put() you release the GPIO hande so something else
can go in and grab the GPIO and assert the reset.

This is not what you want to make possible: you want to hold this gpiod handle
as long as the driver is running. devm_gpiod_get_optional() will do the
trick if you don't want to put the handle under explicit control.

Yours,
Linus Walleij