Messages in this thread | | | Date | Thu, 1 Sep 2022 11:16:10 +0530 | Subject | Re: [PATCH v2 1/2] clk: qcom: gcc-sc7180: Keep the USB GDSC always on | From | Rajendra Nayak <> |
| |
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.
| |