[PATCH] regmap: Check for register readability before checking cache during read

[PATCH] regmap: Check for register readability before checking cache during read

[Mark Brown]

Ensure that we don't return a spurious cache hit for unreadable registers
(eg, with the flat cache which doesn't understand sparseness) by checking
for readability before we do a cache lookup.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/base/regmap/regmap.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index fa2d3fba6ac9..3efbe59ca1a7 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -2897,6 +2897,9 @@ static int _regmap_read(struct regmap *map, unsigned int reg,
 	int ret;
 	void *context = _regmap_map_get_context(map);
 
+	if (!regmap_readable(map, reg))
+		return -EIO;
+
 	if (!map->cache_bypass) {
 		ret = regcache_read(map, reg, val);
 		if (ret == 0)
@@ -2906,9 +2909,6 @@ static int _regmap_read(struct regmap *map, unsigned int reg,
 	if (map->cache_only)
 		return -EBUSY;
 
-	if (!regmap_readable(map, reg))
-		return -EIO;
-
 	ret = map->reg_read(context, reg, val);
 	if (ret == 0) {
 		if (regmap_should_log(map))

---
base-commit: 858fd168a95c5b9669aac8db6c14a9aeab446375
change-id: 20230613-b4-regmap-check-readability-before-cache-9f658338a5c1

Best regards,
-- 
Mark Brown <broonie@kernel.org>

Re: [PATCH] regmap: Check for register readability before checking cache during read

[Mark Brown]

On Tue, 13 Jun 2023 21:07:16 +0100, Mark Brown wrote:
> Ensure that we don't return a spurious cache hit for unreadable registers
> (eg, with the flat cache which doesn't understand sparseness) by checking
> for readability before we do a cache lookup.
> 
> 

Applied to

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

Thanks!

[1/1] regmap: Check for register readability before checking cache during read
      commit: eab5abdeb79f0f694c007c3a76a97902705c86f0

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

Re: [PATCH] regmap: Check for register readability before checking cache during read

[Konrad Dybcio]

On 13.06.2023 22:07, Mark Brown wrote:
> Ensure that we don't return a spurious cache hit for unreadable registers
> (eg, with the flat cache which doesn't understand sparseness) by checking
> for readability before we do a cache lookup.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
(+CC Bjorn)

Hi Mark,

this patch breaks using regmap_field_force_write() on fields that are
parts of registers marked as write-only (e.g. by regmap_access_table.no_ranges)

Is that intended?

What's the recommended fix?

FWIW this breaks soc/qcom/icc-bwmon.c, causing an interrupt storm at boot due
to the "clear the counters" register not being taken care of, so it'd be
appreciated if we can sort this out quickly.

Konrad
>  drivers/base/regmap/regmap.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
> index fa2d3fba6ac9..3efbe59ca1a7 100644
> --- a/drivers/base/regmap/regmap.c
> +++ b/drivers/base/regmap/regmap.c
> @@ -2897,6 +2897,9 @@ static int _regmap_read(struct regmap *map, unsigned int reg,
>  	int ret;
>  	void *context = _regmap_map_get_context(map);
>  
> +	if (!regmap_readable(map, reg))
> +		return -EIO;
> +
>  	if (!map->cache_bypass) {
>  		ret = regcache_read(map, reg, val);
>  		if (ret == 0)
> @@ -2906,9 +2909,6 @@ static int _regmap_read(struct regmap *map, unsigned int reg,
>  	if (map->cache_only)
>  		return -EBUSY;
>  
> -	if (!regmap_readable(map, reg))
> -		return -EIO;
> -
>  	ret = map->reg_read(context, reg, val);
>  	if (ret == 0) {
>  		if (regmap_should_log(map))
> 
> ---
> base-commit: 858fd168a95c5b9669aac8db6c14a9aeab446375
> change-id: 20230613-b4-regmap-check-readability-before-cache-9f658338a5c1
> 
> Best regards,

Re: [PATCH] regmap: Check for register readability before checking cache during read

[Mark Brown]

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

On Thu, Jun 15, 2023 at 10:45:53PM +0200, Konrad Dybcio wrote:
> On 13.06.2023 22:07, Mark Brown wrote:
> > Ensure that we don't return a spurious cache hit for unreadable registers
> > (eg, with the flat cache which doesn't understand sparseness) by checking
> > for readability before we do a cache lookup.

> this patch breaks using regmap_field_force_write() on fields that are
> parts of registers marked as write-only (e.g. by regmap_access_table.no_ranges)

> Is that intended?

> What's the recommended fix?

Ugh, let's just drop it - it's just an optimisation.

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