| Message ID | 1406643628-4061-1-git-send-email-stefan@agner.ch |
|---|---|
| State | New |
| Headers | show |
On Tuesday 29 July 2014 16:20:28 Stefan Agner wrote: > At the end of the boot process, the clock framework might disable > required main PLL's. So far, this was no issue since drivers > requested clocks, which are descended of the main PLL's (e.g. > pll1_pfd1, which provides the system clock). > > To archive the full 500MHz system clock, DDR clock need to be a > descendant of PLL2 rather than PLL1 (DDRC_CLK_SEL set to 0). The > bootloader sets up the clocks accordingly before making use of > DDR at all. However, in Linux, there is no driver using PLL2, > which lead to PLL2 being disabled by the clock framework. > > With this patch, we make sure that the main system clock and the > DDR clock are initially enabled and are kept enabled. > > Signed-off-by: Stefan Agner <stefan@agner.ch> > Wouldn't it be better to list this in the DT as a default for the respective clocks? Arnd
Am 2014-07-29 16:44, schrieb Arnd Bergmann: > On Tuesday 29 July 2014 16:20:28 Stefan Agner wrote: >> At the end of the boot process, the clock framework might disable >> required main PLL's. So far, this was no issue since drivers >> requested clocks, which are descended of the main PLL's (e.g. >> pll1_pfd1, which provides the system clock). >> >> To archive the full 500MHz system clock, DDR clock need to be a >> descendant of PLL2 rather than PLL1 (DDRC_CLK_SEL set to 0). The >> bootloader sets up the clocks accordingly before making use of >> DDR at all. However, in Linux, there is no driver using PLL2, >> which lead to PLL2 being disabled by the clock framework. >> >> With this patch, we make sure that the main system clock and the >> DDR clock are initially enabled and are kept enabled. >> >> Signed-off-by: Stefan Agner <stefan@agner.ch> >> > > Wouldn't it be better to list this in the DT as a default for > the respective clocks? > What do you mean by that exactly? Creating a driver for the main PLL's and add device tree entries to instantiate them, along with a property like "boot-enabled" or similar? The approach chosen in this patch is aligned the way it's done for i.MX6 (see arch/arm/mach-imx/clk-imx6q.c). The whole clock module(s) in Vybrid are slightly striped variants found in i.MX6. For now, I would prefer to leave it that way. -- Stefan
On Tuesday 29 July 2014 20:39:20 Stefan Agner wrote: > Am 2014-07-29 16:44, schrieb Arnd Bergmann: > > On Tuesday 29 July 2014 16:20:28 Stefan Agner wrote: > >> At the end of the boot process, the clock framework might disable > >> required main PLL's. So far, this was no issue since drivers > >> requested clocks, which are descended of the main PLL's (e.g. > >> pll1_pfd1, which provides the system clock). > >> > >> To archive the full 500MHz system clock, DDR clock need to be a > >> descendant of PLL2 rather than PLL1 (DDRC_CLK_SEL set to 0). The > >> bootloader sets up the clocks accordingly before making use of > >> DDR at all. However, in Linux, there is no driver using PLL2, > >> which lead to PLL2 being disabled by the clock framework. > >> > >> With this patch, we make sure that the main system clock and the > >> DDR clock are initially enabled and are kept enabled. > >> > >> Signed-off-by: Stefan Agner <stefan@agner.ch> > >> > > > > Wouldn't it be better to list this in the DT as a default for > > the respective clocks? > > > > What do you mean by that exactly? Creating a driver for the main PLL's > and add device tree entries to instantiate them, along with a property > like "boot-enabled" or similar? I was under the assumption that you already have a driver for the main PLL, I think most other SoCs have that. We talked about the general problem at some point during a conference and the conclusion was that it makes most sense to have defaults encoded as properties in the device node of the clock controller. If you don't have a device node for that, it won't work. > The approach chosen in this patch is aligned the way it's done for i.MX6 > (see arch/arm/mach-imx/clk-imx6q.c). The whole clock module(s) in Vybrid > are slightly striped variants found in i.MX6. For now, I would prefer to > leave it that way. Ok, makes sense. Just leave it like this then. I believe i.MX is a bit different from other SoCs, because it adopted some concepts like common clk earlier than the rest, but consistency within the family is more important than consistency across SoC families. Arnd
On Tue, Jul 29, 2014 at 04:20:28PM +0200, Stefan Agner wrote: > At the end of the boot process, the clock framework might disable > required main PLL's. So far, this was no issue since drivers > requested clocks, which are descended of the main PLL's (e.g. > pll1_pfd1, which provides the system clock). > > To archive the full 500MHz system clock, DDR clock need to be a > descendant of PLL2 rather than PLL1 (DDRC_CLK_SEL set to 0). The > bootloader sets up the clocks accordingly before making use of > DDR at all. However, in Linux, there is no driver using PLL2, > which lead to PLL2 being disabled by the clock framework. > > With this patch, we make sure that the main system clock and the > DDR clock are initially enabled and are kept enabled. > > Signed-off-by: Stefan Agner <stefan@agner.ch> Applied, thanks.
diff --git a/arch/arm/mach-imx/clk-vf610.c b/arch/arm/mach-imx/clk-vf610.c index f60d6d5..a41ed73 100644 --- a/arch/arm/mach-imx/clk-vf610.c +++ b/arch/arm/mach-imx/clk-vf610.c @@ -98,9 +98,15 @@ static struct clk_div_table pll4_main_div_table[] = { static struct clk *clk[VF610_CLK_END]; static struct clk_onecell_data clk_data; +static unsigned int const clks_init_on[] __initconst = { + VF610_CLK_SYS_BUS, + VF610_CLK_DDR_SEL, +}; + static void __init vf610_clocks_init(struct device_node *ccm_node) { struct device_node *np; + int i; clk[VF610_CLK_DUMMY] = imx_clk_fixed("dummy", 0); clk[VF610_CLK_SIRC_128K] = imx_clk_fixed("sirc_128k", 128000); @@ -322,6 +328,9 @@ static void __init vf610_clocks_init(struct device_node *ccm_node) clk_set_parent(clk[VF610_CLK_SAI2_SEL], clk[VF610_CLK_AUDIO_EXT]); clk_set_parent(clk[VF610_CLK_SAI3_SEL], clk[VF610_CLK_AUDIO_EXT]); + for (i = 0; i < ARRAY_SIZE(clks_init_on); i++) + clk_prepare_enable(clk[clks_init_on[i]]); + /* Add the clocks to provider list */ clk_data.clks = clk; clk_data.clk_num = ARRAY_SIZE(clk);
At the end of the boot process, the clock framework might disable required main PLL's. So far, this was no issue since drivers requested clocks, which are descended of the main PLL's (e.g. pll1_pfd1, which provides the system clock). To archive the full 500MHz system clock, DDR clock need to be a descendant of PLL2 rather than PLL1 (DDRC_CLK_SEL set to 0). The bootloader sets up the clocks accordingly before making use of DDR at all. However, in Linux, there is no driver using PLL2, which lead to PLL2 being disabled by the clock framework. With this patch, we make sure that the main system clock and the DDR clock are initially enabled and are kept enabled. Signed-off-by: Stefan Agner <stefan@agner.ch> --- arch/arm/mach-imx/clk-vf610.c | 9 +++++++++ 1 file changed, 9 insertions(+)