[PATCH] soundwire: intel: don't return error when clock stop failed

[PATCH] soundwire: intel: don't return error when clock stop failed

[Bard Liao]

dev->power.runtime_error will be set to the return value of the runtime
suspend callback function, and runtime resume function will return
-EINVAL if dev->power.runtime_error is not 0.

Somehow the codec rarely doesn't return an ACK to the clock prepare
command. If we stop the runtime suspend process and return error, we
will not be able to resume again. Likewise, if the codec lost sync and
did not rejoin, the resume operation will also fail. As a result, the
SoundWire bus can not be used anymore.

This patch suggests to finish the runtime suspend process even if we fail
to stop sdw bus clock. In the case where we do a hardware reset, the codecs
will be reconfigured completely. In the case where we use the regular clock
stop, the codecs keep their state and worst case will fall off the bus and
reattach.

The only drawback is that the power consumption may be higher and
device-initiated interrupts may be lost, but at least audio function can
still work after resume.

Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 drivers/soundwire/intel.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 10b60b7b17bb..a2d5cdaa9998 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1673,10 +1673,12 @@ static int __maybe_unused intel_suspend_runtime(struct device *dev)
 
 	} else if (clock_stop_quirks & SDW_INTEL_CLK_STOP_BUS_RESET ||
 		   !clock_stop_quirks) {
+		bool wake_enable = true;
+
 		ret = sdw_cdns_clock_stop(cdns, true);
 		if (ret < 0) {
 			dev_err(dev, "cannot enable clock stop on suspend\n");
-			return ret;
+			wake_enable = false;
 		}
 
 		ret = sdw_cdns_enable_interrupt(cdns, false);
@@ -1691,7 +1693,7 @@ static int __maybe_unused intel_suspend_runtime(struct device *dev)
 			return ret;
 		}
 
-		intel_shim_wake(sdw, true);
+		intel_shim_wake(sdw, wake_enable);
 	} else {
 		dev_err(dev, "%s clock_stop_quirks %x unsupported\n",
 			__func__, clock_stop_quirks);
-- 
2.17.1

Re: [PATCH] soundwire: intel: don't return error when clock stop failed

[Vinod Koul]

On 14-01-21, 11:02, Bard Liao wrote:
> dev->power.runtime_error will be set to the return value of the runtime
> suspend callback function, and runtime resume function will return
> -EINVAL if dev->power.runtime_error is not 0.
> 
> Somehow the codec rarely doesn't return an ACK to the clock prepare
> command. If we stop the runtime suspend process and return error, we
> will not be able to resume again. Likewise, if the codec lost sync and
> did not rejoin, the resume operation will also fail. As a result, the
> SoundWire bus can not be used anymore.
> 
> This patch suggests to finish the runtime suspend process even if we fail
> to stop sdw bus clock. In the case where we do a hardware reset, the codecs
> will be reconfigured completely. In the case where we use the regular clock
> stop, the codecs keep their state and worst case will fall off the bus and
> reattach.
> 
> The only drawback is that the power consumption may be higher and
> device-initiated interrupts may be lost, but at least audio function can
> still work after resume.

Applied, thanks

-- 
~Vinod