[PATCH] ASoC: tas2783-sdw: do not treat read-only Controls as writable

Andrey Golovko posted 1 patch 1 month, 2 weeks ago
sound/soc/codecs/tas2783-sdw.c | 52 ++++++++++++++++++++++++++++++++++
1 file changed, 52 insertions(+)
[PATCH] ASoC: tas2783-sdw: do not treat read-only Controls as writable
Posted by Andrey Golovko 1 month, 2 weeks ago
The regmap has no writeable_reg callback, so regmap considers every
register up to max_register writable.  That includes the read-only SDCA
Controls the driver itself describes: the Latency of every Entity, the
Clock Valid of every Clock Source, the actual power state of the Power
Domain Entity, the protection status, the algorithm ready flag and the
Extension Unit id, version and firmware download status.  Most of them
are also listed in tas2783_reg_default[] with a placeholder of zero,
even though a default for, say, a latency reading is meaningless.

Reading such a Control caches its real value, which no longer matches
the placeholder, so regcache_sync() then tries to write it back.  The
peripheral rejects the transaction with -ENODATA and the sync aborts,
leaving the rest of the cache unrestored.

Add a writeable_reg callback that refuses the read-only Controls and
otherwise keeps the previous behaviour.  Every selector it lists is the
read-only Control of its Entity type in sdca_function.h, and none of the
Controls the driver writes is affected: the requested power state, the
mutes, the Cluster Index, the protection mode, the algorithm enable and
the firmware download Controls all stay writable.

The list is static because the BIOS on the affected machines describes
no Smart Amp SDCA function, so the driver runs its fallback tables and
sdca_regmap_writeable() is not available to it.  It would be good to
have the list confirmed against the hardware documentation, and to know
whether the read-only Controls belong in tas2783_reg_default[] at all.

Signed-off-by: Andrey Golovko <andrey.golovko@gmail.com>
---
 sound/soc/codecs/tas2783-sdw.c | 52 ++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/sound/soc/codecs/tas2783-sdw.c b/sound/soc/codecs/tas2783-sdw.c
index c217da5fccdf..794d8b7cf4f9 100644
--- a/sound/soc/codecs/tas2783-sdw.c
+++ b/sound/soc/codecs/tas2783-sdw.c
@@ -495,6 +495,57 @@ static bool tas2783_readable_register(struct device *dev, unsigned int reg)
 	return tas2783_sdca_mbq_size(dev, reg) > 0;
 }
 
+static bool tas2783_writeable_register(struct device *dev, unsigned int reg)
+{
+	/*
+	 * The Latency Control of every Entity, together with the Power Domain
+	 * actual state and the protection status, is read-only. They are
+	 * listed in tas2783_reg_default[] with a placeholder value, so without
+	 * this a regcache_sync() would try to write them back and the
+	 * peripheral would reject the transaction, aborting the sync.
+	 */
+	switch (reg) {
+	case SDW_SDCA_CTL(FUNC_NUM_SMART_AMP, TAS2783_SDCA_ENT_FU21, 0x10, 0):
+	case SDW_SDCA_CTL(FUNC_NUM_SMART_AMP, TAS2783_SDCA_ENT_FU23, 0x10, 0):
+	case SDW_SDCA_CTL(FUNC_NUM_SMART_AMP, TAS2783_SDCA_ENT_FU26, 0x10, 0):
+	case SDW_SDCA_CTL(FUNC_NUM_SMART_AMP, TAS2783_SDCA_ENT_XU22, 0x06, 0):
+	case SDW_SDCA_CTL(FUNC_NUM_SMART_AMP, TAS2783_SDCA_ENT_XU22, 0x07, 0):
+	case SDW_SDCA_CTL(FUNC_NUM_SMART_AMP, TAS2783_SDCA_ENT_XU22, 0x08, 0):
+	case SDW_SDCA_CTL(FUNC_NUM_SMART_AMP, TAS2783_SDCA_ENT_XU22, 0x14, 0):
+	case SDW_SDCA_CTL(FUNC_NUM_SMART_AMP, TAS2783_SDCA_ENT_CS24, 0x02, 0):
+	case SDW_SDCA_CTL(FUNC_NUM_SMART_AMP, TAS2783_SDCA_ENT_CS21, 0x02, 0):
+	case SDW_SDCA_CTL(FUNC_NUM_SMART_AMP, TAS2783_SDCA_ENT_CS25, 0x02, 0):
+	case SDW_SDCA_CTL(FUNC_NUM_SMART_AMP, TAS2783_SDCA_ENT_CS26, 0x02, 0):
+	case SDW_SDCA_CTL(FUNC_NUM_SMART_AMP, TAS2783_SDCA_ENT_CS28, 0x02, 0):
+	case SDW_SDCA_CTL(FUNC_NUM_SMART_AMP, TAS2783_SDCA_ENT_PDE23, 0x10, 0):
+	case SDW_SDCA_CTL(FUNC_NUM_SMART_AMP, TAS2783_SDCA_ENT_UDMPU23, 0x06, 0):
+	case SDW_SDCA_CTL(FUNC_NUM_SMART_AMP, TAS2783_SDCA_ENT_SAPU29, 0x05, 0):
+	case SDW_SDCA_CTL(FUNC_NUM_SMART_AMP, TAS2783_SDCA_ENT_SAPU29, 0x11, 0):
+	case SDW_SDCA_CTL(FUNC_NUM_SMART_AMP, TAS2783_SDCA_ENT_PPU21, 0x06, 0):
+	case SDW_SDCA_CTL(FUNC_NUM_SMART_AMP, TAS2783_SDCA_ENT_PPU26, 0x06, 0):
+	case SDW_SDCA_CTL(FUNC_NUM_SMART_AMP, TAS2783_SDCA_ENT_IT21, 0x08, 0):
+	case SDW_SDCA_CTL(FUNC_NUM_SMART_AMP, TAS2783_SDCA_ENT_IT29, 0x08, 0):
+	case SDW_SDCA_CTL(FUNC_NUM_SMART_AMP, TAS2783_SDCA_ENT_IT26, 0x08, 0):
+	case SDW_SDCA_CTL(FUNC_NUM_SMART_AMP, TAS2783_SDCA_ENT_IT28, 0x08, 0):
+	case SDW_SDCA_CTL(FUNC_NUM_SMART_AMP, TAS2783_SDCA_ENT_OT24, 0x08, 0):
+	case SDW_SDCA_CTL(FUNC_NUM_SMART_AMP, TAS2783_SDCA_ENT_OT23, 0x08, 0):
+	case SDW_SDCA_CTL(FUNC_NUM_SMART_AMP, TAS2783_SDCA_ENT_OT25, 0x08, 0):
+	case SDW_SDCA_CTL(FUNC_NUM_SMART_AMP, TAS2783_SDCA_ENT_OT28, 0x08, 0):
+	case SDW_SDCA_CTL(FUNC_NUM_SMART_AMP, TAS2783_SDCA_ENT_MU26, 0x06, 0):
+	case SDW_SDCA_CTL(FUNC_NUM_SMART_AMP, TAS2783_SDCA_ENT_OT127, 0x08, 0):
+	case SDW_SDCA_CTL(FUNC_NUM_SMART_AMP, TAS2783_SDCA_ENT_FU127, 0x10, 0):
+	case SDW_SDCA_CTL(FUNC_NUM_SMART_AMP, TAS2783_SDCA_ENT_CS127, 0x02, 0):
+	case SDW_SDCA_CTL(FUNC_NUM_SMART_AMP, TAS2783_SDCA_ENT_MFPU21, 0x08, 0):
+	case SDW_SDCA_CTL(FUNC_NUM_SMART_AMP, TAS2783_SDCA_ENT_MFPU21, 0x04, 0):
+	case SDW_SDCA_CTL(FUNC_NUM_SMART_AMP, TAS2783_SDCA_ENT_MFPU26, 0x08, 0):
+	case SDW_SDCA_CTL(FUNC_NUM_SMART_AMP, TAS2783_SDCA_ENT_MFPU26, 0x04, 0):
+		return false;
+
+	default:
+		return tas2783_sdca_mbq_size(dev, reg) > 0;
+	}
+}
+
 static bool tas2783_volatile_register(struct device *dev, u32 reg)
 {
 	switch (reg) {
@@ -516,6 +567,7 @@ static const struct regmap_config tas_regmap = {
 	.reg_bits = 32,
 	.val_bits = 8,
 	.readable_reg = tas2783_readable_register,
+	.writeable_reg = tas2783_writeable_register,
 	.volatile_reg = tas2783_volatile_register,
 	.reg_defaults = tas2783_reg_default,
 	.num_reg_defaults = ARRAY_SIZE(tas2783_reg_default),
-- 
2.53.0
Re: [PATCH] ASoC: tas2783-sdw: do not treat read-only Controls as writable
Posted by Mark Brown 1 month, 2 weeks ago
On Thu, Aug 13, 2026 at 08:35:00PM +0300, Andrey Golovko wrote:
> The regmap has no writeable_reg callback, so regmap considers every
> register up to max_register writable.  That includes the read-only SDCA
> Controls the driver itself describes: the Latency of every Entity, the

> +	case SDW_SDCA_CTL(FUNC_NUM_SMART_AMP, TAS2783_SDCA_ENT_XU22, 0x14, 0):

0x14 is SDCA_CTL_XU_FDL_STATUS and AFAICT sdca_fdl_process() might write
to it.  Not sure if that's reachable with this device or not...
Re: [PATCH] ASoC: tas2783-sdw: do not treat read-only Controls as writable
Posted by Andrey Golovko 1 month, 2 weeks ago
On Thu, Aug 13, 2026 at 08:53:10PM +0100, Mark Brown wrote:
> > +	case SDW_SDCA_CTL(FUNC_NUM_SMART_AMP, TAS2783_SDCA_ENT_XU22, 0x14, 0):

> 0x14 is SDCA_CTL_XU_FDL_STATUS and AFAICT sdca_fdl_process() might write
> to it.  Not sure if that's reachable with this device or not...

You are right, and the entry is gone.  I took the Control for a status
the device owns; it is a handshake, and sdca_fdl_process() writes the
host response back into it with response | (status &
~SDCA_CTL_XU_FDLH_MASK).  _regmap_write() answers -EIO for a register
writeable_reg refuses, so the driver would have failed at the first
response.

Not reachable today, as far as I can see: the driver registers no SDCA
interrupts and never calls into sdca_fdl.c.  It downloads its firmware
itself, request_firmware_nowait() plus its own container format in
tas2783_fw_ready(), and the BIOS on the machines I have describes no
Smart Amp SDCA function at all, so nothing populates the interrupt that
would drive the state machine.  But the Function does implement the FDL
Controls - the driver's defaults list 0x10 and 0x12 through 0x16 - so a
driver that later gains the state machine would walk into it, and the
list is not worth that.

The Control is still not cached in 2/2, on its own grounds: the device
sets its bits on its own schedule and sdca_fdl_process() reads the
Control to see them, so an answer from the cache would hide the device
half of the handshake.  Writable and volatile.

v2 of the two patches is posted as a series, since the second depends on
the first:

  https://lore.kernel.org/linux-sound/20260814094000.22118-1-andrey.golovko@gmail.com/

Thanks for catching it.

Andrey