Skip to content

Commit d4884fd

Browse files
rfvirgilbroonie
authored andcommitted
ASoC: cs35l56: Fix unintended bus access while resetting amp
Use the new regmap_read_bypassed() so that the regmap can be left in cache-only mode while it is booting, but the driver can still read boot-status and chip-id information during this time. This fixes race conditions where some writes could be issued to the silicon while it is still rebooting, before the driver has determined that the boot is complete. This is typically prevented by putting regmap into cache-only until the hardware is ready. But this assumes that the driver does not need to access device registers to determine when it is "ready". For cs35l56 this involves polling a register and the original implementation relied on having special handlers to block racing callbacks until dsp_work() is complete. However, some cases were missed, most notably the ASP DAI functions. The regmap_read_bypassed() function allows the fix for this to be simplified to putting regmap into cache-only during the reset. The initial boot stages (poll HALO_STATE and read the chip ID) are all done bypassed. Only when the amp is seen to be booted is the cache-only revoked. Changes are: - cs35l56_system_reset() now leaves the regmap in cache-only status. - cs35l56_wait_for_firmware_boot() polls using regmap_read_bypassed(). - cs35l56_init() revokes cache-only either via cs35l56_hw_init() or when firmware has rebooted after a soft reset. - cs35l56_hw_init() exits cache-only after it has determined that the amp has booted. - cs35l56_sdw_init() doesn't disable cache-only, since this must be deferred to cs35l56_init(). - cs35l56_runtime_resume_common() waits for firmware boot before exiting cache-only. These changes cover three situations where the registers are not accessible: 1) SoundWire first-time enumeration. The regmap is kept in cache-only until the chip is fully booted. The original code had to exit cache-only to read chip status in cs35l56_init() and cs35l56_hw_init() but this is now deferred to after the firmware has rebooted. In this case cs35l56_sdw_probe() leaves regmap in cache-only (unchanged behaviour) and cs35l56_hw_init() exits cache-only after the firmware is booted and the chip identified. 2) Soft reset during first-time initialization. cs35l56_init() calls cs35l56_system_reset(), which puts regmap into cache-only. On I2C/SPI cs35l56_init() then flows through to call cs35l56_wait_for_firmware_boot() and exit cache-only. On SoundWire the re-enumeration will enter cs35l56_init() again, which then drops down to call cs35l56_wait_for_firmware_boot() and exit cache-only. 3) Soft reset after firmware download. dsp_work() calls cs35l56_system_reset(), which puts regmap into cache-only. After this the flow is the same as (2). Signed-off-by: Richard Fitzgerald <[email protected]> Fixes: 8a731fd ("ASoC: cs35l56: Move utility functions to shared file") Link: https://msgid.link/r/[email protected] Signed-off-by: Mark Brown <[email protected]>
1 parent 73580ec commit d4884fd

File tree

3 files changed

+14
-10
lines changed

3 files changed

+14
-10
lines changed

sound/soc/codecs/cs35l56-sdw.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -188,8 +188,6 @@ static void cs35l56_sdw_init(struct sdw_slave *peripheral)
188188
goto out;
189189
}
190190

191-
regcache_cache_only(cs35l56->base.regmap, false);
192-
193191
ret = cs35l56_init(cs35l56);
194192
if (ret < 0) {
195193
regcache_cache_only(cs35l56->base.regmap, true);

sound/soc/codecs/cs35l56-shared.c

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -307,10 +307,10 @@ int cs35l56_wait_for_firmware_boot(struct cs35l56_base *cs35l56_base)
307307
reg = CS35L56_DSP1_HALO_STATE;
308308

309309
/*
310-
* This can't be a regmap_read_poll_timeout() because cs35l56 will NAK
311-
* I2C until it has booted which would terminate the poll
310+
* The regmap must remain in cache-only until the chip has
311+
* booted, so use a bypassed read of the status register.
312312
*/
313-
poll_ret = read_poll_timeout(regmap_read, read_ret,
313+
poll_ret = read_poll_timeout(regmap_read_bypassed, read_ret,
314314
(val < 0xFFFF) && (val >= CS35L56_HALO_STATE_BOOT_DONE),
315315
CS35L56_HALO_STATE_POLL_US,
316316
CS35L56_HALO_STATE_TIMEOUT_US,
@@ -362,7 +362,8 @@ void cs35l56_system_reset(struct cs35l56_base *cs35l56_base, bool is_soundwire)
362362
return;
363363

364364
cs35l56_wait_control_port_ready();
365-
regcache_cache_only(cs35l56_base->regmap, false);
365+
366+
/* Leave in cache-only. This will be revoked when the chip has rebooted. */
366367
}
367368
EXPORT_SYMBOL_NS_GPL(cs35l56_system_reset, SND_SOC_CS35L56_SHARED);
368369

@@ -577,14 +578,14 @@ int cs35l56_runtime_resume_common(struct cs35l56_base *cs35l56_base, bool is_sou
577578
cs35l56_issue_wake_event(cs35l56_base);
578579

579580
out_sync:
580-
regcache_cache_only(cs35l56_base->regmap, false);
581-
582581
ret = cs35l56_wait_for_firmware_boot(cs35l56_base);
583582
if (ret) {
584583
dev_err(cs35l56_base->dev, "Hibernate wake failed: %d\n", ret);
585584
goto err;
586585
}
587586

587+
regcache_cache_only(cs35l56_base->regmap, false);
588+
588589
ret = cs35l56_mbox_send(cs35l56_base, CS35L56_MBOX_CMD_PREVENT_AUTO_HIBERNATE);
589590
if (ret)
590591
goto err;
@@ -757,7 +758,7 @@ int cs35l56_hw_init(struct cs35l56_base *cs35l56_base)
757758
* devices so the REVID needs to be determined before waiting for the
758759
* firmware to boot.
759760
*/
760-
ret = regmap_read(cs35l56_base->regmap, CS35L56_REVID, &revid);
761+
ret = regmap_read_bypassed(cs35l56_base->regmap, CS35L56_REVID, &revid);
761762
if (ret < 0) {
762763
dev_err(cs35l56_base->dev, "Get Revision ID failed\n");
763764
return ret;
@@ -768,7 +769,7 @@ int cs35l56_hw_init(struct cs35l56_base *cs35l56_base)
768769
if (ret)
769770
return ret;
770771

771-
ret = regmap_read(cs35l56_base->regmap, CS35L56_DEVID, &devid);
772+
ret = regmap_read_bypassed(cs35l56_base->regmap, CS35L56_DEVID, &devid);
772773
if (ret < 0) {
773774
dev_err(cs35l56_base->dev, "Get Device ID failed\n");
774775
return ret;
@@ -787,6 +788,9 @@ int cs35l56_hw_init(struct cs35l56_base *cs35l56_base)
787788

788789
cs35l56_base->type = devid & 0xFF;
789790

791+
/* Silicon is now identified and booted so exit cache-only */
792+
regcache_cache_only(cs35l56_base->regmap, false);
793+
790794
ret = regmap_read(cs35l56_base->regmap, CS35L56_DSP_RESTRICT_STS1, &secured);
791795
if (ret) {
792796
dev_err(cs35l56_base->dev, "Get Secure status failed\n");

sound/soc/codecs/cs35l56.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1531,6 +1531,8 @@ int cs35l56_init(struct cs35l56_private *cs35l56)
15311531
return ret;
15321532

15331533
dev_dbg(cs35l56->base.dev, "Firmware rebooted after soft reset\n");
1534+
1535+
regcache_cache_only(cs35l56->base.regmap, false);
15341536
}
15351537

15361538
/* Disable auto-hibernate so that runtime_pm has control */

0 commit comments

Comments
 (0)