On 03/07/23 19:50, Pierre-Louis Bossart wrote: > > On 7/3/23 15:31, Mukunda,Vijendar wrote: >> On 03/07/23 18:30, Shuming [范書銘] wrote: >>>>>>> During ClockStop Mode0, peripheral interrupts are disabled. >>>>>> I can see that the interrupts are disabled in >>>>>> rt5682_dev_system_suspend(), which is NOT a mode where the clock stop >>>>>> is used... I don't think this commit message is correct. >>>>>> >>>>>> The IMPL_DEF interrupt which is used for jack detection is not >>>>>> disabled at all during any clock stop mode, and it shouldn't >>>>>> otherwise that would break the jack detection. >>>>> You are right. The commit message is wrong and not clear. >>>>> The situation is that the manager driver uses the clock stop mode0 to do >>>> system suspension. >>>> >>>> No it does not. The clock stop is ONLY used for pm_runtime, never for system >>>> suspend. We cannot go to system suspend with the link in clock-stop mode, >>>> that will create lots of issues, that's why we perform a full pm_runtime resume >>>> in the .prepare stage. >>> OK, I got your point. Thanks. However, this issue reported by AMD. >>> The AMD platform validated system level pm and runtime pm ops with the different modes. >>> >>> Hi Vijendar, >>> Do you have any comments? >> On AMD platforms, we are supporting two power modes. >> 1) Bus reset mode >> 2) Clock Stop Mode >> >> In Bus reset mode, bus will re-enumerate the peripheral devices >> whereas in ClockStop Mode, applying ClockStop Mode0 >> in both pm ops (runtime pm ops and system level pm ops). >> >> Currently, SDCA interrupts are disabled on peripheral side, when system level >> suspend is invoked. >> For ClockStop mode SDW manager is not receiving any jack alert when >> SoundWire manager device is in D3 state. > That was precisely the point of clock stop mode: a peripheral can > restart the system even when it's in lower-power mode. > > If there's no means to let a peripheral restart, the only benefit is > maybe to skip the enumeration time. That's not what the spec intended.... As per our understanding, you are pointing to ClockStopMode1. ClockStopMode1 requires re-enumeration as peripherals move to unattached state. We have cross-checked ClockStopMode0 description in spec. It doesn't specify about peripheral device state as Unattached. We are referring here "ClockStopMode0" only. > >> Our expectation is when ClockStop Mode is selected, Only ClockStopMode0 >> should be applied for system level suspend as well. >> We are not expecting bus reset. >> >> We have validated these changes on our platform with Clock stop mode. >> It's working fine. >> >>>>> The SdW device will not be re-attached when the system resume. >>>> it will re-attach, and in addition it will lose context because the manager >>>> performs a complete reset of the bus. >>>> >>>> So what's needed is to enable the interrupt, no matter what happened in the >>>> suspend transition. >>>> >>>> >>>> ------Please consider the environment before printing this e-mail.