On 05/29/2014 11:35 AM, Mike Turquette wrote:
> Quoting Alex Elder (2014-05-29 06:26:15)
>> On 05/23/2014 07:53 PM, Mike Turquette wrote:
>>> The above seems like a lot effort to go to. Why not skip all of this and
>>> just implement the prerequisite logic in the .enable & .disable
>>> callbacks? E.g. your kona clk .enable callback would look like:
>>
>> I think the problem is that it means the clock consumers
>> would have to know that prerequisite relationship. And
>> that is dependent on the clock tree. The need for it in
>> this case was because the boot loader didn't initialize
>> all the clocks that were needed. If we could count on
>> the boot loader setting things up initially we might not
>> need to do this.
I think you've convinced me that if the prerequisite is
set up at initialization time, the consumers don't need
to know about the the clock tree.
>>>
>>> diff --git a/drivers/clk/bcm/clk-kona.c b/drivers/clk/bcm/clk-kona.c
>>> index d603c4e..51f35b4 100644
>>> --- a/drivers/clk/bcm/clk-kona.c
>>> +++ b/drivers/clk/bcm/clk-kona.c
>>> @@ -987,6 +987,12 @@ static int kona_peri_clk_enable(struct clk_hw *hw)
>>> {
>>> struct kona_clk *bcm_clk = to_kona_clk(hw);
>>> struct bcm_clk_gate *gate = &bcm_clk->u.peri->gate;
>>> + int ret;
>>> +
>>> + hw->prereq_bus_clk = clk_get(hw->ccu, hw->prereq);
>>> + ret = clk_enable(prereq_bus_clk);
>>> + if (ret)
>>> + return ret;
>>>
>>> return clk_gate(bcm_clk->ccu, bcm_clk->init_data.name, gate, true);
>>> }
>>> @@ -997,6 +1003,9 @@ static void kona_peri_clk_disable(struct clk_hw *hw)
>>> struct bcm_clk_gate *gate = &bcm_clk->u.peri->gate;
>>>
>>> (void)clk_gate(bcm_clk->ccu, bcm_clk->init_data.name, gate, false);
>>> +
>>> + clk_disable(hw->prereq_bus_clk);
>>> + clk_put(hw->prereq_bus_clk);
>>> }
>>>
>>> static int kona_peri_clk_is_enabled(struct clk_hw *hw)
>>>
>>>
>>> I guess it might take some trickery to get clk_get to work like that.
>>> Let me know if I've completely lost the plot.
>>
>> I don't think so, but I think there's a lot of stuff
>> here to try to understand, and you're trying to extract
>> it from the code without the benefit of some background
>> of how and why it's done this way.
>>
>> Hopefully all this verbiage is moving you closer to
>> understanding... I appreciate your patience.
>
> Hi Alex,
>
> Can you comment on my diff above? I basically tossed up some pseudo-code
> to show how clk_enable calls can be nested inside of each other. I'd
> like to know if that approach makes sense for your prereq clocks case.
Yes, I should have looked more closely before.
Are you suggesting this prerequisite notion get elevated into the
common framework? Or is "hw" here just representative of the
Kona-specific clock structure?
In any case, you're suggesting the prerequisite be handled in the
enable path (as opposed to the one-time initialization path),
which during the course of this discussion I've been thinking may
be the right way to do it.
Let me see if I can rework it that way and I'll let you know
what I discover as a result. I hope to have something to
talk about later today.
Thanks a lot Mike.
-Alex
> Note that Linux device drivers that consume leaf clocks do NOT need to
> know about the prereq clocks. All of that prereq clock knowledge is
> stored in the .enable callback for the leaf clock (see above).
>
> Regards,
> Mike
>
>>
>> -Alex
>>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
