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: Rander Wang rander.wang@intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- drivers/soundwire/bus.c | 25 ++++++++++++++++++++++--- drivers/soundwire/bus.h | 1 + drivers/soundwire/bus_type.c | 5 +++++ drivers/soundwire/slave.c | 2 ++ 4 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index 4b22ee996a65..d99acbc614c6 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -961,10 +961,29 @@ 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->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; + } + } + + if (!slave->ops || !slave->ops->update_status) + return 0; + + return slave->ops->update_status(slave, status); }
/** diff --git a/drivers/soundwire/bus.h b/drivers/soundwire/bus.h index be01a5f3d00b..93e7bbab0938 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
int sdw_uevent(struct device *dev, struct kobj_uevent_env *env);
diff --git a/drivers/soundwire/bus_type.c b/drivers/soundwire/bus_type.c index df1271f6db61..c9dbc98ac028 100644 --- a/drivers/soundwire/bus_type.c +++ b/drivers/soundwire/bus_type.c @@ -120,6 +120,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 014c3ece1f17..15ac89ecae01 100644 --- a/drivers/soundwire/slave.c +++ b/drivers/soundwire/slave.c @@ -53,6 +53,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);