[PATCH 0/3] iio: filter: admv8818: Bug fix and other improvements

[PATCH 0/3] iio: filter: admv8818: Bug fix and other improvements

[Ethan Tidmore]

Fix a missing error code problem with Smatch, transition driver to
use guard(), and resolve all checkpatch warnings.


Ethan Tidmore (3):
  iio: filter: admv8818: Add missing error code
  iio: filter: admv8818: Simplify locking with guard()
  iio: filter: admv8818: Minor cleanups

 drivers/iio/filter/admv8818.c | 69 ++++++++++++-----------------------
 1 file changed, 24 insertions(+), 45 deletions(-)

-- 
2.53.0

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

[Ethan Tidmore]

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

[Ethan Tidmore]

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

[Andy Shevchenko]

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

[David Lechner]

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

[Andy Shevchenko]

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

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

[Andy Shevchenko]

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

[David Lechner]

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.

[PATCH 2/3] iio: filter: admv8818: Simplify locking with guard()

[Ethan Tidmore]

Use guard() instead of manual locking to simplify code.

Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
---
 drivers/iio/filter/admv8818.c | 59 ++++++++++++-----------------------
 1 file changed, 20 insertions(+), 39 deletions(-)

diff --git a/drivers/iio/filter/admv8818.c b/drivers/iio/filter/admv8818.c
index 4d5e7a9d806a..2d12700d717f 100644
--- a/drivers/iio/filter/admv8818.c
+++ b/drivers/iio/filter/admv8818.c
@@ -7,6 +7,7 @@
 
 #include <linux/bitfield.h>
 #include <linux/bits.h>
+#include <linux/cleanup.h>
 #include <linux/clk.h>
 #include <linux/device.h>
 #include <linux/iio/iio.h>
@@ -205,13 +206,8 @@ static int __admv8818_hpf_select(struct admv8818_state *st, u64 freq)
 
 static int admv8818_hpf_select(struct admv8818_state *st, u64 freq)
 {
-	int ret;
-
-	mutex_lock(&st->lock);
-	ret = __admv8818_hpf_select(st, freq);
-	mutex_unlock(&st->lock);
-
-	return ret;
+	guard(mutex)(&st->lock);
+	return __admv8818_hpf_select(st, freq);
 }
 
 static int __admv8818_lpf_select(struct admv8818_state *st, u64 freq)
@@ -281,13 +277,8 @@ static int __admv8818_lpf_select(struct admv8818_state *st, u64 freq)
 
 static int admv8818_lpf_select(struct admv8818_state *st, u64 freq)
 {
-	int ret;
-
-	mutex_lock(&st->lock);
-	ret = __admv8818_lpf_select(st, freq);
-	mutex_unlock(&st->lock);
-
-	return ret;
+	guard(mutex)(&st->lock);
+	return __admv8818_lpf_select(st, freq);
 }
 
 static int admv8818_rfin_band_select(struct admv8818_state *st)
@@ -308,16 +299,17 @@ static int admv8818_rfin_band_select(struct admv8818_state *st)
 	if (lpf_corner_target < st->cf_hz)
 		lpf_corner_target = U64_MAX;
 
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
 
 	ret = __admv8818_hpf_select(st, hpf_corner_target);
 	if (ret)
-		goto exit;
+		return ret;
 
 	ret = __admv8818_lpf_select(st, lpf_corner_target);
-exit:
-	mutex_unlock(&st->lock);
-	return ret;
+	if (ret)
+		return ret;
+
+	return 0;
 }
 
 static int __admv8818_read_hpf_freq(struct admv8818_state *st, u64 *hpf_freq)
@@ -352,13 +344,8 @@ static int __admv8818_read_hpf_freq(struct admv8818_state *st, u64 *hpf_freq)
 
 static int admv8818_read_hpf_freq(struct admv8818_state *st, u64 *hpf_freq)
 {
-	int ret;
-
-	mutex_lock(&st->lock);
-	ret = __admv8818_read_hpf_freq(st, hpf_freq);
-	mutex_unlock(&st->lock);
-
-	return ret;
+	guard(mutex)(&st->lock);
+	return __admv8818_read_hpf_freq(st, hpf_freq);
 }
 
 static int __admv8818_read_lpf_freq(struct admv8818_state *st, u64 *lpf_freq)
@@ -393,13 +380,8 @@ static int __admv8818_read_lpf_freq(struct admv8818_state *st, u64 *lpf_freq)
 
 static int admv8818_read_lpf_freq(struct admv8818_state *st, u64 *lpf_freq)
 {
-	int ret;
-
-	mutex_lock(&st->lock);
-	ret = __admv8818_read_lpf_freq(st, lpf_freq);
-	mutex_unlock(&st->lock);
-
-	return ret;
+	guard(mutex)(&st->lock);
+	return __admv8818_read_lpf_freq(st, lpf_freq);
 }
 
 static int admv8818_write_raw_get_fmt(struct iio_dev *indio_dev,
@@ -485,7 +467,7 @@ static int admv8818_filter_bypass(struct admv8818_state *st)
 {
 	int ret;
 
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
 
 	ret = regmap_update_bits(st->regmap, ADMV8818_REG_WR0_SW,
 				 ADMV8818_SW_IN_SET_WR0_MSK |
@@ -497,18 +479,17 @@ static int admv8818_filter_bypass(struct admv8818_state *st)
 				 FIELD_PREP(ADMV8818_SW_OUT_SET_WR0_MSK, 1) |
 				 FIELD_PREP(ADMV8818_SW_OUT_WR0_MSK, 0));
 	if (ret)
-		goto exit;
+		return ret;
 
 	ret = regmap_update_bits(st->regmap, ADMV8818_REG_WR0_FILTER,
 				 ADMV8818_HPF_WR0_MSK |
 				 ADMV8818_LPF_WR0_MSK,
 				 FIELD_PREP(ADMV8818_HPF_WR0_MSK, 0) |
 				 FIELD_PREP(ADMV8818_LPF_WR0_MSK, 0));
+	if (ret)
+		return ret;
 
-exit:
-	mutex_unlock(&st->lock);
-
-	return ret;
+	return 0;
 }
 
 static int admv8818_get_mode(struct iio_dev *indio_dev,
-- 
2.53.0

Re: [PATCH 2/3] iio: filter: admv8818: Simplify locking with guard()

[Andy Shevchenko]

On Fri, Feb 27, 2026 at 12:14:23AM -0600, Ethan Tidmore wrote:
> Use guard() instead of manual locking to simplify code.

...

>  	ret = __admv8818_lpf_select(st, lpf_corner_target);
> -exit:
> -	mutex_unlock(&st->lock);
> -	return ret;
> +	if (ret)
> +		return ret;
> +
> +	return 0;

Why not

	return __admv8818_lpf_select(...);

?

...

>  	ret = regmap_update_bits(st->regmap, ADMV8818_REG_WR0_FILTER,
>  				 ADMV8818_HPF_WR0_MSK |
>  				 ADMV8818_LPF_WR0_MSK,
>  				 FIELD_PREP(ADMV8818_HPF_WR0_MSK, 0) |
>  				 FIELD_PREP(ADMV8818_LPF_WR0_MSK, 0));
> +	if (ret)
> +		return ret;
>  
> -exit:
> -	mutex_unlock(&st->lock);
> -
> -	return ret;
> +	return 0;

In the similar way?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/3] iio: filter: admv8818: Simplify locking with guard()

[David Lechner]

On 2/27/26 12:14 AM, Ethan Tidmore wrote:
> Use guard() instead of manual locking to simplify code.
> 
> Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
> ---


> @@ -352,13 +344,8 @@ static int __admv8818_read_hpf_freq(struct admv8818_state *st, u64 *hpf_freq)
>  
>  static int admv8818_read_hpf_freq(struct admv8818_state *st, u64 *hpf_freq)
>  {
> -	int ret;
> -
> -	mutex_lock(&st->lock);
> -	ret = __admv8818_read_hpf_freq(st, hpf_freq);
> -	mutex_unlock(&st->lock);
> -
> -	return ret;
> +	guard(mutex)(&st->lock);
> +	return __admv8818_read_hpf_freq(st, hpf_freq);
>  }

admv8818_read_hpf_freq() is the only caller of __admv8818_read_hpf_freq()
so we can drop the wrapper, move the guard to __admv8818_read_hpf_freq()
and rename it to admv8818_read_hpf_freq().

I didn't check all functions, but might be others places where we can do
this as well.

Re: [PATCH 2/3] iio: filter: admv8818: Simplify locking with guard()

[Ethan Tidmore]

On Fri Feb 27, 2026 at 10:08 AM CST, David Lechner wrote:
> On 2/27/26 12:14 AM, Ethan Tidmore wrote:
>> Use guard() instead of manual locking to simplify code.
>> 
>> Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
>> ---
>
>
>> @@ -352,13 +344,8 @@ static int __admv8818_read_hpf_freq(struct admv8818_state *st, u64 *hpf_freq)
>>  
>>  static int admv8818_read_hpf_freq(struct admv8818_state *st, u64 *hpf_freq)
>>  {
>> -	int ret;
>> -
>> -	mutex_lock(&st->lock);
>> -	ret = __admv8818_read_hpf_freq(st, hpf_freq);
>> -	mutex_unlock(&st->lock);
>> -
>> -	return ret;
>> +	guard(mutex)(&st->lock);
>> +	return __admv8818_read_hpf_freq(st, hpf_freq);
>>  }
>
> admv8818_read_hpf_freq() is the only caller of __admv8818_read_hpf_freq()
> so we can drop the wrapper, move the guard to __admv8818_read_hpf_freq()
> and rename it to admv8818_read_hpf_freq().
>
> I didn't check all functions, but might be others places where we can do
> this as well.

So this can be easily done with a few wrappers but these two functions
have this odd scenario:

	guard(mutex)(&st->lock);

	ret = __admv8818_hpf_select(st, hpf_corner_target);
	if (ret)
		return ret;

	return __admv8818_lpf_select(st, lpf_corner_target);

Where if I deleted that guard() and just made those hold the lock
separately this would be a behaviour change. Wondering if there was a
nice way to get around this?

Thanks,

ET

[PATCH 3/3] iio: filter: admv8818: Minor cleanups

[Ethan Tidmore]

Resolve every checkpatch error in the driver.

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

diff --git a/drivers/iio/filter/admv8818.c b/drivers/iio/filter/admv8818.c
index 2d12700d717f..1af263007322 100644
--- a/drivers/iio/filter/admv8818.c
+++ b/drivers/iio/filter/admv8818.c
@@ -240,7 +240,6 @@ static int __admv8818_lpf_select(struct admv8818_state *st, u64 freq)
 		freq_step = div_u64(freq_step, ADMV8818_NUM_STATES - 1);
 
 		for (state = ADMV8818_STATE_MAX; state >= ADMV8818_STATE_MIN; --state) {
-
 			freq_corner = freq_range_lpf[band][ADMV8818_BAND_CORNER_LOW] +
 				      state * freq_step;
 
@@ -385,8 +384,8 @@ static int admv8818_read_lpf_freq(struct admv8818_state *st, u64 *lpf_freq)
 }
 
 static int admv8818_write_raw_get_fmt(struct iio_dev *indio_dev,
-								struct iio_chan_spec const *chan,
-								long mask)
+				      struct iio_chan_spec const *chan,
+				      long mask)
 {
 	switch (mask) {
 	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
@@ -722,7 +721,6 @@ static int admv8818_read_properties(struct admv8818_state *st)
 	else
 		return ret;
 
-
 	ret = device_property_read_u32(&spi->dev, "adi,hpf-margin-mhz", &mhz);
 	if (ret == 0)
 		st->hpf_margin_hz = (u64)mhz * HZ_PER_MHZ;
-- 
2.53.0

Re: [PATCH 3/3] iio: filter: admv8818: Minor cleanups

[Andy Shevchenko]

On Fri, Feb 27, 2026 at 12:14:24AM -0600, Ethan Tidmore wrote:
> Resolve every checkpatch error in the driver.

Poorly written commit message and Subject. Please, be more explicit:

Ex.:

  iio: filter: ...: Fix indentation issues in a couple of places

  Fix indentation issue of the parameters in admv8818_write_raw_get_fmt()
  and remove extra blank lines elsewhere.

Read this: https://chris.beams.io/git-commit

-- 
With Best Regards,
Andy Shevchenko