Re: [PATCH 2/2] soundwire: Intel: stop sdw clock in system suspend

[Richard Fitzgerald <rf@opensource.cirrus.com>]

On 30/06/2026 11:35 am, Liao, Bard wrote:
> 
> 
>> -----Original Message-----
>> From: Pierre-Louis Bossart <pierre-louis.bossart@linux.dev>
>> Sent: Tuesday, June 30, 2026 5:33 PM
>> 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 2/2] soundwire: Intel: stop sdw clock in system suspend
>>
>> On 6/29/26 16:44, Bard Liao wrote:
>>> There is no need to keep the SoundWire clock active in system suspend.
>>>
>>> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
>>> Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
>>> ---
>>>   drivers/soundwire/intel_auxdevice.c | 3 ++-
>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/soundwire/intel_auxdevice.c
>> b/drivers/soundwire/intel_auxdevice.c
>>> index 0b8107bec9ab..10fd27f4fe39 100644
>>> --- a/drivers/soundwire/intel_auxdevice.c
>>> +++ b/drivers/soundwire/intel_auxdevice.c
>>> @@ -669,7 +669,8 @@ static int __maybe_unused intel_suspend(struct
>> device *dev)
>>>   		return 0;
>>>   	}
>>>
>>> -	ret = sdw_intel_stop_bus(sdw, false);
>>> +	/* No need to keep the SoundWire clock active in system suspend */
>>> +	ret = sdw_intel_stop_bus(sdw, true);
>>
>> erm, are you sure about this change?
>>
>> What this does is stop the SoundWire clock before entering system suspend.
>> Is this needed? The whole point of system suspend is that the host will stop
>> operating completely.

Not necessarily. There are multiple types of system suspend, like
suspend-to-RAM. The system doesn't always shut down.
Also depends on the OS: Android seems to use system suspend as a deep
sleep state.

> 
> Yes, that's what we need. The purpose is to inform the Peripheral to go
> into the deserved clock stop mode before system suspend.
> 
>> There is in theory no need to stop the clock because the clock restart capability
>> will not be used.

As Bard says, the peripherals should be informed that the clock will be
stopped. They at least should have the opportunity to act on that. And
they could still be powered across one of the lighter forms of system
suspend.

>> Think for example of jack detection, it's supported in pm_runtime suspend
>> with the SoundWire interrupt capability, but not in system suspend.
> 
> An example that we need clock stop1 in system suspend is to keep the amp's
> firmware in system suspend. In other words, we don't want to reload the
> firmware after system resume
> 
>> By changing the clock-stop argument to true, you are requesting the clock
>> stop mechanism to be enabled even though you don't need it.
>>
>> I do recall that in some generations it was actually not supported to turn off
>> power to the IP while the wake detector was enabled.
>>
>>>   	if (ret < 0) {
>>>   		dev_err(dev, "%s: cannot stop bus: %d\n", __func__, ret);
>>>   		return ret;
> 
>