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.