Hi Daniel,
On Thu, Sep 27, 2018 at 1:28 PM Daniel Schwierzeck
<daniel.schwierz...@gmail.com> wrote:
>
> Hi Mario,
>
> Am Do., 27. Sep. 2018 um 11:48 Uhr schrieb Mario Six <mario....@gdsys.cc>:
> >
> > 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>
> >
> > ---
> >
> > v7 -> v8:
> > No changes
> >
> > v6 -> v7:
> > * Fixed wrong variable type in 64-bit read (u32 -> u64)
> > * Added 64-bit case in write function
> >
> > 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 | 64
> > +++++++++++++++++++++++++++++++++++++++++++++------
> > include/regmap.h | 58 ++++++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 115 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/core/regmap.c b/drivers/core/regmap.c
> > index 154426269d9..1025099797f 100644
> > --- a/drivers/core/regmap.c
> > +++ b/drivers/core/regmap.c
> > @@ -188,22 +188,72 @@ 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:
> > + *((u64 *)valp) = in_le64((u64 *)ptr);
> > + break;
> > +#endif
> > + default:
> > + debug("%s: regmap size %zu unknown\n", __func__, val_len);
> > + return -EINVAL;
> > + }
> > + return 0;
> > +}
>
> I'm still not convinced why you want to enforce Little Endian here.
> This makes regmap unuseable on archs like MIPS or PowerPC.
>
> Typically a SoC with a MIPS CPU core can have an AMBA bus or other LE
> based sub-systems. Some SoCs can select the endianess of the CPU core
> via pin strapping. Normally the endianess conversion is automatically
> done in HW.
>
> So I suggest to simply use readl or __raw_readl to support native
> endianess as default. Later we could add a DT properties
> "native-endian", "big-endian" and "little-endian" like in the Linux
> implementation.
>
Ironically, we want to use the interface on a PowerPC platform :-) (for a
little-endian FPGA interface that's not auto-translating). But I do see your
point.
I just finished testing a v9 of this series that implements support for the
"native-endian", "big-endian", and "little-endian" DT properties (and uses
native-endian as the default). That way we can use the interface on our
platform, and native-endian is the default, so MIPS should be able to use
regmap too.
I think that should solve the problem adequately for everyone.
> --
> - Daniel
>
Best regards,
Mario
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
