* [PATCH 0/3] soundwire: qcom: add multi-master support
@ 2026-09-11 8:21 Srinivas Kandagatla
2026-09-11 8:21 ` [PATCH 1/3] dt-bindings: soundwire: qcom: describe multi-master Srinivas Kandagatla
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Srinivas Kandagatla @ 2026-09-11 8:21 UTC (permalink / raw)
To: robh, vkoul
Cc: krzk+dt, conor+dt, srini, yung-chuan.liao, pierre-louis.bossart,
prasad.kumpatla, jingyi.wang, sibi.sankar, quic_srivasam,
linux-arm-msm, devicetree, linux-kernel, linux-sound,
Srinivas Kandagatla
This patch series adds support to the multi-master in existing soundwire
controller driver.
On Qualcomm Glymur SoC, both TX and RX soundwire controllers are wired
up in multi master mode connecting to an SDCA compliant Tambora codec.
One of the two controllers (tx,rx) act as primary controller in this
instance its tx which provides clock and control and some data lines and
other controller instance in this case rx act as dependent controller
providing extra data lanes to primary controller. This is called multi
master mode in Qualcomm Soundwire controller.
Multi Master mode allows synchronous transport across two independent
soundwire masters. This includes synchronized clock-stop, clock-resume,
clock scaling and bank switch between both masters, the Primary master
will ensure that.
From the slave's point of view there is still one SoundWire bus with
one clock and one control lane; the extra DATA lanes just come from
a different pad on the SoC.
Currently the bindings are added in a way that its possible to add more
than one dependent, however to keep things simple the driver only
supports single dependent for now.
Topology (2+2 lane example):
Primary IP Dependent IP
+-----------------+ +-----------------+
| MM_SYNC <=====+=================+=====> MM_SYNC |
| | | |
| SWR_CLK ---+ | | |
| CTRL ---+ | | |
| DATA0 ---+ | | DATA0 ---+ |
| DATA1 ---+ | | DATA1 ---+ |
+--------------|--+ +------------|----+
| |
v v
+-----------+-----------+ +---------+---------+
| SWR_CLK, CTRL, | | bus DATA2, |
| bus DATA0, DATA1 | | DATA3 |
+-----------+-----------+ +---------+---------+
| |
+----------------+----------------+
|
v
+--------------+
| Slave |
+--------------+
This series adds multi-master support.
Patch 1 adds two new DT properties qcom,multi-master-mode and
qcom,multi-master-peer for describing the pair.
Patch 2 wires up the primary and dependent roles in the driver.
Patch 3 programs SWRM_V3_MM_SYNC_CONFIG on both sides so their frame
generators lock to a single SWR_CLK.
Tested on Glymur (SC8480XP) CRD with WCD9378 in compute mode. HPH
playback and mic capture both work.
Srinivas Kandagatla (3):
dt-bindings: soundwire: qcom: describe multi-master
soundwire: qcom: add multi-master support
soundwire: qcom: program MM_SYNC for multi-master
.../bindings/soundwire/qcom,soundwire.yaml | 109 ++++++++-
drivers/soundwire/qcom.c | 218 ++++++++++++++++--
2 files changed, 303 insertions(+), 24 deletions(-)
--
2.53.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/3] dt-bindings: soundwire: qcom: describe multi-master
2026-09-11 8:21 [PATCH 0/3] soundwire: qcom: add multi-master support Srinivas Kandagatla
@ 2026-09-11 8:21 ` Srinivas Kandagatla
2026-09-11 8:38 ` sashiko-bot
2026-09-11 8:21 ` [PATCH 2/3] soundwire: qcom: add multi-master support Srinivas Kandagatla
2026-09-11 8:21 ` [PATCH 3/3] soundwire: qcom: program MM_SYNC for multi-master Srinivas Kandagatla
2 siblings, 1 reply; 7+ messages in thread
From: Srinivas Kandagatla @ 2026-09-11 8:21 UTC (permalink / raw)
To: robh, vkoul
Cc: krzk+dt, conor+dt, srini, yung-chuan.liao, pierre-louis.bossart,
prasad.kumpatla, jingyi.wang, sibi.sankar, quic_srivasam,
linux-arm-msm, devicetree, linux-kernel, linux-sound,
Srinivas Kandagatla
v3.1.0+ Qualcomm SoundWire IPs can be paired to expose one logical
bus with more than two DATA lanes. Add the two properties that
describe such a pair:
qcom,multi-master-mode role in the pair ("primary" or "dependent")
qcom,multi-master-peer <phandle, bus-port-offset> to the other IP
Both nodes carry both properties.
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
---
.../bindings/soundwire/qcom,soundwire.yaml | 109 ++++++++++++++++--
1 file changed, 100 insertions(+), 9 deletions(-)
diff --git a/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml b/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
index 8e6973fa229c..6cf3285ded92 100644
--- a/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
+++ b/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
@@ -215,6 +215,36 @@ properties:
maximum: 4
- const: 0xff
+ qcom,multi-master-mode:
+ description:
+ Role in v3.1.0+ multi-master mode. Both nodes set this; the
+ primary owns SWR_CLK, the dependent contributes DATA lanes
+ only.
+ $ref: /schemas/types.yaml#/definitions/string
+ enum: [ primary, dependent ]
+
+ qcom,multi-master-peer:
+ description:
+ Peers in multi-master mode. Each entry is a
+ <peer-phandle, bus-port-offset> pair. Peer N's DPn slots
+ 1, 2, 3, ... occupy bus ports (offset+1), (offset+2), ...
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+ items:
+ items:
+ - description: peer phandle
+ - description: bus port offset (last primary-local port)
+ minItems: 1
+
+ num-lanes:
+ description:
+ Number of DATA lanes this IP exposes locally. Required in
+ multi-master mode; on the primary it defines the boundary
+ between local bus lanes (0..num-lanes-1) and lanes contributed
+ by the dependent.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 1
+ maximum: 8
+
label:
maxItems: 1
@@ -224,21 +254,82 @@ required:
- interrupts
- clocks
- clock-names
- - '#sound-dai-cells'
- '#address-cells'
- '#size-cells'
- - qcom,ports-offset1
- - qcom,ports-offset2
-
-oneOf:
- - required:
- - qcom,ports-sinterval-low
- - required:
- - qcom,ports-sinterval
allOf:
- $ref: soundwire-controller.yaml#
+ # Single-master (no qcom,multi-master-mode): needs port config.
+ - if:
+ not:
+ required:
+ - qcom,multi-master-mode
+ then:
+ required:
+ - '#sound-dai-cells'
+ - qcom,ports-offset1
+ - qcom,ports-offset2
+ oneOf:
+ - required:
+ - qcom,ports-sinterval-low
+ - required:
+ - qcom,ports-sinterval
+
+ # Multi-master primary: drives the bus, needs port config + MM props.
+ - if:
+ properties:
+ qcom,multi-master-mode:
+ const: primary
+ required:
+ - qcom,multi-master-mode
+ then:
+ properties:
+ compatible:
+ contains:
+ const: qcom,soundwire-v3.1.0
+ required:
+ - '#sound-dai-cells'
+ - qcom,ports-offset1
+ - qcom,ports-offset2
+ - qcom,multi-master-peer
+ - num-lanes
+ oneOf:
+ - required:
+ - qcom,ports-sinterval-low
+ - required:
+ - qcom,ports-sinterval
+
+ # Multi-master dependent: contributes DATA lanes
+ - if:
+ properties:
+ qcom,multi-master-mode:
+ const: dependent
+ required:
+ - qcom,multi-master-mode
+ then:
+ properties:
+ compatible:
+ contains:
+ const: qcom,soundwire-v3.1.0
+ '#sound-dai-cells': false
+ qcom,din-ports: false
+ qcom,dout-ports: false
+ qcom,ports-word-length: false
+ qcom,ports-sinterval-low: false
+ qcom,ports-sinterval: false
+ qcom,ports-offset1: false
+ qcom,ports-offset2: false
+ qcom,ports-lane-control: false
+ qcom,ports-block-pack-mode: false
+ qcom,ports-hstart: false
+ qcom,ports-hstop: false
+ qcom,ports-block-group-count: false
+ qcom,ports-ch-mask: false
+ required:
+ - qcom,multi-master-peer
+ - num-lanes
+
unevaluatedProperties: false
examples:
--
2.53.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/3] soundwire: qcom: add multi-master support
2026-09-11 8:21 [PATCH 0/3] soundwire: qcom: add multi-master support Srinivas Kandagatla
2026-09-11 8:21 ` [PATCH 1/3] dt-bindings: soundwire: qcom: describe multi-master Srinivas Kandagatla
@ 2026-09-11 8:21 ` Srinivas Kandagatla
2026-09-11 8:43 ` sashiko-bot
2026-09-11 8:21 ` [PATCH 3/3] soundwire: qcom: program MM_SYNC for multi-master Srinivas Kandagatla
2 siblings, 1 reply; 7+ messages in thread
From: Srinivas Kandagatla @ 2026-09-11 8:21 UTC (permalink / raw)
To: robh, vkoul
Cc: krzk+dt, conor+dt, srini, yung-chuan.liao, pierre-louis.bossart,
prasad.kumpatla, jingyi.wang, sibi.sankar, quic_srivasam,
linux-arm-msm, devicetree, linux-kernel, linux-sound,
Srinivas Kandagatla
v3.1.0+ Qualcomm SoundWire IPs can be paired to expose one logical
bus with more than two DATA lanes. One IP owns SWR_CLK and drives
the bus; the other contributes DATA lanes only.
Support this pairing on the driver side. A device link between the
two IPs enforces correct unbind ordering and keeps the peer resumed
while the primary is active.
Single-master deployments (no qcom,multi-master-mode in DT) are
unaffected.
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
---
drivers/soundwire/qcom.c | 171 +++++++++++++++++++++++++++++++++++----
1 file changed, 156 insertions(+), 15 deletions(-)
diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index 35ffffd541bd..a5f39868161a 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -10,6 +10,7 @@
#include <linux/debugfs.h>
#include <linux/of.h>
#include <linux/of_irq.h>
+#include <linux/of_platform.h>
#include <linux/pm_runtime.h>
#include <linux/regmap.h>
#include <linux/reset.h>
@@ -111,6 +112,7 @@
#define SWRM_DPn_PORT_HCTRL_BANK(offset, n, m) (offset + 0x100 * (n - 1) + 0x40 * m)
#define SWRM_DPn_BLOCK_CTRL3_BANK(offset, n, m) (offset + 0x100 * (n - 1) + 0x40 * m)
#define SWRM_DPn_SAMPLECTRL2_BANK(offset, n, m) (offset + 0x100 * (n - 1) + 0x40 * m)
+#define SWRM_DPn_SLOT_STRIDE 0x100
#define SWR_V1_3_MSTR_MAX_REG_ADDR 0x1740
#define SWR_V2_0_MSTR_MAX_REG_ADDR 0x50ac
@@ -192,6 +194,13 @@ struct qcom_swrm_ctrl {
const unsigned int *reg_layout;
void __iomem *mmio;
struct reset_control *audio_cgcr;
+ u8 num_lanes;
+ bool is_dependent;
+ struct qcom_swrm_ctrl *peer_ctrl;
+ u8 peer_first_lane;
+ u8 num_peer_lanes;
+ bool is_primary;
+ u32 peer_dpn_offset;
#ifdef CONFIG_DEBUG_FS
struct dentry *debugfs;
#endif
@@ -1040,9 +1049,41 @@ static int qcom_swrm_pre_bank_switch(struct sdw_bus *bus)
u32p_replace_bits(&val, ctrl->cols_index, SWRM_MCP_FRAME_CTRL_BANK_COL_CTRL_BMSK);
u32p_replace_bits(&val, ctrl->rows_index, SWRM_MCP_FRAME_CTRL_BANK_ROW_CTRL_BMSK);
+ if (ctrl->peer_ctrl)
+ ctrl->peer_ctrl->reg_write(ctrl->peer_ctrl, reg, val);
+
return ctrl->reg_write(ctrl, reg, val);
}
+static struct qcom_swrm_ctrl *
+qcom_swrm_port_target(struct qcom_swrm_ctrl *ctrl, u8 port_num, u32 *reg)
+{
+ u8 lane = ctrl->pconfig[port_num].lane_control;
+
+ if (ctrl->peer_ctrl && lane >= ctrl->peer_first_lane &&
+ lane < ctrl->peer_first_lane + ctrl->num_peer_lanes) {
+ *reg -= ctrl->peer_dpn_offset;
+ return ctrl->peer_ctrl;
+ }
+ return ctrl;
+}
+
+static int qcom_swrm_port_reg_write(struct qcom_swrm_ctrl *ctrl, u8 port_num,
+ u32 reg, u32 val)
+{
+ struct qcom_swrm_ctrl *tgt = qcom_swrm_port_target(ctrl, port_num, ®);
+
+ return tgt->reg_write(tgt, reg, val);
+}
+
+static int qcom_swrm_port_reg_read(struct qcom_swrm_ctrl *ctrl, u8 port_num,
+ u32 reg, u32 *val)
+{
+ struct qcom_swrm_ctrl *tgt = qcom_swrm_port_target(ctrl, port_num, ®);
+
+ return tgt->reg_read(tgt, reg, val);
+}
+
static int qcom_swrm_port_params(struct sdw_bus *bus,
struct sdw_port_params *p_params,
unsigned int bank)
@@ -1050,8 +1091,9 @@ static int qcom_swrm_port_params(struct sdw_bus *bus,
struct qcom_swrm_ctrl *ctrl = to_qcom_sdw(bus);
u32 offset = ctrl->reg_layout[SWRM_OFFSET_DP_BLOCK_CTRL_1];
- return ctrl->reg_write(ctrl, SWRM_DPn_BLOCK_CTRL_1(offset, p_params->num),
- p_params->bps - 1);
+ return qcom_swrm_port_reg_write(ctrl, p_params->num,
+ SWRM_DPn_BLOCK_CTRL_1(offset, p_params->num),
+ p_params->bps - 1);
}
static int qcom_swrm_transport_params(struct sdw_bus *bus,
@@ -1072,7 +1114,7 @@ static int qcom_swrm_transport_params(struct sdw_bus *bus,
value |= pcfg->off2 << SWRM_DP_PORT_CTRL_OFFSET2_SHFT;
value |= pcfg->si & 0xff;
- ret = ctrl->reg_write(ctrl, reg, value);
+ ret = qcom_swrm_port_reg_write(ctrl, params->port_num, reg, value);
if (ret)
goto err;
@@ -1081,7 +1123,7 @@ static int qcom_swrm_transport_params(struct sdw_bus *bus,
value = (pcfg->si >> 8) & 0xff;
reg = SWRM_DPn_SAMPLECTRL2_BANK(offset, params->port_num, bank);
- ret = ctrl->reg_write(ctrl, reg, value);
+ ret = qcom_swrm_port_reg_write(ctrl, params->port_num, reg, value);
if (ret)
goto err;
}
@@ -1091,7 +1133,10 @@ static int qcom_swrm_transport_params(struct sdw_bus *bus,
reg = SWRM_DPn_PORT_CTRL_2_BANK(offset, params->port_num, bank);
value = pcfg->lane_control;
- ret = ctrl->reg_write(ctrl, reg, value);
+ if (ctrl->peer_ctrl && value >= ctrl->peer_first_lane &&
+ value < ctrl->peer_first_lane + ctrl->num_peer_lanes)
+ value -= ctrl->peer_first_lane;
+ ret = qcom_swrm_port_reg_write(ctrl, params->port_num, reg, value);
if (ret)
goto err;
}
@@ -1102,7 +1147,7 @@ static int qcom_swrm_transport_params(struct sdw_bus *bus,
reg = SWRM_DPn_BLOCK_CTRL2_BANK(offset, params->port_num, bank);
value = pcfg->blk_group_count;
- ret = ctrl->reg_write(ctrl, reg, value);
+ ret = qcom_swrm_port_reg_write(ctrl, params->port_num, reg, value);
if (ret)
goto err;
}
@@ -1112,10 +1157,10 @@ static int qcom_swrm_transport_params(struct sdw_bus *bus,
if (pcfg->hstart != SWR_INVALID_PARAM && pcfg->hstop != SWR_INVALID_PARAM) {
value = (pcfg->hstop << 4) | pcfg->hstart;
- ret = ctrl->reg_write(ctrl, reg, value);
+ ret = qcom_swrm_port_reg_write(ctrl, params->port_num, reg, value);
} else {
value = (SWR_HSTOP_MAX_VAL << 4) | SWR_HSTART_MIN_VAL;
- ret = ctrl->reg_write(ctrl, reg, value);
+ ret = qcom_swrm_port_reg_write(ctrl, params->port_num, reg, value);
}
if (ret)
@@ -1124,7 +1169,8 @@ static int qcom_swrm_transport_params(struct sdw_bus *bus,
if (pcfg->bp_mode != SWR_INVALID_PARAM) {
offset = ctrl->reg_layout[SWRM_OFFSET_DP_BLOCK_CTRL3_BANK];
reg = SWRM_DPn_BLOCK_CTRL3_BANK(offset, params->port_num, bank);
- ret = ctrl->reg_write(ctrl, reg, pcfg->bp_mode);
+ ret = qcom_swrm_port_reg_write(ctrl, params->port_num, reg,
+ pcfg->bp_mode);
}
err:
@@ -1142,14 +1188,14 @@ static int qcom_swrm_port_enable(struct sdw_bus *bus,
reg = SWRM_DPn_PORT_CTRL_BANK(offset, enable_ch->port_num, bank);
- ctrl->reg_read(ctrl, reg, &val);
+ qcom_swrm_port_reg_read(ctrl, enable_ch->port_num, reg, &val);
if (enable_ch->enable)
val |= (enable_ch->ch_mask << SWRM_DP_PORT_CTRL_EN_CHAN_SHFT);
else
val &= ~(0xff << SWRM_DP_PORT_CTRL_EN_CHAN_SHFT);
- return ctrl->reg_write(ctrl, reg, val);
+ return qcom_swrm_port_reg_write(ctrl, enable_ch->port_num, reg, val);
}
static const struct sdw_master_port_ops qcom_swrm_port_ops = {
@@ -1562,6 +1608,68 @@ static int swrm_reg_show(struct seq_file *s_file, void *data)
DEFINE_SHOW_ATTRIBUTE(swrm_reg);
#endif
+static int qcom_swrm_multi_master_setup(struct qcom_swrm_ctrl *ctrl)
+{
+ struct of_phandle_args args;
+ struct qcom_swrm_ctrl *peer;
+ struct platform_device *pdev;
+ u32 offset;
+ int ret;
+
+ ret = of_parse_phandle_with_fixed_args(ctrl->dev->of_node,
+ "qcom,multi-master-peer",
+ 1, 0, &args);
+ if (ret)
+ return -EINVAL;
+
+ pdev = of_find_device_by_node(args.np);
+ of_node_put(args.np);
+ if (!pdev)
+ return -EPROBE_DEFER;
+
+ if (pdev->dev.driver != ctrl->dev->driver) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ peer = platform_get_drvdata(pdev);
+ if (!peer || !peer->is_dependent) {
+ ret = -EPROBE_DEFER;
+ goto out;
+ }
+
+ if (!ctrl->num_lanes || !peer->num_lanes ||
+ ctrl->num_lanes + peer->num_lanes > SDW_MAX_LANES) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ offset = args.args[0];
+ if (offset < ctrl->num_lanes) {
+ dev_err(ctrl->dev, "qcom,multi-master-peer: offset %u < num-lanes %u\n",
+ offset, ctrl->num_lanes);
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (!device_link_add(ctrl->dev, &pdev->dev,
+ DL_FLAG_AUTOREMOVE_CONSUMER | DL_FLAG_PM_RUNTIME)) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ ctrl->peer_ctrl = peer;
+ peer->peer_ctrl = ctrl;
+ ctrl->peer_first_lane = ctrl->num_lanes;
+ ctrl->num_peer_lanes = peer->num_lanes;
+ ctrl->peer_dpn_offset = offset * SWRM_DPn_SLOT_STRIDE;
+
+ ret = 0;
+out:
+ put_device(&pdev->dev);
+ return ret;
+}
+
static int qcom_swrm_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -1576,6 +1684,9 @@ static int qcom_swrm_probe(struct platform_device *pdev)
if (!ctrl)
return -ENOMEM;
+ if (!of_property_read_u32(dev->of_node, "num-lanes", &val))
+ ctrl->num_lanes = val;
+
memset(ctrl->page1_cache, 0xff, sizeof(ctrl->page1_cache));
memset(ctrl->page2_cache, 0xff, sizeof(ctrl->page2_cache));
@@ -1636,9 +1747,19 @@ static int qcom_swrm_probe(struct platform_device *pdev)
ctrl->bus.compute_params = &qcom_swrm_compute_params;
ctrl->bus.clk_stop_timeout = 300;
- ret = qcom_swrm_get_port_config(ctrl);
- if (ret)
- goto err_clk;
+ if (of_property_match_string(dev->of_node, "qcom,multi-master-mode",
+ "dependent") < 0) {
+ ret = qcom_swrm_get_port_config(ctrl);
+ if (ret)
+ goto err_clk;
+ }
+
+ if (of_property_match_string(dev->of_node, "qcom,multi-master-mode", "primary") >= 0) {
+ ret = qcom_swrm_multi_master_setup(ctrl);
+ if (ret)
+ goto err_clk;
+ ctrl->is_primary = true;
+ }
params = &ctrl->bus.params;
params->max_dr_freq = DEFAULT_CLK_FREQ;
@@ -1689,6 +1810,16 @@ static int qcom_swrm_probe(struct platform_device *pdev)
ctrl->bus.controller_id = val;
}
+ if (of_property_match_string(dev->of_node, "qcom,multi-master-mode", "dependent") >= 0) {
+ ctrl->is_dependent = true;
+ dev_dbg(dev,
+ "Qualcomm SoundWire multi-master dependent v%x.%x.%x registered (%u lanes)\n",
+ (ctrl->version >> 24) & 0xff,
+ (ctrl->version >> 16) & 0xff,
+ ctrl->version & 0xffff, ctrl->num_lanes);
+ return 0;
+ }
+
ret = sdw_bus_master_add(&ctrl->bus, dev, dev->fwnode);
if (ret) {
dev_err(dev, "Failed to register Soundwire controller (%d)\n",
@@ -1696,7 +1827,11 @@ static int qcom_swrm_probe(struct platform_device *pdev)
goto err_clk;
}
+ if (ctrl->is_primary && ctrl->peer_ctrl)
+ qcom_swrm_init(ctrl->peer_ctrl);
+
qcom_swrm_init(ctrl);
+
wait_for_completion_timeout(&ctrl->enumeration,
msecs_to_jiffies(TIMEOUT_MS));
ret = qcom_swrm_register_dais(ctrl);
@@ -1733,7 +1868,13 @@ 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);
+ if (ctrl->is_primary && ctrl->peer_ctrl) {
+ ctrl->peer_ctrl->peer_ctrl = NULL;
+ ctrl->peer_ctrl = NULL;
+ }
+
+ if (!ctrl->is_dependent)
+ sdw_bus_master_delete(&ctrl->bus);
clk_disable_unprepare(ctrl->hclk);
}
--
2.53.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] soundwire: qcom: program MM_SYNC for multi-master
2026-09-11 8:21 [PATCH 0/3] soundwire: qcom: add multi-master support Srinivas Kandagatla
2026-09-11 8:21 ` [PATCH 1/3] dt-bindings: soundwire: qcom: describe multi-master Srinivas Kandagatla
2026-09-11 8:21 ` [PATCH 2/3] soundwire: qcom: add multi-master support Srinivas Kandagatla
@ 2026-09-11 8:21 ` Srinivas Kandagatla
2026-09-11 8:36 ` sashiko-bot
2 siblings, 1 reply; 7+ messages in thread
From: Srinivas Kandagatla @ 2026-09-11 8:21 UTC (permalink / raw)
To: robh, vkoul
Cc: krzk+dt, conor+dt, srini, yung-chuan.liao, pierre-louis.bossart,
prasad.kumpatla, jingyi.wang, sibi.sankar, quic_srivasam,
linux-arm-msm, devicetree, linux-kernel, linux-sound,
Srinivas Kandagatla
The primary and dependent frame generators must lock to a single
SWR_CLK for the shared bus to run. Program SWRM_V3_MM_SYNC_CONFIG
on both sides at init, and re-apply on resume as the register does
not survive a clock stop.
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
---
drivers/soundwire/qcom.c | 47 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 47 insertions(+)
diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index a5f39868161a..3b9b1a53b082 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -46,6 +46,18 @@
#define SWRM_V3_COMP_PARAMS_RD_FIFO_DEPTH GENMASK(23, 18)
#define SWRM_COMP_MASTER_ID 0x104
+
+#define SWRM_V3_MM_SYNC_CONFIG 0x108
+#define SWRM_V3_MM_SYNC_CONNECTED_MASTER_MASK GENMASK(15, 0)
+#define SWRM_V3_MM_SYNC_IS_DEPENDENT_MASTER BIT(16)
+#define SWRM_V3_MM_SYNC_MASK_CONTROL_BITS BIT(17)
+
+#define SWRM_V3_COMP_FEATURE_CFG 0x10
+#define SWRM_V3_COMP_FEATURE_CFG_BASE 0x00E
+#define SWRM_V3_COMP_FEATURE_CFG_FORCE_MODE_EN BIT(9)
+#define SWRM_V3_COMP_FEATURE_CFG_CLK_PIN_AVAIL BIT(10)
+#define SWRM_V3_COMP_FEATURE_CFG_CLK_STOP_EXEC_ON_CMD_IGNORE BIT(12)
+
#define SWRM_V1_3_INTERRUPT_STATUS 0x200
#define SWRM_V2_0_INTERRUPT_STATUS 0x5000
#define SWRM_INTERRUPT_STATUS_RMSK GENMASK(16, 0)
@@ -892,6 +904,37 @@ static bool swrm_wait_for_frame_gen_enabled(struct qcom_swrm_ctrl *ctrl)
return false;
}
+static void qcom_swrm_program_mm_sync(struct qcom_swrm_ctrl *ctrl)
+{
+ struct qcom_swrm_ctrl *peer = ctrl->peer_ctrl;
+ u32 conn_mask;
+
+ if (!peer)
+ return;
+
+ conn_mask = BIT(peer->bus.controller_id - 1) &
+ SWRM_V3_MM_SYNC_CONNECTED_MASTER_MASK;
+
+ if (ctrl->is_primary) {
+ ctrl->reg_write(ctrl, SWRM_V3_MM_SYNC_CONFIG, conn_mask);
+ ctrl->reg_write(ctrl, SWRM_V3_COMP_FEATURE_CFG,
+ SWRM_V3_COMP_FEATURE_CFG_BASE |
+ SWRM_V3_COMP_FEATURE_CFG_FORCE_MODE_EN |
+ SWRM_V3_COMP_FEATURE_CFG_CLK_PIN_AVAIL |
+ SWRM_V3_COMP_FEATURE_CFG_CLK_STOP_EXEC_ON_CMD_IGNORE);
+ return;
+ }
+
+ ctrl->reg_write(ctrl, SWRM_V3_MM_SYNC_CONFIG,
+ SWRM_V3_MM_SYNC_IS_DEPENDENT_MASTER |
+ SWRM_V3_MM_SYNC_MASK_CONTROL_BITS | conn_mask);
+
+ ctrl->reg_write(ctrl, SWRM_V3_COMP_FEATURE_CFG,
+ SWRM_V3_COMP_FEATURE_CFG_BASE |
+ SWRM_V3_COMP_FEATURE_CFG_FORCE_MODE_EN |
+ SWRM_V3_COMP_FEATURE_CFG_CLK_STOP_EXEC_ON_CMD_IGNORE);
+}
+
static int qcom_swrm_init(struct qcom_swrm_ctrl *ctrl)
{
u32 val;
@@ -918,6 +961,8 @@ static int qcom_swrm_init(struct qcom_swrm_ctrl *ctrl)
u32p_replace_bits(&val, SWRM_DEF_CMD_NO_PINGS, SWRM_MCP_CFG_MAX_NUM_OF_CMD_NO_PINGS_BMSK);
ctrl->reg_write(ctrl, SWRM_MCP_CFG_ADDR, val);
+ qcom_swrm_program_mm_sync(ctrl);
+
if (ctrl->version == SWRM_VERSION_1_7_0) {
ctrl->reg_write(ctrl, SWRM_LINK_MANAGER_EE, SWRM_EE_CPU);
ctrl->reg_write(ctrl, SWRM_MCP_BUS_CTRL,
@@ -1909,6 +1954,8 @@ static int __maybe_unused swrm_runtime_resume(struct device *dev)
} else {
reset_control_reset(ctrl->audio_cgcr);
+ qcom_swrm_program_mm_sync(ctrl);
+
if (ctrl->version == SWRM_VERSION_1_7_0) {
ctrl->reg_write(ctrl, SWRM_LINK_MANAGER_EE, SWRM_EE_CPU);
ctrl->reg_write(ctrl, SWRM_MCP_BUS_CTRL,
--
2.53.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] soundwire: qcom: program MM_SYNC for multi-master
2026-09-11 8:21 ` [PATCH 3/3] soundwire: qcom: program MM_SYNC for multi-master Srinivas Kandagatla
@ 2026-09-11 8:36 ` sashiko-bot
0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-09-11 8:36 UTC (permalink / raw)
To: Srinivas Kandagatla; +Cc: devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:
New issues:
- [High] Missing state restoration for the dependent multi-master controller on runtime resume.
- [High] Undefined behavior due to negative bitwise shift in the BIT() macro for controller_id.
- [High] Unconditional write to V3-specific SoundWire configuration registers on all hardware versions.
- [High] Lack of synchronization around cross-device pointer access leads to Use-After-Free during concurrent unbind.
- [High] Probe order race leads to permanent probe failure due to strict driver pointer check.
Pre-existing issues:
- [High] Threaded IRQ handler is not explicitly canceled before freeing the SoundWire bus, leading to Use-After-Free.
- [High] Missing PM runtime disable in the driver removal path leaves background tasks active, causing Use-After-Free.
- [High] The main threaded IRQ handler accesses hardware registers without ensuring the power domain is runtime active.
--
commit 5ac190feb87d94b0d9cbff783e9a732e35943dc4
Author: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
soundwire: qcom: program MM_SYNC for multi-master
This commit configures the SWRM_V3_MM_SYNC_CONFIG register to allow primary
and dependent frame generators to lock to a single SWR_CLK. It programs the
sync config at initialization and re-applies it on resume, as the register
state is lost during clock stop.
> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
> index a5f39868161af..3b9b1a53b0825 100644
> --- a/drivers/soundwire/qcom.c
> +++ b/drivers/soundwire/qcom.c
[ ... ]
> @@ -892,6 +904,37 @@ static bool swrm_wait_for_frame_gen_enabled(struct qcom_swrm_ctrl *ctrl)
> return false;
> }
>
> +static void qcom_swrm_program_mm_sync(struct qcom_swrm_ctrl *ctrl)
> +{
> + struct qcom_swrm_ctrl *peer = ctrl->peer_ctrl;
> + u32 conn_mask;
> +
> + if (!peer)
> + return;
> +
> + conn_mask = BIT(peer->bus.controller_id - 1) &
> + SWRM_V3_MM_SYNC_CONNECTED_MASTER_MASK;
[Severity: High]
Does this code invoke undefined behavior with a negative shift?
If the hardware version is <= 1.3.0, or if SWRM_COMP_MASTER_ID evaluates
to 0, controller_id becomes -1 or 0. This causes
BIT(peer->bus.controller_id - 1) to compute BIT(-2) or BIT(-1).
> +
> + if (ctrl->is_primary) {
> + ctrl->reg_write(ctrl, SWRM_V3_MM_SYNC_CONFIG, conn_mask);
> + ctrl->reg_write(ctrl, SWRM_V3_COMP_FEATURE_CFG,
> + SWRM_V3_COMP_FEATURE_CFG_BASE |
> + SWRM_V3_COMP_FEATURE_CFG_FORCE_MODE_EN |
> + SWRM_V3_COMP_FEATURE_CFG_CLK_PIN_AVAIL |
> + SWRM_V3_COMP_FEATURE_CFG_CLK_STOP_EXEC_ON_CMD_IGNORE);
[Severity: High]
Could this unconditionally write to V3-specific SoundWire configuration
registers on all hardware versions?
Writing to V3-specific configuration registers (0x108 and 0x10) on older
hardware versions (e.g., V1.3 or V1.5) may corrupt unrelated registers
mapped at those offsets or trigger bus faults.
Can we verify that ctrl->version indicates a V3 (or compatible) hardware
revision before writing these registers?
> + return;
> + }
> +
> + ctrl->reg_write(ctrl, SWRM_V3_MM_SYNC_CONFIG,
> + SWRM_V3_MM_SYNC_IS_DEPENDENT_MASTER |
> + SWRM_V3_MM_SYNC_MASK_CONTROL_BITS | conn_mask);
> +
> + ctrl->reg_write(ctrl, SWRM_V3_COMP_FEATURE_CFG,
> + SWRM_V3_COMP_FEATURE_CFG_BASE |
> + SWRM_V3_COMP_FEATURE_CFG_FORCE_MODE_EN |
> + SWRM_V3_COMP_FEATURE_CFG_CLK_STOP_EXEC_ON_CMD_IGNORE);
> +}
> +
[ ... ]
> @@ -1909,6 +1954,8 @@ static int __maybe_unused swrm_runtime_resume(struct device *dev)
> } else {
> reset_control_reset(ctrl->audio_cgcr);
>
> + qcom_swrm_program_mm_sync(ctrl);
[Severity: High]
Does this code miss state restoration for the dependent multi-master
controller on runtime resume?
When the primary device undergoes a runtime PM suspend and resume, this
restores the primary controller's MM_SYNC configuration. However, the
required setup for the dependent controller (ctrl->peer_ctrl) is
missing.
Because the dependent master returns early in qcom_swrm_probe() without
calling pm_runtime_enable(dev), the PM core never calls its own resume
callback:
drivers/soundwire/qcom.c:qcom_swrm_probe() {
...
if (of_property_match_string(dev->of_node, "qcom,multi-master-mode",
"dependent") >= 0) {
ctrl->is_dependent = true;
...
return 0;
}
...
}
Will this cause multi-master synchronization to fail after the first
suspend/resume cycle, breaking the shared bus?
[Severity: High]
Is there a lack of synchronization around cross-device pointer access
that leads to a use-after-free during concurrent unbind?
drivers/soundwire/qcom.c:qcom_swrm_multi_master_setup() {
...
peer = platform_get_drvdata(pdev);
if (!peer || !peer->is_dependent) {
...
}
The dependent controller's devm-allocated driver data is retrieved and
dereferenced locklessly, leaving a race window for it to be freed
before device link creation.
If the dependent device unbinds concurrently (e.g., via sysfs), the ctrl
memory is freed by devres, leaving peer as a dangling pointer before the
device link can be created.
[Severity: High]
Does a probe order race lead to permanent probe failure due to this
strict driver pointer check?
drivers/soundwire/qcom.c:qcom_swrm_multi_master_setup() {
...
if (pdev->dev.driver != ctrl->dev->driver) {
ret = -EINVAL;
goto out;
}
...
}
Because devices probe asynchronously, the dependent controller may not
be bound to its driver yet (making its driver pointer NULL). This
evaluates to true and triggers an -EINVAL return instead of correctly
returning -EPROBE_DEFER, breaking the standard kernel probe deferral
mechanism.
[Severity: High]
This isn't a bug introduced by this patch, but is the threaded IRQ
handler not explicitly canceled before freeing the SoundWire bus?
drivers/soundwire/qcom.c:qcom_swrm_remove() {
...
sdw_bus_master_delete(&ctrl->bus);
clk_disable_unprepare(ctrl->hclk);
...
}
The threaded IRQ handler (requested via devm_request_threaded_irq) is
not explicitly disabled before sdw_bus_master_delete(&ctrl->bus) and
clk_disable_unprepare() in the driver removal path.
If an interrupt fires concurrently or is pending, qcom_swrm_irq_handler()
will execute on a deleted bus and disabled clock, leading to a
use-after-free and potential list corruption inside the SoundWire core,
or bus faults accessing disabled clocks.
[Severity: High]
This is a pre-existing issue, but does missing PM runtime disable in
the driver removal path leave background tasks active, causing a
use-after-free?
drivers/soundwire/qcom.c:qcom_swrm_remove() {
struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(&pdev->dev);
...
}
The qcom_swrm_probe() function manually enables PM runtime and
auto-suspend via pm_runtime_enable(dev), but qcom_swrm_remove() never
calls pm_runtime_disable(dev).
After remove() completes, devres frees the ctrl memory. An active
auto-suspend timer can fire subsequently, calling
swrm_runtime_suspend(), which dereferences the freed ctrl structure.
[Severity: High]
This isn't a bug introduced by this patch, but does the main threaded
IRQ handler access hardware registers without ensuring the power domain
is runtime active?
drivers/soundwire/qcom.c:qcom_swrm_irq_handler() {
...
ctrl->reg_read(ctrl, ctrl->reg_layout[SWRM_REG_INTERRUPT_STATUS],
&intr_sts);
...
}
qcom_swrm_irq_handler() executes asynchronously and reads hardware
registers like SWRM_REG_INTERRUPT_STATUS.
If PM runtime suspends the device power domain before the handler
thread executes, reading registers on a powered-off domain will fault,
as it fails to use pm_runtime_get_sync() or pm_runtime_get_if_active()
to guarantee the power domain is on.
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1789114385.git.srinivas.kandagatla@oss.qualcomm.com?part=3
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] dt-bindings: soundwire: qcom: describe multi-master
2026-09-11 8:21 ` [PATCH 1/3] dt-bindings: soundwire: qcom: describe multi-master Srinivas Kandagatla
@ 2026-09-11 8:38 ` sashiko-bot
0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-09-11 8:38 UTC (permalink / raw)
To: Srinivas Kandagatla; +Cc: robh, conor+dt, devicetree
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Missing `-cells` property definition for `qcom,multi-master-peer` causing dt-schema validation failures.
- [High] Undeclared `qcom,ports-ch-mask` property referenced in conditional block breaks `dt_binding_check`.
- [Medium] Single-master configuration block fails to explicitly forbid multi-master properties, silently allowing incomplete multi-master nodes.
- [Medium] Commit message explicitly lists two properties but the code adds three.
--
commit f97a4adf5082bcb22d04e3fb5cc90da2b743d260
Author: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
dt-bindings: soundwire: qcom: describe multi-master
This commit adds properties to describe multi-master mode in v3.1.0+ Qualcomm
SoundWire IPs. It defines roles and peer relationships for logical buses using
multiple DATA lanes.
> v3.1.0+ Qualcomm SoundWire IPs can be paired to expose one logical
> bus with more than two DATA lanes. Add the two properties that
> describe such a pair:
>
> qcom,multi-master-mode role in the pair ("primary" or "dependent")
> qcom,multi-master-peer <phandle, bus-port-offset> to the other IP
[Severity: Medium]
This isn't a bug, but there is a slight inconsistency in the commit message.
It mentions adding two properties, while the patch actually introduces three
properties, adding num-lanes as well. Should the commit message be updated to
reflect the third property?
> diff --git a/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml b/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
> index 8e6973fa229c9..34ce8caec0a86 100644
> --- a/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
> +++ b/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
[ ... ]
> + qcom,multi-master-peer:
> + description:
> + Peers in multi-master mode. Each entry is a
> + <peer-phandle, bus-port-offset> pair. Peer N's DPn slots
> + 1, 2, 3, ... occupy bus ports (offset+1), (offset+2), ...
> + $ref: /schemas/types.yaml#/definitions/phandle-array
> + items:
> + items:
> + - description: peer phandle
> + - description: bus port offset (last primary-local port)
> + minItems: 1
[Severity: High]
Because qcom,multi-master-peer is defined as a phandle-array, doesn't the
referenced target node need a corresponding #qcom,multi-master-peer-cells
property for dt-schema to parse the arguments?
Since the schema uses unevaluatedProperties: false and does not define
#qcom,multi-master-peer-cells globally, it creates a situation where adding
the cells property fails schema validation, but omitting it causes dtbs_check
to fail. Can this be resolved by declaring the -cells property in this schema?
[ ... ]
> allOf:
> - $ref: soundwire-controller.yaml#
>
> + # Single-master (no qcom,multi-master-mode): needs port config.
> + - if:
> + not:
> + required:
> + - qcom,multi-master-mode
> + then:
> + required:
> + - '#sound-dai-cells'
> + - qcom,ports-offset1
> + - qcom,ports-offset2
[Severity: Medium]
If a user writes an incomplete multi-master device tree node and forgets to
include the qcom,multi-master-mode property, will it silently pass validation
here as a single-master configuration?
Since qcom,multi-master-peer and num-lanes are globally declared,
unevaluatedProperties: false won't catch them. Would it be safer to explicitly
forbid the multi-master properties in this single-master block by setting
qcom,multi-master-peer: false and num-lanes: false?
[ ... ]
> + # Multi-master dependent: contributes DATA lanes
> + - if:
> + properties:
> + qcom,multi-master-mode:
> + const: dependent
> + required:
> + - qcom,multi-master-mode
> + then:
> + properties:
> + compatible:
> + contains:
> + const: qcom,soundwire-v3.1.0
> + '#sound-dai-cells': false
> + qcom,din-ports: false
> + qcom,dout-ports: false
> + qcom,ports-word-length: false
> + qcom,ports-sinterval-low: false
> + qcom,ports-sinterval: false
> + qcom,ports-offset1: false
> + qcom,ports-offset2: false
> + qcom,ports-lane-control: false
> + qcom,ports-block-pack-mode: false
> + qcom,ports-hstart: false
> + qcom,ports-hstop: false
> + qcom,ports-block-group-count: false
> + qcom,ports-ch-mask: false
[Severity: High]
Does qcom,ports-ch-mask need to be globally declared in the top-level
properties block?
The dtschema meta-schema requires all properties evaluated in
if/then/properties blocks to be declared globally, and qcom,ports-ch-mask does
not seem to be present in this file or inherited from soundwire-controller.yaml.
Won't this cause make dt_binding_check to fail with an undeclared property
error?
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1789114385.git.srinivas.kandagatla@oss.qualcomm.com?part=1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] soundwire: qcom: add multi-master support
2026-09-11 8:21 ` [PATCH 2/3] soundwire: qcom: add multi-master support Srinivas Kandagatla
@ 2026-09-11 8:43 ` sashiko-bot
0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-09-11 8:43 UTC (permalink / raw)
To: Srinivas Kandagatla; +Cc: devicetree, conor+dt, robh
Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [High] The primary controller unconditionally enables hardware interrupts on the dependent controller, whose IRQ handler will crash when accessing uninitialized bus structures.
- [High] The dependent controller skips PM runtime initialization, permanently leaving its interface clock enabled and defeating the device link power management.
- [High] If the primary controller's probe fails, the dependent controller is left with a dangling pointer to freed memory and inappropriately enabled hardware interrupts.
- [High] Primary controller permanently fails to probe with -EINVAL if it probes before the dependent controller has bound.
- [High] Lockless access to the peer device's driver state and drvdata during multi-master setup causes a Use-After-Free if the peer device is concurrently unbound.
- [High] Tearing down `peer_ctrl` linkage before calling `sdw_bus_master_delete()` causes data races and potential NULL pointer dereferences for ongoing bus operations.
- [High] The primary controller writes to the peer's hardware registers during probe without ensuring the peer device is runtime resumed.
--
commit 6d727a8db765ee8b093a2252b9a03525a3473f6a
Author: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
soundwire: qcom: add multi-master support
v3.1.0+ Qualcomm SoundWire IPs can be paired to expose one logical bus with
more than two DATA lanes. This patch supports this pairing using a device
link to enforce unbind ordering and power management.
> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
> index 35ffffd541bd1..a5f39868161af 100644
> --- a/drivers/soundwire/qcom.c
> +++ b/drivers/soundwire/qcom.c
[ ... ]
> @@ -1562,6 +1608,68 @@ static int swrm_reg_show(struct seq_file *s_file, void *data)
> DEFINE_SHOW_ATTRIBUTE(swrm_reg);
> #endif
>
> +static int qcom_swrm_multi_master_setup(struct qcom_swrm_ctrl *ctrl)
> +{
> + struct of_phandle_args args;
> + struct qcom_swrm_ctrl *peer;
> + struct platform_device *pdev;
> + u32 offset;
> + int ret;
> +
> + ret = of_parse_phandle_with_fixed_args(ctrl->dev->of_node,
> + "qcom,multi-master-peer",
> + 1, 0, &args);
> + if (ret)
> + return -EINVAL;
> +
> + pdev = of_find_device_by_node(args.np);
> + of_node_put(args.np);
> + if (!pdev)
> + return -EPROBE_DEFER;
> +
> + if (pdev->dev.driver != ctrl->dev->driver) {
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + peer = platform_get_drvdata(pdev);
> + if (!peer || !peer->is_dependent) {
> + ret = -EPROBE_DEFER;
> + goto out;
> + }
[Severity: High]
If the dependent controller is present in the device tree but has not yet
bound to its driver, pdev->dev.driver will be NULL. Does this strict equality
check cause the primary controller to permanently fail with -EINVAL instead of
returning -EPROBE_DEFER to wait for the dependent controller?
[Severity: High]
Additionally, does accessing pdev->dev.driver and platform_get_drvdata(pdev)
without holding device_lock(&pdev->dev) allow for a use-after-free? If the
peer device is concurrently unbound via sysfs, devres could free the
underlying memory while it is being accessed here.
[ ... ]
> @@ -1689,6 +1810,16 @@ static int qcom_swrm_probe(struct platform_device *pdev)
> ctrl->bus.controller_id = val;
> }
>
> + if (of_property_match_string(dev->of_node, "qcom,multi-master-mode", "dependent") >= 0) {
> + ctrl->is_dependent = true;
> + dev_dbg(dev,
> + "Qualcomm SoundWire multi-master dependent v%x.%x.%x registered (%u lanes)\n",
> + (ctrl->version >> 24) & 0xff,
> + (ctrl->version >> 16) & 0xff,
> + ctrl->version & 0xffff, ctrl->num_lanes);
> + return 0;
> + }
> +
[Severity: High]
This early return bypasses all PM runtime setup for the dependent controller,
such as pm_runtime_enable(). Because its PM runtime is disabled, the PM core
considers it permanently suspended. Does this mean its swrm_runtime_suspend
callback is never invoked, permanently leaving ctrl->hclk enabled and
defeating the device link power management?
[Severity: High]
This early return also bypasses the initialization of the bus structures like
ctrl->bus.bus_lock and ctrl->bus.slaves. Later, the primary controller calls
qcom_swrm_init() on the dependent controller, which unconditionally enables
hardware interrupts by writing to SWRM_REG_INTERRUPT_CPU_EN. If the dependent
hardware raises an interrupt, could its IRQ handler crash when attempting to
lock the uninitialized mutex or traverse the uninitialized slave list?
> ret = sdw_bus_master_add(&ctrl->bus, dev, dev->fwnode);
> if (ret) {
> dev_err(dev, "Failed to register Soundwire controller (%d)\n",
> @@ -1696,7 +1827,11 @@ static int qcom_swrm_probe(struct platform_device *pdev)
> goto err_clk;
> }
>
> + if (ctrl->is_primary && ctrl->peer_ctrl)
> + qcom_swrm_init(ctrl->peer_ctrl);
> +
[Severity: High]
This initiates register writes on the peer hardware without first calling
pm_runtime_get_sync() on the primary device. Since the primary controller is
not yet runtime-resumed during this phase of probe, the device link won't
have powered on the dependent controller either. Can this cause an unclocked
access exception or lost writes if the peer hardware is powered off?
[Severity: High]
If probe fails shortly after this (e.g., during qcom_swrm_register_dais()),
the error path unwinds the primary controller's structures. However, it
appears the dependent controller is left with a dangling pointer to freed
memory in peer->peer_ctrl and inappropriately enabled hardware interrupts.
Should the error path clean up the dependent controller's state?
> qcom_swrm_init(ctrl);
> +
> wait_for_completion_timeout(&ctrl->enumeration,
> msecs_to_jiffies(TIMEOUT_MS));
> ret = qcom_swrm_register_dais(ctrl);
> @@ -1733,7 +1868,13 @@ 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);
> + if (ctrl->is_primary && ctrl->peer_ctrl) {
> + ctrl->peer_ctrl->peer_ctrl = NULL;
> + ctrl->peer_ctrl = NULL;
> + }
> +
> + if (!ctrl->is_dependent)
> + sdw_bus_master_delete(&ctrl->bus);
> clk_disable_unprepare(ctrl->hclk);
> }
[Severity: High]
Clearing these cross-references before calling sdw_bus_master_delete() seems
to expose ongoing bus operations to data races and NULL pointer dereferences.
For example, concurrent streams calling qcom_swrm_pre_bank_switch() check
ctrl->peer_ctrl and then dereference it. Should the bus be stopped before
clearing the linkage?
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1789114385.git.srinivas.kandagatla@oss.qualcomm.com?part=2
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-09-11 8:43 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-11 8:21 [PATCH 0/3] soundwire: qcom: add multi-master support Srinivas Kandagatla
2026-09-11 8:21 ` [PATCH 1/3] dt-bindings: soundwire: qcom: describe multi-master Srinivas Kandagatla
2026-09-11 8:38 ` sashiko-bot
2026-09-11 8:21 ` [PATCH 2/3] soundwire: qcom: add multi-master support Srinivas Kandagatla
2026-09-11 8:43 ` sashiko-bot
2026-09-11 8:21 ` [PATCH 3/3] soundwire: qcom: program MM_SYNC for multi-master Srinivas Kandagatla
2026-09-11 8:36 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox