From: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx> Intel validation reported an issue where the HW_RST self-clearing bit is not cleared in hardware, which as a ripple effect creates issues with the clock stop mode. This happens is a specific sequence where the Intel manager is pm_runtime suspended with the clock-stop mode enabled. During the system suspend, we currently do nothing, which can lead to potential issues on system resume and the following pm_runtime suspend, depending on the hardware state. This patch suggests a full resume (parent+child devices) if the clock-stop mode is used. This may require extra time but will make the suspend/resume flows completely symmetric. This also removes a race condition where we could not access SHIM registers if the parent was suspended as well. Resuming the link also resumes the parent by construction. BugLink: https://github.com/thesofproject/linux/issues/2606 Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx> Reviewed-by: Ranjani Sridharan <ranjani.sridharan@xxxxxxxxxxxxxxx> Signed-off-by: Bard Liao <yung-chuan.liao@xxxxxxxxxxxxxxx> --- drivers/soundwire/intel.c | 65 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 46d1645cb7fe..9d05e158fe0e 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -1527,6 +1527,70 @@ int intel_link_process_wakeen_event(struct auxiliary_device *auxdev) * PM calls */ +static int intel_resume_child_device(struct device *dev, void *data) +{ + int ret; + struct sdw_slave *slave = dev_to_sdw_dev(dev); + + if (!slave->probed) { + dev_dbg(dev, "%s: skipping device, no probed driver\n", __func__); + return 0; + } + if (!slave->dev_num_sticky) { + dev_dbg(dev, "%s: skipping device, never detected on bus\n", __func__); + return 0; + } + + ret = pm_request_resume(dev); + if (ret < 0) + dev_err(dev, "%s: pm_request_resume failed: %d\n", __func__, ret); + + return ret; +} + +static int __maybe_unused intel_pm_prepare(struct device *dev) +{ + struct sdw_cdns *cdns = dev_get_drvdata(dev); + struct sdw_intel *sdw = cdns_to_intel(cdns); + struct sdw_bus *bus = &cdns->bus; + u32 clock_stop_quirks; + int ret = 0; + + if (bus->prop.hw_disabled || !sdw->startup_done) { + dev_dbg(dev, "SoundWire master %d is disabled or not-started, ignoring\n", + bus->link_id); + return 0; + } + + clock_stop_quirks = sdw->link_res->clock_stop_quirks; + + if ((clock_stop_quirks & SDW_INTEL_CLK_STOP_BUS_RESET) || + !clock_stop_quirks) { + /* + * Try to resume the entire bus (parent + child devices) to exit + * the clock stop mode. If this fails, we keep going since we don't want + * to prevent system suspend from happening and errors should be recoverable + * on resume. + */ + ret = device_for_each_child(bus->dev, NULL, intel_resume_child_device); + + if (ret < 0) + dev_err(dev, "%s: intel_resume_child_device failed: %d\n", __func__, ret); + + /* + * in the case where a link was started but does not have anything connected, + * we still need to resume to keep link power up/down sequences balanced. + * This is a no-op if a child device was present, since resuming the child + * device would also resume the parent + */ + ret = pm_request_resume(dev); + if (ret < 0) + dev_err(dev, "%s: pm_request_resume failed: %d\n", __func__, ret); + } + + return 0; +} + static int __maybe_unused intel_suspend(struct device *dev) { struct sdw_cdns *cdns = dev_get_drvdata(dev); @@ -1923,6 +1987,7 @@ static int __maybe_unused intel_resume_runtime(struct device *dev) } static const struct dev_pm_ops intel_pm = { + .prepare = intel_pm_prepare, SET_SYSTEM_SLEEP_PM_OPS(intel_suspend, intel_resume) SET_RUNTIME_PM_OPS(intel_suspend_runtime, intel_resume_runtime, NULL) }; -- 2.17.1
