[PATCH 4/4] ARM: clps711x: Added simple clkdev framework

[Turquette, Mike]

On Fri, Jul 20, 2012 at 11:24 AM, Alexander Shiyan <shc_work at mail.ru> wrote:
> Hello.
>
> Fri, 20 Jul 2012 11:12:23 -0700 от "Turquette, Mike" <mturquette at ti.com>:
>> On Fri, Jul 20, 2012 at 11:02 AM, Alexander Shiyan <shc_work at mail.ru> wrote:
>> > Fri, 20 Jul 2012 10:41:59 -0700 от "Turquette, Mike" <mturquette at ti.com>:
>> >> On Fri, Jul 20, 2012 at 10:24 AM, Alexander Shiyan <shc_work at mail.ru> wrote:
>> >> > Thu, 19 Jul 2012 15:02:00 -0700 от "Turquette, Mike" <mturquette at ti.com>:
>> >> >> On Tue, Jul 17, 2012 at 1:33 PM, Arnd Bergmann <arnd at arndb.de> wrote:
>> >> >> > On Monday 16 July 2012, Alexander Shiyan wrote:
>> >> >> >> Modern CPUs from CLPS711X-line can operate at frequencies other than 73 MHz.
>> >> >> >> This patch adds simple clkdev framework for handling all possible CPU rates.
>> >> >> >> Signed-off-by: Alexander Shiyan <shc_work at mail.ru>
>> >> >> > We just had the exact same discussion about a new clock implementation for
>> >> >> > mcs814x and for the socfpga port. My feeling is that it makes no sense
>> >> >> > to keep adding new private implementations of the API as we're trying
>> >> >> > to get rid of the existing ones and replace them with the common code.
>> >> >> >
>> >> >> > If I understand this correctly, you should be using DEFINE_CLK_FIXED_RATE
>> >> >> > and DEFINE_CLK_DIVIDER etc to define the initial clocks and the lookup
>> >> >> > table, and possibly move all of that to drivers/clk.
>> >> >>
>> >> >> Arnd is correct.  Let's go ahead and move everything over to the
>> >> >> common clk framework.  These patches will have to wait for 3.7
>> >> >> anyways, so there is plenty of time to port it over.
>> >> >
>> >> > OK, I keep this patch and will be create new one, for review, but as I say before,
>> >> > COMMON_CLOCK framework is too overloaded because clps711x-target CPU
>> >> > has no enable/disable and change rate functional for clocks. We only can
>> >> > determine CPU speed from fuses and recalculate some values for system
>> >> > timer and UART.
>> >>
>> >> It might be worthwhile to implement something like
>> >> drivers/clk/clk-dummy.c which provides no real clk_ops, but
>> >> initializes parents and rates at clk_register-timer.  I haven't looked
>> >> at your patch yet to know if such an idea is applicable or not, but I
>> >> doubt you are the only platform which has "noop" clocks, so it might
>> >> be nice to expose a standard clock type for this.
>> >> What do you think?
>> >
>> > If I understand correctly, FIXED_CLK have to deal with it.
>> > Why need one more class?
>>
>> Ah, I thought that maybe you would have to dynamically detect parents
>> as well.  Also fixed clk expects to have the rate passed into it, so
>> if you need to read some registers to determine rate then that will be
>> something new.
>>
>> If you don't have mux clocks and you know you rate already (in
>> software) then fixed clk is fine for you.
>
> The multiplexer is really there, but it is determined at boot time.
> It can not be changed during operation.
> I just wanted to say that the use of a large class for such simple operations
> does not make much sense. But if we need to bring everything to the same
> class, I will write a second version of the patch, it is not difficult.

There are benefits to using the common struct clk, including
multi-platform boot (single zImage), and also clock tree
"connectivity" with other devices that may use the new common struct
clk more extensively.

Thanks,
Mike