From c57f304302f08c7934869133e522d508319f2ad3 Mon Sep 17 00:00:00 2001 From: Richard Fitzgerald Date: Fri, 14 Nov 2025 14:01:52 +0000 Subject: [PATCH] soundwire: Intel: Always issue bus-reset after clock stop Always issue a bus reset when coming out of clock stop mode. This reverts commit 08abad9f45f10 ("soundwire: intel: refine runtime pm for SDW_INTEL_CLK_STOP_BUS_RESET"). When entering clock stop the bus link is actually powered-down to save more power. The SoundWire specification allows this but requires that the manager sends a bus reset on wakeup. On Intel hardware before Lunarlake the audio controller can only power- down if all links are powered-down. Commit 08abad9f45f10 ("soundwire: intel: refine runtime pm for SDW_INTEL_CLK_STOP_BUS_RESET") added a special case to skip the bus reset and do a normal exit from clock stop if the link was not fully powered-down. However, this shortcut is not safe. When powering the link up again there will be false edges on SCLK, which will cause the peripherals to clock in one or two false data bits. This causes the peripherals to lose sync and they will drop off the bus, as required by the SoundWire specification. It will take at least 17 frames for the peripherals to regain sync, and they are likely to need re-enumeration. But the core SoundWire code was told that this is an exit from clock stop 0, and unattach_request is not set, so the core code and the drivers are expecting to be able to talk to the peripherals immediately. This will cause read/write errors. This is quite likely to occur when there are different types of peripheral on the links. For example, a headphone codec on one link and amplifiers on another link. While audio is playing to the headphones the amplifiers can runtime-suspend and the link can clock stop. This leaves one link (amps) stopped and the other (headphones) still active, creating the situation where resuming the amps would skip the bus reset because the active headphone link stops the audio controller from fully powering down. On Lunarlake the links can power-down independently. So this shortcut situation does not occur and a bus reset was always sent when the link powers-up. Fixes: 08abad9f45f10 ("soundwire: intel: refine runtime pm for SDW_INTEL_CLK_STOP_BUS_RESET") Closes: https://github.com/thesofproject/linux/issues/5495 Signed-off-by: Richard Fitzgerald --- drivers/soundwire/intel_bus_common.c | 95 +++++++++++----------------- 1 file changed, 36 insertions(+), 59 deletions(-) diff --git a/drivers/soundwire/intel_bus_common.c b/drivers/soundwire/intel_bus_common.c index ad1f8ebdbfc9b1..560aa908f672ea 100644 --- a/drivers/soundwire/intel_bus_common.c +++ b/drivers/soundwire/intel_bus_common.c @@ -77,87 +77,64 @@ int intel_start_bus_after_reset(struct sdw_intel *sdw) struct device *dev = sdw->cdns.dev; struct sdw_cdns *cdns = &sdw->cdns; struct sdw_bus *bus = &cdns->bus; - bool clock_stop0; int status; int ret; /* - * An exception condition occurs for the CLK_STOP_BUS_RESET - * case if one or more masters remain active. In this condition, - * all the masters are powered on for they are in the same power - * domain. Master can preserve its context for clock stop0, so - * there is no need to clear slave status and reset bus. + * make sure all Slaves are tagged as UNATTACHED and + * provide reason for reinitialization */ - clock_stop0 = sdw_cdns_is_clock_stop(&sdw->cdns); - - if (!clock_stop0) { - - /* - * make sure all Slaves are tagged as UNATTACHED and - * provide reason for reinitialization - */ - - status = SDW_UNATTACH_REQUEST_MASTER_RESET; - sdw_clear_slave_status(bus, status); - /* - * follow recommended programming flows to avoid - * timeouts when gsync is enabled - */ - if (bus->multi_link) - sdw_intel_sync_arm(sdw); + status = SDW_UNATTACH_REQUEST_MASTER_RESET; + sdw_clear_slave_status(bus, status); - /* - * Re-initialize the IP since it was powered-off - */ - sdw_cdns_init(&sdw->cdns); + /* + * follow recommended programming flows to avoid + * timeouts when gsync is enabled + */ + if (bus->multi_link) + sdw_intel_sync_arm(sdw); - } else { - ret = sdw_cdns_enable_interrupt(cdns, true); - if (ret < 0) { - dev_err(dev, "cannot enable interrupts during resume\n"); - return ret; - } - } + /* + * Re-initialize the IP since it was powered-off + */ + sdw_cdns_init(&sdw->cdns); - ret = sdw_cdns_clock_restart(cdns, !clock_stop0); + ret = sdw_cdns_clock_restart(cdns, true); if (ret < 0) { dev_err(dev, "unable to restart clock during resume\n"); - if (!clock_stop0) - sdw_cdns_enable_interrupt(cdns, false); + sdw_cdns_enable_interrupt(cdns, false); return ret; } - if (!clock_stop0) { - sdw_cdns_config_update(cdns); - - if (bus->multi_link) { - ret = sdw_intel_sync_go(sdw); - if (ret < 0) { - dev_err(sdw->cdns.dev, "sync go failed during resume\n"); - return ret; - } - } + sdw_cdns_config_update(cdns); - ret = sdw_cdns_config_update_set_wait(cdns); + if (bus->multi_link) { + ret = sdw_intel_sync_go(sdw); if (ret < 0) { - dev_err(dev, "%s: CONFIG_UPDATE BIT still set\n", __func__); + dev_err(sdw->cdns.dev, "sync go failed during resume\n"); return ret; } + } - ret = sdw_cdns_enable_interrupt(cdns, true); - if (ret < 0) { - dev_err(dev, "cannot enable interrupts during resume\n"); - return ret; - } + ret = sdw_cdns_config_update_set_wait(cdns); + if (ret < 0) { + dev_err(dev, "%s: CONFIG_UPDATE BIT still set\n", __func__); + return ret; + } - ret = sdw_cdns_exit_reset(cdns); - if (ret < 0) { - dev_err(dev, "unable to exit bus reset sequence during resume\n"); - return ret; - } + ret = sdw_cdns_enable_interrupt(cdns, true); + if (ret < 0) { + dev_err(dev, "cannot enable interrupts during resume\n"); + return ret; + } + ret = sdw_cdns_exit_reset(cdns); + if (ret < 0) { + dev_err(dev, "unable to exit bus reset sequence during resume\n"); + return ret; } + sdw_cdns_check_self_clearing_bits(cdns, __func__, true, INTEL_MASTER_RESET_ITERATIONS); schedule_delayed_work(&cdns->attach_dwork,