iio: filter: admv8818: Bug fix and other improvements

[PATCH 1/3] iio: filter: admv8818: Add missing error code

Posted by Ethan Tidmore 1 month, 1 week ago

If the macro FIELD_GET() returns an error code the function returns
with 0 because ret was just confirmed to be 0 a few lines above.

Add error code.

Detected by Smatch:
drivers/iio/filter/admv8818.c:335 __admv8818_read_hpf_freq() warn:
missing error code? 'ret'

drivers/iio/filter/admv8818.c:376 __admv8818_read_lpf_freq()
warn: missing error code? 'ret'

Fixes: f34fe888ad054 ("iio:filter:admv8818: add support for ADMV8818")
Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
---
 drivers/iio/filter/admv8818.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/filter/admv8818.c b/drivers/iio/filter/admv8818.c
index e494fd33911b..4d5e7a9d806a 100644
--- a/drivers/iio/filter/admv8818.c
+++ b/drivers/iio/filter/admv8818.c
@@ -332,7 +332,7 @@ static int __admv8818_read_hpf_freq(struct admv8818_state *st, u64 *hpf_freq)
 	hpf_band = FIELD_GET(ADMV8818_SW_IN_WR0_MSK, data);
 	if (!hpf_band || hpf_band > 4) {
 		*hpf_freq = 0;
-		return ret;
+		return -EINVAL;
 	}
 
 	ret = regmap_read(st->regmap, ADMV8818_REG_WR0_FILTER, &data);
@@ -373,7 +373,7 @@ static int __admv8818_read_lpf_freq(struct admv8818_state *st, u64 *lpf_freq)
 	lpf_band = FIELD_GET(ADMV8818_SW_OUT_WR0_MSK, data);
 	if (!lpf_band || lpf_band > 4) {
 		*lpf_freq = 0;
-		return ret;
+		return -EINVAL;
 	}
 
 	ret = regmap_read(st->regmap, ADMV8818_REG_WR0_FILTER, &data);
-- 
2.53.0

Re: [PATCH 1/3] iio: filter: admv8818: Add missing error code

Posted by David Lechner 1 month, 1 week ago

On 2/27/26 12:14 AM, Ethan Tidmore wrote:
> If the macro FIELD_GET() returns an error code the function returns
> with 0 because ret was just confirmed to be 0 a few lines above.
> 
> Add error code.
> 
> Detected by Smatch:
> drivers/iio/filter/admv8818.c:335 __admv8818_read_hpf_freq() warn:
> missing error code? 'ret'
> 
> drivers/iio/filter/admv8818.c:376 __admv8818_read_lpf_freq()
> warn: missing error code? 'ret'
> 
> Fixes: f34fe888ad054 ("iio:filter:admv8818: add support for ADMV8818")
> Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
> ---
>  drivers/iio/filter/admv8818.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/filter/admv8818.c b/drivers/iio/filter/admv8818.c
> index e494fd33911b..4d5e7a9d806a 100644
> --- a/drivers/iio/filter/admv8818.c
> +++ b/drivers/iio/filter/admv8818.c
> @@ -332,7 +332,7 @@ static int __admv8818_read_hpf_freq(struct admv8818_state *st, u64 *hpf_freq)
>  	hpf_band = FIELD_GET(ADMV8818_SW_IN_WR0_MSK, data);
>  	if (!hpf_band || hpf_band > 4) {
>  		*hpf_freq = 0;
> -		return ret;
> +		return -EINVAL;
>  	}
>  
>  	ret = regmap_read(st->regmap, ADMV8818_REG_WR0_FILTER, &data);
> @@ -373,7 +373,7 @@ static int __admv8818_read_lpf_freq(struct admv8818_state *st, u64 *lpf_freq)
>  	lpf_band = FIELD_GET(ADMV8818_SW_OUT_WR0_MSK, data);
>  	if (!lpf_band || lpf_band > 4) {
>  		*lpf_freq = 0;
> -		return ret;
> +		return -EINVAL;
>  	}
>  
>  	ret = regmap_read(st->regmap, ADMV8818_REG_WR0_FILTER, &data);

Probably for a separate patch since this is considered a fix...

The returns at the end of these functions should be changed to
return 0. It is the only possible value ret can have at that point.

Re: [PATCH 1/3] iio: filter: admv8818: Add missing error code

Posted by Andy Shevchenko 1 month, 1 week ago

On Fri, Feb 27, 2026 at 12:14:22AM -0600, Ethan Tidmore wrote:
> If the macro FIELD_GET() returns an error code the function returns
> with 0 because ret was just confirmed to be 0 a few lines above.
> 
> Add error code.
> 
> Detected by Smatch:
> drivers/iio/filter/admv8818.c:335 __admv8818_read_hpf_freq() warn:
> missing error code? 'ret'
> 
> drivers/iio/filter/admv8818.c:376 __admv8818_read_lpf_freq()
> warn: missing error code? 'ret'

...

> @@ -332,7 +332,7 @@ static int __admv8818_read_hpf_freq(struct admv8818_state *st, u64 *hpf_freq)
>  	hpf_band = FIELD_GET(ADMV8818_SW_IN_WR0_MSK, data);
>  	if (!hpf_band || hpf_band > 4) {
>  		*hpf_freq = 0;
> -		return ret;
> +		return -EINVAL;

Looking at the nature of the check I would rather change the condition
to use in_range() and respectively return -ERANGE.

>  	}

...

>  	lpf_band = FIELD_GET(ADMV8818_SW_OUT_WR0_MSK, data);
>  	if (!lpf_band || lpf_band > 4) {
>  		*lpf_freq = 0;
> -		return ret;
> +		return -EINVAL;
>  	}

Ditto.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/3] iio: filter: admv8818: Add missing error code

Posted by Ethan Tidmore 1 month, 1 week ago

On Fri Feb 27, 2026 at 12:14 AM CST, Ethan Tidmore wrote:
> If the macro FIELD_GET() returns an error code the function returns
> with 0 because ret was just confirmed to be 0 a few lines above.
>
> Add error code.
>
> Detected by Smatch:
> drivers/iio/filter/admv8818.c:335 __admv8818_read_hpf_freq() warn:
> missing error code? 'ret'
>
> drivers/iio/filter/admv8818.c:376 __admv8818_read_lpf_freq()
> warn: missing error code? 'ret'
>
> Fixes: f34fe888ad054 ("iio:filter:admv8818: add support for ADMV8818")
> Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
> ---

Just realized I should have used -EIO for the error code. Will wait
however on reviews before v2.

Thanks,

ET

Re: [PATCH 1/3] iio: filter: admv8818: Add missing error code

Posted by Andy Shevchenko 1 month, 1 week ago

On Fri, Feb 27, 2026 at 12:42:11AM -0600, Ethan Tidmore wrote:
> On Fri Feb 27, 2026 at 12:14 AM CST, Ethan Tidmore wrote:

...

> Just realized I should have used -EIO for the error code. Will wait
> however on reviews before v2.

Why -EIO? Is it used in the same situation in other cases in this driver?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/3] iio: filter: admv8818: Add missing error code

Posted by David Lechner 1 month, 1 week ago

On 2/27/26 1:06 AM, Andy Shevchenko wrote:
> On Fri, Feb 27, 2026 at 12:42:11AM -0600, Ethan Tidmore wrote:
>> On Fri Feb 27, 2026 at 12:14 AM CST, Ethan Tidmore wrote:
> 
> ...
> 
>> Just realized I should have used -EIO for the error code. Will wait
>> however on reviews before v2.
> 
> Why -EIO? Is it used in the same situation in other cases in this driver?
> 

I think EIO makes sense here. We are reading from a chip. And if the
values we read aren't one of the expected values, the mostly likely
reason is the chip is malfunctioning or noise on the wires.

Re: [PATCH 1/3] iio: filter: admv8818: Add missing error code

Posted by Andy Shevchenko 1 month, 1 week ago

On Fri, Feb 27, 2026 at 10:01:25AM -0600, David Lechner wrote:
> On 2/27/26 1:06 AM, Andy Shevchenko wrote:
> > On Fri, Feb 27, 2026 at 12:42:11AM -0600, Ethan Tidmore wrote:
> >> On Fri Feb 27, 2026 at 12:14 AM CST, Ethan Tidmore wrote:

...

> >> Just realized I should have used -EIO for the error code. Will wait
> >> however on reviews before v2.
> > 
> > Why -EIO? Is it used in the same situation in other cases in this driver?
> 
> I think EIO makes sense here. We are reading from a chip. And if the
> values we read aren't one of the expected values, the mostly likely
> reason is the chip is malfunctioning or noise on the wires.

Makes sense, but needs to be properly spoken in the commit message.

-- 
With Best Regards,
Andy Shevchenko