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

[Richard Fitzgerald]

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.