[PATCH] iio: frequency: ad9523: check return value of ad9523_io_update()

[PATCH] iio: frequency: ad9523: check return value of ad9523_io_update()

[Bhargav Joshi]

Several functions in ad9523 driver calls ad9523_io_update() to apply
register changes but fail to check its return value.

If the function fails, driver silently ignore the error and returns
success. This results in user-space assuming register changes even if
they are failed.

Fix this by properly catching and returning the error code from
ad9523_io_update() in ad9523_sync(), ad9523_write_raw(), and
ad9523_reg_access()

Signed-off-by: Bhargav Joshi <rougueprince47@gmail.com>
---
 drivers/iio/frequency/ad9523.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/frequency/ad9523.c b/drivers/iio/frequency/ad9523.c
index 63c485e9e44c..66bba37fb199 100644
--- a/drivers/iio/frequency/ad9523.c
+++ b/drivers/iio/frequency/ad9523.c
@@ -495,7 +495,10 @@ static int ad9523_sync(struct iio_dev *indio_dev)
 	if (ret < 0)
 		return ret;
 
-	ad9523_io_update(indio_dev);
+	ret = ad9523_io_update(indio_dev);
+	if (ret < 0)
+		return ret;
+
 	tmp &= ~AD9523_STATUS_SIGNALS_SYNC_MAN_CTRL;
 
 	ret = ad9523_write(indio_dev, AD9523_STATUS_SIGNALS, tmp);
@@ -716,7 +719,7 @@ static int ad9523_write_raw(struct iio_dev *indio_dev,
 	if (ret < 0)
 		goto out;
 
-	ad9523_io_update(indio_dev);
+	ret = ad9523_io_update(indio_dev);
 out:
 	mutex_unlock(&st->lock);
 	return ret;
@@ -732,7 +735,9 @@ static int ad9523_reg_access(struct iio_dev *indio_dev,
 	mutex_lock(&st->lock);
 	if (readval == NULL) {
 		ret = ad9523_write(indio_dev, reg | AD9523_R1B, writeval);
-		ad9523_io_update(indio_dev);
+		if (ret < 0)
+			goto out_unlock;
+		ret = ad9523_io_update(indio_dev);
 	} else {
 		ret = ad9523_read(indio_dev, reg | AD9523_R1B);
 		if (ret < 0)
-- 
2.53.0

Re: [PATCH] iio: frequency: ad9523: check return value of ad9523_io_update()

[Andy Shevchenko]

On Wed, Mar 18, 2026 at 04:55:52AM +0530, Bhargav Joshi wrote:
> Several functions in ad9523 driver calls ad9523_io_update() to apply
> register changes but fail to check its return value.
> 
> If the function fails, driver silently ignore the error and returns
> success. This results in user-space assuming register changes even if
> they are failed.
> 
> Fix this by properly catching and returning the error code from
> ad9523_io_update() in ad9523_sync(), ad9523_write_raw(), and
> ad9523_reg_access()

Missing period at the end.

...

So, now the errors that were non-fatal become fatal. Is it a problem from
functional behaviour point of view? How did you test this?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] iio: frequency: ad9523: check return value of ad9523_io_update()

[Bhargav Joshi]

hi,

On Wed, Mar 18, 2026 at 1:40 PM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Wed, Mar 18, 2026 at 04:55:52AM +0530, Bhargav Joshi wrote:
> > Several functions in ad9523 driver calls ad9523_io_update() to apply
> > register changes but fail to check its return value.
> >
> > If the function fails, driver silently ignore the error and returns
> > success. This results in user-space assuming register changes even if
> > they are failed.
> >
> > Fix this by properly catching and returning the error code from
> > ad9523_io_update() in ad9523_sync(), ad9523_write_raw(), and
> > ad9523_reg_access()
>
> Missing period at the end.
>
> ...
>
> So, now the errors that were non-fatal become fatal. Is it a problem from
> functional behaviour point of view? How did you test this?
>

Regarding errors become fatal, is true for function ad9523_sync()
because even if error occurred it clears the sync bit and calls second
io_update anyways so returning early here would make it worse, Apologies
for this oversight on my part.

However for the  ad9523_write_raw() and ad9523_reg_access()
ad9523_io_update() is the last step and Returning the error in these
cases seems strictly correct to prevent silently ignoring I/O failures
and returning false success to user-space regarding the active
configuration.

Regarding testing: I do not have access to the physical hardware, so the
patch is compile-tested only.

Would you like to send me v2 with dropping ad9523_sync() changes and
keeping the other two or just drop the whole patch?

Thanks,
Bhargav Joshi

> --
> With Best Regards,
> Andy Shevchenko
>
>

Re: [PATCH] iio: frequency: ad9523: check return value of ad9523_io_update()

[Andy Shevchenko]

On Wed, Mar 18, 2026 at 08:59:09PM +0530, Bhargav Joshi wrote:
> On Wed, Mar 18, 2026 at 1:40 PM Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> > On Wed, Mar 18, 2026 at 04:55:52AM +0530, Bhargav Joshi wrote:

...

> > So, now the errors that were non-fatal become fatal. Is it a problem from
> > functional behaviour point of view? How did you test this?
> 
> Regarding errors become fatal, is true for function ad9523_sync()
> because even if error occurred it clears the sync bit and calls second
> io_update anyways so returning early here would make it worse, Apologies
> for this oversight on my part.
> 
> However for the  ad9523_write_raw() and ad9523_reg_access()
> ad9523_io_update() is the last step and Returning the error in these
> cases seems strictly correct to prevent silently ignoring I/O failures
> and returning false success to user-space regarding the active
> configuration.
> 
> Regarding testing: I do not have access to the physical hardware, so the
> patch is compile-tested only.
> 
> Would you like to send me v2 with dropping ad9523_sync() changes and
> keeping the other two or just drop the whole patch?

Don't be in a hurry. Let's wait for others to comment. We have ADI people here,
they might chime in and may be even help with testing if they consider the changes
valuable.

-- 
With Best Regards,
Andy Shevchenko