From 6466b376e927d51ea3eadc1965714305d8c3c066 Mon Sep 17 00:00:00 2001 From: Charles Keepax Date: Thu, 12 Jan 2023 17:18:39 +0000 Subject: [PATCH 01/19] regmap: sdw: Update misleading comment In the regmap config reg_bits represents the number of address bits not the number of value bits. Correct the misleading comment which looks a lot like it suggests the register value itself is 32-bits wide. Signed-off-by: Charles Keepax Link: https://lore.kernel.org/r/20230112171840.2098463-2-ckeepax@opensource.cirrus.com Signed-off-by: Mark Brown --- drivers/base/regmap/regmap-sdw.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/base/regmap/regmap-sdw.c b/drivers/base/regmap/regmap-sdw.c index 81b0327f719d..95801fd411b2 100644 --- a/drivers/base/regmap/regmap-sdw.c +++ b/drivers/base/regmap/regmap-sdw.c @@ -43,7 +43,7 @@ static int regmap_sdw_config_check(const struct regmap_config *config) if (config->val_bits != 8) return -ENOTSUPP; - /* Registers are 32 bits wide */ + /* Register addresses are 32 bits wide */ if (config->reg_bits != 32) return -ENOTSUPP; From 522272047dc631610dd180be0c3670043bcdf42e Mon Sep 17 00:00:00 2001 From: Lucas Tanure Date: Thu, 12 Jan 2023 17:18:40 +0000 Subject: [PATCH 02/19] regmap: sdw: Remove 8-bit value size restriction Some SoundWire devices have larger width device specific register maps, in addition to the standard SoundWire 8-bit map. Update the helpers to allow accessing arbitrarily sized register values and remove the explicit 8-bit restriction from regmap_sdw_config_check. Signed-off-by: Lucas Tanure Signed-off-by: Charles Keepax Link: https://lore.kernel.org/r/20230112171840.2098463-3-ckeepax@opensource.cirrus.com Signed-off-by: Mark Brown --- drivers/base/regmap/regmap-sdw.c | 39 ++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/drivers/base/regmap/regmap-sdw.c b/drivers/base/regmap/regmap-sdw.c index 95801fd411b2..09899ae99fc1 100644 --- a/drivers/base/regmap/regmap-sdw.c +++ b/drivers/base/regmap/regmap-sdw.c @@ -6,43 +6,52 @@ #include #include #include +#include #include "internal.h" -static int regmap_sdw_write(void *context, unsigned int reg, unsigned int val) +static int regmap_sdw_write(void *context, const void *val_buf, size_t val_size) { struct device *dev = context; struct sdw_slave *slave = dev_to_sdw_dev(dev); + /* First word of buffer contains the destination address */ + u32 addr = le32_to_cpu(*(const __le32 *)val_buf); + const u8 *val = val_buf; - return sdw_write_no_pm(slave, reg, val); + return sdw_nwrite_no_pm(slave, addr, val_size - sizeof(addr), val + sizeof(addr)); } -static int regmap_sdw_read(void *context, unsigned int reg, unsigned int *val) +static int regmap_sdw_gather_write(void *context, + const void *reg_buf, size_t reg_size, + const void *val_buf, size_t val_size) { struct device *dev = context; struct sdw_slave *slave = dev_to_sdw_dev(dev); - int read; + u32 addr = le32_to_cpu(*(const __le32 *)reg_buf); - read = sdw_read_no_pm(slave, reg); - if (read < 0) - return read; + return sdw_nwrite_no_pm(slave, addr, val_size, val_buf); +} - *val = read; - return 0; +static int regmap_sdw_read(void *context, + const void *reg_buf, size_t reg_size, + void *val_buf, size_t val_size) +{ + struct device *dev = context; + struct sdw_slave *slave = dev_to_sdw_dev(dev); + u32 addr = le32_to_cpu(*(const __le32 *)reg_buf); + + return sdw_nread_no_pm(slave, addr, val_size, val_buf); } static const struct regmap_bus regmap_sdw = { - .reg_read = regmap_sdw_read, - .reg_write = regmap_sdw_write, + .write = regmap_sdw_write, + .gather_write = regmap_sdw_gather_write, + .read = regmap_sdw_read, .reg_format_endian_default = REGMAP_ENDIAN_LITTLE, .val_format_endian_default = REGMAP_ENDIAN_LITTLE, }; static int regmap_sdw_config_check(const struct regmap_config *config) { - /* All register are 8-bits wide as per MIPI Soundwire 1.0 Spec */ - if (config->val_bits != 8) - return -ENOTSUPP; - /* Register addresses are 32 bits wide */ if (config->reg_bits != 32) return -ENOTSUPP; From 4d60cac951fd2dfbf261b83abb805748abc8b995 Mon Sep 17 00:00:00 2001 From: William Breathitt Gray Date: Mon, 27 Feb 2023 11:45:22 -0500 Subject: [PATCH 03/19] regmap-irq: Add no_status support Some devices lack status registers, yet expect to handle interrupts. Introduce a no_status flag to indicate such a configuration, where rather than read a status register to verify, all interrupts received are assumed to be active. Cc: Mark Brown Signed-off-by: William Breathitt Gray Link: https://lore.kernel.org/r/bd501b4b5ff88da24d467f75e8c71b4e0e6f21e2.1677515341.git.william.gray@linaro.org Signed-off-by: Mark Brown --- drivers/base/regmap/regmap-irq.c | 22 +++++++++++++++------- include/linux/regmap.h | 2 ++ 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/drivers/base/regmap/regmap-irq.c b/drivers/base/regmap/regmap-irq.c index 8c903b8c9714..f4d544ee7c30 100644 --- a/drivers/base/regmap/regmap-irq.c +++ b/drivers/base/regmap/regmap-irq.c @@ -433,7 +433,10 @@ static irqreturn_t regmap_irq_thread(int irq, void *d) * possible in order to reduce the I/O overheads. */ - if (chip->num_main_regs) { + if (chip->no_status) { + /* no status register so default to all active */ + memset32(data->status_buf, GENMASK(31, 0), chip->num_regs); + } else if (chip->num_main_regs) { unsigned int max_main_bits; unsigned long size; @@ -949,12 +952,17 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode, continue; /* Ack masked but set interrupts */ - reg = d->get_irq_reg(d, d->chip->status_base, i); - ret = regmap_read(map, reg, &d->status_buf[i]); - if (ret != 0) { - dev_err(map->dev, "Failed to read IRQ status: %d\n", - ret); - goto err_alloc; + if (d->chip->no_status) { + /* no status register so default to all active */ + d->status_buf[i] = GENMASK(31, 0); + } else { + reg = d->get_irq_reg(d, d->chip->status_base, i); + ret = regmap_read(map, reg, &d->status_buf[i]); + if (ret != 0) { + dev_err(map->dev, "Failed to read IRQ status: %d\n", + ret); + goto err_alloc; + } } if (chip->status_invert) diff --git a/include/linux/regmap.h b/include/linux/regmap.h index 4d10790adeb0..301286149643 100644 --- a/include/linux/regmap.h +++ b/include/linux/regmap.h @@ -1567,6 +1567,7 @@ struct regmap_irq_chip_data; * the need for a @sub_reg_offsets table. * @status_invert: Inverted status register: cleared bits are active interrupts. * @runtime_pm: Hold a runtime PM lock on the device when accessing it. + * @no_status: No status register: all interrupts assumed generated by device. * * @num_regs: Number of registers in each control bank. * @irqs: Descriptors for individual IRQs. Interrupt numbers are @@ -1631,6 +1632,7 @@ struct regmap_irq_chip { unsigned int clear_on_unmask:1; unsigned int not_fixed_stride:1; unsigned int status_invert:1; + unsigned int no_status:1; int num_regs; From 9b400171a69d2487c3196cc3b6de60de3b08e1ee Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Mon, 20 Feb 2023 17:33:34 +0200 Subject: [PATCH 04/19] regmap-irq: Place kernel doc of struct regmap_irq_chip in order It seems that a couple of members got lost theirorder, put them back. Besides that, split field descriptions into groups in the same way as it's done in the structure definition. Signed-off-by: Andy Shevchenko Link: https://lore.kernel.org/r/20230220153334.87049-1-andriy.shevchenko@linux.intel.com Signed-off-by: Mark Brown --- include/linux/regmap.h | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/include/linux/regmap.h b/include/linux/regmap.h index 4d10790adeb0..1c777566fb7d 100644 --- a/include/linux/regmap.h +++ b/include/linux/regmap.h @@ -1551,6 +1551,7 @@ struct regmap_irq_chip_data; * @use_ack: Use @ack register even if it is zero. * @ack_invert: Inverted ack register: cleared bits for ack. * @clear_ack: Use this to set 1 and 0 or vice-versa to clear interrupts. + * @status_invert: Inverted status register: cleared bits are active interrupts. * @wake_invert: Inverted wake register: cleared bits are wake enabled. * @type_in_mask: Use the mask registers for controlling irq type. Use this if * the hardware provides separate bits for rising/falling edge @@ -1560,18 +1561,19 @@ struct regmap_irq_chip_data; * @clear_on_unmask: For chips with interrupts cleared on read: read the status * registers before unmasking interrupts to clear any bits * set when they were masked. + * @runtime_pm: Hold a runtime PM lock on the device when accessing it. * @not_fixed_stride: Used when chip peripherals are not laid out with fixed * stride. Must be used with sub_reg_offsets containing the * offsets to each peripheral. Deprecated; the same thing * can be accomplished with a @get_irq_reg callback, without * the need for a @sub_reg_offsets table. - * @status_invert: Inverted status register: cleared bits are active interrupts. - * @runtime_pm: Hold a runtime PM lock on the device when accessing it. * * @num_regs: Number of registers in each control bank. + * * @irqs: Descriptors for individual IRQs. Interrupt numbers are * assigned based on the index in the array of the interrupt. * @num_irqs: Number of descriptors. + * * @num_type_reg: Number of type registers. Deprecated, use config registers * instead. * @num_virt_regs: Number of non-standard irq configuration registers. @@ -1579,6 +1581,7 @@ struct regmap_irq_chip_data; * instead. * @num_config_bases: Number of config base registers. * @num_config_regs: Number of config registers for each config base register. + * * @handle_pre_irq: Driver specific callback to handle interrupt from device * before regmap_irq_handler process the interrupts. * @handle_post_irq: Driver specific callback to handle interrupt from device @@ -1625,12 +1628,12 @@ struct regmap_irq_chip { unsigned int use_ack:1; unsigned int ack_invert:1; unsigned int clear_ack:1; + unsigned int status_invert:1; unsigned int wake_invert:1; - unsigned int runtime_pm:1; unsigned int type_in_mask:1; unsigned int clear_on_unmask:1; + unsigned int runtime_pm:1; unsigned int not_fixed_stride:1; - unsigned int status_invert:1; int num_regs; From fd883d79e4dcd2417c2b80756f22a2ff03b0f6e0 Mon Sep 17 00:00:00 2001 From: Alexander Stein Date: Mon, 13 Mar 2023 08:18:11 +0100 Subject: [PATCH 05/19] regmap: cache: Return error in cache sync operations for REGCACHE_NONE There is no sense in doing a cache sync on REGCACHE_NONE regmaps. Instead of panicking the kernel due to missing cache_ops, return an error to client driver. Signed-off-by: Alexander Stein Link: https://lore.kernel.org/r/20230313071812.13577-1-alexander.stein@ew.tq-group.com Signed-off-by: Mark Brown --- drivers/base/regmap/regcache.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c index 362e043e26d8..8031007b4887 100644 --- a/drivers/base/regmap/regcache.c +++ b/drivers/base/regmap/regcache.c @@ -349,6 +349,9 @@ int regcache_sync(struct regmap *map) const char *name; bool bypass; + if (WARN_ON(map->cache_type == REGCACHE_NONE)) + return -EINVAL; + BUG_ON(!map->cache_ops); map->lock(map->lock_arg); @@ -418,6 +421,9 @@ int regcache_sync_region(struct regmap *map, unsigned int min, const char *name; bool bypass; + if (WARN_ON(map->cache_type == REGCACHE_NONE)) + return -EINVAL; + BUG_ON(!map->cache_ops); map->lock(map->lock_arg); From 24d80fde40c995b2e2faaf72034e12e60a416630 Mon Sep 17 00:00:00 2001 From: Alexander Stein Date: Mon, 13 Mar 2023 08:18:12 +0100 Subject: [PATCH 06/19] regmap: cache: Silence checkpatch warning checkpatch.pl warned: WARNING: ENOSYS means 'invalid syscall nr' and nothing else Align the return value to regcache_drop_region(). Signed-off-by: Alexander Stein Link: https://lore.kernel.org/r/20230313071812.13577-2-alexander.stein@ew.tq-group.com Signed-off-by: Mark Brown --- drivers/base/regmap/regcache.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c index 8031007b4887..0482cf1c3231 100644 --- a/drivers/base/regmap/regcache.c +++ b/drivers/base/regmap/regcache.c @@ -242,7 +242,7 @@ int regcache_read(struct regmap *map, int ret; if (map->cache_type == REGCACHE_NONE) - return -ENOSYS; + return -EINVAL; BUG_ON(!map->cache_ops); From 3f58f6dc4d92ed6fae4a4da0d5b091e00ec10fa8 Mon Sep 17 00:00:00 2001 From: Maxime Chevallier Date: Fri, 24 Mar 2023 10:36:38 +0100 Subject: [PATCH 07/19] regmap: add a helper to translate the register address Register addresses passed to regmap operations can be offset with regmap.reg_base and downshifted with regmap.reg_downshift. Add a helper to apply both these operations and return the translated address, that we can then use to perform the actual register operation ont the underlying bus. Signed-off-by: Maxime Chevallier Link: https://lore.kernel.org/r/20230324093644.464704-2-maxime.chevallier@bootlin.com Signed-off-by: Mark Brown --- drivers/base/regmap/regmap.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c index d2a54eb0efd9..a4e4367648bf 100644 --- a/drivers/base/regmap/regmap.c +++ b/drivers/base/regmap/regmap.c @@ -1676,6 +1676,12 @@ static void regmap_set_work_buf_flag_mask(struct regmap *map, int max_bytes, buf[i] |= (mask >> (8 * i)) & 0xff; } +static unsigned int regmap_reg_addr(struct regmap *map, unsigned int reg) +{ + reg += map->reg_base; + return reg >> map->format.reg_downshift; +} + static int _regmap_raw_write_impl(struct regmap *map, unsigned int reg, const void *val, size_t val_len, bool noinc) { @@ -1753,8 +1759,7 @@ static int _regmap_raw_write_impl(struct regmap *map, unsigned int reg, return ret; } - reg += map->reg_base; - reg >>= map->format.reg_downshift; + reg = regmap_reg_addr(map, reg); 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); @@ -1924,8 +1929,7 @@ static int _regmap_bus_formatted_write(void *context, unsigned int reg, return ret; } - reg += map->reg_base; - reg >>= map->format.reg_downshift; + reg = regmap_reg_addr(map, reg); map->format.format_write(map, reg, val); trace_regmap_hw_write_start(map, reg, 1); @@ -1942,8 +1946,7 @@ static int _regmap_bus_reg_write(void *context, unsigned int reg, { struct regmap *map = context; - reg += map->reg_base; - reg >>= map->format.reg_downshift; + reg = regmap_reg_addr(map, reg); return map->bus->reg_write(map->bus_context, reg, val); } @@ -2494,8 +2497,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->reg_base; - reg >>= map->format.reg_downshift; + reg = regmap_reg_addr(map, reg); map->format.format_reg(u8, reg, map->reg_shift); u8 += reg_bytes + pad_bytes; map->format.format_val(u8, val, 0); @@ -2821,8 +2823,7 @@ static int _regmap_raw_read(struct regmap *map, unsigned int reg, void *val, return ret; } - reg += map->reg_base; - reg >>= map->format.reg_downshift; + reg = regmap_reg_addr(map, reg); 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); @@ -2842,8 +2843,7 @@ static int _regmap_bus_reg_read(void *context, unsigned int reg, { struct regmap *map = context; - reg += map->reg_base; - reg >>= map->format.reg_downshift; + reg = regmap_reg_addr(map, reg); return map->bus->reg_read(map->bus_context, reg, val); } @@ -3235,8 +3235,7 @@ 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; + reg = regmap_reg_addr(map, reg); ret = map->reg_update_bits(map->bus_context, reg, mask, val); if (ret == 0 && change) *change = true; From 2c89db8f8d1e544fd817d4c0dc508a00b78a8f7f Mon Sep 17 00:00:00 2001 From: Mark Brown Date: Sat, 25 Mar 2023 21:49:49 +0000 Subject: [PATCH 08/19] regmap: Handle sparse caches in the default sync If there is no cache entry available we will get -ENOENT from the cache implementation, handle this gracefully and skip rather than treating it as an error. Signed-off-by: Mark Brown Link: https://lore.kernel.org/r/20230325-regcache-sparse-sync-v1-1-2a890239d061@kernel.org --- drivers/base/regmap/regcache.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c index 0482cf1c3231..c53eabd4855d 100644 --- a/drivers/base/regmap/regcache.c +++ b/drivers/base/regmap/regcache.c @@ -311,6 +311,8 @@ static int regcache_default_sync(struct regmap *map, unsigned int min, continue; ret = regcache_read(map, reg, &val); + if (ret == -ENOENT) + continue; if (ret) return ret; From 2d38e8615a21e264042870f811247d5c52c27f4e Mon Sep 17 00:00:00 2001 From: Mark Brown Date: Fri, 24 Mar 2023 16:22:26 +0000 Subject: [PATCH 09/19] regmap: Clarify error for unknown cache types The error message printed when we fail to locate the cache type the map requested says it can't find a compress type rather than a cache type, fix that. Since the compressed type is the only one currently compiled conditionally it's likely to be the missing type but that might not always be true and is still unclear. Signed-off-by: Mark Brown Link: https://lore.kernel.org/r/20230324-regcache-unknown-v1-1-80deecbf196b@kernel.org --- drivers/base/regmap/regcache.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c index c53eabd4855d..7a337e8d08a9 100644 --- a/drivers/base/regmap/regcache.c +++ b/drivers/base/regmap/regcache.c @@ -148,7 +148,7 @@ int regcache_init(struct regmap *map, const struct regmap_config *config) break; if (i == ARRAY_SIZE(cache_types)) { - dev_err(map->dev, "Could not match compress type: %d\n", + dev_err(map->dev, "Could not match cache type: %d\n", map->cache_type); return -EINVAL; } From f18ee501e233a2b830a0c84a2e780ab02d946c04 Mon Sep 17 00:00:00 2001 From: Mark Brown Date: Fri, 24 Mar 2023 22:58:42 +0000 Subject: [PATCH 10/19] regmap: Support paging for buses with reg_read()/reg_write() We don't currently support paging for regmaps where the I/O happens through bus provided reg_read() and reg_write() operatons, we simply ignore the range since nothing is wired up properly. Wire things up. Signed-off-by: Mark Brown Link: https://lore.kernel.org/r/20230324-regmap-reg-read-write-page-v1-1-1fbc0dac67ae@kernel.org --- drivers/base/regmap/regmap.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c index a4e4367648bf..473b65b102db 100644 --- a/drivers/base/regmap/regmap.c +++ b/drivers/base/regmap/regmap.c @@ -1945,6 +1945,15 @@ static int _regmap_bus_reg_write(void *context, unsigned int reg, unsigned int val) { struct regmap *map = context; + struct regmap_range_node *range; + int ret; + + range = _regmap_range_lookup(map, reg); + if (range) { + ret = _regmap_select_page(map, ®, range, 1); + if (ret != 0) + return ret; + } reg = regmap_reg_addr(map, reg); return map->bus->reg_write(map->bus_context, reg, val); @@ -2842,6 +2851,15 @@ static int _regmap_bus_reg_read(void *context, unsigned int reg, unsigned int *val) { struct regmap *map = context; + struct regmap_range_node *range; + int ret; + + range = _regmap_range_lookup(map, reg); + if (range) { + ret = _regmap_select_page(map, ®, range, 1); + if (ret != 0) + return ret; + } reg = regmap_reg_addr(map, reg); return map->bus->reg_read(map->bus_context, reg, val); From 1e2bae6ae8f6b404b295edd5ba11a0bb1566544c Mon Sep 17 00:00:00 2001 From: Mark Brown Date: Fri, 24 Mar 2023 16:21:49 +0000 Subject: [PATCH 11/19] regmap: Removed compressed cache support The compressed register cache support has assumptions that make it hard to cover in testing, mainly that it requires raw registers defaults be provided. Rather than either address these assumptions or leave it untested by the forthcoming KUnit tests let's remove it, the use case is quite thin and there are no current users. Signed-off-by: Mark Brown Link: https://lore.kernel.org/r/20230324-regcache-lzo-v1-1-08c5d63e2a5e@kernel.org --- drivers/base/regmap/Kconfig | 5 - drivers/base/regmap/Makefile | 1 - drivers/base/regmap/internal.h | 1 - drivers/base/regmap/regcache-lzo.c | 368 ----------------------------- drivers/base/regmap/regcache.c | 3 - include/linux/regmap.h | 1 - 6 files changed, 379 deletions(-) delete mode 100644 drivers/base/regmap/regcache-lzo.c diff --git a/drivers/base/regmap/Kconfig b/drivers/base/regmap/Kconfig index cd4bb642b9de..9f8dcb32c24b 100644 --- a/drivers/base/regmap/Kconfig +++ b/drivers/base/regmap/Kconfig @@ -9,11 +9,6 @@ config REGMAP select MDIO_BUS if REGMAP_MDIO bool -config REGCACHE_COMPRESSED - select LZO_COMPRESS - select LZO_DECOMPRESS - bool - config REGMAP_AC97 tristate diff --git a/drivers/base/regmap/Makefile b/drivers/base/regmap/Makefile index 6990de7ca9a9..8900c340e352 100644 --- a/drivers/base/regmap/Makefile +++ b/drivers/base/regmap/Makefile @@ -4,7 +4,6 @@ CFLAGS_regmap.o := -I$(src) obj-$(CONFIG_REGMAP) += regmap.o regcache.o obj-$(CONFIG_REGMAP) += regcache-rbtree.o regcache-flat.o -obj-$(CONFIG_REGCACHE_COMPRESSED) += regcache-lzo.o obj-$(CONFIG_DEBUG_FS) += regmap-debugfs.o obj-$(CONFIG_REGMAP_AC97) += regmap-ac97.o obj-$(CONFIG_REGMAP_I2C) += regmap-i2c.o diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h index da8996e7a1f1..919d790a01b7 100644 --- a/drivers/base/regmap/internal.h +++ b/drivers/base/regmap/internal.h @@ -281,7 +281,6 @@ enum regmap_endian regmap_get_val_endian(struct device *dev, const struct regmap_config *config); extern struct regcache_ops regcache_rbtree_ops; -extern struct regcache_ops regcache_lzo_ops; extern struct regcache_ops regcache_flat_ops; static inline const char *regmap_name(const struct regmap *map) diff --git a/drivers/base/regmap/regcache-lzo.c b/drivers/base/regmap/regcache-lzo.c deleted file mode 100644 index 7886303eb026..000000000000 --- a/drivers/base/regmap/regcache-lzo.c +++ /dev/null @@ -1,368 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0 -// -// Register cache access API - LZO caching support -// -// Copyright 2011 Wolfson Microelectronics plc -// -// Author: Dimitris Papastamos - -#include -#include -#include - -#include "internal.h" - -static int regcache_lzo_exit(struct regmap *map); - -struct regcache_lzo_ctx { - void *wmem; - void *dst; - const void *src; - size_t src_len; - size_t dst_len; - size_t decompressed_size; - unsigned long *sync_bmp; - int sync_bmp_nbits; -}; - -#define LZO_BLOCK_NUM 8 -static int regcache_lzo_block_count(struct regmap *map) -{ - return LZO_BLOCK_NUM; -} - -static int regcache_lzo_prepare(struct regcache_lzo_ctx *lzo_ctx) -{ - lzo_ctx->wmem = kmalloc(LZO1X_MEM_COMPRESS, GFP_KERNEL); - if (!lzo_ctx->wmem) - return -ENOMEM; - return 0; -} - -static int regcache_lzo_compress(struct regcache_lzo_ctx *lzo_ctx) -{ - size_t compress_size; - int ret; - - ret = lzo1x_1_compress(lzo_ctx->src, lzo_ctx->src_len, - lzo_ctx->dst, &compress_size, lzo_ctx->wmem); - if (ret != LZO_E_OK || compress_size > lzo_ctx->dst_len) - return -EINVAL; - lzo_ctx->dst_len = compress_size; - return 0; -} - -static int regcache_lzo_decompress(struct regcache_lzo_ctx *lzo_ctx) -{ - size_t dst_len; - int ret; - - dst_len = lzo_ctx->dst_len; - ret = lzo1x_decompress_safe(lzo_ctx->src, lzo_ctx->src_len, - lzo_ctx->dst, &dst_len); - if (ret != LZO_E_OK || dst_len != lzo_ctx->dst_len) - return -EINVAL; - return 0; -} - -static int regcache_lzo_compress_cache_block(struct regmap *map, - struct regcache_lzo_ctx *lzo_ctx) -{ - int ret; - - lzo_ctx->dst_len = lzo1x_worst_compress(PAGE_SIZE); - lzo_ctx->dst = kmalloc(lzo_ctx->dst_len, GFP_KERNEL); - if (!lzo_ctx->dst) { - lzo_ctx->dst_len = 0; - return -ENOMEM; - } - - ret = regcache_lzo_compress(lzo_ctx); - if (ret < 0) - return ret; - return 0; -} - -static int regcache_lzo_decompress_cache_block(struct regmap *map, - struct regcache_lzo_ctx *lzo_ctx) -{ - int ret; - - lzo_ctx->dst_len = lzo_ctx->decompressed_size; - lzo_ctx->dst = kmalloc(lzo_ctx->dst_len, GFP_KERNEL); - if (!lzo_ctx->dst) { - lzo_ctx->dst_len = 0; - return -ENOMEM; - } - - ret = regcache_lzo_decompress(lzo_ctx); - if (ret < 0) - return ret; - return 0; -} - -static inline int regcache_lzo_get_blkindex(struct regmap *map, - unsigned int reg) -{ - return ((reg / map->reg_stride) * map->cache_word_size) / - DIV_ROUND_UP(map->cache_size_raw, - regcache_lzo_block_count(map)); -} - -static inline int regcache_lzo_get_blkpos(struct regmap *map, - unsigned int reg) -{ - return (reg / map->reg_stride) % - (DIV_ROUND_UP(map->cache_size_raw, - regcache_lzo_block_count(map)) / - map->cache_word_size); -} - -static inline int regcache_lzo_get_blksize(struct regmap *map) -{ - return DIV_ROUND_UP(map->cache_size_raw, - regcache_lzo_block_count(map)); -} - -static int regcache_lzo_init(struct regmap *map) -{ - struct regcache_lzo_ctx **lzo_blocks; - size_t bmp_size; - int ret, i, blksize, blkcount; - const char *p, *end; - unsigned long *sync_bmp; - - ret = 0; - - blkcount = regcache_lzo_block_count(map); - map->cache = kcalloc(blkcount, sizeof(*lzo_blocks), - GFP_KERNEL); - if (!map->cache) - return -ENOMEM; - lzo_blocks = map->cache; - - /* - * allocate a bitmap to be used when syncing the cache with - * the hardware. Each time a register is modified, the corresponding - * bit is set in the bitmap, so we know that we have to sync - * that register. - */ - bmp_size = map->num_reg_defaults_raw; - sync_bmp = bitmap_zalloc(bmp_size, GFP_KERNEL); - if (!sync_bmp) { - ret = -ENOMEM; - goto err; - } - - /* allocate the lzo blocks and initialize them */ - for (i = 0; i < blkcount; i++) { - lzo_blocks[i] = kzalloc(sizeof **lzo_blocks, - GFP_KERNEL); - if (!lzo_blocks[i]) { - bitmap_free(sync_bmp); - ret = -ENOMEM; - goto err; - } - lzo_blocks[i]->sync_bmp = sync_bmp; - lzo_blocks[i]->sync_bmp_nbits = bmp_size; - /* alloc the working space for the compressed block */ - ret = regcache_lzo_prepare(lzo_blocks[i]); - if (ret < 0) - goto err; - } - - blksize = regcache_lzo_get_blksize(map); - p = map->reg_defaults_raw; - end = map->reg_defaults_raw + map->cache_size_raw; - /* compress the register map and fill the lzo blocks */ - for (i = 0; i < blkcount; i++, p += blksize) { - lzo_blocks[i]->src = p; - if (p + blksize > end) - lzo_blocks[i]->src_len = end - p; - else - lzo_blocks[i]->src_len = blksize; - ret = regcache_lzo_compress_cache_block(map, - lzo_blocks[i]); - if (ret < 0) - goto err; - lzo_blocks[i]->decompressed_size = - lzo_blocks[i]->src_len; - } - - return 0; -err: - regcache_lzo_exit(map); - return ret; -} - -static int regcache_lzo_exit(struct regmap *map) -{ - struct regcache_lzo_ctx **lzo_blocks; - int i, blkcount; - - lzo_blocks = map->cache; - if (!lzo_blocks) - return 0; - - blkcount = regcache_lzo_block_count(map); - /* - * the pointer to the bitmap used for syncing the cache - * is shared amongst all lzo_blocks. Ensure it is freed - * only once. - */ - if (lzo_blocks[0]) - bitmap_free(lzo_blocks[0]->sync_bmp); - for (i = 0; i < blkcount; i++) { - if (lzo_blocks[i]) { - kfree(lzo_blocks[i]->wmem); - kfree(lzo_blocks[i]->dst); - } - /* each lzo_block is a pointer returned by kmalloc or NULL */ - kfree(lzo_blocks[i]); - } - kfree(lzo_blocks); - map->cache = NULL; - return 0; -} - -static int regcache_lzo_read(struct regmap *map, - unsigned int reg, unsigned int *value) -{ - struct regcache_lzo_ctx *lzo_block, **lzo_blocks; - int ret, blkindex, blkpos; - size_t tmp_dst_len; - void *tmp_dst; - - /* index of the compressed lzo block */ - blkindex = regcache_lzo_get_blkindex(map, reg); - /* register index within the decompressed block */ - blkpos = regcache_lzo_get_blkpos(map, reg); - lzo_blocks = map->cache; - lzo_block = lzo_blocks[blkindex]; - - /* save the pointer and length of the compressed block */ - tmp_dst = lzo_block->dst; - tmp_dst_len = lzo_block->dst_len; - - /* prepare the source to be the compressed block */ - lzo_block->src = lzo_block->dst; - lzo_block->src_len = lzo_block->dst_len; - - /* decompress the block */ - ret = regcache_lzo_decompress_cache_block(map, lzo_block); - if (ret >= 0) - /* fetch the value from the cache */ - *value = regcache_get_val(map, lzo_block->dst, blkpos); - - kfree(lzo_block->dst); - /* restore the pointer and length of the compressed block */ - lzo_block->dst = tmp_dst; - lzo_block->dst_len = tmp_dst_len; - - return ret; -} - -static int regcache_lzo_write(struct regmap *map, - unsigned int reg, unsigned int value) -{ - struct regcache_lzo_ctx *lzo_block, **lzo_blocks; - int ret, blkindex, blkpos; - size_t tmp_dst_len; - void *tmp_dst; - - /* index of the compressed lzo block */ - blkindex = regcache_lzo_get_blkindex(map, reg); - /* register index within the decompressed block */ - blkpos = regcache_lzo_get_blkpos(map, reg); - lzo_blocks = map->cache; - lzo_block = lzo_blocks[blkindex]; - - /* save the pointer and length of the compressed block */ - tmp_dst = lzo_block->dst; - tmp_dst_len = lzo_block->dst_len; - - /* prepare the source to be the compressed block */ - lzo_block->src = lzo_block->dst; - lzo_block->src_len = lzo_block->dst_len; - - /* decompress the block */ - ret = regcache_lzo_decompress_cache_block(map, lzo_block); - if (ret < 0) { - kfree(lzo_block->dst); - goto out; - } - - /* write the new value to the cache */ - if (regcache_set_val(map, lzo_block->dst, blkpos, value)) { - kfree(lzo_block->dst); - goto out; - } - - /* prepare the source to be the decompressed block */ - lzo_block->src = lzo_block->dst; - lzo_block->src_len = lzo_block->dst_len; - - /* compress the block */ - ret = regcache_lzo_compress_cache_block(map, lzo_block); - if (ret < 0) { - kfree(lzo_block->dst); - kfree(lzo_block->src); - goto out; - } - - /* set the bit so we know we have to sync this register */ - set_bit(reg / map->reg_stride, lzo_block->sync_bmp); - kfree(tmp_dst); - kfree(lzo_block->src); - return 0; -out: - lzo_block->dst = tmp_dst; - lzo_block->dst_len = tmp_dst_len; - return ret; -} - -static int regcache_lzo_sync(struct regmap *map, unsigned int min, - unsigned int max) -{ - struct regcache_lzo_ctx **lzo_blocks; - unsigned int val; - int i; - int ret; - - lzo_blocks = map->cache; - i = min; - for_each_set_bit_from(i, lzo_blocks[0]->sync_bmp, - lzo_blocks[0]->sync_bmp_nbits) { - if (i > max) - continue; - - ret = regcache_read(map, i, &val); - if (ret) - return ret; - - /* Is this the hardware default? If so skip. */ - ret = regcache_lookup_reg(map, i); - if (ret > 0 && val == map->reg_defaults[ret].def) - continue; - - map->cache_bypass = true; - ret = _regmap_write(map, i, val); - map->cache_bypass = false; - if (ret) - return ret; - dev_dbg(map->dev, "Synced register %#x, value %#x\n", - i, val); - } - - return 0; -} - -struct regcache_ops regcache_lzo_ops = { - .type = REGCACHE_COMPRESSED, - .name = "lzo", - .init = regcache_lzo_init, - .exit = regcache_lzo_exit, - .read = regcache_lzo_read, - .write = regcache_lzo_write, - .sync = regcache_lzo_sync -}; diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c index 7a337e8d08a9..3f19f68c6a6c 100644 --- a/drivers/base/regmap/regcache.c +++ b/drivers/base/regmap/regcache.c @@ -17,9 +17,6 @@ static const struct regcache_ops *cache_types[] = { ®cache_rbtree_ops, -#if IS_ENABLED(CONFIG_REGCACHE_COMPRESSED) - ®cache_lzo_ops, -#endif ®cache_flat_ops, }; diff --git a/include/linux/regmap.h b/include/linux/regmap.h index 19d54f019cc4..b63204d51b35 100644 --- a/include/linux/regmap.h +++ b/include/linux/regmap.h @@ -50,7 +50,6 @@ struct sdw_slave; enum regcache_type { REGCACHE_NONE, REGCACHE_RBTREE, - REGCACHE_COMPRESSED, REGCACHE_FLAT, }; From f6352424e37e7bf72291ceab87dc620172be0999 Mon Sep 17 00:00:00 2001 From: Mark Brown Date: Sun, 26 Mar 2023 15:11:12 +0100 Subject: [PATCH 12/19] regmap: Add RAM backed register map Add a register map that is a simple array of memory, for use in KUnit testing of the framework. This is not exposed in regmap.h since I can't think of a non-test use case, it is purely for use internally. To facilitate testing we track if registers have been read or written to. Signed-off-by: Mark Brown Link: https://lore.kernel.org/r/20230324-regmap-kunit-v2-1-b208801dc2c8@kernel.org --- drivers/base/regmap/Kconfig | 3 ++ drivers/base/regmap/Makefile | 1 + drivers/base/regmap/internal.h | 19 +++++++ drivers/base/regmap/regmap-ram.c | 85 ++++++++++++++++++++++++++++++++ 4 files changed, 108 insertions(+) create mode 100644 drivers/base/regmap/regmap-ram.c diff --git a/drivers/base/regmap/Kconfig b/drivers/base/regmap/Kconfig index 9f8dcb32c24b..3d3445bff511 100644 --- a/drivers/base/regmap/Kconfig +++ b/drivers/base/regmap/Kconfig @@ -41,6 +41,9 @@ config REGMAP_MMIO config REGMAP_IRQ bool +config REGMAP_RAM + tristate + config REGMAP_SOUNDWIRE tristate depends on SOUNDWIRE diff --git a/drivers/base/regmap/Makefile b/drivers/base/regmap/Makefile index 8900c340e352..b4b0e8fe6c1f 100644 --- a/drivers/base/regmap/Makefile +++ b/drivers/base/regmap/Makefile @@ -7,6 +7,7 @@ obj-$(CONFIG_REGMAP) += regcache-rbtree.o regcache-flat.o obj-$(CONFIG_DEBUG_FS) += regmap-debugfs.o obj-$(CONFIG_REGMAP_AC97) += regmap-ac97.o obj-$(CONFIG_REGMAP_I2C) += regmap-i2c.o +obj-$(CONFIG_REGMAP_RAM) += regmap-ram.o obj-$(CONFIG_REGMAP_SLIMBUS) += regmap-slimbus.o obj-$(CONFIG_REGMAP_SPI) += regmap-spi.o obj-$(CONFIG_REGMAP_SPMI) += regmap-spmi.o diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h index 919d790a01b7..10aca7119d33 100644 --- a/drivers/base/regmap/internal.h +++ b/drivers/base/regmap/internal.h @@ -306,4 +306,23 @@ static inline unsigned int regcache_get_index_by_order(const struct regmap *map, return reg >> map->reg_stride_order; } +struct regmap_ram_data { + unsigned int *vals; /* Allocatd by caller */ + bool *read; + bool *written; +}; + +/* + * Create a test register map with data stored in RAM, not intended + * for practical use. + */ +struct regmap *__regmap_init_ram(const struct regmap_config *config, + struct regmap_ram_data *data, + struct lock_class_key *lock_key, + const char *lock_name); + +#define regmap_init_ram(config, data) \ + __regmap_lockdep_wrapper(__regmap_init_ram, #config, config, data) + + #endif diff --git a/drivers/base/regmap/regmap-ram.c b/drivers/base/regmap/regmap-ram.c new file mode 100644 index 000000000000..85f34a5dee04 --- /dev/null +++ b/drivers/base/regmap/regmap-ram.c @@ -0,0 +1,85 @@ +// SPDX-License-Identifier: GPL-2.0 +// +// Register map access API - Memory region +// +// This is intended for testing only +// +// Copyright (c) 2023, Arm Ltd + +#include +#include +#include +#include +#include +#include +#include + +#include "internal.h" + +static int regmap_ram_write(void *context, unsigned int reg, unsigned int val) +{ + struct regmap_ram_data *data = context; + + data->vals[reg] = val; + data->written[reg] = true; + + return 0; +} + +static int regmap_ram_read(void *context, unsigned int reg, unsigned int *val) +{ + struct regmap_ram_data *data = context; + + *val = data->vals[reg]; + data->read[reg] = true; + + return 0; +} + +static void regmap_ram_free_context(void *context) +{ + struct regmap_ram_data *data = context; + + kfree(data->vals); + kfree(data->read); + kfree(data->written); + kfree(data); +} + +static const struct regmap_bus regmap_ram = { + .fast_io = true, + .reg_write = regmap_ram_write, + .reg_read = regmap_ram_read, + .free_context = regmap_ram_free_context, +}; + +struct regmap *__regmap_init_ram(const struct regmap_config *config, + struct regmap_ram_data *data, + struct lock_class_key *lock_key, + const char *lock_name) +{ + struct regmap *map; + + if (!config->max_register) { + pr_crit("No max_register specified for RAM regmap\n"); + return ERR_PTR(-EINVAL); + } + + data->read = kcalloc(sizeof(bool), config->max_register + 1, + GFP_KERNEL); + if (!data->read) + return ERR_PTR(-ENOMEM); + + data->written = kcalloc(sizeof(bool), config->max_register + 1, + GFP_KERNEL); + if (!data->written) + return ERR_PTR(-ENOMEM); + + map = __regmap_init(NULL, ®map_ram, data, config, + lock_key, lock_name); + + return map; +} +EXPORT_SYMBOL_GPL(__regmap_init_ram); + +MODULE_LICENSE("GPL v2"); From 2238959b6ad27040275439edd6893e309bc729a3 Mon Sep 17 00:00:00 2001 From: Mark Brown Date: Sun, 26 Mar 2023 15:11:13 +0100 Subject: [PATCH 13/19] regmap: Add some basic kunit tests On the theory that it's better to make a start let's add some KUnit tests for regmap. Currently this is a bit of a mess but it passes and hopefully will at some point help catch problems. We provide very basic cover for most of the core functionality that operates at the register level, repeating each test for each cache type in order to exercise the caches. There is no coverage of anything to do with the bulk operations at the bus level or formatting for byte stream buses yet. Each test creates it's own regmap since the cache structures are built incrementally, meaning we gain coverage from the different access patterns, and some of the tests cover different init scenarios. Signed-off-by: Mark Brown Link: https://lore.kernel.org/r/20230324-regmap-kunit-v2-2-b208801dc2c8@kernel.org --- drivers/base/regmap/Kconfig | 7 + drivers/base/regmap/Makefile | 1 + drivers/base/regmap/regmap-kunit.c | 736 +++++++++++++++++++++++++++++ 3 files changed, 744 insertions(+) create mode 100644 drivers/base/regmap/regmap-kunit.c diff --git a/drivers/base/regmap/Kconfig b/drivers/base/regmap/Kconfig index 3d3445bff511..33a8366e22a5 100644 --- a/drivers/base/regmap/Kconfig +++ b/drivers/base/regmap/Kconfig @@ -9,6 +9,13 @@ config REGMAP select MDIO_BUS if REGMAP_MDIO bool +config REGMAP_KUNIT + tristate "KUnit tests for regmap" + depends on KUNIT + default KUNIT_ALL_TESTS + select REGMAP + select REGMAP_RAM + config REGMAP_AC97 tristate diff --git a/drivers/base/regmap/Makefile b/drivers/base/regmap/Makefile index b4b0e8fe6c1f..4cb73468a197 100644 --- a/drivers/base/regmap/Makefile +++ b/drivers/base/regmap/Makefile @@ -5,6 +5,7 @@ CFLAGS_regmap.o := -I$(src) obj-$(CONFIG_REGMAP) += regmap.o regcache.o obj-$(CONFIG_REGMAP) += regcache-rbtree.o regcache-flat.o obj-$(CONFIG_DEBUG_FS) += regmap-debugfs.o +obj-$(CONFIG_REGMAP_KUNIT) += regmap-kunit.o obj-$(CONFIG_REGMAP_AC97) += regmap-ac97.o obj-$(CONFIG_REGMAP_I2C) += regmap-i2c.o obj-$(CONFIG_REGMAP_RAM) += regmap-ram.o diff --git a/drivers/base/regmap/regmap-kunit.c b/drivers/base/regmap/regmap-kunit.c new file mode 100644 index 000000000000..c78f45cf9a8d --- /dev/null +++ b/drivers/base/regmap/regmap-kunit.c @@ -0,0 +1,736 @@ +// SPDX-License-Identifier: GPL-2.0 +// +// regmap KUnit tests +// +// Copyright 2023 Arm Ltd + +#include +#include "internal.h" + +#define BLOCK_TEST_SIZE 12 + +static const struct regmap_config test_regmap_config = { + .max_register = BLOCK_TEST_SIZE, + .reg_stride = 1, + .val_bits = sizeof(unsigned int) * 8, +}; + +struct regcache_types { + enum regcache_type type; + const char *name; +}; + +static void case_to_desc(const struct regcache_types *t, char *desc) +{ + strcpy(desc, t->name); +} + +static const struct regcache_types regcache_types_list[] = { + { REGCACHE_NONE, "none" }, + { REGCACHE_FLAT, "flat" }, + { REGCACHE_RBTREE, "rbtree" }, +}; + +KUNIT_ARRAY_PARAM(regcache_types, regcache_types_list, case_to_desc); + +static const struct regcache_types real_cache_types_list[] = { + { REGCACHE_FLAT, "flat" }, + { REGCACHE_RBTREE, "rbtree" }, +}; + +KUNIT_ARRAY_PARAM(real_cache_types, real_cache_types_list, case_to_desc); + +static const struct regcache_types sparse_cache_types_list[] = { + { REGCACHE_RBTREE, "rbtree" }, +}; + +KUNIT_ARRAY_PARAM(sparse_cache_types, sparse_cache_types_list, case_to_desc); + +static struct regmap *gen_regmap(struct regmap_config *config, + struct regmap_ram_data **data) +{ + unsigned int *buf; + struct regmap *ret; + size_t size = (config->max_register + 1) * sizeof(unsigned int); + int i; + struct reg_default *defaults; + + buf = kmalloc(size, GFP_KERNEL); + if (!buf) + return ERR_PTR(-ENOMEM); + + get_random_bytes(buf, size); + + *data = kzalloc(sizeof(**data), GFP_KERNEL); + if (!(*data)) + return ERR_PTR(-ENOMEM); + (*data)->vals = buf; + + if (config->num_reg_defaults) { + defaults = kcalloc(config->num_reg_defaults, + sizeof(struct reg_default), + GFP_KERNEL); + if (!defaults) + return ERR_PTR(-ENOMEM); + config->reg_defaults = defaults; + + for (i = 0; i < config->num_reg_defaults; i++) { + defaults[i].reg = i * config->reg_stride; + defaults[i].def = buf[i * config->reg_stride]; + } + } + + ret = regmap_init_ram(config, *data); + if (IS_ERR(ret)) { + kfree(buf); + kfree(*data); + } + + return ret; +} + +static void basic_read_write(struct kunit *test) +{ + struct regcache_types *t = (struct regcache_types *)test->param_value; + struct regmap *map; + struct regmap_config config; + struct regmap_ram_data *data; + unsigned int val, rval; + + config = test_regmap_config; + config.cache_type = t->type; + + map = gen_regmap(&config, &data); + KUNIT_ASSERT_FALSE(test, IS_ERR(map)); + if (IS_ERR(map)) + return; + + get_random_bytes(&val, sizeof(val)); + + /* If we write a value to a register we can read it back */ + KUNIT_EXPECT_EQ(test, 0, regmap_write(map, 0, val)); + KUNIT_EXPECT_EQ(test, 0, regmap_read(map, 0, &rval)); + KUNIT_EXPECT_EQ(test, val, rval); + + /* If using a cache the cache satisfied the read */ + KUNIT_EXPECT_EQ(test, t->type == REGCACHE_NONE, data->read[0]); + + regmap_exit(map); +} + +static void bulk_write(struct kunit *test) +{ + struct regcache_types *t = (struct regcache_types *)test->param_value; + struct regmap *map; + struct regmap_config config; + struct regmap_ram_data *data; + unsigned int val[BLOCK_TEST_SIZE], rval[BLOCK_TEST_SIZE]; + int i; + + config = test_regmap_config; + config.cache_type = t->type; + + map = gen_regmap(&config, &data); + KUNIT_ASSERT_FALSE(test, IS_ERR(map)); + if (IS_ERR(map)) + return; + + get_random_bytes(&val, sizeof(val)); + + /* + * Data written via the bulk API can be read back with single + * reads. + */ + KUNIT_EXPECT_EQ(test, 0, regmap_bulk_write(map, 0, val, + BLOCK_TEST_SIZE)); + for (i = 0; i < BLOCK_TEST_SIZE; i++) + KUNIT_EXPECT_EQ(test, 0, regmap_read(map, i, &rval[i])); + + KUNIT_EXPECT_MEMEQ(test, val, rval, sizeof(val)); + + /* If using a cache the cache satisfied the read */ + for (i = 0; i < BLOCK_TEST_SIZE; i++) + KUNIT_EXPECT_EQ(test, t->type == REGCACHE_NONE, data->read[i]); + + regmap_exit(map); +} + +static void bulk_read(struct kunit *test) +{ + struct regcache_types *t = (struct regcache_types *)test->param_value; + struct regmap *map; + struct regmap_config config; + struct regmap_ram_data *data; + unsigned int val[BLOCK_TEST_SIZE], rval[BLOCK_TEST_SIZE]; + int i; + + config = test_regmap_config; + config.cache_type = t->type; + + map = gen_regmap(&config, &data); + KUNIT_ASSERT_FALSE(test, IS_ERR(map)); + if (IS_ERR(map)) + return; + + get_random_bytes(&val, sizeof(val)); + + /* Data written as single writes can be read via the bulk API */ + for (i = 0; i < BLOCK_TEST_SIZE; i++) + KUNIT_EXPECT_EQ(test, 0, regmap_write(map, i, val[i])); + KUNIT_EXPECT_EQ(test, 0, regmap_bulk_read(map, 0, rval, + BLOCK_TEST_SIZE)); + KUNIT_EXPECT_MEMEQ(test, val, rval, sizeof(val)); + + /* If using a cache the cache satisfied the read */ + for (i = 0; i < BLOCK_TEST_SIZE; i++) + KUNIT_EXPECT_EQ(test, t->type == REGCACHE_NONE, data->read[i]); + + regmap_exit(map); +} + +static void reg_defaults(struct kunit *test) +{ + struct regcache_types *t = (struct regcache_types *)test->param_value; + struct regmap *map; + struct regmap_config config; + struct regmap_ram_data *data; + unsigned int rval[BLOCK_TEST_SIZE]; + int i; + + config = test_regmap_config; + config.cache_type = t->type; + config.num_reg_defaults = BLOCK_TEST_SIZE; + + map = gen_regmap(&config, &data); + KUNIT_ASSERT_FALSE(test, IS_ERR(map)); + if (IS_ERR(map)) + return; + + /* Read back the expected default data */ + KUNIT_EXPECT_EQ(test, 0, regmap_bulk_read(map, 0, rval, + BLOCK_TEST_SIZE)); + KUNIT_EXPECT_MEMEQ(test, data->vals, rval, sizeof(rval)); + + /* The data should have been read from cache if there was one */ + for (i = 0; i < BLOCK_TEST_SIZE; i++) + KUNIT_EXPECT_EQ(test, t->type == REGCACHE_NONE, data->read[i]); +} + +static void reg_defaults_read_dev(struct kunit *test) +{ + struct regcache_types *t = (struct regcache_types *)test->param_value; + struct regmap *map; + struct regmap_config config; + struct regmap_ram_data *data; + unsigned int rval[BLOCK_TEST_SIZE]; + int i; + + config = test_regmap_config; + config.cache_type = t->type; + config.num_reg_defaults_raw = BLOCK_TEST_SIZE; + + map = gen_regmap(&config, &data); + KUNIT_ASSERT_FALSE(test, IS_ERR(map)); + if (IS_ERR(map)) + return; + + /* We should have read the cache defaults back from the map */ + for (i = 0; i < BLOCK_TEST_SIZE; i++) { + KUNIT_EXPECT_EQ(test, t->type != REGCACHE_NONE, data->read[i]); + data->read[i] = false; + } + + /* Read back the expected default data */ + KUNIT_EXPECT_EQ(test, 0, regmap_bulk_read(map, 0, rval, + BLOCK_TEST_SIZE)); + KUNIT_EXPECT_MEMEQ(test, data->vals, rval, sizeof(rval)); + + /* The data should have been read from cache if there was one */ + for (i = 0; i < BLOCK_TEST_SIZE; i++) + KUNIT_EXPECT_EQ(test, t->type == REGCACHE_NONE, data->read[i]); +} + +static void register_patch(struct kunit *test) +{ + struct regcache_types *t = (struct regcache_types *)test->param_value; + struct regmap *map; + struct regmap_config config; + struct regmap_ram_data *data; + struct reg_sequence patch[2]; + unsigned int rval[BLOCK_TEST_SIZE]; + int i; + + /* We need defaults so readback works */ + config = test_regmap_config; + config.cache_type = t->type; + config.num_reg_defaults = BLOCK_TEST_SIZE; + + map = gen_regmap(&config, &data); + KUNIT_ASSERT_FALSE(test, IS_ERR(map)); + if (IS_ERR(map)) + return; + + /* Stash the original values */ + KUNIT_EXPECT_EQ(test, 0, regmap_bulk_read(map, 0, rval, + BLOCK_TEST_SIZE)); + + /* Patch a couple of values */ + patch[0].reg = 2; + patch[0].def = rval[2] + 1; + patch[0].delay_us = 0; + patch[1].reg = 5; + patch[1].def = rval[5] + 1; + patch[1].delay_us = 0; + KUNIT_EXPECT_EQ(test, 0, regmap_register_patch(map, patch, + ARRAY_SIZE(patch))); + + /* Only the patched registers are written */ + for (i = 0; i < BLOCK_TEST_SIZE; i++) { + switch (i) { + case 2: + case 5: + KUNIT_EXPECT_TRUE(test, data->written[i]); + KUNIT_EXPECT_EQ(test, data->vals[i], rval[i] + 1); + break; + default: + KUNIT_EXPECT_FALSE(test, data->written[i]); + KUNIT_EXPECT_EQ(test, data->vals[i], rval[i]); + break; + } + } + + regmap_exit(map); +} + +static void stride(struct kunit *test) +{ + struct regcache_types *t = (struct regcache_types *)test->param_value; + struct regmap *map; + struct regmap_config config; + struct regmap_ram_data *data; + unsigned int rval; + int i; + + config = test_regmap_config; + config.cache_type = t->type; + config.reg_stride = 2; + config.num_reg_defaults = BLOCK_TEST_SIZE / 2; + + map = gen_regmap(&config, &data); + KUNIT_ASSERT_FALSE(test, IS_ERR(map)); + if (IS_ERR(map)) + return; + + /* Only even registers can be accessed, try both read and write */ + for (i = 0; i < BLOCK_TEST_SIZE; i++) { + data->read[i] = false; + data->written[i] = false; + + if (i % 2) { + KUNIT_EXPECT_NE(test, 0, regmap_read(map, i, &rval)); + KUNIT_EXPECT_NE(test, 0, regmap_write(map, i, rval)); + KUNIT_EXPECT_FALSE(test, data->read[i]); + KUNIT_EXPECT_FALSE(test, data->written[i]); + } else { + KUNIT_EXPECT_EQ(test, 0, regmap_read(map, i, &rval)); + KUNIT_EXPECT_EQ(test, data->vals[i], rval); + KUNIT_EXPECT_EQ(test, t->type == REGCACHE_NONE, + data->read[i]); + + KUNIT_EXPECT_EQ(test, 0, regmap_write(map, i, rval)); + KUNIT_EXPECT_TRUE(test, data->written[i]); + } + } + + regmap_exit(map); +} + +static struct regmap_range_cfg test_range = { + .selector_reg = 1, + .selector_mask = 0xff, + + .window_start = 4, + .window_len = 10, + + .range_min = 20, + .range_max = 40, +}; + +static bool test_range_volatile(struct device *dev, unsigned int reg) +{ + if (reg >= test_range.window_start && + reg <= test_range.selector_reg + test_range.window_len) + return true; + + if (reg >= test_range.range_min && reg <= test_range.range_max) + return true; + + return false; +} + +static void basic_ranges(struct kunit *test) +{ + struct regcache_types *t = (struct regcache_types *)test->param_value; + struct regmap *map; + struct regmap_config config; + struct regmap_ram_data *data; + unsigned int val; + int i; + + config = test_regmap_config; + config.cache_type = t->type; + config.volatile_reg = test_range_volatile; + config.ranges = &test_range; + config.num_ranges = 1; + config.max_register = test_range.range_max; + + map = gen_regmap(&config, &data); + KUNIT_ASSERT_FALSE(test, IS_ERR(map)); + if (IS_ERR(map)) + return; + + for (i = test_range.range_min; i < test_range.range_max; i++) { + data->read[i] = false; + data->written[i] = false; + } + + /* Reset the page to a non-zero value to trigger a change */ + KUNIT_EXPECT_EQ(test, 0, regmap_write(map, test_range.selector_reg, + test_range.range_max)); + + /* Check we set the page and use the window for writes */ + data->written[test_range.selector_reg] = false; + data->written[test_range.window_start] = false; + KUNIT_EXPECT_EQ(test, 0, regmap_write(map, test_range.range_min, 0)); + KUNIT_EXPECT_TRUE(test, data->written[test_range.selector_reg]); + KUNIT_EXPECT_TRUE(test, data->written[test_range.window_start]); + + data->written[test_range.selector_reg] = false; + data->written[test_range.window_start] = false; + KUNIT_EXPECT_EQ(test, 0, regmap_write(map, + test_range.range_min + + test_range.window_len, + 0)); + KUNIT_EXPECT_TRUE(test, data->written[test_range.selector_reg]); + KUNIT_EXPECT_TRUE(test, data->written[test_range.window_start]); + + /* Same for reads */ + data->written[test_range.selector_reg] = false; + data->read[test_range.window_start] = false; + KUNIT_EXPECT_EQ(test, 0, regmap_read(map, test_range.range_min, &val)); + KUNIT_EXPECT_TRUE(test, data->written[test_range.selector_reg]); + KUNIT_EXPECT_TRUE(test, data->read[test_range.window_start]); + + data->written[test_range.selector_reg] = false; + data->read[test_range.window_start] = false; + KUNIT_EXPECT_EQ(test, 0, regmap_read(map, + test_range.range_min + + test_range.window_len, + &val)); + KUNIT_EXPECT_TRUE(test, data->written[test_range.selector_reg]); + KUNIT_EXPECT_TRUE(test, data->read[test_range.window_start]); + + /* No physical access triggered in the virtual range */ + for (i = test_range.range_min; i < test_range.range_max; i++) { + KUNIT_EXPECT_FALSE(test, data->read[i]); + KUNIT_EXPECT_FALSE(test, data->written[i]); + } + + regmap_exit(map); +} + +/* Try to stress dynamic creation of cache data structures */ +static void stress_insert(struct kunit *test) +{ + struct regcache_types *t = (struct regcache_types *)test->param_value; + struct regmap *map; + struct regmap_config config; + struct regmap_ram_data *data; + unsigned int rval, *vals; + size_t buf_sz; + int i; + + config = test_regmap_config; + config.cache_type = t->type; + config.max_register = 300; + + map = gen_regmap(&config, &data); + KUNIT_ASSERT_FALSE(test, IS_ERR(map)); + if (IS_ERR(map)) + return; + + vals = kunit_kcalloc(test, sizeof(unsigned long), config.max_register, + GFP_KERNEL); + KUNIT_ASSERT_FALSE(test, vals == NULL); + buf_sz = sizeof(unsigned long) * config.max_register; + + get_random_bytes(vals, buf_sz); + + /* Write data into the map/cache in ever decreasing strides */ + for (i = 0; i < config.max_register; i += 100) + KUNIT_EXPECT_EQ(test, 0, regmap_write(map, i, vals[i])); + for (i = 0; i < config.max_register; i += 50) + KUNIT_EXPECT_EQ(test, 0, regmap_write(map, i, vals[i])); + for (i = 0; i < config.max_register; i += 25) + KUNIT_EXPECT_EQ(test, 0, regmap_write(map, i, vals[i])); + for (i = 0; i < config.max_register; i += 10) + KUNIT_EXPECT_EQ(test, 0, regmap_write(map, i, vals[i])); + for (i = 0; i < config.max_register; i += 5) + KUNIT_EXPECT_EQ(test, 0, regmap_write(map, i, vals[i])); + for (i = 0; i < config.max_register; i += 3) + KUNIT_EXPECT_EQ(test, 0, regmap_write(map, i, vals[i])); + for (i = 0; i < config.max_register; i += 2) + KUNIT_EXPECT_EQ(test, 0, regmap_write(map, i, vals[i])); + for (i = 0; i < config.max_register; i++) + KUNIT_EXPECT_EQ(test, 0, regmap_write(map, i, vals[i])); + + /* Do reads from the cache (if there is one) match? */ + for (i = 0; i < config.max_register; i ++) { + KUNIT_EXPECT_EQ(test, 0, regmap_read(map, i, &rval)); + KUNIT_EXPECT_EQ(test, rval, vals[i]); + KUNIT_EXPECT_EQ(test, t->type == REGCACHE_NONE, data->read[i]); + } + + regmap_exit(map); +} + +static void cache_bypass(struct kunit *test) +{ + struct regcache_types *t = (struct regcache_types *)test->param_value; + struct regmap *map; + struct regmap_config config; + struct regmap_ram_data *data; + unsigned int val, rval; + + config = test_regmap_config; + config.cache_type = t->type; + + map = gen_regmap(&config, &data); + KUNIT_ASSERT_FALSE(test, IS_ERR(map)); + if (IS_ERR(map)) + return; + + get_random_bytes(&val, sizeof(val)); + + /* Ensure the cache has a value in it */ + KUNIT_EXPECT_EQ(test, 0, regmap_write(map, 0, val)); + + /* Bypass then write a different value */ + regcache_cache_bypass(map, true); + KUNIT_EXPECT_EQ(test, 0, regmap_write(map, 0, val + 1)); + + /* Read the bypassed value */ + KUNIT_EXPECT_EQ(test, 0, regmap_read(map, 0, &rval)); + KUNIT_EXPECT_EQ(test, val + 1, rval); + KUNIT_EXPECT_EQ(test, data->vals[0], rval); + + /* Disable bypass, the cache should still return the original value */ + regcache_cache_bypass(map, false); + KUNIT_EXPECT_EQ(test, 0, regmap_read(map, 0, &rval)); + KUNIT_EXPECT_EQ(test, val, rval); + + regmap_exit(map); +} + +static void cache_sync(struct kunit *test) +{ + struct regcache_types *t = (struct regcache_types *)test->param_value; + struct regmap *map; + struct regmap_config config; + struct regmap_ram_data *data; + unsigned int val[BLOCK_TEST_SIZE]; + int i; + + config = test_regmap_config; + config.cache_type = t->type; + + map = gen_regmap(&config, &data); + KUNIT_ASSERT_FALSE(test, IS_ERR(map)); + if (IS_ERR(map)) + return; + + get_random_bytes(&val, sizeof(val)); + + /* Put some data into the cache */ + KUNIT_EXPECT_EQ(test, 0, regmap_bulk_write(map, 0, val, + BLOCK_TEST_SIZE)); + for (i = 0; i < BLOCK_TEST_SIZE; i++) + data->written[i] = false; + + /* Trash the data on the device itself then resync */ + regcache_mark_dirty(map); + memset(data->vals, 0, sizeof(val)); + KUNIT_EXPECT_EQ(test, 0, regcache_sync(map)); + + /* Did we just write the correct data out? */ + KUNIT_EXPECT_MEMEQ(test, data->vals, val, sizeof(val)); + for (i = 0; i < BLOCK_TEST_SIZE; i++) + KUNIT_EXPECT_EQ(test, true, data->written[i]); + + regmap_exit(map); +} + +static void cache_sync_defaults(struct kunit *test) +{ + struct regcache_types *t = (struct regcache_types *)test->param_value; + struct regmap *map; + struct regmap_config config; + struct regmap_ram_data *data; + unsigned int val; + int i; + + config = test_regmap_config; + config.cache_type = t->type; + config.num_reg_defaults = BLOCK_TEST_SIZE; + + map = gen_regmap(&config, &data); + KUNIT_ASSERT_FALSE(test, IS_ERR(map)); + if (IS_ERR(map)) + return; + + get_random_bytes(&val, sizeof(val)); + + /* Change the value of one register */ + KUNIT_EXPECT_EQ(test, 0, regmap_write(map, 2, val)); + + /* Resync */ + regcache_mark_dirty(map); + for (i = 0; i < BLOCK_TEST_SIZE; i++) + data->written[i] = false; + KUNIT_EXPECT_EQ(test, 0, regcache_sync(map)); + + /* Did we just sync the one register we touched? */ + for (i = 0; i < BLOCK_TEST_SIZE; i++) + KUNIT_EXPECT_EQ(test, i == 2, data->written[i]); + + regmap_exit(map); +} + +static void cache_sync_patch(struct kunit *test) +{ + struct regcache_types *t = (struct regcache_types *)test->param_value; + struct regmap *map; + struct regmap_config config; + struct regmap_ram_data *data; + struct reg_sequence patch[2]; + unsigned int rval[BLOCK_TEST_SIZE], val; + int i; + + /* We need defaults so readback works */ + config = test_regmap_config; + config.cache_type = t->type; + config.num_reg_defaults = BLOCK_TEST_SIZE; + + map = gen_regmap(&config, &data); + KUNIT_ASSERT_FALSE(test, IS_ERR(map)); + if (IS_ERR(map)) + return; + + /* Stash the original values */ + KUNIT_EXPECT_EQ(test, 0, regmap_bulk_read(map, 0, rval, + BLOCK_TEST_SIZE)); + + /* Patch a couple of values */ + patch[0].reg = 2; + patch[0].def = rval[2] + 1; + patch[0].delay_us = 0; + patch[1].reg = 5; + patch[1].def = rval[5] + 1; + patch[1].delay_us = 0; + KUNIT_EXPECT_EQ(test, 0, regmap_register_patch(map, patch, + ARRAY_SIZE(patch))); + + /* Sync the cache */ + regcache_mark_dirty(map); + for (i = 0; i < BLOCK_TEST_SIZE; i++) + data->written[i] = false; + KUNIT_EXPECT_EQ(test, 0, regcache_sync(map)); + + /* The patch should be on the device but not in the cache */ + for (i = 0; i < BLOCK_TEST_SIZE; i++) { + KUNIT_EXPECT_EQ(test, 0, regmap_read(map, i, &val)); + KUNIT_EXPECT_EQ(test, val, rval[i]); + + switch (i) { + case 2: + case 5: + KUNIT_EXPECT_EQ(test, true, data->written[i]); + KUNIT_EXPECT_EQ(test, data->vals[i], rval[i] + 1); + break; + default: + KUNIT_EXPECT_EQ(test, false, data->written[i]); + KUNIT_EXPECT_EQ(test, data->vals[i], rval[i]); + break; + } + } + + regmap_exit(map); +} + +static void cache_drop(struct kunit *test) +{ + struct regcache_types *t = (struct regcache_types *)test->param_value; + struct regmap *map; + struct regmap_config config; + struct regmap_ram_data *data; + unsigned int rval[BLOCK_TEST_SIZE]; + int i; + + config = test_regmap_config; + config.cache_type = t->type; + config.num_reg_defaults = BLOCK_TEST_SIZE; + + map = gen_regmap(&config, &data); + KUNIT_ASSERT_FALSE(test, IS_ERR(map)); + if (IS_ERR(map)) + return; + + /* Ensure the data is read from the cache */ + for (i = 0; i < BLOCK_TEST_SIZE; i++) + data->read[i] = false; + KUNIT_EXPECT_EQ(test, 0, regmap_bulk_read(map, 0, rval, + BLOCK_TEST_SIZE)); + for (i = 0; i < BLOCK_TEST_SIZE; i++) { + KUNIT_EXPECT_FALSE(test, data->read[i]); + data->read[i] = false; + } + KUNIT_EXPECT_MEMEQ(test, data->vals, rval, sizeof(rval)); + + /* Drop some registers */ + KUNIT_EXPECT_EQ(test, 0, regcache_drop_region(map, 3, 5)); + + /* Reread and check only the dropped registers hit the device. */ + KUNIT_EXPECT_EQ(test, 0, regmap_bulk_read(map, 0, rval, + BLOCK_TEST_SIZE)); + for (i = 0; i < BLOCK_TEST_SIZE; i++) + KUNIT_EXPECT_EQ(test, data->read[i], i >= 3 && i <= 5); + KUNIT_EXPECT_MEMEQ(test, data->vals, rval, sizeof(rval)); + + regmap_exit(map); +} + +static struct kunit_case regmap_test_cases[] = { + KUNIT_CASE_PARAM(basic_read_write, regcache_types_gen_params), + KUNIT_CASE_PARAM(bulk_write, regcache_types_gen_params), + KUNIT_CASE_PARAM(bulk_read, regcache_types_gen_params), + KUNIT_CASE_PARAM(reg_defaults, regcache_types_gen_params), + KUNIT_CASE_PARAM(reg_defaults_read_dev, regcache_types_gen_params), + KUNIT_CASE_PARAM(register_patch, regcache_types_gen_params), + KUNIT_CASE_PARAM(stride, regcache_types_gen_params), + KUNIT_CASE_PARAM(basic_ranges, regcache_types_gen_params), + KUNIT_CASE_PARAM(stress_insert, regcache_types_gen_params), + KUNIT_CASE_PARAM(cache_bypass, real_cache_types_gen_params), + KUNIT_CASE_PARAM(cache_sync, real_cache_types_gen_params), + KUNIT_CASE_PARAM(cache_sync_defaults, real_cache_types_gen_params), + KUNIT_CASE_PARAM(cache_sync_patch, real_cache_types_gen_params), + KUNIT_CASE_PARAM(cache_drop, sparse_cache_types_gen_params), + {} +}; + +static struct kunit_suite regmap_test_suite = { + .name = "regmap", + .test_cases = regmap_test_cases, +}; +kunit_test_suite(regmap_test_suite); + +MODULE_LICENSE("GPL v2"); From 05933e2d44607767ecb4937a33df4e882bdf9ad3 Mon Sep 17 00:00:00 2001 From: Mark Brown Date: Thu, 30 Mar 2023 01:10:23 +0100 Subject: [PATCH 14/19] regmap: Factor out single value register syncing In order to support sparse caches that don't store data in raw format factor out the parts of the raw block sync implementation that deal with writing a single register via _regmap_write(). Signed-off-by: Mark Brown Link: https://lore.kernel.org/r/20230325-regcache-maple-v3-1-23e271f93dc7@kernel.org Signed-off-by: Mark Brown --- drivers/base/regmap/internal.h | 1 + drivers/base/regmap/regcache.c | 40 ++++++++++++++++++++++------------ 2 files changed, 27 insertions(+), 14 deletions(-) diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h index 10aca7119d33..7b9ef43bcea6 100644 --- a/drivers/base/regmap/internal.h +++ b/drivers/base/regmap/internal.h @@ -270,6 +270,7 @@ unsigned int regcache_get_val(struct regmap *map, const void *base, bool regcache_set_val(struct regmap *map, void *base, unsigned int idx, unsigned int val); int regcache_lookup_reg(struct regmap *map, unsigned int reg); +int regcache_sync_val(struct regmap *map, unsigned int reg, unsigned int val); int _regmap_raw_write(struct regmap *map, unsigned int reg, const void *val, size_t val_len, bool noinc); diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c index 3f19f68c6a6c..a5f11bcc1215 100644 --- a/drivers/base/regmap/regcache.c +++ b/drivers/base/regmap/regcache.c @@ -677,6 +677,30 @@ static bool regcache_reg_present(unsigned long *cache_present, unsigned int idx) return test_bit(idx, cache_present); } +int regcache_sync_val(struct regmap *map, unsigned int reg, unsigned int val) +{ + int ret; + + if (!regcache_reg_needs_sync(map, reg, val)) + return 0; + + map->cache_bypass = true; + + ret = _regmap_write(map, reg, val); + + map->cache_bypass = false; + + if (ret != 0) { + dev_err(map->dev, "Unable to sync register %#x. %d\n", + reg, ret); + return ret; + } + dev_dbg(map->dev, "Synced register %#x, value %#x\n", + reg, val); + + return 0; +} + static int regcache_sync_block_single(struct regmap *map, void *block, unsigned long *cache_present, unsigned int block_base, @@ -693,21 +717,9 @@ static int regcache_sync_block_single(struct regmap *map, void *block, continue; val = regcache_get_val(map, block, i); - if (!regcache_reg_needs_sync(map, regtmp, val)) - continue; - - map->cache_bypass = true; - - ret = _regmap_write(map, regtmp, val); - - map->cache_bypass = false; - if (ret != 0) { - dev_err(map->dev, "Unable to sync register %#x. %d\n", - regtmp, ret); + ret = regcache_sync_val(map, regtmp, val); + if (ret != 0) return ret; - } - dev_dbg(map->dev, "Synced register %#x, value %#x\n", - regtmp, val); } return 0; From f033c26de5a5734625d2dd1dc196745fae186f1b Mon Sep 17 00:00:00 2001 From: Mark Brown Date: Thu, 30 Mar 2023 01:10:24 +0100 Subject: [PATCH 15/19] regmap: Add maple tree based register cache The current state of the art for sparse register maps is the rbtree cache. This works well for most applications but isn't always ideal for sparser register maps since the rbtree can get deep, requiring a lot of walking. Fortunately the kernel has a data structure intended to address this very problem, the maple tree. Provide an initial implementation of a register cache based on the maple tree to start taking advantage of it. The entries stored in the maple tree are arrays of register values, with the maple tree keys holding the register addresses. We store data in host native format rather than device native format as we do for rbtree, this will be a benefit for devices where we don't marshal data within regmap and simplifies the code but will result in additional CPU overhead when syncing the cache on devices where we do marshal data in regmap. This should work well for a lot of devices, though there's some additional areas that could be looked at such as caching the last accessed entry like we do for rbtree and trying to minimise the maple tree level locking. We should also use bulk writes rather than single register writes when resyncing the cache where possible, even if we don't store in device native format. Very small register maps may continue to to better with rbtree longer term. Signed-off-by: Mark Brown Link: https://lore.kernel.org/r/20230325-regcache-maple-v3-2-23e271f93dc7@kernel.org Signed-off-by: Mark Brown --- drivers/base/regmap/Makefile | 2 +- drivers/base/regmap/internal.h | 1 + drivers/base/regmap/regcache-maple.c | 278 +++++++++++++++++++++++++++ drivers/base/regmap/regcache.c | 1 + drivers/base/regmap/regmap-kunit.c | 3 + include/linux/regmap.h | 1 + 6 files changed, 285 insertions(+), 1 deletion(-) create mode 100644 drivers/base/regmap/regcache-maple.c diff --git a/drivers/base/regmap/Makefile b/drivers/base/regmap/Makefile index 4cb73468a197..f6c6cb017200 100644 --- a/drivers/base/regmap/Makefile +++ b/drivers/base/regmap/Makefile @@ -3,7 +3,7 @@ CFLAGS_regmap.o := -I$(src) obj-$(CONFIG_REGMAP) += regmap.o regcache.o -obj-$(CONFIG_REGMAP) += regcache-rbtree.o regcache-flat.o +obj-$(CONFIG_REGMAP) += regcache-rbtree.o regcache-flat.o regcache-maple.o obj-$(CONFIG_DEBUG_FS) += regmap-debugfs.o obj-$(CONFIG_REGMAP_KUNIT) += regmap-kunit.o obj-$(CONFIG_REGMAP_AC97) += regmap-ac97.o diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h index 7b9ef43bcea6..6361df6f553a 100644 --- a/drivers/base/regmap/internal.h +++ b/drivers/base/regmap/internal.h @@ -282,6 +282,7 @@ enum regmap_endian regmap_get_val_endian(struct device *dev, const struct regmap_config *config); extern struct regcache_ops regcache_rbtree_ops; +extern struct regcache_ops regcache_maple_ops; extern struct regcache_ops regcache_flat_ops; static inline const char *regmap_name(const struct regmap *map) diff --git a/drivers/base/regmap/regcache-maple.c b/drivers/base/regmap/regcache-maple.c new file mode 100644 index 000000000000..497cc708d277 --- /dev/null +++ b/drivers/base/regmap/regcache-maple.c @@ -0,0 +1,278 @@ +// SPDX-License-Identifier: GPL-2.0 +// +// Register cache access API - maple tree based cache +// +// Copyright 2023 Arm, Ltd +// +// Author: Mark Brown + +#include +#include +#include +#include + +#include "internal.h" + +static int regcache_maple_read(struct regmap *map, + unsigned int reg, unsigned int *value) +{ + struct maple_tree *mt = map->cache; + MA_STATE(mas, mt, reg, reg); + unsigned long *entry; + + rcu_read_lock(); + + entry = mas_find(&mas, reg); + if (!entry) { + rcu_read_unlock(); + return -ENOENT; + } + + *value = entry[reg - mas.index]; + + rcu_read_unlock(); + + return 0; +} + +static int regcache_maple_write(struct regmap *map, unsigned int reg, + unsigned int val) +{ + struct maple_tree *mt = map->cache; + MA_STATE(mas, mt, reg, reg); + unsigned long *entry, *upper, *lower; + unsigned long index, last; + size_t lower_sz, upper_sz; + int ret; + + rcu_read_lock(); + + entry = mas_find(&mas, reg); + if (entry) { + entry[reg - mas.index] = val; + rcu_read_unlock(); + return 0; + } + + /* Any adjacent entries to extend/merge? */ + mas_set_range(&mas, reg - 1, reg + 1); + index = reg; + last = reg; + + lower = mas_find(&mas, reg - 1); + if (lower) { + index = mas.index; + lower_sz = (mas.last - mas.index + 1) * sizeof(unsigned long); + } + + upper = mas_find(&mas, reg + 1); + if (upper) { + last = mas.last; + upper_sz = (mas.last - mas.index + 1) * sizeof(unsigned long); + } + + rcu_read_unlock(); + + entry = kmalloc((last - index + 1) * sizeof(unsigned long), + GFP_KERNEL); + if (!entry) + return -ENOMEM; + + if (lower) + memcpy(entry, lower, lower_sz); + entry[reg - index] = val; + if (upper) + memcpy(&entry[reg - index + 1], upper, upper_sz); + + /* + * This is safe because the regmap lock means the Maple lock + * is redundant, but we need to take it due to lockdep asserts + * in the maple tree code. + */ + mas_lock(&mas); + + mas_set_range(&mas, index, last); + ret = mas_store_gfp(&mas, entry, GFP_KERNEL); + + mas_unlock(&mas); + + if (ret == 0) { + kfree(lower); + kfree(upper); + } + + return ret; +} + +static int regcache_maple_drop(struct regmap *map, unsigned int min, + unsigned int max) +{ + struct maple_tree *mt = map->cache; + MA_STATE(mas, mt, min, max); + unsigned long *entry, *lower, *upper; + unsigned long lower_index, lower_last; + unsigned long upper_index, upper_last; + int ret; + + lower = NULL; + upper = NULL; + + mas_lock(&mas); + + mas_for_each(&mas, entry, max) { + /* + * This is safe because the regmap lock means the + * Maple lock is redundant, but we need to take it due + * to lockdep asserts in the maple tree code. + */ + mas_unlock(&mas); + + /* Do we need to save any of this entry? */ + if (mas.index < min) { + lower_index = mas.index; + lower_last = min -1; + + lower = kmemdup(entry, ((min - mas.index) * + sizeof(unsigned long)), + GFP_KERNEL); + if (!lower) { + ret = -ENOMEM; + goto out; + } + } + + if (mas.last > max) { + upper_index = max + 1; + upper_last = mas.last; + + upper = kmemdup(&entry[max + 1], + ((mas.last - max) * + sizeof(unsigned long)), + GFP_KERNEL); + if (!upper) { + ret = -ENOMEM; + goto out; + } + } + + kfree(entry); + mas_lock(&mas); + mas_erase(&mas); + + /* Insert new nodes with the saved data */ + if (lower) { + mas_set_range(&mas, lower_index, lower_last); + ret = mas_store_gfp(&mas, lower, GFP_KERNEL); + if (ret != 0) + goto out; + lower = NULL; + } + + if (upper) { + mas_set_range(&mas, upper_index, upper_last); + ret = mas_store_gfp(&mas, upper, GFP_KERNEL); + if (ret != 0) + goto out; + upper = NULL; + } + } + +out: + mas_unlock(&mas); + kfree(lower); + kfree(upper); + + return ret; +} + +static int regcache_maple_sync(struct regmap *map, unsigned int min, + unsigned int max) +{ + struct maple_tree *mt = map->cache; + unsigned long *entry; + MA_STATE(mas, mt, min, max); + unsigned long lmin = min; + unsigned long lmax = max; + unsigned int r; + int ret; + + map->cache_bypass = true; + + rcu_read_lock(); + + mas_for_each(&mas, entry, max) { + for (r = max(mas.index, lmin); r <= min(mas.last, lmax); r++) { + ret = regcache_sync_val(map, r, entry[r - mas.index]); + if (ret != 0) + goto out; + } + } + +out: + rcu_read_unlock(); + + map->cache_bypass = false; + + return ret; +} + +static int regcache_maple_exit(struct regmap *map) +{ + struct maple_tree *mt = map->cache; + MA_STATE(mas, mt, 0, UINT_MAX); + unsigned int *entry;; + + /* if we've already been called then just return */ + if (!mt) + return 0; + + mas_lock(&mas); + mas_for_each(&mas, entry, UINT_MAX) + kfree(entry); + __mt_destroy(mt); + mas_unlock(&mas); + + kfree(mt); + map->cache = NULL; + + return 0; +} + +static int regcache_maple_init(struct regmap *map) +{ + struct maple_tree *mt; + int i; + int ret; + + mt = kmalloc(sizeof(*mt), GFP_KERNEL); + if (!mt) + return -ENOMEM; + map->cache = mt; + + mt_init(mt); + + for (i = 0; i < map->num_reg_defaults; i++) { + ret = regcache_maple_write(map, + map->reg_defaults[i].reg, + map->reg_defaults[i].def); + if (ret) + goto err; + } + + return 0; + +err: + regcache_maple_exit(map); + return ret; +} + +struct regcache_ops regcache_maple_ops = { + .type = REGCACHE_MAPLE, + .name = "maple", + .init = regcache_maple_init, + .exit = regcache_maple_exit, + .read = regcache_maple_read, + .write = regcache_maple_write, + .drop = regcache_maple_drop, + .sync = regcache_maple_sync, +}; diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c index a5f11bcc1215..029564695dbb 100644 --- a/drivers/base/regmap/regcache.c +++ b/drivers/base/regmap/regcache.c @@ -17,6 +17,7 @@ static const struct regcache_ops *cache_types[] = { ®cache_rbtree_ops, + ®cache_maple_ops, ®cache_flat_ops, }; diff --git a/drivers/base/regmap/regmap-kunit.c b/drivers/base/regmap/regmap-kunit.c index c78f45cf9a8d..f76d41688134 100644 --- a/drivers/base/regmap/regmap-kunit.c +++ b/drivers/base/regmap/regmap-kunit.c @@ -29,6 +29,7 @@ static const struct regcache_types regcache_types_list[] = { { REGCACHE_NONE, "none" }, { REGCACHE_FLAT, "flat" }, { REGCACHE_RBTREE, "rbtree" }, + { REGCACHE_MAPLE, "maple" }, }; KUNIT_ARRAY_PARAM(regcache_types, regcache_types_list, case_to_desc); @@ -36,12 +37,14 @@ KUNIT_ARRAY_PARAM(regcache_types, regcache_types_list, case_to_desc); static const struct regcache_types real_cache_types_list[] = { { REGCACHE_FLAT, "flat" }, { REGCACHE_RBTREE, "rbtree" }, + { REGCACHE_MAPLE, "maple" }, }; KUNIT_ARRAY_PARAM(real_cache_types, real_cache_types_list, case_to_desc); static const struct regcache_types sparse_cache_types_list[] = { { REGCACHE_RBTREE, "rbtree" }, + { REGCACHE_MAPLE, "maple" }, }; KUNIT_ARRAY_PARAM(sparse_cache_types, sparse_cache_types_list, case_to_desc); diff --git a/include/linux/regmap.h b/include/linux/regmap.h index b63204d51b35..4d55ac88ba9e 100644 --- a/include/linux/regmap.h +++ b/include/linux/regmap.h @@ -51,6 +51,7 @@ enum regcache_type { REGCACHE_NONE, REGCACHE_RBTREE, REGCACHE_FLAT, + REGCACHE_MAPLE, }; /** From 451941ac1ee2be125ac5029593a64b04badaa314 Mon Sep 17 00:00:00 2001 From: Mark Brown Date: Mon, 3 Apr 2023 21:02:39 +0100 Subject: [PATCH 16/19] regmap: Fix double unlock in the maple cache Doing the dance to drop the maple tree's internal spinlock means we need multiple exit paths in our error handling. Reported-by: Liam R. Howlett Signed-off-by: Mark Brown Link: https://lore.kernel.org/r/20230403-regmap-maple-unlock-v1-1-89998991b16c@kernel.org Signed-off-by: Mark Brown --- drivers/base/regmap/regcache-maple.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/base/regmap/regcache-maple.c b/drivers/base/regmap/regcache-maple.c index 497cc708d277..20fb7228fc6b 100644 --- a/drivers/base/regmap/regcache-maple.c +++ b/drivers/base/regmap/regcache-maple.c @@ -137,7 +137,7 @@ static int regcache_maple_drop(struct regmap *map, unsigned int min, GFP_KERNEL); if (!lower) { ret = -ENOMEM; - goto out; + goto out_unlocked; } } @@ -151,7 +151,7 @@ static int regcache_maple_drop(struct regmap *map, unsigned int min, GFP_KERNEL); if (!upper) { ret = -ENOMEM; - goto out; + goto out_unlocked; } } @@ -179,6 +179,7 @@ static int regcache_maple_drop(struct regmap *map, unsigned int min, out: mas_unlock(&mas); +out_unlocked: kfree(lower); kfree(upper); From fac79bad889bb167a37492181646992c8c48903b Mon Sep 17 00:00:00 2001 From: Mark Brown Date: Tue, 4 Apr 2023 20:42:28 +0100 Subject: [PATCH 17/19] regmap: Use mas_walk() instead of mas_find() Liam recommends using mas_walk() instead of mas_find() for our use case so let's do that, it avoids some minor overhead associated with being able to restart the operation which we don't need since we do a simple search. Suggested-by: Liam R. Howlett Signed-off-by: Mark Brown Link: https://lore.kernel.org/r/20230403-regmap-maple-walk-fine-v2-1-c07371c8a867@kernel.org Signed-off-by: Mark Brown --- drivers/base/regmap/regcache-maple.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/base/regmap/regcache-maple.c b/drivers/base/regmap/regcache-maple.c index 20fb7228fc6b..9b1b559107ef 100644 --- a/drivers/base/regmap/regcache-maple.c +++ b/drivers/base/regmap/regcache-maple.c @@ -22,7 +22,7 @@ static int regcache_maple_read(struct regmap *map, rcu_read_lock(); - entry = mas_find(&mas, reg); + entry = mas_walk(&mas); if (!entry) { rcu_read_unlock(); return -ENOENT; @@ -47,7 +47,7 @@ static int regcache_maple_write(struct regmap *map, unsigned int reg, rcu_read_lock(); - entry = mas_find(&mas, reg); + entry = mas_walk(&mas); if (entry) { entry[reg - mas.index] = val; rcu_read_unlock(); From 7697c64b9e4908196f0ae68aa6d423dd40607973 Mon Sep 17 00:00:00 2001 From: William Breathitt Gray Date: Wed, 5 Apr 2023 11:45:42 -0400 Subject: [PATCH 18/19] regmap: Pass irq_drv_data as a parameter for set_type_config() Allow the struct regmap_irq_chip set_type_config() callback to access irq_drv_data by passing it as a parameter. Signed-off-by: William Breathitt Gray Reviewed-by: Andy Shevchenko Link: https://lore.kernel.org/r/20e15cd3afae80922b7e0577c7741df86b3390c5.1680708357.git.william.gray@linaro.org Signed-off-by: Mark Brown --- drivers/base/regmap/regmap-irq.c | 8 +++++--- include/linux/regmap.h | 6 ++++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/base/regmap/regmap-irq.c b/drivers/base/regmap/regmap-irq.c index 8c903b8c9714..e489b235b36b 100644 --- a/drivers/base/regmap/regmap-irq.c +++ b/drivers/base/regmap/regmap-irq.c @@ -329,8 +329,8 @@ static int regmap_irq_set_type(struct irq_data *data, unsigned int type) } if (d->chip->set_type_config) { - ret = d->chip->set_type_config(d->config_buf, type, - irq_data, reg); + ret = d->chip->set_type_config(d->config_buf, type, irq_data, + reg, d->chip->irq_drv_data); if (ret) return ret; } @@ -651,13 +651,15 @@ EXPORT_SYMBOL_GPL(regmap_irq_get_irq_reg_linear); * @type: The requested IRQ type. * @irq_data: The IRQ being configured. * @idx: Index of the irq's config registers within each array `buf[i]` + * @irq_drv_data: Driver specific IRQ data * * This is a &struct regmap_irq_chip->set_type_config callback suitable for * chips with one config register. Register values are updated according to * the &struct regmap_irq_type data associated with an IRQ. */ int regmap_irq_set_type_config_simple(unsigned int **buf, unsigned int type, - const struct regmap_irq *irq_data, int idx) + const struct regmap_irq *irq_data, + int idx, void *irq_drv_data) { const struct regmap_irq_type *t = &irq_data->type; diff --git a/include/linux/regmap.h b/include/linux/regmap.h index 4d10790adeb0..fb92d0ac13af 100644 --- a/include/linux/regmap.h +++ b/include/linux/regmap.h @@ -1650,7 +1650,8 @@ struct regmap_irq_chip { int (*set_type_virt)(unsigned int **buf, unsigned int type, unsigned long hwirq, int reg); int (*set_type_config)(unsigned int **buf, unsigned int type, - const struct regmap_irq *irq_data, int idx); + const struct regmap_irq *irq_data, int idx, + void *irq_drv_data); unsigned int (*get_irq_reg)(struct regmap_irq_chip_data *data, unsigned int base, int index); void *irq_drv_data; @@ -1659,7 +1660,8 @@ struct regmap_irq_chip { unsigned int regmap_irq_get_irq_reg_linear(struct regmap_irq_chip_data *data, unsigned int base, int index); int regmap_irq_set_type_config_simple(unsigned int **buf, unsigned int type, - const struct regmap_irq *irq_data, int idx); + const struct regmap_irq *irq_data, + int idx, void *irq_drv_data); int regmap_add_irq_chip(struct regmap *map, int irq, int irq_flags, int irq_base, const struct regmap_irq_chip *chip, From 4a670ac3e75e517c96cbd01ef870dbd598c3ce71 Mon Sep 17 00:00:00 2001 From: Maxime Chevallier Date: Fri, 7 Apr 2023 17:26:04 +0200 Subject: [PATCH 19/19] regmap: allow upshifting register addresses before performing operations 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 Tested-by: Colin Foster Link: https://lore.kernel.org/r/20230407152604.105467-1-maxime.chevallier@bootlin.com Signed-off-by: Mark Brown --- 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 6361df6f553a..9bd0dfd1e259 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 473b65b102db..db7851f0e3b8 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 8743b177399e..c2b9cc5db824 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;