| Message ID | 1413805281-6269-5-git-send-email-thomas.ab@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Thomas, On Mon, Oct 20, 2014 at 5:11 PM, Thomas Abraham <thomas.ab@samsung.com> wrote: > The new CPU clock type allows the use of generic CPUfreq drivers. So for > Exynos4210/5250, switch to using generic cpufreq driver. For Exynos5420, > which did not have CPUfreq driver support, enable the use of generic > CPUfreq driver. > > Suggested-by: Tomasz Figa <tomasz.figa@gmail.com> > Cc: Kukjin Kim <kgene.kim@samsung.com> > Signed-off-by: Thomas Abraham <thomas.ab@samsung.com> > Reviewed-by: Tomasz Figa <tomasz.figa@gmail.com> > Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk> > Tested-by: Chander Kashyap <k.chander@samsung.com> > --- > arch/arm/mach-exynos/exynos.c | 24 +++++++++++++++++++++++- > 1 files changed, 23 insertions(+), 1 deletions(-) > > diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c > index 6b283eb..a1be294 100644 > --- a/arch/arm/mach-exynos/exynos.c > +++ b/arch/arm/mach-exynos/exynos.c > @@ -282,6 +282,28 @@ static void __init exynos_init_irq(void) > exynos_map_pmu(); > } > > +static const struct of_device_id exynos_cpufreq_matches[] = { > + { .compatible = "samsung,exynos5420", .data = "arm-bL-cpufreq-dt" }, While you're at it, can you add this to so we don't have to patch kernels for the Chromebook2 and Odroid-XU3? { .compatible = "samsung,exynos5422", .data = "arm-bL-cpufreq-dt" }, { .compatible = "samsung,exynos5800", .data = "arm-bL-cpufreq-dt" }, > + { .compatible = "samsung,exynos5250", .data = "cpufreq-dt" }, > + { .compatible = "samsung,exynos4210", .data = "cpufreq-dt" }, > + { .compatible = "samsung,exynos5440", .data = "exynos5440-cpufreq" }, > + { /* sentinel */ }
Hi Thomas, On Mon, Oct 20, 2014 at 4:41 AM, Thomas Abraham <thomas.ab@samsung.com> wrote: > The new CPU clock type allows the use of generic CPUfreq drivers. So for > Exynos4210/5250, switch to using generic cpufreq driver. For Exynos5420, > which did not have CPUfreq driver support, enable the use of generic > CPUfreq driver. > > Suggested-by: Tomasz Figa <tomasz.figa@gmail.com> > Cc: Kukjin Kim <kgene.kim@samsung.com> > Signed-off-by: Thomas Abraham <thomas.ab@samsung.com> > Reviewed-by: Tomasz Figa <tomasz.figa@gmail.com> > Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk> > Tested-by: Chander Kashyap <k.chander@samsung.com> What's the status of the exynos5 CPUfreq support for upstream? This was pretty broadly reviewed and tested, but I still don't see this either in linux-next or Kukjin's for-next. Kevin
On 20/10/14 12:41, Thomas Abraham wrote: > The new CPU clock type allows the use of generic CPUfreq drivers. So for > Exynos4210/5250, switch to using generic cpufreq driver. For Exynos5420, > which did not have CPUfreq driver support, enable the use of generic > CPUfreq driver. > > Suggested-by: Tomasz Figa <tomasz.figa@gmail.com> > Cc: Kukjin Kim <kgene.kim@samsung.com> > Signed-off-by: Thomas Abraham <thomas.ab@samsung.com> > Reviewed-by: Tomasz Figa <tomasz.figa@gmail.com> > Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk> > Tested-by: Chander Kashyap <k.chander@samsung.com> > --- > arch/arm/mach-exynos/exynos.c | 24 +++++++++++++++++++++++- > 1 files changed, 23 insertions(+), 1 deletions(-) > > diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c > index 6b283eb..a1be294 100644 > --- a/arch/arm/mach-exynos/exynos.c > +++ b/arch/arm/mach-exynos/exynos.c > @@ -282,6 +282,28 @@ static void __init exynos_init_irq(void) > exynos_map_pmu(); > } > > +static const struct of_device_id exynos_cpufreq_matches[] = { > + { .compatible = "samsung,exynos5420", .data = "arm-bL-cpufreq-dt" }, Sorry for raising this issue always with Exynos cpufreq drivers. IMO the bindings for "arm-bL-cpufreq-dt" is broken. Currently no one is using it and it's better to fix it before we have a real user of it. If you look at the binding document for it[1], it has a fixme which shouldn't have been there at first place. It assumes the ordering of CPU's specified in the DT and the logical index allocation to them. It even breaks for hotplug especially if you hotplug-in back in different order. We can work around that probably, but it's better to fix the binding. I failed to grab much attention in my previous attempts to address this[2]. Viresh also started a discussion more recently[3]. Regards, Sudeep [1] Documentation/devicetree/bindings/cpufreq/arm_big_little_dt.txt [2] http://www.spinics.net/lists/arm-kernel/msg303977.html [3] https://lkml.org/lkml/2014/6/25/152
Oh, you are still alive? I thought you were about to get married :) Just kidding !! On 20 November 2014 00:58, Sudeep Holla <sudeep.holla@arm.com> wrote: > Sorry for raising this issue always with Exynos cpufreq drivers. IMO the > bindings for "arm-bL-cpufreq-dt" is broken. Currently no one is using it > and it's better to fix it before we have a real user of it. Hmm, yeah if we can. I haven't found a easy way to go ahead and then got caught in other activities. > If you look at the binding document for it[1], it has a fixme which > shouldn't have been there at first place. It assumes the ordering of > CPU's specified in the DT and the logical index allocation to them. Ok, I believe the FIXME is a bit outdated. From the code I can see only this limitation. - For every cluster, the cpu which boots up first should carry the OPPs. Otherwise there is no restriction on ordering of CPUs. - I believe CPUs boot in the order they are present in DT except for the boot CPU. So, the first node for every cluster should have it. Correct ? Then we can update the fixme. > It even breaks for hotplug especially if you hotplug-in back in > different order. Hmm, I never thought about it. But yes the CPU with the OPPs should be the first one to come back. > We can work around that probably, but it's better to > fix the binding. I failed to grab much attention in my previous attempts > to address this[2]. Viresh also started a discussion more recently[3]. They are just stuck and went nowhere :(
Hi Kevin, On Thu, Nov 20, 2014 at 2:30 AM, Kevin Hilman <khilman@kernel.org> wrote: > Hi Thomas, > > On Mon, Oct 20, 2014 at 4:41 AM, Thomas Abraham <thomas.ab@samsung.com> wrote: >> The new CPU clock type allows the use of generic CPUfreq drivers. So for >> Exynos4210/5250, switch to using generic cpufreq driver. For Exynos5420, >> which did not have CPUfreq driver support, enable the use of generic >> CPUfreq driver. >> >> Suggested-by: Tomasz Figa <tomasz.figa@gmail.com> >> Cc: Kukjin Kim <kgene.kim@samsung.com> >> Signed-off-by: Thomas Abraham <thomas.ab@samsung.com> >> Reviewed-by: Tomasz Figa <tomasz.figa@gmail.com> >> Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk> >> Tested-by: Chander Kashyap <k.chander@samsung.com> > > What's the status of the exynos5 CPUfreq support for upstream? This > was pretty broadly reviewed and tested, but I still don't see this > either in linux-next or Kukjin's for-next. The rebased version of the patches have been posted [1]. My apologies for the delay in following up on this patch series. Thanks, Thomas. [1] http://www.spinics.net/lists/arm-kernel/msg380011.html > > Kevin > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Amit, On Wed, Nov 12, 2014 at 3:03 PM, Amit Kucheria <amit.kucheria@linaro.org> wrote: > Hi Thomas, > > On Mon, Oct 20, 2014 at 5:11 PM, Thomas Abraham <thomas.ab@samsung.com> wrote: >> The new CPU clock type allows the use of generic CPUfreq drivers. So for >> Exynos4210/5250, switch to using generic cpufreq driver. For Exynos5420, >> which did not have CPUfreq driver support, enable the use of generic >> CPUfreq driver. >> >> Suggested-by: Tomasz Figa <tomasz.figa@gmail.com> >> Cc: Kukjin Kim <kgene.kim@samsung.com> >> Signed-off-by: Thomas Abraham <thomas.ab@samsung.com> >> Reviewed-by: Tomasz Figa <tomasz.figa@gmail.com> >> Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk> >> Tested-by: Chander Kashyap <k.chander@samsung.com> >> --- >> arch/arm/mach-exynos/exynos.c | 24 +++++++++++++++++++++++- >> 1 files changed, 23 insertions(+), 1 deletions(-) >> >> diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c >> index 6b283eb..a1be294 100644 >> --- a/arch/arm/mach-exynos/exynos.c >> +++ b/arch/arm/mach-exynos/exynos.c >> @@ -282,6 +282,28 @@ static void __init exynos_init_irq(void) >> exynos_map_pmu(); >> } >> >> +static const struct of_device_id exynos_cpufreq_matches[] = { >> + { .compatible = "samsung,exynos5420", .data = "arm-bL-cpufreq-dt" }, > > While you're at it, can you add this to so we don't have to patch > kernels for the Chromebook2 and Odroid-XU3? > > { .compatible = "samsung,exynos5422", .data = "arm-bL-cpufreq-dt" }, > { .compatible = "samsung,exynos5800", .data = "arm-bL-cpufreq-dt" }, > Okay, I this has been included in the updated patches [1] [1] http://www.spinics.net/lists/arm-kernel/msg380011.html Thanks, Thomas. > >> + { .compatible = "samsung,exynos5250", .data = "cpufreq-dt" }, >> + { .compatible = "samsung,exynos4210", .data = "cpufreq-dt" }, >> + { .compatible = "samsung,exynos5440", .data = "exynos5440-cpufreq" }, >> + { /* sentinel */ } > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 20/11/14 03:48, Viresh Kumar wrote: > Oh, you are still alive? I thought you were about to get married :) > Just kidding !! > > On 20 November 2014 00:58, Sudeep Holla <sudeep.holla@arm.com> wrote: >> Sorry for raising this issue always with Exynos cpufreq drivers. IMO the >> bindings for "arm-bL-cpufreq-dt" is broken. Currently no one is using it >> and it's better to fix it before we have a real user of it. > > Hmm, yeah if we can. I haven't found a easy way to go ahead and then > got caught in other activities. > Agreed, it's not easy but needs to be fixed :) >> If you look at the binding document for it[1], it has a fixme which >> shouldn't have been there at first place. It assumes the ordering of >> CPU's specified in the DT and the logical index allocation to them. > > Ok, I believe the FIXME is a bit outdated. From the code I can see only > this limitation. > > - For every cluster, the cpu which boots up first should carry the OPPs. > Otherwise there is no restriction on ordering of CPUs. > Not entirely true, it assuming the fact that the logical index provided by the OS is completely based on the ordering in the DT. > - I believe CPUs boot in the order they are present in DT except for the > boot CPU. So, the first node for every cluster should have it. > > Correct ? Then we can update the fixme. > No, I disagree as you trying to implicitly depend on the logic Linux uses to assign logical index. It can be changed any time, and depending on it might break things in future which can't be fixed easily later especially if it's DT related. >> It even breaks for hotplug especially if you hotplug-in back in >> different order. > > Hmm, I never thought about it. But yes the CPU with the OPPs should > be the first one to come back. > How can you assume that ? I can write a simple test which hot-plugs out all the CPUs in the (esp. multi-cluster) system in ascending order of logical index and hot-plug back in descending order. Then the above logic fails. >> We can work around that probably, but it's better to >> fix the binding. I failed to grab much attention in my previous attempts >> to address this[2]. Viresh also started a discussion more recently[3]. > > They are just stuck and went nowhere :( > The platforms needing it have to get involved in such discussions to make any progress. Regards, Sudeep
On 24 November 2014 at 19:14, Sudeep Holla <sudeep.holla@arm.com> wrote: > On 20/11/14 03:48, Viresh Kumar wrote: >> - I believe CPUs boot in the order they are present in DT except for the >> boot CPU. So, the first node for every cluster should have it. >> >> Correct ? Then we can update the fixme. >> > > No, I disagree as you trying to implicitly depend on the logic Linux uses to > assign logical index. It can be changed any time, and depending > on it might break things in future which can't be fixed easily later > especially if it's DT related. Yeah, correct. >> Hmm, I never thought about it. But yes the CPU with the OPPs should >> be the first one to come back. >> > > How can you assume that ? I can write a simple test which hot-plugs out > all the CPUs in the (esp. multi-cluster) system in ascending order of > logical index and hot-plug back in descending order. Then the above > logic fails. What I said is: Yes the CPU with the OPPs should be the first one to come back, if this has to work correctly. Otherwise it will break.. And you repeated the same :) > The platforms needing it have to get involved in such discussions to > make any progress. Yes there were people involved. I hope I cc'd you as well...
diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c index 6b283eb..a1be294 100644 --- a/arch/arm/mach-exynos/exynos.c +++ b/arch/arm/mach-exynos/exynos.c @@ -282,6 +282,28 @@ static void __init exynos_init_irq(void) exynos_map_pmu(); } +static const struct of_device_id exynos_cpufreq_matches[] = { + { .compatible = "samsung,exynos5420", .data = "arm-bL-cpufreq-dt" }, + { .compatible = "samsung,exynos5250", .data = "cpufreq-dt" }, + { .compatible = "samsung,exynos4210", .data = "cpufreq-dt" }, + { .compatible = "samsung,exynos5440", .data = "exynos5440-cpufreq" }, + { /* sentinel */ } +}; + +static void __init exynos_cpufreq_init(void) +{ + struct device_node *root = of_find_node_by_path("/"); + const struct of_device_id *match; + + match = of_match_node(exynos_cpufreq_matches, root); + if (!match) { + platform_device_register_simple("exynos-cpufreq", -1, NULL, 0); + return; + } + + platform_device_register_simple(match->data, -1, NULL, 0); +} + static void __init exynos_dt_machine_init(void) { struct device_node *i2c_np; @@ -321,7 +343,7 @@ static void __init exynos_dt_machine_init(void) of_machine_is_compatible("samsung,exynos5250")) platform_device_register(&exynos_cpuidle); - platform_device_register_simple("exynos-cpufreq", -1, NULL, 0); + exynos_cpufreq_init(); of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL); }