The existing SoundWire support misses a clear Controller/Manager hiearchical definition to deal with all variants across SOC vendors. a) Intel platforms have one controller with 4 or more Managers. b) AMD platforms have two controllers with one Manager each, but due to BIOS issues use two different link_id values within the scope of a single controller. c) QCOM platforms have one or more controller with one Manager each. This patch adds a 'controller_id' which can be set by higher levels. If assigned to -1, the controller_id will be set to the system-unique IDA-assigned bus->id. The main change is that the bus->id is no longer used for any device name, which makes the definition completely predictable and not dependent on any enumeration order. The bus->id is only used to insert the Managers in the stream rt context. Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com> Reviewed-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> --- drivers/soundwire/amd_manager.c | 8 ++++++++ drivers/soundwire/bus.c | 4 ++++ drivers/soundwire/debugfs.c | 2 +- drivers/soundwire/intel_auxdevice.c | 3 +++ drivers/soundwire/master.c | 2 +- drivers/soundwire/qcom.c | 3 +++ include/linux/soundwire/sdw.h | 4 +++- 7 files changed, 23 insertions(+), 3 deletions(-) diff --git a/drivers/soundwire/amd_manager.c b/drivers/soundwire/amd_manager.c index 3a99f6dcdfaf..a3b1f4e6f0f9 100644 --- a/drivers/soundwire/amd_manager.c +++ b/drivers/soundwire/amd_manager.c @@ -927,6 +927,14 @@ static int amd_sdw_manager_probe(struct platform_device *pdev) amd_manager->bus.clk_stop_timeout = 200; amd_manager->bus.link_id = amd_manager->instance; + /* + * Due to BIOS compatibility, the two links are exposed within + * the scope of a single controller. If this changes, the + * controller_id will have to be updated with drv_data + * information. + */ + amd_manager->bus.controller_id = 0; + switch (amd_manager->instance) { case ACP_SDW0: amd_manager->num_dout_ports = AMD_SDW0_MAX_TX_PORTS; diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index 1720031f35a3..025d3df32bd0 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -22,6 +22,10 @@ static int sdw_get_id(struct sdw_bus *bus) return rc; bus->id = rc; + + if (bus->controller_id == -1) + bus->controller_id = rc; + return 0; } diff --git a/drivers/soundwire/debugfs.c b/drivers/soundwire/debugfs.c index d1553cb77187..67abd7e52f09 100644 --- a/drivers/soundwire/debugfs.c +++ b/drivers/soundwire/debugfs.c @@ -20,7 +20,7 @@ void sdw_bus_debugfs_init(struct sdw_bus *bus) return; /* create the debugfs master-N */ - snprintf(name, sizeof(name), "master-%d-%d", bus->id, bus->link_id); + snprintf(name, sizeof(name), "master-%d-%d", bus->controller_id, bus->link_id); bus->debugfs = debugfs_create_dir(name, sdw_debugfs_root); } diff --git a/drivers/soundwire/intel_auxdevice.c b/drivers/soundwire/intel_auxdevice.c index 7f15e3549e53..93698532deac 100644 --- a/drivers/soundwire/intel_auxdevice.c +++ b/drivers/soundwire/intel_auxdevice.c @@ -234,6 +234,9 @@ static int intel_link_probe(struct auxiliary_device *auxdev, cdns->instance = sdw->instance; cdns->msg_count = 0; + /* single controller for all SoundWire links */ + bus->controller_id = 0; + bus->link_id = auxdev->id; bus->clk_stop_timeout = 1; diff --git a/drivers/soundwire/master.c b/drivers/soundwire/master.c index 9b05c9e25ebe..51abedbbaa66 100644 --- a/drivers/soundwire/master.c +++ b/drivers/soundwire/master.c @@ -145,7 +145,7 @@ int sdw_master_device_add(struct sdw_bus *bus, struct device *parent, md->dev.fwnode = fwnode; md->dev.dma_mask = parent->dma_mask; - dev_set_name(&md->dev, "sdw-master-%d", bus->id); + dev_set_name(&md->dev, "sdw-master-%d-%d", bus->controller_id, bus->link_id); ret = device_register(&md->dev); if (ret) { diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c index 55be9f4b8d59..e3ae4e4e07ac 100644 --- a/drivers/soundwire/qcom.c +++ b/drivers/soundwire/qcom.c @@ -1612,6 +1612,9 @@ static int qcom_swrm_probe(struct platform_device *pdev) } } + /* FIXME: is there a DT-defined value to use ? */ + ctrl->bus.controller_id = -1; + ret = sdw_bus_master_add(&ctrl->bus, dev, dev->fwnode); if (ret) { dev_err(dev, "Failed to register Soundwire controller (%d)\n", diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h index 4f3d14bb1538..c383579a008b 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -886,7 +886,8 @@ struct sdw_master_ops { * struct sdw_bus - SoundWire bus * @dev: Shortcut to &bus->md->dev to avoid changing the entire code. * @md: Master device - * @link_id: Link id number, can be 0 to N, unique for each Master + * @controller_id: system-unique controller ID. If set to -1, the bus @id will be used. + * @link_id: Link id number, can be 0 to N, unique for each Controller * @id: bus system-wide unique id * @slaves: list of Slaves on this bus * @assigned: Bitmap for Slave device numbers. @@ -918,6 +919,7 @@ struct sdw_master_ops { struct sdw_bus { struct device *dev; struct sdw_master_device *md; + int controller_id; unsigned int link_id; int id; struct list_head slaves; -- 2.39.2

Thanks Pierre for the patch, On 17/10/2023 17:09, Pierre-Louis Bossart wrote:

diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c index 55be9f4b8d59..e3ae4e4e07ac 100644 --- a/drivers/soundwire/qcom.c +++ b/drivers/soundwire/qcom.c @@ -1612,6 +1612,9 @@ static int qcom_swrm_probe(struct platform_device *pdev) } }

+ /* FIXME: is there a DT-defined value to use ? */ + ctrl->bus.controller_id = -1; +

We could do a better than this, on Qcom IP we have a dedicated register to provide a master id value. I will send a patch to add this change on top of this patchset. --------------------------------->cut<------------------------------- From 78c516995d652324daadbe848fa787dabcede73f Mon Sep 17 00:00:00 2001 From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> Date: Thu, 23 Nov 2023 10:43:02 +0000 Subject: [PATCH] soundwire: qcom: set controller id to hw master id Qualcomm Soundwire Controllers IP version after 1.3 have a dedicated master id register which will provide a unique id value for each controller instance. Use this value instead of artificially generated value from idr. Versions 1.3 and below only have one instance of soundwire controller which does no have this register, so let them use value from idr. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> --- drivers/soundwire/qcom.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c index 8e027eee8b73..48291fbaf674 100644 --- a/drivers/soundwire/qcom.c +++ b/drivers/soundwire/qcom.c @@ -1624,9 +1624,13 @@ static int qcom_swrm_probe(struct platform_device *pdev) } } - /* FIXME: is there a DT-defined value to use ? */ ctrl->bus.controller_id = -1; + if (ctrl->version > SWRM_VERSION_1_3_0) { + ctrl->reg_read(ctrl, SWRM_COMP_MASTER_ID, &val); + ctrl->bus.controller_id = val; + } + ret = sdw_bus_master_add(&ctrl->bus, dev, dev->fwnode); if (ret) { dev_err(dev, "Failed to register Soundwire controller (%d)\n", -- 2.42.0 --------------------------------->cut<------------------------------- thanks, Srini

ret = sdw_bus_master_add(&ctrl->bus, dev, dev->fwnode); if (ret) { dev_err(dev, "Failed to register Soundwire controller (%d)\n",