Only exit sdw_handle_slave_status() right after calling
sdw_program_device_num() if it actually programmed an ID into at
least one device.
sdw_handle_slave_status() should protect itself against phantom
device #0 ATTACHED indications. In that case there is no actual
device still on #0. The early exit relies on there being a status
change to ATTACHED on the reprogrammed device to trigger another
call to sdw_handle_slave_status() which will then handle the status
of all peripherals. If no device was actually programmed with an
ID there won't be a new ATTACHED indication. This can lead to the
status of other peripherals not being handled.
The status passed to sdw_handle_slave_status() is obviously always
from a point of time in the past, and may indicate accumulated
unhandled events (depending how the bus manager operates). It's
possible that a device ID is reprogrammed but the last PING status
captured state just before that, when it was still reporting on
ID #0. Then sdw_handle_slave_status() is called with this PING info,
just before a new PING status is available showing it now on its new
ID. So sdw_handle_slave_status() will receive a phantom report of a
device on #0, but it will not find one.
Signed-off-by: Richard Fitzgerald <rf@xxxxxxxxxxxxxxxxxxxxx>
---
drivers/soundwire/bus.c | 27 +++++++++++++++------------
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 6e569a875a9b..0bcc2d161eb9 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -736,20 +736,19 @@ static int sdw_program_device_num(struct sdw_bus *bus)
struct sdw_slave_id id;
struct sdw_msg msg;
bool found;
- int count = 0, ret;
+ int count = 0, num_programmed = 0, ret;
u64 addr;
/* No Slave, so use raw xfer api */
ret = sdw_fill_msg(&msg, NULL, SDW_SCP_DEVID_0,
SDW_NUM_DEV_ID_REGISTERS, 0, SDW_MSG_FLAG_READ, buf);
if (ret < 0)
- return ret;
+ return 0;
do {
ret = sdw_transfer(bus, &msg);
if (ret == -ENODATA) { /* end of device id reads */
dev_dbg(bus->dev, "No more devices to enumerate\n");
- ret = 0;
break;
}
if (ret < 0) {
@@ -781,7 +780,7 @@ static int sdw_program_device_num(struct sdw_bus *bus)
* assigned a device ID.
*/
if (slave->status != SDW_SLAVE_UNATTACHED)
- return 0;
+ return num_programmed;
/*
* Assign a new dev_num to this Slave and
@@ -794,9 +793,11 @@ static int sdw_program_device_num(struct sdw_bus *bus)
dev_err(bus->dev,
"Assign dev_num failed:%d\n",
ret);
- return ret;
+ return num_programmed;
}
+ ++num_programmed;
+
break;
}
}
@@ -825,7 +826,7 @@ static int sdw_program_device_num(struct sdw_bus *bus)
} while (ret == 0 && count < (SDW_MAX_DEVICES * 2));
- return ret;
+ return num_programmed;
}
static void sdw_modify_slave_status(struct sdw_slave *slave,
@@ -1787,14 +1788,16 @@ int sdw_handle_slave_status(struct sdw_bus *bus,
if (status[0] == SDW_SLAVE_ATTACHED) {
dev_dbg(bus->dev, "Slave attached, programming device number\n");
- ret = sdw_program_device_num(bus);
- if (ret < 0)
- dev_err(bus->dev, "Slave attach failed: %d\n", ret);
+
/*
- * programming a device number will have side effects,
- * so we deal with other devices at a later time
+ * Programming a device number will have side effects,
+ * so we deal with other devices at a later time.
+ * But only if any devices were reprogrammed, because
+ * this relies on its PING state changing to ATTACHED,
+ * triggering a status change.
*/
- return ret;
+ if (sdw_program_device_num(bus))
+ return 0;
}
/* Continue to check other slave statuses */
--
2.30.2
