Re: [PATCH] iio: bmi323: Fix array reference in bmi323_core_runtime_suspend()

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 9 Sep 2024 21:10:35 +0200
Denis Benato <benato.denis96@gmail.com> wrote:

> On 09/09/24 18:38, Nathan Chancellor wrote:
> > Clang warns (or errors with CONFIG_WERROR):
> > 
> >   drivers/iio/imu/bmi323/bmi323_core.c:133:27: error: variable 'bmi323_ext_reg_savestate' is not needed and will not be emitted [-Werror,-Wunneeded-internal-declaration]
> >     133 | static const unsigned int bmi323_ext_reg_savestate[] = {
> >         |                           ^~~~~~~~~~~~~~~~~~~~~~~~
> > 
> > bmi323_ext_reg_savestate is only used within sizeof() through
> > ARRAY_SIZE(), so it is not unused, but it will not be emitted in the
> > final binary because sizeof() is evaluated only at compile time.
> > bmi323_ext_reg_savestate should have been used in the second parameter
> > in the call to bmi323_read_ext_reg() in the second for loop in
> > bmi323_core_runtime_suspend().
> > 
> > Fixes: 16531118ba63 ("iio: bmi323: peripheral in lowest power state on suspend")
> > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> > ---
> >  drivers/iio/imu/bmi323/bmi323_core.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iio/imu/bmi323/bmi323_core.c b/drivers/iio/imu/bmi323/bmi323_core.c
> > index 671401ce80dcf947b7b64ea3af112d2a42ca5501..64dbce23ce17bcdd11c0d4c454dbeb9de17ef56c 100644
> > --- a/drivers/iio/imu/bmi323/bmi323_core.c
> > +++ b/drivers/iio/imu/bmi323/bmi323_core.c
> > @@ -2199,7 +2199,7 @@ static int bmi323_core_runtime_suspend(struct device *dev)
> >  	}
> >  
> >  	for (unsigned int i = 0; i < ARRAY_SIZE(bmi323_ext_reg_savestate); i++) {
> > -		ret = bmi323_read_ext_reg(data, bmi323_reg_savestate[i],
> > +		ret = bmi323_read_ext_reg(data, bmi323_ext_reg_savestate[i],
> >  					  &savestate->reg_settings[i]);
This is saving it in the wrong place.
Noticed as Dan sent out another patch for this that was different.

So I'll pick up his instead.  Note that similar bug exists in resume.

Jonathan

> >  		if (ret) {
> >  			dev_err(data->dev,
> > 
> > ---
> > base-commit: 5ba0cb92584ba5e107c97001e09013c1da0772a8
> > change-id: 20240909-iio-bmi323-fix-array-ref-a0672a8213f0
> > 
> > Best regards,  
> Hello Nathan,
> 
> Thank you kindly for spotting and fixing it.
> 
> Regrettably while integrating suggestions I received I also changed the patch semantic and due to my hardware not having the irq pin connected to the cpu this went unnoticed.
> 
> Best regards,
> Denis Benato