linux-kernel - [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] [day] [month] [year] [list]

Message-ID: <HE1P101MB01883D37543C63B3B020F91DE60F0@HE1P101MB0188.NAMP101.PROD.OUTLOOK.COM>
Date:   Sat, 8 Apr 2017 07:38:50 +0000
From:   "Han, Nandor (GE Healthcare)" <nandor.han@...com>
To:     Linus Walleij <linus.walleij@...aro.org>
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: [PATCH 1/3] gpio - Add EXAR XRA1403 SPI GPIO expander driver



> -----Original Message-----
> From: Linus Walleij [mailto:linus.walleij@...aro.org]
> Sent: 07 April 2017 13:07
> 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; devicetree@...r.kernel.org; linux-kernel@...r.kernel.org;
> Malinen, Semi (GE Healthcare) <semi.malinen@...com>
> Subject: EXT: 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.
> 

That was my first intention to release the reset line in case somebody else wants to control it. I did it
like that because usually reset line controls multiple devices and probably some upper layer wants to
control that. 

After your comment I did some analysing and I will follow your advice and change the reset line handling. 
Once the GPIO is provided to the driver the driver will own it and bring out the device from reset. In case not
provided the reset line is somebody else responsibility. 

This way we are able to cover multiple use-cases. 

Thanks Linus,
Nandy

> Yours,
> Linus Walleij