mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [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
  2026-09-18 12:14 ` kernel test robot
  0 siblings, 2 replies; 4+ 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] 4+ 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
  2026-09-18 12:14 ` kernel test robot
  1 sibling, 1 reply; 4+ 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] 4+ 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; 4+ 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] 4+ 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-18 12:14 ` kernel test robot
  1 sibling, 0 replies; 4+ messages in thread
From: kernel test robot @ 2026-09-18 12:14 UTC (permalink / raw)
  To: Salah Triki, Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko
  Cc: llvm, oe-kbuild-all, linux-iio, linux-kernel, Salah Triki

Hi Salah,

kernel test robot noticed the following build errors:

[auto build test ERROR on jic23-iio/togreg]
[also build test ERROR on linus/master v7.3-rc3 next-20260916]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Salah-Triki/iio-temperature-tsys02d-Fix-state-desynchronization-on-resolution-write-error/20260917-101608
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link:    https://lore.kernel.org/r/20260917091609.626043-1-salah.triki%40gmail.com
patch subject: [PATCH v2] iio: temperature: tsys02d: Fix state desynchronization on resolution write error
config: riscv-randconfig-1000-20260918 (https://download.01.org/0day-ci/archive/20260918/202609182052.HpuVt6ba-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260918/202609182052.HpuVt6ba-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202609182052.HpuVt6ba-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/iio/temperature/tsys02d.c:89:2: error: cannot jump from switch statement to this case label
      89 |         default:
         |         ^
   drivers/iio/temperature/tsys02d.c:82:3: note: jump bypasses initialization of variable with __attribute__((cleanup))
      82 |                 guard(mutex)(&dev_data->lock);
         |                 ^
   include/linux/cleanup.h:423:2: note: expanded from macro 'guard'
     423 |         CLASS(_name, __UNIQUE_ID(guard))
         |         ^
   include/linux/cleanup.h:303:3: note: expanded from macro 'CLASS'
     303 |                 class_##_name##_constructor
         |                 ^
   <scratch space>:93:1: note: expanded from here
      93 | class_mutex_constructor
         | ^
   note: (skipping 3 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:16:23: note: expanded from macro '__PASTE'
      16 | #define __PASTE(a, b) ___PASTE(a, b)
         |                       ^
   include/linux/compiler_types.h:15:24: note: expanded from macro '___PASTE'
      15 | #define ___PASTE(a, b) a##b
         |                        ^
   <scratch space>:99:1: note: expanded from here
      99 | __UNIQUE_ID_unlock_472
         | ^
   drivers/iio/temperature/tsys02d.c:82:3: note: jump bypasses initialization of variable with __attribute__((cleanup))
   include/linux/cleanup.h:423:15: note: expanded from macro 'guard'
     423 |         CLASS(_name, __UNIQUE_ID(guard))
         |                      ^
   include/linux/compiler.h:165:2: note: expanded from macro '__UNIQUE_ID'
     165 |         __PASTE(__UNIQUE_ID_,                                   \
         |         ^
   include/linux/compiler_types.h:16:23: note: expanded from macro '__PASTE'
      16 | #define __PASTE(a, b) ___PASTE(a, b)
         |                       ^
   include/linux/compiler_types.h:15:24: note: expanded from macro '___PASTE'
      15 | #define ___PASTE(a, b) a##b
         |                        ^
   <scratch space>:87:1: note: expanded from here
      87 | __UNIQUE_ID_guard_471
         | ^
   1 error generated.


vim +89 drivers/iio/temperature/tsys02d.c

53bf4d067d5115 Ludovic Tancerel 2015-10-01  65  
53bf4d067d5115 Ludovic Tancerel 2015-10-01  66  static int tsys02d_write_raw(struct iio_dev *indio_dev,
53bf4d067d5115 Ludovic Tancerel 2015-10-01  67  			     struct iio_chan_spec const *chan,
53bf4d067d5115 Ludovic Tancerel 2015-10-01  68  			     int val, int val2, long mask)
53bf4d067d5115 Ludovic Tancerel 2015-10-01  69  {
53bf4d067d5115 Ludovic Tancerel 2015-10-01  70  	struct ms_ht_dev *dev_data = iio_priv(indio_dev);
53bf4d067d5115 Ludovic Tancerel 2015-10-01  71  	int i, ret;
53bf4d067d5115 Ludovic Tancerel 2015-10-01  72  
53bf4d067d5115 Ludovic Tancerel 2015-10-01  73  	switch (mask) {
53bf4d067d5115 Ludovic Tancerel 2015-10-01  74  	case IIO_CHAN_INFO_SAMP_FREQ:
53bf4d067d5115 Ludovic Tancerel 2015-10-01  75  		i = ARRAY_SIZE(tsys02d_samp_freq);
53bf4d067d5115 Ludovic Tancerel 2015-10-01  76  		while (i-- > 0)
53bf4d067d5115 Ludovic Tancerel 2015-10-01  77  			if (val == tsys02d_samp_freq[i])
53bf4d067d5115 Ludovic Tancerel 2015-10-01  78  				break;
53bf4d067d5115 Ludovic Tancerel 2015-10-01  79  		if (i < 0)
53bf4d067d5115 Ludovic Tancerel 2015-10-01  80  			return -EINVAL;
53bf4d067d5115 Ludovic Tancerel 2015-10-01  81  
3c8dd88f5e71a2 Salah Triki      2026-09-17  82  		guard(mutex)(&dev_data->lock);
3c8dd88f5e71a2 Salah Triki      2026-09-17  83  
3c8dd88f5e71a2 Salah Triki      2026-09-17  84  		ret = ms_sensors_write_resolution(dev_data, i);
3c8dd88f5e71a2 Salah Triki      2026-09-17  85  		if (ret)
53bf4d067d5115 Ludovic Tancerel 2015-10-01  86  			return ret;
3c8dd88f5e71a2 Salah Triki      2026-09-17  87  
3c8dd88f5e71a2 Salah Triki      2026-09-17  88  		dev_data->res_index = i;
53bf4d067d5115 Ludovic Tancerel 2015-10-01 @89  	default:
53bf4d067d5115 Ludovic Tancerel 2015-10-01  90  		return -EINVAL;
53bf4d067d5115 Ludovic Tancerel 2015-10-01  91  	}
53bf4d067d5115 Ludovic Tancerel 2015-10-01  92  }
53bf4d067d5115 Ludovic Tancerel 2015-10-01  93  

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-18 12:15 UTC | newest]

Thread overview: 4+ 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
2026-09-18 12:14 ` kernel test robot

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®