Skip to content

Commit

Permalink
soundwire: intel: exit clock stop mode on system suspend
Browse files Browse the repository at this point in the history
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: #2606
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Bard Liao <bard.liao@intel.com>
  • Loading branch information
plbossart committed Aug 2, 2021
1 parent 6507052 commit 917075f
Showing 1 changed file with 65 additions and 0 deletions.
65 changes: 65 additions & 0 deletions drivers/soundwire/intel.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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)
};
Expand Down

0 comments on commit 917075f

Please sign in to comment.