linux-kernel - Re: [PATCH v2 1/2] clk: qcom: gcc-sc7180: Keep the USB GDSC always on

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] [day] [month] [year] [list]

Message-ID: <095d3acb-b04f-3d24-3f00-b8974bdc2648@quicinc.com>
Date:   Thu, 1 Sep 2022 11:16:10 +0530
From:   Rajendra Nayak <quic_rjendra@...cinc.com>
To:     Stephen Boyd <sboyd@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Matthias Kaehlcke <mka@...omium.org>
CC:     Andy Gross <agross@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...ainline.org>,
        Michael Turquette <mturquette@...libre.com>,
        Johan Hovold <johan+linaro@...nel.org>,
        <linux-clk@...r.kernel.org>,
        "Krishna Kurapati" <quic_kriskura@...cinc.com>,
        <linux-arm-msm@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        Douglas Anderson <dianders@...omium.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>
Subject: Re: [PATCH v2 1/2] clk: qcom: gcc-sc7180: Keep the USB GDSC always on



On 8/31/2022 3:05 AM, Stephen Boyd wrote:
> Quoting Rajendra Nayak (2022-08-29 01:12:02)
>> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
>> index d3244006c661..0fe017ba901b 100644
>> --- a/drivers/clk/qcom/gdsc.c
>> +++ b/drivers/clk/qcom/gdsc.c
>> @@ -368,6 +368,10 @@ static int _gdsc_disable(struct gdsc *sc)
>>           if (sc->pwrsts & PWRSTS_OFF)
>>                   gdsc_clear_mem_on(sc);
>>
>> +       /* If the GDSC supports RET, do not explicitly power it off */
>> +       if (sc->pwrsts & PWRSTS_RET)
>> +               return 0;
>> +
>>           ret = gdsc_toggle_logic(sc, GDSC_OFF);
>>           if (ret)
>>                   return ret;
>>
>>
>> So with that change, we would then not need the ALWAYS_ON flag set for usb gdsc,
>> instead we would update the .pwrsts to PWRSTS_RET_ON instead of PWRSTS_OFF_ON,
>> and that should make both usb wake-ups to work and we can still have the usb_gdsc as
>> a subdomain of CX for performance state voting.
> 
> To clarify, usb_gdsc is not setup as a subdomain of CX so far, right?
> Just that eventually we'll make usb_gdsc a subdomain of CX so that
> dev_pm_opp_set_rate() can target the CX domain instead of the usb one?

Thats right,

> 
>> Does that sounds like a reasonable solution?
> 
> It sounds good to me. What about the existing users of PWRSTS_RET

hmm, I thought no ones been actually using PWRSTS_RET but looks like
there is *one* user on msm8974, I'll need to dig up how this change would
affect it,

> though? If I understand correctly this flag means the domain will never
> be turned off, instead it will hit retention during low power modes.

right,

> 
> While you're crafting this patch can you also document the PWRSTS_*
> defines so that we know what they mean? I can guess that PWRSTS_RET
> means "retention" but I don't know what it really means. I guess it
> means "Deepest power off state is retention of memory cells".

Sure I will update the PWRSTS_RET description. The definition of 'retention'
here I think remains the same, it means memory is retained (if RETAIN_MEM is set)
or memory *and* some part of logic is retained (if both RETAIN_MEM and RETAIN_PERIPH
are set) in Deepest power off state, however there is a disconnected in terms of
'how to enter the retention state' for a given domain.
There is no SW control to enter the retention state and this happens in HW when the
parent domain/rail transitions to low power. Either the parents lowest power state
is also retention (like is the case with sc7180/sc7280 for usb with cx as parent) or the
parent makes sure it hands the domain over to another parent (CX to MX in some cases)
to support retention.