diff mbox series

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

Message ID 20230310073911.3470892-1-alexander.stein@ew.tq-group.com
State New
Headers
Series [1/2] regmap: cache: Do not panic for REGCACHE_NONE regmap |

Commit Message

Alexander Stein March 10, 2023, 7:39 a.m. UTC 
  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(+)

Comments

Mark Brown March 10, 2023, 1:02 p.m. UTC | #1 
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?
Alexander Stein March 10, 2023, 1:35 p.m. UTC | #2 
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
Mark Brown March 10, 2023, 2:21 p.m. UTC | #3 
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.
diff mbox series

Patch

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);