On Mon, Dec 05, 2011 at 02:15:56PM -0700, Paul Walmsley wrote:
The types associated with clock rates in the clock interface (include/linux/clk.h) are inconsistent, and we should fix this.
Rubbish. They're different with good reason. Rates are primerily unsigned quantities - and should be treated as such.
The exception is clk_round_rate() which returns the rate, but also _may_ return an error. Therefore, its return type has to be signed.
We could fix the immediate problem by changing the prototype of clk_round_rate() to pass the rounded rate back to the caller via a pointer in one of the arguments, and return an error code (if any) via the return value:
int clk_round_rate(struct clk *clk, unsigned long rate, unsigned long *rounded_rate);
Yes that might have been a better solution.
But I'd propose that we instead increase the size of struct clk.rate to be s64:
s64 clk_round_rate(struct clk *clk, s64 desired_rate); int clk_set_rate(struct clk *clk, s64 rate); s64 clk_get_rate(struct clk *clk);
struct clk { ... s64 rate; ... };
That way the clock framework can accommodate current clock rates, as well as any conceivable future clock rate. (Some production CPUs are already running at clock rates greater than 4 GiHZ[1]. RF devices with 4 GiHz+ clock rates are also common, such as 802.11a devices running in the 5.8 GHz band, and drivers for those may eventually wish to use the clock framework.)
Yuck. You are aware that 64-bit math on 32-bit CPUs sucks? So burdening _everything_ with 64-bit rate quantities is absurd. As for making then 64-bit signed quantities, that's asking for horrid code from gcc for the majority of cases.