linux-kernel - Re: [PATCH 3/3] clk: qcom: gcc-sc7280: Update the .pwrsts for usb gdsc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 14 Sep 2022 14:07:06 +0530
From:   Krishna Kurapati PSSNV <quic_kriskura@...cinc.com>
To:     Johan Hovold <johan@...nel.org>,
        Rajendra Nayak <quic_rjendra@...cinc.com>
CC:     <andersson@...nel.org>, <agross@...nel.org>,
        <konrad.dybcio@...ainline.org>, <mturquette@...libre.com>,
        <sboyd@...nel.org>, <mka@...omium.org>, <johan+linaro@...nel.org>,
        <dianders@...omium.org>, <linux-clk@...r.kernel.org>,
        <linux-arm-msm@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/3] clk: qcom: gcc-sc7280: Update the .pwrsts for usb
 gdsc



On 9/14/2022 12:39 PM, Johan Hovold wrote:
> On Thu, Sep 01, 2022 at 03:47:56PM +0530, Rajendra Nayak wrote:
>> USB on sc7280 cannot support wakeups from low power states
>> if the GDSC is turned OFF. Update the .pwrsts for usb GDSC so it
>> only transitions to RET in low power.
> 
> It seems this isn't just needed for wakeup to work. On both sc7280 and
> sc8280xp the controller doesn't resume properly if the domain has been
> powered off (i.e. regardless of whether wakeup is enabled or not).
> 
Hi Johan,

   I believe you are referring to the reinit that happens in xhci resume 
path after wakeup happens:

[   48.675839] xhci-hcd xhci-hcd.14.auto: xHC error in resume, USBSTS 
0x411, Reinit

I see that when USB GDSC is not in retention, we don't retain controller 
state and go for reinit and re-enum of connected devices. We are seeing 
an additional delay of around ~0.7 sec (in chromebooks running on 
SC7280) in the wakeup path for re-enumeration of connected USB devices. 
To avoid this, we wanted to put GDSC in retention during PM suspend.

> Are you sure there's no state that needs to be retained regardless of
> the wakeup setting?
> 
>> Signed-off-by: Rajendra Nayak <quic_rjendra@...cinc.com>
>> ---
>>   drivers/clk/qcom/gcc-sc7280.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/qcom/gcc-sc7280.c b/drivers/clk/qcom/gcc-sc7280.c
>> index 7ff64d4d5920..de29a034e725 100644
>> --- a/drivers/clk/qcom/gcc-sc7280.c
>> +++ b/drivers/clk/qcom/gcc-sc7280.c
>> @@ -3126,7 +3126,7 @@ static struct gdsc gcc_usb30_prim_gdsc = {
>>   	.pd = {
>>   		.name = "gcc_usb30_prim_gdsc",
>>   	},
>> -	.pwrsts = PWRSTS_OFF_ON,
>> +	.pwrsts = PWRSTS_RET_ON,
>>   	.flags = VOTABLE,
>>   };
> 
> And what about gcc_usb30_sec_gdsc?

Currently wakeup is not enabled on secondary controller as its not 
required for end product platform (herobrine variant). So leaving the 
usb30_sec_gdsc as it is for now.

Regards,
Krishna,