| Message ID | 1380619803-2760-1-git-send-email-Sudeep.KarkadaNagesha@arm.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On 1 October 2013 15:00, Sudeep KarkadaNagesha <Sudeep.KarkadaNagesha@arm.com> wrote: > From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com> > > Currently clk_get_sys is used with cpu-cluster.<n> as the device id > which is incorrect. It should be connection/consumer ID instead. > > It is possible to specify input clock in the cpu device node along > with the optional clock-name. clk_get_sys can't handle that. > > This patch replaces clk_get_sys with clk_get to extend support for > clocks specified in the device tree cpu node. > > Cc: Viresh Kumar <viresh.kumar@linaro.org> > Cc: "Rafael J. Wysocki" <rjw@sisk.pl> > Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com> > --- > drivers/cpufreq/arm_big_little.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/arm_big_little.c > index 3549f07..501a091 100644 > --- a/drivers/cpufreq/arm_big_little.c > +++ b/drivers/cpufreq/arm_big_little.c > @@ -127,7 +127,7 @@ static int get_cluster_clk_and_freq_table(struct device *cpu_dev) > } > > name[12] = cluster + '0'; > - clk[cluster] = clk_get_sys(name, NULL); > + clk[cluster] = clk_get(cpu_dev, name); This is not really CPUs clock and so passing cpu_dev would be wrong here. So, either this change should be clk_get(NULL, name); Or change driver to use CPU clocks instead of cluster ones.. -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/10/13 05:52, Viresh Kumar wrote: > On 1 October 2013 15:00, Sudeep KarkadaNagesha > <Sudeep.KarkadaNagesha@arm.com> wrote: >> From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com> >> >> Currently clk_get_sys is used with cpu-cluster.<n> as the device id >> which is incorrect. It should be connection/consumer ID instead. >> >> It is possible to specify input clock in the cpu device node along >> with the optional clock-name. clk_get_sys can't handle that. >> >> This patch replaces clk_get_sys with clk_get to extend support for >> clocks specified in the device tree cpu node. >> >> Cc: Viresh Kumar <viresh.kumar@linaro.org> >> Cc: "Rafael J. Wysocki" <rjw@sisk.pl> >> Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com> >> --- >> drivers/cpufreq/arm_big_little.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/arm_big_little.c >> index 3549f07..501a091 100644 >> --- a/drivers/cpufreq/arm_big_little.c >> +++ b/drivers/cpufreq/arm_big_little.c >> @@ -127,7 +127,7 @@ static int get_cluster_clk_and_freq_table(struct device *cpu_dev) >> } >> >> name[12] = cluster + '0'; >> - clk[cluster] = clk_get_sys(name, NULL); >> + clk[cluster] = clk_get(cpu_dev, name); > > This is not really CPUs clock and so passing cpu_dev would be wrong here. > So, either this change should be clk_get(NULL, name); Agreed, but there's no cluster node in DT. So platforms using DT would have these clocks in cpu@n node as it's used for CPU frequency scaling. One example I can see is highbank(it uses cpufreq-cpu0 meaning its single cluster clock). So AFAICT if any big-little systems wants to pass cluster clocks through DT, it has to be cpu@n nodes. Let me know if you have any alternate thoughts ? Regards, Sudeep -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 3 October 2013 14:27, Sudeep KarkadaNagesha <Sudeep.KarkadaNagesha@arm.com> wrote: > Agreed, but there's no cluster node in DT. So platforms using DT would have > these clocks in cpu@n node as it's used for CPU frequency scaling. One example I > can see is highbank(it uses cpufreq-cpu0 meaning its single cluster clock). So > AFAICT if any big-little systems wants to pass cluster clocks through DT, it has > to be cpu@n nodes. Let me know if you have any alternate thoughts ? I understand your point, but shouldn't we name it then: cpu clock? Probably cluster clock was wrong from the beginning.. As there is no cluster clock, or there is ? :) -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/10/13 10:00, Viresh Kumar wrote: > On 3 October 2013 14:27, Sudeep KarkadaNagesha > <Sudeep.KarkadaNagesha@arm.com> wrote: >> Agreed, but there's no cluster node in DT. So platforms using DT would have >> these clocks in cpu@n node as it's used for CPU frequency scaling. One example I >> can see is highbank(it uses cpufreq-cpu0 meaning its single cluster clock). So >> AFAICT if any big-little systems wants to pass cluster clocks through DT, it has >> to be cpu@n nodes. Let me know if you have any alternate thoughts ? > > I understand your point, but shouldn't we name it then: cpu clock? > Probably cluster clock was wrong from the beginning.. As there is no cluster clock, > or there is ? :) > Ok I would phrase it as, it's a clock shared by all the cpus in the cluster. So like any other shared clocks in the system, each device sharing it would have it as input clock but it would be single clock output from the clock/power controller. Does it make any sense ? Regards, Sudeep -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 3 October 2013 14:53, Sudeep KarkadaNagesha <Sudeep.KarkadaNagesha@arm.com> wrote: > Ok I would phrase it as, it's a clock shared by all the cpus in the cluster. > So like any other shared clocks in the system, each device sharing it would have > it as input clock but it would be single clock output from the clock/power > controller. Does it make any sense ? Sure.. I already know that :) .. what I am not able to decide is, if we should talk about CPU clocks in our driver or cluster clocks.. -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/10/13 10:38, Viresh Kumar wrote: > On 3 October 2013 14:53, Sudeep KarkadaNagesha > <Sudeep.KarkadaNagesha@arm.com> wrote: >> Ok I would phrase it as, it's a clock shared by all the cpus in the cluster. >> So like any other shared clocks in the system, each device sharing it would have >> it as input clock but it would be single clock output from the clock/power >> controller. Does it make any sense ? > > Sure.. I already know that :) .. what I am not able to decide is, if we should > talk about CPU clocks in our driver or cluster clocks.. > I understand. Having cpu-cluster.<n> helps platforms not specifying cpu clocks in DT or not registering clocks using cpu_dev. If you enforce clock lookups with dev_id(cpu<n>) then I believe you can remove 'cpu-cluster.<n>', that's your call :) Regards, Sudeep -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 1 October 2013 15:00, Sudeep KarkadaNagesha <Sudeep.KarkadaNagesha@arm.com> wrote: > From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com> > > Currently clk_get_sys is used with cpu-cluster.<n> as the device id > which is incorrect. It should be connection/consumer ID instead. > > It is possible to specify input clock in the cpu device node along > with the optional clock-name. clk_get_sys can't handle that. > > This patch replaces clk_get_sys with clk_get to extend support for > clocks specified in the device tree cpu node. > > Cc: Viresh Kumar <viresh.kumar@linaro.org> > Cc: "Rafael J. Wysocki" <rjw@sisk.pl> > Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com> > --- > drivers/cpufreq/arm_big_little.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Acked-by: Viresh Kumar <viresh.kumar@linaro.org> -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/arm_big_little.c index 3549f07..501a091 100644 --- a/drivers/cpufreq/arm_big_little.c +++ b/drivers/cpufreq/arm_big_little.c @@ -127,7 +127,7 @@ static int get_cluster_clk_and_freq_table(struct device *cpu_dev) } name[12] = cluster + '0'; - clk[cluster] = clk_get_sys(name, NULL); + clk[cluster] = clk_get(cpu_dev, name); if (!IS_ERR(clk[cluster])) { dev_dbg(cpu_dev, "%s: clk: %p & freq table: %p, cluster: %d\n", __func__, clk[cluster], freq_table[cluster],