Hi Mario,
On 21 September 2018 at 01:02, Mario Six <mario....@gdsys.cc> wrote:
>
> Hi Simon,
>
> On Fri, Aug 17, 2018 at 2:52 PM Simon Glass <s...@chromium.org> wrote:
> >
> > Hi Mario,
> >
> > On 13 August 2018 at 00:09, Mario Six <mario....@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 <ag...@denx.de>
> > > Signed-off-by: Mario Six <mario....@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
