[PATCH v2 0/2] soundwire: qcom: Add Shikra soundwire support

[PATCH v2 0/2] soundwire: qcom: Add Shikra soundwire support

[Mohammad Rafi Shaik]

This series adds support for the Shikra SoundWire controller.

Unlike existing Qualcomm platforms, Shikra assigns the SoundWire master
controller to EE0 instead of EE1. On SoundWire v2.0 and later hardware,
FIFO, and status registers are banked per Execution Environment (EE),
making the register layout dependent on the EE assignment.

To support this difference, the series introduces a dedicated
qcom,shikra-soundwire compatible and updates the driver to select
the appropriate register layout based on SoC match data. Existing
Soc's continue to use the default EE1 configuration without any
functional changes.

Signed-off-by: Mohammad Rafi Shaik <mohammad.rafi.shaik@oss.qualcomm.com>
---
Changes in v2:
- Drop qcom,swr-master-ee-val DT property; the EE assignment is a
  fixed SoC integration detail, not a board-level property. Encode
  it in qcom_swrm_data match data instead and introduce a dedicated
  qcom,shikra-soundwire compatible to select EE0 at probe time.
- Remove the dt-bindings patch for qcom,swr-master-ee-val (no longer needed).
- Apply EE stride offset to SWRM_V2_0_CLK_CTRL in qcom_swrm_init()
  and swrm_runtime_resume() for correctness on EE0.
- Remove the mmio guard from the INTERRUPT_CPU_EN write; always enable
  CPU IRQs for the selected EE window.
- Link to v1: https://patch.msgid.link/20260608175345.3118060-1-mohammad.rafi.shaik@oss.qualcomm.com

To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Srinivas Kandagatla <srini@kernel.org>
To: Rao Mandadapu <quic_srivasam@quicinc.com>
To: Vinod Koul <vkoul@kernel.org>
To: Bard Liao <yung-chuan.liao@linux.intel.com>
To: Pierre-Louis Bossart <pierre-louis.bossart@linux.dev>
Cc: linux-arm-msm@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-sound@vger.kernel.org

---
Mohammad Rafi Shaik (2):
      dt-bindings: soundwire: qcom: add qcom,shikra-soundwire compatible
      soundwire: qcom: add EE-aware register layout for Shikra

 .../bindings/soundwire/qcom,soundwire.yaml         |  1 +
 drivers/soundwire/qcom.c                           | 85 ++++++++++++++++++----
 2 files changed, 72 insertions(+), 14 deletions(-)
---
base-commit: c68a982815dcce5464e3bf2a31ac94f5146c04ca
change-id: 20260910-shikra_soundwire_support-96dd8130874a

Best regards,
--  
Mohammad Rafi Shaik <mohammad.rafi.shaik@oss.qualcomm.com>

[PATCH v2 1/2] dt-bindings: soundwire: qcom: add qcom,shikra-soundwire compatible

[Mohammad Rafi Shaik]

Add qcom,shikra-soundwire as a standalone compatible for the Shikra SoC
SoundWire controller.

On Shikra SoC, the SoundWire controller is assigned to an Execution
Environment (EE), which determines which banked register window must be
used for interrupt, FIFO, and status register access on SoundWire v2.0
and later hardware.

Add a dedicated qcom,shikra-soundwire compatible so the driver can select
the correct register layout for Shikra at probe time.

Signed-off-by: Mohammad Rafi Shaik <mohammad.rafi.shaik@oss.qualcomm.com>
---
 Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml b/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
index 8e6973fa2..81532cf69 100644
--- a/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
+++ b/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
@@ -17,6 +17,7 @@ properties:
   compatible:
     oneOf:
       - enum:
+          - qcom,shikra-soundwire
           - qcom,soundwire-v1.3.0
           - qcom,soundwire-v1.5.0
           - qcom,soundwire-v1.5.1

-- 
2.34.1

[PATCH v2 2/2] soundwire: qcom: add EE-aware register layout for Shikra

[Mohammad Rafi Shaik]

On SoundWire v2.0 and later hardware, FIFO, and status
registers are banked per Execution Environment (EE).
The existing driver assumes the SoundWire controller is
assigned to EE1, which is true for all currently supported
Qualcomm SoCs.

Shikra is an exception and assigns the controller to EE0.
As a result, the relevant register windows are located at
different offsets and cannot be accessed using the default
EE1 register layout.

Add an ee field to the SoC match data and introduce a
dedicated qcom,shikra-soundwire compatible with ee = 0.
The driver uses this information at probe time to select
the correct register layout, while all existing platforms
continue to use the default EE1 configuration.

Signed-off-by: Mohammad Rafi Shaik <mohammad.rafi.shaik@oss.qualcomm.com>
---
 drivers/soundwire/qcom.c | 85 ++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 71 insertions(+), 14 deletions(-)

diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index 35ffffd54..6e0c5fb10 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -25,7 +25,9 @@
 #define SWRM_COMP_SW_RESET					0x008
 #define SWRM_COMP_STATUS					0x014
 #define SWRM_LINK_MANAGER_EE					0x018
-#define SWRM_EE_CPU						1
+#define SWRM_EE_CPU0						0
+#define SWRM_EE_CPU1						1
+#define SWRM_EE_CPU						SWRM_EE_CPU1
 #define SWRM_FRM_GEN_ENABLED					BIT(0)
 #define SWRM_VERSION_1_3_0					0x01030000
 #define SWRM_VERSION_1_5_1					0x01050001
@@ -118,6 +120,7 @@
 #define SWRM_V2_0_CLK_CTRL					0x5060
 #define SWRM_V2_0_CLK_CTRL_CLK_START				BIT(0)
 #define SWRM_V2_0_LINK_STATUS					0x5064
+#define SWRM_V2_REG_EE_STRIDE					0x1000
 
 #define SWRM_DP_PORT_CTRL_EN_CHAN_SHFT				0x18
 #define SWRM_DP_PORT_CTRL_OFFSET2_SHFT				0x10
@@ -201,6 +204,7 @@ struct qcom_swrm_ctrl {
 	struct mutex port_lock;
 	struct clk *hclk;
 	int irq;
+	u32 ee;
 	unsigned int version;
 	int wake_irq;
 	int num_din_ports;
@@ -224,6 +228,7 @@ struct qcom_swrm_ctrl {
 	/* Per-Slave SCP_ADDRPAGE1/2 shadow; -1 = unknown. */
 	s16 page1_cache[SDW_MAX_DEVICES + 1];
 	s16 page2_cache[SDW_MAX_DEVICES + 1];
+	unsigned int reg_layout_local[SWRM_OFFSET_DP_SAMPLECTRL2_BANK + 1];
 };
 
 struct qcom_swrm_data {
@@ -232,6 +237,7 @@ struct qcom_swrm_data {
 	bool sw_clk_gate_required;
 	u32 max_reg;
 	const unsigned int *reg_layout;
+	u32 ee;
 };
 
 static const unsigned int swrm_v1_3_reg_layout[] = {
@@ -258,6 +264,7 @@ static const struct qcom_swrm_data swrm_v1_3_data = {
 	.default_cols = 16,
 	.max_reg = SWR_V1_3_MSTR_MAX_REG_ADDR,
 	.reg_layout = swrm_v1_3_reg_layout,
+	.ee = SWRM_EE_CPU,
 };
 
 static const struct qcom_swrm_data swrm_v1_5_data = {
@@ -265,6 +272,7 @@ static const struct qcom_swrm_data swrm_v1_5_data = {
 	.default_cols = 16,
 	.max_reg = SWR_V1_3_MSTR_MAX_REG_ADDR,
 	.reg_layout = swrm_v1_3_reg_layout,
+	.ee = SWRM_EE_CPU,
 };
 
 static const struct qcom_swrm_data swrm_v1_6_data = {
@@ -273,6 +281,7 @@ static const struct qcom_swrm_data swrm_v1_6_data = {
 	.sw_clk_gate_required = true,
 	.max_reg = SWR_V1_3_MSTR_MAX_REG_ADDR,
 	.reg_layout = swrm_v1_3_reg_layout,
+	.ee = SWRM_EE_CPU,
 };
 
 static const unsigned int swrm_v2_0_reg_layout[] = {
@@ -300,6 +309,7 @@ static const struct qcom_swrm_data swrm_v2_0_data = {
 	.sw_clk_gate_required = true,
 	.max_reg = SWR_V2_0_MSTR_MAX_REG_ADDR,
 	.reg_layout = swrm_v2_0_reg_layout,
+	.ee = SWRM_EE_CPU,
 };
 
 static const unsigned int swrm_v3_0_reg_layout[] = {
@@ -327,9 +337,51 @@ static const struct qcom_swrm_data swrm_v3_0_data = {
 	.sw_clk_gate_required = true,
 	.max_reg = SWR_V2_0_MSTR_MAX_REG_ADDR,
 	.reg_layout = swrm_v3_0_reg_layout,
+	.ee = SWRM_EE_CPU,
+};
+
+static const struct qcom_swrm_data swrm_shikra_data = {
+	.default_rows = 50,
+	.default_cols = 16,
+	.sw_clk_gate_required = true,
+	.max_reg = SWR_V2_0_MSTR_MAX_REG_ADDR,
+	.reg_layout = swrm_v3_0_reg_layout,
+	.ee = SWRM_EE_CPU0,
 };
 #define to_qcom_sdw(b)	container_of(b, struct qcom_swrm_ctrl, bus)
 
+static void qcom_swrm_set_ee_register_layout(struct qcom_swrm_ctrl *ctrl,
+					     const struct qcom_swrm_data *data)
+{
+	int ee_offset;
+
+	memcpy(ctrl->reg_layout_local, data->reg_layout,
+	       sizeof(ctrl->reg_layout_local));
+	ctrl->reg_layout = ctrl->reg_layout_local;
+
+	if (ctrl->version < SWRM_VERSION_2_0_0)
+		return;
+
+	/*
+	 * The register layout constants are defined for EE1 (the default for
+	 * most Qualcomm SoCs). For SoCs where the SoundWire master is assigned
+	 * to EE0, the interrupt, FIFO and status register windows are shifted
+	 * by one EE stride (0x1000) relative to the EE1 base addresses.
+	 */
+	ee_offset = ((int)ctrl->ee - SWRM_EE_CPU) * SWRM_V2_REG_EE_STRIDE;
+	if (!ee_offset)
+		return;
+
+	ctrl->reg_layout_local[SWRM_REG_FRAME_GEN_ENABLED] += ee_offset;
+	ctrl->reg_layout_local[SWRM_REG_INTERRUPT_STATUS] += ee_offset;
+	ctrl->reg_layout_local[SWRM_REG_INTERRUPT_CLEAR] += ee_offset;
+	ctrl->reg_layout_local[SWRM_REG_INTERRUPT_CPU_EN] += ee_offset;
+	ctrl->reg_layout_local[SWRM_REG_CMD_FIFO_WR_CMD] += ee_offset;
+	ctrl->reg_layout_local[SWRM_REG_CMD_FIFO_RD_CMD] += ee_offset;
+	ctrl->reg_layout_local[SWRM_REG_CMD_FIFO_STATUS] += ee_offset;
+	ctrl->reg_layout_local[SWRM_REG_CMD_FIFO_RD_FIFO_ADDR] += ee_offset;
+}
+
 static int qcom_swrm_ahb_reg_read(struct qcom_swrm_ctrl *ctrl, int reg,
 				  u32 *val)
 {
@@ -910,12 +962,13 @@ static int qcom_swrm_init(struct qcom_swrm_ctrl *ctrl)
 	ctrl->reg_write(ctrl, SWRM_MCP_CFG_ADDR, val);
 
 	if (ctrl->version == SWRM_VERSION_1_7_0) {
-		ctrl->reg_write(ctrl, SWRM_LINK_MANAGER_EE, SWRM_EE_CPU);
+		ctrl->reg_write(ctrl, SWRM_LINK_MANAGER_EE, ctrl->ee);
 		ctrl->reg_write(ctrl, SWRM_MCP_BUS_CTRL,
-				SWRM_MCP_BUS_CLK_START << SWRM_EE_CPU);
+				SWRM_MCP_BUS_CLK_START << ctrl->ee);
 	} else if (ctrl->version >= SWRM_VERSION_2_0_0) {
-		ctrl->reg_write(ctrl, SWRM_LINK_MANAGER_EE, SWRM_EE_CPU);
-		ctrl->reg_write(ctrl, SWRM_V2_0_CLK_CTRL,
+		ctrl->reg_write(ctrl, SWRM_LINK_MANAGER_EE, ctrl->ee);
+		ctrl->reg_write(ctrl, SWRM_V2_0_CLK_CTRL +
+				((int)ctrl->ee - SWRM_EE_CPU) * SWRM_V2_REG_EE_STRIDE,
 				SWRM_V2_0_CLK_CTRL_CLK_START);
 	} else {
 		ctrl->reg_write(ctrl, SWRM_MCP_BUS_CTRL, SWRM_MCP_BUS_CLK_START);
@@ -941,11 +994,9 @@ static int qcom_swrm_init(struct qcom_swrm_ctrl *ctrl)
 	ctrl->reg_write(ctrl, ctrl->reg_layout[SWRM_REG_INTERRUPT_CLEAR],
 			0xFFFFFFFF);
 
-	/* enable CPU IRQs */
-	if (ctrl->mmio) {
-		ctrl->reg_write(ctrl, ctrl->reg_layout[SWRM_REG_INTERRUPT_CPU_EN],
-				SWRM_INTERRUPT_STATUS_RMSK);
-	}
+	/* enable CPU IRQs for the selected EE window */
+	ctrl->reg_write(ctrl, ctrl->reg_layout[SWRM_REG_INTERRUPT_CPU_EN],
+			SWRM_INTERRUPT_STATUS_RMSK);
 
 	/* Set IRQ to PULSE */
 	ctrl->reg_write(ctrl, SWRM_COMP_CFG_ADDR,
@@ -1580,6 +1631,7 @@ static int qcom_swrm_probe(struct platform_device *pdev)
 	memset(ctrl->page2_cache, 0xff, sizeof(ctrl->page2_cache));
 
 	data = of_device_get_match_data(dev);
+	ctrl->ee = data->ee;
 	ctrl->max_reg = data->max_reg;
 	ctrl->reg_layout = data->reg_layout;
 	ctrl->rows_index = sdw_find_row_index(data->default_rows);
@@ -1659,6 +1711,7 @@ static int qcom_swrm_probe(struct platform_device *pdev)
 	prop->default_row = data->default_rows;
 
 	ctrl->reg_read(ctrl, SWRM_COMP_HW_VERSION, &ctrl->version);
+	qcom_swrm_set_ee_register_layout(ctrl, data);
 
 	ret = devm_request_threaded_irq(dev, ctrl->irq, NULL,
 					qcom_swrm_irq_handler,
@@ -1769,16 +1822,19 @@ static int __maybe_unused swrm_runtime_resume(struct device *dev)
 		reset_control_reset(ctrl->audio_cgcr);
 
 		if (ctrl->version == SWRM_VERSION_1_7_0) {
-			ctrl->reg_write(ctrl, SWRM_LINK_MANAGER_EE, SWRM_EE_CPU);
+			ctrl->reg_write(ctrl, SWRM_LINK_MANAGER_EE, ctrl->ee);
 			ctrl->reg_write(ctrl, SWRM_MCP_BUS_CTRL,
-					SWRM_MCP_BUS_CLK_START << SWRM_EE_CPU);
+					SWRM_MCP_BUS_CLK_START << ctrl->ee);
 		} else if (ctrl->version >= SWRM_VERSION_2_0_0) {
-			ctrl->reg_write(ctrl, SWRM_LINK_MANAGER_EE, SWRM_EE_CPU);
-			ctrl->reg_write(ctrl, SWRM_V2_0_CLK_CTRL,
+			ctrl->reg_write(ctrl, SWRM_LINK_MANAGER_EE, ctrl->ee);
+			ctrl->reg_write(ctrl, SWRM_V2_0_CLK_CTRL +
+					((int)ctrl->ee - SWRM_EE_CPU) *
+					SWRM_V2_REG_EE_STRIDE,
 					SWRM_V2_0_CLK_CTRL_CLK_START);
 		} else {
 			ctrl->reg_write(ctrl, SWRM_MCP_BUS_CTRL, SWRM_MCP_BUS_CLK_START);
 		}
+
 		ctrl->reg_write(ctrl, ctrl->reg_layout[SWRM_REG_INTERRUPT_CLEAR],
 			SWRM_INTERRUPT_STATUS_MASTER_CLASH_DET);
 
@@ -1854,6 +1910,7 @@ static const struct of_device_id qcom_swrm_of_match[] = {
 	{ .compatible = "qcom,soundwire-v1.7.0", .data = &swrm_v1_5_data },
 	{ .compatible = "qcom,soundwire-v2.0.0", .data = &swrm_v2_0_data },
 	{ .compatible = "qcom,soundwire-v3.1.0", .data = &swrm_v3_0_data },
+	{ .compatible = "qcom,shikra-soundwire", .data = &swrm_shikra_data },
 	{/* sentinel */},
 };
 

-- 
2.34.1

Re: [PATCH v2 2/2] soundwire: qcom: add EE-aware register layout for Shikra

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] IRQ handler remains active after subsystem unregistration (Use-After-Free risk).
- [Medium] Missing clock cleanup on error path in probe.
- [Medium] Missing pm_runtime_disable() in remove.
--

commit ea7f212648855ca1f1b27ca9b52687d659dc81f7
Author: Mohammad Rafi Shaik <mohammad.rafi.shaik@oss.qualcomm.com>

soundwire: qcom: add EE-aware register layout for Shikra

On SoundWire v2.0 and later hardware, FIFO, and status
registers are banked per Execution Environment (EE).
The existing driver assumes the SoundWire controller is
assigned to EE1, which is true for all currently supported
Qualcomm SoCs.

Shikra is an exception and assigns the controller to EE0.
As a result, the relevant register windows are located at
different offsets and cannot be accessed using the default
EE1 register layout.

Add an ee field to the SoC match data and introduce a
dedicated qcom,shikra-soundwire compatible with ee = 0.
The driver uses this information at probe time to select
the correct register layout, while all existing platforms
continue to use the default EE1 configuration.

> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
> index 35ffffd541bd1..6e0c5fb10efad 100644
> --- a/drivers/soundwire/qcom.c
> +++ b/drivers/soundwire/qcom.c

[ ... ]

> @@ -1659,6 +1711,7 @@ static int qcom_swrm_probe(struct platform_device *pdev)
>  	prop->default_row = data->default_rows;
>  
>  	ctrl->reg_read(ctrl, SWRM_COMP_HW_VERSION, &ctrl->version);
> +	qcom_swrm_set_ee_register_layout(ctrl, data);
>  
>  	ret = devm_request_threaded_irq(dev, ctrl->irq, NULL,
>  					qcom_swrm_irq_handler,

[Severity: Medium]
This isn't a bug introduced by this patch, but looking at the error handling
further down in qcom_swrm_probe():

	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;
		}
	}

If the devm_request_threaded_irq() for wake_irq fails, the code jumps directly
to the err_init label. Does this bypass the err_clk label, leaking the clock
reference and keeping it prepared and enabled unexpectedly?

[ ... ]

> @@ -1769,16 +1822,19 @@ static int __maybe_unused swrm_runtime_resume(struct device *dev)
>  		reset_control_reset(ctrl->audio_cgcr);

[Severity: High]
This is also a pre-existing issue, but I have a question about the teardown
sequence in qcom_swrm_remove() which sits just above this function:

static void qcom_swrm_remove(struct platform_device *pdev)
{
	struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(&pdev->dev);

	sdw_bus_master_delete(&ctrl->bus);
	clk_disable_unprepare(ctrl->hclk);
}

During driver unbind or module unload, this function tears down the bus and
slave devices. However, the driver uses a devm-managed IRQ handler which
remains active until devres release (after remove returns).

If a hardware interrupt fires during this window, could qcom_swrm_irq_handler()
execute and operate on the already destroyed bus and slave structures, leading
to a use-after-free or null pointer dereference?

[Severity: Medium]
This is another pre-existing issue in qcom_swrm_remove():

The qcom_swrm_probe() function sets up PM runtime:

	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);

Should qcom_swrm_remove() call pm_runtime_disable() and related cleanup
functions like pm_runtime_dont_use_autosuspend() to balance this PM setup and
avoid leaving the device in an unbalanced state upon driver reload?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910-shikra_soundwire_support-v2-0-f98c9a6ea1e1@oss.qualcomm.com?part=2

Re: [PATCH v2 1/2] dt-bindings: soundwire: qcom: add qcom,shikra-soundwire compatible

[Krzysztof Kozlowski]

On Thu, Sep 10, 2026 at 11:50:14AM +0530, Mohammad Rafi Shaik wrote:
> Add qcom,shikra-soundwire as a standalone compatible for the Shikra SoC
> SoundWire controller.
> 
> On Shikra SoC, the SoundWire controller is assigned to an Execution
> Environment (EE), which determines which banked register window must be
> used for interrupt, FIFO, and status register access on SoundWire v2.0
> and later hardware.
> 
> Add a dedicated qcom,shikra-soundwire compatible so the driver can select
> the correct register layout for Shikra at probe time.
> 
> Signed-off-by: Mohammad Rafi Shaik <mohammad.rafi.shaik@oss.qualcomm.com>
> ---
>  Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml | 1 +
>  1 file changed, 1 insertion(+)

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof