[PATCH v2 0/2] ASoC: tas2783-sdw: fix the read-only Control handling

[PATCH v2 0/2] ASoC: tas2783-sdw: fix the read-only Control handling

[Andrey Golovko]

The two read-only Control patches from 13 August, resent together as a
series because the second one depends on the first, and with the change
Mark asked for.

He pointed out that the XU22 Control 0x14 in the read-only list is
SDCA_CTL_XU_FDL_STATUS, which sdca_fdl_process() writes.  He is right,
and the description I gave it - "firmware download status" - reads like
a status the device owns, which it is not.  It is a handshake: the
device sets the bits it owns, and the host writes its response back into
the same Control, keeping the device bits, which is exactly what
sdca_fdl_process() does with response | (status & ~SDCA_CTL_XU_FDLH_MASK).
So it is out of the list in 1/2.

Whether the write is reachable on this device today: the driver
registers no SDCA interrupts and never calls into sdca_fdl.c.  It
downloads its firmware itself, with request_firmware_nowait() and its
own container format in tas2783_fw_ready(), and the machines I have
describe no Smart Amp SDCA function in the BIOS at all, so nothing
populates the interrupt that would drive the state machine.  But the
Function does implement the FDL Controls - the driver lists 0x10 and
0x12 through 0x16 in its defaults - so the state machine is a plausible
thing for this driver to grow, and _regmap_write() answers -EIO for a
register the callback refuses.  That would break the handshake at the
first response.  Not a trap worth planting for one entry in a list.

2/2 still keeps the Control out of the cache, and now says why on its
own: sdca_fdl_process() reads it to see what the device asked for, and a
cached answer would hide the device half of the handshake.  Writable and
volatile, which is the normal shape for a handshake register.

The two are otherwise unchanged.  The behaviour I measured on an ASUS
ProArt PX13 for v1 stands: the calibration data written at probe is byte
for byte what it was before the series, and a read of the PDE23 actual
power state answers 0x0 with the Function powered, where before it
answered the 0x3 placeholder from the defaults.  The only difference in
v2 is that a write to the File Download Status is no longer rejected,
and nothing in the driver issues one.

The questions in 1/2 stand as well: the list is static because
sdca_regmap_writeable() is not available to a driver running its
fallback tables, so it would be good to have it confirmed against the
documentation, and to know whether the read-only Controls were meant to
be in tas2783_reg_default[] at all.

Andrey Golovko (2):
  ASoC: tas2783-sdw: do not treat read-only Controls as writable
  ASoC: tas2783-sdw: do not cache read-only Controls

 sound/soc/codecs/tas2783-sdw.c | 99 +++++++++++++++++++++++-----------
 1 file changed, 68 insertions(+), 31 deletions(-)

[PATCH v2 2/2] ASoC: tas2783-sdw: do not cache read-only Controls

[Andrey Golovko]

The read-only SDCA Controls are readings of device state, but the regmap
caches them and tas2783_reg_default[] gives each one a placeholder, so a
read is answered from the cache and never reaches the peripheral.  The
answer is whatever the cache happens to hold: on an ASUS ProArt PX13 a
read of the PDE23 actual power state returns the 0x3 placeholder, i.e.
PS3, while the Function is powered and a cache-bypassing read of the
same Control over the bus answers 0x0.  Once the cache has been given a
value it keeps it, so the Control reports a state the device left long
ago.

Mark those Controls volatile and drop their entries from the defaults.
regcache_read() and regcache_write() skip volatile registers, so a
read-only Control no longer enters the cache at all: reads reach the
peripheral, and a sync cannot attempt to write a reading back.  That is
what regcache_sync() asks for in the first place - "any registers that
should not be synced should be marked as volatile".

The list is the same one the writeable_reg callback needs, so move it
into a helper both callbacks use.  The File Download Status is not on
it, because the host writes that Control, but it is volatile all the
same: the device raises its own bits on its own schedule, and
sdca_fdl_process() reads the Control to see them.  A cached answer would
hide the device half of the handshake.

Nothing in the driver reads any of these Controls - it performs no
register reads at all - so the change is visible only through the
regmap.

Signed-off-by: Andrey Golovko <andrey.golovko@gmail.com>
---
v2: File Download Status is no longer read-only, so mark it volatile
    on its own; its entry stays out of tas2783_reg_default[].
 sound/soc/codecs/tas2783-sdw.c | 68 ++++++++++++++--------------------
 1 file changed, 27 insertions(+), 41 deletions(-)

diff --git a/sound/soc/codecs/tas2783-sdw.c b/sound/soc/codecs/tas2783-sdw.c
index caf8fe1bf4db..f3485ce4b0c5 100644
--- a/sound/soc/codecs/tas2783-sdw.c
+++ b/sound/soc/codecs/tas2783-sdw.c
@@ -185,35 +185,17 @@ static const struct reg_default tas2783_reg_default[] = {
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_FU26, 0x01, 1), 0x1},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_FU26, 0x0b, 1), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_XU22, 0x01, 0), 0x1},
-	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_XU22, 0x06, 0), 0x0},
-	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_XU22, 0x07, 0), 0x0},
-	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_XU22, 0x08, 0), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_XU22, 0x09, 0), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_XU22, 0x0a, 0), 0x0},
-	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_CS24, 0x02, 0), 0x0},
-	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_CS21, 0x02, 0), 0x0},
-	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_CS26, 0x02, 0), 0x0},
-	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_CS28, 0x02, 0), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_PDE23, 0x1, 0), 0x3},
-	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_SAPU29, 0x05, 0), 0x0},
-	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_PPU21, 0x06, 0), 0x0},
-	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_PPU26, 0x06, 0), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_IT21, 0x04, 0), 0x0},
-	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_IT21, 0x08, 0), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_IT29, 0x04, 0), 0x0},
-	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_IT29, 0x08, 0), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_IT26, 0x04, 0), 0x0},
-	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_IT26, 0x08, 0), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_IT28, 0x04, 0), 0x0},
-	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_IT28, 0x08, 0), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_OT24, 0x04, 0), 0x0},
-	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_OT24, 0x08, 0), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_OT23, 0x04, 0), 0x0},
-	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_OT23, 0x08, 0), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_OT25, 0x04, 0), 0x0},
-	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_OT25, 0x08, 0), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_OT28, 0x04, 0), 0x0},
-	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_OT28, 0x08, 0), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_MU26, 0x01, 0), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_MU26, 0x01, 1), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_MU26, 0x01, 2), 0x0},
@@ -222,40 +204,28 @@ static const struct reg_default tas2783_reg_default[] = {
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_MU26, 0x01, 5), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_MU26, 0x01, 6), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_MU26, 0x01, 7), 0x0},
-	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_MU26, 0x06, 0), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_OT127, 0x04, 0), 0x0},
-	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_OT127, 0x08, 0), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_FU127, 0x01, 0), 0x1},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_FU127, 0x01, 1), 0x1},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_FU127, 0x01, 2), 0x1},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_FU127, 0x0b, 0), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_FU127, 0x0b, 1), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_FU127, 0x0b, 2), 0x0},
-	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_CS127, 0x02, 0), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_MFPU21, 0x01, 0), 0x0},
-	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_MFPU21, 0x04, 0), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_MFPU21, 0x05, 0), 0x1},
-	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_MFPU21, 0x08, 0), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_MFPU26, 0x01, 0), 0x0},
-	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_MFPU26, 0x04, 0), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_MFPU26, 0x05, 0), 0x1},
-	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_MFPU26, 0x08, 0), 0x0},
-	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_FU23, 0x10, 0), 0x0},
-	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_FU26, 0x10, 0), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_XU22, 0x10, 0), 0x1},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_XU22, 0x12, 0), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_XU22, 0x13, 0), 0x0},
-	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_XU22, 0x14, 0), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_XU22, 0x15, 0), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_XU22, 0x16, 0), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_CS24, 0x10, 0), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_CS21, 0x10, 0), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_CS26, 0x10, 0), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_CS28, 0x10, 0), 0x0},
-	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_PDE23, 0x10, 0), 0x3},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_UDMPU23, 0x10, 0), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_SAPU29, 0x10, 0), 0x1},
-	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_SAPU29, 0x11, 0), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_SAPU29, 0x12, 0), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_PPU21, 0x10, 0), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_PPU21, 0x11, 0), 0x0},
@@ -286,7 +256,6 @@ static const struct reg_default tas2783_reg_default[] = {
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_OT127, 0x12, 5), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_OT127, 0x12, 6), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_OT127, 0x12, 7), 0x0},
-	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_FU127, 0x10, 0), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_CS127, 0x10, 0), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_MFPU21, 0x10, 0), 0x0},
 	{SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_MFPU21, 0x11, 0), 0x0},
@@ -495,15 +464,15 @@ 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 read-only SDCA Controls the driver 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.  None of
+ * them is a setting; every one is a reading of device state.
+ */
+static bool tas2783_read_only_control(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):
@@ -538,16 +507,33 @@ static bool tas2783_writeable_register(struct device *dev, unsigned int reg)
 	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;
+		return true;
 
 	default:
-		return tas2783_sdca_mbq_size(dev, reg) > 0;
+		return false;
 	}
 }
 
+static bool tas2783_writeable_register(struct device *dev, unsigned int reg)
+{
+	if (tas2783_read_only_control(reg))
+		return false;
+
+	return tas2783_sdca_mbq_size(dev, reg) > 0;
+}
+
 static bool tas2783_volatile_register(struct device *dev, u32 reg)
 {
+	if (tas2783_read_only_control(reg))
+		return true;
+
 	switch (reg) {
+	/*
+	 * The File Download Status is a handshake: the device raises its own
+	 * bits on its own schedule and the host writes back a response, so it
+	 * is writable, but it must never be answered from the cache.
+	 */
+	case SDW_SDCA_CTL(FUNC_NUM_SMART_AMP, TAS2783_SDCA_ENT_XU22, 0x14, 0):
 	case 0x000 ... 0x080: /* Data port 0. */
 	case 0x100 ... 0x140: /* Data port 1. */
 	case 0x200 ... 0x240: /* Data port 2. */
-- 
2.53.0

Re: [PATCH v2 2/2] ASoC: tas2783-sdw: do not cache read-only Controls

[Mark Brown]

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

On Fri, Aug 14, 2026 at 09:40:00AM +0300, Andrey Golovko wrote:
> The read-only SDCA Controls are readings of device state, but the regmap
> caches them and tas2783_reg_default[] gives each one a placeholder, so a
> read is answered from the cache and never reaches the peripheral.  The

>  static bool tas2783_volatile_register(struct device *dev, u32 reg)
>  {
> +	if (tas2783_read_only_control(reg))
> +		return true;
> +

Does this work for multibyte reads?  The regmap has val_size set to 8
but some of the read only registers are multibyte.  For SoundWire
val_bits doesn't really matter so you should just be able to set that to
32.

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

Re: [PATCH v2 2/2] ASoC: tas2783-sdw: do not cache read-only Controls

[Andrey Golovko]

On Fri, Aug 14, 2026 at 04:13:42PM +0100, Mark Brown wrote:

> Does this work for multibyte reads?  The regmap has val_size set to 8
> but some of the read only registers are multibyte.  For SoundWire
> val_bits doesn't really matter so you should just be able to set that to
> 32.

It does not, thank you.  Twenty of the Controls in the list are wider
than one byte, and with val_bits = 8 the MBQ layer refuses each of them
before anything reaches the bus, so the patch on its own would replace
an answer from the cache with -EINVAL.

The widening is a patch of its own because the calibration write relied
on the old width, so I have sent both as v3 with it in front, against
for-7.3:

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

The cover letter has the list of the multi-byte Controls and what the
two patches were measured to do on the machine here.

One thing I noticed while checking the sizes, unrelated to this series:
FU21 0x10 and UDMPU23 0x06 are in the read-only list but appear nowhere
in tas2783_sdca_mbq_size(), which returns 0 for them, so
tas2783_readable_register() refuses them and they are unreachable
through the regmap in either direction.  Harmless as it stands, nothing
touches them, but the size callback looks incomplete rather than
deliberate.  I can add them, or leave that to TI along with the rest of
the reg_defaults questions I owe them a mail about.

Andrey

Re: [PATCH v2 2/2] ASoC: tas2783-sdw: do not cache read-only Controls

[Charles Keepax]

On Sat, Aug 15, 2026 at 02:45:00PM +0300, Andrey Golovko wrote:
> On Fri, Aug 14, 2026 at 04:13:42PM +0100, Mark Brown wrote:
> 
> > Does this work for multibyte reads?  The regmap has val_size set to 8
> > but some of the read only registers are multibyte.  For SoundWire
> > val_bits doesn't really matter so you should just be able to set that to
> > 32.
> 
> It does not, thank you.  Twenty of the Controls in the list are wider
> than one byte, and with val_bits = 8 the MBQ layer refuses each of them
> before anything reaches the bus, so the patch on its own would replace
> an answer from the cache with -EINVAL.
> 
> The widening is a patch of its own because the calibration write relied
> on the old width, so I have sent both as v3 with it in front, against
> for-7.3:
> 
>   https://lore.kernel.org/linux-sound/20260815112000.4180-1-andrey.golovko@gmail.com/
> 
> The cover letter has the list of the multi-byte Controls and what the
> two patches were measured to do on the machine here.
> 
> One thing I noticed while checking the sizes, unrelated to this series:
> FU21 0x10 and UDMPU23 0x06 are in the read-only list but appear nowhere
> in tas2783_sdca_mbq_size(), which returns 0 for them, so
> tas2783_readable_register() refuses them and they are unreachable
> through the regmap in either direction.  Harmless as it stands, nothing
> touches them, but the size callback looks incomplete rather than
> deliberate.  I can add them, or leave that to TI along with the rest of
> the reg_defaults questions I owe them a mail about.

One thing that is worth noting, is that sometimes in SDCA values
are DisCo constants, these are registers where the value is just
hard coded in the ACPI and typically not implemented in the
hardware. Generally, these are implemented as non-read/non-write
registers with an entry in the default array. This allows the
constant value to be read through the regmap, and driver code
generally doesn't need to care that the register is fictional.

Both of the controls you list are latency which is commonly done
as a DisCo constant, although that said it is also not a control
that is AFAIK used by any implementation so could also just have
been forgotten.

Thanks,
Charles

Re: [PATCH v2 2/2] ASoC: tas2783-sdw: do not cache read-only Controls

[Andrey Golovko]

On Tue, Aug 18, 2026 at 09:16:19AM +0100, Charles Keepax wrote:

> One thing that is worth noting, is that sometimes in SDCA values
> are DisCo constants, these are registers where the value is just
> hard coded in the ACPI and typically not implemented in the
> hardware. Generally, these are implemented as non-read/non-write
> registers with an entry in the default array. This allows the
> constant value to be read through the regmap, and driver code
> generally doesn't need to care that the register is fictional.

Thank you - that sent me to the ACPI, and what is there says something
about the whole read-only list rather than only those two Controls.

The Function is described in DisCo on this machine, in some detail: both
amplifiers carry an identical Function node with 27 Entities and their
Controls.  The driver ignores all of it and works from its own tables,
so the two descriptions can be compared.  They disagree, and they
disagree in a way that lines up with what the peripheral answers.

tas2783_read_only_control() names 33 Controls.  DisCo declares seven of
them: the Latency of FU21, the Clock Valid of CS21, CS25, CS26 and CS28,
the Actual Power State of PDE23 and the Protection Status of SAPU29.
For eighteen the Entity is described but the Control is not - that is
every remaining Latency, and the XU_ID and XU_Version of XU22.  The last
eight belong to six Entities that do not appear in DisCo at all: CS24,
OT24, OT127, CS127, MFPU21 and MFPU26.

The three of those I have read off the bus agree with the description.
PDE23 Actual PS answers 0x0 and SAPU29 Protection Status 0x3 on both
amplifiers, both declared.  XU22 XU_ID answers COMMAND_IGNORED, powered
or not, and is not declared.  (XU22 0x12 answers COMMAND_IGNORED too and
is declared, so the correspondence is not perfect, but for the read-only
list it holds so far.)

So of the 21 Latency Controls in that list, DisCo describes exactly one:
FU21 0x10.  Which is one of the two I wrote to you about, the ones
tas2783_sdca_mbq_size() has no case for and which are therefore
unreachable through the regmap.  The single Latency the platform
actually describes is the one the driver cannot read, while twenty
Latencies that nothing describes are declared four bytes wide and
answered from a 0x0 placeholder in the cache.

That reads to me as two separate things rather than one, and the DSDT
settles the first.  FU21 0x10 is declared with access mode 0x05 - DC,
your DisCo constant - and mipi-sdca-control-dc-value 0x0.  So it is
exactly the case you described: fictional in the hardware, constant in
the ACPI, and it wants a default-array entry with no readable or
writeable case rather than a size case.  The other twenty Latencies have
no DisCo entry at all to be constants of.

Reading the access modes for the whole Function turned up the rest of
the disagreement, which runs in both directions.  Of the seven Controls
the driver's read-only list and DisCo have in common, six are declared
RO - the four Clock Valids, PDE23 Actual PS, SAPU29 Protection Status -
and the seventh is that DC.  Meanwhile DisCo marks as DC or RO several
Controls the driver treats as ordinary writable registers:

  XU22 0x09 and 0x0a  DC, dc-value 0x0   (the two Selectors SDCA does
                                          not define - so they are
                                          constants, not registers)
  UDMPU23 0x10        DC, dc-value 0x1   (driver default says 0x0)
  SAPU29 0x10         RO                 (Protection Mode)
  XU22 0x15           RO
  IT21 and IT26 0x10 and 0x11, IT28 0x11, OT25 and OT28 0x11,
  FU127 0x01, 0x04 and 0x05, MU26 0x01      all DC

None of that is reachable from the read-only series as posted, and I am
not proposing to chase it there.  But it does mean the list 2/2 acts on
was assembled independently of what the platform says, and agrees with
it in six places out of thirty-three.

So for the series I would rather narrow than guess: keep 2/2 for the six
Controls DisCo declares RO, which are also the ones I have measured
answering real values off the bus, and leave everything else - the
constants, the phantom Latencies, the writable-but-RO ones - to TI,
whose mail goes out today.  FU21 0x10 would then be a separate small
patch giving it the DC treatment, once TI confirm the driver should
follow this platform's description.

Would you rather review that, or have me hold 2/2 entirely until TI
answer and send only 1/2, which stands on its own?

Andrey

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

[Andrey Golovko]

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 and version.  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 file 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>
---
v2: dropped the XU22 File Download Status (0x14) from the list; Mark
    pointed out that the host writes it.  Reworded the changelog to
    match.
 sound/soc/codecs/tas2783-sdw.c | 51 ++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/sound/soc/codecs/tas2783-sdw.c b/sound/soc/codecs/tas2783-sdw.c
index c217da5fccdf..caf8fe1bf4db 100644
--- a/sound/soc/codecs/tas2783-sdw.c
+++ b/sound/soc/codecs/tas2783-sdw.c
@@ -495,6 +495,56 @@ 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_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 +566,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: (subset) [PATCH v2 0/2] ASoC: tas2783-sdw: fix the read-only Control handling

[Mark Brown]

On Fri, 14 Aug 2026 09:40:00 +0300, Andrey Golovko wrote:
> ASoC: tas2783-sdw: fix the read-only Control handling
> 
> The two read-only Control patches from 13 August, resent together as a
> series because the second one depends on the first, and with the change
> Mark asked for.
> 
> He pointed out that the XU22 Control 0x14 in the read-only list is
> SDCA_CTL_XU_FDL_STATUS, which sdca_fdl_process() writes.  He is right,
> and the description I gave it - "firmware download status" - reads like
> a status the device owns, which it is not.  It is a handshake: the
> device sets the bits it owns, and the host writes its response back into
> the same Control, keeping the device bits, which is exactly what
> sdca_fdl_process() does with response | (status & ~SDCA_CTL_XU_FDLH_MASK).
> So it is out of the list in 1/2.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-7.3

Thanks!

[1/2] ASoC: tas2783-sdw: do not treat read-only Controls as writable
      https://git.kernel.org/broonie/sound/c/0c7aeb0f5ece

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark