On 23/07/14 07:44, Viresh Kumar wrote:

On 21 July 2014 21:09, Tuomas Tynkkynen [off-list ref] wrote:

quoted

diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index 7364a53..df3c73e 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm

@@ -244,6 +244,7 @@ config ARM_SPEAR_CPUFREQ
 config ARM_TEGRA_CPUFREQ
        bool "TEGRA CPUFreq support"
        depends on ARCH_TEGRA
+       depends on GENERIC_CPUFREQ_CPU0

Wouldn't this also disturb the existing cpufreq driver for earlier
tegra platforms? i.e. we don't need cpufreq-cpu0 for them
atleast as of now.

quoted

        default y
        help
          This adds the CPUFreq driver support for TEGRA SOCs.

diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index db6d9a2..3437d24 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile

@@ -76,6 +76,7 @@ obj-$(CONFIG_ARM_SA1100_CPUFREQ)      += sa1100-cpufreq.o
 obj-$(CONFIG_ARM_SA1110_CPUFREQ)       += sa1110-cpufreq.o
 obj-$(CONFIG_ARM_SPEAR_CPUFREQ)                += spear-cpufreq.o
 obj-$(CONFIG_ARM_TEGRA_CPUFREQ)                += tegra-cpufreq.o
+obj-$(CONFIG_ARM_TEGRA_CPUFREQ)                += tegra124-cpufreq.o

Maybe, you can update the same line if you want.

Oh, right.

quoted

 obj-$(CONFIG_ARM_VEXPRESS_SPC_CPUFREQ) += vexpress-spc-cpufreq.o

 ##################################################################################

diff --git a/drivers/cpufreq/tegra124-cpufreq.c b/drivers/cpufreq/tegra124-cpufreq.c

quoted

+static struct cpufreq_frequency_table *freq_table;
+
+static struct device *cpu_dev;
+static struct clk *cpu_clk;
+static struct clk *pllp_clk;
+static struct clk *pllx_clk;
+static struct clk *dfll_clk;

The routines in this file are going to be called just once at boot, right?
In that case we are actually wasting some memory by creating globals.
Probably just move all these in a struct and allocate it at runtime.

Sure.

[...]

quoted

+
+       ret = dev_pm_opp_init_cpufreq_table(cpu_dev, &freq_table);
+       if (ret)
+               goto out_put_pllp_clk;

Why do you need this? cpufreq-cpu0 also does it and this freq_table is
just not getting used at all then.

Oops, yes I forgot to remove that part when making the conversion.

quoted

+
+       pdev = platform_device_register_simple("cpufreq-tegra124", -1, NULL, 0);
+       if (IS_ERR(pdev)) {
+               platform_driver_unregister(&tegra124_cpufreq_platdrv);
+               return PTR_ERR(pdev);
+       }

Why create another unnecessary platform-device/driver here? If
of_find_matching_node() passes and you really need to probe cpufreq-cpu0
here, then just remove the other two calls and move probe's implementation
here only.

The platform device is required for the deferred probe that can happen if the
DFLL driver hasn't initialized yet, and module_init() callbacks don't seem to
respect -EPROBE_DEFER.

[...]

-- 
nvpublic