clk: mediatek: Export CPU mux clocks for CPU frequency control

Message ID 1425466152-7867-1-git-send-email-pi-cheng.chen@linaro.org
State New
Headers show

Commit Message

pi-cheng.chen March 4, 2015, 10:49 a.m.
This patch adds CPU mux clocks which are used by Mediatek cpufreq driver
for intermediate clock source switching. This patch is based on Mediatek
clock driver patches[1].

[1] http://thread.gmane.org/gmane.linux.kernel/1892436

Signed-off-by: pi-cheng.chen <pi-cheng.chen@linaro.org>
---
 drivers/clk/mediatek/Makefile          |   2 +-
 drivers/clk/mediatek/clk-cpumux.c      | 119 +++++++++++++++++++++++++++++++++
 drivers/clk/mediatek/clk-cpumux.h      |  32 +++++++++
 drivers/clk/mediatek/clk-mt8173.c      |  23 +++++++
 drivers/clk/mediatek/clk-mtk.c         |  39 +++++++++++
 drivers/clk/mediatek/clk-mtk.h         |   4 ++
 include/dt-bindings/clock/mt8173-clk.h |   4 +-
 7 files changed, 221 insertions(+), 2 deletions(-)
 create mode 100644 drivers/clk/mediatek/clk-cpumux.c
 create mode 100644 drivers/clk/mediatek/clk-cpumux.h

Comments

pi-cheng.chen March 5, 2015, 2:43 a.m. | #1
Hi Sascha,

On 4 March 2015 at 19:21, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Wed, Mar 04, 2015 at 06:49:11PM +0800, pi-cheng.chen wrote:
>> This patch adds CPU mux clocks which are used by Mediatek cpufreq driver
>> for intermediate clock source switching. This patch is based on Mediatek
>> clock driver patches[1].
>>
>> [1] http://thread.gmane.org/gmane.linux.kernel/1892436
>>
>> Signed-off-by: pi-cheng.chen <pi-cheng.chen@linaro.org>
>> ---
>> +static long clk_cpumux_determine_rate(struct clk_hw *hw, unsigned long rate,
>> +                                   unsigned long min_rate,
>> +                                   unsigned long max_rate,
>> +                                   unsigned long *best_parent_rate,
>> +                                   struct clk_hw **best_parent_p)
>> +{
>> +     struct clk *clk = hw->clk, *parent;
>> +     unsigned long parent_rate;
>> +     int i;
>> +
>> +     for (i = MAINPLL_INDEX; i >= ARMPLL_INDEX; i--) {
>> +             parent = clk_get_parent_by_index(clk, i);
>> +             if (!parent)
>> +                     return 0;
>> +
>> +             if (i == MAINPLL_INDEX) {
>> +                     parent_rate = __clk_get_rate(parent);
>> +                     if (parent_rate == rate)
>> +                             break;
>> +             }
>> +
>> +             parent_rate = __clk_round_rate(parent, rate);
>> +     }
>> +
>> +     *best_parent_rate = parent_rate;
>> +     *best_parent_p = __clk_get_hw(parent);
>> +     return parent_rate;
>> +}
>
> Why this determine_rate hook? If you want to switch the clock to some
> intermediate parent I would assume you do this explicitly by setting the
> parent and not implicitly by setting a rate.
>

I use determine_rate hook here because I am using generic cpufreq-dt
driver and I want to make clock switching transparent to cpufreq-dt.
I.e. when I am trying to switch the clock from ARMPLL to MAINPLL, I
call clk_set_rate() with the rate of MAINPLL, and determine_rate will
select MAINPLL as the new parent.

>> +int mtk_clk_register_cpumuxes(struct device_node *node,
>> +                           struct mtk_composite *clks, int num,
>> +                           struct clk_onecell_data *clk_data)
>> +{
>> +     int i;
>> +     struct clk *clk;
>> +     struct regmap *regmap;
>> +
>> +     if (!clk_data)
>> +             return -ENOMEM;
>> +
>> +     regmap = syscon_node_to_regmap(node);
>> +     if (IS_ERR(regmap)) {
>> +             pr_err("Cannot find regmap for %s: %ld\n", node->full_name,
>> +                    PTR_ERR(regmap));
>> +             return PTR_ERR(regmap);
>> +     }
>> +
>> +     for (i = 0; i < num; i++) {
>> +             struct mtk_composite *mux = &clks[i];
>> +
>> +             clk = mtk_clk_register_cpumux(mux->name, mux->parent_names,
>> +                                           mux->num_parents, regmap,
>> +                                           mux->mux_reg, mux->mux_shift,
>> +                                           mux->mux_width);
>
> Pass 'mux' directly instead of dispatching this struct into the
> individual fields.
> Also, probably better to move this function to
> drivers/clk/mediatek/clk-cpumux.c

Will do it. Thanks for reviewing.

Best Regards,
Pi-Cheng

>
> Sascha
>
> --
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
pi-cheng.chen March 5, 2015, 8:58 a.m. | #2
On 5 March 2015 at 15:42, Sascha Hauer <s.hauer@pengutronix.de> wrote:
>
> +Cc Viresh Kumar
>
> Viresh, this is the patch for the underlying clocks for the Mediatek
> cpufreq driver.
>
> On Thu, Mar 05, 2015 at 10:43:21AM +0800, Pi-Cheng Chen wrote:
>> Hi Sascha,
>>
>> On 4 March 2015 at 19:21, Sascha Hauer <s.hauer@pengutronix.de> wrote:
>> > On Wed, Mar 04, 2015 at 06:49:11PM +0800, pi-cheng.chen wrote:
>> >> This patch adds CPU mux clocks which are used by Mediatek cpufreq driver
>> >> for intermediate clock source switching. This patch is based on Mediatek
>> >> clock driver patches[1].
>> >>
>> >> [1] http://thread.gmane.org/gmane.linux.kernel/1892436
>> >>
>> >> Signed-off-by: pi-cheng.chen <pi-cheng.chen@linaro.org>
>> >> ---
>> >> +static long clk_cpumux_determine_rate(struct clk_hw *hw, unsigned long rate,
>> >> +                                   unsigned long min_rate,
>> >> +                                   unsigned long max_rate,
>> >> +                                   unsigned long *best_parent_rate,
>> >> +                                   struct clk_hw **best_parent_p)
>> >> +{
>> >> +     struct clk *clk = hw->clk, *parent;
>> >> +     unsigned long parent_rate;
>> >> +     int i;
>> >> +
>> >> +     for (i = MAINPLL_INDEX; i >= ARMPLL_INDEX; i--) {
>> >> +             parent = clk_get_parent_by_index(clk, i);
>> >> +             if (!parent)
>> >> +                     return 0;
>> >> +
>> >> +             if (i == MAINPLL_INDEX) {
>> >> +                     parent_rate = __clk_get_rate(parent);
>> >> +                     if (parent_rate == rate)
>> >> +                             break;
>> >> +             }
>> >> +
>> >> +             parent_rate = __clk_round_rate(parent, rate);
>> >> +     }
>> >> +
>> >> +     *best_parent_rate = parent_rate;
>> >> +     *best_parent_p = __clk_get_hw(parent);
>> >> +     return parent_rate;
>> >> +}
>> >
>> > Why this determine_rate hook? If you want to switch the clock to some
>> > intermediate parent I would assume you do this explicitly by setting the
>> > parent and not implicitly by setting a rate.
>> >
>>
>> I use determine_rate hook here because I am using generic cpufreq-dt
>> driver and I want to make clock switching transparent to cpufreq-dt.
>> I.e. when I am trying to switch the clock from ARMPLL to MAINPLL, I
>> call clk_set_rate() with the rate of MAINPLL, and determine_rate will
>> select MAINPLL as the new parent.
>
> We have clk_set_parent for changing the parent and clk_set_rate to
> change the rate. Use the former for changing the parent and the latter
> for changing the rate. What you are interested in is changing the
> parent, so use clk_set_parent for this and not abuse a side effect
> of clk_set_rate.
>
> My suggestion is to take another approach. Implement clk_set_rate for
> these muxes and in the set_rate hook:
>
> - switch mux to intermediate PLL parent
> - call clk_set_rate() for the real parent PLL
> - switch mux back to real parent PLL
>
> This way the things happening behind the scenes are completely transparent
> to the cpufreq driver and you can use cpufreq-dt as is without changes.

Hi,

Yes. But the frequency of intermediate PLL parent is different from the original
frequency and the frequency after changed of original PLL clock in most cases.
If the frequency of intermediate PLL parent is higher than the
original frequency,
then we have to scale up the voltage first before switch mux to intermediate PLL
parent. I am not sure this could be properly handled if I implement it
in the way
you suggested. For now, I put the mux switching logic in the determine_rate
hook so that the voltage scaling could be handled by target_intermediate hook
of cpufreq-dt which I am also working on[1].

[1] http://marc.info/?l=linux-kernel&m=142545898313351&w=2

Best Regards,
Pi-Cheng

>
> Sascha
>
>
> --
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Viresh Kumar March 5, 2015, 8:59 a.m. | #3
On 5 March 2015 at 13:12, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> We have clk_set_parent for changing the parent and clk_set_rate to
> change the rate. Use the former for changing the parent and the latter
> for changing the rate. What you are interested in is changing the
> parent, so use clk_set_parent for this and not abuse a side effect
> of clk_set_rate.

clk_set_rate() for CPUs clock is responsible to change clock rate
of the CPU. Whether it plays with PLLs or muxes, its not that relevant.

> My suggestion is to take another approach. Implement clk_set_rate for
> these muxes and in the set_rate hook:
>
> - switch mux to intermediate PLL parent
> - call clk_set_rate() for the real parent PLL
> - switch mux back to real parent PLL
>
> This way the things happening behind the scenes are completely transparent
> to the cpufreq driver and you can use cpufreq-dt as is without changes.

CPUFreq wants to change to intermediate frequency by itself against
some magic change behind the scene. The major requirement for that
comes from the fact that we want to send PRE/POST freq notifiers on
which loops-per-jiffie depends.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
pi-cheng.chen March 5, 2015, 9:39 a.m. | #4
On 5 March 2015 at 17:19, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Thu, Mar 05, 2015 at 02:29:50PM +0530, Viresh Kumar wrote:
>> On 5 March 2015 at 13:12, Sascha Hauer <s.hauer@pengutronix.de> wrote:
>> > We have clk_set_parent for changing the parent and clk_set_rate to
>> > change the rate. Use the former for changing the parent and the latter
>> > for changing the rate. What you are interested in is changing the
>> > parent, so use clk_set_parent for this and not abuse a side effect
>> > of clk_set_rate.
>>
>> clk_set_rate() for CPUs clock is responsible to change clock rate
>> of the CPU. Whether it plays with PLLs or muxes, its not that relevant.
>
> The sequence to change the CPU frequency on the Mediatek SoCs is like this:
>
> - Change CPU from CPU PLL to another clock source (intermediate source)
> - Change CPU PLL frequency
> - wait until PLL has settled
> - switch back to CPU PLL
>
> The frequency of the intermediate source is irrelevant, the important
> thing is that the CPU is switched to this source while the CPU PLL is
> reconfigured.
>
> In Pi-Chengs patches the switch to th eintermediate clock is done like:
>
>         rate = clk_get_rate(intermediate_clk);
>         clk_set_rate(cpu_clk, rate);
>
> Now the clk framework does the switch not because it's told to switch
> to another parent, but only because the other parent happens to be the
> only provider for that rate. That's rubbish, when the parent must be
> changed, then it should be done explicitly.

Hi,

Please correct me if I was wrong. But I think that's the reason why we have
a determine_rate hook here, isn't it?

> What if the CPU PLL and the intermediate clk happen to have the same
> rate? Then the clk_set_rate above simply does nothing, no parent is
> changed and the following rate change of the CPU PLL just crashes the
> system.

I think what I am trying to do in the determine_rate hook of cpumux is:

* if the target rate of clk_set_rate is equal to the rate of MAINPLL,
then switch
  the mux to MAINPLL
* otherwise, set the rate directly on ARMPLL and switch the mux back to
  ARMPLL if the current parent of mux is not ARMPLL

And if the CPU PLL and the intermediate clk happen to have the same rate,
I think the cpufreq-dt driver will change nothing on the clk framework. Or do
I misunderstand your point?

Thanks.

Best Regards,
Pi-Cheng

>
>>
>> > My suggestion is to take another approach. Implement clk_set_rate for
>> > these muxes and in the set_rate hook:
>> >
>> > - switch mux to intermediate PLL parent
>> > - call clk_set_rate() for the real parent PLL
>> > - switch mux back to real parent PLL
>> >
>> > This way the things happening behind the scenes are completely transparent
>> > to the cpufreq driver and you can use cpufreq-dt as is without changes.
>>
>> CPUFreq wants to change to intermediate frequency by itself against
>> some magic change behind the scene. The major requirement for that
>> comes from the fact that we want to send PRE/POST freq notifiers on
>> which loops-per-jiffie depends.
>
> Maybe, I don't know the internals of CPUFreq. But here the important
> thing is the rate change, not the parent change.
>
> Sascha
>
> --
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Viresh Kumar March 5, 2015, 10:23 a.m. | #5
On 5 March 2015 at 14:49, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> The sequence to change the CPU frequency on the Mediatek SoCs is like this:
>
> - Change CPU from CPU PLL to another clock source (intermediate source)
> - Change CPU PLL frequency
> - wait until PLL has settled
> - switch back to CPU PLL

This should be the case for most of the intermediate-freq users..

> The frequency of the intermediate source is irrelevant, the important
> thing is that the CPU is switched to this source while the CPU PLL is
> reconfigured.

Right.

> In Pi-Chengs patches the switch to th eintermediate clock is done like:
>
>         rate = clk_get_rate(intermediate_clk);
>         clk_set_rate(cpu_clk, rate);
>
> Now the clk framework does the switch not because it's told to switch
> to another parent, but only because the other parent happens to be the
> only provider for that rate. That's rubbish, when the parent must be
> changed, then it should be done explicitly.
> What if the CPU PLL and the intermediate clk happen to have the same
> rate? Then the clk_set_rate above simply does nothing, no parent is
> changed and the following rate change of the CPU PLL just crashes the
> system.

The problem is that the code is common across platforms that need to
reparent or just change rate for intermediate clocks. And the best we
can do is clk_set_rate() and so probably the clk driver need to take care
of this somehow and make sure we don't result in a crash like you just
demonstrated.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
pi-cheng.chen March 10, 2015, 1:53 a.m. | #6
On 5 March 2015 at 15:42, Sascha Hauer <s.hauer@pengutronix.de> wrote:
>
> +Cc Viresh Kumar
>
> Viresh, this is the patch for the underlying clocks for the Mediatek
> cpufreq driver.
>
> On Thu, Mar 05, 2015 at 10:43:21AM +0800, Pi-Cheng Chen wrote:
>> Hi Sascha,
>>
>> On 4 March 2015 at 19:21, Sascha Hauer <s.hauer@pengutronix.de> wrote:
>> > On Wed, Mar 04, 2015 at 06:49:11PM +0800, pi-cheng.chen wrote:
>> >> This patch adds CPU mux clocks which are used by Mediatek cpufreq driver
>> >> for intermediate clock source switching. This patch is based on Mediatek
>> >> clock driver patches[1].
>> >>
>> >> [1] http://thread.gmane.org/gmane.linux.kernel/1892436
>> >>
>> >> Signed-off-by: pi-cheng.chen <pi-cheng.chen@linaro.org>
>> >> ---
>> >> +static long clk_cpumux_determine_rate(struct clk_hw *hw, unsigned long rate,
>> >> +                                   unsigned long min_rate,
>> >> +                                   unsigned long max_rate,
>> >> +                                   unsigned long *best_parent_rate,
>> >> +                                   struct clk_hw **best_parent_p)
>> >> +{
>> >> +     struct clk *clk = hw->clk, *parent;
>> >> +     unsigned long parent_rate;
>> >> +     int i;
>> >> +
>> >> +     for (i = MAINPLL_INDEX; i >= ARMPLL_INDEX; i--) {
>> >> +             parent = clk_get_parent_by_index(clk, i);
>> >> +             if (!parent)
>> >> +                     return 0;
>> >> +
>> >> +             if (i == MAINPLL_INDEX) {
>> >> +                     parent_rate = __clk_get_rate(parent);
>> >> +                     if (parent_rate == rate)
>> >> +                             break;
>> >> +             }
>> >> +
>> >> +             parent_rate = __clk_round_rate(parent, rate);
>> >> +     }
>> >> +
>> >> +     *best_parent_rate = parent_rate;
>> >> +     *best_parent_p = __clk_get_hw(parent);
>> >> +     return parent_rate;
>> >> +}
>> >
>> > Why this determine_rate hook? If you want to switch the clock to some
>> > intermediate parent I would assume you do this explicitly by setting the
>> > parent and not implicitly by setting a rate.
>> >
>>
>> I use determine_rate hook here because I am using generic cpufreq-dt
>> driver and I want to make clock switching transparent to cpufreq-dt.
>> I.e. when I am trying to switch the clock from ARMPLL to MAINPLL, I
>> call clk_set_rate() with the rate of MAINPLL, and determine_rate will
>> select MAINPLL as the new parent.
>
> We have clk_set_parent for changing the parent and clk_set_rate to
> change the rate. Use the former for changing the parent and the latter
> for changing the rate. What you are interested in is changing the
> parent, so use clk_set_parent for this and not abuse a side effect
> of clk_set_rate.
>
> My suggestion is to take another approach. Implement clk_set_rate for
> these muxes and in the set_rate hook:
>
> - switch mux to intermediate PLL parent
> - call clk_set_rate() for the real parent PLL
> - switch mux back to real parent PLL

Hi Sascha,

Thanks for your suggestion. I've tried to take this approach, but there's some
issues here.

Calling clk_set_rate() inside the set_rate callback of cpumux will cause
an infinite recursive calling in the clock framework:
mux.set_rate() -> pll.set_rate() -> mux.set_rate -> ...

I've also tries to update pll register settings in the set_rate()
callback of cpumux,
but the PLL clock information will not be correctly updated in this case.

How do you think to create a new "CPU PLL" type of clock and do underlying
mux switching in the set_rate callback of "CPU PLL"?

Best Regards,
Pi-Cheng

>
> This way the things happening behind the scenes are completely transparent
> to the cpufreq driver and you can use cpufreq-dt as is without changes.
>
> Sascha
>
>
> --
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Mike Turquette March 10, 2015, 11:59 p.m. | #7
Quoting Viresh Kumar (2015-03-05 00:59:50)
> On 5 March 2015 at 13:12, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > We have clk_set_parent for changing the parent and clk_set_rate to
> > change the rate. Use the former for changing the parent and the latter
> > for changing the rate. What you are interested in is changing the
> > parent, so use clk_set_parent for this and not abuse a side effect
> > of clk_set_rate.
> 
> clk_set_rate() for CPUs clock is responsible to change clock rate
> of the CPU. Whether it plays with PLLs or muxes, its not that relevant.

Agreed.

> 
> > My suggestion is to take another approach. Implement clk_set_rate for
> > these muxes and in the set_rate hook:
> >
> > - switch mux to intermediate PLL parent
> > - call clk_set_rate() for the real parent PLL
> > - switch mux back to real parent PLL
> >
> > This way the things happening behind the scenes are completely transparent
> > to the cpufreq driver and you can use cpufreq-dt as is without changes.
> 
> CPUFreq wants to change to intermediate frequency by itself against
> some magic change behind the scene. The major requirement for that
> comes from the fact that we want to send PRE/POST freq notifiers on
> which loops-per-jiffie depends.

I assume you are saying that you want to update loops-per-jiffie while
at an intermediate frequency. Why? This operation should not take very
long.

Imagine a (hypothetical?) processor that changes frequency in many small
steps until it converges to the target rate. Would you want to update
lpj for every step?

Regards,
Mike
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
pi-cheng.chen March 11, 2015, 7 a.m. | #8
On Tue, Mar 10, 2015 at 3:55 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Tue, Mar 10, 2015 at 09:53:19AM +0800, Pi-Cheng Chen wrote:
>> On 5 March 2015 at 15:42, Sascha Hauer <s.hauer@pengutronix.de> wrote:
>> >
>> > My suggestion is to take another approach. Implement clk_set_rate for
>> > these muxes and in the set_rate hook:
>> >
>> > - switch mux to intermediate PLL parent
>> > - call clk_set_rate() for the real parent PLL
>> > - switch mux back to real parent PLL
>>
>> Hi Sascha,
>>
>> Thanks for your suggestion. I've tried to take this approach, but there's some
>> issues here.
>>
>> Calling clk_set_rate() inside the set_rate callback of cpumux will cause
>> an infinite recursive calling in the clock framework:
>> mux.set_rate() -> pll.set_rate() -> mux.set_rate -> ...
>
> I don't understand why setting the PLL rate should call into the mux
> set_rate. Are you sure you call clk_set_rate for the mux parent clk?
> I think the general approach should work, drivers/clk/sirf/clk-common.c
> does something similar in cpu_clk_set_rate(). If you like you can send
> me your work in progress state privatly, I'll have a look then.

Thanks for pointing me out the reference. I think I misunderstood the way you
suggested to do it. I'll post the new version once the design including cpufreq
part is finalized.

Best Regards,
Pi-Cheng
>
>>
>> I've also tries to update pll register settings in the set_rate()
>> callback of cpumux,
>> but the PLL clock information will not be correctly updated in this case.
>
> No, that won't work.
>
> Sascha
>
>
> --
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Viresh Kumar March 12, 2015, 9:22 a.m. | #9
On 11 March 2015 at 05:29, Mike Turquette <mturquette@linaro.org> wrote:
> I assume you are saying that you want to update loops-per-jiffie while
> at an intermediate frequency. Why? This operation should not take very
> long.
>
> Imagine a (hypothetical?) processor that changes frequency in many small
> steps until it converges to the target rate. Would you want to update
> lpj for every step?

That's why I have asked Russell specifically about this but haven't got a reply
yet. But it looks wrong to me as well now.. Probably we don't need to care
about it at all..
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/clk/mediatek/Makefile b/drivers/clk/mediatek/Makefile
index 8e4b2a4..299917a 100644
--- a/drivers/clk/mediatek/Makefile
+++ b/drivers/clk/mediatek/Makefile
@@ -1,4 +1,4 @@ 
-obj-y += clk-mtk.o clk-pll.o clk-gate.o
+obj-y += clk-mtk.o clk-pll.o clk-gate.o clk-cpumux.o
 obj-$(CONFIG_RESET_CONTROLLER) += reset.o
 obj-y += clk-mt8135.o
 obj-y += clk-mt8173.o
diff --git a/drivers/clk/mediatek/clk-cpumux.c b/drivers/clk/mediatek/clk-cpumux.c
new file mode 100644
index 0000000..2026d56
--- /dev/null
+++ b/drivers/clk/mediatek/clk-cpumux.c
@@ -0,0 +1,119 @@ 
+/*
+ * Copyright (c) 2015 Linaro Ltd.
+ * Author: Pi-Cheng Chen <pi-cheng.chen@linaro.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+#include "clk-cpumux.h"
+
+#define ARMPLL_INDEX	1
+#define MAINPLL_INDEX	2
+
+static inline struct mtk_clk_cpumux *to_clk_mux(struct clk_hw *_hw)
+{
+	return container_of(_hw, struct mtk_clk_cpumux, hw);
+}
+
+static long clk_cpumux_determine_rate(struct clk_hw *hw, unsigned long rate,
+				      unsigned long min_rate,
+				      unsigned long max_rate,
+				      unsigned long *best_parent_rate,
+				      struct clk_hw **best_parent_p)
+{
+	struct clk *clk = hw->clk, *parent;
+	unsigned long parent_rate;
+	int i;
+
+	for (i = MAINPLL_INDEX; i >= ARMPLL_INDEX; i--) {
+		parent = clk_get_parent_by_index(clk, i);
+		if (!parent)
+			return 0;
+
+		if (i == MAINPLL_INDEX) {
+			parent_rate = __clk_get_rate(parent);
+			if (parent_rate == rate)
+				break;
+		}
+
+		parent_rate = __clk_round_rate(parent, rate);
+	}
+
+	*best_parent_rate = parent_rate;
+	*best_parent_p = __clk_get_hw(parent);
+	return parent_rate;
+}
+
+static u8 clk_cpumux_get_parent(struct clk_hw *hw)
+{
+	struct mtk_clk_cpumux *mux = to_clk_mux(hw);
+	int num_parents = __clk_get_num_parents(hw->clk);
+	u32 val;
+
+	regmap_read(mux->regmap, mux->reg, &val);
+	val = (val & mux->mask) >> mux->shift;
+
+	if (val >= num_parents)
+		return -EINVAL;
+
+	return val;
+}
+
+static int clk_cpumux_set_parent(struct clk_hw *hw, u8 index)
+{
+	struct mtk_clk_cpumux *mux = to_clk_mux(hw);
+
+	return regmap_update_bits(mux->regmap, mux->reg, mux->mask,
+				  index << mux->shift);
+}
+
+static struct clk_ops clk_cpumux_ops = {
+	.get_parent = clk_cpumux_get_parent,
+	.set_parent = clk_cpumux_set_parent,
+	.determine_rate = clk_cpumux_determine_rate,
+};
+
+struct clk *mtk_clk_register_cpumux(const char *name, const char **parent_names,
+				    u8 num_parents, struct regmap *regmap,
+				    u32 reg, u8 shift, u8 width)
+{
+	struct mtk_clk_cpumux *mux;
+	struct clk *clk;
+	struct clk_init_data init;
+	u32 mask = (BIT(width) - 1) << shift;
+
+	mux = kzalloc(sizeof(*mux), GFP_KERNEL);
+	if (!mux)
+		return ERR_PTR(-ENOMEM);
+
+	init.name = name;
+	init.ops = &clk_cpumux_ops;
+	init.parent_names = parent_names;
+	init.num_parents = num_parents;
+	init.flags = CLK_SET_RATE_PARENT;
+
+	mux->regmap = regmap;
+	mux->reg = reg;
+	mux->shift = shift;
+	mux->mask = mask;
+	mux->hw.init = &init;
+
+	clk = clk_register(NULL, &mux->hw);
+	if (IS_ERR(clk))
+		kfree(mux);
+
+	return clk;
+}
+
diff --git a/drivers/clk/mediatek/clk-cpumux.h b/drivers/clk/mediatek/clk-cpumux.h
new file mode 100644
index 0000000..e1c8369
--- /dev/null
+++ b/drivers/clk/mediatek/clk-cpumux.h
@@ -0,0 +1,32 @@ 
+/*
+ * Copyright (c) 2015 Linaro Ltd.
+ * Author: Pi-Cheng Chen <pi-cheng.chen@linaro.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __DRV_CLK_CPUMUX_H
+#define __DRV_CLK_CPUMUX_H
+
+#include <linux/regmap.h>
+
+struct mtk_clk_cpumux {
+	struct clk_hw	hw;
+	struct regmap	*regmap;
+	u32		reg;
+	u32		mask;
+	u8		shift;
+};
+
+struct clk *mtk_clk_register_cpumux(const char *name, const char **parent_names,
+				    u8 num_parents, struct regmap *regmap,
+				    u32 reg, u8 shift, u8 width);
+
+#endif /* __DRV_CLK_CPUMUX_H */
diff --git a/drivers/clk/mediatek/clk-mt8173.c b/drivers/clk/mediatek/clk-mt8173.c
index 014e552..124c7da 100644
--- a/drivers/clk/mediatek/clk-mt8173.c
+++ b/drivers/clk/mediatek/clk-mt8173.c
@@ -19,6 +19,7 @@ 
 
 #include "clk-mtk.h"
 #include "clk-gate.h"
+#include "clk-cpumux.h"
 
 #include <dt-bindings/clock/mt8173-clk.h>
 
@@ -517,6 +518,25 @@  static const char *i2s3_b_ck_parents[] __initconst = {
 	"apll2_div5"
 };
 
+static const char *ca53_parents[] __initconst = {
+	"clk26m",
+	"armca7pll",
+	"mainpll",
+	"univpll"
+};
+
+static const char *ca57_parents[] __initconst = {
+	"clk26m",
+	"armca15pll",
+	"mainpll",
+	"univpll"
+};
+
+static struct mtk_composite cpu_muxes[] __initdata = {
+	MUX(INFRA_CA53SEL, "infra_ca53_sel", ca53_parents, 0x0000, 0, 2),
+	MUX(INFRA_CA57SEL, "infra_ca57_sel", ca57_parents, 0x0000, 2, 2),
+};
+
 static struct mtk_composite top_muxes[] __initdata = {
 	/* CLK_CFG_0 */
 	MUX(TOP_AXI_SEL, "axi_sel", axi_parents, 0x0040, 0, 3),
@@ -745,6 +765,9 @@  static void __init mtk_infrasys_init(struct device_node *node)
 	mtk_clk_register_gates(node, infra_clks, ARRAY_SIZE(infra_clks),
 						clk_data);
 
+	mtk_clk_register_cpumuxes(node, cpu_muxes, ARRAY_SIZE(cpu_muxes),
+				  clk_data);
+
 	r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
 	if (r)
 		pr_err("%s(): could not register clock provider: %d\n",
diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
index 2bcade0..5c65cc4 100644
--- a/drivers/clk/mediatek/clk-mtk.c
+++ b/drivers/clk/mediatek/clk-mtk.c
@@ -23,6 +23,7 @@ 
 
 #include "clk-mtk.h"
 #include "clk-gate.h"
+#include "clk-cpumux.h"
 
 struct clk_onecell_data *mtk_alloc_clk_data(unsigned int clk_num)
 {
@@ -193,3 +194,41 @@  err_out:
 
 	return ERR_PTR(ret);
 }
+
+int mtk_clk_register_cpumuxes(struct device_node *node,
+			      struct mtk_composite *clks, int num,
+			      struct clk_onecell_data *clk_data)
+{
+	int i;
+	struct clk *clk;
+	struct regmap *regmap;
+
+	if (!clk_data)
+		return -ENOMEM;
+
+	regmap = syscon_node_to_regmap(node);
+	if (IS_ERR(regmap)) {
+		pr_err("Cannot find regmap for %s: %ld\n", node->full_name,
+		       PTR_ERR(regmap));
+		return PTR_ERR(regmap);
+	}
+
+	for (i = 0; i < num; i++) {
+		struct mtk_composite *mux = &clks[i];
+
+		clk = mtk_clk_register_cpumux(mux->name, mux->parent_names,
+					      mux->num_parents, regmap,
+					      mux->mux_reg, mux->mux_shift,
+					      mux->mux_width);
+
+		if (IS_ERR(clk)) {
+			pr_err("Failed to register clk %s: %ld\n",
+			       mux->name, PTR_ERR(clk));
+			continue;
+		}
+
+		clk_data->clks[mux->id] = clk;
+	}
+
+	return 0;
+}
diff --git a/drivers/clk/mediatek/clk-mtk.h b/drivers/clk/mediatek/clk-mtk.h
index 8f5190c..f3b1840 100644
--- a/drivers/clk/mediatek/clk-mtk.h
+++ b/drivers/clk/mediatek/clk-mtk.h
@@ -126,6 +126,10 @@  struct mtk_gate {
 int mtk_clk_register_gates(struct device_node *node, struct mtk_gate *clks, int num,
 		struct clk_onecell_data *clk_data);
 
+int mtk_clk_register_cpumuxes(struct device_node *node,
+			      struct mtk_composite *clks, int num,
+			      struct clk_onecell_data *clk_data);
+
 struct clk_onecell_data *mtk_alloc_clk_data(unsigned int clk_num);
 
 #define HAVE_RST_BAR	BIT(0)
diff --git a/include/dt-bindings/clock/mt8173-clk.h b/include/dt-bindings/clock/mt8173-clk.h
index e648b28..2503b03 100644
--- a/include/dt-bindings/clock/mt8173-clk.h
+++ b/include/dt-bindings/clock/mt8173-clk.h
@@ -187,7 +187,9 @@ 
 #define INFRA_CEC		9
 #define INFRA_PMICSPI		10
 #define INFRA_PMICWRAP		11
-#define INFRA_NR_CLK		12
+#define INFRA_CA53SEL		12
+#define INFRA_CA57SEL		13
+#define INFRA_NR_CLK		14
 
 /* PERI_SYS */