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

[bardliao]

dev->power.runtime_error will be set to the return value of runtime
suspend callback function, and runtime resume function will return
-EINVAL if dev->power.runtime_error is not 0.
Somehow codec can rarely doesn't ack to the clock prepare command.
If we stop the runtime suspend process and return error, we will not
be able to resume again. As a result, the sdw bus can not be used
anymore.
This patch suggests to finish the runtime suspend process even if we
fail to stop sdw bus clock. Power consumption may be higher and lose
interrupt if fail to stop clock, but at least audio function can still
work after that.

Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com

[bardliao]

@aiChaoSONG Could you test if the audio can work after #2606 happens with this PR?

[aiChaoSONG]

@bardliao Bad news, the only CML STRA device is packaged and will be returned. I cannot verify it on our side, sorry.

[bardliao]

Verified it on CML sdw platform.

[lyakh]

one could retry a couple of times, not sure if it's worth it though

[RanderWang]

@bardliao it will affect resume path. Can you confirm the following path?

clock_stop0 = sdw_cdns_is_clock_stop(&sdw->cdns);

		if (!clock_stop0)

[RanderWang]

@bardliao it will affect resume path. Can you confirm the following path?

clock_stop0 = sdw_cdns_is_clock_stop(&sdw->cdns);

		if (!clock_stop0)

if the path is ok, in my opinion you can refine the commit message. It is fine to do as this since we will do hardware reset to make everything work in resume function.

[bardliao]

@bardliao it will affect resume path. Can you confirm the following path?

clock_stop0 = sdw_cdns_is_clock_stop(&sdw->cdns);

		if (!clock_stop0)

if the path is ok, in my opinion you can refine the commit message. It is fine to do as this since we will do hardware reset to make everything work in resume function.

Thanks @RanderWang I checked clock_stop0 is 0 as expected when the issue occurred. And my commit message is updated.

[plbossart]

minor edits in commit message, see below

[plbossart]

this seems good to me, let's do this.

Minor edits to the commit message

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

[bardliao]

@plbossart Welcome back. Updated.

[bardliao]

one could retry a couple of times, not sure if it's worth it though

I tried to retry as below, but it didn't help. The result is always -61 in the 10 iterations.

--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -863,6 +863,7 @@ static int sdw_slave_clk_stop_prepare(struct sdw_slave *slave,
        bool wake_en;
        u32 val = 0;
        int ret;
+       int i;

        wake_en = slave->prop.wake_capable;

@@ -882,6 +883,11 @@ static int sdw_slave_clk_stop_prepare(struct sdw_slave *slave,

        ret = sdw_write_no_pm(slave, SDW_SCP_SYSTEMCTRL, val);

+       for (i = 0; i < 10 && ret == -ENODATA; i++) {
+               dev_err(&slave->dev, "bard: Rewrite SDW_SCP_SYSTEMCTRL %d ret %d\n", i, ret);
+               ret = sdw_write_no_pm(slave, SDW_SCP_SYSTEMCTRL, val);
+       }
+
        if (ret != 0)
                dev_err(&slave->dev,
                        "Clock Stop prepare failed for slave: %d", ret);