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@opensource.cirrus.com --- 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 */