The driver probe takes care of basic initialization and is invoked when a Slave becomes attached, after a match between the Slave DevID registers and ACPI/DT entries.
The update_status callback is invoked when a Slave state changes, e.g. when it is assigned a non-zero Device Number and it reports with an ATTACHED/ALERT state.
The state change detection is usually hardware-based and based on the SoundWire frame rate (e.g. double-digit microseconds) while the probe is a pure software operation, which may involve a kernel module load. In corner cases, it's possible that the state changes before the probe completes.
This patch suggests the use of wait_for_completion to avoid races on startup, so that the update_status callback does not rely on invalid pointers/data structures.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- drivers/soundwire/bus.c | 24 +++++++++++++++++++++--- drivers/soundwire/bus.h | 1 + drivers/soundwire/bus_type.c | 5 +++++ drivers/soundwire/slave.c | 2 ++ 4 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index 4b22ee996a65..903aee258800 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -961,10 +961,28 @@ static int sdw_handle_slave_alerts(struct sdw_slave *slave) static int sdw_update_slave_status(struct sdw_slave *slave, enum sdw_slave_status status) { - if (slave->ops && slave->ops->update_status) - return slave->ops->update_status(slave, status); + unsigned long time;
- return 0; + if (!slave->ops || !slave->ops->update_status) + return 0; + + if (!slave->probed) { + /* + * the slave status update is typically handled in an + * interrupt thread, which can race with the driver + * probe, e.g. when a module needs to be loaded. + * + * make sure the probe is complete before updating + * status. + */ + time = wait_for_completion_timeout(&slave->probe_complete, + msecs_to_jiffies(DEFAULT_PROBE_TIMEOUT)); + if (!time) { + dev_err(&slave->dev, "Probe not complete, timed out\n"); + return -ETIMEDOUT; + } + } + return slave->ops->update_status(slave, status); }
/** diff --git a/drivers/soundwire/bus.h b/drivers/soundwire/bus.h index cb482da914da..acb8d11a4c84 100644 --- a/drivers/soundwire/bus.h +++ b/drivers/soundwire/bus.h @@ -5,6 +5,7 @@ #define __SDW_BUS_H
#define DEFAULT_BANK_SWITCH_TIMEOUT 3000 +#define DEFAULT_PROBE_TIMEOUT 2000
#if IS_ENABLED(CONFIG_ACPI) int sdw_acpi_find_slaves(struct sdw_bus *bus); diff --git a/drivers/soundwire/bus_type.c b/drivers/soundwire/bus_type.c index fd234f63bf2f..ac484384bd6e 100644 --- a/drivers/soundwire/bus_type.c +++ b/drivers/soundwire/bus_type.c @@ -118,6 +118,11 @@ static int sdw_slave_drv_probe(struct device *dev) slave->bus->clk_stop_timeout = max_t(u32, slave->bus->clk_stop_timeout, slave->prop.clk_stop_timeout);
+ slave->probed = true; + complete(&slave->probe_complete); + + dev_dbg(dev, "probe complete\n"); + return 0; }
diff --git a/drivers/soundwire/slave.c b/drivers/soundwire/slave.c index c87267f12a3b..81b94cd3985e 100644 --- a/drivers/soundwire/slave.c +++ b/drivers/soundwire/slave.c @@ -52,6 +52,8 @@ static int sdw_slave_add(struct sdw_bus *bus, slave->bus = bus; slave->status = SDW_SLAVE_UNATTACHED; slave->dev_num = 0; + init_completion(&slave->probe_complete); + slave->probed = false;
mutex_lock(&bus->bus_lock); list_add_tail(&slave->node, &bus->slaves);