[U-Boot] [PATCH v8 08/15] regmap: Add raw read/write functions

[Daniel Schwierzeck]

Hi Mario,

Am Do., 27. Sep. 2018 um 11:48 Uhr schrieb Mario Six <mario.six at 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 at denx.de>
> Signed-off-by: Mario Six <mario.six at 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.

-- 
- Daniel