Re: [PATCH v2 07/14] mfd: zl3073x: Add components versions register defs

[Andrew Lunn]

61;8000;1cOn Fri, Apr 11, 2025 at 03:17:14PM +0200, Ivan Vecera wrote:
> On 11. 04. 25 1:19 odp., Ivan Vecera wrote:
> > The range for regmap 1: (registers 0x000-0x4FF)
> > regmap_range_cfg {
> >      .range_min = 0,
> >      .range_max = 10 * 128 - 1, /* 10 pages, 128 registers each */
> >      .selector_reg = 0x7f,      /* page selector at each page */
> >      .selector_shift = 0,       /* no shift in page selector */
> >      .selector_mask = GENMASK(3, 0),    /* 4 bits for page sel */
> >      .window_start = 0,         /* 128 regs from 0x00-0x7f */
> >      .window_len = 128,
> > };
> > 
> > The range for regmap 2: (registers 0x500-0x77F)
> > regmap_range_cfg {
> >      .range_min = 10 * 128,
> >      .range_max = 15 * 128 - 1, /* 5 pages, 128 registers each */
> >      .selector_reg = 0x7f,      /* page selector at each page */
> >      .selector_shift = 0,       /* no shift in page selector */
> >      .selector_mask = GENMASK(3, 0),    /* 4 bits for page sel */
> >      .window_start = 0,         /* 128 regs from 0x00-0x7f */
> >      .window_len = 128,
> > };
> > 
> > Is it now OK?
> 
> No this is not good... I cannot use 2 ranges.
> 
> This is not safe... if the caller use regmap 2 to read/write something below
> 0x500 (by mistake), no mapping is applied and value is directly used as
> register number that's wrong :-(.
> 
> Should I use rather single mapping range to cover all pages and ensure at
> driver level that regmap 2 is not used for regs < 0x500?

I don't know regmap too well, but cannot your mailbox regmap have a
reg_base of 10 * 128. Going blow that would then require a negative
reg, but they are unsigned int.

One of that things the core MFD driver is about is giving you safe
access to shared registers on some sort of bus. So it could well be
your MFD exports an higher level API for mailboxs, a mailbox read and
mailbox write, etc. The regmap below it is not exposed outside of the
MFD core. And the MFD core does all the locking.

	Andrew