From: Joshua Crofts <joshua.crofts1@gmail.com>
To: Salah Triki <salah.triki@gmail.com>
Cc: "Jonathan Cameron" <jic23@kernel.org>,
"David Lechner" <dlechner@baylibre.com>,
"Nuno Sá" <nuno.sa@analog.com>,
"Andy Shevchenko" <andy@kernel.org>,
linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] iio: temperature: tsys02d: Fix state desynchronization on resolution write error
Date: Thu, 17 Sep 2026 13:16:38 +0200 [thread overview]
Message-ID: <20260917131638.000034e5@gmail.com> (raw)
In-Reply-To: <20260917091609.626043-1-salah.triki@gmail.com>
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
next prev parent reply other threads:[~2026-09-17 11:16 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-17 9:16 Salah Triki
2026-09-17 11:16 ` Joshua Crofts [this message]
2026-09-17 12:23 ` Andy Shevchenko
2026-09-18 12:14 ` kernel test robot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260917131638.000034e5@gmail.com \
--to=joshua.crofts1@gmail.com \
--cc=andy@kernel.org \
--cc=dlechner@baylibre.com \
--cc=jic23@kernel.org \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=nuno.sa@analog.com \
--cc=salah.triki@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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®