All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/3] soundwire: qcom: add pm runtime support
@ 2022-02-28 17:25 ` Srinivas Kandagatla
  0 siblings, 0 replies; 24+ messages in thread
From: Srinivas Kandagatla @ 2022-02-28 17:25 UTC (permalink / raw)
  To: robh+dt, vkoul, yung-chuan.liao
  Cc: pierre-louis.bossart, devicetree, linux-kernel, alsa-devel,
	quic_srivasam, Srinivas Kandagatla

This patchset adds pm runtime support to Qualcomm SounWire Controller using
SoundWire Clock Stop and Wake up using Headset events on supported instances and
instances like WSA which do not support clock stop a soft reset of controller
along with full rest of slaves is done to resume from a low power state.

Tested it on SM8250 MTP and Dragon Board DB845c

Changes since v2:
 - update log as suggested by Pierre
 - removing handling clk stop for cases where the controller is soft reset.
 - add more error checks when calling  sdw_bus_prep_clk_stop
 - update dt-bindings with wakeup-source and interrupt-names properties.

--srini

Srinivas Kandagatla (3):
  soundwire: qcom: add runtime pm support
  dt-bindings: soundwire: qcom: document optional wake irq
  soundwire: qcom: add in-band wake up interrupt support

 .../bindings/soundwire/qcom,sdw.txt           |  14 +-
 drivers/soundwire/qcom.c                      | 206 +++++++++++++++++-
 2 files changed, 218 insertions(+), 2 deletions(-)

-- 
2.21.0


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v3 0/3] soundwire: qcom: add pm runtime support
@ 2022-02-28 17:25 ` Srinivas Kandagatla
  0 siblings, 0 replies; 24+ messages in thread
From: Srinivas Kandagatla @ 2022-02-28 17:25 UTC (permalink / raw)
  To: robh+dt, vkoul, yung-chuan.liao
  Cc: devicetree, alsa-devel, pierre-louis.bossart, linux-kernel,
	Srinivas Kandagatla, quic_srivasam

This patchset adds pm runtime support to Qualcomm SounWire Controller using
SoundWire Clock Stop and Wake up using Headset events on supported instances and
instances like WSA which do not support clock stop a soft reset of controller
along with full rest of slaves is done to resume from a low power state.

Tested it on SM8250 MTP and Dragon Board DB845c

Changes since v2:
 - update log as suggested by Pierre
 - removing handling clk stop for cases where the controller is soft reset.
 - add more error checks when calling  sdw_bus_prep_clk_stop
 - update dt-bindings with wakeup-source and interrupt-names properties.

--srini

Srinivas Kandagatla (3):
  soundwire: qcom: add runtime pm support
  dt-bindings: soundwire: qcom: document optional wake irq
  soundwire: qcom: add in-band wake up interrupt support

 .../bindings/soundwire/qcom,sdw.txt           |  14 +-
 drivers/soundwire/qcom.c                      | 206 +++++++++++++++++-
 2 files changed, 218 insertions(+), 2 deletions(-)

-- 
2.21.0


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v3 1/3] soundwire: qcom: add runtime pm support
  2022-02-28 17:25 ` Srinivas Kandagatla
@ 2022-02-28 17:25   ` Srinivas Kandagatla
  -1 siblings, 0 replies; 24+ messages in thread
From: Srinivas Kandagatla @ 2022-02-28 17:25 UTC (permalink / raw)
  To: robh+dt, vkoul, yung-chuan.liao
  Cc: pierre-louis.bossart, devicetree, linux-kernel, alsa-devel,
	quic_srivasam, Srinivas Kandagatla

Add support to runtime PM using SoundWire clock stop Mode0 on supported
controller instances and soft reset on instances that do not support
clock stop.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/soundwire/qcom.c | 156 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 155 insertions(+), 1 deletion(-)

diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index 54813417ef8e..810232686196 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -11,6 +11,7 @@
 #include <linux/of.h>
 #include <linux/of_irq.h>
 #include <linux/of_device.h>
+#include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/slab.h>
 #include <linux/slimbus.h>
@@ -20,6 +21,9 @@
 #include <sound/soc.h>
 #include "bus.h"
 
+#define SWRM_COMP_SW_RESET					0x008
+#define SWRM_COMP_STATUS					0x014
+#define SWRM_FRM_GEN_ENABLED					BIT(0)
 #define SWRM_COMP_HW_VERSION					0x00
 #define SWRM_COMP_CFG_ADDR					0x04
 #define SWRM_COMP_CFG_IRQ_LEVEL_OR_PULSE_MSK			BIT(1)
@@ -29,6 +33,7 @@
 #define SWRM_COMP_PARAMS_RD_FIFO_DEPTH				GENMASK(19, 15)
 #define SWRM_COMP_PARAMS_DOUT_PORTS_MASK			GENMASK(4, 0)
 #define SWRM_COMP_PARAMS_DIN_PORTS_MASK				GENMASK(9, 5)
+#define SWRM_COMP_MASTER_ID					0x104
 #define SWRM_INTERRUPT_STATUS					0x200
 #define SWRM_INTERRUPT_STATUS_RMSK				GENMASK(16, 0)
 #define SWRM_INTERRUPT_STATUS_SLAVE_PEND_IRQ			BIT(0)
@@ -111,6 +116,13 @@
 #define SWR_MAX_CMD_ID	14
 #define MAX_FIFO_RD_RETRY 3
 #define SWR_OVERFLOW_RETRY_COUNT 30
+#define SWRM_LINK_STATUS_RETRY_CNT 100
+
+enum {
+	MASTER_ID_WSA = 1,
+	MASTER_ID_RX,
+	MASTER_ID_TX
+};
 
 struct qcom_swrm_port_config {
 	u8 si;
@@ -159,6 +171,7 @@ struct qcom_swrm_ctrl {
 	u32 slave_status;
 	u32 wr_fifo_depth;
 	u32 rd_fifo_depth;
+	bool clock_stop_not_supported;
 };
 
 struct qcom_swrm_data {
@@ -497,6 +510,7 @@ static irqreturn_t qcom_swrm_irq_handler(int irq, void *dev_id)
 	u32 i;
 	int devnum;
 	int ret = IRQ_HANDLED;
+	clk_prepare_enable(swrm->hclk);
 
 	swrm->reg_read(swrm, SWRM_INTERRUPT_STATUS, &intr_sts);
 	intr_sts_masked = intr_sts & swrm->intr_mask;
@@ -604,6 +618,7 @@ static irqreturn_t qcom_swrm_irq_handler(int irq, void *dev_id)
 		intr_sts_masked = intr_sts & swrm->intr_mask;
 	} while (intr_sts_masked);
 
+	clk_disable_unprepare(swrm->hclk);
 	return ret;
 }
 
@@ -1017,6 +1032,15 @@ static int qcom_swrm_startup(struct snd_pcm_substream *substream,
 	struct snd_soc_dai *codec_dai;
 	int ret, i;
 
+	ret = pm_runtime_get_sync(ctrl->dev);
+	if (ret < 0 && ret != -EACCES) {
+		dev_err_ratelimited(ctrl->dev,
+				    "pm_runtime_get_sync failed in %s, ret %d\n",
+				    __func__, ret);
+		pm_runtime_put_noidle(ctrl->dev);
+		return ret;
+	}
+
 	sruntime = sdw_alloc_stream(dai->name);
 	if (!sruntime)
 		return -ENOMEM;
@@ -1044,6 +1068,9 @@ static void qcom_swrm_shutdown(struct snd_pcm_substream *substream,
 
 	sdw_release_stream(ctrl->sruntime[dai->id]);
 	ctrl->sruntime[dai->id] = NULL;
+	pm_runtime_mark_last_busy(ctrl->dev);
+	pm_runtime_put_autosuspend(ctrl->dev);
+
 }
 
 static const struct snd_soc_dai_ops qcom_swrm_pdm_dai_ops = {
@@ -1197,12 +1224,23 @@ static int qcom_swrm_get_port_config(struct qcom_swrm_ctrl *ctrl)
 static int swrm_reg_show(struct seq_file *s_file, void *data)
 {
 	struct qcom_swrm_ctrl *swrm = s_file->private;
-	int reg, reg_val;
+	int reg, reg_val, ret;
+
+	ret = pm_runtime_get_sync(swrm->dev);
+	if (ret < 0 && ret != -EACCES) {
+		dev_err_ratelimited(swrm->dev,
+				    "pm_runtime_get_sync failed in %s, ret %d\n",
+				    __func__, ret);
+		pm_runtime_put_noidle(swrm->dev);
+	}
 
 	for (reg = 0; reg <= SWR_MSTR_MAX_REG_ADDR; reg += 4) {
 		swrm->reg_read(swrm, reg, &reg_val);
 		seq_printf(s_file, "0x%.3x: 0x%.2x\n", reg, reg_val);
 	}
+	pm_runtime_mark_last_busy(swrm->dev);
+	pm_runtime_put_autosuspend(swrm->dev);
+
 
 	return 0;
 }
@@ -1267,6 +1305,7 @@ static int qcom_swrm_probe(struct platform_device *pdev)
 	ctrl->bus.ops = &qcom_swrm_ops;
 	ctrl->bus.port_ops = &qcom_swrm_port_ops;
 	ctrl->bus.compute_params = &qcom_swrm_compute_params;
+	ctrl->bus.clk_stop_timeout = 300;
 
 	ret = qcom_swrm_get_port_config(ctrl);
 	if (ret)
@@ -1319,6 +1358,21 @@ static int qcom_swrm_probe(struct platform_device *pdev)
 		 (ctrl->version >> 24) & 0xff, (ctrl->version >> 16) & 0xff,
 		 ctrl->version & 0xffff);
 
+	pm_runtime_set_autosuspend_delay(dev, 3000);
+	pm_runtime_use_autosuspend(dev);
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_set_active(dev);
+	pm_runtime_enable(dev);
+
+	/* Clk stop is not supported on WSA Soundwire masters */
+	if (ctrl->version <= 0x01030000) {
+		ctrl->clock_stop_not_supported = true;
+	} else {
+		ctrl->reg_read(ctrl, SWRM_COMP_MASTER_ID, &val);
+		if (val == MASTER_ID_WSA)
+			ctrl->clock_stop_not_supported = true;
+	}
+
 #ifdef CONFIG_DEBUG_FS
 	ctrl->debugfs = debugfs_create_dir("qualcomm-sdw", ctrl->bus.debugfs);
 	debugfs_create_file("qualcomm-registers", 0400, ctrl->debugfs, ctrl,
@@ -1345,6 +1399,105 @@ static int qcom_swrm_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static bool swrm_wait_for_frame_gen_enabled(struct qcom_swrm_ctrl *swrm)
+{
+	int retry = SWRM_LINK_STATUS_RETRY_CNT;
+	int comp_sts;
+
+	do {
+		swrm->reg_read(swrm, SWRM_COMP_STATUS, &comp_sts);
+
+		if (comp_sts & SWRM_FRM_GEN_ENABLED)
+			return true;
+
+		usleep_range(500, 510);
+	} while (retry--);
+
+	dev_err(swrm->dev, "%s: link status not %s\n", __func__,
+		comp_sts && SWRM_FRM_GEN_ENABLED ? "connected" : "disconnected");
+
+	return false;
+}
+
+static int swrm_runtime_resume(struct device *dev)
+{
+	struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(dev);
+	int ret;
+
+	clk_prepare_enable(ctrl->hclk);
+
+	if (ctrl->clock_stop_not_supported) {
+		reinit_completion(&ctrl->enumeration);
+		ctrl->reg_write(ctrl, SWRM_COMP_SW_RESET, 0x01);
+		usleep_range(100, 105);
+
+		qcom_swrm_init(ctrl);
+
+		usleep_range(100, 105);
+		if (!swrm_wait_for_frame_gen_enabled(ctrl))
+			dev_err(ctrl->dev, "link failed to connect\n");
+
+		/* wait for hw enumeration to complete */
+		wait_for_completion_timeout(&ctrl->enumeration,
+					    msecs_to_jiffies(TIMEOUT_MS));
+		qcom_swrm_get_device_status(ctrl);
+		sdw_handle_slave_status(&ctrl->bus, ctrl->status);
+	} else {
+		ctrl->reg_write(ctrl, SWRM_MCP_BUS_CTRL, SWRM_MCP_BUS_CLK_START);
+		ctrl->reg_write(ctrl, SWRM_INTERRUPT_CLEAR,
+			SWRM_INTERRUPT_STATUS_MASTER_CLASH_DET);
+
+		ctrl->intr_mask |= SWRM_INTERRUPT_STATUS_MASTER_CLASH_DET;
+		ctrl->reg_write(ctrl, SWRM_INTERRUPT_MASK_ADDR, ctrl->intr_mask);
+		ctrl->reg_write(ctrl, SWRM_INTERRUPT_CPU_EN, ctrl->intr_mask);
+
+		usleep_range(100, 105);
+		if (!swrm_wait_for_frame_gen_enabled(ctrl))
+			dev_err(ctrl->dev, "link failed to connect\n");
+
+		ret = sdw_bus_exit_clk_stop(&ctrl->bus);
+		if (ret < 0)
+			dev_err(ctrl->dev, "bus failed to exit clock stop %d\n", ret);
+	}
+
+	return 0;
+}
+
+static int __maybe_unused swrm_runtime_suspend(struct device *dev)
+{
+	struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(dev);
+	int ret;
+
+	if (!ctrl->clock_stop_not_supported) {
+		/* Mask bus clash interrupt */
+		ctrl->intr_mask &= ~SWRM_INTERRUPT_STATUS_MASTER_CLASH_DET;
+		ctrl->reg_write(ctrl, SWRM_INTERRUPT_MASK_ADDR, ctrl->intr_mask);
+		ctrl->reg_write(ctrl, SWRM_INTERRUPT_CPU_EN, ctrl->intr_mask);
+		/* Prepare slaves for clock stop */
+		ret = sdw_bus_prep_clk_stop(&ctrl->bus);
+		if (ret < 0 && ret != -ENODATA) {
+			dev_err(dev, "prepare clock stop failed %d", ret);
+			return ret;
+		}
+
+		ret = sdw_bus_clk_stop(&ctrl->bus);
+		if (ret < 0 && ret != -ENODATA) {
+			dev_err(dev, "bus clock stop failed %d", ret);
+			return ret;
+		}
+	}
+
+	clk_disable_unprepare(ctrl->hclk);
+
+	usleep_range(300, 305);
+
+	return 0;
+}
+
+static const struct dev_pm_ops swrm_dev_pm_ops = {
+	SET_RUNTIME_PM_OPS(swrm_runtime_suspend, swrm_runtime_resume, NULL)
+};
+
 static const struct of_device_id qcom_swrm_of_match[] = {
 	{ .compatible = "qcom,soundwire-v1.3.0", .data = &swrm_v1_3_data },
 	{ .compatible = "qcom,soundwire-v1.5.1", .data = &swrm_v1_5_data },
@@ -1359,6 +1512,7 @@ static struct platform_driver qcom_swrm_driver = {
 	.driver = {
 		.name	= "qcom-soundwire",
 		.of_match_table = qcom_swrm_of_match,
+		.pm = &swrm_dev_pm_ops,
 	}
 };
 module_platform_driver(qcom_swrm_driver);
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v3 1/3] soundwire: qcom: add runtime pm support
@ 2022-02-28 17:25   ` Srinivas Kandagatla
  0 siblings, 0 replies; 24+ messages in thread
From: Srinivas Kandagatla @ 2022-02-28 17:25 UTC (permalink / raw)
  To: robh+dt, vkoul, yung-chuan.liao
  Cc: devicetree, alsa-devel, pierre-louis.bossart, linux-kernel,
	Srinivas Kandagatla, quic_srivasam

Add support to runtime PM using SoundWire clock stop Mode0 on supported
controller instances and soft reset on instances that do not support
clock stop.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/soundwire/qcom.c | 156 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 155 insertions(+), 1 deletion(-)

diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index 54813417ef8e..810232686196 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -11,6 +11,7 @@
 #include <linux/of.h>
 #include <linux/of_irq.h>
 #include <linux/of_device.h>
+#include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/slab.h>
 #include <linux/slimbus.h>
@@ -20,6 +21,9 @@
 #include <sound/soc.h>
 #include "bus.h"
 
+#define SWRM_COMP_SW_RESET					0x008
+#define SWRM_COMP_STATUS					0x014
+#define SWRM_FRM_GEN_ENABLED					BIT(0)
 #define SWRM_COMP_HW_VERSION					0x00
 #define SWRM_COMP_CFG_ADDR					0x04
 #define SWRM_COMP_CFG_IRQ_LEVEL_OR_PULSE_MSK			BIT(1)
@@ -29,6 +33,7 @@
 #define SWRM_COMP_PARAMS_RD_FIFO_DEPTH				GENMASK(19, 15)
 #define SWRM_COMP_PARAMS_DOUT_PORTS_MASK			GENMASK(4, 0)
 #define SWRM_COMP_PARAMS_DIN_PORTS_MASK				GENMASK(9, 5)
+#define SWRM_COMP_MASTER_ID					0x104
 #define SWRM_INTERRUPT_STATUS					0x200
 #define SWRM_INTERRUPT_STATUS_RMSK				GENMASK(16, 0)
 #define SWRM_INTERRUPT_STATUS_SLAVE_PEND_IRQ			BIT(0)
@@ -111,6 +116,13 @@
 #define SWR_MAX_CMD_ID	14
 #define MAX_FIFO_RD_RETRY 3
 #define SWR_OVERFLOW_RETRY_COUNT 30
+#define SWRM_LINK_STATUS_RETRY_CNT 100
+
+enum {
+	MASTER_ID_WSA = 1,
+	MASTER_ID_RX,
+	MASTER_ID_TX
+};
 
 struct qcom_swrm_port_config {
 	u8 si;
@@ -159,6 +171,7 @@ struct qcom_swrm_ctrl {
 	u32 slave_status;
 	u32 wr_fifo_depth;
 	u32 rd_fifo_depth;
+	bool clock_stop_not_supported;
 };
 
 struct qcom_swrm_data {
@@ -497,6 +510,7 @@ static irqreturn_t qcom_swrm_irq_handler(int irq, void *dev_id)
 	u32 i;
 	int devnum;
 	int ret = IRQ_HANDLED;
+	clk_prepare_enable(swrm->hclk);
 
 	swrm->reg_read(swrm, SWRM_INTERRUPT_STATUS, &intr_sts);
 	intr_sts_masked = intr_sts & swrm->intr_mask;
@@ -604,6 +618,7 @@ static irqreturn_t qcom_swrm_irq_handler(int irq, void *dev_id)
 		intr_sts_masked = intr_sts & swrm->intr_mask;
 	} while (intr_sts_masked);
 
+	clk_disable_unprepare(swrm->hclk);
 	return ret;
 }
 
@@ -1017,6 +1032,15 @@ static int qcom_swrm_startup(struct snd_pcm_substream *substream,
 	struct snd_soc_dai *codec_dai;
 	int ret, i;
 
+	ret = pm_runtime_get_sync(ctrl->dev);
+	if (ret < 0 && ret != -EACCES) {
+		dev_err_ratelimited(ctrl->dev,
+				    "pm_runtime_get_sync failed in %s, ret %d\n",
+				    __func__, ret);
+		pm_runtime_put_noidle(ctrl->dev);
+		return ret;
+	}
+
 	sruntime = sdw_alloc_stream(dai->name);
 	if (!sruntime)
 		return -ENOMEM;
@@ -1044,6 +1068,9 @@ static void qcom_swrm_shutdown(struct snd_pcm_substream *substream,
 
 	sdw_release_stream(ctrl->sruntime[dai->id]);
 	ctrl->sruntime[dai->id] = NULL;
+	pm_runtime_mark_last_busy(ctrl->dev);
+	pm_runtime_put_autosuspend(ctrl->dev);
+
 }
 
 static const struct snd_soc_dai_ops qcom_swrm_pdm_dai_ops = {
@@ -1197,12 +1224,23 @@ static int qcom_swrm_get_port_config(struct qcom_swrm_ctrl *ctrl)
 static int swrm_reg_show(struct seq_file *s_file, void *data)
 {
 	struct qcom_swrm_ctrl *swrm = s_file->private;
-	int reg, reg_val;
+	int reg, reg_val, ret;
+
+	ret = pm_runtime_get_sync(swrm->dev);
+	if (ret < 0 && ret != -EACCES) {
+		dev_err_ratelimited(swrm->dev,
+				    "pm_runtime_get_sync failed in %s, ret %d\n",
+				    __func__, ret);
+		pm_runtime_put_noidle(swrm->dev);
+	}
 
 	for (reg = 0; reg <= SWR_MSTR_MAX_REG_ADDR; reg += 4) {
 		swrm->reg_read(swrm, reg, &reg_val);
 		seq_printf(s_file, "0x%.3x: 0x%.2x\n", reg, reg_val);
 	}
+	pm_runtime_mark_last_busy(swrm->dev);
+	pm_runtime_put_autosuspend(swrm->dev);
+
 
 	return 0;
 }
@@ -1267,6 +1305,7 @@ static int qcom_swrm_probe(struct platform_device *pdev)
 	ctrl->bus.ops = &qcom_swrm_ops;
 	ctrl->bus.port_ops = &qcom_swrm_port_ops;
 	ctrl->bus.compute_params = &qcom_swrm_compute_params;
+	ctrl->bus.clk_stop_timeout = 300;
 
 	ret = qcom_swrm_get_port_config(ctrl);
 	if (ret)
@@ -1319,6 +1358,21 @@ static int qcom_swrm_probe(struct platform_device *pdev)
 		 (ctrl->version >> 24) & 0xff, (ctrl->version >> 16) & 0xff,
 		 ctrl->version & 0xffff);
 
+	pm_runtime_set_autosuspend_delay(dev, 3000);
+	pm_runtime_use_autosuspend(dev);
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_set_active(dev);
+	pm_runtime_enable(dev);
+
+	/* Clk stop is not supported on WSA Soundwire masters */
+	if (ctrl->version <= 0x01030000) {
+		ctrl->clock_stop_not_supported = true;
+	} else {
+		ctrl->reg_read(ctrl, SWRM_COMP_MASTER_ID, &val);
+		if (val == MASTER_ID_WSA)
+			ctrl->clock_stop_not_supported = true;
+	}
+
 #ifdef CONFIG_DEBUG_FS
 	ctrl->debugfs = debugfs_create_dir("qualcomm-sdw", ctrl->bus.debugfs);
 	debugfs_create_file("qualcomm-registers", 0400, ctrl->debugfs, ctrl,
@@ -1345,6 +1399,105 @@ static int qcom_swrm_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static bool swrm_wait_for_frame_gen_enabled(struct qcom_swrm_ctrl *swrm)
+{
+	int retry = SWRM_LINK_STATUS_RETRY_CNT;
+	int comp_sts;
+
+	do {
+		swrm->reg_read(swrm, SWRM_COMP_STATUS, &comp_sts);
+
+		if (comp_sts & SWRM_FRM_GEN_ENABLED)
+			return true;
+
+		usleep_range(500, 510);
+	} while (retry--);
+
+	dev_err(swrm->dev, "%s: link status not %s\n", __func__,
+		comp_sts && SWRM_FRM_GEN_ENABLED ? "connected" : "disconnected");
+
+	return false;
+}
+
+static int swrm_runtime_resume(struct device *dev)
+{
+	struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(dev);
+	int ret;
+
+	clk_prepare_enable(ctrl->hclk);
+
+	if (ctrl->clock_stop_not_supported) {
+		reinit_completion(&ctrl->enumeration);
+		ctrl->reg_write(ctrl, SWRM_COMP_SW_RESET, 0x01);
+		usleep_range(100, 105);
+
+		qcom_swrm_init(ctrl);
+
+		usleep_range(100, 105);
+		if (!swrm_wait_for_frame_gen_enabled(ctrl))
+			dev_err(ctrl->dev, "link failed to connect\n");
+
+		/* wait for hw enumeration to complete */
+		wait_for_completion_timeout(&ctrl->enumeration,
+					    msecs_to_jiffies(TIMEOUT_MS));
+		qcom_swrm_get_device_status(ctrl);
+		sdw_handle_slave_status(&ctrl->bus, ctrl->status);
+	} else {
+		ctrl->reg_write(ctrl, SWRM_MCP_BUS_CTRL, SWRM_MCP_BUS_CLK_START);
+		ctrl->reg_write(ctrl, SWRM_INTERRUPT_CLEAR,
+			SWRM_INTERRUPT_STATUS_MASTER_CLASH_DET);
+
+		ctrl->intr_mask |= SWRM_INTERRUPT_STATUS_MASTER_CLASH_DET;
+		ctrl->reg_write(ctrl, SWRM_INTERRUPT_MASK_ADDR, ctrl->intr_mask);
+		ctrl->reg_write(ctrl, SWRM_INTERRUPT_CPU_EN, ctrl->intr_mask);
+
+		usleep_range(100, 105);
+		if (!swrm_wait_for_frame_gen_enabled(ctrl))
+			dev_err(ctrl->dev, "link failed to connect\n");
+
+		ret = sdw_bus_exit_clk_stop(&ctrl->bus);
+		if (ret < 0)
+			dev_err(ctrl->dev, "bus failed to exit clock stop %d\n", ret);
+	}
+
+	return 0;
+}
+
+static int __maybe_unused swrm_runtime_suspend(struct device *dev)
+{
+	struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(dev);
+	int ret;
+
+	if (!ctrl->clock_stop_not_supported) {
+		/* Mask bus clash interrupt */
+		ctrl->intr_mask &= ~SWRM_INTERRUPT_STATUS_MASTER_CLASH_DET;
+		ctrl->reg_write(ctrl, SWRM_INTERRUPT_MASK_ADDR, ctrl->intr_mask);
+		ctrl->reg_write(ctrl, SWRM_INTERRUPT_CPU_EN, ctrl->intr_mask);
+		/* Prepare slaves for clock stop */
+		ret = sdw_bus_prep_clk_stop(&ctrl->bus);
+		if (ret < 0 && ret != -ENODATA) {
+			dev_err(dev, "prepare clock stop failed %d", ret);
+			return ret;
+		}
+
+		ret = sdw_bus_clk_stop(&ctrl->bus);
+		if (ret < 0 && ret != -ENODATA) {
+			dev_err(dev, "bus clock stop failed %d", ret);
+			return ret;
+		}
+	}
+
+	clk_disable_unprepare(ctrl->hclk);
+
+	usleep_range(300, 305);
+
+	return 0;
+}
+
+static const struct dev_pm_ops swrm_dev_pm_ops = {
+	SET_RUNTIME_PM_OPS(swrm_runtime_suspend, swrm_runtime_resume, NULL)
+};
+
 static const struct of_device_id qcom_swrm_of_match[] = {
 	{ .compatible = "qcom,soundwire-v1.3.0", .data = &swrm_v1_3_data },
 	{ .compatible = "qcom,soundwire-v1.5.1", .data = &swrm_v1_5_data },
@@ -1359,6 +1512,7 @@ static struct platform_driver qcom_swrm_driver = {
 	.driver = {
 		.name	= "qcom-soundwire",
 		.of_match_table = qcom_swrm_of_match,
+		.pm = &swrm_dev_pm_ops,
 	}
 };
 module_platform_driver(qcom_swrm_driver);
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v3 2/3] dt-bindings: soundwire: qcom: document optional wake irq
  2022-02-28 17:25 ` Srinivas Kandagatla
@ 2022-02-28 17:25   ` Srinivas Kandagatla
  -1 siblings, 0 replies; 24+ messages in thread
From: Srinivas Kandagatla @ 2022-02-28 17:25 UTC (permalink / raw)
  To: robh+dt, vkoul, yung-chuan.liao
  Cc: pierre-louis.bossart, devicetree, linux-kernel, alsa-devel,
	quic_srivasam, Srinivas Kandagatla

Wake IRQ is optional interrupt that can be wired up on SoundWire controller
instances like RX path along with MBHC(Multi Button Headset connection).
Document this in bindings.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 .../devicetree/bindings/soundwire/qcom,sdw.txt     | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/soundwire/qcom,sdw.txt b/Documentation/devicetree/bindings/soundwire/qcom,sdw.txt
index b93a2b3e029d..51ddbc509382 100644
--- a/Documentation/devicetree/bindings/soundwire/qcom,sdw.txt
+++ b/Documentation/devicetree/bindings/soundwire/qcom,sdw.txt
@@ -22,7 +22,19 @@ board specific bus parameters.
 - interrupts:
 	Usage: required
 	Value type: <prop-encoded-array>
-	Definition: should specify the SoundWire Controller IRQ
+	Definition: should specify the SoundWire Controller core and optional
+		    wake IRQ
+
+- interrupt-names:
+	Usage: Optional
+	Value type: boolean
+	Value type: <stringlist>
+	Definition: should be "core" for core and "wakeup" for wake interrupt.
+
+- wakeup-source:
+	Usage: Optional
+	Value type: boolean
+	Definition: should specify if SoundWire Controller is wake up capable.
 
 - clock-names:
 	Usage: required
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v3 2/3] dt-bindings: soundwire: qcom: document optional wake irq
@ 2022-02-28 17:25   ` Srinivas Kandagatla
  0 siblings, 0 replies; 24+ messages in thread
From: Srinivas Kandagatla @ 2022-02-28 17:25 UTC (permalink / raw)
  To: robh+dt, vkoul, yung-chuan.liao
  Cc: devicetree, alsa-devel, pierre-louis.bossart, linux-kernel,
	Srinivas Kandagatla, quic_srivasam

Wake IRQ is optional interrupt that can be wired up on SoundWire controller
instances like RX path along with MBHC(Multi Button Headset connection).
Document this in bindings.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 .../devicetree/bindings/soundwire/qcom,sdw.txt     | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/soundwire/qcom,sdw.txt b/Documentation/devicetree/bindings/soundwire/qcom,sdw.txt
index b93a2b3e029d..51ddbc509382 100644
--- a/Documentation/devicetree/bindings/soundwire/qcom,sdw.txt
+++ b/Documentation/devicetree/bindings/soundwire/qcom,sdw.txt
@@ -22,7 +22,19 @@ board specific bus parameters.
 - interrupts:
 	Usage: required
 	Value type: <prop-encoded-array>
-	Definition: should specify the SoundWire Controller IRQ
+	Definition: should specify the SoundWire Controller core and optional
+		    wake IRQ
+
+- interrupt-names:
+	Usage: Optional
+	Value type: boolean
+	Value type: <stringlist>
+	Definition: should be "core" for core and "wakeup" for wake interrupt.
+
+- wakeup-source:
+	Usage: Optional
+	Value type: boolean
+	Definition: should specify if SoundWire Controller is wake up capable.
 
 - clock-names:
 	Usage: required
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v3 3/3] soundwire: qcom: add in-band wake up interrupt support
  2022-02-28 17:25 ` Srinivas Kandagatla
@ 2022-02-28 17:25   ` Srinivas Kandagatla
  -1 siblings, 0 replies; 24+ messages in thread
From: Srinivas Kandagatla @ 2022-02-28 17:25 UTC (permalink / raw)
  To: robh+dt, vkoul, yung-chuan.liao
  Cc: pierre-louis.bossart, devicetree, linux-kernel, alsa-devel,
	quic_srivasam, Srinivas Kandagatla

Some of the Qualcomm SoundWire Controller instances like the ones that are
connected to RX path along with Headset connections support Waking up
Controller from Low power clock stop state using SoundWire In-band interrupt.
SoundWire Slave on the bus would initiate this by pulling the data line high,
while the clock is stopped.

Add support to this wake up interrupt.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/soundwire/qcom.c | 50 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index 810232686196..e893aee1b057 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -14,6 +14,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/slab.h>
+#include <linux/pm_wakeirq.h>
 #include <linux/slimbus.h>
 #include <linux/soundwire/sdw.h>
 #include <linux/soundwire/sdw_registers.h>
@@ -154,6 +155,7 @@ struct qcom_swrm_ctrl {
 	u8 rd_cmd_id;
 	int irq;
 	unsigned int version;
+	int wake_irq;
 	int num_din_ports;
 	int num_dout_ports;
 	int cols_index;
@@ -503,6 +505,31 @@ static int qcom_swrm_enumerate(struct sdw_bus *bus)
 	return 0;
 }
 
+static irqreturn_t qcom_swrm_wake_irq_handler(int irq, void *dev_id)
+{
+	struct qcom_swrm_ctrl *swrm = dev_id;
+	int ret;
+
+	ret = pm_runtime_get_sync(swrm->dev);
+	if (ret < 0 && ret != -EACCES) {
+		dev_err_ratelimited(swrm->dev,
+				    "pm_runtime_get_sync failed in %s, ret %d\n",
+				    __func__, ret);
+		pm_runtime_put_noidle(swrm->dev);
+	}
+
+	if (swrm->wake_irq > 0) {
+		if (!irqd_irq_disabled(irq_get_irq_data(swrm->wake_irq)))
+			disable_irq_nosync(swrm->wake_irq);
+	}
+
+	pm_runtime_mark_last_busy(swrm->dev);
+	pm_runtime_put_autosuspend(swrm->dev);
+
+	return IRQ_HANDLED;
+}
+
+
 static irqreturn_t qcom_swrm_irq_handler(int irq, void *dev_id)
 {
 	struct qcom_swrm_ctrl *swrm = dev_id;
@@ -1340,6 +1367,19 @@ static int qcom_swrm_probe(struct platform_device *pdev)
 		goto err_clk;
 	}
 
+	ctrl->wake_irq = of_irq_get(dev->of_node, 1);
+	if (ctrl->wake_irq > 0) {
+		ret = devm_request_threaded_irq(dev, ctrl->wake_irq, NULL,
+						qcom_swrm_wake_irq_handler,
+						IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
+						"swr_wake_irq", ctrl);
+		if (ret) {
+			dev_err(dev, "Failed to request soundwire wake irq\n");
+			goto err_init;
+		}
+	}
+
+
 	ret = sdw_bus_master_add(&ctrl->bus, dev, dev->fwnode);
 	if (ret) {
 		dev_err(dev, "Failed to register Soundwire controller (%d)\n",
@@ -1424,6 +1464,11 @@ static int swrm_runtime_resume(struct device *dev)
 	struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(dev);
 	int ret;
 
+	if (ctrl->wake_irq > 0) {
+		if (!irqd_irq_disabled(irq_get_irq_data(ctrl->wake_irq)))
+			disable_irq_nosync(ctrl->wake_irq);
+	}
+
 	clk_prepare_enable(ctrl->hclk);
 
 	if (ctrl->clock_stop_not_supported) {
@@ -1491,6 +1536,11 @@ static int __maybe_unused swrm_runtime_suspend(struct device *dev)
 
 	usleep_range(300, 305);
 
+	if (ctrl->wake_irq > 0) {
+		if (irqd_irq_disabled(irq_get_irq_data(ctrl->wake_irq)))
+			enable_irq(ctrl->wake_irq);
+	}
+
 	return 0;
 }
 
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v3 3/3] soundwire: qcom: add in-band wake up interrupt support
@ 2022-02-28 17:25   ` Srinivas Kandagatla
  0 siblings, 0 replies; 24+ messages in thread
From: Srinivas Kandagatla @ 2022-02-28 17:25 UTC (permalink / raw)
  To: robh+dt, vkoul, yung-chuan.liao
  Cc: devicetree, alsa-devel, pierre-louis.bossart, linux-kernel,
	Srinivas Kandagatla, quic_srivasam

Some of the Qualcomm SoundWire Controller instances like the ones that are
connected to RX path along with Headset connections support Waking up
Controller from Low power clock stop state using SoundWire In-band interrupt.
SoundWire Slave on the bus would initiate this by pulling the data line high,
while the clock is stopped.

Add support to this wake up interrupt.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/soundwire/qcom.c | 50 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index 810232686196..e893aee1b057 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -14,6 +14,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/slab.h>
+#include <linux/pm_wakeirq.h>
 #include <linux/slimbus.h>
 #include <linux/soundwire/sdw.h>
 #include <linux/soundwire/sdw_registers.h>
@@ -154,6 +155,7 @@ struct qcom_swrm_ctrl {
 	u8 rd_cmd_id;
 	int irq;
 	unsigned int version;
+	int wake_irq;
 	int num_din_ports;
 	int num_dout_ports;
 	int cols_index;
@@ -503,6 +505,31 @@ static int qcom_swrm_enumerate(struct sdw_bus *bus)
 	return 0;
 }
 
+static irqreturn_t qcom_swrm_wake_irq_handler(int irq, void *dev_id)
+{
+	struct qcom_swrm_ctrl *swrm = dev_id;
+	int ret;
+
+	ret = pm_runtime_get_sync(swrm->dev);
+	if (ret < 0 && ret != -EACCES) {
+		dev_err_ratelimited(swrm->dev,
+				    "pm_runtime_get_sync failed in %s, ret %d\n",
+				    __func__, ret);
+		pm_runtime_put_noidle(swrm->dev);
+	}
+
+	if (swrm->wake_irq > 0) {
+		if (!irqd_irq_disabled(irq_get_irq_data(swrm->wake_irq)))
+			disable_irq_nosync(swrm->wake_irq);
+	}
+
+	pm_runtime_mark_last_busy(swrm->dev);
+	pm_runtime_put_autosuspend(swrm->dev);
+
+	return IRQ_HANDLED;
+}
+
+
 static irqreturn_t qcom_swrm_irq_handler(int irq, void *dev_id)
 {
 	struct qcom_swrm_ctrl *swrm = dev_id;
@@ -1340,6 +1367,19 @@ static int qcom_swrm_probe(struct platform_device *pdev)
 		goto err_clk;
 	}
 
+	ctrl->wake_irq = of_irq_get(dev->of_node, 1);
+	if (ctrl->wake_irq > 0) {
+		ret = devm_request_threaded_irq(dev, ctrl->wake_irq, NULL,
+						qcom_swrm_wake_irq_handler,
+						IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
+						"swr_wake_irq", ctrl);
+		if (ret) {
+			dev_err(dev, "Failed to request soundwire wake irq\n");
+			goto err_init;
+		}
+	}
+
+
 	ret = sdw_bus_master_add(&ctrl->bus, dev, dev->fwnode);
 	if (ret) {
 		dev_err(dev, "Failed to register Soundwire controller (%d)\n",
@@ -1424,6 +1464,11 @@ static int swrm_runtime_resume(struct device *dev)
 	struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(dev);
 	int ret;
 
+	if (ctrl->wake_irq > 0) {
+		if (!irqd_irq_disabled(irq_get_irq_data(ctrl->wake_irq)))
+			disable_irq_nosync(ctrl->wake_irq);
+	}
+
 	clk_prepare_enable(ctrl->hclk);
 
 	if (ctrl->clock_stop_not_supported) {
@@ -1491,6 +1536,11 @@ static int __maybe_unused swrm_runtime_suspend(struct device *dev)
 
 	usleep_range(300, 305);
 
+	if (ctrl->wake_irq > 0) {
+		if (irqd_irq_disabled(irq_get_irq_data(ctrl->wake_irq)))
+			enable_irq(ctrl->wake_irq);
+	}
+
 	return 0;
 }
 
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 3/3] soundwire: qcom: add in-band wake up interrupt support
  2022-02-28 17:25   ` Srinivas Kandagatla
  (?)
@ 2022-02-28 18:01   ` Pierre-Louis Bossart
  2022-03-01 11:13     ` Srinivas Kandagatla
  -1 siblings, 1 reply; 24+ messages in thread
From: Pierre-Louis Bossart @ 2022-02-28 18:01 UTC (permalink / raw)
  To: Srinivas Kandagatla, robh+dt, vkoul, yung-chuan.liao
  Cc: devicetree, alsa-devel, linux-kernel, quic_srivasam


> @@ -1424,6 +1464,11 @@ static int swrm_runtime_resume(struct device *dev)
>  	struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(dev);
>  	int ret;
>  
> +	if (ctrl->wake_irq > 0) {
> +		if (!irqd_irq_disabled(irq_get_irq_data(ctrl->wake_irq)))
> +			disable_irq_nosync(ctrl->wake_irq);
> +	}
> +
>  	clk_prepare_enable(ctrl->hclk);

This one is quite interesting. If you disable the IRQ mechanism but
haven't yet resumed the clock, that leaves a time window where the
peripheral could attempt to drive the line high. what happens in that case?

>  
>  	if (ctrl->clock_stop_not_supported) {
> @@ -1491,6 +1536,11 @@ static int __maybe_unused swrm_runtime_suspend(struct device *dev)
>  
>  	usleep_range(300, 305);
>  
> +	if (ctrl->wake_irq > 0) {
> +		if (irqd_irq_disabled(irq_get_irq_data(ctrl->wake_irq)))
> +			enable_irq(ctrl->wake_irq);
> +	}
> +

and this one is similar, you could have a case where the peripheral
signals a wake immediately after the ClockStopNow frame, but you may not
yet have enabled the wake detection interrupt.

Would that imply that the wake is missed?



>  	return 0;
>  }
>  

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 3/3] soundwire: qcom: add in-band wake up interrupt support
  2022-02-28 18:01   ` Pierre-Louis Bossart
@ 2022-03-01 11:13     ` Srinivas Kandagatla
  2022-03-01 13:51       ` Pierre-Louis Bossart
  0 siblings, 1 reply; 24+ messages in thread
From: Srinivas Kandagatla @ 2022-03-01 11:13 UTC (permalink / raw)
  To: Pierre-Louis Bossart, robh+dt, vkoul, yung-chuan.liao
  Cc: devicetree, alsa-devel, linux-kernel, quic_srivasam



On 28/02/2022 18:01, Pierre-Louis Bossart wrote:
> 
>> @@ -1424,6 +1464,11 @@ static int swrm_runtime_resume(struct device *dev)
>>   	struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(dev);
>>   	int ret;
>>   
>> +	if (ctrl->wake_irq > 0) {
>> +		if (!irqd_irq_disabled(irq_get_irq_data(ctrl->wake_irq)))
>> +			disable_irq_nosync(ctrl->wake_irq);
>> +	}
>> +
>>   	clk_prepare_enable(ctrl->hclk);
> 
> This one is quite interesting. If you disable the IRQ mechanism but
> haven't yet resumed the clock, that leaves a time window where the
> peripheral could attempt to drive the line high. what happens in that case?


We did call pm_runtime_get_sync() from Wake IRQ handler, which means 
that resume should be finished as part of Wake IRQ handler. Any new 
Interrupt conditions/status generated by slave in the meantime will be 
cleared while handling SLAVE PEND interrupt.

> 
>>   
>>   	if (ctrl->clock_stop_not_supported) {
>> @@ -1491,6 +1536,11 @@ static int __maybe_unused swrm_runtime_suspend(struct device *dev)
>>   
>>   	usleep_range(300, 305);
>>   
>> +	if (ctrl->wake_irq > 0) {
>> +		if (irqd_irq_disabled(irq_get_irq_data(ctrl->wake_irq)))
>> +			enable_irq(ctrl->wake_irq);
>> +	}
>> +
> 
> and this one is similar, you could have a case where the peripheral
> signals a wake immediately after the ClockStopNow frame, but you may not
> yet have enabled the wake detection interrupt.
> 
> Would that imply that the wake is missed?
Its Possible it might be missed at that instance, however as the Slave 
interrupt source condition/status (Ex: button Press) is still not 
cleared it should generate a Wake interrupt as soon as its enabled.

--srini
> 
> 
> 
>>   	return 0;
>>   }
>>   

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 3/3] soundwire: qcom: add in-band wake up interrupt support
  2022-03-01 11:13     ` Srinivas Kandagatla
@ 2022-03-01 13:51       ` Pierre-Louis Bossart
  0 siblings, 0 replies; 24+ messages in thread
From: Pierre-Louis Bossart @ 2022-03-01 13:51 UTC (permalink / raw)
  To: Srinivas Kandagatla, robh+dt, vkoul, yung-chuan.liao
  Cc: devicetree, alsa-devel, linux-kernel, quic_srivasam



On 3/1/22 05:13, Srinivas Kandagatla wrote:
> 
> 
> On 28/02/2022 18:01, Pierre-Louis Bossart wrote:
>>
>>> @@ -1424,6 +1464,11 @@ static int swrm_runtime_resume(struct device
>>> *dev)
>>>       struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(dev);
>>>       int ret;
>>>   +    if (ctrl->wake_irq > 0) {
>>> +        if (!irqd_irq_disabled(irq_get_irq_data(ctrl->wake_irq)))
>>> +            disable_irq_nosync(ctrl->wake_irq);
>>> +    }
>>> +
>>>       clk_prepare_enable(ctrl->hclk);
>>
>> This one is quite interesting. If you disable the IRQ mechanism but
>> haven't yet resumed the clock, that leaves a time window where the
>> peripheral could attempt to drive the line high. what happens in that
>> case?
> 
> 
> We did call pm_runtime_get_sync() from Wake IRQ handler, which means
> that resume should be finished as part of Wake IRQ handler. Any new
> Interrupt conditions/status generated by slave in the meantime will be
> cleared while handling SLAVE PEND interrupt.
> 
>>
>>>         if (ctrl->clock_stop_not_supported) {
>>> @@ -1491,6 +1536,11 @@ static int __maybe_unused
>>> swrm_runtime_suspend(struct device *dev)
>>>         usleep_range(300, 305);
>>>   +    if (ctrl->wake_irq > 0) {
>>> +        if (irqd_irq_disabled(irq_get_irq_data(ctrl->wake_irq)))
>>> +            enable_irq(ctrl->wake_irq);
>>> +    }
>>> +
>>
>> and this one is similar, you could have a case where the peripheral
>> signals a wake immediately after the ClockStopNow frame, but you may not
>> yet have enabled the wake detection interrupt.
>>
>> Would that imply that the wake is missed?
> Its Possible it might be missed at that instance, however as the Slave
> interrupt source condition/status (Ex: button Press) is still not
> cleared it should generate a Wake interrupt as soon as its enabled.

ok, thanks for the answers - both make sense.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 1/3] soundwire: qcom: add runtime pm support
  2022-02-28 17:25   ` Srinivas Kandagatla
@ 2022-03-01 13:52     ` Pierre-Louis Bossart
  -1 siblings, 0 replies; 24+ messages in thread
From: Pierre-Louis Bossart @ 2022-03-01 13:52 UTC (permalink / raw)
  To: Srinivas Kandagatla, robh+dt, vkoul, yung-chuan.liao
  Cc: devicetree, linux-kernel, alsa-devel, quic_srivasam



On 2/28/22 11:25, Srinivas Kandagatla wrote:
> Add support to runtime PM using SoundWire clock stop Mode0 on supported
> controller instances and soft reset on instances that do not support
> clock stop.
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

> ---
>  drivers/soundwire/qcom.c | 156 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 155 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
> index 54813417ef8e..810232686196 100644
> --- a/drivers/soundwire/qcom.c
> +++ b/drivers/soundwire/qcom.c
> @@ -11,6 +11,7 @@
>  #include <linux/of.h>
>  #include <linux/of_irq.h>
>  #include <linux/of_device.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/regmap.h>
>  #include <linux/slab.h>
>  #include <linux/slimbus.h>
> @@ -20,6 +21,9 @@
>  #include <sound/soc.h>
>  #include "bus.h"
>  
> +#define SWRM_COMP_SW_RESET					0x008
> +#define SWRM_COMP_STATUS					0x014
> +#define SWRM_FRM_GEN_ENABLED					BIT(0)
>  #define SWRM_COMP_HW_VERSION					0x00
>  #define SWRM_COMP_CFG_ADDR					0x04
>  #define SWRM_COMP_CFG_IRQ_LEVEL_OR_PULSE_MSK			BIT(1)
> @@ -29,6 +33,7 @@
>  #define SWRM_COMP_PARAMS_RD_FIFO_DEPTH				GENMASK(19, 15)
>  #define SWRM_COMP_PARAMS_DOUT_PORTS_MASK			GENMASK(4, 0)
>  #define SWRM_COMP_PARAMS_DIN_PORTS_MASK				GENMASK(9, 5)
> +#define SWRM_COMP_MASTER_ID					0x104
>  #define SWRM_INTERRUPT_STATUS					0x200
>  #define SWRM_INTERRUPT_STATUS_RMSK				GENMASK(16, 0)
>  #define SWRM_INTERRUPT_STATUS_SLAVE_PEND_IRQ			BIT(0)
> @@ -111,6 +116,13 @@
>  #define SWR_MAX_CMD_ID	14
>  #define MAX_FIFO_RD_RETRY 3
>  #define SWR_OVERFLOW_RETRY_COUNT 30
> +#define SWRM_LINK_STATUS_RETRY_CNT 100
> +
> +enum {
> +	MASTER_ID_WSA = 1,
> +	MASTER_ID_RX,
> +	MASTER_ID_TX
> +};
>  
>  struct qcom_swrm_port_config {
>  	u8 si;
> @@ -159,6 +171,7 @@ struct qcom_swrm_ctrl {
>  	u32 slave_status;
>  	u32 wr_fifo_depth;
>  	u32 rd_fifo_depth;
> +	bool clock_stop_not_supported;
>  };
>  
>  struct qcom_swrm_data {
> @@ -497,6 +510,7 @@ static irqreturn_t qcom_swrm_irq_handler(int irq, void *dev_id)
>  	u32 i;
>  	int devnum;
>  	int ret = IRQ_HANDLED;
> +	clk_prepare_enable(swrm->hclk);
>  
>  	swrm->reg_read(swrm, SWRM_INTERRUPT_STATUS, &intr_sts);
>  	intr_sts_masked = intr_sts & swrm->intr_mask;
> @@ -604,6 +618,7 @@ static irqreturn_t qcom_swrm_irq_handler(int irq, void *dev_id)
>  		intr_sts_masked = intr_sts & swrm->intr_mask;
>  	} while (intr_sts_masked);
>  
> +	clk_disable_unprepare(swrm->hclk);
>  	return ret;
>  }
>  
> @@ -1017,6 +1032,15 @@ static int qcom_swrm_startup(struct snd_pcm_substream *substream,
>  	struct snd_soc_dai *codec_dai;
>  	int ret, i;
>  
> +	ret = pm_runtime_get_sync(ctrl->dev);
> +	if (ret < 0 && ret != -EACCES) {
> +		dev_err_ratelimited(ctrl->dev,
> +				    "pm_runtime_get_sync failed in %s, ret %d\n",
> +				    __func__, ret);
> +		pm_runtime_put_noidle(ctrl->dev);
> +		return ret;
> +	}
> +
>  	sruntime = sdw_alloc_stream(dai->name);
>  	if (!sruntime)
>  		return -ENOMEM;
> @@ -1044,6 +1068,9 @@ static void qcom_swrm_shutdown(struct snd_pcm_substream *substream,
>  
>  	sdw_release_stream(ctrl->sruntime[dai->id]);
>  	ctrl->sruntime[dai->id] = NULL;
> +	pm_runtime_mark_last_busy(ctrl->dev);
> +	pm_runtime_put_autosuspend(ctrl->dev);
> +
>  }
>  
>  static const struct snd_soc_dai_ops qcom_swrm_pdm_dai_ops = {
> @@ -1197,12 +1224,23 @@ static int qcom_swrm_get_port_config(struct qcom_swrm_ctrl *ctrl)
>  static int swrm_reg_show(struct seq_file *s_file, void *data)
>  {
>  	struct qcom_swrm_ctrl *swrm = s_file->private;
> -	int reg, reg_val;
> +	int reg, reg_val, ret;
> +
> +	ret = pm_runtime_get_sync(swrm->dev);
> +	if (ret < 0 && ret != -EACCES) {
> +		dev_err_ratelimited(swrm->dev,
> +				    "pm_runtime_get_sync failed in %s, ret %d\n",
> +				    __func__, ret);
> +		pm_runtime_put_noidle(swrm->dev);
> +	}
>  
>  	for (reg = 0; reg <= SWR_MSTR_MAX_REG_ADDR; reg += 4) {
>  		swrm->reg_read(swrm, reg, &reg_val);
>  		seq_printf(s_file, "0x%.3x: 0x%.2x\n", reg, reg_val);
>  	}
> +	pm_runtime_mark_last_busy(swrm->dev);
> +	pm_runtime_put_autosuspend(swrm->dev);
> +
>  
>  	return 0;
>  }
> @@ -1267,6 +1305,7 @@ static int qcom_swrm_probe(struct platform_device *pdev)
>  	ctrl->bus.ops = &qcom_swrm_ops;
>  	ctrl->bus.port_ops = &qcom_swrm_port_ops;
>  	ctrl->bus.compute_params = &qcom_swrm_compute_params;
> +	ctrl->bus.clk_stop_timeout = 300;
>  
>  	ret = qcom_swrm_get_port_config(ctrl);
>  	if (ret)
> @@ -1319,6 +1358,21 @@ static int qcom_swrm_probe(struct platform_device *pdev)
>  		 (ctrl->version >> 24) & 0xff, (ctrl->version >> 16) & 0xff,
>  		 ctrl->version & 0xffff);
>  
> +	pm_runtime_set_autosuspend_delay(dev, 3000);
> +	pm_runtime_use_autosuspend(dev);
> +	pm_runtime_mark_last_busy(dev);
> +	pm_runtime_set_active(dev);
> +	pm_runtime_enable(dev);
> +
> +	/* Clk stop is not supported on WSA Soundwire masters */
> +	if (ctrl->version <= 0x01030000) {
> +		ctrl->clock_stop_not_supported = true;
> +	} else {
> +		ctrl->reg_read(ctrl, SWRM_COMP_MASTER_ID, &val);
> +		if (val == MASTER_ID_WSA)
> +			ctrl->clock_stop_not_supported = true;
> +	}
> +
>  #ifdef CONFIG_DEBUG_FS
>  	ctrl->debugfs = debugfs_create_dir("qualcomm-sdw", ctrl->bus.debugfs);
>  	debugfs_create_file("qualcomm-registers", 0400, ctrl->debugfs, ctrl,
> @@ -1345,6 +1399,105 @@ static int qcom_swrm_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +static bool swrm_wait_for_frame_gen_enabled(struct qcom_swrm_ctrl *swrm)
> +{
> +	int retry = SWRM_LINK_STATUS_RETRY_CNT;
> +	int comp_sts;
> +
> +	do {
> +		swrm->reg_read(swrm, SWRM_COMP_STATUS, &comp_sts);
> +
> +		if (comp_sts & SWRM_FRM_GEN_ENABLED)
> +			return true;
> +
> +		usleep_range(500, 510);
> +	} while (retry--);
> +
> +	dev_err(swrm->dev, "%s: link status not %s\n", __func__,
> +		comp_sts && SWRM_FRM_GEN_ENABLED ? "connected" : "disconnected");
> +
> +	return false;
> +}
> +
> +static int swrm_runtime_resume(struct device *dev)
> +{
> +	struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(dev);
> +	int ret;
> +
> +	clk_prepare_enable(ctrl->hclk);
> +
> +	if (ctrl->clock_stop_not_supported) {
> +		reinit_completion(&ctrl->enumeration);
> +		ctrl->reg_write(ctrl, SWRM_COMP_SW_RESET, 0x01);
> +		usleep_range(100, 105);
> +
> +		qcom_swrm_init(ctrl);
> +
> +		usleep_range(100, 105);
> +		if (!swrm_wait_for_frame_gen_enabled(ctrl))
> +			dev_err(ctrl->dev, "link failed to connect\n");
> +
> +		/* wait for hw enumeration to complete */
> +		wait_for_completion_timeout(&ctrl->enumeration,
> +					    msecs_to_jiffies(TIMEOUT_MS));
> +		qcom_swrm_get_device_status(ctrl);
> +		sdw_handle_slave_status(&ctrl->bus, ctrl->status);
> +	} else {
> +		ctrl->reg_write(ctrl, SWRM_MCP_BUS_CTRL, SWRM_MCP_BUS_CLK_START);
> +		ctrl->reg_write(ctrl, SWRM_INTERRUPT_CLEAR,
> +			SWRM_INTERRUPT_STATUS_MASTER_CLASH_DET);
> +
> +		ctrl->intr_mask |= SWRM_INTERRUPT_STATUS_MASTER_CLASH_DET;
> +		ctrl->reg_write(ctrl, SWRM_INTERRUPT_MASK_ADDR, ctrl->intr_mask);
> +		ctrl->reg_write(ctrl, SWRM_INTERRUPT_CPU_EN, ctrl->intr_mask);
> +
> +		usleep_range(100, 105);
> +		if (!swrm_wait_for_frame_gen_enabled(ctrl))
> +			dev_err(ctrl->dev, "link failed to connect\n");
> +
> +		ret = sdw_bus_exit_clk_stop(&ctrl->bus);
> +		if (ret < 0)
> +			dev_err(ctrl->dev, "bus failed to exit clock stop %d\n", ret);
> +	}
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused swrm_runtime_suspend(struct device *dev)
> +{
> +	struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(dev);
> +	int ret;
> +
> +	if (!ctrl->clock_stop_not_supported) {
> +		/* Mask bus clash interrupt */
> +		ctrl->intr_mask &= ~SWRM_INTERRUPT_STATUS_MASTER_CLASH_DET;
> +		ctrl->reg_write(ctrl, SWRM_INTERRUPT_MASK_ADDR, ctrl->intr_mask);
> +		ctrl->reg_write(ctrl, SWRM_INTERRUPT_CPU_EN, ctrl->intr_mask);
> +		/* Prepare slaves for clock stop */
> +		ret = sdw_bus_prep_clk_stop(&ctrl->bus);
> +		if (ret < 0 && ret != -ENODATA) {
> +			dev_err(dev, "prepare clock stop failed %d", ret);
> +			return ret;
> +		}
> +
> +		ret = sdw_bus_clk_stop(&ctrl->bus);
> +		if (ret < 0 && ret != -ENODATA) {
> +			dev_err(dev, "bus clock stop failed %d", ret);
> +			return ret;
> +		}
> +	}
> +
> +	clk_disable_unprepare(ctrl->hclk);
> +
> +	usleep_range(300, 305);
> +
> +	return 0;
> +}
> +
> +static const struct dev_pm_ops swrm_dev_pm_ops = {
> +	SET_RUNTIME_PM_OPS(swrm_runtime_suspend, swrm_runtime_resume, NULL)
> +};
> +
>  static const struct of_device_id qcom_swrm_of_match[] = {
>  	{ .compatible = "qcom,soundwire-v1.3.0", .data = &swrm_v1_3_data },
>  	{ .compatible = "qcom,soundwire-v1.5.1", .data = &swrm_v1_5_data },
> @@ -1359,6 +1512,7 @@ static struct platform_driver qcom_swrm_driver = {
>  	.driver = {
>  		.name	= "qcom-soundwire",
>  		.of_match_table = qcom_swrm_of_match,
> +		.pm = &swrm_dev_pm_ops,
>  	}
>  };
>  module_platform_driver(qcom_swrm_driver);

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 1/3] soundwire: qcom: add runtime pm support
@ 2022-03-01 13:52     ` Pierre-Louis Bossart
  0 siblings, 0 replies; 24+ messages in thread
From: Pierre-Louis Bossart @ 2022-03-01 13:52 UTC (permalink / raw)
  To: Srinivas Kandagatla, robh+dt, vkoul, yung-chuan.liao
  Cc: devicetree, alsa-devel, linux-kernel, quic_srivasam



On 2/28/22 11:25, Srinivas Kandagatla wrote:
> Add support to runtime PM using SoundWire clock stop Mode0 on supported
> controller instances and soft reset on instances that do not support
> clock stop.
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

> ---
>  drivers/soundwire/qcom.c | 156 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 155 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
> index 54813417ef8e..810232686196 100644
> --- a/drivers/soundwire/qcom.c
> +++ b/drivers/soundwire/qcom.c
> @@ -11,6 +11,7 @@
>  #include <linux/of.h>
>  #include <linux/of_irq.h>
>  #include <linux/of_device.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/regmap.h>
>  #include <linux/slab.h>
>  #include <linux/slimbus.h>
> @@ -20,6 +21,9 @@
>  #include <sound/soc.h>
>  #include "bus.h"
>  
> +#define SWRM_COMP_SW_RESET					0x008
> +#define SWRM_COMP_STATUS					0x014
> +#define SWRM_FRM_GEN_ENABLED					BIT(0)
>  #define SWRM_COMP_HW_VERSION					0x00
>  #define SWRM_COMP_CFG_ADDR					0x04
>  #define SWRM_COMP_CFG_IRQ_LEVEL_OR_PULSE_MSK			BIT(1)
> @@ -29,6 +33,7 @@
>  #define SWRM_COMP_PARAMS_RD_FIFO_DEPTH				GENMASK(19, 15)
>  #define SWRM_COMP_PARAMS_DOUT_PORTS_MASK			GENMASK(4, 0)
>  #define SWRM_COMP_PARAMS_DIN_PORTS_MASK				GENMASK(9, 5)
> +#define SWRM_COMP_MASTER_ID					0x104
>  #define SWRM_INTERRUPT_STATUS					0x200
>  #define SWRM_INTERRUPT_STATUS_RMSK				GENMASK(16, 0)
>  #define SWRM_INTERRUPT_STATUS_SLAVE_PEND_IRQ			BIT(0)
> @@ -111,6 +116,13 @@
>  #define SWR_MAX_CMD_ID	14
>  #define MAX_FIFO_RD_RETRY 3
>  #define SWR_OVERFLOW_RETRY_COUNT 30
> +#define SWRM_LINK_STATUS_RETRY_CNT 100
> +
> +enum {
> +	MASTER_ID_WSA = 1,
> +	MASTER_ID_RX,
> +	MASTER_ID_TX
> +};
>  
>  struct qcom_swrm_port_config {
>  	u8 si;
> @@ -159,6 +171,7 @@ struct qcom_swrm_ctrl {
>  	u32 slave_status;
>  	u32 wr_fifo_depth;
>  	u32 rd_fifo_depth;
> +	bool clock_stop_not_supported;
>  };
>  
>  struct qcom_swrm_data {
> @@ -497,6 +510,7 @@ static irqreturn_t qcom_swrm_irq_handler(int irq, void *dev_id)
>  	u32 i;
>  	int devnum;
>  	int ret = IRQ_HANDLED;
> +	clk_prepare_enable(swrm->hclk);
>  
>  	swrm->reg_read(swrm, SWRM_INTERRUPT_STATUS, &intr_sts);
>  	intr_sts_masked = intr_sts & swrm->intr_mask;
> @@ -604,6 +618,7 @@ static irqreturn_t qcom_swrm_irq_handler(int irq, void *dev_id)
>  		intr_sts_masked = intr_sts & swrm->intr_mask;
>  	} while (intr_sts_masked);
>  
> +	clk_disable_unprepare(swrm->hclk);
>  	return ret;
>  }
>  
> @@ -1017,6 +1032,15 @@ static int qcom_swrm_startup(struct snd_pcm_substream *substream,
>  	struct snd_soc_dai *codec_dai;
>  	int ret, i;
>  
> +	ret = pm_runtime_get_sync(ctrl->dev);
> +	if (ret < 0 && ret != -EACCES) {
> +		dev_err_ratelimited(ctrl->dev,
> +				    "pm_runtime_get_sync failed in %s, ret %d\n",
> +				    __func__, ret);
> +		pm_runtime_put_noidle(ctrl->dev);
> +		return ret;
> +	}
> +
>  	sruntime = sdw_alloc_stream(dai->name);
>  	if (!sruntime)
>  		return -ENOMEM;
> @@ -1044,6 +1068,9 @@ static void qcom_swrm_shutdown(struct snd_pcm_substream *substream,
>  
>  	sdw_release_stream(ctrl->sruntime[dai->id]);
>  	ctrl->sruntime[dai->id] = NULL;
> +	pm_runtime_mark_last_busy(ctrl->dev);
> +	pm_runtime_put_autosuspend(ctrl->dev);
> +
>  }
>  
>  static const struct snd_soc_dai_ops qcom_swrm_pdm_dai_ops = {
> @@ -1197,12 +1224,23 @@ static int qcom_swrm_get_port_config(struct qcom_swrm_ctrl *ctrl)
>  static int swrm_reg_show(struct seq_file *s_file, void *data)
>  {
>  	struct qcom_swrm_ctrl *swrm = s_file->private;
> -	int reg, reg_val;
> +	int reg, reg_val, ret;
> +
> +	ret = pm_runtime_get_sync(swrm->dev);
> +	if (ret < 0 && ret != -EACCES) {
> +		dev_err_ratelimited(swrm->dev,
> +				    "pm_runtime_get_sync failed in %s, ret %d\n",
> +				    __func__, ret);
> +		pm_runtime_put_noidle(swrm->dev);
> +	}
>  
>  	for (reg = 0; reg <= SWR_MSTR_MAX_REG_ADDR; reg += 4) {
>  		swrm->reg_read(swrm, reg, &reg_val);
>  		seq_printf(s_file, "0x%.3x: 0x%.2x\n", reg, reg_val);
>  	}
> +	pm_runtime_mark_last_busy(swrm->dev);
> +	pm_runtime_put_autosuspend(swrm->dev);
> +
>  
>  	return 0;
>  }
> @@ -1267,6 +1305,7 @@ static int qcom_swrm_probe(struct platform_device *pdev)
>  	ctrl->bus.ops = &qcom_swrm_ops;
>  	ctrl->bus.port_ops = &qcom_swrm_port_ops;
>  	ctrl->bus.compute_params = &qcom_swrm_compute_params;
> +	ctrl->bus.clk_stop_timeout = 300;
>  
>  	ret = qcom_swrm_get_port_config(ctrl);
>  	if (ret)
> @@ -1319,6 +1358,21 @@ static int qcom_swrm_probe(struct platform_device *pdev)
>  		 (ctrl->version >> 24) & 0xff, (ctrl->version >> 16) & 0xff,
>  		 ctrl->version & 0xffff);
>  
> +	pm_runtime_set_autosuspend_delay(dev, 3000);
> +	pm_runtime_use_autosuspend(dev);
> +	pm_runtime_mark_last_busy(dev);
> +	pm_runtime_set_active(dev);
> +	pm_runtime_enable(dev);
> +
> +	/* Clk stop is not supported on WSA Soundwire masters */
> +	if (ctrl->version <= 0x01030000) {
> +		ctrl->clock_stop_not_supported = true;
> +	} else {
> +		ctrl->reg_read(ctrl, SWRM_COMP_MASTER_ID, &val);
> +		if (val == MASTER_ID_WSA)
> +			ctrl->clock_stop_not_supported = true;
> +	}
> +
>  #ifdef CONFIG_DEBUG_FS
>  	ctrl->debugfs = debugfs_create_dir("qualcomm-sdw", ctrl->bus.debugfs);
>  	debugfs_create_file("qualcomm-registers", 0400, ctrl->debugfs, ctrl,
> @@ -1345,6 +1399,105 @@ static int qcom_swrm_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +static bool swrm_wait_for_frame_gen_enabled(struct qcom_swrm_ctrl *swrm)
> +{
> +	int retry = SWRM_LINK_STATUS_RETRY_CNT;
> +	int comp_sts;
> +
> +	do {
> +		swrm->reg_read(swrm, SWRM_COMP_STATUS, &comp_sts);
> +
> +		if (comp_sts & SWRM_FRM_GEN_ENABLED)
> +			return true;
> +
> +		usleep_range(500, 510);
> +	} while (retry--);
> +
> +	dev_err(swrm->dev, "%s: link status not %s\n", __func__,
> +		comp_sts && SWRM_FRM_GEN_ENABLED ? "connected" : "disconnected");
> +
> +	return false;
> +}
> +
> +static int swrm_runtime_resume(struct device *dev)
> +{
> +	struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(dev);
> +	int ret;
> +
> +	clk_prepare_enable(ctrl->hclk);
> +
> +	if (ctrl->clock_stop_not_supported) {
> +		reinit_completion(&ctrl->enumeration);
> +		ctrl->reg_write(ctrl, SWRM_COMP_SW_RESET, 0x01);
> +		usleep_range(100, 105);
> +
> +		qcom_swrm_init(ctrl);
> +
> +		usleep_range(100, 105);
> +		if (!swrm_wait_for_frame_gen_enabled(ctrl))
> +			dev_err(ctrl->dev, "link failed to connect\n");
> +
> +		/* wait for hw enumeration to complete */
> +		wait_for_completion_timeout(&ctrl->enumeration,
> +					    msecs_to_jiffies(TIMEOUT_MS));
> +		qcom_swrm_get_device_status(ctrl);
> +		sdw_handle_slave_status(&ctrl->bus, ctrl->status);
> +	} else {
> +		ctrl->reg_write(ctrl, SWRM_MCP_BUS_CTRL, SWRM_MCP_BUS_CLK_START);
> +		ctrl->reg_write(ctrl, SWRM_INTERRUPT_CLEAR,
> +			SWRM_INTERRUPT_STATUS_MASTER_CLASH_DET);
> +
> +		ctrl->intr_mask |= SWRM_INTERRUPT_STATUS_MASTER_CLASH_DET;
> +		ctrl->reg_write(ctrl, SWRM_INTERRUPT_MASK_ADDR, ctrl->intr_mask);
> +		ctrl->reg_write(ctrl, SWRM_INTERRUPT_CPU_EN, ctrl->intr_mask);
> +
> +		usleep_range(100, 105);
> +		if (!swrm_wait_for_frame_gen_enabled(ctrl))
> +			dev_err(ctrl->dev, "link failed to connect\n");
> +
> +		ret = sdw_bus_exit_clk_stop(&ctrl->bus);
> +		if (ret < 0)
> +			dev_err(ctrl->dev, "bus failed to exit clock stop %d\n", ret);
> +	}
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused swrm_runtime_suspend(struct device *dev)
> +{
> +	struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(dev);
> +	int ret;
> +
> +	if (!ctrl->clock_stop_not_supported) {
> +		/* Mask bus clash interrupt */
> +		ctrl->intr_mask &= ~SWRM_INTERRUPT_STATUS_MASTER_CLASH_DET;
> +		ctrl->reg_write(ctrl, SWRM_INTERRUPT_MASK_ADDR, ctrl->intr_mask);
> +		ctrl->reg_write(ctrl, SWRM_INTERRUPT_CPU_EN, ctrl->intr_mask);
> +		/* Prepare slaves for clock stop */
> +		ret = sdw_bus_prep_clk_stop(&ctrl->bus);
> +		if (ret < 0 && ret != -ENODATA) {
> +			dev_err(dev, "prepare clock stop failed %d", ret);
> +			return ret;
> +		}
> +
> +		ret = sdw_bus_clk_stop(&ctrl->bus);
> +		if (ret < 0 && ret != -ENODATA) {
> +			dev_err(dev, "bus clock stop failed %d", ret);
> +			return ret;
> +		}
> +	}
> +
> +	clk_disable_unprepare(ctrl->hclk);
> +
> +	usleep_range(300, 305);
> +
> +	return 0;
> +}
> +
> +static const struct dev_pm_ops swrm_dev_pm_ops = {
> +	SET_RUNTIME_PM_OPS(swrm_runtime_suspend, swrm_runtime_resume, NULL)
> +};
> +
>  static const struct of_device_id qcom_swrm_of_match[] = {
>  	{ .compatible = "qcom,soundwire-v1.3.0", .data = &swrm_v1_3_data },
>  	{ .compatible = "qcom,soundwire-v1.5.1", .data = &swrm_v1_5_data },
> @@ -1359,6 +1512,7 @@ static struct platform_driver qcom_swrm_driver = {
>  	.driver = {
>  		.name	= "qcom-soundwire",
>  		.of_match_table = qcom_swrm_of_match,
> +		.pm = &swrm_dev_pm_ops,
>  	}
>  };
>  module_platform_driver(qcom_swrm_driver);

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 3/3] soundwire: qcom: add in-band wake up interrupt support
  2022-02-28 17:25   ` Srinivas Kandagatla
  (?)
  (?)
@ 2022-03-01 13:52   ` Pierre-Louis Bossart
  2022-04-20 17:40     ` Pierre-Louis Bossart
  -1 siblings, 1 reply; 24+ messages in thread
From: Pierre-Louis Bossart @ 2022-03-01 13:52 UTC (permalink / raw)
  To: Srinivas Kandagatla, robh+dt, vkoul, yung-chuan.liao
  Cc: devicetree, alsa-devel, linux-kernel, quic_srivasam



On 2/28/22 11:25, Srinivas Kandagatla wrote:
> Some of the Qualcomm SoundWire Controller instances like the ones that are
> connected to RX path along with Headset connections support Waking up
> Controller from Low power clock stop state using SoundWire In-band interrupt.
> SoundWire Slave on the bus would initiate this by pulling the data line high,
> while the clock is stopped.
> 
> Add support to this wake up interrupt.
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

> ---
>  drivers/soundwire/qcom.c | 50 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
> 
> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
> index 810232686196..e893aee1b057 100644
> --- a/drivers/soundwire/qcom.c
> +++ b/drivers/soundwire/qcom.c
> @@ -14,6 +14,7 @@
>  #include <linux/pm_runtime.h>
>  #include <linux/regmap.h>
>  #include <linux/slab.h>
> +#include <linux/pm_wakeirq.h>
>  #include <linux/slimbus.h>
>  #include <linux/soundwire/sdw.h>
>  #include <linux/soundwire/sdw_registers.h>
> @@ -154,6 +155,7 @@ struct qcom_swrm_ctrl {
>  	u8 rd_cmd_id;
>  	int irq;
>  	unsigned int version;
> +	int wake_irq;
>  	int num_din_ports;
>  	int num_dout_ports;
>  	int cols_index;
> @@ -503,6 +505,31 @@ static int qcom_swrm_enumerate(struct sdw_bus *bus)
>  	return 0;
>  }
>  
> +static irqreturn_t qcom_swrm_wake_irq_handler(int irq, void *dev_id)
> +{
> +	struct qcom_swrm_ctrl *swrm = dev_id;
> +	int ret;
> +
> +	ret = pm_runtime_get_sync(swrm->dev);
> +	if (ret < 0 && ret != -EACCES) {
> +		dev_err_ratelimited(swrm->dev,
> +				    "pm_runtime_get_sync failed in %s, ret %d\n",
> +				    __func__, ret);
> +		pm_runtime_put_noidle(swrm->dev);
> +	}
> +
> +	if (swrm->wake_irq > 0) {
> +		if (!irqd_irq_disabled(irq_get_irq_data(swrm->wake_irq)))
> +			disable_irq_nosync(swrm->wake_irq);
> +	}
> +
> +	pm_runtime_mark_last_busy(swrm->dev);
> +	pm_runtime_put_autosuspend(swrm->dev);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +
>  static irqreturn_t qcom_swrm_irq_handler(int irq, void *dev_id)
>  {
>  	struct qcom_swrm_ctrl *swrm = dev_id;
> @@ -1340,6 +1367,19 @@ static int qcom_swrm_probe(struct platform_device *pdev)
>  		goto err_clk;
>  	}
>  
> +	ctrl->wake_irq = of_irq_get(dev->of_node, 1);
> +	if (ctrl->wake_irq > 0) {
> +		ret = devm_request_threaded_irq(dev, ctrl->wake_irq, NULL,
> +						qcom_swrm_wake_irq_handler,
> +						IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
> +						"swr_wake_irq", ctrl);
> +		if (ret) {
> +			dev_err(dev, "Failed to request soundwire wake irq\n");
> +			goto err_init;
> +		}
> +	}
> +
> +
>  	ret = sdw_bus_master_add(&ctrl->bus, dev, dev->fwnode);
>  	if (ret) {
>  		dev_err(dev, "Failed to register Soundwire controller (%d)\n",
> @@ -1424,6 +1464,11 @@ static int swrm_runtime_resume(struct device *dev)
>  	struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(dev);
>  	int ret;
>  
> +	if (ctrl->wake_irq > 0) {
> +		if (!irqd_irq_disabled(irq_get_irq_data(ctrl->wake_irq)))
> +			disable_irq_nosync(ctrl->wake_irq);
> +	}
> +
>  	clk_prepare_enable(ctrl->hclk);
>  
>  	if (ctrl->clock_stop_not_supported) {
> @@ -1491,6 +1536,11 @@ static int __maybe_unused swrm_runtime_suspend(struct device *dev)
>  
>  	usleep_range(300, 305);
>  
> +	if (ctrl->wake_irq > 0) {
> +		if (irqd_irq_disabled(irq_get_irq_data(ctrl->wake_irq)))
> +			enable_irq(ctrl->wake_irq);
> +	}
> +
>  	return 0;
>  }
>  

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 0/3] soundwire: qcom: add pm runtime support
  2022-02-28 17:25 ` Srinivas Kandagatla
@ 2022-03-02 15:42   ` Vinod Koul
  -1 siblings, 0 replies; 24+ messages in thread
From: Vinod Koul @ 2022-03-02 15:42 UTC (permalink / raw)
  To: Srinivas Kandagatla
  Cc: robh+dt, yung-chuan.liao, pierre-louis.bossart, devicetree,
	linux-kernel, alsa-devel, quic_srivasam

On 28-02-22, 17:25, Srinivas Kandagatla wrote:
> This patchset adds pm runtime support to Qualcomm SounWire Controller using
> SoundWire Clock Stop and Wake up using Headset events on supported instances and
> instances like WSA which do not support clock stop a soft reset of controller
> along with full rest of slaves is done to resume from a low power state.
> 
> Tested it on SM8250 MTP and Dragon Board DB845c

Applied, thanks

-- 
~Vinod

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 0/3] soundwire: qcom: add pm runtime support
@ 2022-03-02 15:42   ` Vinod Koul
  0 siblings, 0 replies; 24+ messages in thread
From: Vinod Koul @ 2022-03-02 15:42 UTC (permalink / raw)
  To: Srinivas Kandagatla
  Cc: devicetree, alsa-devel, pierre-louis.bossart, linux-kernel,
	robh+dt, yung-chuan.liao, quic_srivasam

On 28-02-22, 17:25, Srinivas Kandagatla wrote:
> This patchset adds pm runtime support to Qualcomm SounWire Controller using
> SoundWire Clock Stop and Wake up using Headset events on supported instances and
> instances like WSA which do not support clock stop a soft reset of controller
> along with full rest of slaves is done to resume from a low power state.
> 
> Tested it on SM8250 MTP and Dragon Board DB845c

Applied, thanks

-- 
~Vinod

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 1/3] soundwire: qcom: add runtime pm support
  2022-02-28 17:25   ` Srinivas Kandagatla
@ 2022-03-03 10:52     ` Geert Uytterhoeven
  -1 siblings, 0 replies; 24+ messages in thread
From: Geert Uytterhoeven @ 2022-03-03 10:52 UTC (permalink / raw)
  To: Srinivas Kandagatla
  Cc: Rob Herring, Vinod, yung-chuan.liao, Pierre-Louis Bossart,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux Kernel Mailing List, ALSA Development Mailing List,
	quic_srivasam

Hi Srinivas,

On Mon, Feb 28, 2022 at 7:13 PM Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> wrote:
> Add support to runtime PM using SoundWire clock stop Mode0 on supported
> controller instances and soft reset on instances that do not support
> clock stop.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Thanks for your patch, which is now commit 74e79da9fd46a597
("soundwire: qcom: add runtime pm support") in next-20220303.

> --- a/drivers/soundwire/qcom.c
> +++ b/drivers/soundwire/qcom.c

> @@ -1345,6 +1399,105 @@ static int qcom_swrm_remove(struct platform_device *pdev)
>         return 0;
>  }
>
> +static bool swrm_wait_for_frame_gen_enabled(struct qcom_swrm_ctrl *swrm)
> +{

[...]

> +}
> +
> +static int __maybe_unused swrm_runtime_suspend(struct device *dev)

This is causing build failures if CONFIG_PM=n:

drivers/soundwire/qcom.c:1460:12: error: 'swrm_runtime_resume' defined
but not used [-Werror=unused-function]

http://kisskb.ellerman.id.au/kisskb/buildresult/14704362/

> +{

[...]

> +}
> +
> +static const struct dev_pm_ops swrm_dev_pm_ops = {
> +       SET_RUNTIME_PM_OPS(swrm_runtime_suspend, swrm_runtime_resume, NULL)
> +};
> +
>  static const struct of_device_id qcom_swrm_of_match[] = {
>         { .compatible = "qcom,soundwire-v1.3.0", .data = &swrm_v1_3_data },
>         { .compatible = "qcom,soundwire-v1.5.1", .data = &swrm_v1_5_data },

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 1/3] soundwire: qcom: add runtime pm support
@ 2022-03-03 10:52     ` Geert Uytterhoeven
  0 siblings, 0 replies; 24+ messages in thread
From: Geert Uytterhoeven @ 2022-03-03 10:52 UTC (permalink / raw)
  To: Srinivas Kandagatla
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	ALSA Development Mailing List, Pierre-Louis Bossart,
	Linux Kernel Mailing List, Vinod, Rob Herring, yung-chuan.liao,
	quic_srivasam

Hi Srinivas,

On Mon, Feb 28, 2022 at 7:13 PM Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> wrote:
> Add support to runtime PM using SoundWire clock stop Mode0 on supported
> controller instances and soft reset on instances that do not support
> clock stop.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Thanks for your patch, which is now commit 74e79da9fd46a597
("soundwire: qcom: add runtime pm support") in next-20220303.

> --- a/drivers/soundwire/qcom.c
> +++ b/drivers/soundwire/qcom.c

> @@ -1345,6 +1399,105 @@ static int qcom_swrm_remove(struct platform_device *pdev)
>         return 0;
>  }
>
> +static bool swrm_wait_for_frame_gen_enabled(struct qcom_swrm_ctrl *swrm)
> +{

[...]

> +}
> +
> +static int __maybe_unused swrm_runtime_suspend(struct device *dev)

This is causing build failures if CONFIG_PM=n:

drivers/soundwire/qcom.c:1460:12: error: 'swrm_runtime_resume' defined
but not used [-Werror=unused-function]

http://kisskb.ellerman.id.au/kisskb/buildresult/14704362/

> +{

[...]

> +}
> +
> +static const struct dev_pm_ops swrm_dev_pm_ops = {
> +       SET_RUNTIME_PM_OPS(swrm_runtime_suspend, swrm_runtime_resume, NULL)
> +};
> +
>  static const struct of_device_id qcom_swrm_of_match[] = {
>         { .compatible = "qcom,soundwire-v1.3.0", .data = &swrm_v1_3_data },
>         { .compatible = "qcom,soundwire-v1.5.1", .data = &swrm_v1_5_data },

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 0/3] soundwire: qcom: add pm runtime support
  2022-03-02 15:42   ` Vinod Koul
@ 2022-04-04 21:42     ` Amit Pundir
  -1 siblings, 0 replies; 24+ messages in thread
From: Amit Pundir @ 2022-04-04 21:42 UTC (permalink / raw)
  To: Vinod Koul
  Cc: devicetree, alsa-devel, pierre-louis.bossart, linux-kernel,
	robh+dt, Srinivas Kandagatla, yung-chuan.liao, quic_srivasam

On Wed, 2 Mar 2022 at 21:13, Vinod Koul <vkoul@kernel.org> wrote:
>
> On 28-02-22, 17:25, Srinivas Kandagatla wrote:
> > This patchset adds pm runtime support to Qualcomm SounWire Controller using
> > SoundWire Clock Stop and Wake up using Headset events on supported instances and
> > instances like WSA which do not support clock stop a soft reset of controller
> > along with full rest of slaves is done to resume from a low power state.
> >
> > Tested it on SM8250 MTP and Dragon Board DB845c
>
> Applied, thanks
>

Hi, this patch series broke audio on SDM845 running AOSP. I can
reproduce it on both DB845c and PocoF1
https://www.toptal.com/developers/hastebin/raw/rodazupayu. It is not
100% reproducible but can be triggered on every alternate reboot or
so.

Regards,
Amit Pundir

> --
> ~Vinod

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 0/3] soundwire: qcom: add pm runtime support
@ 2022-04-04 21:42     ` Amit Pundir
  0 siblings, 0 replies; 24+ messages in thread
From: Amit Pundir @ 2022-04-04 21:42 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Srinivas Kandagatla, robh+dt, yung-chuan.liao,
	pierre-louis.bossart, devicetree, linux-kernel, alsa-devel,
	quic_srivasam

On Wed, 2 Mar 2022 at 21:13, Vinod Koul <vkoul@kernel.org> wrote:
>
> On 28-02-22, 17:25, Srinivas Kandagatla wrote:
> > This patchset adds pm runtime support to Qualcomm SounWire Controller using
> > SoundWire Clock Stop and Wake up using Headset events on supported instances and
> > instances like WSA which do not support clock stop a soft reset of controller
> > along with full rest of slaves is done to resume from a low power state.
> >
> > Tested it on SM8250 MTP and Dragon Board DB845c
>
> Applied, thanks
>

Hi, this patch series broke audio on SDM845 running AOSP. I can
reproduce it on both DB845c and PocoF1
https://www.toptal.com/developers/hastebin/raw/rodazupayu. It is not
100% reproducible but can be triggered on every alternate reboot or
so.

Regards,
Amit Pundir

> --
> ~Vinod

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 1/3] soundwire: qcom: add runtime pm support
  2022-02-28 17:25   ` Srinivas Kandagatla
                     ` (2 preceding siblings ...)
  (?)
@ 2022-04-20 17:39   ` Pierre-Louis Bossart
  2022-04-20 17:46     ` Srinivas Kandagatla
  -1 siblings, 1 reply; 24+ messages in thread
From: Pierre-Louis Bossart @ 2022-04-20 17:39 UTC (permalink / raw)
  To: Srinivas Kandagatla, robh+dt, vkoul, yung-chuan.liao
  Cc: devicetree, alsa-devel, linux-kernel, quic_srivasam



> @@ -1017,6 +1032,15 @@ static int qcom_swrm_startup(struct snd_pcm_substream *substream,
>  	struct snd_soc_dai *codec_dai;
>  	int ret, i;
>  
> +	ret = pm_runtime_get_sync(ctrl->dev);
> +	if (ret < 0 && ret != -EACCES) {
> +		dev_err_ratelimited(ctrl->dev,
> +				    "pm_runtime_get_sync failed in %s, ret %d\n",
> +				    __func__, ret);
> +		pm_runtime_put_noidle(ctrl->dev);
> +		return ret;

here there's an error handling, but ...

> +	}
> +
>  	sruntime = sdw_alloc_stream(dai->name);
>  	if (!sruntime)
>  		return -ENOMEM;
> @@ -1044,6 +1068,9 @@ static void qcom_swrm_shutdown(struct snd_pcm_substream *substream,
>  
>  	sdw_release_stream(ctrl->sruntime[dai->id]);
>  	ctrl->sruntime[dai->id] = NULL;
> +	pm_runtime_mark_last_busy(ctrl->dev);
> +	pm_runtime_put_autosuspend(ctrl->dev);
> +
>  }
>  
>  static const struct snd_soc_dai_ops qcom_swrm_pdm_dai_ops = {
> @@ -1197,12 +1224,23 @@ static int qcom_swrm_get_port_config(struct qcom_swrm_ctrl *ctrl)
>  static int swrm_reg_show(struct seq_file *s_file, void *data)
>  {
>  	struct qcom_swrm_ctrl *swrm = s_file->private;
> -	int reg, reg_val;
> +	int reg, reg_val, ret;
> +
> +	ret = pm_runtime_get_sync(swrm->dev);
> +	if (ret < 0 && ret != -EACCES) {
> +		dev_err_ratelimited(swrm->dev,
> +				    "pm_runtime_get_sync failed in %s, ret %d\n",
> +				    __func__, ret);
> +		pm_runtime_put_noidle(swrm->dev);

... here it's missing?

I have a fix ready but thought I would check first if this was intentional

https://github.com/thesofproject/linux/pull/3602/commits/6353eec8dc971c5f0fda0166ae1777f71784ea32

> +	}
>  
>  	for (reg = 0; reg <= SWR_MSTR_MAX_REG_ADDR; reg += 4) {
>  		swrm->reg_read(swrm, reg, &reg_val);
>  		seq_printf(s_file, "0x%.3x: 0x%.2x\n", reg, reg_val);
>  	}
> +	pm_runtime_mark_last_busy(swrm->dev);
> +	pm_runtime_put_autosuspend(swrm->dev);
> +
>  
>  	return 0;

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 3/3] soundwire: qcom: add in-band wake up interrupt support
  2022-03-01 13:52   ` Pierre-Louis Bossart
@ 2022-04-20 17:40     ` Pierre-Louis Bossart
  0 siblings, 0 replies; 24+ messages in thread
From: Pierre-Louis Bossart @ 2022-04-20 17:40 UTC (permalink / raw)
  To: Srinivas Kandagatla, robh+dt, vkoul, yung-chuan.liao
  Cc: devicetree, alsa-devel, linux-kernel, quic_srivasam


>> +static irqreturn_t qcom_swrm_wake_irq_handler(int irq, void *dev_id)
>> +{
>> +	struct qcom_swrm_ctrl *swrm = dev_id;
>> +	int ret;
>> +
>> +	ret = pm_runtime_get_sync(swrm->dev);
>> +	if (ret < 0 && ret != -EACCES) {
>> +		dev_err_ratelimited(swrm->dev,
>> +				    "pm_runtime_get_sync failed in %s, ret %d\n",
>> +				    __func__, ret);
>> +		pm_runtime_put_noidle(swrm->dev);

missing 'return ret' here as well, is this intentional?

Fix at https://github.com/thesofproject/linux/pull/3602/commits/6353eec8dc971c5f0fda0166ae1777f71784ea32 ready to go, but not sure what the intent was.

>> +	}
>> +
>> +	if (swrm->wake_irq > 0) {
>> +		if (!irqd_irq_disabled(irq_get_irq_data(swrm->wake_irq)))
>> +			disable_irq_nosync(swrm->wake_irq);
>> +	}
>> +
>> +	pm_runtime_mark_last_busy(swrm->dev);
>> +	pm_runtime_put_autosuspend(swrm->dev);
>> +
>> +	return IRQ_HANDLED;
>> +}

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 1/3] soundwire: qcom: add runtime pm support
  2022-04-20 17:39   ` Pierre-Louis Bossart
@ 2022-04-20 17:46     ` Srinivas Kandagatla
  2022-04-20 17:53       ` Pierre-Louis Bossart
  0 siblings, 1 reply; 24+ messages in thread
From: Srinivas Kandagatla @ 2022-04-20 17:46 UTC (permalink / raw)
  To: Pierre-Louis Bossart, robh+dt, vkoul, yung-chuan.liao
  Cc: devicetree, alsa-devel, linux-kernel, quic_srivasam



On 20/04/2022 18:39, Pierre-Louis Bossart wrote:
>> @@ -1197,12 +1224,23 @@ static int qcom_swrm_get_port_config(struct qcom_swrm_ctrl *ctrl)
>>   static int swrm_reg_show(struct seq_file *s_file, void *data)
>>   {
>>   	struct qcom_swrm_ctrl *swrm = s_file->private;
>> -	int reg, reg_val;
>> +	int reg, reg_val, ret;
>> +
>> +	ret = pm_runtime_get_sync(swrm->dev);
>> +	if (ret < 0 && ret != -EACCES) {
>> +		dev_err_ratelimited(swrm->dev,
>> +				    "pm_runtime_get_sync failed in %s, ret %d\n",
>> +				    __func__, ret);
>> +		pm_runtime_put_noidle(swrm->dev);
> ... here it's missing?
> 
> I have a fix ready but thought I would check first if this was intentional

Its not intentional.


> 
> https://github.com/thesofproject/linux/pull/3602/commits/6353eec8dc971c5f0fda0166ae1777f71784ea32

Fix looks good.

--srini
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 1/3] soundwire: qcom: add runtime pm support
  2022-04-20 17:46     ` Srinivas Kandagatla
@ 2022-04-20 17:53       ` Pierre-Louis Bossart
  0 siblings, 0 replies; 24+ messages in thread
From: Pierre-Louis Bossart @ 2022-04-20 17:53 UTC (permalink / raw)
  To: Srinivas Kandagatla, robh+dt, vkoul, yung-chuan.liao, Mark Brown
  Cc: devicetree, alsa-devel, linux-kernel, quic_srivasam


>>> @@ -1197,12 +1224,23 @@ static int qcom_swrm_get_port_config(struct qcom_swrm_ctrl *ctrl)
>>>   static int swrm_reg_show(struct seq_file *s_file, void *data)
>>>   {
>>>       struct qcom_swrm_ctrl *swrm = s_file->private;
>>> -    int reg, reg_val;
>>> +    int reg, reg_val, ret;
>>> +
>>> +    ret = pm_runtime_get_sync(swrm->dev);
>>> +    if (ret < 0 && ret != -EACCES) {
>>> +        dev_err_ratelimited(swrm->dev,
>>> +                    "pm_runtime_get_sync failed in %s, ret %d\n",
>>> +                    __func__, ret);
>>> +        pm_runtime_put_noidle(swrm->dev);
>> ... here it's missing?
>>
>> I have a fix ready but thought I would check first if this was intentional
> 
> Its not intentional.
> 
> 
>>
>> https://github.com/thesofproject/linux/pull/3602/commits/6353eec8dc971c5f0fda0166ae1777f71784ea32
> 
> Fix looks good.

Thanks Srini, I'll add Fixes tags and share on alsa-devel.

I guess we need to thank Mark Brown for his guidance to use pm_runtime_resume_and_get(), that's how I saw those two cases :-)

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2022-04-20 18:00 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-28 17:25 [PATCH v3 0/3] soundwire: qcom: add pm runtime support Srinivas Kandagatla
2022-02-28 17:25 ` Srinivas Kandagatla
2022-02-28 17:25 ` [PATCH v3 1/3] soundwire: qcom: add runtime pm support Srinivas Kandagatla
2022-02-28 17:25   ` Srinivas Kandagatla
2022-03-01 13:52   ` Pierre-Louis Bossart
2022-03-01 13:52     ` Pierre-Louis Bossart
2022-03-03 10:52   ` Geert Uytterhoeven
2022-03-03 10:52     ` Geert Uytterhoeven
2022-04-20 17:39   ` Pierre-Louis Bossart
2022-04-20 17:46     ` Srinivas Kandagatla
2022-04-20 17:53       ` Pierre-Louis Bossart
2022-02-28 17:25 ` [PATCH v3 2/3] dt-bindings: soundwire: qcom: document optional wake irq Srinivas Kandagatla
2022-02-28 17:25   ` Srinivas Kandagatla
2022-02-28 17:25 ` [PATCH v3 3/3] soundwire: qcom: add in-band wake up interrupt support Srinivas Kandagatla
2022-02-28 17:25   ` Srinivas Kandagatla
2022-02-28 18:01   ` Pierre-Louis Bossart
2022-03-01 11:13     ` Srinivas Kandagatla
2022-03-01 13:51       ` Pierre-Louis Bossart
2022-03-01 13:52   ` Pierre-Louis Bossart
2022-04-20 17:40     ` Pierre-Louis Bossart
2022-03-02 15:42 ` [PATCH v3 0/3] soundwire: qcom: add pm runtime support Vinod Koul
2022-03-02 15:42   ` Vinod Koul
2022-04-04 21:42   ` Amit Pundir
2022-04-04 21:42     ` Amit Pundir

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.