[PATCH v1 0/2] soundwire: qcom: add support for EE-aware register layout

[PATCH v1 0/2] soundwire: qcom: add support for EE-aware register layout

[Mohammad Rafi Shaik]

This series adds support for execution environment (EE) aware register
layouts in the Qualcomm SoundWire master controller driver.

On SoundWire v2.0+ hardware, some controllers bank the interrupt, FIFO, and
status registers in EE-specific windows separated by a stride of 0x1000.
Typically, the driver assumes EE1, but platforms like Shikra have the
controller routed on EE0.

To support platforms with varying EEs, we:
1. Introduce the `qcom,swr-master-ee-val` DT property to configure the
   target execution environment.
2. Add remapping logic to adjust the register layout offsets by the
   EE window stride on v2.0+ hardware.
   
The first patch adds DT binding documentation for the new property, and
the second implements the EE-aware layout selection in the soundwire
driver.

Mohammad Rafi Shaik (2):
  dt-bindings: soundwire: qcom: add qcom,swr-master-ee-val property
  soundwire: qcom: add EE-aware register layout and cpu selection

 .../bindings/soundwire/qcom,soundwire.yaml    |  6 ++
 drivers/soundwire/qcom.c                      | 78 +++++++++++++++----
 2 files changed, 71 insertions(+), 13 deletions(-)

-- 
2.34.1

[PATCH v1 1/2] dt-bindings: soundwire: qcom: add qcom,swr-master-ee-val property

[Mohammad Rafi Shaik]

Add documentation for the qcom,swr-master-ee-val Device Tree property
used by Qualcomm SoundWire masters to describe the execution-environment
value for interrupt routing.

This property allows platform DTs to specify the EE value used to direct
SoundWire master interrupts to the appropriate CPU target.

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

diff --git a/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml b/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
index 9447a2f37..5b06cc1a5 100644
--- a/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
+++ b/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
@@ -215,6 +215,12 @@ properties:
           maximum: 4
         - const: 0xff
 
+  qcom,swr-master-ee-val:
+    $ref: /schemas/types.yaml#/definitions/uint8-array
+    description:
+      Execution-environment value used to route SoundWire master
+      interrupts to CPU0 or CPU1.
+
   label:
     maxItems: 1
 
-- 
2.34.1

Re: [PATCH v1 1/2] dt-bindings: soundwire: qcom: add qcom,swr-master-ee-val property

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Type mismatch for `qcom,swr-master-ee-val` between DT schema and driver implementation.
--

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

dt-bindings: soundwire: qcom: add qcom,swr-master-ee-val property

This commit adds documentation for the qcom,swr-master-ee-val Device Tree
property used by Qualcomm SoundWire masters to describe the execution
environment value for interrupt routing.

> diff --git a/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml b/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
> index 9447a2f371b56..5b06cc1a5f781 100644
> --- a/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
> +++ b/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
> @@ -215,6 +215,12 @@ properties:
>            maximum: 4
>          - const: 0xff
>  
> +  qcom,swr-master-ee-val:
> +    $ref: /schemas/types.yaml#/definitions/uint8-array

[Severity: Medium]
Should this property be defined as a uint32 type rather than a uint8-array?

In the qcom SoundWire driver, this property is read using
of_property_read_u32():

drivers/soundwire/qcom.c:qcom_swrm_probe() {
    ...
    ret = of_property_read_u32(dev->of_node, "qcom,swr-master-ee-val", &ctrl->ee);
    ...
}

If a device tree strictly follows the schema and specifies a byte array
such as /bits/ 8 <1>;, will the driver fail to parse it with
of_property_read_u32() due to the length being less than 4 bytes?

This might cause the driver to silently ignore the property and fall back to
incorrect execution environment defaults, potentially breaking SoundWire
functionality. 

Conversely, if a device tree specifies a 32-bit integer like <1> to work at
runtime, won't it fail dtbs_check validation?

> +    description:
> +      Execution-environment value used to route SoundWire master
> +      interrupts to CPU0 or CPU1.
> +
>    label:
>      maxItems: 1

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608175345.3118060-1-mohammad.rafi.shaik@oss.qualcomm.com?part=1

Re: [PATCH v1 1/2] dt-bindings: soundwire: qcom: add qcom,swr-master-ee-val property

[Krzysztof Kozlowski]

On 08/06/2026 19:53, Mohammad Rafi Shaik wrote:
> Add documentation for the qcom,swr-master-ee-val Device Tree property
> used by Qualcomm SoundWire masters to describe the execution-environment
> value for interrupt routing.
> 
> This property allows platform DTs to specify the EE value used to direct

Please describe more what this EE value is for.

> SoundWire master interrupts to the appropriate CPU target.

Interrupt affinity defines where the interrupts are routed. Not custom
properties.

> 
> Signed-off-by: Mohammad Rafi Shaik <mohammad.rafi.shaik@oss.qualcomm.com>
> ---
>  .../devicetree/bindings/soundwire/qcom,soundwire.yaml       | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml b/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
> index 9447a2f37..5b06cc1a5 100644
> --- a/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
> +++ b/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
> @@ -215,6 +215,12 @@ properties:
>            maximum: 4
>          - const: 0xff
>  
> +  qcom,swr-master-ee-val:

Aren't there existing properties for execution context? I think Qualcomm
has something, so you should not come with a new property. Assuming this
stays in the first place...


> +    $ref: /schemas/types.yaml#/definitions/uint8-array

Missing constraints.

> +    description:
> +      Execution-environment value used to route SoundWire master
> +      interrupts to CPU0 or CPU1.
> +
>    label:
>      maxItems: 1
>  


Best regards,
Krzysztof

[PATCH v1 2/2] soundwire: qcom: add EE-aware register layout and cpu selection

[Mohammad Rafi Shaik]

Some Qualcomm SoundWire masters expose interrupt, FIFO and status
registers in EE-specific register windows on v2.0 and newer hardware.

Add support for selecting the SoundWire execution environment from DT
and use it to program the correct register window for the active EE.
The driver now reads the EE value from the new
qcom,swr-master-ee-val property, with qcom,ee as a fallback for
backward compatibility.

For v2.0+ hardware, the IRQ/FIFO/status register layout is adjusted by
the EE window stride so the driver programs the correct bank for the
selected EE. The interrupt enable path is also updated to always use
the selected EE window.

This change allows SoundWire interrupt routing and register accesses to
work correctly on platforms where the master is not mapped to the
default EE1 window.

In Shikra, the soundwire execution environment is set to 0 unlike other
Qualcomm boards.

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

diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index 3d8f5a81e..f4b8ff77b 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -26,6 +26,7 @@
 #define SWRM_COMP_STATUS					0x014
 #define SWRM_LINK_MANAGER_EE					0x018
 #define SWRM_EE_CPU						1
+#define SWRM_MAX_EE						1
 #define SWRM_FRM_GEN_ENABLED					BIT(0)
 #define SWRM_VERSION_1_3_0					0x01030000
 #define SWRM_VERSION_1_5_1					0x01050001
@@ -118,6 +119,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
@@ -202,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;
@@ -222,6 +225,7 @@ struct qcom_swrm_ctrl {
 	u32 slave_status;
 	u32 wr_fifo_depth;
 	bool clock_stop_not_supported;
+	unsigned int reg_layout_local[SWRM_OFFSET_DP_SAMPLECTRL2_BANK + 1];
 };
 
 struct qcom_swrm_data {
@@ -328,6 +332,36 @@ static const struct qcom_swrm_data swrm_v3_0_data = {
 };
 #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;
+
+	/*
+	 * Current register constants map EE1. For EE0, use the EE register
+	 * window stride to access status/IRQ/FIFO registers.
+	 */
+	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)
 {
@@ -904,12 +938,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);
@@ -935,11 +970,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,
@@ -1545,7 +1578,22 @@ static int qcom_swrm_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	data = of_device_get_match_data(dev);
+	ctrl->ee = SWRM_EE_CPU;
+	ret = of_property_read_u32(dev->of_node, "qcom,swr-master-ee-val", &ctrl->ee);
+	if (ret)
+		ret = of_property_read_u32(dev->of_node, "qcom,ee", &ctrl->ee);
+	if (ret)
+		ctrl->ee = SWRM_EE_CPU;
+	if (ctrl->ee > SWRM_MAX_EE) {
+		dev_warn(dev, "invalid SoundWire EE %u, using EE%u\n",
+			 ctrl->ee, SWRM_EE_CPU);
+		ctrl->ee = SWRM_EE_CPU;
+	}
 	ctrl->max_reg = data->max_reg;
+	/*
+	 * Defer EE register window selection until HW version is known.
+	 * For v2.0+ the IRQ/FIFO window is EE-banked.
+	 */
 	ctrl->reg_layout = data->reg_layout;
 	ctrl->rows_index = sdw_find_row_index(data->default_rows);
 	ctrl->cols_index = sdw_find_col_index(data->default_cols);
@@ -1623,6 +1671,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,
@@ -1733,16 +1782,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);
 
-- 
2.34.1

Re: [PATCH v1 2/2] soundwire: qcom: add EE-aware register layout and cpu selection

[sashiko-bot]

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

Pre-existing issues:
- [High] Data race on `ctrl->intr_mask` between threaded interrupt handler and PM runtime callbacks.
- [High] Missing NULL pointer validation for `data` from `of_device_get_match_data()`, leading to a NULL pointer dereference crash.
--

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

soundwire: qcom: add EE-aware register layout and cpu selection

This commit adds support for selecting the SoundWire execution environment
from the device tree and adjusts register windows based on the EE value.
For v2.0+ hardware, the IRQ, FIFO, and status register layout is offset by
the EE window stride.

> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
> index 3d8f5a81eff19..f4b8ff77bc2f8 100644
> --- a/drivers/soundwire/qcom.c
> +++ b/drivers/soundwire/qcom.c

[ ... ]

> @@ -1545,7 +1578,22 @@ static int qcom_swrm_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  
>  	data = of_device_get_match_data(dev);
> +	ctrl->ee = SWRM_EE_CPU;
> +	ret = of_property_read_u32(dev->of_node, "qcom,swr-master-ee-val", &ctrl->ee);
> +	if (ret)
> +		ret = of_property_read_u32(dev->of_node, "qcom,ee", &ctrl->ee);
> +	if (ret)
> +		ctrl->ee = SWRM_EE_CPU;
> +	if (ctrl->ee > SWRM_MAX_EE) {
> +		dev_warn(dev, "invalid SoundWire EE %u, using EE%u\n",
> +			 ctrl->ee, SWRM_EE_CPU);
> +		ctrl->ee = SWRM_EE_CPU;
> +	}
>  	ctrl->max_reg = data->max_reg;

[Severity: High]
This isn't a bug introduced by this patch, but could this dereference of
data->max_reg cause a NULL pointer crash?

If the device is bound to the driver manually via sysfs (e.g., by writing to
driver_override), the platform bus falls back to matching by name rather than
the device tree.

In this case, dev->of_node is missing, and of_device_get_match_data() returns
NULL.

Does this data pointer need to be validated before being dereferenced?

> +	/*
> +	 * Defer EE register window selection until HW version is known.
> +	 * For v2.0+ the IRQ/FIFO window is EE-banked.
> +	 */
>  	ctrl->reg_layout = data->reg_layout;

[ ... ]

> @@ -1733,16 +1782,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);
>  

[Severity: High]
This is a pre-existing issue, but could there be a data race on ctrl->intr_mask
between the threaded interrupt handler and PM runtime callbacks?

Just below this code in swrm_runtime_resume(), ctrl->intr_mask is modified
locklessly:

    ctrl->intr_mask |= SWRM_INTERRUPT_STATUS_MASTER_CLASH_DET;

At the same time, the threaded IRQ handler performs its own lockless
read-modify-write on ctrl->intr_mask:

drivers/soundwire/qcom.c:qcom_swrm_irq_handler() {
    ...
    ctrl->intr_mask &= ~SWRM_INTERRUPT_STATUS_MASTER_CLASH_DET;
    ...
}

Both execute in process context without synchronization, and the PM runtime
worker can be scheduled via wake_irq independently.

If these run concurrently on different CPUs, could this result in a
read-modify-write race where an update is lost?

If an update is lost, could an interrupt be unintentionally left unmasked,
potentially causing a persistent IRQ storm?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608175345.3118060-1-mohammad.rafi.shaik@oss.qualcomm.com?part=2

Re: [PATCH v1 2/2] soundwire: qcom: add EE-aware register layout and cpu selection

[Krzysztof Kozlowski]

On 08/06/2026 19:53, Mohammad Rafi Shaik wrote:
> Some Qualcomm SoundWire masters expose interrupt, FIFO and status
> registers in EE-specific register windows on v2.0 and newer hardware.
> 
> Add support for selecting the SoundWire execution environment from DT
> and use it to program the correct register window for the active EE.
> The driver now reads the EE value from the new

No, it does not. "now" means before the commit, you are making the
changes - see submitting patches.

> qcom,swr-master-ee-val property, with qcom,ee as a fallback for
> backward compatibility.

And where did you document qcom,ee and why do you need fallback in the
first place? For downstream DTS? We do not support anything from
downstream here.

Best regards,
Krzysztof