| Message ID | 1524493012-2587-1-git-send-email-narmstrong@baylibre.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Simon Glass |
| Headers | show |
| Series | [U-Boot] regmap: add regmap_update_bits() helper | expand |
Hi Neil, On 23 April 2018 at 08:16, Neil Armstrong <narmstrong@baylibre.com> wrote: > Add the regmap_update_bits() to simply the read/modify/write of registers > in a single command. The function is taken from Linux regmap > implementation. > > Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> > --- > include/regmap.h | 23 +++++++++++++++++++++++ > 1 file changed, 23 insertions(+) > > diff --git a/include/regmap.h b/include/regmap.h > index 493a5d8..3c463e9 100644 > --- a/include/regmap.h > +++ b/include/regmap.h > @@ -47,6 +47,29 @@ int regmap_read(struct regmap *map, uint offset, uint *valp); > regmap_read(map, (uint32_t *)(ptr)->member - (uint32_t *)(ptr), valp) > > /** > + * regmap_update_bits() - Perform a read/modify/write using a mask > + * > + * @map: The map returned by regmap_init_mem*() > + * @offset: Offset of the memory > + * @mask: Mask to apply to the read value > + * @val: Value to apply to the value to write > + */ > +static inline int regmap_update_bits(struct regmap *map, uint offset, > + uint mask, uint val) Why is this inline? I think it would save code size to make it a normal function. Also can you add a call to this function somewhere for sandbox, as a test? > +{ > + uint reg; > + int ret; > + > + ret = regmap_read(map, offset, ®); > + if (ret) > + return ret; > + > + reg &= ~mask; > + > + return regmap_write(map, offset, reg | val); > +} > + > +/** > * regmap_init_mem() - Set up a new register map that uses memory access > * > * Use regmap_uninit() to free it. > -- > 2.7.4 > Regards, Simon
Hi, On 26/04/2018 16:40, Simon Glass wrote: > Hi Neil, > > On 23 April 2018 at 08:16, Neil Armstrong <narmstrong@baylibre.com> wrote: >> Add the regmap_update_bits() to simply the read/modify/write of registers >> in a single command. The function is taken from Linux regmap >> implementation. >> >> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> >> --- >> include/regmap.h | 23 +++++++++++++++++++++++ >> 1 file changed, 23 insertions(+) >> >> diff --git a/include/regmap.h b/include/regmap.h >> index 493a5d8..3c463e9 100644 >> --- a/include/regmap.h >> +++ b/include/regmap.h >> @@ -47,6 +47,29 @@ int regmap_read(struct regmap *map, uint offset, uint *valp); >> regmap_read(map, (uint32_t *)(ptr)->member - (uint32_t *)(ptr), valp) >> >> /** >> + * regmap_update_bits() - Perform a read/modify/write using a mask >> + * >> + * @map: The map returned by regmap_init_mem*() >> + * @offset: Offset of the memory >> + * @mask: Mask to apply to the read value >> + * @val: Value to apply to the value to write >> + */ >> +static inline int regmap_update_bits(struct regmap *map, uint offset, >> + uint mask, uint val) > > Why is this inline? I think it would save code size to make it a > normal function. Seemed a good idea, but I'll move it to the uclass. > > Also can you add a call to this function somewhere for sandbox, as a test? I did not find any test for regmap_read/write for sandbox, or maybe I missed it. > >> +{ >> + uint reg; >> + int ret; >> + >> + ret = regmap_read(map, offset, ®); >> + if (ret) >> + return ret; >> + >> + reg &= ~mask; >> + >> + return regmap_write(map, offset, reg | val); >> +} >> + >> +/** >> * regmap_init_mem() - Set up a new register map that uses memory access >> * >> * Use regmap_uninit() to free it. >> -- >> 2.7.4 >> > > Regards, > Simon > Thanks, Neil
Hi Neil, On 26 April 2018 at 09:23, Neil Armstrong <narmstrong@baylibre.com> wrote: > Hi, > > On 26/04/2018 16:40, Simon Glass wrote: >> Hi Neil, >> >> On 23 April 2018 at 08:16, Neil Armstrong <narmstrong@baylibre.com> wrote: >>> Add the regmap_update_bits() to simply the read/modify/write of registers >>> in a single command. The function is taken from Linux regmap >>> implementation. >>> >>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> >>> --- >>> include/regmap.h | 23 +++++++++++++++++++++++ >>> 1 file changed, 23 insertions(+) >>> >>> diff --git a/include/regmap.h b/include/regmap.h >>> index 493a5d8..3c463e9 100644 >>> --- a/include/regmap.h >>> +++ b/include/regmap.h >>> @@ -47,6 +47,29 @@ int regmap_read(struct regmap *map, uint offset, uint *valp); >>> regmap_read(map, (uint32_t *)(ptr)->member - (uint32_t *)(ptr), valp) >>> >>> /** >>> + * regmap_update_bits() - Perform a read/modify/write using a mask >>> + * >>> + * @map: The map returned by regmap_init_mem*() >>> + * @offset: Offset of the memory >>> + * @mask: Mask to apply to the read value >>> + * @val: Value to apply to the value to write >>> + */ >>> +static inline int regmap_update_bits(struct regmap *map, uint offset, >>> + uint mask, uint val) >> >> Why is this inline? I think it would save code size to make it a >> normal function. > > Seemed a good idea, but I'll move it to the uclass. > >> >> Also can you add a call to this function somewhere for sandbox, as a test? > > I did not find any test for regmap_read/write for sandbox, or maybe I missed it. > No, that's why I'm asking :-) Regards, Simon
diff --git a/include/regmap.h b/include/regmap.h index 493a5d8..3c463e9 100644 --- a/include/regmap.h +++ b/include/regmap.h @@ -47,6 +47,29 @@ int regmap_read(struct regmap *map, uint offset, uint *valp); regmap_read(map, (uint32_t *)(ptr)->member - (uint32_t *)(ptr), valp) /** + * regmap_update_bits() - Perform a read/modify/write using a mask + * + * @map: The map returned by regmap_init_mem*() + * @offset: Offset of the memory + * @mask: Mask to apply to the read value + * @val: Value to apply to the value to write + */ +static inline int regmap_update_bits(struct regmap *map, uint offset, + uint mask, uint val) +{ + uint reg; + int ret; + + ret = regmap_read(map, offset, ®); + if (ret) + return ret; + + reg &= ~mask; + + return regmap_write(map, offset, reg | val); +} + +/** * regmap_init_mem() - Set up a new register map that uses memory access * * Use regmap_uninit() to free it.
Add the regmap_update_bits() to simply the read/modify/write of registers in a single command. The function is taken from Linux regmap implementation. Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> --- include/regmap.h | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)