Message-ID: <baaa378164ae45c43a521eca467b49e0eda38818.camel@svanheule.net>
Date: Thu, 16 Oct 2025 22:49:40 +0200
From: Sander Vanheule <sander@...nheule.net>
To: Mark Brown <broonie@...nel.org>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>, "Rafael J. Wysocki"
	 <rafael@...nel.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] regmap: add cache validity to REGCACHE_FLAT

On Thu, 2025-10-16 at 14:55 +0100, Mark Brown wrote:
> On Thu, Oct 16, 2025 at 02:08:41PM +0200, Sander Vanheule wrote:
> > This difference in caching behavior isn't (wasn't) immediately clear to me from
> > the documentation. Don't the different cache types exist to optimize speed or
> > memory for different use-cases? Because then I would only expect differences in
> > memory/speed, not in way the cache is initialized.
> 
> The flat cache is a bit of a sharp edge, it's specifically there to be
> the absolute bare minimum cache.  It's there mainly for MMIO devices
> that are on the edge of being able to use regmap at all for performance
> reasons, and as a simple way of guaranteeing that we never do any
> allocations at runtime for things that do register access in hardirq
> context.
> 
> The documentation could definitely use some improvement here.

The proposed patch also won't perform any allocations outside of the regmap
initialization. There is the additional overhead of checking a bitmap, yes, but
that's probably not a huge problem if the main issue are the allocations?

A number of REGCACHE_FLAT users provide only num_reg_defaults_raw. That looked
strange to me, because why would a user provide a buffer size, but no buffer?
This even conflicts with the documentation, as "Number of elements in
reg_defaults_raw" doesn't make sense if reg_defaults_raw is NULL. As you are
probably aware, this configuration causes the regmap init to read registers from
the device to generate a default state, which is then used to fill the flat
cache. So this effectively replaces the (invalid) zero-initialization. What
feels problematic to me, is that only the flat cache requires this sort of
"hack" for it to contain values that correctly reflect the hardware state.

Some quick grepping turned up 35 drivers which use REGCACHE_FLAT without any
cache initialization (see below), thus defaulting to the all-zero cache. This
includes the driver I think you found issues with on the pine64: sun8i-codec.c
If this driver (accidentally) relies on the zero-initialization, I would expect
the spurious interrupts to also pop up when switching to a maple cache, or when
using the num_reg_defaults_raw workaround.

drivers/clk/clk-si521xx.c
drivers/gpio/gpio-104-dio-48e.c
drivers/gpio/gpio-104-idio-16.c
drivers/gpio/gpio-gpio-mm.c
drivers/gpio/gpio-pcie-idio-24.c
drivers/gpio/gpio-pci-idio-16.c
drivers/gpio/gpio-ws16c48.c
drivers/gpu/drm/panel/panel-abt-y030xx067a.c
drivers/gpu/drm/panel/panel-novatek-nt39016.c
drivers/i2c/muxes/i2c-mux-ltc4306.c
drivers/iio/addac/stx104.c
drivers/iio/health/max30100.c
drivers/mfd/sec-i2c.c
drivers/platform/mellanox/mlxreg-dpu.c
drivers/pwm/pwm-fsl-ftm.c
sound/soc/atmel/mchp-pdmc.c
sound/soc/atmel/mchp-spdifrx.c
sound/soc/atmel/mchp-spdiftx.c
sound/soc/bcm/bcm63xx-i2s-whistler.c
sound/soc/codecs/msm8916-wcd-digital.c
sound/soc/loongson/loongson_i2s_pci.c
sound/soc/loongson/loongson_i2s_plat.c
sound/soc/qcom/lpass-cpu.c
sound/soc/rockchip/rockchip_spdif.c
sound/soc/soc-ops-test.c
sound/soc/stm/stm32_sai_sub.c
sound/soc/sunxi/sun8i-codec.c
sound/soc/tegra/tegra20_ac97.c
sound/soc/tegra/tegra20_das.c
sound/soc/tegra/tegra20_i2s.c
sound/soc/tegra/tegra20_spdif.c
sound/soc/tegra/tegra210_ahub.c
sound/soc/tegra/tegra30_ahub.c
sound/soc/tegra/tegra30_i2s.c
sound/soc/xtensa/xtfpga-i2s.c

Powered by blists - more mailing lists