Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1619239

[PATCH 1/3] gpio - Add EXAR XRA1403 SPI GPIO expander driver

Show all headers | View raw

> -----Original Message-----
> From: Linus Walleij [mailto:linus.walleij@linaro.org]
> Sent: 07 April 2017 13:07
> To: Han, Nandor (GE Healthcare) <nandor.han@ge.com>
> Cc: Alexandre Courbot <gnurou@gmail.com>; Rob Herring <robh+dt@kernel.org>; Mark Rutland
> <mark.rutland@arm.com>; linux-gpio@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> Malinen, Semi (GE Healthcare) <semi.malinen@ge.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@ge.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

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

Re: [PATCH 1/3] gpio - Add EXAR XRA1403 SPI GPIO expander driver Linus Walleij <linus.walleij@linaro.org> - 2017-03-29 04:10 +0200
  [PATCH 1/3] gpio - Add EXAR XRA1403 SPI GPIO expander driver "Han, Nandor (GE Healthcare)" <nandor.han@ge.com> - 2017-04-05 15:30 +0200
    Re: [PATCH 1/3] gpio - Add EXAR XRA1403 SPI GPIO expander driver Linus Walleij <linus.walleij@linaro.org> - 2017-04-07 12:10 +0200
      [PATCH 1/3] gpio - Add EXAR XRA1403 SPI GPIO expander driver "Han, Nandor (GE Healthcare)" <nandor.han@ge.com> - 2017-04-08 09:40 +0200

csiph-web