| Message ID | 20200527125208.24881-4-p.yadav@ti.com |
|---|---|
| State | Superseded |
| Delegated to: | Simon Glass |
| Headers | show |
| Series | regmap: Add managed API, regmap fields, regmap config | expand |
Hi Pratyush, On Wed, 27 May 2020 at 06:52, Pratyush Yadav <p.yadav@ti.com> wrote: > > Right now, regmap_read() and regmap_write() read/write a 32-bit value > only. To write other lengths, regmap_raw_read() and regmap_raw_write() > need to be used. > > This means that any driver ported from Linux that relies on > regmap_{read,write}() to know the size already has to be updated at each > callsite. This makes the port harder to maintain. > > So, allow specifying the read/write width to make it easier to port the > drivers, since now the only change needed is when initializing the > regmap. > > Signed-off-by: Pratyush Yadav <p.yadav@ti.com> > --- > drivers/core/regmap.c | 12 +++++++++--- > include/regmap.h | 28 ++++++++++++++++++---------- > 2 files changed, 27 insertions(+), 13 deletions(-) Reviewed-by: Simon Glass <sjg@chromium.org> See below > > diff --git a/drivers/core/regmap.c b/drivers/core/regmap.c > index 24651fb3ec..0180246095 100644 > --- a/drivers/core/regmap.c > +++ b/drivers/core/regmap.c > @@ -245,7 +245,7 @@ struct regmap *devm_regmap_init(struct udevice *dev, > const struct regmap_config *config) > { > int rc; > - struct regmap **mapp; > + struct regmap **mapp, *map; > > mapp = devres_alloc(devm_regmap_release, sizeof(struct regmap *), > __GFP_ZERO); > @@ -256,6 +256,10 @@ struct regmap *devm_regmap_init(struct udevice *dev, > if (rc) > return ERR_PTR(rc); > > + map = *mapp; > + if (config) > + map->width = config->width; > + > devres_add(dev, mapp); > return *mapp; > } > @@ -378,7 +382,8 @@ int regmap_raw_read(struct regmap *map, uint offset, void *valp, size_t val_len) > > int regmap_read(struct regmap *map, uint offset, uint *valp) > { > - return regmap_raw_read(map, offset, valp, REGMAP_SIZE_32); > + return regmap_raw_read(map, offset, valp, > + map->width ? map->width : REGMAP_SIZE_32); Can you set up map->width so you can avoid the checks? Regards, Simon
On 31/05/20 08:08AM, Simon Glass wrote: > Hi Pratyush, > > On Wed, 27 May 2020 at 06:52, Pratyush Yadav <p.yadav@ti.com> wrote: > > > > Right now, regmap_read() and regmap_write() read/write a 32-bit value > > only. To write other lengths, regmap_raw_read() and regmap_raw_write() > > need to be used. > > > > This means that any driver ported from Linux that relies on > > regmap_{read,write}() to know the size already has to be updated at each > > callsite. This makes the port harder to maintain. > > > > So, allow specifying the read/write width to make it easier to port the > > drivers, since now the only change needed is when initializing the > > regmap. > > > > Signed-off-by: Pratyush Yadav <p.yadav@ti.com> > > --- > > drivers/core/regmap.c | 12 +++++++++--- > > include/regmap.h | 28 ++++++++++++++++++---------- > > 2 files changed, 27 insertions(+), 13 deletions(-) > > Reviewed-by: Simon Glass <sjg@chromium.org> > > See below > > > > > diff --git a/drivers/core/regmap.c b/drivers/core/regmap.c > > index 24651fb3ec..0180246095 100644 > > --- a/drivers/core/regmap.c > > +++ b/drivers/core/regmap.c > > @@ -245,7 +245,7 @@ struct regmap *devm_regmap_init(struct udevice *dev, > > const struct regmap_config *config) > > { > > int rc; > > - struct regmap **mapp; > > + struct regmap **mapp, *map; > > > > mapp = devres_alloc(devm_regmap_release, sizeof(struct regmap *), > > __GFP_ZERO); > > @@ -256,6 +256,10 @@ struct regmap *devm_regmap_init(struct udevice *dev, > > if (rc) > > return ERR_PTR(rc); > > > > + map = *mapp; > > + if (config) > > + map->width = config->width; > > + > > devres_add(dev, mapp); > > return *mapp; > > } > > @@ -378,7 +382,8 @@ int regmap_raw_read(struct regmap *map, uint offset, void *valp, size_t val_len) > > > > int regmap_read(struct regmap *map, uint offset, uint *valp) > > { > > - return regmap_raw_read(map, offset, valp, REGMAP_SIZE_32); > > + return regmap_raw_read(map, offset, valp, > > + map->width ? map->width : REGMAP_SIZE_32); > > Can you set up map->width so you can avoid the checks? Ok. I'll set the default to 4 in regmap_alloc() and let callers over-ride it if they need.
diff --git a/drivers/core/regmap.c b/drivers/core/regmap.c index 24651fb3ec..0180246095 100644 --- a/drivers/core/regmap.c +++ b/drivers/core/regmap.c @@ -245,7 +245,7 @@ struct regmap *devm_regmap_init(struct udevice *dev, const struct regmap_config *config) { int rc; - struct regmap **mapp; + struct regmap **mapp, *map; mapp = devres_alloc(devm_regmap_release, sizeof(struct regmap *), __GFP_ZERO); @@ -256,6 +256,10 @@ struct regmap *devm_regmap_init(struct udevice *dev, if (rc) return ERR_PTR(rc); + map = *mapp; + if (config) + map->width = config->width; + devres_add(dev, mapp); return *mapp; } @@ -378,7 +382,8 @@ int regmap_raw_read(struct regmap *map, uint offset, void *valp, size_t val_len) int regmap_read(struct regmap *map, uint offset, uint *valp) { - return regmap_raw_read(map, offset, valp, REGMAP_SIZE_32); + return regmap_raw_read(map, offset, valp, + map->width ? map->width : REGMAP_SIZE_32); } static inline void __write_8(u8 *addr, const u8 *val, @@ -488,7 +493,8 @@ int regmap_raw_write(struct regmap *map, uint offset, const void *val, int regmap_write(struct regmap *map, uint offset, uint val) { - return regmap_raw_write(map, offset, &val, REGMAP_SIZE_32); + return regmap_raw_write(map, offset, &val, + map->width ? map->width : 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 c7dd240a74..9362d864c0 100644 --- a/include/regmap.h +++ b/include/regmap.h @@ -76,16 +76,28 @@ struct regmap_range { }; struct regmap_bus; -struct regmap_config; + +/** + * struct regmap_config - Configure the behaviour of a regmap + * + * @width: Width of the read/write operations. Defaults to + * REGMAP_SIZE_32 if set to 0. + */ +struct regmap_config { + enum regmap_size_t width; +}; /** * struct regmap - a way of accessing hardware/bus registers * + * @width: Width of the read/write operations. Defaults to + * REGMAP_SIZE_32 if set to 0. * @range_count: Number of ranges available within the map * @ranges: Array of ranges */ struct regmap { enum regmap_endianness_t endianness; + enum regmap_size_t width; int range_count; struct regmap_range ranges[0]; }; @@ -96,31 +108,27 @@ struct regmap { */ /** - * regmap_write() - Write a 32-bit value to a regmap + * regmap_write() - Write a value to a regmap * * @map: Regmap to write to * @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. + * If the map's width is not set, this function will write a 32-bit value. * * Return: 0 if OK, -ve on error */ int regmap_write(struct regmap *map, uint offset, uint val); /** - * regmap_read() - Read a 32-bit value from a regmap + * regmap_read() - Read a value 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 * - * 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. + * If the map's width is not set, this function will read a 32-bit value. * * Return: 0 if OK, -ve on error */ @@ -344,7 +352,7 @@ int regmap_init_mem_index(ofnode node, struct regmap **mapp, int index); * @dev: Device that will be interacted with * @bus: Bus-specific callbacks to use with device (IGNORED) * @bus_context: Data passed to bus-specific callbacks (IGNORED) - * @config: Configuration for register map (IGNORED) + * @config: Configuration for register map * * @Return a valid pointer to a struct regmap or a ERR_PTR() on error. * The structure is automatically freed when the device is unbound
Right now, regmap_read() and regmap_write() read/write a 32-bit value only. To write other lengths, regmap_raw_read() and regmap_raw_write() need to be used. This means that any driver ported from Linux that relies on regmap_{read,write}() to know the size already has to be updated at each callsite. This makes the port harder to maintain. So, allow specifying the read/write width to make it easier to port the drivers, since now the only change needed is when initializing the regmap. Signed-off-by: Pratyush Yadav <p.yadav@ti.com> --- drivers/core/regmap.c | 12 +++++++++--- include/regmap.h | 28 ++++++++++++++++++---------- 2 files changed, 27 insertions(+), 13 deletions(-)