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

[rnayak@codeaurora.org (Rajendra Nayak)]

On 07/31/2017 01:55 PM, Rajendra Nayak wrote:
[]..

>>>> 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.

So one issue I run into trying to move the clk_get() to during attach is,
most of these GDSCs to which we are associating the mmagic clocks are
votable and some of them happen to be ON when the gdsc driver probes.

And we have this to handle the situation..

        /*
         * Votable GDSCs can be ON due to Vote from other masters.
         * If a Votable GDSC is ON, make sure we have a Vote.
         */
        if ((sc->flags & VOTABLE) && on)
                gdsc_enable(&sc->pd);

And now since we defer clk_get() to a later point, the gdsc
can't really be enabled along with the enable/voting of the 
associated clocks.

I was thinking we do something like this instead,

	/*
	 * Votable GDSCs can be ON due to Vote from other masters.
	 * If *we* haven't Voted for it, tell genpd its actually OFF.
	 */
	if ((sc->flags & VOTABLE) && on)
		on = !on;

What do you think?

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