mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Andrey Golovko <andrey.golovko@gmail.com>
To: Shenghao Ding <shenghao-ding@ti.com>, Kevin Lu <kevin-lu@ti.com>,
	Baojun Xu <baojun.xu@ti.com>, Sen Wang <sen@ti.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>, Jaroslav Kysela <perex@perex.cz>,
	Takashi Iwai <tiwai@suse.com>
Cc: "Holalu Yogendra, Niranjan" <niranjan.hy@ti.com>,
	Pierre-Louis Bossart <pierre-louis.bossart@linux.dev>,
	Charles Keepax <ckeepax@opensource.cirrus.com>,
	Vijendar Mukunda <Vijendar.Mukunda@amd.com>,
	Antoine Monnet <antoine@montane.tech>,
	Robin Everaars <robineveraars@pm.me>,
	Ville Saarinen <wiza@saarinenkoti.fi>,
	linux-sound@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH v3 1/2] ASoC: tas2783-sdw: make multi-byte MBQ registers reachable
Date: Sat, 15 Aug 2026 14:40:00 +0300	[thread overview]
Message-ID: <20260815112000.4180-2-andrey.golovko@gmail.com> (raw)
In-Reply-To: <20260815112000.4180-1-andrey.golovko@gmail.com>

tas2783_sdca_mbq_size() declares registers of one, two and four bytes,
but the regmap is created with val_bits = 8.  The SoundWire MBQ bus
rejects any register whose declared size exceeds the configured value
width:

	ctx->val_size = config->val_bits / BITS_PER_BYTE;
	...
	size = ctx->cfg.mbq_size(ctx->dev, reg);
	if (!size || size > ctx->val_size)
		return -EINVAL;

Both regmap_sdw_mbq_read() and regmap_sdw_mbq_write() bail out on that,
so every register the driver itself declares as two or four bytes wide
is unreachable through the regmap, in both directions, and the failure
happens before anything reaches the bus.

That is measurable.  On an ASUS ProArt PX13 HN7306EAC with two TAS2783
aggregated on one link, a regmap read of XU22 Control 0x07, declared two
bytes, and of XU22 Control 0x12, declared four bytes, returns -EINVAL on
both amplifiers.  With val_bits widened the same reads reach the
peripheral, which answers -ENODATA for both, powered or not - this part
appears not to implement them.  So the change makes the registers
addressable; whether a given peripheral answers is up to the peripheral.

Widen val_bits to cover the largest size the callback returns.  The
callback keeps deciding the size of each individual transfer, so
single-byte registers are unaffected.

The calibration path needs adjusting along with it.
regmap_bulk_write(..., buf, sizeof(u32)) relied on val_bits = 8 to
spread one calibration value over four consecutive single-byte
registers; with a four-byte value width the same call would ask for four
four-byte values and read past the end of the four-byte buffer.  Write
the four registers explicitly instead, keeping the most-significant-byte
-first layout.  Read back over the bus, bypassing the cache, the five
calibration values of both amplifiers are byte for byte what they were
before this patch.

What this does not do is repair regcache_sync().  On this machine a sync
aborts earlier, at a single-byte Control - the FU23 Mute of channel 0,
which tas2783_reg_default[] gives 0x1 while the driver's init sequence
writes 0x00, so the sync tries to restore it and the peripheral answers
-ENODATA - and no multi-byte register is reached at all.

Signed-off-by: Andrey Golovko <andrey.golovko@gmail.com>
---
v3: new in this revision.  Written on 28 July, held back until the
    read-only Control patches settled; it is the answer to Mark's
    question on v2 2/2, which cannot stand without it.
 sound/soc/codecs/tas2783-sdw.c | 37 +++++++++++++++++++++++++---------
 1 file changed, 28 insertions(+), 9 deletions(-)

diff --git a/sound/soc/codecs/tas2783-sdw.c b/sound/soc/codecs/tas2783-sdw.c
index caf8fe1bf4db..b657ad031306 100644
--- a/sound/soc/codecs/tas2783-sdw.c
+++ b/sound/soc/codecs/tas2783-sdw.c
@@ -564,7 +564,13 @@ static bool tas2783_volatile_register(struct device *dev, u32 reg)
 
 static const struct regmap_config tas_regmap = {
 	.reg_bits = 32,
-	.val_bits = 8,
+	/*
+	 * tas2783_sdca_mbq_size() declares registers of one, two and four
+	 * bytes. val_bits has to cover the widest of those: the SoundWire MBQ
+	 * bus rejects any register whose declared size exceeds val_bits, which
+	 * would make every multi-byte register unreachable in both directions.
+	 */
+	.val_bits = 32,
 	.readable_reg = tas2783_readable_register,
 	.writeable_reg = tas2783_writeable_register,
 	.volatile_reg = tas2783_volatile_register,
@@ -667,9 +673,9 @@ static s32 tas2783_validate_calibdata(struct tas2783_prv *tas_dev,
 
 static void tas2783_set_calib_params_to_device(struct tas2783_prv *tas_dev, u32 *cali_data)
 {
-	u32 dev_count, offset, i, device_num;
+	u32 dev_count, offset, i, j, device_num;
 	u32 reg_value;
-	u8 buf[4];
+	s32 ret;
 
 	dev_count = cali_data[1];
 	offset = 3;
@@ -683,12 +689,25 @@ static void tas2783_set_calib_params_to_device(struct tas2783_prv *tas_dev, u32
 
 		for (i = 0; i < ARRAY_SIZE(tas2783_cali_reg); i++) {
 			reg_value = cali_data[offset + i];
-			buf[0] = reg_value >> 24;
-			buf[1] = reg_value >> 16;
-			buf[2] = reg_value >> 8;
-			buf[3] = reg_value & 0xff;
-			regmap_bulk_write(tas_dev->regmap, tas2783_cali_reg[i],
-					  buf, sizeof(u32));
+
+			/*
+			 * A calibration value occupies four consecutive
+			 * single-byte registers, most significant byte first.
+			 * Write them one by one: these registers are declared
+			 * as one byte wide by tas2783_sdca_mbq_size(), so a
+			 * multi-byte regmap access would not match the layout.
+			 */
+			for (j = 0; j < sizeof(u32); j++) {
+				ret = regmap_write(tas_dev->regmap,
+						   tas2783_cali_reg[i] + j,
+						   (reg_value >> (24 - 8 * j)) & 0xff);
+				if (ret) {
+					dev_err(tas_dev->dev,
+						"calib write to 0x%x failed, err=%d\n",
+						tas2783_cali_reg[i] + j, ret);
+					break;
+				}
+			}
 		}
 		break;
 	}
-- 
2.53.0


  parent reply	other threads:[~2026-08-15 11:37 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-15 11:40 [PATCH v3 0/2] ASoC: tas2783-sdw: fix the read-only Control handling Andrey Golovko
2026-08-15 11:40 ` [PATCH v3 2/2] ASoC: tas2783-sdw: do not cache read-only Controls Andrey Golovko
2026-08-15 11:40 ` Andrey Golovko [this message]
2026-09-09 12:17 ` [PATCH v3 0/2] ASoC: tas2783-sdw: fix the read-only Control handling Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260815112000.4180-2-andrey.golovko@gmail.com \
    --to=andrey.golovko@gmail.com \
    --cc=Vijendar.Mukunda@amd.com \
    --cc=antoine@montane.tech \
    --cc=baojun.xu@ti.com \
    --cc=broonie@kernel.org \
    --cc=ckeepax@opensource.cirrus.com \
    --cc=kevin-lu@ti.com \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sound@vger.kernel.org \
    --cc=niranjan.hy@ti.com \
    --cc=perex@perex.cz \
    --cc=pierre-louis.bossart@linux.dev \
    --cc=robineveraars@pm.me \
    --cc=sen@ti.com \
    --cc=shenghao-ding@ti.com \
    --cc=tiwai@suse.com \
    --cc=wiza@saarinenkoti.fi \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®