[PATCH] regmap: allow upshifting register addresses before performing operations

[PATCH] regmap: allow upshifting register addresses before performing operations

[Maxime Chevallier]

Similar to the existing reg_downshift mechanism, that is used to
translate register addresses on busses that have a smaller address
stride, it's also possible to want to upshift register addresses.

Such a case was encountered when network PHYs and PCS that usually sit
on a MDIO bus (16-bits register with a stride of 1) are integrated
directly as memory-mapped devices. Here, the same register layout
defined in 802.3 is used, but the register now have a larger stride.

Introduce a mechanism to also allow upshifting register addresses.
Re-purpose reg_downshift into a more generic, signed reg_shift, whose
sign indicates the direction of the shift. To avoid confusion, also
introduce macros to explicitly indicate if we want to downshift or
upshift.

For bisectability, change any use of reg_downshift to use reg_shift.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
This is a followup to [1], taking reviews from Andrew and Mark into
account.

Changes are just about the type of reg_shift, from int to s8.

[1] : https://lore.kernel.org/all/20230324093644.464704-1-maxime.chevallier@bootlin.com/

 drivers/base/regmap/internal.h |  2 +-
 drivers/base/regmap/regmap.c   | 10 ++++++++--
 drivers/mfd/ocelot-spi.c       |  2 +-
 include/linux/regmap.h         | 15 ++++++++++++---
 4 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index da8996e7a1f1..dae76ceab6e8 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -31,8 +31,8 @@ struct regmap_format {
 	size_t buf_size;
 	size_t reg_bytes;
 	size_t pad_bytes;
-	size_t reg_downshift;
 	size_t val_bytes;
+	s8 reg_shift;
 	void (*format_write)(struct regmap *map,
 			     unsigned int reg, unsigned int val);
 	void (*format_reg)(void *buf, unsigned int reg, unsigned int shift);
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 726f59612fd6..c4cde4f45b05 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -814,7 +814,7 @@ struct regmap *__regmap_init(struct device *dev,
 
 	map->format.reg_bytes = DIV_ROUND_UP(config->reg_bits, 8);
 	map->format.pad_bytes = config->pad_bits / 8;
-	map->format.reg_downshift = config->reg_downshift;
+	map->format.reg_shift = config->reg_shift;
 	map->format.val_bytes = DIV_ROUND_UP(config->val_bits, 8);
 	map->format.buf_size = DIV_ROUND_UP(config->reg_bits +
 			config->val_bits + config->pad_bits, 8);
@@ -1679,7 +1679,13 @@ static void regmap_set_work_buf_flag_mask(struct regmap *map, int max_bytes,
 static unsigned int regmap_reg_addr(struct regmap *map, unsigned int reg)
 {
 	reg += map->reg_base;
-	return reg >> map->format.reg_downshift;
+
+	if (map->format.reg_shift > 0)
+		reg >>= map->format.reg_shift;
+	else if (map->format.reg_shift < 0)
+		reg <<= -(map->format.reg_shift);
+
+	return reg;
 }
 
 static int _regmap_raw_write_impl(struct regmap *map, unsigned int reg,
diff --git a/drivers/mfd/ocelot-spi.c b/drivers/mfd/ocelot-spi.c
index 2ecd271de2fb..2d1349a10ca9 100644
--- a/drivers/mfd/ocelot-spi.c
+++ b/drivers/mfd/ocelot-spi.c
@@ -125,7 +125,7 @@ static int ocelot_spi_initialize(struct device *dev)
 static const struct regmap_config ocelot_spi_regmap_config = {
 	.reg_bits = 24,
 	.reg_stride = 4,
-	.reg_downshift = 2,
+	.reg_shift = REGMAP_DOWNSHIFT(2),
 	.val_bits = 32,
 
 	.write_flag_mask = 0x80,
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 4d10790adeb0..f02c3857b023 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -46,6 +46,14 @@ struct sdw_slave;
 #define REGMAP_MDIO_C45_DEVAD_MASK	GENMASK(20, 16)
 #define REGMAP_MDIO_C45_REGNUM_MASK	GENMASK(15, 0)
 
+/*
+ * regmap.reg_shift indicates by how much we must shift registers prior to
+ * performing any operation. It's a signed value, positive numbers means
+ * downshifting the register's address, while negative numbers means upshifting.
+ */
+#define REGMAP_UPSHIFT(s)	(-(s))
+#define REGMAP_DOWNSHIFT(s)	(s)
+
 /* An enum of all the supported cache types */
 enum regcache_type {
 	REGCACHE_NONE,
@@ -246,8 +254,9 @@ typedef void (*regmap_unlock)(void *);
  * @reg_stride: The register address stride. Valid register addresses are a
  *              multiple of this value. If set to 0, a value of 1 will be
  *              used.
- * @reg_downshift: The number of bits to downshift the register before
- *		   performing any operations.
+ * @reg_shift: The number of bits to shift the register before performing any
+ *	       operations. Any positive number will be downshifted, and negative
+ *	       values will be upshifted
  * @reg_base: Value to be added to every register address before performing any
  *	      operation.
  * @pad_bits: Number of bits of padding between register and value.
@@ -381,7 +390,7 @@ struct regmap_config {
 
 	int reg_bits;
 	int reg_stride;
-	int reg_downshift;
+	int reg_shift;
 	unsigned int reg_base;
 	int pad_bits;
 	int val_bits;
-- 
2.39.2

Re: [PATCH] regmap: allow upshifting register addresses before performing operations

[Colin Foster]

Hi Maxime,

On Fri, Apr 07, 2023 at 05:26:04PM +0200, Maxime Chevallier wrote:
>  	.reg_stride = 4,
> -	.reg_downshift = 2,
> +	.reg_shift = REGMAP_DOWNSHIFT(2),
>  	.val_bits = 32,

Looks great! Thanks. I tested this with a merge of net-next and
regmap-next + this patch.

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

Re: [PATCH] regmap: allow upshifting register addresses before performing operations

[Maxime Chevallier]

Hi Colin,

On Fri, 7 Apr 2023 08:58:53 -0700
Colin Foster <colin.foster@in-advantage.com> wrote:

> Hi Maxime,
> 
> On Fri, Apr 07, 2023 at 05:26:04PM +0200, Maxime Chevallier wrote:
> >  	.reg_stride = 4,
> > -	.reg_downshift = 2,
> > +	.reg_shift = REGMAP_DOWNSHIFT(2),
> >  	.val_bits = 32,  
> 
> Looks great! Thanks. I tested this with a merge of net-next and
> regmap-next + this patch.
> 
> Tested-by: Colin Foster <colin.foster@in-advantage.com>

Thanks for the test !

Maxime

Re: [PATCH] regmap: allow upshifting register addresses before performing operations

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 364 bytes --]

On Fri, Apr 07, 2023 at 05:26:04PM +0200, Maxime Chevallier wrote:
> Similar to the existing reg_downshift mechanism, that is used to
> translate register addresses on busses that have a smaller address
> stride, it's also possible to want to upshift register addresses.

There were some KUnit tests added for regmap, could you add
coverage for this there please?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] regmap: allow upshifting register addresses before performing operations

[Maxime Chevallier]

Hello Mark,

On Fri, 7 Apr 2023 17:25:58 +0100
Mark Brown <broonie@kernel.org> wrote:

> On Fri, Apr 07, 2023 at 05:26:04PM +0200, Maxime Chevallier wrote:
> > Similar to the existing reg_downshift mechanism, that is used to
> > translate register addresses on busses that have a smaller address
> > stride, it's also possible to want to upshift register addresses.  
> 
> There were some KUnit tests added for regmap, could you add
> coverage for this there please?

Sure, I will take a look and update the test.

Thanks,

Maxime

Re: [PATCH] regmap: allow upshifting register addresses before performing operations

[Mark Brown]

On Fri, 07 Apr 2023 17:26:04 +0200, Maxime Chevallier wrote:
> Similar to the existing reg_downshift mechanism, that is used to
> translate register addresses on busses that have a smaller address
> stride, it's also possible to want to upshift register addresses.
> 
> Such a case was encountered when network PHYs and PCS that usually sit
> on a MDIO bus (16-bits register with a stride of 1) are integrated
> directly as memory-mapped devices. Here, the same register layout
> defined in 802.3 is used, but the register now have a larger stride.
> 
> [...]

Applied to

   broonie/regmap.git for-next

Thanks!

[1/1] regmap: allow upshifting register addresses before performing operations
      commit: 4a670ac3e75e517c96cbd01ef870dbd598c3ce71

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