From: Richard Fitzgerald <rf@opensource.cirrus.com>
To: broonie@kernel.org
Cc: linux-sound@vger.kernel.org, linux-kernel@vger.kernel.org,
patches@opensource.cirrus.com
Subject: [PATCH 2/2] ASoC: cs35l56: Fix unwanted writes to ASP config registers
Date: Fri, 13 Nov 2026 11:29:35 +0000 [thread overview]
Message-ID: <20261113112935.2926431-3-rf@opensource.cirrus.com> (raw)
In-Reply-To: <20261113112935.2926431-1-rf@opensource.cirrus.com>
Remove the ASP config registers from the regmap defaults table and use
regmap_multi_reg_write() instead of regmap_register_patch() to write
default values. Use custom event handlers for the ASP1 widgets so
ASP1_ENABLES1 is only written if the ASP is used. Move the HIZ mode
write to ASP1_CONTROL3 into the register sequence instead of writing
it on every driver probe.
This is to prevent accidentally writing to these registers when the driver
does not own them.
The ASP config registers have multiple ownership. They can be owned and
written by the firmware on SKUs that use the ASP as a chip-to-chip audio
interface managed by the firmware. They can be owned and written by an MCU
through one of the other control ports. Or they can be owned by the ASoC
codec driver if they are used for the CPU->amp audio. Because of this
multiple ownership the codec driver must not write the registers if it
doesn't own control of the ASP.
Commit 9351cf3fd92dc ("ASoC: cs35l56: Only patch ASP registers if the DAI
is part of a DAIlink") attempted to deal with this by only writing default
values into the cache if the dai probe() callback was called (which only
happens when the DAI is part of a dailink). The intention was that the
cache values would only be populated if the driver owns the DAI. But this
commit was flawed in three ways:
1. A regcache_sync() without a regcache_mark_dirty() would write the
registers.
The cs35l56/57/62/63 amps are designed to preserve state across suspend
states, so the registers have not necessarily reset when the driver
resumes. For this reason the driver may call regcache_sync() without
regcache_mark_dirty() to sync any changes made while in cache-only.
Regmap pre-populates the cache from the values in the defaults table. So
the cache always contained values for these registers. The regmap cache
doesn't have a per-register dirty flag, only a global dirty. It will write
out everything in the cache if it is dirty. Filtering of cache values that
are at default only happens if regcache_mark_dirty() has been called. So
the ASP config registers were being written for every non-default cache
sync.
2. regmap_register_patch() writes to hardware
The intent was to populate the cache. But regmap_register_patch() disables
cache-only and tries to write the patch to hardware. The DAI probe() can
be called while in runtime-suspend with the hardware not accessible, so
this could fail. There is no need to write the registers to hardware at
this time, the only need is to populate the cache so that the register
settings can be accumulated in the cache as the DAI is configured.
3. ASP1_ENABLES1 was read into the cache during widget initialization
The ASP1 RX and TX widgets provided the address of ASP1_ENABLES1 for ASoC
to manage. But during widget initialization ASoC reads the initial value
of the registers. This meant that the cache was populated with a value
for ASP1_ENABLES1, so that would be written out by regcache_sync().
Fixes: 9351cf3fd92dc ("ASoC: cs35l56: Only patch ASP registers if the DAI is part of a DAIlink")
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
---
include/sound/cs35l56.h | 2 +-
sound/soc/codecs/cs35l56-shared.c | 26 +++----------
sound/soc/codecs/cs35l56.c | 63 ++++++++++++++++++++-----------
3 files changed, 48 insertions(+), 43 deletions(-)
diff --git a/include/sound/cs35l56.h b/include/sound/cs35l56.h
index e1b098d1d146..362dfa2761bc 100644
--- a/include/sound/cs35l56.h
+++ b/include/sound/cs35l56.h
@@ -412,7 +412,7 @@ extern const char * const cs35l56_cal_set_status_text[3];
extern const char * const cs35l56_tx_input_texts[CS35L56_NUM_INPUT_SRC];
extern const unsigned int cs35l56_tx_input_values[CS35L56_NUM_INPUT_SRC];
-int cs35l56_set_asp_patch(struct cs35l56_base *cs35l56_base);
+int cs35l56_set_asp_defaults(struct cs35l56_base *cs35l56_base);
int cs35l56_set_patch(struct cs35l56_base *cs35l56_base);
int cs35l56_mbox_send(struct cs35l56_base *cs35l56_base, unsigned int command);
int cs35l56_firmware_shutdown(struct cs35l56_base *cs35l56_base);
diff --git a/sound/soc/codecs/cs35l56-shared.c b/sound/soc/codecs/cs35l56-shared.c
index c44bf8bc4e0c..4d76f1777927 100644
--- a/sound/soc/codecs/cs35l56-shared.c
+++ b/sound/soc/codecs/cs35l56-shared.c
@@ -34,19 +34,19 @@ static const struct reg_sequence cs35l56_asp_patch[] = {
{ CS35L56_ASP1_ENABLES1, 0x00000000 },
{ CS35L56_ASP1_CONTROL1, 0x00000028 },
{ CS35L56_ASP1_CONTROL2, 0x18180200 },
- { CS35L56_ASP1_CONTROL3, 0x00000002 },
+ { CS35L56_ASP1_CONTROL3, 0x00000003 },
{ CS35L56_ASP1_FRAME_CONTROL1, 0x03020100 },
{ CS35L56_ASP1_FRAME_CONTROL5, 0x00020100 },
{ CS35L56_ASP1_DATA_CONTROL1, 0x00000018 },
{ CS35L56_ASP1_DATA_CONTROL5, 0x00000018 },
};
-int cs35l56_set_asp_patch(struct cs35l56_base *cs35l56_base)
+int cs35l56_set_asp_defaults(struct cs35l56_base *cs35l56_base)
{
- return regmap_register_patch(cs35l56_base->regmap, cs35l56_asp_patch,
- ARRAY_SIZE(cs35l56_asp_patch));
+ return regmap_multi_reg_write(cs35l56_base->regmap, cs35l56_asp_patch,
+ ARRAY_SIZE(cs35l56_asp_patch));
}
-EXPORT_SYMBOL_NS_GPL(cs35l56_set_asp_patch, "SND_SOC_CS35L56_SHARED");
+EXPORT_SYMBOL_NS_GPL(cs35l56_set_asp_defaults, "SND_SOC_CS35L56_SHARED");
static const struct reg_sequence cs35l56_patch[] = {
/*
@@ -105,14 +105,6 @@ int cs35l56_set_patch(struct cs35l56_base *cs35l56_base)
EXPORT_SYMBOL_NS_GPL(cs35l56_set_patch, "SND_SOC_CS35L56_SHARED");
static const struct reg_default cs35l56_reg_defaults[] = {
- { CS35L56_ASP1_ENABLES1, 0x00000000 },
- { CS35L56_ASP1_CONTROL1, 0x00000028 },
- { CS35L56_ASP1_CONTROL2, 0x18180200 },
- { CS35L56_ASP1_CONTROL3, 0x00000002 },
- { CS35L56_ASP1_FRAME_CONTROL1, 0x03020100 },
- { CS35L56_ASP1_FRAME_CONTROL5, 0x00020100 },
- { CS35L56_ASP1_DATA_CONTROL1, 0x00000018 },
- { CS35L56_ASP1_DATA_CONTROL5, 0x00000018 },
{ CS35L56_SWIRE_DP3_CH1_INPUT, 0x00000018 },
{ CS35L56_SWIRE_DP3_CH2_INPUT, 0x00000019 },
{ CS35L56_SWIRE_DP3_CH3_INPUT, 0x00000029 },
@@ -129,14 +121,6 @@ static const struct reg_default cs35l56_reg_defaults[] = {
};
static const struct reg_default cs35l63_reg_defaults[] = {
- { CS35L56_ASP1_ENABLES1, 0x00000000 },
- { CS35L56_ASP1_CONTROL1, 0x00000028 },
- { CS35L56_ASP1_CONTROL2, 0x18180200 },
- { CS35L56_ASP1_CONTROL3, 0x00000002 },
- { CS35L56_ASP1_FRAME_CONTROL1, 0x03020100 },
- { CS35L56_ASP1_FRAME_CONTROL5, 0x00020100 },
- { CS35L56_ASP1_DATA_CONTROL1, 0x00000018 },
- { CS35L56_ASP1_DATA_CONTROL5, 0x00000018 },
{ CS35L56_SWIRE_DP3_CH1_INPUT, 0x00000018 },
{ CS35L56_SWIRE_DP3_CH2_INPUT, 0x00000019 },
{ CS35L56_SWIRE_DP3_CH3_INPUT, 0x00000029 },
diff --git a/sound/soc/codecs/cs35l56.c b/sound/soc/codecs/cs35l56.c
index 852491f8340b..ded8d1cdfd24 100644
--- a/sound/soc/codecs/cs35l56.c
+++ b/sound/soc/codecs/cs35l56.c
@@ -332,14 +332,38 @@ static int cs35l56_play_event(struct snd_soc_dapm_widget *w,
}
}
+static int cs35l56_asp_event(struct snd_soc_dapm_widget *w,
+ struct snd_kcontrol *kcontrol, int event)
+{
+ struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm);
+ struct cs35l56_private *cs35l56 = snd_soc_component_get_drvdata(component);
+
+ return regmap_assign_bits(cs35l56->base.regmap, CS35L56_ASP1_ENABLES1,
+ BIT(w->shift),
+ SND_SOC_DAPM_EVENT_ON(event));
+}
+
static int cs35l56_asp_tx_event(struct snd_soc_dapm_widget *w,
struct snd_kcontrol *kcontrol, int event)
{
struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm);
struct cs35l56_private *cs35l56 = snd_soc_component_get_drvdata(component);
+ int ret;
- return regmap_write(cs35l56->base.regmap, CS35L56_ASP1TX1_INPUT + (4 * w->shift),
- cs35l56->asp_tx_input[w->shift]);
+ switch (event) {
+ case SND_SOC_DAPM_PRE_PMU:
+ ret = regmap_write(cs35l56->base.regmap,
+ CS35L56_ASP1TX1_INPUT + (4 * w->shift),
+ cs35l56->asp_tx_input[w->shift]);
+ if (ret)
+ return ret;
+
+ break;
+ default:
+ break;
+ }
+
+ return cs35l56_asp_event(w, kcontrol, event);
}
static const struct snd_soc_dapm_widget cs35l56_dapm_widgets[] = {
@@ -355,22 +379,25 @@ static const struct snd_soc_dapm_widget cs35l56_dapm_widgets[] = {
SND_SOC_DAPM_PGA_E("DSP1", SND_SOC_NOPM, 0, 0, NULL, 0, cs35l56_dsp_event,
SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD),
- SND_SOC_DAPM_AIF_IN("ASP1RX1", NULL, 0, CS35L56_ASP1_ENABLES1,
- CS35L56_ASP_RX1_EN_SHIFT, 0),
- SND_SOC_DAPM_AIF_IN("ASP1RX2", NULL, 1, CS35L56_ASP1_ENABLES1,
- CS35L56_ASP_RX2_EN_SHIFT, 0),
- SND_SOC_DAPM_AIF_OUT_E("ASP1TX1", NULL, 0, CS35L56_ASP1_ENABLES1,
+ /* Avoid reading ASP1_ENABLES1 when DAPM initializes the widgets. */
+ SND_SOC_DAPM_AIF_IN_E("ASP1RX1", NULL, 0, SND_SOC_NOPM,
+ CS35L56_ASP_RX1_EN_SHIFT, 0,
+ cs35l56_asp_event, SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
+ SND_SOC_DAPM_AIF_IN_E("ASP1RX2", NULL, 1, SND_SOC_NOPM,
+ CS35L56_ASP_RX2_EN_SHIFT, 0,
+ cs35l56_asp_event, SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
+ SND_SOC_DAPM_AIF_OUT_E("ASP1TX1", NULL, 0, SND_SOC_NOPM,
CS35L56_ASP_TX1_EN_SHIFT, 0,
- cs35l56_asp_tx_event, SND_SOC_DAPM_PRE_PMU),
- SND_SOC_DAPM_AIF_OUT_E("ASP1TX2", NULL, 1, CS35L56_ASP1_ENABLES1,
+ cs35l56_asp_tx_event, SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
+ SND_SOC_DAPM_AIF_OUT_E("ASP1TX2", NULL, 1, SND_SOC_NOPM,
CS35L56_ASP_TX2_EN_SHIFT, 0,
- cs35l56_asp_tx_event, SND_SOC_DAPM_PRE_PMU),
- SND_SOC_DAPM_AIF_OUT_E("ASP1TX3", NULL, 2, CS35L56_ASP1_ENABLES1,
+ cs35l56_asp_tx_event, SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
+ SND_SOC_DAPM_AIF_OUT_E("ASP1TX3", NULL, 2, SND_SOC_NOPM,
CS35L56_ASP_TX3_EN_SHIFT, 0,
- cs35l56_asp_tx_event, SND_SOC_DAPM_PRE_PMU),
- SND_SOC_DAPM_AIF_OUT_E("ASP1TX4", NULL, 3, CS35L56_ASP1_ENABLES1,
+ cs35l56_asp_tx_event, SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
+ SND_SOC_DAPM_AIF_OUT_E("ASP1TX4", NULL, 3, SND_SOC_NOPM,
CS35L56_ASP_TX4_EN_SHIFT, 0,
- cs35l56_asp_tx_event, SND_SOC_DAPM_PRE_PMU),
+ cs35l56_asp_tx_event, SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
SND_SOC_DAPM_MUX("ASP1 TX1 Source", SND_SOC_NOPM, 0, 0, &asp1_tx1_mux),
SND_SOC_DAPM_MUX("ASP1 TX2 Source", SND_SOC_NOPM, 0, 0, &asp1_tx2_mux),
@@ -473,7 +500,7 @@ static int cs35l56_asp_dai_probe(struct snd_soc_dai *codec_dai)
{
struct cs35l56_private *cs35l56 = snd_soc_component_get_drvdata(codec_dai->component);
- return cs35l56_set_asp_patch(&cs35l56->base);
+ return cs35l56_set_asp_defaults(&cs35l56->base);
}
static int cs35l56_asp_dai_set_fmt(struct snd_soc_dai *codec_dai, unsigned int fmt)
@@ -2315,12 +2342,6 @@ int cs35l56_init(struct cs35l56_private *cs35l56)
/* Registers could be dirty after soft reset or SoundWire enumeration */
regcache_sync(cs35l56->base.regmap);
- /* Set ASP1 DOUT to high-impedance when it is not transmitting audio data. */
- ret = regmap_set_bits(cs35l56->base.regmap, CS35L56_ASP1_CONTROL3,
- CS35L56_ASP1_DOUT_HIZ_CTRL_MASK);
- if (ret)
- return dev_err_probe(cs35l56->base.dev, ret, "Failed to write ASP1_CONTROL3\n");
-
cs35l56->needs_wait_for_fw_idle =
cs35l56_needs_wait_for_firmware_timer_expiry(&cs35l56->base);
--
2.47.3
prev parent reply other threads:[~2026-11-13 11:34 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-11-13 11:29 [PATCH 0/2] ASoC: cs35l56: Series to stop writing ASP registers the driver doesn't own Richard Fitzgerald
2026-11-13 11:29 ` [PATCH 1/2] ASoC: cs35l56: Only write ASP1TXn registers when widget is powered-up Richard Fitzgerald
2026-11-13 11:29 ` Richard Fitzgerald [this message]
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=20261113112935.2926431-3-rf@opensource.cirrus.com \
--to=rf@opensource.cirrus.com \
--cc=broonie@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-sound@vger.kernel.org \
--cc=patches@opensource.cirrus.com \
/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®