Re: [PATCH master 2/7] nvmem: bsec: correct regmap's max_register

[Robin van der Gracht <robin@protonic.nl>]

Hello Ahmad,

On Mon, 8 Jan 2024 11:44:00 +0100
Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:

> Hello Robin,
> 
> On 08.01.24 11:29, Robin van der Gracht wrote:
> > Hi Ahmad,
> > 
> > Comments are below.
> > 
> > On Tue,  2 Jan 2024 18:00:55 +0100
> > Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> >   
> >> The max_register must be a multiple of the register stride, which is not
> >> the case for (384 / 4) - 1 == 95. Instead, we should be setting 380, so
> >> fix the calculation to do this.
> >>
> >> Fixes: 094ce0ee7cdf ("nvmem: bsec: correct regmap's max_register")
> >> Reported-by: Robin van der Gracht <robin@protonic.nl>
> >> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> >> ---
> >>  drivers/nvmem/bsec.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/nvmem/bsec.c b/drivers/nvmem/bsec.c
> >> index 889f14428d59..22e30c6c2e82 100644
> >> --- a/drivers/nvmem/bsec.c
> >> +++ b/drivers/nvmem/bsec.c
> >> @@ -218,7 +218,7 @@ static int stm32_bsec_probe(struct device *dev)
> >>  	priv->map_config.reg_bits = 32;
> >>  	priv->map_config.val_bits = 32;
> >>  	priv->map_config.reg_stride = 4;
> >> -	priv->map_config.max_register = (data->size / 4) - 1;
> >> +	priv->map_config.max_register = data->size - priv->map_config.reg_stride;
> >>  
> >>  	priv->lower = data->lower;
> >>    
> > 
> > This patch gives a bsec device size of 1520 bytes. Which means I'm now
> > allowed to read/write beyond register 95 without an error.
> > 
> > barebox@board:/ ls -l /dev/stm32-bsec
> > crw-------           1520 /dev/stm32-bsec
> > 
> > The device size is now in bytes, but the read/write offsets given to
> > the md and mw commands is still in bytes/stride.
> > 
> > I.e. to read register 95:
> > md -l -s /dev/stm32-bsec 380+4 
> > 0000017c: xxxxxxxx
> > 
> > I can now also read register 100:
> > md -l -s /dev/stm32-bsec 400+4 
> > 00000190: 00000000                                           ....
> > 
> > This doesn't seem right.
> > 
> > max_register should probably stay 95. See doc[1]
> > 
> > 1:https://git.pengutronix.de/cgit/barebox/tree/include/linux/regmap.h?h=v2023.12.0#n33  
> 
> Did you apply the whole series? With the whole series applied I have:

Argh. No. I missed it was part of a series. I was only cc'd to this
one. 

> 
> barebox@Linux Automation MC-1 board:/ ls -l /dev/stm32-bsec 
> crw-------            384 /dev/stm32-bsec

Thats more like it. I'll apply the full series and recheck.

Robin