Re: Question about regmap_range_cfg and regmap_mmio

[Mark Brown <broonie@kernel.org>]

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

On Wed, Mar 04, 2020 at 02:28:25PM +0100, Lars Möllendorf wrote:
> On 04.03.20 13:03, Mark Brown wrote:
> > On Wed, Mar 04, 2020 at 12:25:09PM +0100, Lars Möllendorf wrote:

> In `__regmap_init()` `_regmap_bus_reg_read()`is assigned to
> `regmap.reg_read()` if there are no `bus->read/write` functions, else
> `_regmap_bus_read()` is assigned.
> 
> `_regmap_bus_reg_read()` calls the `reg_read` function of the bus
> directly, `_regmap_bus_read()` instead calls `_regmap_raw_read()`.
> 
> `_regmap_raw_read()` in turn calls `_regmap_range_lookup()` and
> `_regmap_select_page()` which do the paging.
> 
> `regmap_mmio` does not contain `bus->read/write`, but does contain
> `bus->reg_read/reg_write` only.

This is basically just saying that the paging is done in the bulk I/O
code only (partly for performance, partly since we may need to page in
the middle of a bulk operation), not in the per-register I/O paths.

> >>   - Enhance the current `regmap-mmio` implementation so it does paging
> >> and submit a patch?

> > That's not really possible since MMIO never writes the register address
> > to the bus

> Sorry, but I do not get why this shouldn't work with MMIO? If I
> understood the code correctly in  `_regmap_raw_read` the address is
> checked before it is used anywhere. If
> `_regmap_range_lookup()` returns a range it does the paging, i.e.
> translates the virtual address into the real address
> (`_regmap_select_page`). If so the real address is passed to
> `bus->read/write`, else the given address is used directly. Do I miss
> something here?

The plain read and write operations sit at the bottom of a stack that
(especially in the write path) deals with byte streams rather than
parsed data, they're only differentiated for read since there's a mix of
read and write I/O in a read operation.  Since for MMIO the register is
never part of a byte stream you'd need to contort things to parse the
address back out of the byte stream which is not great for abstraction
and likely to lead to bugs as different parts of the stack get confused
about what is handling endinaness and register size issues.

> >>   - Write my own `better-regmap-mmio` implementation?

> > It's not clear what that would mean.

> Maybe for some reason the current MMIO implementation should not be
> touched, or paging for MMIO is not wanted?

That doesn't really answer the question - what such an implementation
would look like?

> > You could also look into making the paging code not rely on explicit
> > register read and write operations.

> Maybe it is sufficient to implement `bus->read/write` as a wrapper of
> `bus->reg_read/reg_write` in regmap-mmio.c?

For the reasons outlined above that's an abstraction problem.  This is
not something that should be being done in the physical I/O code, that's
not the right layer of abstraction.  Like I say push it up into the
core.

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