Re: Page select register restrictions in regmap core

[Guenter Roeck]

On 6/17/24 15:47, Mark Brown wrote:
> On Mon, Jun 17, 2024 at 02:55:09PM -0700, Guenter Roeck wrote:
> > On 6/17/24 10:22, Mark Brown wrote:
>
> > > The range is *entirely* defined within the driver, it is 100% a software
> > > construct, the hardware only influences our choice of range in that we
> > > can't place it on top of hardware registers.
>
> > I _think_ what you are saying is that I'd have to model all registers
> > which are to be addressed through regmap as virtual registers with an offset
> > outside the range of real registers. Something like adding 0x100 to the
>
> No, only registers that are accessed through a window need to be
> mapped into a range.  Any other registers can just be accessed.
See below.

> > each register address and then accessing, say, the revision register
> > not as register 0x02 but as register 0x102. I would then define the matching
> > range from 0x100 .. 0x17f and the window from 0x00..0x7f.
>
> That would make the range exactly the same size as the window so there'd
> be no paging going on and the registers could be accessed directly?  I
> guess that's another check that should be added...

I tried to explain this before. The registers in address range 00..0x7f
are physical, but they are only accessible from page 0 with the exception
of the page select register. So, sure, the registers are not actually paged,
but page 0 must be selected to access them. That is the one and only reason
for specifying that first range and window. It ensures that page 0 is
selected when accessing the registers. If that wasn't the case, I could
define a single range for the actually paged addresses in the 0x80..0xff
window and be done with it.

> > Hmm, yes, I see that this should work. I don't think it is worth doing though
> > since I need to be able to access some registers outside regmap, and I'd have
> > to define two sets of addresses for all those registers. That would simplify
> > the code a bit but one would have to remember that register addresses through
> > regmap are different than register addresses when calling smbus functions
> > directly. I think we'll just stick with the current code and keep the paging
> > implementation in the driver.
>
> Mixing regmap and non-regmap access to the same registers seems like a
> bad idea in general, you will have locking issues (especially around the
> paging).

The non-regmap access all happens in the probe function before regmap is
initialized. It is needed for basic chip identification, to prevent someone
from instantiating the driver on a random nvram/eeprom and messing it up
with attempts to write the page select register. I would not want to be
held responsible for someone with, say, DDR4 DIMMs force-instantiating
the spd5118 driver and then complaining about bricked DIMMs.

Thanks,
Guenter