On 17 February 2016 at 01:56, Michael Turquette <mturquette@xxxxxxxxxxxx> wrote: > Quoting Joachim Eastwood (2016-02-13 06:38:45) >> Hi Mike, >> >> Seems your reply got lost in my mailbox and I didn't notice it before >> Stephen replied on the cover letter. Sorry about that. >> >> On 18 August 2015 at 02:26, Michael Turquette <mturquette@xxxxxxxxxxxx> wrote: >> > Quoting Joachim Eastwood (2015-08-13 13:43:11) >> >> On 11 August 2015 at 22:41, Michael Turquette <mturquette@xxxxxxxxxxxx> wrote: >> >> > Hi Joachim, >> >> > >> >> > Quoting Joachim Eastwood (2015-07-11 14:48:26) >> >> >> +static void __init lpc18xx_creg_clk_init(struct device_node *np) >> >> >> +{ >> >> >> + const char *clk_32khz_parent; >> >> >> + struct regmap *syscon; >> >> >> + >> >> >> + syscon = syscon_node_to_regmap(np->parent); >> >> >> + if (IS_ERR(syscon)) { >> >> >> + pr_err("%s: syscon lookup failed\n", __func__); >> >> >> + return; >> >> >> + } >> >> >> + >> >> >> + clk_32khz_parent = of_clk_get_parent_name(np, 0); >> >> >> + >> >> >> + clk_creg[CREG_CLK_32KHZ] = >> >> >> + clk_register_creg_clk(&clk_creg_clocks[CREG_CLK_32KHZ], >> >> >> + &clk_32khz_parent, syscon); >> >> >> + >> >> >> + clk_creg[CREG_CLK_1KHZ] = >> >> >> + clk_register_creg_clk(&clk_creg_clocks[CREG_CLK_1KHZ], >> >> >> + &clk_creg_clocks[CREG_CLK_32KHZ].name, >> >> >> + syscon); >> >> >> + >> >> >> + of_clk_add_provider(np, of_clk_src_onecell_get, &clk_base_data); >> >> >> +} >> >> >> +CLK_OF_DECLARE(lpc18xx_creg_clk, "nxp,lpc1850-creg-clk", lpc18xx_creg_clk_init); >> >> > >> >> > I'll ask the same question that Stephen asked in your CCU/CGU driver >> >> > series: is it necessary to use CLK_OF_DECLARE here or can you use the >> >> > platform device model? >> >> >> >> The 32 kHz clock from the CREG block is a clock parent to the CGU >> >> block so it's possible that it will required early. This is all >> >> depends on how the boot loader initially configures the CGU. >> >> >> >> Currently in the DTS for lpc18xx cgu it has: >> >> clocks = <&xtal>, <&xtal32>, <...>; >> >> xtal32 is just a temporary placeholder until the CREG clock is in place. >> > >> > Well that seems wrong. Is it just a matter of probe order where you try >> > to probe the cgu driver before the creg driver? >> >> The ideal probe order for the clk drivers on the lpc18xx platform >> would be; creg-clk, cgu and ccu. >> If the 32k clk is used by any of timers we need creg-clk to enable the >> 32k clock and determine the rate. >> >> Note that the cgu and ccu driver is using CLK_OF_DECLARE now. >> >> >> I have tired to create an overview of the lpc18xx clock system here: >> https://github.com/manabian/linux-lpc/wiki/LPC18xx-LPC43xx-clocks > > That's great, thanks a lot for the link and the nice documentation. > > It's been a while since I've looked at this thread. Do any of the > creg-clk, cgu or ccu clock drivers need to use CLK_OF_DECLARE? If they > were platform_drivers then you could use -EPROBE_DEFER to solve your > ordering issue. The clock for the clocksource/timer (drivers/clocksource/time-lpc32xx.c) comes from cgu/ccu and can also come from creg-clk. So afaik they must use CLK_OF_DECLARE. Or is there something I am missing? Isn't true that most SoC system clock drivers must use CLK_OF_DECLARE because the system timer require clocks early? I thought this was the whole point. Since it is possible that the 32k clock from creg-clk can be use as a parent clock for the timer doesn't this mean it also must use CLK_OF_DECLARE? If it wasn't possible for creg-clk to be used as a parent clock for the timer it could have been a platform device. The lpc18xx clock configuration that require creg-clk early would be: 32k - > PLL0 -> BASE_M3/M4_CLK -> CLK_M3/M4_TIMER0. See second figure on the webpage. Note that this is probably not a common configuration, but it is a valid one. (PLL0 accepts input down to 14 kHz) regards, Joachim Eastwood -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html