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 for Android: free password hash cracker in your pocket

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

Message-ID: <20140723135918.GC14263@ulmo.nvidia.com>
Date:	Wed, 23 Jul 2014 15:59:20 +0200
From:	Thierry Reding <thierry.reding@...il.com>
To:	Tuomas Tynkkynen <ttynkkynen@...dia.com>
Cc:	"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <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>,
	Peter De Schrijver <pdeschrijver@...dia.com>,
	Prashant Gaikwad <pgaikwad@...dia.com>,
	Mike Turquette <mturquette@...aro.org>,
	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	Viresh Kumar <viresh.kumar@...aro.org>,
	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 Wed, Jul 23, 2014 at 03:35:46PM +0300, Tuomas Tynkkynen wrote:
> On 23/07/14 10:09, Thierry Reding wrote:
> > On Mon, Jul 21, 2014 at 06:39:00PM +0300, Tuomas Tynkkynen wrote:
[...]
> >> diff --git a/drivers/cpufreq/tegra124-cpufreq.c b/drivers/cpufreq/tegra124-cpufreq.c
[...]
> >> +	cpu_clk = of_clk_get_by_name(cpu_dev->of_node, "cpu_g");
> >> +	if (IS_ERR(cpu_clk))
> >> +		return PTR_ERR(cpu_clk);
[...]
> >> +	pllp_clk = of_clk_get_by_name(cpu_dev->of_node, "pll_p");
> >> +	if (IS_ERR(pllp_clk)) {
> >> +		ret = PTR_ERR(pllp_clk);
> >> +		goto out_put_pllx_clk;
> >> +	}
> > 
> > Can the above not be devm_clk_get(cpu_dev, "...") so that you can remove
> > all the clk_put() calls in the cleanup code below?
> 
> That would allocate the clks under the cpu_dev's devres list, i.e. all the
> clk_puts wouldn't happen when the cpufreq driver goes away, but only when
> cpu_dev itself goes away.

I don't think so. devres_release_all() is called on driver detach as
well.

> > But is there even a reason why we need that? Couldn't we make the
> > driver's .remove() undo what .probe() did so that the driver can be
> > unloaded?
> 
> I guess that could be done, though to fully undo everything the regulator
> voltage would also need to be saved/restored.

That would certainly be my prefered approach. that way the driver can
simply be unloaded, leaving the CPU in the same state as it was after
boot.

> > Otherwise it probably makes more sense not to use a driver (and dummy
> > device) at all as Viresh already mentioned.
> > 
> 
> The dummy platform device is only required for probe deferral, if that
> could be solved in a different way then yes.

I don't think it can. Probe deferral is pretty closely tied to devices
so it's unlikely to ever get implemented for regular initcalls. And in
this case I really think making the driver removable is a good thing.

> >> +};
> >> +
> >> +static const struct of_device_id soc_of_matches[] = {
> >> +	{ .compatible = "nvidia,tegra124", },
> >> +	{}
> >> +};
> >> +
> >> +static int __init tegra_cpufreq_init(void)
> >> +{
> >> +	int ret;
> >> +	struct platform_device *pdev;
> >> +
> >> +	if (!of_find_matching_node(NULL, soc_of_matches))
> >> +		return -ENODEV;
> > 
> > I think this could be of_machine_is_compatible() since there's only a
> > single entry in the match table. If there's a good chance that we may
> > end up with more entries, perhaps now would be a good time to add an
> > of_match_machine() function?
> 
> I think this driver should work on Tegra132 without modifications.
> of_match_machine() does sound useful for some of the other cpufreq
> drivers as well and likely for your soc_is_tegra() from the PMC
> series as well.

Yes, indeed. I'll give it a shot if you don't beat me to it with this
series.

Thierry

Content of type "application/pgp-signature" skipped