Hi Bin,
On Tue, Sep 25, 2018 at 7:48 AM Bin Meng bmeng.cn@gmail.com wrote:
On Sat, Sep 22, 2018 at 3:55 AM Simon Glass sjg@chromium.org wrote:
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.
If regmap currently only supports little-endian, can we document this somewhere?
Yes, good idea. I'll send a v7 to address this.
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?
In this version of the FPGA driver, the regmap is not strictly needed, true. But we also have devices that have multiple FPGAs where the "slave" FPGA's register maps are accessed through a custom bus interface (called MCLink) on the first "master FPGA". With the regmap, I'll be able to use the same driver for both, all that's needed is another version of regmap for the MCLink bus. So the regmap is really there for forward-compatibility reasons.
Regards, Bin
Best regards, Mario