CREG clk driver for NXP LPC18xx family
Subsystems (3)
cover CREG clk driver for NXP LPC18xx family
This patch set adds a clk driver for the low power clocks found in the CREG block on lpc18xx. CREG is a collection of miscellaneous configuration registers that can be accessed through a syscon regmap interface. The clk driver makes it possible to setup and enabled these two clocks.
This need to support peripherals like the internal RTC on the lpc18xx platform.
Based on v4.2-rc1.
Joachim Eastwood (2):
clk: add lpc18xx creg clk driver
doc: dt: add documentation for lpc1850-creg-clk driver
.../devicetree/bindings/clock/lpc1850-creg-clk.txt | 52 +++++++ drivers/clk/nxp/Makefile | 1 + drivers/clk/nxp/clk-lpc18xx-creg.c | 173 +++++++++++++++++++++ 3 files changed, 226 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/lpc1850-creg-clk.txt create mode 100644 drivers/clk/nxp/clk-lpc18xx-creg.c
Signature · 1 line
1.8.0
PATCH 1/2 clk: add lpc18xx creg clk driver
The CREG block on lpc18xx contains configuration register for two low power clocks. Support enabling of these two clocks with a clk driver that access CREG trough the syscon regmap interface.
These clocks are needed to support peripherals like the internal RTC on lpc18xx.
--- drivers/clk/nxp/Makefile | 1 + drivers/clk/nxp/clk-lpc18xx-creg.c | 173 +++++++++++++++++++++++++++++++++++++ 2 files changed, 174 insertions(+) create mode 100644 drivers/clk/nxp/clk-lpc18xx-creg.c
drivers/clk/nxp/Makefile +1 -0
@@ -1,2 +1,3 @@ obj-$(CONFIG_ARCH_LPC18XX) += clk-lpc18xx-cgu.o obj-$(CONFIG_ARCH_LPC18XX) += clk-lpc18xx-ccu.o +obj-$(CONFIG_ARCH_LPC18XX) += clk-lpc18xx-creg.o
drivers/clk/nxp/clk-lpc18xx-creg.c new file +173 -0
173 lines not on this page · raw message
Signature · 1 line
1.8.0
PATCH 1/2 clk: add lpc18xx creg clk driver
Hi Joachim,
Quoting Joachim Eastwood (2015-07-11 14:48:26)
Quoted 15 lines
drivers/clk/nxp/clk-lpc18xx-creg.c new file +15
-0+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],
drivers/clk/nxp/clk-lpc18xx-creg.c line 173
Quoted 10 lines
drivers/clk/nxp/clk-lpc18xx-creg.c new file +10
-0+ &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?
Thanks,
Mike
PATCH 1/2 clk: add lpc18xx creg clk driver
Quoted 24 lines · Michael Turquette
Hi Joachim,
Quoting Joachim Eastwood (2015-07-11 14:48:26)
Quoted 21 lines
drivers/clk/nxp/clk-lpc18xx-creg.c new file +21
-0+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);
Quoted 8 lines
drivers/clk/nxp/clk-lpc18xx-creg.c line 173
Quoted 4 lines
drivers/clk/nxp/clk-lpc18xx-creg.c new file +4
-0+ +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 useCLK_OF_DECLAREhere 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.
Note that while it's possible to use the 32k clock together with PLL0 to generate the main CPU bus clock I doubt it will be common configuration. It is however fully possible to do.
regards,
Joachim Eastwood
PATCH 1/2 clk: add lpc18xx creg clk driver
Quoting Joachim Eastwood (2015-08-13 13:43:11)
Quoted 34 lines · Michael Turquette
Quoted 24 lines
Hi Joachim,
Quoting Joachim Eastwood (2015-07-11 14:48:26)
Quoted 21 lines
drivers/clk/nxp/clk-lpc18xx-creg.c new file +21
-0+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);Quoted 8 lines
drivers/clk/nxp/clk-lpc18xx-creg.c line 173
Quoted 4 lines
drivers/clk/nxp/clk-lpc18xx-creg.c new file +4
-0+ +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 useCLK_OF_DECLAREhere or can you use the platform device model?
Quoted 7 lines
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?
Regards,
Mike
Quoted 14 lines
Note that while it's possible to use the 32k clock together with PLL0 to generate the main CPU bus clock I doubt it will be common configuration. It is however fully possible to do.
regards,
Joachim Eastwood_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH 1/2 clk: add lpc18xx creg clk driver
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.
Quoted 39 lines · Michael Turquette
Quoting Joachim Eastwood (2015-08-13 13:43:11)
Quoted 38 lines · Michael Turquette
Quoted 24 lines
Hi Joachim,
Quoting Joachim Eastwood (2015-07-11 14:48:26)
Quoted 21 lines
drivers/clk/nxp/clk-lpc18xx-creg.c new file +21
-0+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);Quoted 8 lines
drivers/clk/nxp/clk-lpc18xx-creg.c line 173
Quoted 4 lines
drivers/clk/nxp/clk-lpc18xx-creg.c new file +4
-0+ +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 useCLK_OF_DECLAREhere 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.
Quoted 6 lines
Quoted 3 lines
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
regards,
Joachim Eastwood
PATCH 1/2 clk: add lpc18xx creg clk driver
Quoting Joachim Eastwood (2016-02-13 06:38:45)
Quoted 57 lines
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.
Quoted 39 lines · Michael Turquette
Quoting Joachim Eastwood (2015-08-13 13:43:11)
Quoted 38 lines · Michael Turquette
Quoted 24 lines
Hi Joachim,
Quoting Joachim Eastwood (2015-07-11 14:48:26)
Quoted 21 lines
drivers/clk/nxp/clk-lpc18xx-creg.c new file +21
-0+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);Quoted 8 lines
drivers/clk/nxp/clk-lpc18xx-creg.c line 173
Quoted 4 lines
drivers/clk/nxp/clk-lpc18xx-creg.c new file +4
-0+ +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 useCLK_OF_DECLAREhere 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.
Quoted 6 lines
Quoted 3 lines
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.
Quoted 5 lines
Note that the cgu and ccu driver is using
CLK_OF_DECLAREnow.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.
Regards,
Mike
Quoted 4 lines
regards,
Joachim Eastwood
PATCH 1/2 clk: add lpc18xx creg clk driver
Quoted 64 lines · Michael Turquette
Quoting Joachim Eastwood (2016-02-13 06:38:45)
Quoted 57 lines
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.
Quoted 39 lines · Michael Turquette
Quoting Joachim Eastwood (2015-08-13 13:43:11)
Quoted 38 lines · Michael Turquette
Quoted 24 lines
Hi Joachim,
Quoting Joachim Eastwood (2015-07-11 14:48:26)
Quoted 21 lines
drivers/clk/nxp/clk-lpc18xx-creg.c new file +21
-0+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);Quoted 8 lines
drivers/clk/nxp/clk-lpc18xx-creg.c line 173
Quoted 4 lines
drivers/clk/nxp/clk-lpc18xx-creg.c new file +4
-0+ +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 useCLK_OF_DECLAREhere 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.
Quoted 6 lines
Quoted 3 lines
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.Quoted 5 lines
Note that the cgu and ccu driver is using
CLK_OF_DECLAREnow.I have tired to create an overview of the lpc18xx clock system here:
https://github.com/manabian/linux-lpc/wiki/LPC18xx-LPC43xx-clocks
Quoted 6 lines
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 wereplatform_driversthen 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
PATCH 1/2 clk: add lpc18xx creg clk driver
Quoting Joachim Eastwood (2016-02-17 10:24:12)
Quoted 72 lines · Michael Turquette
Quoted 64 lines
Quoting Joachim Eastwood (2016-02-13 06:38:45)
Quoted 57 lines
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.
Quoted 39 lines · Michael Turquette
Quoting Joachim Eastwood (2015-08-13 13:43:11)
Quoted 38 lines · Michael Turquette
Quoted 24 lines
Hi Joachim,
Quoting Joachim Eastwood (2015-07-11 14:48:26)
Quoted 21 lines
(quoted diff) +21
-0+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);Quoted 8 lines
Review
Quoted 4 lines
(quoted diff) +4
-0+ + 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 useCLK_OF_DECLAREhere 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.
Quoted 6 lines
Quoted 3 lines
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.Quoted 5 lines
Note that the cgu and ccu driver is using
CLK_OF_DECLAREnow.I have tired to create an overview of the lpc18xx clock system here:
https://github.com/manabian/linux-lpc/wiki/LPC18xx-LPC43xx-clocksQuoted 6 lines
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 wereplatform_driversthen you could use -EPROBE_DEFER to solve your ordering issue.
Quoted 4 lines
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 useCLK_OF_DECLARE. Or is there something I am missing?
No, you're not missing anything. What I want is for clock provider drivers to actually be real Linux device drivers. Right now the drivers that only call CLK_OF_DECLARE are essentially just initcall hacks, and this causes problems down the road when you decide you want your clk provider driver to have suspend/resume ops, etc. Also I feel that having proper drivers will be important as runtime pm and the generic power domains stuff matures.
Quoted 3 lines
Isn't true that most SoC system clock drivers must use
CLK_OF_DECLAREbecause the system timer require clocks early? I thought this was the whole point.
On ARMv8, mandatory use of architected timers removes the need for the clk framework to be involved with timer init, so it's definitely not true for all platforms.
On QCOM's 32-bit platforms they have an always-on clock with a known fixed rate, so they can either put the fixed-rate clock in DT, or even just put the frequency as a property into the timer node with no need to involve the clk framework (which is what they do today). That is also a bit hacky, but it is why you won't find CLK_OF_DECLARE in drivers/clk/qcom and all of their drivers are proper platform_drivers.
Quoted 4 lines
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?
Right, if your 32k clock is a fixed-clock, and if it is provided on the board (e.g. external extal -> lpc timer) then you could just put the fixed-rate clock into DT (which uses CLK_OF_DECLARE) and reference that from your timer node. The rest of the clocks could be registered later through a platform_driver.
Sounds like your timer clocking scheme is more complicated than that however.
Quoted 8 lines
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)
I was just discussing this with Stephen and we're both curious to know if the same "nxp,lpc1850-creg-clk" compatible string can be used with CLK_OF_DECLARE for registering the bare minimum early clks, and also with a platform_driver using a different table of clocks for late registration.
The compat string could be the same, and the driver would have different tables/setup functions to keep from re-registering the same clocks twice.
I know that nobody likes to be the guinea pig for this type of stuff, but the lack of participation in the Linux driver model for clk drivers needs to curbed. Do you think the above proposed scheme could work for your clock provider drivers?
Regards,
Mike
Quoted 4 lines
regards,
Joachim Eastwood
PATCH 1/2 clk: add lpc18xx creg clk driver
Quoted 130 lines · Michael Turquette
Quoting Joachim Eastwood (2016-02-17 10:24:12)
Quoted 72 lines · Michael Turquette
Quoted 64 lines
Quoting Joachim Eastwood (2016-02-13 06:38:45)
Quoted 57 lines
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.
Quoted 39 lines · Michael Turquette
Quoting Joachim Eastwood (2015-08-13 13:43:11)
Quoted 38 lines · Michael Turquette
Quoted 24 lines
Hi Joachim,
Quoting Joachim Eastwood (2015-07-11 14:48:26)
Quoted 21 lines
(quoted diff) +21
-0+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);Quoted 8 lines
Review
Quoted 4 lines
(quoted diff) +4
-0+ + 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 useCLK_OF_DECLAREhere 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.
Quoted 6 lines
Quoted 3 lines
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.Quoted 5 lines
Note that the cgu and ccu driver is using
CLK_OF_DECLAREnow.I have tired to create an overview of the lpc18xx clock system here:
https://github.com/manabian/linux-lpc/wiki/LPC18xx-LPC43xx-clocksQuoted 6 lines
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 wereplatform_driversthen you could use -EPROBE_DEFER to solve your ordering issue.Quoted 4 lines
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 useCLK_OF_DECLARE. Or is there something I am missing?No, you're not missing anything. What I want is for clock provider drivers to actually be real Linux device drivers. Right now the drivers that only call
CLK_OF_DECLAREare essentially just initcall hacks, and this causes problems down the road when you decide you want your clk provider driver to have suspend/resume ops, etc. Also I feel that having proper drivers will be important as runtime pm and the generic power domains stuff matures.Quoted 3 lines
Isn't true that most SoC system clock drivers must use
CLK_OF_DECLAREbecause the system timer require clocks early? I thought this was the whole point.On ARMv8, mandatory use of architected timers removes the need for the clk framework to be involved with timer init, so it's definitely not true for all platforms.
On QCOM's 32-bit platforms they have an always-on clock with a known fixed rate, so they can either put the fixed-rate clock in DT, or even just put the frequency as a property into the timer node with no need to involve the clk framework (which is what they do today). That is also a bit hacky, but it is why you won't find
CLK_OF_DECLAREindrivers/clk/qcomand all of their drivers are properplatform_drivers.Quoted 4 lines
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?Right, if your 32k clock is a fixed-clock, and if it is provided on the board (e.g. external extal -> lpc timer) then you could just put the fixed-rate clock into DT (which uses
CLK_OF_DECLARE) and reference that from your timer node. The rest of the clocks could be registered later through aplatform_driver.Sounds like your timer clocking scheme is more complicated than that however.
Quoted 8 lines
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)I was just discussing this with Stephen and we're both curious to know if the same "nxp,lpc1850-creg-clk" compatible string can be used with
CLK_OF_DECLAREfor registering the bare minimum early clks, and also with aplatform_driverusing a different table of clocks for late registration.
Quoted 3 lines
The compat string could be the same, and the driver would have different tables/setup functions to keep from re-registering the same clocks twice.
creg-clk is quite simple with only two clocks and only one of them may be needed during boot.
Quoted 4 lines
I know that nobody likes to be the guinea pig for this type of stuff, but the lack of participation in the Linux driver model for clk drivers needs to curbed. Do you think the above proposed scheme could work for your clock provider drivers?
For creg-clk it would probably work.
cgu would need to register most of clocks early since it needs to support whatever setup the bootloader throws at it. The only clocks that could be taken out is the base clocks (clk_onecell_data clk_base_data) except for one of them. But splitting this table would be a bad idea as hw clk numbers are used as indexes for easy look up.
Come to think of it; isn't splitting the clock table between a early init and platform driver going become a problem?
At least the lookup in the driver or framework would become more complex(?) And won't you confuse the lookup?
If the early driver register one clock table with of_clk_add_provider() and then sometime later the platform driver register another clock table, both using the same of_node. How is that suppose to work?
ccu has only 4 clocks needed early; CLK_CPU_TIMERx with x = 0..3.
Note that I don't have much time to spare on lpc18xx right now so I might be a bit slow if you want me to implement and test stuff.
regards,
Joachim Eastwood
PATCH 1/2 clk: add lpc18xx creg clk driver
Quoted 9 lines · Joachim Eastwood
For creg-clk it would probably work.
cgu would need to register most of clocks early since it needs to support whatever setup the bootloader throws at it. The only clocks that could be taken out is the base clocks (
clk_onecell_dataclk_base_data) except for one of them. But splitting this table would be a bad idea as hw clk numbers are used as indexes for easy look up.
Quoted 8 lines
Come to think of it; isn't splitting the clock table between a early init and platform driver going become a problem?
At least the lookup in the driver or framework would become more complex(?) And won't you confuse the lookup?
If the early driver register one clock table withof_clk_add_provider()and then sometime later the platform driver register another clock table, both using the sameof_node. How is that suppose to work?
I believe that should work, although I haven't tested it. The code looks until it finds a provider that matches the device node and gives us a valid clk pointer. If we don't get a valid pointer we keep looking for another provider until we exhaust all providers.
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH 2/2 doc: dt: add documentation for lpc1850-creg-clk driver
In reply to Joachim EastwoodAdd DT binding documentation for lpc1850-creg-clk driver.
--- .../devicetree/bindings/clock/lpc1850-creg-clk.txt | 52 ++++++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/lpc1850-creg-clk.txt
Documentation/devicetree/bindings/clock/lpc1850-creg-clk.txt new file +52 -0
@@ -0,0 +1,52 @@ +* NXP LPC1850 CREG clocks + +The NXP LPC18xx/43xx CREG (Configuration Registers) block contains +control registers for two low speed clocks. One of the clocks is a +32 kHz oscillator driver with power up/down and clock gating. Next +is a fixed divider that creates a 1 kHz clock from the 32 kHz osc. + +These clocks are used by the RTC and the Event Router peripherials. +The 32 kHz can also be routed to other peripherials to enable low +power modes. + +This binding uses the common clock binding: + Documentation/devicetree/bindings/clock/clock-bindings.txt + +Required properties: +- compatible: + Should be "nxp,lpc1850-creg-clk" +- #clock-cells: + Shall have value <1>. +- clocks: + Shall contain a phandle to the fix 32 kHz crystall. + +The creg-clk node must be a child of the creg syscon node. + +The following clocks are available from the clock node. + +Clock ID Name + 0 1 kHz clock + 1 32 kHz Oscillator + +Example: +soc { + creg: syscon at 40043000 { + compatible = "nxp,lpc1850-creg", "syscon", "simple-mfd"; + reg = <0x40043000 0x1000>; + + creg_clk: clock-controller at 004 { + compatible = "nxp,lpc1850-creg-clk"; + clocks = <&xtal32>; + #clock-cells = <1>; + }; + + ... + }; + + rtc: rtc at 40046000 { + ... + clocks = <&creg_clk 0>, <&ccu1 CLK_CPU_BUS>; + clock-names = "rtc", "reg"; + ... + }; +};
Signature · 1 line
1.8.0
cover CREG clk driver for NXP LPC18xx family
In reply to Joachim EastwoodQuoted 9 lines · Joachim Eastwood
This patch set adds a clk driver for the low power clocks found in the CREG block on lpc18xx. CREG is a collection of miscellaneous configuration registers that can be accessed through a syscon regmap interface. The clk driver makes it possible to setup and enabled these two clocks.
This need to support peripherals like the internal RTC on the lpc18xx platform.
I know this is many months old, but is this driver still required? I can apply it to clk-next if so.
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
cover CREG clk driver for NXP LPC18xx family
Hi Stephen,
Quoted 7 lines · Stephen Boyd
Quoted 6 lines · Joachim Eastwood
This patch set adds a clk driver for the low power clocks found in the CREG block on lpc18xx. CREG is a collection of miscellaneous configuration registers that can be accessed through a syscon regmap interface. The clk driver makes it possible to setup and enabled these two clocks.
Quoted 6 lines
Quoted 3 lines
This need to support peripherals like the internal RTC on the lpc18xx platform.
I know this is many months old, but is this driver still required? I can apply it to clk-next if so.
It is still needed for the internal RTC (rtc-lpc24xx) on lpc18xx. The RTC uses a 1 kHz clock that is controlled in CREG (misc system regs).
Note; if you apply it you will most likely get a very trivial conflict in drivers/clk/nxp/Makefile because clk-lpc32xx.c has been added since these patches was sent.
regards,
Joachim Eastwood
cover CREG clk driver for NXP LPC18xx family
Quoted 17 lines · Joachim Eastwood
Hi Stephen,
Quoted 7 lines · Stephen Boyd
Quoted 6 lines · Joachim Eastwood
This patch set adds a clk driver for the low power clocks found in the CREG block on lpc18xx. CREG is a collection of miscellaneous configuration registers that can be accessed through a syscon regmap interface. The clk driver makes it possible to setup and enabled these two clocks.
Quoted 6 lines
Quoted 3 lines
This need to support peripherals like the internal RTC on the lpc18xx platform.
I know this is many months old, but is this driver still required? I can apply it to clk-next if so.
Quoted 7 lines
It is still needed for the internal RTC (rtc-lpc24xx) on lpc18xx. The RTC uses a 1 kHz clock that is controlled in CREG (misc system regs).
Note; if you apply it you will most likely get a very trivial conflict in
drivers/clk/nxp/Makefilebecauseclk-lpc32xx.chas been added since these patches was sent.
Hm, ok. I see that Mike had some comments on the thread that look to be left unanswered? Care to address those first?
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project