soundwire: only handle alert events when the peripheral is attached

[PATCH] soundwire: only handle alert events when the peripheral is attached

Posted by Bard Liao 4 days, 21 hours ago

It doesn't make sense to handle an alert event when the peripheral is
not attached. The slave->status could be SDW_SLAVE_ATTACHED or
SDW_SLAVE_ALERT when it is attached on the bus.

Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
---
 drivers/soundwire/bus.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index fe5316d93fef..0490777fa406 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -1958,6 +1958,10 @@ int sdw_handle_slave_status(struct sdw_bus *bus,
 			break;
 
 		case SDW_SLAVE_ALERT:
+			if (slave->status != SDW_SLAVE_ATTACHED &&
+			    slave->status != SDW_SLAVE_ALERT)
+				continue;
+
 			ret = sdw_handle_slave_alerts(slave);
 			if (ret < 0)
 				dev_err(&slave->dev,
-- 
2.43.0

Re: [PATCH] soundwire: only handle alert events when the peripheral is attached

Posted by Pierre-Louis Bossart 4 days, 7 hours ago

On 5/20/26 04:57, Bard Liao wrote:
> It doesn't make sense to handle an alert event when the peripheral is
> not attached. The slave->status could be SDW_SLAVE_ATTACHED or
> SDW_SLAVE_ALERT when it is attached on the bus.

How would you get an ALERT if the peripheral is not attached in the first place?
The status is only reported in a PING frame after enumeration.

Not following what this new test is needed for...
 
> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
> Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
> Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> ---
>  drivers/soundwire/bus.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
> index fe5316d93fef..0490777fa406 100644
> --- a/drivers/soundwire/bus.c
> +++ b/drivers/soundwire/bus.c
> @@ -1958,6 +1958,10 @@ int sdw_handle_slave_status(struct sdw_bus *bus,
>  			break;
>  
>  		case SDW_SLAVE_ALERT:
> +			if (slave->status != SDW_SLAVE_ATTACHED &&
> +			    slave->status != SDW_SLAVE_ALERT)
> +				continue;
> +
>  			ret = sdw_handle_slave_alerts(slave);
>  			if (ret < 0)
>  				dev_err(&slave->dev,

RE: [PATCH] soundwire: only handle alert events when the peripheral is attached

Posted by Liao, Bard 3 days, 21 hours ago


> -----Original Message-----
> From: Pierre-Louis Bossart <pierre-louis.bossart@linux.dev>
> Sent: Thursday, May 21, 2026 1:21 AM
> To: Bard Liao <yung-chuan.liao@linux.intel.com>; linux-
> sound@vger.kernel.org; vkoul@kernel.org
> Cc: vinod.koul@linaro.org; linux-kernel@vger.kernel.org;
> peter.ujfalusi@linux.intel.com; Liao, Bard <bard.liao@intel.com>
> Subject: Re: [PATCH] soundwire: only handle alert events when the peripheral
> is attached
> 
> On 5/20/26 04:57, Bard Liao wrote:
> > It doesn't make sense to handle an alert event when the peripheral is
> > not attached. The slave->status could be SDW_SLAVE_ATTACHED or
> > SDW_SLAVE_ALERT when it is attached on the bus.
> 
> How would you get an ALERT if the peripheral is not attached in the first place?
> The status is only reported in a PING frame after enumeration.
> 
> Not following what this new test is needed for...

It could be a race condition. We meet the issue rarely when running
suspend/resume test. sdw_handle_slave_alerts() is called when the
peripheral is still unattached. Hope this patch can filter this corner
case.
> 
> > Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
> > Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
> > Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> > ---
> >  drivers/soundwire/bus.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
> > index fe5316d93fef..0490777fa406 100644
> > --- a/drivers/soundwire/bus.c
> > +++ b/drivers/soundwire/bus.c
> > @@ -1958,6 +1958,10 @@ int sdw_handle_slave_status(struct sdw_bus
> *bus,
> >  			break;
> >
> >  		case SDW_SLAVE_ALERT:
> > +			if (slave->status != SDW_SLAVE_ATTACHED &&
> > +			    slave->status != SDW_SLAVE_ALERT)
> > +				continue;
> > +
> >  			ret = sdw_handle_slave_alerts(slave);
> >  			if (ret < 0)
> >  				dev_err(&slave->dev,

Re: [PATCH] soundwire: only handle alert events when the peripheral is attached

Posted by Pierre-Louis Bossart 3 days, 11 hours ago

On 5/21/26 04:24, Liao, Bard wrote:
> 
> 
>> -----Original Message-----
>> From: Pierre-Louis Bossart <pierre-louis.bossart@linux.dev>
>> Sent: Thursday, May 21, 2026 1:21 AM
>> To: Bard Liao <yung-chuan.liao@linux.intel.com>; linux-
>> sound@vger.kernel.org; vkoul@kernel.org
>> Cc: vinod.koul@linaro.org; linux-kernel@vger.kernel.org;
>> peter.ujfalusi@linux.intel.com; Liao, Bard <bard.liao@intel.com>
>> Subject: Re: [PATCH] soundwire: only handle alert events when the peripheral
>> is attached
>>
>> On 5/20/26 04:57, Bard Liao wrote:
>>> It doesn't make sense to handle an alert event when the peripheral is
>>> not attached. The slave->status could be SDW_SLAVE_ATTACHED or
>>> SDW_SLAVE_ALERT when it is attached on the bus.
>>
>> How would you get an ALERT if the peripheral is not attached in the first place?
>> The status is only reported in a PING frame after enumeration.
>>
>> Not following what this new test is needed for...
> 
> It could be a race condition. We meet the issue rarely when running
> suspend/resume test. sdw_handle_slave_alerts() is called when the
> peripheral is still unattached. Hope this patch can filter this corner
> case.

Humm, it doesn't seem like this patch fixes the issue completely.
Either the status read from the Cadence registers is wrong, possibly due to a power-up sequence that doesn't reset registers properly.
Or there's a race indeed where the enumeration happens but somehow the status isn't saved before the alert comes in.

At any rate this patch is probably not enough but it's not wrong either so

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.dev>

>>> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
>>> Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
>>> Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
>>> ---
>>>  drivers/soundwire/bus.c | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
>>> index fe5316d93fef..0490777fa406 100644
>>> --- a/drivers/soundwire/bus.c
>>> +++ b/drivers/soundwire/bus.c
>>> @@ -1958,6 +1958,10 @@ int sdw_handle_slave_status(struct sdw_bus
>> *bus,
>>>  			break;
>>>
>>>  		case SDW_SLAVE_ALERT:
>>> +			if (slave->status != SDW_SLAVE_ATTACHED &&
>>> +			    slave->status != SDW_SLAVE_ALERT)
>>> +				continue;
>>> +
>>>  			ret = sdw_handle_slave_alerts(slave);
>>>  			if (ret < 0)
>>>  				dev_err(&slave->dev,
>