linux-kernel - Re: [PATCH v2 14/16] cpufreq: Add cpufreq driver for Tegra124

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <53CFA33F.6030307@nvidia.com>
Date:	Wed, 23 Jul 2014 14:57:51 +0300
From:	Tuomas Tynkkynen <ttynkkynen@...dia.com>
To:	Viresh Kumar <viresh.kumar@...aro.org>
CC:	"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
	Stephen Warren <swarren@...dotorg.org>,
	Thierry Reding <thierry.reding@...il.com>,
	Peter De Schrijver <pdeschrijver@...dia.com>,
	Prashant Gaikwad <pgaikwad@...dia.com>,
	Mike Turquette <mturquette@...aro.org>,
	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	Paul Walmsley <pwalmsley@...dia.com>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: Re: [PATCH v2 14/16] cpufreq: Add cpufreq driver for Tegra124



On 23/07/14 07:44, Viresh Kumar wrote:
> On 21 July 2014 21:09, Tuomas Tynkkynen <ttynkkynen@...dia.com> wrote:
> 
>> 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.
> 
>>         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.

> 
>>  obj-$(CONFIG_ARM_VEXPRESS_SPC_CPUFREQ) += vexpress-spc-cpufreq.o
>>
>>  ##################################################################################
>> diff --git a/drivers/cpufreq/tegra124-cpufreq.c b/drivers/cpufreq/tegra124-cpufreq.c
> 
>> +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.

[...]
>> +
>> +       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.

>> +
>> +       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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/