Re: [PATCH v2 6/7] ARM: Exynos: switch to using generic cpufreq driver for Exynos4x12

Krzysztof Kozlowski
2015-07-10T08:55:10Z

On 10.07.2015 00:43, Bartlomiej Zolnierkiewicz wrote:
> The new CPU clock type allows the use of generic CPUfreq driver.
> Switch Exynos4x12 to using generic cpufreq driver.
>
> This patch also takes care of making ARM_EXYNOS_CPU_FREQ_BOOST_SW
> config option depend on cpufreq-dt driver instead of exynos-cpufreq
> one and fixes the minor issue present with the old code (support
> for 'boost' mode in the exynos-cpufreq driver was enabled for all
> supported SoCs even though 'boost' frequency was provided only for
> Exynos4x12 ones).
>
> Cc: Tomasz Figa <tomasz.figa@gmail.com>
> Cc: Kukjin Kim <kgene.kim@samsung.com>
> Cc: Javier Martinez Canillas <javier@dowhile0.org>
> Cc: Thomas Abraham <thomas.ab@samsung.com>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---
> arch/arm/mach-exynos/exynos.c | 14 ++++++++++++++
> drivers/cpufreq/Kconfig.arm | 2 +-
> drivers/cpufreq/exynos-cpufreq.c | 3 ---
> 3 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
> index 4015ec3..0c74e90 100644
> --- a/arch/arm/mach-exynos/exynos.c
> +++ b/arch/arm/mach-exynos/exynos.c
> @@ -20,6 +20,7 @@
> #include <linux/platform_device.h>
> #include <linux/pm_domain.h>
> #include <linux/irqchip.h>
> +#include <linux/cpufreq-dt.h>
>
> #include <asm/cacheflush.h>
> #include <asm/hardware/cache-l2x0.h>
> @@ -224,6 +225,12 @@ static void __init exynos_init_irq(void)
> exynos_map_pmu();
> }
>
> +struct cpufreq_dt_platform_data cpufreq_dt_pd = {

Shouldn't this be static?

> +#ifdef CONFIG_ARM_EXYNOS_CPU_FREQ_BOOST_SW
> + .boost_supported = true,
> +#endif
> +};
> +
> static const struct of_device_id exynos_cpufreq_matches[] = {
> { .compatible = "samsung,exynos4210", .data = "cpufreq-dt" },
> { .compatible = "samsung,exynos5250", .data = "cpufreq-dt" },
> @@ -235,6 +242,13 @@ static void __init exynos_cpufreq_init(void)
> struct device_node *root = of_find_node_by_path("/");
> const struct of_device_id *match;
>
> + if (of_machine_is_compatible("samsung,exynos4212") ||
> + of_machine_is_compatible("samsung,exynos4412")) {
> + platform_device_register_data(NULL, "cpufreq-dt", -1,
> + &cpufreq_dt_pd, sizeof(cpufreq_dt_pd));
> + return;
> + }
> +
> match = of_match_node(exynos_cpufreq_matches, root);
> if (!match) {
> platform_device_register_simple("exynos-cpufreq", -1, NULL, 0);

Now we have two places with compatibles for matching cpufreq-dt driver.
It is confusing especially when grepping for compatibles.

Instead why not use 'data' from exynos_cpufreq_matches? Currently it
stores the "cpufreq-dt" string which looks useless.

Best regards,
Krzysztof

> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> index 9ab6388c1..5f5a15a 100644
> --- a/drivers/cpufreq/Kconfig.arm
> +++ b/drivers/cpufreq/Kconfig.arm
> @@ -49,7 +49,7 @@ config ARM_EXYNOS4X12_CPUFREQ
>
> config ARM_EXYNOS_CPU_FREQ_BOOST_SW
> bool "EXYNOS Frequency Overclocking - Software"
> - depends on ARM_EXYNOS_CPUFREQ && THERMAL
> + depends on CPUFREQ_DT && THERMAL
> select CPU_FREQ_BOOST_SW
> select EXYNOS_THERMAL
> help
> diff --git a/drivers/cpufreq/exynos-cpufreq.c b/drivers/cpufreq/exynos-cpufreq.c
> index 71d8891..89d252e 100644
> --- a/drivers/cpufreq/exynos-cpufreq.c
> +++ b/drivers/cpufreq/exynos-cpufreq.c
> @@ -150,9 +150,6 @@ static struct cpufreq_driver exynos_driver = {
> .init = exynos_cpufreq_cpu_init,
> .name = "exynos_cpufreq",
> .attr = cpufreq_generic_attr,
> -#ifdef CONFIG_ARM_EXYNOS_CPU_FREQ_BOOST_SW
> - .boost_supported = true,
> -#endif
> #ifdef CONFIG_PM
> .suspend = cpufreq_generic_suspend,
> #endif
>