diff mbox series

soundwire: bus: improve error handling for clock stop prepare/deprepare

Message ID 20231013010812.114216-1-yung-chuan.liao@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series soundwire: bus: improve error handling for clock stop prepare/deprepare | expand

Commit Message

Bard Liao Oct. 13, 2023, 1:08 a.m. UTC 
From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

The same logic is used for clock stop prepare and deprepare, and
having different logs for the two steps helps identify problems.

In addition, when the "NotFinished" bit remains set, the error
handling is not quite right:

a) for the clock stop prepare, the error is handled at the caller
level, and the error is ignored: there's no good reason to prevent the
pm_runtime suspend from happening. Throwing an error that is later
ignored is confusing.

b) for the clock stop deprepare, the error is ignored in bus.c and a
dev_warn() log shown. Throwing an error is also alarming users for no
good reason.

For both cases, demoting the error to dev_dbg() makes more sense.

Link: https://github.com/thesofproject/linux/issues/4619
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/bus.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

Comments

Vinod Koul Oct. 16, 2023, 11:20 a.m. UTC | #1 
On Fri, 13 Oct 2023 09:08:12 +0800, Bard Liao wrote:
> The same logic is used for clock stop prepare and deprepare, and
> having different logs for the two steps helps identify problems.
> 
> In addition, when the "NotFinished" bit remains set, the error
> handling is not quite right:
> 
> a) for the clock stop prepare, the error is handled at the caller
> level, and the error is ignored: there's no good reason to prevent the
> pm_runtime suspend from happening. Throwing an error that is later
> ignored is confusing.
> 
> [...]

Applied, thanks!

[1/1] soundwire: bus: improve error handling for clock stop prepare/deprepare
      commit: ff435da4cc8b6137be9f0b6b4a9af095590d4b8c

Best regards,
diff mbox series

Patch

diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 1720031f35a3..327ce316fed9 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -1022,7 +1022,7 @@  static int sdw_slave_clk_stop_prepare(struct sdw_slave *slave,
 	return ret;
 }
 
-static int sdw_bus_wait_for_clk_prep_deprep(struct sdw_bus *bus, u16 dev_num)
+static int sdw_bus_wait_for_clk_prep_deprep(struct sdw_bus *bus, u16 dev_num, bool prepare)
 {
 	int retry = bus->clk_stop_timeout;
 	int val;
@@ -1036,7 +1036,8 @@  static int sdw_bus_wait_for_clk_prep_deprep(struct sdw_bus *bus, u16 dev_num)
 		}
 		val &= SDW_SCP_STAT_CLK_STP_NF;
 		if (!val) {
-			dev_dbg(bus->dev, "clock stop prep/de-prep done slave:%d\n",
+			dev_dbg(bus->dev, "clock stop %s done slave:%d\n",
+				prepare ? "prepare" : "deprepare",
 				dev_num);
 			return 0;
 		}
@@ -1045,7 +1046,8 @@  static int sdw_bus_wait_for_clk_prep_deprep(struct sdw_bus *bus, u16 dev_num)
 		retry--;
 	} while (retry);
 
-	dev_err(bus->dev, "clock stop prep/de-prep failed slave:%d\n",
+	dev_dbg(bus->dev, "clock stop %s did not complete for slave:%d\n",
+		prepare ? "prepare" : "deprepare",
 		dev_num);
 
 	return -ETIMEDOUT;
@@ -1116,7 +1118,7 @@  int sdw_bus_prep_clk_stop(struct sdw_bus *bus)
 	 */
 	if (!simple_clk_stop) {
 		ret = sdw_bus_wait_for_clk_prep_deprep(bus,
-						       SDW_BROADCAST_DEV_NUM);
+						       SDW_BROADCAST_DEV_NUM, true);
 		/*
 		 * if there are no Slave devices present and the reply is
 		 * Command_Ignored/-ENODATA, we don't need to continue with the
@@ -1236,7 +1238,7 @@  int sdw_bus_exit_clk_stop(struct sdw_bus *bus)
 	 * state machine
 	 */
 	if (!simple_clk_stop) {
-		ret = sdw_bus_wait_for_clk_prep_deprep(bus, SDW_BROADCAST_DEV_NUM);
+		ret = sdw_bus_wait_for_clk_prep_deprep(bus, SDW_BROADCAST_DEV_NUM, false);
 		if (ret < 0)
 			dev_warn(bus->dev, "clock stop deprepare wait failed:%d\n", ret);
 	}