[U-Boot] [PATCH v6 08/14] regmap: Add raw read/write functions

[Bin Meng]

On Sat, Sep 22, 2018 at 3:55 AM Simon Glass <sjg at chromium.org> wrote:
>
> Hi Mario,
>
> On 21 September 2018 at 01:02, Mario Six <mario.six at gdsys.cc> wrote:
> >
> > Hi Simon,
> >
> > On Fri, Aug 17, 2018 at 2:52 PM Simon Glass <sjg at chromium.org> wrote:
> > >
> > > Hi Mario,
> > >
> > > On 13 August 2018 at 00:09, Mario Six <mario.six at gdsys.cc> wrote:
> > > > The regmap functions currently assume that all register map accesses
> > > > have a data width of 32 bits, but there are maps that have different
> > > > widths.
> > > >
> > > > To rectify this, implement the regmap_raw_read and regmap_raw_write
> > > > functions from the Linux kernel API that specify the width of a desired
> > > > read or write operation on a regmap.
> > > >
> > > > Implement the regmap_read and regmap_write functions using these raw
> > > > functions in a backwards-compatible manner.
> > > >
> > > > Reviewed-by: Anatolij Gustschin <agust at denx.de>
> > > > Signed-off-by: Mario Six <mario.six at gdsys.cc>
> > > >
> > > > ---
> > > >
> > > > v5 -> v6:
> > > > * Corrected format specifier
> > > > * Added support for 64-bit reads/writes
> > > >
> > > > v4 -> v5:
> > > > No changes
> > > >
> > > > v3 -> v4:
> > > > * Switched 'ranges[0] + offset' to 'ranges[0].start + offset'
> > > > * Explained the difference between the raw and non-raw read/write
> > > >   functions better in the docs
> > > >
> > > > v2 -> v3:
> > > > * Implement the "raw" functions from Linux instead of adding a size
> > > >   parameter to the regmap_{read,write} functions
> > > > * Fixed style violation
> > > > * Improved error handling
> > > >
> > > > v1 -> v2:
> > > > New in v2
> > > >
> > > > ---
> > > >  drivers/core/regmap.c | 59 +++++++++++++++++++++++++++++++++++++++++++++------
> > > >  include/regmap.h      | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > > >  2 files changed, 110 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/drivers/core/regmap.c b/drivers/core/regmap.c
> > > > index 154426269d9..a2f82091af0 100644
> > > > --- a/drivers/core/regmap.c
> > > > +++ b/drivers/core/regmap.c
> > > > @@ -188,22 +188,67 @@ int regmap_uninit(struct regmap *map)
> > > >         return 0;
> > > >  }
> > > >
> > > > +int regmap_raw_read(struct regmap *map, uint offset, void *valp, size_t val_len)
> > > > +{
> > > > +       void *ptr;
> > > > +
> > > > +       ptr = map_physmem(map->ranges[0].start + offset, val_len, MAP_NOCACHE);
> > > > +
> > > > +       switch (val_len) {
> > > > +       case REGMAP_SIZE_8:
> > > > +               *((u8 *)valp) = in_8((u8 *)ptr);
> > > > +               break;
> > > > +       case REGMAP_SIZE_16:
> > > > +               *((u16 *)valp) = in_le16((u16 *)ptr);
> > > > +               break;
> > > > +       case REGMAP_SIZE_32:
> > > > +               *((u32 *)valp) = in_le32((u32 *)ptr);
> > > > +               break;
> > > > +#if defined(in_le64) && defined(readq)
> > > > +       case REGMAP_SIZE_64:
> > > > +               *((u32 *)valp) = in_le64((u64 *)ptr);
> > >
> > > How come this is u32? Can you please add a comment?
> > >
> >
> > That was a development version of the patch, sorry (I was in a bit of a hurry).
> >
> > I'll send a corrected version with v7.
> >
> > > Why is this using in/out rather than read/write? Does it not support
> > > memory-mapped I/O?
> > >
> >
> > It does, but I think the endianness of the read/write operations of the regmap
> > should not depend on the architecture, but only on the regmap itself (which is
> > little-endian for now; big-endian support can be introduced later on), so I
> > used in/out rather than read/write.
>

If regmap currently only supports little-endian, can we document this somewhere?

> What does endianness have to do with whether you use readl/writel or in/out?
>
> On x86 at least these are actually different things, so regmap() won't
> work on x86 with this change.
>

Looks so far x86's {in,out}_{8,16,32} are using MMIO.

> Adding Bin who may understand this better.

BTW: I was trying to understand why we need regmap here. Shouldn't the
driver directly call {in,out}{bwl}, or {read,write}{bwlq} APIs?

Regards,
Bin