* [PATCH v2] iio: temperature: tsys02d: Fix state desynchronization on resolution write error
@ 2026-09-17 9:16 Salah Triki
2026-09-17 11:16 ` Joshua Crofts
0 siblings, 1 reply; 3+ messages in thread
From: Salah Triki @ 2026-09-17 9:16 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko
Cc: linux-iio, linux-kernel, Salah Triki
In tsys02d_write_raw(), dev_data->res_index is updated before checking
the return value of ms_sensors_write_resolution(). If the hardware I2C
write operation fails, dev_data->res_index remains updated with the
new index despite the physical sensor remaining in its previous state.
This leads to a state desynchronization where subsequent reads via
tsys02d_read_raw() return an incorrect sampling frequency.
Fix this by handling the error path first, updating dev_data->res_index
only on success, and using guard(mutex) to clean up locking.
This was found through manual code review.
Fixes: 53bf4d067d51 ("Add tsys02d meas-spec driver support")
Signed-off-by: Salah Triki <salah.triki@gmail.com>
---
v1 -> v2:
- Use error-first pattern (if (ret) return ret) instead of if (!ret).
- Use guard(mutex) for clean locking and simplified return path.
- Add note stating the issue was found through manual code review.
- Add proper blank lines.
drivers/iio/temperature/tsys02d.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/temperature/tsys02d.c b/drivers/iio/temperature/tsys02d.c
index 3ef72347456e..20f67134affd 100644
--- a/drivers/iio/temperature/tsys02d.c
+++ b/drivers/iio/temperature/tsys02d.c
@@ -72,12 +72,14 @@ static int tsys02d_write_raw(struct iio_dev *indio_dev,
break;
if (i < 0)
return -EINVAL;
- mutex_lock(&dev_data->lock);
- dev_data->res_index = i;
+
+ guard(mutex)(&dev_data->lock);
+
ret = ms_sensors_write_resolution(dev_data, i);
- mutex_unlock(&dev_data->lock);
+ if (ret)
+ return ret;
- return ret;
+ dev_data->res_index = i;
default:
return -EINVAL;
}
--
2.43.0
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] iio: temperature: tsys02d: Fix state desynchronization on resolution write error
2026-09-17 9:16 [PATCH v2] iio: temperature: tsys02d: Fix state desynchronization on resolution write error Salah Triki
@ 2026-09-17 11:16 ` Joshua Crofts
2026-09-17 12:23 ` Andy Shevchenko
0 siblings, 1 reply; 3+ messages in thread
From: Joshua Crofts @ 2026-09-17 11:16 UTC (permalink / raw)
To: Salah Triki
Cc: Jonathan Cameron, David Lechner, Nuno Sá,
Andy Shevchenko, linux-iio, linux-kernel
On Thu, 17 Sep 2026 10:16:08 +0100
Salah Triki <salah.triki@gmail.com> wrote:
> In tsys02d_write_raw(), dev_data->res_index is updated before checking
> the return value of ms_sensors_write_resolution(). If the hardware I2C
> write operation fails, dev_data->res_index remains updated with the
> new index despite the physical sensor remaining in its previous state.
>
> This leads to a state desynchronization where subsequent reads via
> tsys02d_read_raw() return an incorrect sampling frequency.
>
> Fix this by handling the error path first, updating dev_data->res_index
> only on success, and using guard(mutex) to clean up locking.
>
> This was found through manual code review.
>
> Fixes: 53bf4d067d51 ("Add tsys02d meas-spec driver support")
> Signed-off-by: Salah Triki <salah.triki@gmail.com>
> ---
> v1 -> v2:
> - Use error-first pattern (if (ret) return ret) instead of if (!ret).
> - Use guard(mutex) for clean locking and simplified return path.
> - Add note stating the issue was found through manual code review.
> - Add proper blank lines.
>
> drivers/iio/temperature/tsys02d.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iio/temperature/tsys02d.c b/drivers/iio/temperature/tsys02d.c
> index 3ef72347456e..20f67134affd 100644
> --- a/drivers/iio/temperature/tsys02d.c
> +++ b/drivers/iio/temperature/tsys02d.c
> @@ -72,12 +72,14 @@ static int tsys02d_write_raw(struct iio_dev *indio_dev,
> break;
> if (i < 0)
> return -EINVAL;
> - mutex_lock(&dev_data->lock);
> - dev_data->res_index = i;
> +
> + guard(mutex)(&dev_data->lock);
Please wrap the case in curly brackets to define the scope for guard().
Additionally, add a <linux/cleanup.h> include. I recently removed kernel.h
from this driver and cleaned the headers, so I'd recommend basing this on
the iio/togreg branch to get the changes if you haven't yet.
https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/commit/?id=6b5856d16c2e9213fd7eed8a4121aa3ad41081b4
> ret = ms_sensors_write_resolution(dev_data, i);
> - mutex_unlock(&dev_data->lock);
> + if (ret)
> + return ret;
>
> - return ret;
> + dev_data->res_index = i;
You're missing a return, this will just fall through to the default
case below.
> default:
> return -EINVAL;
> }
--
Kind regards,
Joshua Crofts
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] iio: temperature: tsys02d: Fix state desynchronization on resolution write error
2026-09-17 11:16 ` Joshua Crofts
@ 2026-09-17 12:23 ` Andy Shevchenko
0 siblings, 0 replies; 3+ messages in thread
From: Andy Shevchenko @ 2026-09-17 12:23 UTC (permalink / raw)
To: Joshua Crofts
Cc: Salah Triki, Jonathan Cameron, David Lechner, Nuno Sá,
Andy Shevchenko, linux-iio, linux-kernel
On Thu, Sep 17, 2026 at 01:16:38PM +0200, Joshua Crofts wrote:
> On Thu, 17 Sep 2026 10:16:08 +0100
> Salah Triki <salah.triki@gmail.com> wrote:
...
> > - return ret;
> > + dev_data->res_index = i;
>
> You're missing a return, this will just fall through to the default
> case below.
>
> > default:
> > return -EINVAL;
...and having a such without explicit annotation will lead to a compiler
warning.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-09-17 12:24 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-17 9:16 [PATCH v2] iio: temperature: tsys02d: Fix state desynchronization on resolution write error Salah Triki
2026-09-17 11:16 ` Joshua Crofts
2026-09-17 12:23 ` Andy Shevchenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®