[U-Boot] [PATCH v6 08/14] regmap: Add raw read/write functions
Mario Six
mario.six at gdsys.cc
Fri Sep 21 07:02:15 UTC 2018
Hi Simon,
On Fri, Aug 17, 2018 at 2:52 PM Simon Glass <sjg at chromium.org> wrote:
>
> Hi Mario,
>
> On 13 August 2018 at 00:09, Mario Six <mario.six at 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 at denx.de>
> > Signed-off-by: Mario Six <mario.six at 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.
> > + break;
> > +#endif
> > + default:
> > + debug("%s: regmap size %zu unknown\n", __func__, val_len);
> > + return -EINVAL;
> > + }
> > + return 0;
> > +}
> > +
> > int regmap_read(struct regmap *map, uint offset, uint *valp)
> > {
> > - u32 *ptr = map_physmem(map->ranges[0].start + offset, 4, MAP_NOCACHE);
> > + return regmap_raw_read(map, offset, valp, REGMAP_SIZE_32);
> > +}
> >
> > - *valp = le32_to_cpu(readl(ptr));
> > +int regmap_raw_write(struct regmap *map, uint offset, const void *val,
> > + 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:
> > + out_8((u8 *)ptr, *((u8 *)val));
> > + break;
> > + case REGMAP_SIZE_16:
> > + out_le16((u16 *)ptr, *((u16 *)val));
> > + break;
> > + case REGMAP_SIZE_32:
> > + out_le32((u32 *)ptr, *((u32 *)val));
> > + break;
>
> No 64-bit?
>
See above; half-finished version of the patch.
> > + default:
> > + debug("%s: regmap size %zu unknown\n", __func__, val_len);
> > + return -EINVAL;
> > + }
> >
> > return 0;
> > }
> >
> > int regmap_write(struct regmap *map, uint offset, uint val)
> > {
> > - u32 *ptr = map_physmem(map->ranges[0].start + offset, 4, MAP_NOCACHE);
> > -
> > - writel(cpu_to_le32(val), ptr);
> > -
> > - return 0;
> > + return regmap_raw_write(map, offset, &val, REGMAP_SIZE_32);
> > }
> >
> > int regmap_update_bits(struct regmap *map, uint offset, uint mask, uint val)
> > diff --git a/include/regmap.h b/include/regmap.h
> > index 32f75e06f59..8a9759281a5 100644
> > --- a/include/regmap.h
> > +++ b/include/regmap.h
> > @@ -8,6 +8,21 @@
> > #define __REGMAP_H
> >
> > /**
> > + * enum regmap_size_t - Access sizes for fpgamap reads and writes
> > + *
> > + * @REGMAP_SIZE_8: 8-bit read/write access size
> > + * @REGMAP_SIZE_16: 16-bit read/write access size
> > + * @REGMAP_SIZE_32: 32-bit read/write access size
> > + * @REGMAP_SIZE_64: 64-bit read/write access size
> > + */
> > +enum regmap_size_t {
> > + REGMAP_SIZE_8 = 1,
> > + REGMAP_SIZE_16 = 2,
> > + REGMAP_SIZE_32 = 4,
> > + REGMAP_SIZE_64 = 8,
> > +};
> > +
> > +/**
> > * struct regmap_range - a register map range
> > *
> > * @start: Start address
> > @@ -41,6 +56,10 @@ struct regmap {
> > * @offset: Offset in the regmap to write to
> > * @val: Data to write to the regmap at the specified offset
> > *
> > + * Note that this function will only write values of 32 bit width to the
> > + * regmap; if the size of data to be read is different, the regmap_raw_write
> > + * function can be used.
> > + *
> > * Return: 0 if OK, -ve on error
> > */
> > int regmap_write(struct regmap *map, uint offset, uint val);
> > @@ -53,10 +72,49 @@ int regmap_write(struct regmap *map, uint offset, uint val);
> > * @valp: Pointer to the buffer to receive the data read from the regmap
> > * at the specified offset
> > *
> > + * Note that this function will only read values of 32 bit width from the
> > + * regmap; if the size of data to be read is different, the regmap_raw_read
> > + * function can be used.
> > + *
> > * Return: 0 if OK, -ve on error
> > */
> > int regmap_read(struct regmap *map, uint offset, uint *valp);
> >
> > +/**
> > + * regmap_raw_write() - Write a value of specified length to a regmap
> > + *
> > + * @map: Regmap to write to
> > + * @offset: Offset in the regmap to write to
> > + * @val: Value to write to the regmap at the specified offset
> > + * @val_len: Length of the data to be written to the regmap
> > + *
> > + * Note that this function will, as opposed to regmap_write, write data of
> > + * arbitrary length to the regmap, and not just 32-bit values, and is thus a
> > + * generalized version of regmap_write.
> > + *
> > + * Return: 0 if OK, -ve on error
> > + */
> > +int regmap_raw_write(struct regmap *map, uint offset, const void *val,
> > + size_t val_len);
> > +
> > +/**
> > + * regmap_raw_read() - Read a value of specified length from a regmap
> > + *
> > + * @map: Regmap to read from
> > + * @offset: Offset in the regmap to read from
> > + * @valp: Pointer to the buffer to receive the data read from the regmap
> > + * at the specified offset
> > + * @val_len: Length of the data to be read from the regmap
> > + *
> > + * Note that this function will, as opposed to regmap_read, read data of
> > + * arbitrary length from the regmap, and not just 32-bit values, and is thus a
> > + * generalized version of regmap_read.
> > + *
> > + * Return: 0 if OK, -ve on error
> > + */
> > +int regmap_raw_read(struct regmap *map, uint offset, void *valp,
> > + size_t val_len);
> > +
> > #define regmap_write32(map, ptr, member, val) \
> > regmap_write(map, (uint32_t *)(ptr)->member - (uint32_t *)(ptr), val)
> >
> > --
> > 2.11.0
> >
>
> Regards,
> Simon
>
Best regards,
Mario
More information about the U-Boot
mailing list