Hello Ahmad,
On Mon, 8 Jan 2024 11:44:00 +0100
Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx> 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@xxxxxxxxxxx>
> >> Signed-off-by: Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx>
> >> ---
> >> 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
