Re: [PATCH v2] nvmem: regmap: Fix nvmem size

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

Hello Robin,

On 21.12.23 15:02, Ahmad Fatoum wrote:
> On 20.12.23 13:38, Robin van der Gracht wrote:
>> Hi Ahmad,
>>
>> On 2023-12-20 10:00, Ahmad Fatoum wrote:
>>> Hello Robin,
>>>
>>> Thanks for the fix.
>>>
>>> On 20.12.23 09:29, Robin van der Gracht wrote:
>>>> -    if (roffset + rbytes > stride * regmap_get_max_register(map))
>>>> +    if (roffset + rbytes > regmap_size_bytes(map) * stride)
>>>
>>> Shouldn't stride on the right hand side be dropped?
>>
>> roffset = register index * stride.
>>
>> I.e. 380 for register with index 95.
>>
>> For stm32mp1x bsec:
>>  map->format.val_bytes = 4
>>  map->reg_stride = 4
>>
>> regmap_size_bytes() = map->format.val_bytes * (95 + 1) / map->reg_stride = 96
> 
> Ouch. What have I been thinking when I named regmap_size_bytes() this way...
> Change looks ok then, but we should really rename regmap_size_bytes()...

After talking it over with Sascha, I see that the name is correct with regard
to how the function was initially used. What's wrong it the implementation and
that goes back to 2016..

I just sent out a series fixing this. md -s /dev/stm32-bsec 380 now works.
Please verify if it's behaving as you expect.

Thanks,
Ahmad

> 
>>
>> So the result with the stride on the right size is correct.
>>
>> I moved stride from left to right to be consistent with the size calculation
>> in nvmem_regmap_register_with_pp()
>>
>> Kind regards,
>> Robin
>>
>>
>>>
>>> Cheers,
>>> Ahmad
>>>
>>>>          return -EINVAL;
>>>>
>>>>      for (i = roffset; i < roffset + rbytes; i += stride) {
>>>> @@ -78,7 +78,7 @@ nvmem_regmap_register_with_pp(struct regmap *map, const char *name,
>>>>      config.priv = map;
>>>>      config.stride = 1;
>>>>      config.word_size = 1;
>>>> -    config.size = regmap_get_max_register(map) * regmap_get_reg_stride(map);
>>>> +    config.size = regmap_size_bytes(map) * regmap_get_reg_stride(map);
>>>>      config.cell_post_process = cell_post_process;
>>>>      config.reg_write = nvmem_regmap_write;
>>>>      config.reg_read = nvmem_regmap_read;
>>
> 

-- 
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 |