| Message ID | 1342462908-18547-1-git-send-email-shc_work@mail.ru (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Monday 16 July 2012, Alexander Shiyan wrote: > Since we are trying to do to support multiple machines in a single kernel, > we need to eliminate the use of __initcall to be used for all machines. > Using .init_early call solves this problem. > > Signed-off-by: Alexander Shiyan <shc_work@mail.ru> I've added patches 1-3 to the next/cleanup branch now. Let's wait for comments on the clock patch first. My feeling is that you should be using the clock framework. Arnd
Hello. Thu, 19 Jul 2012 15:02:00 -0700 ?? "Turquette, Mike" <mturquette@ti.com>: > On Tue, Jul 17, 2012 at 1:33 PM, Arnd Bergmann <arnd@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@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.
On Fri, Jul 20, 2012 at 10:24 AM, Alexander Shiyan <shc_work@mail.ru> wrote: > Hello. > > Thu, 19 Jul 2012 15:02:00 -0700 ?? "Turquette, Mike" <mturquette@ti.com>: >> On Tue, Jul 17, 2012 at 1:33 PM, Arnd Bergmann <arnd@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@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? Regards, Mike
Hello. Fri, 20 Jul 2012 11:12:23 -0700 ?? "Turquette, Mike" <mturquette@ti.com>: > On Fri, Jul 20, 2012 at 11:02 AM, Alexander Shiyan <shc_work@mail.ru> wrote: > > Fri, 20 Jul 2012 10:41:59 -0700 ?? "Turquette, Mike" <mturquette@ti.com>: > >> On Fri, Jul 20, 2012 at 10:24 AM, Alexander Shiyan <shc_work@mail.ru> wrote: > >> > Thu, 19 Jul 2012 15:02:00 -0700 ?? "Turquette, Mike" <mturquette@ti.com>: > >> >> On Tue, Jul 17, 2012 at 1:33 PM, Arnd Bergmann <arnd@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@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.
On Fri, Jul 20, 2012 at 11:24 AM, Alexander Shiyan <shc_work@mail.ru> wrote: > Hello. > > Fri, 20 Jul 2012 11:12:23 -0700 ?? "Turquette, Mike" <mturquette@ti.com>: >> On Fri, Jul 20, 2012 at 11:02 AM, Alexander Shiyan <shc_work@mail.ru> wrote: >> > Fri, 20 Jul 2012 10:41:59 -0700 ?? "Turquette, Mike" <mturquette@ti.com>: >> >> On Fri, Jul 20, 2012 at 10:24 AM, Alexander Shiyan <shc_work@mail.ru> wrote: >> >> > Thu, 19 Jul 2012 15:02:00 -0700 ?? "Turquette, Mike" <mturquette@ti.com>: >> >> >> On Tue, Jul 17, 2012 at 1:33 PM, Arnd Bergmann <arnd@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@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
diff --git a/arch/arm/mach-clps711x/p720t.c b/arch/arm/mach-clps711x/p720t.c index 42ee8f3..f266d90 100644 --- a/arch/arm/mach-clps711x/p720t.c +++ b/arch/arm/mach-clps711x/p720t.c @@ -86,17 +86,7 @@ static void __init p720t_map_io(void) iotable_init(p720t_io_desc, ARRAY_SIZE(p720t_io_desc)); } -MACHINE_START(P720T, "ARM-Prospector720T") - /* Maintainer: ARM Ltd/Deep Blue Solutions Ltd */ - .atag_offset = 0x100, - .fixup = fixup_p720t, - .map_io = p720t_map_io, - .init_irq = clps711x_init_irq, - .timer = &clps711x_timer, - .restart = clps711x_restart, -MACHINE_END - -static int p720t_hw_init(void) +static void __init p720t_init_early(void) { /* * Power down as much as possible in case we don't @@ -111,13 +101,19 @@ static int p720t_hw_init(void) PLD_CODEC = 0; PLD_TCH = 0; PLD_SPI = 0; -#ifndef CONFIG_DEBUG_LL - PLD_COM2 = 0; - PLD_COM1 = 0; -#endif - - return 0; + if (!IS_ENABLED(CONFIG_DEBUG_LL)) { + PLD_COM2 = 0; + PLD_COM1 = 0; + } } -__initcall(p720t_hw_init); - +MACHINE_START(P720T, "ARM-Prospector720T") + /* Maintainer: ARM Ltd/Deep Blue Solutions Ltd */ + .atag_offset = 0x100, + .fixup = fixup_p720t, + .init_early = p720t_init_early, + .map_io = p720t_map_io, + .init_irq = clps711x_init_irq, + .timer = &clps711x_timer, + .restart = clps711x_restart, +MACHINE_END
Since we are trying to do to support multiple machines in a single kernel, we need to eliminate the use of __initcall to be used for all machines. Using .init_early call solves this problem. Signed-off-by: Alexander Shiyan <shc_work@mail.ru> --- arch/arm/mach-clps711x/p720t.c | 34 +++++++++++++++------------------- 1 files changed, 15 insertions(+), 19 deletions(-)