[PATCH 1/2] iio: adc: ti-ads1298: Fix error code in probe()

[PATCH 1/2] iio: adc: ti-ads1298: Fix error code in probe()

[Dan Carpenter]

There is a copy and paste bug here, it should be "reg_vref" instead of
"reg_avdd".  The "priv->reg_avdd" variable is zero so it ends up
returning success.

Fixes: 00ef7708fa60 ("iio: adc: ti-ads1298: Add driver")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
 drivers/iio/adc/ti-ads1298.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/adc/ti-ads1298.c b/drivers/iio/adc/ti-ads1298.c
index ed895a30beed..67637f1abdc7 100644
--- a/drivers/iio/adc/ti-ads1298.c
+++ b/drivers/iio/adc/ti-ads1298.c
@@ -657,7 +657,7 @@ static int ads1298_probe(struct spi_device *spi)
 	priv->reg_vref = devm_regulator_get_optional(dev, "vref");
 	if (IS_ERR(priv->reg_vref)) {
 		if (PTR_ERR(priv->reg_vref) != -ENODEV)
-			return dev_err_probe(dev, PTR_ERR(priv->reg_avdd),
+			return dev_err_probe(dev, PTR_ERR(priv->reg_vref),
 					     "Failed to get vref regulator\n");
 
 		priv->reg_vref = NULL;
-- 
2.43.0

[PATCH 2/2] iio: adc: ti-ads1298: prevent divide by zero in ads1298_set_samp_freq()

[Dan Carpenter]

The "val" variable comes from the user so we need to ensure that it's not
zero.  In fact, all negative values are invalid as well.  Add a check for
that.

Fixes: 00ef7708fa60 ("iio: adc: ti-ads1298: Add driver")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
I wrote this commit message in a very confident tone of voice and I have
spent a long time looking at this code so I'm reasonably sure this
patch is correct.  However, I'm not super familiar with this subsystem
so probably you should double check.

 drivers/iio/adc/ti-ads1298.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iio/adc/ti-ads1298.c b/drivers/iio/adc/ti-ads1298.c
index 67637f1abdc7..1d1eaba3d6d1 100644
--- a/drivers/iio/adc/ti-ads1298.c
+++ b/drivers/iio/adc/ti-ads1298.c
@@ -258,6 +258,8 @@ static int ads1298_set_samp_freq(struct ads1298_private *priv, int val)
 		rate = ADS1298_CLK_RATE_HZ;
 	if (!rate)
 		return -EINVAL;
+	if (val <= 0)
+		return -EINVAL;
 
 	factor = (rate >> ADS1298_SHIFT_DR_HR) / val;
 	if (factor >= BIT(ADS1298_SHIFT_DR_LP))
-- 
2.43.0

Re: [PATCH 1/2] iio: adc: ti-ads1298: Fix error code in probe()

[Mike Looijmans]

Good catch on both patches.

If so desired, you have my

Acked-by: Mike Looijmans <mike.looijmans@topic.nl>



On 22-02-2024 07:14, Dan Carpenter wrote:
> There is a copy and paste bug here, it should be "reg_vref" instead of
> "reg_avdd".  The "priv->reg_avdd" variable is zero so it ends up
> returning success.
>
> Fixes: 00ef7708fa60 ("iio: adc: ti-ads1298: Add driver")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
>   drivers/iio/adc/ti-ads1298.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/adc/ti-ads1298.c b/drivers/iio/adc/ti-ads1298.c
> index ed895a30beed..67637f1abdc7 100644
> --- a/drivers/iio/adc/ti-ads1298.c
> +++ b/drivers/iio/adc/ti-ads1298.c
> @@ -657,7 +657,7 @@ static int ads1298_probe(struct spi_device *spi)
>   	priv->reg_vref = devm_regulator_get_optional(dev, "vref");
>   	if (IS_ERR(priv->reg_vref)) {
>   		if (PTR_ERR(priv->reg_vref) != -ENODEV)
> -			return dev_err_probe(dev, PTR_ERR(priv->reg_avdd),
> +			return dev_err_probe(dev, PTR_ERR(priv->reg_vref),
>   					     "Failed to get vref regulator\n");
>   
>   		priv->reg_vref = NULL;


-- 
Mike Looijmans
System Expert

TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands

T: +31 (0) 499 33 69 69
E: mike.looijmans@topic.nl
W: www.topic.nl

Re: [PATCH 1/2] iio: adc: ti-ads1298: Fix error code in probe()

[Jonathan Cameron]

On Thu, 22 Feb 2024 08:10:25 +0100
Mike Looijmans <mike.looijmans@topic.nl> wrote:

> Good catch on both patches.
> 
> If so desired, you have my
> 
> Acked-by: Mike Looijmans <mike.looijmans@topic.nl>

Dan, here is a classic example of why I think any series with more than
1 patch could benefit from a cover letter. It gives somewhere for
reviewers to give tags for the lot in a fashion b4 can understand.

Otherwise great find and applied to the togreg branch of iio.git
with Mike's tag added to both of them!  Hopefully the fixes tags will
remain stable - whilst in theory that tree doesn't get rebased, in practice
it might if I messed anything up enough :( 

Thanks

Jonathan


> 
> 
> 
> On 22-02-2024 07:14, Dan Carpenter wrote:
> > There is a copy and paste bug here, it should be "reg_vref" instead of
> > "reg_avdd".  The "priv->reg_avdd" variable is zero so it ends up
> > returning success.
> >
> > Fixes: 00ef7708fa60 ("iio: adc: ti-ads1298: Add driver")
> > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> > ---
> >   drivers/iio/adc/ti-ads1298.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/iio/adc/ti-ads1298.c b/drivers/iio/adc/ti-ads1298.c
> > index ed895a30beed..67637f1abdc7 100644
> > --- a/drivers/iio/adc/ti-ads1298.c
> > +++ b/drivers/iio/adc/ti-ads1298.c
> > @@ -657,7 +657,7 @@ static int ads1298_probe(struct spi_device *spi)
> >   	priv->reg_vref = devm_regulator_get_optional(dev, "vref");
> >   	if (IS_ERR(priv->reg_vref)) {
> >   		if (PTR_ERR(priv->reg_vref) != -ENODEV)
> > -			return dev_err_probe(dev, PTR_ERR(priv->reg_avdd),
> > +			return dev_err_probe(dev, PTR_ERR(priv->reg_vref),
> >   					     "Failed to get vref regulator\n");
> >   
> >   		priv->reg_vref = NULL;  
> 
> 

Re: [PATCH 1/2] iio: adc: ti-ads1298: Fix error code in probe()

[Dan Carpenter]

On Sat, Feb 24, 2024 at 06:13:54PM +0000, Jonathan Cameron wrote:
> On Thu, 22 Feb 2024 08:10:25 +0100
> Mike Looijmans <mike.looijmans@topic.nl> wrote:
> 
> > Good catch on both patches.
> > 
> > If so desired, you have my
> > 
> > Acked-by: Mike Looijmans <mike.looijmans@topic.nl>
> 
> Dan, here is a classic example of why I think any series with more than
> 1 patch could benefit from a cover letter. It gives somewhere for
> reviewers to give tags for the lot in a fashion b4 can understand.
> 
> Otherwise great find and applied to the togreg branch of iio.git
> with Mike's tag added to both of them!  Hopefully the fixes tags will
> remain stable - whilst in theory that tree doesn't get rebased, in practice
> it might if I messed anything up enough :( 

Sure.  I can start writing cover letters.

regards,
dan carpenter

Re: [PATCH 1/2] iio: adc: ti-ads1298: Fix error code in probe()

[Andy Shevchenko]

On Sat, Feb 24, 2024 at 06:13:54PM +0000, Jonathan Cameron wrote:
> On Thu, 22 Feb 2024 08:10:25 +0100
> Mike Looijmans <mike.looijmans@topic.nl> wrote:
> 
> > Good catch on both patches.
> > 
> > If so desired, you have my
> > 
> > Acked-by: Mike Looijmans <mike.looijmans@topic.nl>
> 
> Dan, here is a classic example of why I think any series with more than
> 1 patch could benefit from a cover letter. It gives somewhere for
> reviewers to give tags for the lot in a fashion b4 can understand.
> 
> Otherwise great find and applied to the togreg branch of iio.git
> with Mike's tag added to both of them!  Hopefully the fixes tags will
> remain stable - whilst in theory that tree doesn't get rebased, in practice
> it might if I messed anything up enough :( 

I even added this to be automatically done when I use my script [1].
Dunno if b4 relay (or what is there to send patch series?) can handle
that.

[1]: https://github.com/andy-shev/home-bin-tools/blob/master/ge2maintainer.sh

-- 
With Best Regards,
Andy Shevchenko