From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 To: Jon Hunter , "Paul Walmsley" , "Boris Brezillon" From: Michael Turquette In-Reply-To: <557161D1.3040107@nvidia.com> Cc: Stephen Boyd" , linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org, "Jonathan Corbet" , "Shawn Guo" , "ascha Hauer" , "David Brown" , "Daniel Walker" , "Bryan Huntsman" , "Tony Lindgren" , "Liviu Dudau" , "Sudeep Holla" , "Lorenzo Pieralisi" , "Ralf Baechle" , "Max Filippov" , "Heiko Stuebner" , "Sylwester Nawrocki" , "Tomasz Figa" , "Barry Song" , "Viresh Kumar" , =?utf-8?b?IiBFbWlsaW8gTMOzcGV6?= , "Maxime Ripard" , "Peter De Schrijver" , "Prashant Gaikwad" , "Stephen Warren" , "Thierry Reding" , "Alexandre Courbot" , "Tero Kristo" , "Ulf Hansson" , "Michal Simek" , "Philipp Zabel" , linux-doc@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-arm-msm@vger.kernel.org, linux-omap@vger.kernel.org, linux-mips@linux-mips.org, patches@opensource.wolfsonmicro.com, linux-rockchip@lists.infradead.org, linux-samsung-soc@vger.kernel.org, spear-devel@list.st.com, linux-tegra@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-media@vger.kernel.org, rtc-linux@googlegroups.com References: <1430407809-31147-1-git-send-email-boris.brezillon@free-electrons.com> <1430407809-31147-2-git-send-email-boris.brezillon@free-electrons.com> <557161D1.3040107@nvidia.com> Message-ID: <20150610030254.6017.98627@quantum> Subject: Re: [PATCH v2 1/2] clk: change clk_ops' ->round_rate() prototype Date: Tue, 09 Jun 2015 20:02:54 -0700 List-ID: Quoting Jon Hunter (2015-06-05 01:46:09) > = > On 05/06/15 00:02, Paul Walmsley wrote: > > Hi folks > > = > > just a brief comment on this one: > > = > > On Thu, 30 Apr 2015, Boris Brezillon wrote: > > = > >> Clock rates are stored in an unsigned long field, but ->round_rate() > >> (which returns a rounded rate from a requested one) returns a long > >> value (errors are reported using negative error codes), which can lead > >> to long overflow if the clock rate exceed 2Ghz. > >> > >> Change ->round_rate() prototype to return 0 or an error code, and pass= the > >> requested rate as a pointer so that it can be adjusted depending on > >> hardware capabilities. > > = > > ... > > = > >> diff --git a/Documentation/clk.txt b/Documentation/clk.txt > >> index 0e4f90a..fca8b7a 100644 > >> --- a/Documentation/clk.txt > >> +++ b/Documentation/clk.txt > >> @@ -68,8 +68,8 @@ the operations defined in clk.h: > >> int (*is_enabled)(struct clk_hw *hw); > >> unsigned long (*recalc_rate)(struct clk_hw *hw, > >> unsigned long parent_rate= ); > >> - long (*round_rate)(struct clk_hw *hw, > >> - unsigned long rate, > >> + int (*round_rate)(struct clk_hw *hw, > >> + unsigned long *rate, > >> unsigned long *parent_rat= e); > >> long (*determine_rate)(struct clk_hw *hw, > >> unsigned long rate, > > = > > I'd suggest that we should probably go straight to 64-bit rates. There = > > are already plenty of clock sources that can generate rates higher than = > > 4GiHz. Paul, I agree. Changing the underlying struct clk.rate to 64 bits is on my radar. > = > An alternative would be to introduce to a frequency "base" the default > could be Hz (for backwards compatibility), but for CPUs we probably only > care about MHz (or may be kHz) and so 32-bits would still suffice. Even > if CPUs cared about Hz they could still use Hz, but in that case they > probably don't care about GHz. Obviously, we don't want to break DT > compatibility but may be the frequency base could be defined in DT and > if it is missing then Hz is assumed. Just a thought ... I was thinking of doing it the other way. E.g. use 64-bit rates throughout the clk framework and have a base unit of millihertz instead of hertz. 64 bits gives us a LOT of room to grow, and the current base of hertz completely ignores all applications with fractional hertz requirements. Your point came up a while ago in the context of cpufreq and the clk framework agreeing on the type for a rate[0]. The consensus was to continue using KHz as the base type for that subsystem. [0] https://lkml.org/lkml/2014/5/26/582 (There is more to the above conversation that the mail archives didn't seem to catch :-/ ) Regards, Mike > = > Jon