linux-iio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] iio: amplifiers: ad8366: Change devm_gpiod_get() to optional and add the missed check
@ 2020-06-03  9:26 Chuhong Yuan
  2020-06-03 10:19 ` Ardelean, Alexandru
  0 siblings, 1 reply; 4+ messages in thread
From: Chuhong Yuan @ 2020-06-03  9:26 UTC (permalink / raw)
  Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	Hartmut Knaack, Peter Meerwald-Stadler, Alexandru Ardelean,
	Paul Cercueil, linux-iio, linux-kernel, Chuhong Yuan

Since if there is no GPIO, nothing happens, replace devm_gpiod_get()
with devm_gpiod_get_optional().
Also add IS_ERR() to fix the missing-check bug.

Fixes: cee211f4e5a0 ("iio: amplifiers: ad8366: Add support for the ADA4961 DGA")
Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
---
Changes in v3:
  - Change devm_gpiod_get() to optional.
  - Modify description.

 drivers/iio/amplifiers/ad8366.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/amplifiers/ad8366.c b/drivers/iio/amplifiers/ad8366.c
index 62167b87caea..8819e8997f76 100644
--- a/drivers/iio/amplifiers/ad8366.c
+++ b/drivers/iio/amplifiers/ad8366.c
@@ -262,8 +262,12 @@ static int ad8366_probe(struct spi_device *spi)
 	case ID_ADA4961:
 	case ID_ADL5240:
 	case ID_HMC1119:
-		st->reset_gpio = devm_gpiod_get(&spi->dev, "reset",
+		st->reset_gpio = devm_gpiod_get_optional(&spi->dev, "reset",
 			GPIOD_OUT_HIGH);
+		if (IS_ERR(st->reset_gpio)) {
+			ret = PTR_ERR(st->reset_gpio);
+			goto error_disable_reg;
+		}
 		indio_dev->channels = ada4961_channels;
 		indio_dev->num_channels = ARRAY_SIZE(ada4961_channels);
 		break;
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH v3] iio: amplifiers: ad8366: Change devm_gpiod_get() to optional and add the missed check
  2020-06-03  9:26 [PATCH v3] iio: amplifiers: ad8366: Change devm_gpiod_get() to optional and add the missed check Chuhong Yuan
@ 2020-06-03 10:19 ` Ardelean, Alexandru
  2020-06-06 15:12   ` Jonathan Cameron
  0 siblings, 1 reply; 4+ messages in thread
From: Ardelean, Alexandru @ 2020-06-03 10:19 UTC (permalink / raw)
  To: hslester96
  Cc: pmeerw, linux-iio, lars, linux-kernel, paul.cercueil, Hennerich,
	Michael, knaack.h, jic23

On Wed, 2020-06-03 at 17:26 +0800, Chuhong Yuan wrote:
> Since if there is no GPIO, nothing happens, replace devm_gpiod_get()
> with devm_gpiod_get_optional().
> Also add IS_ERR() to fix the missing-check bug.
> 

Acked-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

> Fixes: cee211f4e5a0 ("iio: amplifiers: ad8366: Add support for the ADA4961
> DGA")
> Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
> ---
> Changes in v3:
>   - Change devm_gpiod_get() to optional.
>   - Modify description.
> 
>  drivers/iio/amplifiers/ad8366.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/amplifiers/ad8366.c b/drivers/iio/amplifiers/ad8366.c
> index 62167b87caea..8819e8997f76 100644
> --- a/drivers/iio/amplifiers/ad8366.c
> +++ b/drivers/iio/amplifiers/ad8366.c
> @@ -262,8 +262,12 @@ static int ad8366_probe(struct spi_device *spi)
>  	case ID_ADA4961:
>  	case ID_ADL5240:
>  	case ID_HMC1119:
> -		st->reset_gpio = devm_gpiod_get(&spi->dev, "reset",
> +		st->reset_gpio = devm_gpiod_get_optional(&spi->dev, "reset",
>  			GPIOD_OUT_HIGH);
> +		if (IS_ERR(st->reset_gpio)) {
> +			ret = PTR_ERR(st->reset_gpio);
> +			goto error_disable_reg;
> +		}
>  		indio_dev->channels = ada4961_channels;
>  		indio_dev->num_channels = ARRAY_SIZE(ada4961_channels);
>  		break;

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH v3] iio: amplifiers: ad8366: Change devm_gpiod_get() to optional and add the missed check
  2020-06-03 10:19 ` Ardelean, Alexandru
@ 2020-06-06 15:12   ` Jonathan Cameron
  0 siblings, 0 replies; 4+ messages in thread
From: Jonathan Cameron @ 2020-06-06 15:12 UTC (permalink / raw)
  To: Ardelean, Alexandru
  Cc: hslester96, pmeerw, linux-iio, lars, linux-kernel, paul.cercueil,
	Hennerich, Michael, knaack.h

On Wed, 3 Jun 2020 10:19:12 +0000
"Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote:

> On Wed, 2020-06-03 at 17:26 +0800, Chuhong Yuan wrote:
> > Since if there is no GPIO, nothing happens, replace devm_gpiod_get()
> > with devm_gpiod_get_optional().
> > Also add IS_ERR() to fix the missing-check bug.
> >   
> 
> Acked-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

Applied with layout tweaked a bit as per Markus' comment which made
sense in this particular case.

I also tweaked the description to say "missing-check" warning given
as we've established the failure to check for an error wasn't a problem
as such.  On that basis I thought about dropping the fixes tag, but
it's 'sort' of a fix so I'll leave it there.  Not sending this the
fast path however as is not urgent (shout if you disagree!)

Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.

Thanks,

Jonathan

> 
> > Fixes: cee211f4e5a0 ("iio: amplifiers: ad8366: Add support for the ADA4961
> > DGA")
> > Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
> > ---
> > Changes in v3:
> >   - Change devm_gpiod_get() to optional.
> >   - Modify description.
> > 
> >  drivers/iio/amplifiers/ad8366.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iio/amplifiers/ad8366.c b/drivers/iio/amplifiers/ad8366.c
> > index 62167b87caea..8819e8997f76 100644
> > --- a/drivers/iio/amplifiers/ad8366.c
> > +++ b/drivers/iio/amplifiers/ad8366.c
> > @@ -262,8 +262,12 @@ static int ad8366_probe(struct spi_device *spi)
> >  	case ID_ADA4961:
> >  	case ID_ADL5240:
> >  	case ID_HMC1119:
> > -		st->reset_gpio = devm_gpiod_get(&spi->dev, "reset",
> > +		st->reset_gpio = devm_gpiod_get_optional(&spi->dev, "reset",
> >  			GPIOD_OUT_HIGH);
> > +		if (IS_ERR(st->reset_gpio)) {
> > +			ret = PTR_ERR(st->reset_gpio);
> > +			goto error_disable_reg;
> > +		}
> >  		indio_dev->channels = ada4961_channels;
> >  		indio_dev->num_channels = ARRAY_SIZE(ada4961_channels);
> >  		break;  


^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH v3] iio: amplifiers: ad8366: Change devm_gpiod_get() to optional and add the missed check
@ 2020-06-03 18:00 Markus Elfring
  0 siblings, 0 replies; 4+ messages in thread
From: Markus Elfring @ 2020-06-03 18:00 UTC (permalink / raw)
  To: Chuhong Yuan, linux-iio
  Cc: Michael Hennerich, Alexandru Ardelean, Jonathan Cameron,
	Hartmut Knaack, Lars-Peter Clausen, Paul Cercueil,
	Peter Meerwald-Stadler, linux-kernel

> Since if there is no GPIO, nothing happens, replace devm_gpiod_get()
> with devm_gpiod_get_optional().

…
> +++ b/drivers/iio/amplifiers/ad8366.c
> @@ -262,8 +262,12 @@  static int ad8366_probe(struct spi_device *spi)
…
>  	case ID_HMC1119:
> -		st->reset_gpio = devm_gpiod_get(&spi->dev, "reset",
> +		st->reset_gpio = devm_gpiod_get_optional(&spi->dev, "reset",
>  			GPIOD_OUT_HIGH);
…

I suggest to choose an other source code formatting for this statement.

A)
+		st->reset_gpio = devm_gpiod_get_optional(&spi->dev, "reset",
+							 GPIOD_OUT_HIGH);

B)
+		st->reset_gpio = devm_gpiod_get_optional(&spi->dev, "reset", GPIOD_OUT_HIGH);
-			GPIOD_OUT_HIGH);


Regards,
Markus

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-06-06 15:13 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-03  9:26 [PATCH v3] iio: amplifiers: ad8366: Change devm_gpiod_get() to optional and add the missed check Chuhong Yuan
2020-06-03 10:19 ` Ardelean, Alexandru
2020-06-06 15:12   ` Jonathan Cameron
2020-06-03 18:00 Markus Elfring

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).