[PATCH 3/3] soundwire: ignore uniqueID when irrelevant

From: Pierre-Louis Bossart
Date: Tue Oct 22 2019 - 19:48:58 EST


The uniqueID is useful when there are two or more devices of the same
type (identical manufacturer ID, part ID) on the same link.

When there is a single device of a given type on a link, its uniqueID
is irrelevant. It's not uncommon on actual platforms to see variations
of the uniqueID, or differences between devID registers and ACPI _ADR
fields.

This patch suggests a filter on startup to identify 'single' devices
and tag them accordingly. The uniqueID is then not used for the probe,
and the device name omits the uniqueID as well.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>
---
drivers/soundwire/bus.c | 7 +++---
drivers/soundwire/slave.c | 52 ++++++++++++++++++++++++++++++++++++---
2 files changed, 52 insertions(+), 7 deletions(-)

diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index fc53dbe57f85..be5d437058ed 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -422,10 +422,11 @@ static struct sdw_slave *sdw_get_slave(struct sdw_bus *bus, int i)

static int sdw_compare_devid(struct sdw_slave *slave, struct sdw_slave_id id)
{
- if (slave->id.unique_id != id.unique_id ||
- slave->id.mfg_id != id.mfg_id ||
+ if (slave->id.mfg_id != id.mfg_id ||
slave->id.part_id != id.part_id ||
- slave->id.class_id != id.class_id)
+ slave->id.class_id != id.class_id ||
+ (slave->id.unique_id != SDW_IGNORED_UNIQUE_ID &&
+ slave->id.unique_id != id.unique_id))
return -ENODEV;

return 0;
diff --git a/drivers/soundwire/slave.c b/drivers/soundwire/slave.c
index 5dbc76772d21..19919975bb6d 100644
--- a/drivers/soundwire/slave.c
+++ b/drivers/soundwire/slave.c
@@ -29,10 +29,17 @@ static int sdw_slave_add(struct sdw_bus *bus,
slave->dev.parent = bus->dev;
slave->dev.fwnode = fwnode;

- /* name shall be sdw:link:mfg:part:class:unique */
- dev_set_name(&slave->dev, "sdw:%x:%x:%x:%x:%x",
- bus->link_id, id->mfg_id, id->part_id,
- id->class_id, id->unique_id);
+ if (id->unique_id == SDW_IGNORED_UNIQUE_ID) {
+ /* name shall be sdw:link:mfg:part:class */
+ dev_set_name(&slave->dev, "sdw:%x:%x:%x:%x",
+ bus->link_id, id->mfg_id, id->part_id,
+ id->class_id);
+ } else {
+ /* name shall be sdw:link:mfg:part:class:unique */
+ dev_set_name(&slave->dev, "sdw:%x:%x:%x:%x:%x",
+ bus->link_id, id->mfg_id, id->part_id,
+ id->class_id, id->unique_id);
+ }

slave->dev.release = sdw_slave_release;
slave->dev.bus = &sdw_bus_type;
@@ -103,6 +110,7 @@ static bool find_slave(struct sdw_bus *bus,
int sdw_acpi_find_slaves(struct sdw_bus *bus)
{
struct acpi_device *adev, *parent;
+ struct acpi_device *adev2, *parent2;

parent = ACPI_COMPANION(bus->dev);
if (!parent) {
@@ -112,10 +120,46 @@ int sdw_acpi_find_slaves(struct sdw_bus *bus)

list_for_each_entry(adev, &parent->children, node) {
struct sdw_slave_id id;
+ struct sdw_slave_id id2;
+ bool ignore_unique_id = true;

if (!find_slave(bus, adev, &id))
continue;

+ /* brute-force O(N^2) search for duplicates */
+ parent2 = parent;
+ list_for_each_entry(adev2, &parent2->children, node) {
+
+ if (adev == adev2)
+ continue;
+
+ if (!find_slave(bus, adev2, &id2))
+ continue;
+
+ if (id.sdw_version != id2.sdw_version ||
+ id.mfg_id != id2.mfg_id ||
+ id.part_id != id2.part_id ||
+ id.class_id != id2.class_id)
+ continue;
+
+ if (id.unique_id != id2.unique_id) {
+ dev_dbg(bus->dev,
+ "Valid unique IDs %x %x for Slave mfg %x part %d\n",
+ id.unique_id, id2.unique_id,
+ id.mfg_id, id.part_id);
+ ignore_unique_id = false;
+ } else {
+ dev_err(bus->dev,
+ "Invalid unique IDs %x %x for Slave mfg %x part %d\n",
+ id.unique_id, id2.unique_id,
+ id.mfg_id, id.part_id);
+ return -ENODEV;
+ }
+ }
+
+ if (ignore_unique_id)
+ id.unique_id = SDW_IGNORED_UNIQUE_ID;
+
/*
* don't error check for sdw_slave_add as we want to continue
* adding Slaves
--
2.20.1