Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Charles Keepax <ckeepax@opensource.cirrus.com>
To:     <broonie@kernel.org>
CC:     <gregkh@linuxfoundation.org>, <rafael@kernel.org>,
        <patches@opensource.cirrus.com>, <linux-kernel@vger.kernel.org>
Subject: [PATCH 2/2] regmap: Add missing cache_only checks
Date:   Thu, 1 Jun 2023 11:10:36 +0100
Message-ID: <20230601101036.1499612-2-ckeepax@opensource.cirrus.com>
In-Reply-To: <20230601101036.1499612-1-ckeepax@opensource.cirrus.com>
References: <20230601101036.1499612-1-ckeepax@opensource.cirrus.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Content-Type: text/plain
Precedence: bulk

The current behaviour around cache_only is slightly inconsistent,
most paths will only check cache_only if cache_bypass is false,
and will return -EBUSY if a read attempts to go to the hardware
whilst cache_only is true. However, a couple of paths will not check
cache_only at all.  The most notable of these being regmap_raw_read
which will check cache_only in the case it processes the transaction
one register at a time, but not in the case it handles them as a
block. In the typical case a device has been put into cache_only
whilst powered down this can cause physical reads to happen whilst the
device is unavailable.

Add a check in regmap_raw_read and move the check in regmap_noinc_read,
adding a check for cache_bypass, such that all paths are covered and
consistent.

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
 drivers/base/regmap/regmap.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index fa2d3fba6ac9d..627a767fa0470 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -2983,6 +2983,11 @@ int regmap_raw_read(struct regmap *map, unsigned int reg, void *val,
 		size_t chunk_count, chunk_bytes;
 		size_t chunk_regs = val_count;
 
+		if (!map->cache_bypass && map->cache_only) {
+			ret = -EBUSY;
+			goto out;
+		}
+
 		if (!map->read) {
 			ret = -ENOTSUPP;
 			goto out;
@@ -3078,18 +3083,19 @@ int regmap_noinc_read(struct regmap *map, unsigned int reg,
 		goto out_unlock;
 	}
 
+	/*
+	 * We have not defined the FIFO semantics for cache, as the
+	 * cache is just one value deep. Should we return the last
+	 * written value? Just avoid this by always reading the FIFO
+	 * even when using cache. Cache only will not work.
+	 */
+	if (!map->cache_bypass && map->cache_only) {
+		ret = -EBUSY;
+		goto out_unlock;
+	}
+
 	/* Use the accelerated operation if we can */
 	if (map->bus->reg_noinc_read) {
-		/*
-		 * We have not defined the FIFO semantics for cache, as the
-		 * cache is just one value deep. Should we return the last
-		 * written value? Just avoid this by always reading the FIFO
-		 * even when using cache. Cache only will not work.
-		 */
-		if (map->cache_only) {
-			ret = -EBUSY;
-			goto out_unlock;
-		}
 		ret = regmap_noinc_readwrite(map, reg, val, val_len, false);
 		goto out_unlock;
 	}
-- 
2.30.2