reg_base and reg_downshift currently don't have any effect if used with
a regmap_bus or regmap_config which only offers single register
operations (ie. reg_read, reg_write and optionally reg_update_bits).

Fix that and take them into account also for regmap_bus with only
reg_read and read_write operations by applying reg_base and
reg_downshift in _regmap_bus_reg_write, _regmap_bus_reg_read.

Also apply reg_base and reg_downshift in _regmap_update_bits, but only
in case the operation is carried out with a reg_update_bits call
defined in either regmap_bus or regmap_config.

Fixes: 0074f3f2b1e43d ("regmap: allow a defined reg_base to be added to every address")
Fixes: 86fc59ef818beb ("regmap: add configurable downshift for addresses")
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
I hope that I didn't miss anything there...

@Colin Please let me know if this breaks anything with your ocelot_spi
use-case.

 drivers/base/regmap/regmap.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index d12d669157f24..d2a54eb0efd9b 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -1942,6 +1942,8 @@ static int _regmap_bus_reg_write(void *context, unsigned int reg,
 {
 	struct regmap *map = context;
 
+	reg += map->reg_base;
+	reg >>= map->format.reg_downshift;
 	return map->bus->reg_write(map->bus_context, reg, val);
 }
 
@@ -2840,6 +2842,8 @@ static int _regmap_bus_reg_read(void *context, unsigned int reg,
 {
 	struct regmap *map = context;
 
+	reg += map->reg_base;
+	reg >>= map->format.reg_downshift;
 	return map->bus->reg_read(map->bus_context, reg, val);
 }
 
@@ -3231,6 +3235,8 @@ static int _regmap_update_bits(struct regmap *map, unsigned int reg,
 		*change = false;
 
 	if (regmap_volatile(map, reg) && map->reg_update_bits) {
+		reg += map->reg_base;
+		reg >>= map->format.reg_downshift;
 		ret = map->reg_update_bits(map->bus_context, reg, mask, val);
 		if (ret == 0 && change)
 			*change = true;
-- 
2.39.1

On Mon, 30 Jan 2023 02:04:57 +0000, Daniel Golle wrote:
> reg_base and reg_downshift currently don't have any effect if used with
> a regmap_bus or regmap_config which only offers single register
> operations (ie. reg_read, reg_write and optionally reg_update_bits).
> 
> Fix that and take them into account also for regmap_bus with only
> reg_read and read_write operations by applying reg_base and
> reg_downshift in _regmap_bus_reg_write, _regmap_bus_reg_read.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git for-next

Thanks!

[1/1] regmap: apply reg_base and reg_downshift for single register ops
      commit: 697c3892d825fb78f42ec8e53bed065dd728db3e

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

Hi Daniel,

On Mon, Jan 30, 2023 at 02:04:57AM +0000, Daniel Golle wrote:
> reg_base and reg_downshift currently don't have any effect if used with
> a regmap_bus or regmap_config which only offers single register
> operations (ie. reg_read, reg_write and optionally reg_update_bits).
> 
> Fix that and take them into account also for regmap_bus with only
> reg_read and read_write operations by applying reg_base and
> reg_downshift in _regmap_bus_reg_write, _regmap_bus_reg_read.
> 
> Also apply reg_base and reg_downshift in _regmap_update_bits, but only
> in case the operation is carried out with a reg_update_bits call
> defined in either regmap_bus or regmap_config.
> 
> Fixes: 0074f3f2b1e43d ("regmap: allow a defined reg_base to be added to every address")
> Fixes: 86fc59ef818beb ("regmap: add configurable downshift for addresses")
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
> I hope that I didn't miss anything there...
> 
> @Colin Please let me know if this breaks anything with your ocelot_spi
> use-case.

I see we're working on similar things! (DSA hardware, that is)

This patch works for me. I don't konw if there's any value in
back-porting it to affected kernels, as ocelot_spi is the only user as
far as I can tell. (wishing I called it something more greppable than
'reg_base')

Tested-by: Colin Foster <colin.foster@in-advantage.com>

> 
>  drivers/base/regmap/regmap.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
> index d12d669157f24..d2a54eb0efd9b 100644
> --- a/drivers/base/regmap/regmap.c
> +++ b/drivers/base/regmap/regmap.c
> @@ -1942,6 +1942,8 @@ static int _regmap_bus_reg_write(void *context, unsigned int reg,
>  {
>  	struct regmap *map = context;
>  
> +	reg += map->reg_base;
> +	reg >>= map->format.reg_downshift;
>  	return map->bus->reg_write(map->bus_context, reg, val);
>  }
>  
> @@ -2840,6 +2842,8 @@ static int _regmap_bus_reg_read(void *context, unsigned int reg,
>  {
>  	struct regmap *map = context;
>  
> +	reg += map->reg_base;
> +	reg >>= map->format.reg_downshift;
>  	return map->bus->reg_read(map->bus_context, reg, val);
>  }
>  
> @@ -3231,6 +3235,8 @@ static int _regmap_update_bits(struct regmap *map, unsigned int reg,
>  		*change = false;
>  
>  	if (regmap_volatile(map, reg) && map->reg_update_bits) {
> +		reg += map->reg_base;
> +		reg >>= map->format.reg_downshift;
>  		ret = map->reg_update_bits(map->bus_context, reg, mask, val);
>  		if (ret == 0 && change)
>  			*change = true;
> -- 
> 2.39.1
>