2022-03-14 20:03:08

by Colin Foster

[permalink] [raw]
Subject: [RFC v1 0/2] Add more detailed regmap formatting capabilities

The Ocelot chips (specifically the VSC7512 I'm using) have a method of
accessing their registers internally via MMIO, or externally via SPI.
When accessing these registers externally, a 24-bit address is used and
downshifted by two. The manual references it as:

SI_ADDR = (REG_ADDR & 0x00FFFFFF) >> 2;

By adding this configurable downshift, and a configurable register base
address, the regmap definitions can be shared between MMIO and SPI
configurations.

This also allows regmap to be used in a bus configuration. My initial
testing shows that even at a much slower bus speed of 500KHz, I'm seeing
an improvement of 10ms (was 14... now 4) to perform bulk read operations.

The relevant MMIO code can be found in drivers/net/mscc/ocelot_io.c:

static struct regmap_config ocelot_regmap_config = {
.reg_bits = 32,
.val_bits = 32,
.reg_stride = 4,
};

struct regmap *ocelot_regmap_init(struct ocelot *ocelot, struct resource *res)
{
void __iomem *regs;

regs = devm_ioremap_resource(ocelot->dev, res);
if (IS_ERR(regs))
return ERR_CAST(regs);

ocelot_regmap_config.name = res->name;

return devm_regmap_init_mmio(ocelot->dev, regs, &ocelot_regmap_config);
}


And the SPI counterpart is slightly more complex:


static const struct regmap_config ocelot_spi_regmap_config = {
.reg_bits = 24,
.reg_stride = 4,
.reg_downshift = 2,
.val_bits = 32,

.write_flag_mask = 0x80,

.max_register = 0xffffffff,
.use_single_write = true,
.can_multi_write = false,

.reg_format_endian = REGMAP_ENDIAN_BIG,
.val_format_endian = REGMAP_ENDIAN_NATIVE,
};

static const struct regmap_bus ocelot_spi_regmap_bus = {
.write = ocelot_spi_regmap_bus_write,
.read = ocelot_spi_regmap_bus_read,
};

struct regmap *
ocelot_spi_devm_get_regmap(struct ocelot_core *core, struct device *child,
const struct resource *res)
{
struct regmap_config regmap_config;

memcpy(&regmap_config, &ocelot_spi_regmap_config,
sizeof(ocelot_spi_regmap_config));

regmap_config.name = res->name;
regmap_config.max_register = res->end - res->start;
regmap_config.reg_base = res->start;

return devm_regmap_init(child, &ocelot_spi_regmap_bus, core,
&regmap_config);
}


If there's anything I missed, or if there's a different way to go about
this, please let me know. I can also drag this along with my VSC7512
development or I can send this patch (or whatever it might become) after
the merge window.


Colin Foster (2):
regmap: add configurable downshift for addresses
regmap: allow a defined reg_base to be added to every address

drivers/base/regmap/internal.h | 2 ++
drivers/base/regmap/regmap.c | 11 +++++++++++
include/linux/regmap.h | 6 ++++++
3 files changed, 19 insertions(+)

--
2.25.1


2022-03-15 01:07:30

by Colin Foster

[permalink] [raw]
Subject: [RFC v1 1/2] regmap: add configurable downshift for addresses

Add an additional reg_downshift to be applied to register addresses before
any register accesses. An example of a device that uses this is a VSC7514
chip, which require each register address to be downshifted by two if the
access is performed over a SPI bus.

Signed-off-by: Colin Foster <[email protected]>
---
drivers/base/regmap/internal.h | 1 +
drivers/base/regmap/regmap.c | 5 +++++
include/linux/regmap.h | 3 +++
3 files changed, 9 insertions(+)

diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index b1905916f7af..88f710e7ce31 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -31,6 +31,7 @@ struct regmap_format {
size_t buf_size;
size_t reg_bytes;
size_t pad_bytes;
+ size_t reg_downshift;
size_t val_bytes;
void (*format_write)(struct regmap *map,
unsigned int reg, unsigned int val);
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 8f9fe5fd4707..1c7c6d6361af 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -823,6 +823,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.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);
@@ -1735,6 +1736,7 @@ static int _regmap_raw_write_impl(struct regmap *map, unsigned int reg,
return ret;
}

+ reg >>= map->format.reg_downshift;
map->format.format_reg(map->work_buf, reg, map->reg_shift);
regmap_set_work_buf_flag_mask(map, map->format.reg_bytes,
map->write_flag_mask);
@@ -1905,6 +1907,7 @@ static int _regmap_bus_formatted_write(void *context, unsigned int reg,
return ret;
}

+ reg >>= map->format.reg_downshift;
map->format.format_write(map, reg, val);

trace_regmap_hw_write_start(map, reg, 1);
@@ -2346,6 +2349,7 @@ static int _regmap_raw_multi_reg_write(struct regmap *map,
unsigned int reg = regs[i].reg;
unsigned int val = regs[i].def;
trace_regmap_hw_write_start(map, reg, 1);
+ reg >>= map->format.reg_downshift;
map->format.format_reg(u8, reg, map->reg_shift);
u8 += reg_bytes + pad_bytes;
map->format.format_val(u8, val, 0);
@@ -2673,6 +2677,7 @@ static int _regmap_raw_read(struct regmap *map, unsigned int reg, void *val,
return ret;
}

+ reg >>= map->format.reg_downshift;
map->format.format_reg(map->work_buf, reg, map->reg_shift);
regmap_set_work_buf_flag_mask(map, map->format.reg_bytes,
map->read_flag_mask);
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 22652e5fbc38..40fb9399add6 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -237,6 +237,8 @@ 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.
* @pad_bits: Number of bits of padding between register and value.
* @val_bits: Number of bits in a register value, mandatory.
*
@@ -360,6 +362,7 @@ struct regmap_config {

int reg_bits;
int reg_stride;
+ int reg_downshift;
int pad_bits;
int val_bits;

--
2.25.1

2022-03-21 14:12:45

by Mark Brown

[permalink] [raw]
Subject: Re: [RFC v1 0/2] Add more detailed regmap formatting capabilities

On Sun, 13 Mar 2022 15:45:22 -0700, Colin Foster wrote:
> The Ocelot chips (specifically the VSC7512 I'm using) have a method of
> accessing their registers internally via MMIO, or externally via SPI.
> When accessing these registers externally, a 24-bit address is used and
> downshifted by two. The manual references it as:
>
> SI_ADDR = (REG_ADDR & 0x00FFFFFF) >> 2;
>
> [...]

Applied to

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

Thanks!

[1/2] regmap: add configurable downshift for addresses
commit: 86fc59ef818beb0e1945d17f8e734898baba7e4e
[2/2] regmap: allow a defined reg_base to be added to every address
commit: 0074f3f2b1e43d3cedd97e47fb6980db6d2ba79e

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

2022-03-21 20:07:15

by Colin Foster

[permalink] [raw]
Subject: Re: [RFC v1 0/2] Add more detailed regmap formatting capabilities

On Fri, Mar 18, 2022 at 08:57:54PM +0000, Mark Brown wrote:
> On Sun, 13 Mar 2022 15:45:22 -0700, Colin Foster wrote:
> > The Ocelot chips (specifically the VSC7512 I'm using) have a method of
> > accessing their registers internally via MMIO, or externally via SPI.
> > When accessing these registers externally, a 24-bit address is used and
> > downshifted by two. The manual references it as:
> >
> > SI_ADDR = (REG_ADDR & 0x00FFFFFF) >> 2;
> >
> > [...]
>
> Applied to
>
> https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git for-next
>
> Thanks!
>
> [1/2] regmap: add configurable downshift for addresses
> commit: 86fc59ef818beb0e1945d17f8e734898baba7e4e
> [2/2] regmap: allow a defined reg_base to be added to every address
> commit: 0074f3f2b1e43d3cedd97e47fb6980db6d2ba79e
>
> 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! Based on your initial feedback I wasn't expecting patch 1
to be applied and was expecting to have to work around it (which could
be done) but I don't think patch 2 could have been reasonably worked
around. Hopefully these are patches that nobody notices... unless they
need them too :-D

>
> Thanks,
> Mark