Re: [RFC] regmap: clairify max_register meaning

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2665 bytes --]

On Mon, Jan 18, 2016 at 09:19:52AM -0800, Stefan Agner wrote:
> On 2016-01-18 08:05, Mark Brown wrote:

> >> Maybe I see something completely wrong here, but to me it seems
> >> that the regcache-flat.c is the only code interpreting max_register
> >> as register indexes rather then the maximum relative address...

> > I don't think I understand what the above means, sorry.  What is a
> > "relative address" in the context of a register map?

> My interpretation:

> relative address = index * stride
> absolute address = base address + relative address

Oh, OK.  I would have got that for absolute address for MMIO but
definitely not relative address.  For relative addresses I'd interpret
index and address as synonyms.

> > Your changelog doesn't actually say this but I think what this is trying
> > to do is make the cache more memory efficient by not allocating space
> > for the registers that don't exist due to striding (ie, if we have a 4
> > byte stride then currently we'll allocate space for 4 times as many
> > registers as actually exist).  That's a reasonable goal but I don't
> > immediately have a good idea for doing it bearing in mind that we are
> > very performance sensitive here.

> Yeah I saw a bug in that, but I see that this is actually just a not
> very space efficient implementation detail.

Indeed.  It is true that having that 300% overhead in a common case is a
bit annoying so if you do have any ideas for fixing that without taking
a performance hit that'd be really good.

> >> - * @max_register: Optional, specifies the maximum valid register index.
> >> + * @max_register: Optional, specifies the maximum valid register address.

> > This is reasonable (index and address are synonyms here).
> 
> Ok, in that case I clearly interpreted index wrong.

> Btw, range_min and range_max in struct regmap_range_cfg are defined as
> memory addresses:
> ...
>  * @range_min: Address of the lowest register address in virtual range. 
>         
>  * @range_max: Address of the highest register in virtual range.
> ...

Not memory addresses, register addresses.  Remember that most regmap
register maps are not memory mapped at all but rather are completely
separate ranges on some external device (especially things using ranges,
you're normally not short enough on addresses on a memory mapped device
to bother with a window) so when looking at memory mapped devices it's
best to think only of the area that's actually mapped into the regmap.

> Will send out a patch for the documentation change only, if you think
> that this is a reasonable change too?

Yes, please do.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]