On 28.09.2018 09:16, Mario Six wrote:
Hi Daniel, On Thu, Sep 27, 2018 at 1:28 PM Daniel Schwierzeck daniel.schwierzeck@gmail.com wrote:
Hi Mario,
Am Do., 27. Sep. 2018 um 11:48 Uhr schrieb Mario Six mario.six@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 agust@denx.de Signed-off-by: Mario Six mario.six@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.
looks good, thank you.