Re: [PATCH 2/2] clk: uniphier: add clock data for cpufreq

[Masahiro Yamada]

Hi Stephen,


2016-11-24 9:05 GMT+09:00 Stephen Boyd <sboyd@xxxxxxxxxxxxxx>:

>> +#if 1
>> +     /*
>> +      * TODO:
>> +      * The return type of .round_rate() is "long", which is 32 bit wide on
>> +      * 32 bit systems.  Clock rate greater than LONG_MAX (~ 2.15 GHz) is
>> +      * treated as an error.  Needs a workaround until the problem is fixed.
>> +      */
>
> Just curious is the problem internal to the clk framework because
> of the clk_ops::round_rate design? Or does the consumer, cpufreq
> in this case, have to deal with rates that are larger than
> unsigned long on a 32 bit system? If it's just a clk_ops problem
> and we need to support rates up to 32 bits wide (~ 4.3 GHz) on
> the system then the driver could be changed to use
> .determine_rate() ops and that would allow us to use all the bits
> of unsigned long to figure out rates.
>
> If the problem is rates even larger than unsigned long on 32 bit
> systems, then at the least I'd like to see some sort of plan to
> fix that in the framework before merging code. Hopefully it can
> be done gradually, but as I start looking at it it seems more and
> more complicated to support this so this will be a long term
> project.
>
> We can discuss the clk API changes needed as well if those are
> required, but that is another issue that requires changes in
> other places outside of clk drivers.
>

I understand your point, but core frame-work changes
need more careful review than clk data changes in low-level drivers.
It is too late to be included in v4.10.

If I drop 32bit SoC things, and send v2 only for 64bit SoCs,
is that acceptable for 4.10-rc1?

Our ARMv8 SoCs are more actively developed, so
getting 64bit clk data in first will be helpful.

I postpone the 32bit SoC data until
I figure out the root cause of the problem
(and possible change for the API if necessary).


-- 
Best Regards
Masahiro Yamada