From 7a795ac8d49e2433e1b97caf5e99129daf8e1b08 Mon Sep 17 00:00:00 2001 From: Richard Fitzgerald Date: Fri, 22 Sep 2023 16:37:11 +0100 Subject: [PATCH 1/3] regmap: rbtree: Fix wrong register marked as in-cache when creating new node When regcache_rbtree_write() creates a new rbtree_node it was passing the wrong bit number to regcache_rbtree_set_register(). The bit number is the offset __in number of registers__, but in the case of creating a new block regcache_rbtree_write() was not dividing by the address stride to get the number of registers. Fix this by dividing by map->reg_stride. Compare with regcache_rbtree_read() where the bit is checked. This bug meant that the wrong register was marked as present. The register that was written to the cache could not be read from the cache because it was not marked as cached. But a nearby register could be marked as having a cached value even if it was never written to the cache. Signed-off-by: Richard Fitzgerald Fixes: 3f4ff561bc88 ("regmap: rbtree: Make cache_present bitmap per node") Link: https://lore.kernel.org/r/20230922153711.28103-1-rf@opensource.cirrus.com Signed-off-by: Mark Brown --- drivers/base/regmap/regcache-rbtree.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/base/regmap/regcache-rbtree.c b/drivers/base/regmap/regcache-rbtree.c index db716ffd083e..3db88bbcae0f 100644 --- a/drivers/base/regmap/regcache-rbtree.c +++ b/drivers/base/regmap/regcache-rbtree.c @@ -453,7 +453,8 @@ static int regcache_rbtree_write(struct regmap *map, unsigned int reg, if (!rbnode) return -ENOMEM; regcache_rbtree_set_register(map, rbnode, - reg - rbnode->base_reg, value); + (reg - rbnode->base_reg) / map->reg_stride, + value); regcache_rbtree_insert(map, &rbtree_ctx->root, rbnode); rbtree_ctx->cached_rbnode = rbnode; } From c6df843348d6b71ea986266c12831cb60c2cf325 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Fri, 6 Oct 2023 10:21:04 +0200 Subject: [PATCH 2/3] regmap: fix NULL deref on lookup Not all regmaps have a name so make sure to check for that to avoid dereferencing a NULL pointer when dev_get_regmap() is used to lookup a named regmap. Fixes: e84861fec32d ("regmap: dev_get_regmap_match(): fix string comparison") Cc: stable@vger.kernel.org # 5.8 Cc: Marc Kleine-Budde Signed-off-by: Johan Hovold Link: https://lore.kernel.org/r/20231006082104.16707-1-johan+linaro@kernel.org Signed-off-by: Mark Brown --- drivers/base/regmap/regmap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c index 884cb51c8f67..234a84ecde8b 100644 --- a/drivers/base/regmap/regmap.c +++ b/drivers/base/regmap/regmap.c @@ -1478,7 +1478,7 @@ static int dev_get_regmap_match(struct device *dev, void *res, void *data) /* If the user didn't specify a name match any */ if (data) - return !strcmp((*r)->name, data); + return (*r)->name && !strcmp((*r)->name, data); else return 1; } From 0ec7731655de196bc1e4af99e495b38778109d22 Mon Sep 17 00:00:00 2001 From: Mark Brown Date: Thu, 26 Oct 2023 16:49:19 +0100 Subject: [PATCH 3/3] regmap: Ensure range selector registers are updated after cache sync When we sync the register cache we do so with the cache bypassed in order to avoid overhead from writing the synced values back into the cache. If the regmap has ranges and the selector register for those ranges is in a register which is cached this has the unfortunate side effect of meaning that the physical and cached copies of the selector register can be out of sync after a cache sync. The cache will have whatever the selector was when the sync started and the hardware will have the selector for the register that was synced last. Fix this by rewriting all cached selector registers after every sync, ensuring that the hardware and cache have the same content. This will result in extra writes that wouldn't otherwise be needed but is simple so hopefully robust. We don't read from the hardware since not all devices have physical read support. Given that nobody noticed this until now it is likely that we are rarely if ever hitting this case. Reported-by: Hector Martin Cc: stable@vger.kernel.org Signed-off-by: Mark Brown Link: https://lore.kernel.org/r/20231026-regmap-fix-selector-sync-v1-1-633ded82770d@kernel.org Signed-off-by: Mark Brown --- drivers/base/regmap/regcache.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c index c5d151e9c481..92592f944a3d 100644 --- a/drivers/base/regmap/regcache.c +++ b/drivers/base/regmap/regcache.c @@ -334,6 +334,11 @@ static int regcache_default_sync(struct regmap *map, unsigned int min, return 0; } +static int rbtree_all(const void *key, const struct rb_node *node) +{ + return 0; +} + /** * regcache_sync - Sync the register cache with the hardware. * @@ -351,6 +356,7 @@ int regcache_sync(struct regmap *map) unsigned int i; const char *name; bool bypass; + struct rb_node *node; if (WARN_ON(map->cache_type == REGCACHE_NONE)) return -EINVAL; @@ -392,6 +398,30 @@ int regcache_sync(struct regmap *map) /* Restore the bypass state */ map->cache_bypass = bypass; map->no_sync_defaults = false; + + /* + * If we did any paging with cache bypassed and a cached + * paging register then the register and cache state might + * have gone out of sync, force writes of all the paging + * registers. + */ + rb_for_each(node, 0, &map->range_tree, rbtree_all) { + struct regmap_range_node *this = + rb_entry(node, struct regmap_range_node, node); + + /* If there's nothing in the cache there's nothing to sync */ + ret = regcache_read(map, this->selector_reg, &i); + if (ret != 0) + continue; + + ret = _regmap_write(map, this->selector_reg, i); + if (ret != 0) { + dev_err(map->dev, "Failed to write %x = %x: %d\n", + this->selector_reg, i, ret); + break; + } + } + map->unlock(map->lock_arg); regmap_async_complete(map);