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

From: Simon Glass <sjg@chromium.org>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v6 08/14] regmap: Add raw read/write functions
Date: Fri, 21 Sep 2018 13:55:19 -0600	[thread overview]
Message-ID: <CAPnjgZ0Om3zWxW0scmvJRGCigpPwCRw6=zP0oJTEBMXQhRWgMw@mail.gmail.com> (raw)
In-Reply-To: <CAN1kZopp7Kn87-b8LytP5oOfwh1MSvTzei9WNnf8D-0ZWJud4w@mail.gmail.com>

Hi Mario,

On 21 September 2018 at 01:02, Mario Six <mario.six@gdsys.cc> wrote:
>
> Hi Simon,
>
> On Fri, Aug 17, 2018 at 2:52 PM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Mario,
> >
> > On 13 August 2018 at 00:09, Mario Six <mario.six@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@denx.de>
> > > Signed-off-by: Mario Six <mario.six@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.

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.

Adding Bin who may understand this better.

Regards,
Simon