[PATCH 1/2] regmap: cache: Do not panic for REGCACHE_NONE regmap

[PATCH 1/2] regmap: cache: Do not panic for REGCACHE_NONE regmap

[Alexander Stein]

Most regcache operations do check for REGCACHE_NONE, before ensuring
doing BUG_ON(!map->cache_ops). The missing regcache_sync* functions
panic on REGCACHE_NONE regmaps instead. Add an early return for non-cached
ones.

Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
 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..b61763dbfc68 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 (map->cache_type == REGCACHE_NONE)
+		return 0;
+
 	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 (map->cache_type == REGCACHE_NONE)
+		return 0;
+
 	BUG_ON(!map->cache_ops);
 
 	map->lock(map->lock_arg);
-- 
2.34.1

[PATCH 2/2] regmap: cache: Fix return value

[Alexander Stein]

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 <alexander.stein@ew.tq-group.com>
---
This warning popped up while initially returning ENOSYS in patch 1.

But I'm wondering if returning 0 for regcache_write is correct or not.
This might be a follow-up patch though.

 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 b61763dbfc68..c13f5f8872ac 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);
 
-- 
2.34.1

Re: [PATCH 1/2] regmap: cache: Do not panic for REGCACHE_NONE regmap

[Mark Brown]

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

On Fri, Mar 10, 2023 at 08:39:10AM +0100, Alexander Stein wrote:
> Most regcache operations do check for REGCACHE_NONE, before ensuring
> doing BUG_ON(!map->cache_ops). The missing regcache_sync* functions
> panic on REGCACHE_NONE regmaps instead. Add an early return for non-cached
> ones.

Why would we be trying to do a regcache_sync() on a device with
no cache?

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

Re: [PATCH 1/2] regmap: cache: Do not panic for REGCACHE_NONE regmap

[Alexander Stein]

Hi Mark,

Am Freitag, 10. März 2023, 14:02:13 CET schrieb Mark Brown:
> * PGP Signed by an unknown key
> 
> On Fri, Mar 10, 2023 at 08:39:10AM +0100, Alexander Stein wrote:
> > Most regcache operations do check for REGCACHE_NONE, before ensuring
> > doing BUG_ON(!map->cache_ops). The missing regcache_sync* functions
> > panic on REGCACHE_NONE regmaps instead. Add an early return for non-cached
> > ones.
> 
> Why would we be trying to do a regcache_sync() on a device with
> no cache?

Indeed, that makes no sense. That's indicating a bug in a driver, but why do 
we need to panic the kernel in this case?
On the other hand the same question applies to other regcache related 
functions currently checking for non-cached maps. There is no common 
behaviour:

panic:
* regcache_sync
* regcache_sync_region

returning -EINVAL:
* regcache_drop_region

returning -ENOSYS:
* regcache_read

returning success (0):
* regcache_write

early return (void return value):
* regcache_exit

Given all these possibilities I have no idea what's the right thing to do.

Best regards,
Alexander

-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

Re: [PATCH 1/2] regmap: cache: Do not panic for REGCACHE_NONE regmap

[Mark Brown]

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

On Fri, Mar 10, 2023 at 02:35:13PM +0100, Alexander Stein wrote:
> Am Freitag, 10. März 2023, 14:02:13 CET schrieb Mark Brown:

> > Why would we be trying to do a regcache_sync() on a device with
> > no cache?

> Indeed, that makes no sense. That's indicating a bug in a driver, but why do 
> we need to panic the kernel in this case?

You're trying to change this to silently ignore the call which
isn't going to make anything happy.

> On the other hand the same question applies to other regcache related 
> functions currently checking for non-cached maps. There is no common 
> behaviour:

> panic:
> * regcache_sync
> * regcache_sync_region

These are only ever triggered from a client driver, nothing in
regmap will ever sync the regmap without being explicitly asked
to.

> returning -ENOSYS:
> * regcache_read

> returning success (0):
> * regcache_write

> early return (void return value):
> * regcache_exit

These are all called transparently as part of the regmap core
regardless of if there is a cache, users never directly read or
write values to the cache.

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