typos in commit title... On 4/20/23 05:16, Krzysztof Kozlowski wrote: > The Soundwire master controllers might want to check for bus->md Apologies for being pedantic but 'manager' and 'controller' are different concepts in SoundWire, see DisCo spec. It's not a 1:1 mapping, a controller can rely on M managers > initialization to avoid race between early interrupt and finish of > sdw_bus_master_add()/sdw_master_device_add(). Such early interrupt can > happen if Soundwire devices are not powered off during their probe. > > Add a store release barrier, so the Soundwire controllers can safely > check it in concurrent (e.g. in interrupt) way. Can you elaborate on the race condition? I am not following what breaks, and what entity generates the 'early interrupt'. I am specifically concerned about adding this in common code without any matching smp_load_acquire() - which is only added in the following patch for the Qualcomm manager only, but not added for Intel/AMD managers. Is this not a problem? > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> > > --- > > Cc: Patrick Lai <quic_plai@xxxxxxxxxxx> > --- > drivers/soundwire/master.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/soundwire/master.c b/drivers/soundwire/master.c > index 9b05c9e25ebe..d5bf13e7e602 100644 > --- a/drivers/soundwire/master.c > +++ b/drivers/soundwire/master.c > @@ -161,7 +161,12 @@ int sdw_master_device_add(struct sdw_bus *bus, struct device *parent, > /* add shortcuts to improve code readability/compactness */ > md->bus = bus; > bus->dev = &md->dev; > - bus->md = md; > + /* > + * Make sure the contents of md is stored before storing bus->md. > + * Paired with new slave attached and slave status interrupts > + * on the Soundwire master side. > + */ > + smp_store_release(&bus->md, md); > > pm_runtime_set_autosuspend_delay(&bus->md->dev, SDW_MASTER_SUSPEND_DELAY_MS); > pm_runtime_use_autosuspend(&bus->md->dev);
