[PATCH v2 3/5] clk: qcom: gdsc: Add support to control associated clks

[Rajendra Nayak]

On 07/28/2017 10:07 PM, Stephen Boyd wrote:
> On 07/28, Rajendra Nayak wrote:
>>
>>
>> On 07/28/2017 04:32 AM, Stephen Boyd wrote:
>>> On 07/20, Rajendra Nayak wrote:
>>>
>>>> +		clk_disable_unprepare(sc->clks[i]);
>>>> +}
>>>> +
>>>>  static int gdsc_enable(struct generic_pm_domain *domain)
>>>>  {
>>>>  	struct gdsc *sc = domain_to_gdsc(domain);
>>>> @@ -193,6 +219,10 @@ static int gdsc_enable(struct generic_pm_domain *domain)
>>>>  	 */
>>>>  	udelay(1);
>>>>  
>>>> +	ret = gdsc_clk_enable(sc);
>>>> +	if (ret)
>>>> +		return ret;
>>>> +
>>>>  	/* Turn on HW trigger mode if supported */
>>>>  	if (sc->flags & HW_CTRL) {
>>>>  		ret = gdsc_hwctrl(sc, true);
>>>> @@ -241,6 +271,8 @@ static int gdsc_disable(struct generic_pm_domain *domain)
>>>>  			return ret;
>>>>  	}
>>>>  
>>>> +	gdsc_clk_disable(sc);
>>>
>>> Can we call sleeping APIs (i.e. prepare/unprepare) from within
>>> the power domains? I thought that didn't work generically because
>>> someone could set their runtime PM configuration to be atomic
>>> context friendly. Is there some way we can block that from
>>> happening if we hook up a power domain that is no longer safe to
>>> call from atomic context?
>>
>> hmm, I don't see a way to do that. Perhaps we could prepare/unprepare
>> these as part of the pm_domain attach/detach to a device and then
>> only enable/disable them as part of the gdsc_enable/disable?
>>
> 
> The problem there is if we keep these clks prepared while the
> domain is attached to a device I don't see how we can ever turn
> off the XO clk and achieve XO shutdown in low power modes. A pm
> domain is basically never detached, right? This is where we need
> different power levels in PM domains if I understand correctly.

right, I did not realize the XO shutdown would happen via a unprepare
callback and not disable.
So looking some more into this, genpd does have a GENPD_FLAG_IRQ_SAFE
used to indicate an IRQ safe domain. We don't set this today for any
of the gdscs so genpd uses mutex locking anyway.
When we do end up supporting/have a need to support IRQ safe domains
we might end up putting a check to make sure we don't associate any
clocks with those gdscs and leave it to the devices to handle it from
within the drivers.

> 
>>>
>>> This concerns me if we do probe defer on orphan clks. We may get
>>> into some situation where the gdsc is setup by a clk driver that
>>> is trying to probe before some parent clk has registered for the
>>> clks we're "getting" here. For example, this could easily happen
>>> if we insert XO into the tree at the top and everything defers.
>>>
>>> I suppose this is not a problem because in this case we don't
>>> have any clk here that could be orphaned even if RPM clks are
>>> inserted into the clk tree for XO? I mean to say that we won't
>>> get into a probe defer due to orphan clk loop. I'm always afraid
>>> someone will make hardware that send a clk from one controller to
>>> another and then back again (we did that with pcie) and then
>>> we'll be unable to get out of the defer loop because we'll be
>>> deferred on orphan status.
>>
>> Yes, we would probably run into these issues with probe defer for
>> orphan clks. One way to handle it could be to decouple the probing
>> of the clocks and powerdomain parts of the controller. Like the clock
>> driver can probe and then register a dummy device for powerdomains
>> (like is done for tsens on 8960) so it could probe independently?
>>
> 
> Well is it a problem right now? I think we're OK here because we
> don't have a "cycle" between clk providers in the clk provider
> hierarchy. Can we clk_get() during attach and then fail attach if
> we can't get clks at that point? If this works, we have a backup
> plan should something go wrong, but we can ignore this concern
> for now until it becomes a real problem.

we don't have a cycle but I was worried about gcc being deferred until
RPM clocks are probed and MMCC getting probed in between, and MMCC init
before GCC init would have us run into issues.
But I should be able to defer the clk_get()'s to during pm domain attach
and it should all work in that case.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation