linux-kernel - Re: [PATCH v8 00/18] Tegra124 CL-DVFS / DFLL clocksource + cpufreq

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: <20150414192108.6a376ce8@bbrezillon>
Date:	Tue, 14 Apr 2015 19:21:08 +0200
From:	Boris Brezillon <boris.brezillon@...e-electrons.com>
To:	Mikko Perttunen <mikko.perttunen@...si.fi>
Cc:	Michael Turquette <mturquette@...aro.org>,
	Thierry Reding <thierry.reding@...il.com>,
	swarren@...dotorg.org, gnurou@...il.com, pdeschrijver@...dia.com,
	rjw@...ysocki.net, viresh.kumar@...aro.org, pwalmsley@...dia.com,
	vinceh@...dia.com, pgaikwad@...dia.com,
	linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org,
	linux-tegra@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	tuomas.tynkkynen@....fi
Subject: Re: [PATCH v8 00/18] Tegra124 CL-DVFS / DFLL clocksource + cpufreq

Hi Mikko,

On Tue, 14 Apr 2015 14:25:59 +0300
Mikko Perttunen <mikko.perttunen@...si.fi> wrote:

> On 04/11/2015 12:11 AM, Michael Turquette wrote:
> > Quoting Thierry Reding (2015-03-11 03:07:43)
> >> Hi Mike,
> >>
> >> Have you had a chance to look at these changes to the Tegra clock
> >> driver? If you're fine with it, I'd like to take these patches through
> >> the Tegra tree because the rest of the series depends on them. I can
> >> provide a stable branch in case we need to base other Tegra clock
> >> changes on top of this.
> >
> > Hi Thierry,
> >
> > Clock patches (and corresponding DT binding descriptions and changes to
> > DTS) look fine to me. Please add:
> >
> > Acked-by: Michael Turquette <mturquette@...aro.org>
> >
> > I did have a question about the beahvior of clk_put in one of Mikko's
> > patches but it should not gate this series. I'm just trying to find out
> > if we have a bug in the framework or if the Tegra driver is a special
> > case.
> >
> > Also I do not think a stable branch is necessary.
> >
> > Regards,
> > Mike
> >
> 
> Looks like in the meantime, this has been partially broken by
> 03bc10ab5b0f "clk: check ->determine/round_rate() return value in 
> clk_calc_new_rates". The highest rates supported by the DFLL clock have 
> 1 in the MSB, so those cannot be entered after the aforementioned patch, 
> as the return value of round_rate is interpreted as an error. Avenues 
> that I can see: 1) revert the above patch 2) restrict the cpu clock rate 
> to those with 0 in the MSB 3) move to 64-bit clock rates.

How about changing ->determine_rate() and ->round_rate() prototypes so
that they always return 0 or an error code and passing the adjusted_rate
as an argument ?

Something like that:

	int (*round_rate)(struct clk_hw *hw, unsigned long *rate,
			  unsigned long *parent_rate);
	int (*determine_rate)(struct clk_hw *hw, unsigned long *rate,
			      unsigned long min_rate,
			      unsigned long max_rate,
			      unsigned long *best_parent_rate,
			      struct clk_hw **best_parent_hw);

I know this implies a lot of changes (in all clock drivers and in the
core infrastructure), but I really think we should not mix error codes
and clock frequencies (even if we decide to move to a 64 bits rate
approach).

Anyway, IMHO the only alternative to this solution is solution #3,
because #1 implies re-introducing another bug where
->round_rate()/->determine_rate() are silently ignored, and #2 implies
lying about your DFLL capabilities.

Mike, what's your opinion ?

Best Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
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/