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

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

On 30/06/2026 1:36 pm, Pierre-Louis Bossart wrote:
> 
>>>>> --- 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.
> 
> Let's be clear: all types of system suspend do not use the clock stop mode, at least on Intel platforms.
> 
> The clock stop mode is ONLY used in the pm_runtime suspend.
> 
> The main reason for this is that the host cannot keep a detector alive to restart the bus at the request of a peripheral.
> As a result, the jack detection only works in pm_runtime suspend and NOT in system suspend. It's been the default since we introduced support of clock stop...
> 
> Note that in case where the peripheral is not capable of generating a wake, the clock stop is not used either. No need to keep that detector alive on the host if it's never going to see a wake.
> 
>> 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.
> 
> Sorry but this isn't what this patch does.
> 
> I think you are confusing power savings in clock stop mode with the ability for the codec to enter a lower power in system suspend.
> 
> If you do need the ability to put the peripheral in a lower power mode, it's perfectly acceptable, but that can be done in the system suspend routine of the peripheral driver. You do not need or want to piggy-back on the clock stop mechanism which isn't used in 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.
> 
> Again it'd be fine to reach a lower power for the peripheral or keep some power rails active, but that's orthogonal to the clock stop.
> 
>>>> 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
> 
> If you want to use lower power modes in system suspend, that's fine. The suspend/resume routines can be modified to introduce lower power modes on the peripheral side.
> I am not disagreeing with the concept of lowering power consumption in system suspend, just on the mechanisms that you trying to use.
> 
> But keeping the context on the peripheral is one bridge too far I am afraid. Intel platforms perform a bus reset in all cases, including in clock-stop modes, that was the hardware guidance. That means that no matter what you do prior to pm_runtime and system suspend the peripheral WILL lose context.
> 
> To rehash the different points:
> - The only significant difference between pm_runtime and system suspend is the wake detection capability which is only supported in the former case.
> - On Intel platforms, context is lost on the peripheral in all resume routines because of the bus reset, the latency on resume is pretty much the same in both cases
> - The peripheral driver can introduce a lower-power mode in system suspend by changing the system suspend routine.
> 

If the Manager doesn't send a clock-stop, the peripherals don't get a
notification that they can enter a lower-power mode. The clock suddenly
disappears without warning and without the peripherals being notified
why, so they don't have any information to know what is happening.

The Manager should send a clock-stop notification before stopping the
clock.