Skip to content

Commit 5d7e328

Browse files
rfvirgilbroonie
authored andcommitted
ASoC: cs35l56: Revert support for dual-ownership of ASP registers
This patch reverts a series of commits that allowed for the ASP registers to be owned by either the driver or the firmware. Nothing currently depends on the functionality that is being reverted, so it is safe to remove. The commits being reverted are (last 3 are bugfixes to the first 2): commit 72a77d7 ("ASoC: cs35l56: Fix to ensure ASP1 registers match cache") commit 07f7d6e ("ASoC: cs35l56: Fix for initializing ASP1 mixer registers") commit 4703b01 ("ASoC: cs35l56: fix reversed if statement in cs35l56_dspwait_asp1tx_put()") commit c14f09f ("ASoC: cs35l56: Fix deadlock in ASP1 mixer register initialization") commit dfd2ffb ("ASoC: cs35l56: Prevent overwriting firmware ASP config") These reverts have been squashed into a single commit because there would be no reason to revert only some of them (which would just reintroduce bugs). The changes introduced by the commits were well-intentioned but somewhat misguided. ACPI does not provide any information about how audio hardware is linked together, so that information has to be hardcoded into drivers. On Windows the firmware is customized to statically setup appropriate configuration of the audio links, and the intent of the commits was to re-use this information if the Linux host drivers aren't taking control of the ASP. This would avoid having to hardcode the ASP config into the machine driver on some systems. However, this added complexity and race conditions into the driver. It also complicates implementation of new code. The only case where the ASP is used but the host is not taking ownership is when CS35L56 is used in SoundWire mode with the ASP as a reference audio interconnect. But even in that case it's not necessarily required even if the firmware initialized it. Typically it is used to avoid the host SDCA drivers having to be capable of aggregating capture paths from multiple SoundWire peripherals. But the SOF SoundWire support is capable of doing that aggregation. Reverting all these commits significantly simplifies the driver. Let's just use the normal Linux mechanisms of the machine driver and ALSA controls to set things up instead of trying to use the firmware to do use-case setup. Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com> Link: https://patch.msgid.link/20240701104444.172556-2-rf@opensource.cirrus.com Signed-off-by: Mark Brown <broonie@kernel.org>
1 parent 4235c80 commit 5d7e328

File tree

4 files changed

+43
-273
lines changed

4 files changed

+43
-273
lines changed

include/sound/cs35l56.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -267,13 +267,18 @@ struct cs35l56_base {
267267
bool fw_patched;
268268
bool secured;
269269
bool can_hibernate;
270-
bool fw_owns_asp1;
271270
bool cal_data_valid;
272271
s8 cal_index;
273272
struct cirrus_amp_cal_data cal_data;
274273
struct gpio_desc *reset_gpio;
275274
};
276275

276+
/* Temporary to avoid a build break with the HDA driver */
277+
static inline int cs35l56_force_sync_asp1_registers_from_cache(struct cs35l56_base *cs35l56_base)
278+
{
279+
return 0;
280+
}
281+
277282
extern struct regmap_config cs35l56_regmap_i2c;
278283
extern struct regmap_config cs35l56_regmap_spi;
279284
extern struct regmap_config cs35l56_regmap_sdw;
@@ -284,8 +289,6 @@ extern const char * const cs35l56_tx_input_texts[CS35L56_NUM_INPUT_SRC];
284289
extern const unsigned int cs35l56_tx_input_values[CS35L56_NUM_INPUT_SRC];
285290

286291
int cs35l56_set_patch(struct cs35l56_base *cs35l56_base);
287-
int cs35l56_init_asp1_regs_for_driver_control(struct cs35l56_base *cs35l56_base);
288-
int cs35l56_force_sync_asp1_registers_from_cache(struct cs35l56_base *cs35l56_base);
289292
int cs35l56_mbox_send(struct cs35l56_base *cs35l56_base, unsigned int command);
290293
int cs35l56_firmware_shutdown(struct cs35l56_base *cs35l56_base);
291294
int cs35l56_wait_for_firmware_boot(struct cs35l56_base *cs35l56_base);

sound/soc/codecs/cs35l56-shared.c

Lines changed: 24 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,18 @@ static const struct reg_sequence cs35l56_patch[] = {
2020
* Firmware can change these to non-defaults to satisfy SDCA.
2121
* Ensure that they are at known defaults.
2222
*/
23+
{ CS35L56_ASP1_ENABLES1, 0x00000000 },
24+
{ CS35L56_ASP1_CONTROL1, 0x00000028 },
25+
{ CS35L56_ASP1_CONTROL2, 0x18180200 },
26+
{ CS35L56_ASP1_CONTROL3, 0x00000002 },
27+
{ CS35L56_ASP1_FRAME_CONTROL1, 0x03020100 },
28+
{ CS35L56_ASP1_FRAME_CONTROL5, 0x00020100 },
29+
{ CS35L56_ASP1_DATA_CONTROL1, 0x00000018 },
30+
{ CS35L56_ASP1_DATA_CONTROL5, 0x00000018 },
31+
{ CS35L56_ASP1TX1_INPUT, 0x00000000 },
32+
{ CS35L56_ASP1TX2_INPUT, 0x00000000 },
33+
{ CS35L56_ASP1TX3_INPUT, 0x00000000 },
34+
{ CS35L56_ASP1TX4_INPUT, 0x00000000 },
2335
{ CS35L56_SWIRE_DP3_CH1_INPUT, 0x00000018 },
2436
{ CS35L56_SWIRE_DP3_CH2_INPUT, 0x00000019 },
2537
{ CS35L56_SWIRE_DP3_CH3_INPUT, 0x00000029 },
@@ -41,12 +53,18 @@ EXPORT_SYMBOL_NS_GPL(cs35l56_set_patch, SND_SOC_CS35L56_SHARED);
4153
static const struct reg_default cs35l56_reg_defaults[] = {
4254
/* no defaults for OTP_MEM - first read populates cache */
4355

44-
/*
45-
* No defaults for ASP1 control or ASP1TX mixer. See
46-
* cs35l56_populate_asp1_register_defaults() and
47-
* cs35l56_sync_asp1_mixer_widgets_with_firmware().
48-
*/
49-
56+
{ CS35L56_ASP1_ENABLES1, 0x00000000 },
57+
{ CS35L56_ASP1_CONTROL1, 0x00000028 },
58+
{ CS35L56_ASP1_CONTROL2, 0x18180200 },
59+
{ CS35L56_ASP1_CONTROL3, 0x00000002 },
60+
{ CS35L56_ASP1_FRAME_CONTROL1, 0x03020100 },
61+
{ CS35L56_ASP1_FRAME_CONTROL5, 0x00020100 },
62+
{ CS35L56_ASP1_DATA_CONTROL1, 0x00000018 },
63+
{ CS35L56_ASP1_DATA_CONTROL5, 0x00000018 },
64+
{ CS35L56_ASP1TX1_INPUT, 0x00000000 },
65+
{ CS35L56_ASP1TX2_INPUT, 0x00000000 },
66+
{ CS35L56_ASP1TX3_INPUT, 0x00000000 },
67+
{ CS35L56_ASP1TX4_INPUT, 0x00000000 },
5068
{ CS35L56_SWIRE_DP3_CH1_INPUT, 0x00000018 },
5169
{ CS35L56_SWIRE_DP3_CH2_INPUT, 0x00000019 },
5270
{ CS35L56_SWIRE_DP3_CH3_INPUT, 0x00000029 },
@@ -206,77 +224,6 @@ static bool cs35l56_volatile_reg(struct device *dev, unsigned int reg)
206224
}
207225
}
208226

209-
static const struct reg_sequence cs35l56_asp1_defaults[] = {
210-
REG_SEQ0(CS35L56_ASP1_ENABLES1, 0x00000000),
211-
REG_SEQ0(CS35L56_ASP1_CONTROL1, 0x00000028),
212-
REG_SEQ0(CS35L56_ASP1_CONTROL2, 0x18180200),
213-
REG_SEQ0(CS35L56_ASP1_CONTROL3, 0x00000002),
214-
REG_SEQ0(CS35L56_ASP1_FRAME_CONTROL1, 0x03020100),
215-
REG_SEQ0(CS35L56_ASP1_FRAME_CONTROL5, 0x00020100),
216-
REG_SEQ0(CS35L56_ASP1_DATA_CONTROL1, 0x00000018),
217-
REG_SEQ0(CS35L56_ASP1_DATA_CONTROL5, 0x00000018),
218-
REG_SEQ0(CS35L56_ASP1TX1_INPUT, 0x00000000),
219-
REG_SEQ0(CS35L56_ASP1TX2_INPUT, 0x00000000),
220-
REG_SEQ0(CS35L56_ASP1TX3_INPUT, 0x00000000),
221-
REG_SEQ0(CS35L56_ASP1TX4_INPUT, 0x00000000),
222-
};
223-
224-
/*
225-
* The firmware can have control of the ASP so we don't provide regmap
226-
* with defaults for these registers, to prevent a regcache_sync() from
227-
* overwriting the firmware settings. But if the machine driver hooks up
228-
* the ASP it means the driver is taking control of the ASP, so then the
229-
* registers are populated with the defaults.
230-
*/
231-
int cs35l56_init_asp1_regs_for_driver_control(struct cs35l56_base *cs35l56_base)
232-
{
233-
if (!cs35l56_base->fw_owns_asp1)
234-
return 0;
235-
236-
cs35l56_base->fw_owns_asp1 = false;
237-
238-
return regmap_multi_reg_write(cs35l56_base->regmap, cs35l56_asp1_defaults,
239-
ARRAY_SIZE(cs35l56_asp1_defaults));
240-
}
241-
EXPORT_SYMBOL_NS_GPL(cs35l56_init_asp1_regs_for_driver_control, SND_SOC_CS35L56_SHARED);
242-
243-
/*
244-
* The firmware boot sequence can overwrite the ASP1 config registers so that
245-
* they don't match regmap's view of their values. Rewrite the values from the
246-
* regmap cache into the hardware registers.
247-
*/
248-
int cs35l56_force_sync_asp1_registers_from_cache(struct cs35l56_base *cs35l56_base)
249-
{
250-
struct reg_sequence asp1_regs[ARRAY_SIZE(cs35l56_asp1_defaults)];
251-
int i, ret;
252-
253-
if (cs35l56_base->fw_owns_asp1)
254-
return 0;
255-
256-
memcpy(asp1_regs, cs35l56_asp1_defaults, sizeof(asp1_regs));
257-
258-
/* Read current values from regmap cache into the write sequence */
259-
for (i = 0; i < ARRAY_SIZE(asp1_regs); ++i) {
260-
ret = regmap_read(cs35l56_base->regmap, asp1_regs[i].reg, &asp1_regs[i].def);
261-
if (ret)
262-
goto err;
263-
}
264-
265-
/* Write the values cache-bypassed so that they will be written to silicon */
266-
ret = regmap_multi_reg_write_bypassed(cs35l56_base->regmap, asp1_regs,
267-
ARRAY_SIZE(asp1_regs));
268-
if (ret)
269-
goto err;
270-
271-
return 0;
272-
273-
err:
274-
dev_err(cs35l56_base->dev, "Failed to sync ASP1 registers: %d\n", ret);
275-
276-
return ret;
277-
}
278-
EXPORT_SYMBOL_NS_GPL(cs35l56_force_sync_asp1_registers_from_cache, SND_SOC_CS35L56_SHARED);
279-
280227
int cs35l56_mbox_send(struct cs35l56_base *cs35l56_base, unsigned int command)
281228
{
282229
unsigned int val;

0 commit comments

Comments
 (0)