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

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

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:

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

Because there are two issues (size in bsec driver is wrong, cdev size is calculated wrong),
I need to decide which to fix first or squash them. I chose to make the size intermittently
bigger (so reading valid offsets still work) instead of making it smaller and breaking
bisect (or squashing all and making it less easy to review).

Cheers,
Ahmad

> 
> Robin
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |