DT clock bindings
Subsystems (8)
v3 cover DT clock bindings
From: Rob Herring <[email protected]>
This series defines clock bindings for Device-Tree and adds kernel support using the common clock infrastructure. The last patch enables DT clock support for the Calxeda Highbank platform.
I'm posting this again to solicit further review. There has been some discussion[1], but no definite path forward. This series is not changed from the last post other than rebasing to v3.5-rc2.
Rob
[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2012-May/100932.html
Grant Likely (2):
clk: add DT clock binding support
clk: add DT fixed-clock binding support
Rob Herring (2):
dt: add clock binding doc to primecell bindings
clk: add highbank clock support
.../devicetree/bindings/arm/primecell.txt | 6 + .../devicetree/bindings/clock/calxeda.txt | 17 + .../devicetree/bindings/clock/clock-bindings.txt | 117 +++++++ .../devicetree/bindings/clock/fixed-clock.txt | 21 ++ arch/arm/Kconfig | 1 + arch/arm/boot/dts/highbank.dts | 91 +++++- arch/arm/mach-highbank/Makefile | 2 +- arch/arm/mach-highbank/clock.c | 62 ---- arch/arm/mach-highbank/highbank.c | 7 + drivers/clk/Makefile | 1 + drivers/clk/clk-fixed-rate.c | 23 ++ drivers/clk/clk-highbank.c | 345 ++++++++++++++++++++ drivers/clk/clk.c | 140 ++++++++ drivers/clk/clkdev.c | 77 +++++ include/linux/clk-provider.h | 16 + include/linux/clk.h | 19 ++ 16 files changed, 881 insertions(+), 64 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/calxeda.txt create mode 100644 Documentation/devicetree/bindings/clock/clock-bindings.txt create mode 100644 Documentation/devicetree/bindings/clock/fixed-clock.txt delete mode 100644 arch/arm/mach-highbank/clock.c create mode 100644 drivers/clk/clk-highbank.c
Signature · 1 line
1.7.9.5
PATCH v3 1/4 clk: add DT clock binding support
From: Grant Likely <[email protected]>
Based on work 1st by Ben Herrenschmidt and Jeremy Kerr, then by Grant Likely, this patch adds support to clk_get to allow drivers to retrieve clock data from the device tree.
Platforms scan for clocks in DT with of_clk_init and a match table, and the register a provider through of_clk_add_provider. The provider's clk_src_get function will be called when a device references the provider's OF node for a clock reference.
v5 (Rob Herring):
- Move from drivers/of into common clock subsystem
- Squashed "dt/clock: add a simple provider get function" and "dt/clock: add function to get parent clock name"
- Rebase to 3.4-rc1
- Drop CONFIG_OF_CLOCK and just use CONFIG_OF
- Add missing EXPORT_SYMBOL to various functions
- s/clock-output-name/clock-output-names/
- Define that fixed-clock binding is a single output
v4 (Rob Herring):
- Rework for common clk subsystem
- Add of_clk_get_parent_name function
v3: - Clarified documentation
v2: - fixed errant ';' causing compile error
- Editorial fixes from Shawn Guo
- merged in adding lookup to clkdev
- changed property names to match established convention. After working with the binding a bit it really made more sense to follow the lead of 'reg', 'gpios' and 'interrupts' by making the input simply 'clocks' & 'clock-names' instead of 'clock-input-*', and to only use clock-output* for the producer nodes. (Sorry Shawn, this will mean you need to change some code, but it should be trivial)
- Add ability to inherit clocks from parent nodes by using an empty 'clock-ranges' property. Useful for busses. I could use some feedback on the new property name, 'clock-ranges' doesn't feel right to me.
--- .../devicetree/bindings/clock/clock-bindings.txt | 117 ++++++++++++++++ .../devicetree/bindings/clock/fixed-clock.txt | 21 +++ drivers/clk/clk.c | 140 ++++++++++++++++++++ drivers/clk/clkdev.c | 77 +++++++++++ include/linux/clk-provider.h | 14 ++ include/linux/clk.h | 19 +++ 6 files changed, 388 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/clock-bindings.txt create mode 100644 Documentation/devicetree/bindings/clock/fixed-clock.txt
Documentation/devicetree/bindings/clock/clock-bindings.txt new file +117 -0
@@ -0,0 +1,117 @@ +This binding is a work-in-progress, and are based on some experimental +work by benh[1]. + +Sources of clock signal can be represented by any node in the device +tree. Those nodes are designated as clock providers. Clock consumer +nodes use a phandle and clock specifier pair to connect clock provider +outputs to clock inputs. Similar to the gpio specifiers, a clock +specifier is an array of one more more cells identifying the clock +output on a device. The length of a clock specifier is defined by the +value of a #clock-cells property in the clock provider node. + +[1] http://patchwork.ozlabs.org/patch/31551/ + +==Clock providers== + +Required properties: +#clock-cells: Number of cells in a clock specifier; Typically 0 for nodes + with a single clock output and 1 for nodes with multiple + clock outputs. + +Optional properties: +clock-output-names: Recommended to be a list of strings of clock output signal + names indexed by the first cell in the clock specifier. + However, the meaning of clock-output-names is domain + specific to the clock provider, and is only provided to + encourage using the same meaning for the majority of clock + providers. This format may not work for clock providers + using a complex clock specifier format. In those cases it + is recommended to omit this property and create a binding + specific names property. + + Clock consumer nodes must never directly reference + the provider's clock-output-names property. + +For example: + + oscillator { + #clock-cells = <1>; + clock-output-names = "ckil", "ckih"; + }; + +- this node defines a device with two clock outputs, the first named + "ckil" and the second named "ckih". Consumer nodes always reference + clocks by index. The names should reflect the clock output signal + names for the device. + +==Clock consumers== + +Required properties: +clocks: List of phandle and clock specifier pairs, one pair + for each clock input to the device. Note: if the + clock provider specifies '0' for #clock-cells, then + only the phandle portion of the pair will appear. + +Optional properties: +clock-names: List of clock input name strings sorted in the same + order as the clocks property. Consumers drivers + will use clock-names to match clock input names + with clocks specifiers. +clock-ranges: Empty property indicating that child nodes can inherit named + clocks from this node. Useful for bus nodes to provide a + clock to their children. + +For example: + + device { + clocks = <&osc 1>, <&ref 0>; + clock-names = "baud", "register"; + }; + + +This represents a device with two clock inputs, named "baud" and "register". +The baud clock is connected to output 1 of the &osc device, and the register +clock is connected to output 0 of the &ref. + +==Example== + + /* external oscillator */ + osc: oscillator { + compatible = "fixed-clock"; + #clock-cells = <1>; + clock-frequency = <32678>; + clock-output-names = "osc"; + }; + + /* phase-locked-loop device, generates a higher frequency clock + * from the external oscillator reference */ + pll: pll at 4c000 { + compatible = "vendor,some-pll-interface" + #clock-cells = <1>; + clocks = <&osc 0>; + clock-names = "ref"; + reg = <0x4c000 0x1000>; + clock-output-names = "pll", "pll-switched"; + }; + + /* UART, using the low frequency oscillator for the baud clock, + * and the high frequency switched PLL output for register + * clocking */ + uart at a000 { + compatible = "fsl,imx-uart"; + reg = <0xa000 0x1000>; + interrupts = <33>; + clocks = <&osc 0>, <&pll 1>; + clock-names = "baud", "register"; + }; + +This DT fragment defines three devices: an external oscillator to provide a +low-frequency reference clock, a PLL device to generate a higher frequency +clock signal, and a UART. + +* The oscillator is fixed-frequency, and provides one clock output, named "osc". +* The PLL is both a clock provider and a clock consumer. It uses the clock + signal generated by the external oscillator, and provides two output signals + ("pll" and "pll-switched"). +* The UART has its baud clock connected the external oscillator and its + register clock connected to the PLL clock (the "pll-switched" signal)
Documentation/devicetree/bindings/clock/fixed-clock.txt new file +21 -0
@@ -0,0 +1,21 @@ +Binding for simple fixed-rate clock sources. + +This binding uses the common clock binding[1]. + +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt + +Required properties: +- compatible : shall be "fixed-clock". +- #clock-cells : from common clock binding; shall be set to 0. +- clock-frequency : frequency of clock in Hz. Should be a single cell. + +Optional properties: +- gpios : From common gpio binding; gpio connection to clock enable pin. +- clock-output-names : From common clock binding. + +Example: + clock { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <1000000000>; + };
drivers/clk/clk.c +140 -0
149 lines not on this page · raw message
drivers/clk/clkdev.c +77 -0
93 lines not on this page · raw message
include/linux/clk-provider.h +14 -0
@@ -334,5 +334,19 @@ void __clk_unprepare(struct clk *clk);void __clk_reparent (struct clk *clk,struct clk *new_parent);unsigned long __clk_round_rate (struct clk *clk,unsigned long rate); +struct of_device_id ; + +typedef void (*of_clk_init_cb_t )(struct device_node *); + +int of_clk_add_provider (struct device_node *np, +struct clk *(*clk_src_get)(struct of_phandle_args *args, +void *data), +void *data); +void of_clk_del_provider (struct device_node *np); +struct clk *of_clk_src_simple_get (struct of_phandle_args *clkspec, +void *data); +const char *of_clk_get_parent_name (struct device_node *np,int index); +void of_clk_init (const struct of_device_id *matches); +#endif /* CONFIG_COMMON_CLK */ #endif /* CLK_PROVIDER_H */
include/linux/clk.h +19 -0
@@ -310,4 +310,23 @@ struct clk *clk_get_sys(const char *dev_id, const char *con_id);int clk_add_alias (const char *alias,const char *alias_dev_name,char *id,struct device *dev); +struct device_node ; +struct of_phandle_args ; + +#ifdef CONFIG_OF +struct clk *of_clk_get (struct device_node *np,int index); +struct clk *of_clk_get_by_name (struct device_node *np,const char *name); +struct clk *of_clk_get_from_provider (struct of_phandle_args *clkspec); +#else +static inline struct clk *of_clk_get (struct device_node *np,int index) +{ +return NULL ; +} +static inline struct clk *of_clk_get_by_name (struct device_node *np, +const char *name) +{ +return NULL ; +} +#endif +#endif
Signature · 1 line
1.7.9.5
Re: [PATCH v3 1/4] clk: add DT clock binding support
drivers/clk/clk.c int clk_notifier_unregister(struct clk *clk, struct notifier_block *nb) · line 1630
Quoted 4 lines · Rob Herring
drivers/clk/clk.c +4
-0+struct clk *of_clk_get_from_provider (struct of_phandle_args *clkspec) +{ +struct of_clk_provider *provider; +struct clk *clk =NULL ;
Both clk and clkdev treat NULL as a valid clock and return ERR_PTR for error case, while all the codes in this patch just return NULL for error and check (clk != NULL) for valid clock.
Should we force the consistent behavior between DT and non-DT on this?
Quoted 13 lines
drivers/clk/clk.c +13
-0+ +/* Check if we have such a provider in our array */ +mutex_lock (&of_clk_lock); +list_for_each_entry (provider, &of_clk_providers, link) { +if (provider->node == clkspec->np) + clk = provider->get (clkspec, provider->data); +if (clk) +break ; + } +mutex_unlock (&of_clk_lock); + +return clk; +}
Signature · 2 lines
Regards, Shawn
Re: [PATCH v3 1/4] clk: add DT clock binding support
Quoted 9 lines · Shawn Guo
drivers/clk/clk.c int clk_notifier_unregister(struct clk *clk, struct notifier_block *nb) · line 1630
Quoted 4 lines · Rob Herring
drivers/clk/clk.c +4
-0+struct clk *of_clk_get_from_provider (struct of_phandle_args *clkspec) +{ +struct of_clk_provider *provider; +struct clk *clk =NULL ;Both clk and clkdev treat NULL as a valid clock and return
ERR_PTRfor error case, while all the codes in this patch just return NULL for error and check (clk != NULL) for valid clock.
Because Grant hates ERR_PTR... :)
Quoted 2 lines
Should we force the consistent behavior between DT and non-DT on this?
Yes, I agree and will change it.
Rob
Quoted 15 lines
Quoted 14 lines
drivers/clk/clk.c +13
-0+ +/* Check if we have such a provider in our array */ +mutex_lock (&of_clk_lock); +list_for_each_entry (provider, &of_clk_providers, link) { +if (provider->node == clkspec->np) + clk = provider->get (clkspec, provider->data); +if (clk) +break ; + } +mutex_unlock (&of_clk_lock); + +return clk; +}
PATCH v6 1/1 clk: add DT clock binding support
In reply to Rob HerringFrom: Grant Likely <[email protected]>
Based on work 1st by Ben Herrenschmidt and Jeremy Kerr, then by Grant Likely, this patch adds support to clk_get to allow drivers to retrieve clock data from the device tree.
Platforms scan for clocks in DT with of_clk_init and a match table, and the register a provider through of_clk_add_provider. The provider's clk_src_get function will be called when a device references the provider's OF node for a clock reference.
v6 (Rob Herring):
- Return error values instead of NULL to match clock framework expectations
v5 (Rob Herring):
- Move from drivers/of into common clock subsystem
- Squashed "dt/clock: add a simple provider get function" and "dt/clock: add function to get parent clock name"
- Rebase to 3.4-rc1
- Drop CONFIG_OF_CLOCK and just use CONFIG_OF
- Add missing EXPORT_SYMBOL to various functions
- s/clock-output-name/clock-output-names/
- Define that fixed-clock binding is a single output
v4 (Rob Herring):
- Rework for common clk subsystem
- Add of_clk_get_parent_name function
v3: - Clarified documentation
v2: - fixed errant ';' causing compile error
- Editorial fixes from Shawn Guo
- merged in adding lookup to clkdev
- changed property names to match established convention. After working with the binding a bit it really made more sense to follow the lead of 'reg', 'gpios' and 'interrupts' by making the input simply 'clocks' & 'clock-names' instead of 'clock-input-*', and to only use clock-output* for the producer nodes. (Sorry Shawn, this will mean you need to change some code, but it should be trivial)
- Add ability to inherit clocks from parent nodes by using an empty 'clock-ranges' property. Useful for busses. I could use some feedback on the new property name, 'clock-ranges' doesn't feel right to me.
--- .../devicetree/bindings/clock/clock-bindings.txt | 117 ++++++++++++++++ .../devicetree/bindings/clock/fixed-clock.txt | 21 +++ drivers/clk/clk.c | 140 ++++++++++++++++++++ drivers/clk/clkdev.c | 77 +++++++++++ include/linux/clk-provider.h | 14 ++ include/linux/clk.h | 19 +++ 6 files changed, 388 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/clock-bindings.txt create mode 100644 Documentation/devicetree/bindings/clock/fixed-clock.txt
Documentation/devicetree/bindings/clock/clock-bindings.txt new file +117 -0
@@ -0,0 +1,117 @@ +This binding is a work-in-progress, and are based on some experimental +work by benh[1]. + +Sources of clock signal can be represented by any node in the device +tree. Those nodes are designated as clock providers. Clock consumer +nodes use a phandle and clock specifier pair to connect clock provider +outputs to clock inputs. Similar to the gpio specifiers, a clock +specifier is an array of one more more cells identifying the clock +output on a device. The length of a clock specifier is defined by the +value of a #clock-cells property in the clock provider node. + +[1] http://patchwork.ozlabs.org/patch/31551/ + +==Clock providers== + +Required properties: +#clock-cells: Number of cells in a clock specifier; Typically 0 for nodes + with a single clock output and 1 for nodes with multiple + clock outputs. + +Optional properties: +clock-output-names: Recommended to be a list of strings of clock output signal + names indexed by the first cell in the clock specifier. + However, the meaning of clock-output-names is domain + specific to the clock provider, and is only provided to + encourage using the same meaning for the majority of clock + providers. This format may not work for clock providers + using a complex clock specifier format. In those cases it + is recommended to omit this property and create a binding + specific names property. + + Clock consumer nodes must never directly reference + the provider's clock-output-names property. + +For example: + + oscillator { + #clock-cells = <1>; + clock-output-names = "ckil", "ckih"; + }; + +- this node defines a device with two clock outputs, the first named + "ckil" and the second named "ckih". Consumer nodes always reference + clocks by index. The names should reflect the clock output signal + names for the device. + +==Clock consumers== + +Required properties: +clocks: List of phandle and clock specifier pairs, one pair + for each clock input to the device. Note: if the + clock provider specifies '0' for #clock-cells, then + only the phandle portion of the pair will appear. + +Optional properties: +clock-names: List of clock input name strings sorted in the same + order as the clocks property. Consumers drivers + will use clock-names to match clock input names + with clocks specifiers. +clock-ranges: Empty property indicating that child nodes can inherit named + clocks from this node. Useful for bus nodes to provide a + clock to their children. + +For example: + + device { + clocks = <&osc 1>, <&ref 0>; + clock-names = "baud", "register"; + }; + + +This represents a device with two clock inputs, named "baud" and "register". +The baud clock is connected to output 1 of the &osc device, and the register +clock is connected to output 0 of the &ref. + +==Example== + + /* external oscillator */ + osc: oscillator { + compatible = "fixed-clock"; + #clock-cells = <1>; + clock-frequency = <32678>; + clock-output-names = "osc"; + }; + + /* phase-locked-loop device, generates a higher frequency clock + * from the external oscillator reference */ + pll: pll at 4c000 { + compatible = "vendor,some-pll-interface" + #clock-cells = <1>; + clocks = <&osc 0>; + clock-names = "ref"; + reg = <0x4c000 0x1000>; + clock-output-names = "pll", "pll-switched"; + }; + + /* UART, using the low frequency oscillator for the baud clock, + * and the high frequency switched PLL output for register + * clocking */ + uart at a000 { + compatible = "fsl,imx-uart"; + reg = <0xa000 0x1000>; + interrupts = <33>; + clocks = <&osc 0>, <&pll 1>; + clock-names = "baud", "register"; + }; + +This DT fragment defines three devices: an external oscillator to provide a +low-frequency reference clock, a PLL device to generate a higher frequency +clock signal, and a UART. + +* The oscillator is fixed-frequency, and provides one clock output, named "osc". +* The PLL is both a clock provider and a clock consumer. It uses the clock + signal generated by the external oscillator, and provides two output signals + ("pll" and "pll-switched"). +* The UART has its baud clock connected the external oscillator and its + register clock connected to the PLL clock (the "pll-switched" signal)
Documentation/devicetree/bindings/clock/fixed-clock.txt new file +21 -0
@@ -0,0 +1,21 @@ +Binding for simple fixed-rate clock sources. + +This binding uses the common clock binding[1]. + +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt + +Required properties: +- compatible : shall be "fixed-clock". +- #clock-cells : from common clock binding; shall be set to 0. +- clock-frequency : frequency of clock in Hz. Should be a single cell. + +Optional properties: +- gpios : From common gpio binding; gpio connection to clock enable pin. +- clock-output-names : From common clock binding. + +Example: + clock { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <1000000000>; + };
drivers/clk/clk.c +140 -0
149 lines not on this page · raw message
drivers/clk/clkdev.c +77 -0
93 lines not on this page · raw message
include/linux/clk-provider.h +14 -0
@@ -334,5 +334,19 @@ void __clk_unprepare(struct clk *clk);void __clk_reparent (struct clk *clk,struct clk *new_parent);unsigned long __clk_round_rate (struct clk *clk,unsigned long rate); +struct of_device_id ; + +typedef void (*of_clk_init_cb_t )(struct device_node *); + +int of_clk_add_provider (struct device_node *np, +struct clk *(*clk_src_get)(struct of_phandle_args *args, +void *data), +void *data); +void of_clk_del_provider (struct device_node *np); +struct clk *of_clk_src_simple_get (struct of_phandle_args *clkspec, +void *data); +const char *of_clk_get_parent_name (struct device_node *np,int index); +void of_clk_init (const struct of_device_id *matches); +#endif /* CONFIG_COMMON_CLK */ #endif /* CLK_PROVIDER_H */
include/linux/clk.h +19 -0
@@ -310,4 +310,23 @@ struct clk *clk_get_sys(const char *dev_id, const char *con_id);int clk_add_alias (const char *alias,const char *alias_dev_name,char *id,struct device *dev); +struct device_node ; +struct of_phandle_args ; + +#ifdef CONFIG_OF +struct clk *of_clk_get (struct device_node *np,int index); +struct clk *of_clk_get_by_name (struct device_node *np,const char *name); +struct clk *of_clk_get_from_provider (struct of_phandle_args *clkspec); +#else +static inline struct clk *of_clk_get (struct device_node *np,int index) +{ +return NULL ; +} +static inline struct clk *of_clk_get_by_name (struct device_node *np, +const char *name) +{ +return NULL ; +} +#endif +#endif
Signature · 1 line
1.7.9.5
PATCH v3 2/4 clk: add DT fixed-clock binding support
In reply to Rob HerringFrom: Grant Likely <[email protected]>
Add support for DT "fixed-clock" binding to the common fixed rate clock support.
[Rob Herring] Rework and move into common clock infrastructure
--- drivers/clk/clk-fixed-rate.c | 23 +++++++++++++++++++++++ include/linux/clk-provider.h | 2 ++ 2 files changed, 25 insertions(+)
drivers/clk/clk-fixed-rate.c +23 -0
@@ -14,6 +14,7 @@#include <linux/slab.h> #include <linux/io.h> #include <linux/err.h> +#include <linux/of.h> /* * DOC: basic fixed-rate clock that cannot gate
@@ -79,3 +80,25 @@ struct clk *clk_register_fixed_rate(struct device *dev, const char *name,return clk; } + +#ifdef CONFIG_OF +/** +* of_fixed_clk_setup() - Setup function for simple fixed rate clock +*/ +void __init of_fixed_clk_setup (struct device_node *node) +{ +struct clk *clk; +const char *clk_name = node->name; +u32 rate; + +if (of_property_read_u32 (node,"clock-frequency" , &rate)) +return ; + +of_property_read_string (node,"clock-output-names" , &clk_name); + + clk =clk_register_fixed_rate (NULL , clk_name,NULL ,CLK_IS_ROOT , rate); +if (clk) +of_clk_add_provider (node, of_clk_src_simple_get, clk); +} +EXPORT_SYMBOL_GPL (of_fixed_clk_setup); +#endif
include/linux/clk-provider.h +2 -0
@@ -171,6 +171,8 @@ struct clk *clk_register_fixed_rate(struct device *dev, const char *name, const char *parent_name, unsigned long flags, unsigned long fixed_rate); +void of_fixed_clk_setup (struct device_node *np); +/** * struct clk_gate - gating clock *
Signature · 1 line
1.7.9.5
PATCH v3 3/4 dt: add clock binding doc to primecell bindings
In reply to Rob HerringFrom: Rob Herring <[email protected]>
Add clock binding information for primecell peripherals. For most, a clock input name of "apb_pclk" is required. Any primecell peripherals which are different will need to be documented separately.
--- .../devicetree/bindings/arm/primecell.txt | 6 ++++++ 1 file changed, 6 insertions(+)
Documentation/devicetree/bindings/arm/primecell.txt +6 -0
@@ -13,11 +13,17 @@ Required properties: Optional properties: - arm,primecell-periphid : Value to override the h/w value with +- clocks : From common clock binding. First clock is phandle to clock for apb + pclk. Additional clocks are optional and specific to those peripherals. +- clock-names : From common clock binding. Shall be "apb_pclk" for first clock. Example: serial at fff36000 { compatible = "arm,pl011", "arm,primecell"; arm,primecell-periphid = <0x00341011>; + clocks = <&pclk>; + clock-names = "apb_pclk"; + };
Signature · 1 line
1.7.9.5
PATCH v3 3/4 dt: add clock binding doc to primecell bindings
Quoted 5 lines · Rob Herring
From: Rob Herring<[email protected]>
Add clock binding information for primecell peripherals. For most, a clock input name of "
apb_pclk" is required. Any primecell peripherals which are different will need to be documented separately.
Really they should just use NULL as their bus clock name, and the drivers be fixed for this.
Signature · 2 lines
Ben Dooks http://www.codethink.co.uk/ Senior Engineer Codethink - Providing Genius
PATCH v3 3/4 dt: add clock binding doc to primecell bindings
Quoted 9 lines · Ben Dooks
Quoted 5 lines · Rob Herring
From: Rob Herring<[email protected]>
Add clock binding information for primecell peripherals. For most, a clock input name of "
apb_pclk" is required. Any primecell peripherals which are different will need to be documented separately.Really they should just use NULL as their bus clock name, and the drivers be fixed for this.
Ben,
Do you know what you're talking about, because I don't think you do. You seem to be making random spontaneous comments without really knowing the background behind things.
In this case, apb_pclk is the APB PCLK signal, as opposed to any _functional_ clock required by the primecell peripheral. To use NULL there when the primecell probably _also_ takes a functional clock is absurd, and to go on to suggest that the bus layer should be using NULL here is insane.
We really do not want the bus layer picking up on the functional clock "by accident" because someone forgot to give it. Using a non-NULL name here is entirely appropriate.
Please take some time to research the things that you're commenting on before commenting on them, to avoid leading people down inappropriate paths.
Thank you.
PATCH v3 4/4 clk: add highbank clock support
In reply to Rob HerringFrom: Rob Herring <[email protected]>
This adds real clock support to Calxeda Highbank SOC using the common clock infrastructure.
--- .../devicetree/bindings/clock/calxeda.txt | 17 + arch/arm/Kconfig | 1 + arch/arm/boot/dts/highbank.dts | 91 +++++- arch/arm/mach-highbank/Makefile | 2 +- arch/arm/mach-highbank/clock.c | 62 ---- arch/arm/mach-highbank/highbank.c | 7 + drivers/clk/Makefile | 1 + drivers/clk/clk-highbank.c | 345 ++++++++++++++++++++ 8 files changed, 462 insertions(+), 64 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/calxeda.txt delete mode 100644 arch/arm/mach-highbank/clock.c create mode 100644 drivers/clk/clk-highbank.c
Documentation/devicetree/bindings/clock/calxeda.txt new file +17 -0
@@ -0,0 +1,17 @@ +Device Tree Clock bindings for Calxeda highbank platform + +This binding uses the common clock binding[1]. + +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt + +Required properties: +- compatible : shall be one of the following: + "calxeda,hb-pll-clock" - for a PLL clock + "calxeda,hb-a9periph-clock" - The A9 peripheral clock divided from the + A9 clock. + "calxeda,hb-a9bus-clock" - The A9 bus clock divided from the A9 clock. + "calxeda,hb-emmc-clock" - Divided clock for MMC/SD controller. +- reg : shall be the control register offset from SYSREGs base for the clock. +- clocks : shall be the input parent clock phandle for the clock. This is + either an oscillator or a pll output. +- #clock-cells : from common clock binding; shall be set to 0.
arch/arm/Kconfig +1 -0
@@ -348,6 +348,7 @@ config ARCH_HIGHBANK select ARM_TIMER_SP804 select CACHE_L2X0 select CLKDEV_LOOKUP + select COMMON_CLK select CPU_V7 select GENERIC_CLOCKEVENTS select HAVE_ARM_SCU
arch/arm/boot/dts/highbank.dts +90 -1
203 lines not on this page · raw message
arch/arm/mach-highbank/Makefile +1 -1
@@ -1,4 +1,4 @@ -obj-y := clock.o highbank.o system.o +obj-y := highbank.o system.o obj-$(CONFIG_DEBUG_HIGHBANK_UART) += lluart.o obj-$(CONFIG_SMP) += platsmp.o obj-$(CONFIG_HOTPLUG_CPU) += hotplug.o
arch/arm/mach-highbank/clock.c deleted +0 -62
62 lines not on this page · raw message
arch/arm/mach-highbank/highbank.c +7 -0
@@ -91,6 +91,11 @@ static void __init highbank_init_irq(void)l2x0_of_init (0 , ~0UL ); } +static struct clk_lookup lookup = { + .dev_id ="sp804" , + .con_id =NULL , +}; +static void __init highbank_timer_init (void ) {int irq;
@@ -108,6 +113,8 @@ static void __init highbank_timer_init(void) irq =irq_of_parse_and_map (np,0 );highbank_clocks_init (); + lookup.clk =of_clk_get (np,0 ); +clkdev_add (&lookup);sp804_clocksource_and_sched_clock_init (timer_base +0x20 ,"timer1" );sp804_clockevents_init (timer_base, irq,"timer0" );
drivers/clk/Makefile +1 -0
@@ -3,5 +3,6 @@ obj-$(CONFIG_CLKDEV_LOOKUP) += clkdev.o obj-$(CONFIG_COMMON_CLK) += clk.o clk-fixed-rate.o clk-gate.o \ clk-mux.o clk-divider.o clk-fixed-factor.o # SoCs specific +obj-$(CONFIG_ARCH_HIGHBANK) += clk-highbank.o obj-$(CONFIG_ARCH_MXS) += mxs/ obj-$(CONFIG_PLAT_SPEAR) += spear/
drivers/clk/clk-highbank.c new file +345 -0
345 lines not on this page · raw message
Signature · 1 line
1.7.9.5
Quoted 10 lines · Rob Herring
From: Rob Herring <[email protected]>
This series defines clock bindings for Device-Tree and adds kernel support using the common clock infrastructure. The last patch enables DT clock support for the Calxeda Highbank platform.
I'm posting this again to solicit further review. There has been some discussion[1], but no definite path forward. This series is not changed from the last post other than rebasing to v3.5-rc2.
Hi Rob,
Just FYI, I was following the first thread but it did not seem to reach resolution. Namely Saravana seemed to have some lingering concerns. But I'm definitely not ignoring this series. I assume you still want this to go through me once everyone is on board?
Regards,
Mike
Quoted 12 lines · Mike Turquette
Quoted 10 lines · Rob Herring
From: Rob Herring <[email protected]>
This series defines clock bindings for Device-Tree and adds kernel support using the common clock infrastructure. The last patch enables DT clock support for the Calxeda Highbank platform.
I'm posting this again to solicit further review. There has been some discussion[1], but no definite path forward. This series is not changed from the last post other than rebasing to v3.5-rc2.
Quoted 6 lines
Hi Rob,
Just FYI, I was following the first thread but it did not seem to reach resolution. Namely Saravana seemed to have some lingering concerns. But I'm definitely not ignoring this series. I assume you still want this to go through me once everyone is on board?
Right. This is why I have reposted and copied those whom commented on my pull request. I think at least some of Saravana's concerns boiled down to not requiring using DT clock bindings and not requiring driver changes to move to DT bindings. Both of these are met with this series. A platform can still use clkdev tables for binding clocks to devices and does not have to use DT for clocks. Drivers generally should not require any changes that I'm aware of, but if there are any concrete examples I'd like to know.
Yes, I think this should go thru you.
Rob
Quoted 11 lines · Rob Herring
Right. This is why I have reposted and copied those whom commented on my pull request. I think at least some of Saravana's concerns boiled down to not requiring using DT clock bindings and not requiring driver changes to move to DT bindings. Both of these are met with this series. A platform can still use clkdev tables for binding clocks to devices and does not have to use DT for clocks. Drivers generally should not require any changes that I'm aware of, but if there are any concrete examples I'd like to know.
Yes, I think this should go thru you.
I still have a little doubt about how this binding will best work for imx clock. I'm trying to cook a RFC patch discussing.
Signature · 2 lines
Regards, Shawn
Quoted 6 lines · Rob Herring
From: Rob Herring <[email protected]>
This series defines clock bindings for Device-Tree and adds kernel support using the common clock infrastructure. The last patch enables DT clock support for the Calxeda Highbank platform.
Quoted 8 lines
I'm posting this again to solicit further review. There has been some discussion[1], but no definite path forward. This series is not changed from the last post other than rebasing to v3.5-rc2.
Rob
[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2012-May/100932.html
How would this binding work for clocks which can take a spinlock pointer as a init parameter (eg. clk-gate).
Cheers,
Peter.
Peter,
Quoted 11 lines · Peter De Schrijver
Quoted 10 lines · Rob Herring
From: Rob Herring <[email protected]>
This series defines clock bindings for Device-Tree and adds kernel support using the common clock infrastructure. The last patch enables DT clock support for the Calxeda Highbank platform.
I'm posting this again to solicit further review. There has been some discussion[1], but no definite path forward. This series is not changed from the last post other than rebasing to v3.5-rc2.
Quoted 7 lines
Quoted 4 lines
Rob
[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2012-May/100932.html
How would this binding work for clocks which can take a spinlock pointer as a init parameter (eg. clk-gate).
Other than fixed clocks, this makes no attempt at generic bindings. There is still per clock init required which can allocate a spinlock if necessary. If you look at the highbank code, it would be trivial to initialize spinlocks as needed.
Rob
Quoted 3 lines
Cheers,
Peter.
Quoted 4 lines · Rob Herring
I'm posting this again to solicit further review. There has been some discussion[1], but no definite path forward. This series is not changed from the last post other than rebasing to v3.5-rc2.
Hi Rob,
Per your comment[1], the patch below takes imx6q as example to define single CCM node with a whole bunch of outputs to support clk lookup with device tree. (Only uart and usdhc clocks are being put there for demonstration.) Though it seems working, going through the patch you will see a couple problems which may need to be solved to make the binding useful for cases like imx.
* Different clk matching behavior between DT and non-DT lookup
Let's take usdhc example (drivers/mmc/host/sdhci-esdhc-imx.c) to elaborate the problem. The driver is being shared by several imx SoCs, imx25, imx35, imx5 and imx6. It needs to get 3 clocks below.
imx_data->clk_ipg = devm_clk_get(&pdev->dev, "ipg"); imx_data->clk_ahb = devm_clk_get(&pdev->dev, "ahb"); imx_data->clk_per = devm_clk_get(&pdev->dev, "per");
But not every single imx SoC clock tree implementation has all these 3 clocks for sdhc available. The imx6q happens to have only "per" clock, and clk-imx6q driver registers one clkdev for each usdhc instance with con_id being NULL.
clk_register_clkdev(clk[usdhc1], NULL, "2190000.usdhc"); clk_register_clkdev(clk[usdhc2], NULL, "2194000.usdhc"); clk_register_clkdev(clk[usdhc3], NULL, "2198000.usdhc"); clk_register_clkdev(clk[usdhc4], NULL, "219c000.usdhc");
The non-DT lookup will match all those three clk_get calls to that single clkdev entry. That said, the NULL con_id is treated as wildcard. When translating above clk_register_clkdev calls into DT lookup, I would expect having "clock-names" absent should just work with all those 3 clk_get calls to get <&clks 2>.
usdhc at 02190000 {
...
clocks = <&clks 2>;
};But with the current of_clk implementation, we will have to have something like below to keep the usdhc behavior same as non-DT lookup.
usdhc at 02190000 {
...
clocks = <&clks 2>, <&clks 2>, <&clks 2>;
clock-names = "ipg", "ahb", "per";
};Can we force all the clk_get calls with different con_id to get the only clk listed in "clocks" when "clock-names" is absent, so that we can somehow match the behavior with non-DT lookup?
* phandle argument is not easy for engineering
As we will have a whole bunch of clock outputs listed in ccm node, which will be referenced by peripheral phandle in form of <&clks index>. When the list gets long, it becomes hard for people to find the correct index number for the clock referenced.
Regards,
Shawn
[1] http://thread.gmane.org/gmane.linux.kernel/1300259/focus=1301107
8<---
.../devicetree/bindings/clk/clk-imx6q.txt | 58 ++++++++++++++++++++ arch/arm/boot/dts/imx6q.dtsi | 27 +++++++++- arch/arm/mach-imx/clk-imx6q.c | 31 ++++++----- 3 files changed, 101 insertions(+), 15 deletions(-) create mode 100644 Documentation/devicetree/bindings/clk/clk-imx6q.txt
Documentation/devicetree/bindings/clk/clk-imx6q.txt new file +58 -0
@@ -0,0 +1,58 @@ +* Clock bindings for Freescale i.MX6Q + +Required properties: +- compatible: Should be "fsl,imx6q-ccm" +- reg: Address and length of the register set +- interrupts: Should contain CCM interrupt +- #clock-cells: Should be <1> +- clock-output-names: A list of clock output that CCM provides. The valid + clock names include: + + dummy, ckil, ckih, osc, pll2_pfd0_352m, pll2_pfd1_594m, pll2_pfd2_396m, + pll3_pfd0_720m, pll3_pfd1_540m, pll3_pfd2_508m, pll3_pfd3_454m, + pll2_198m, pll3_120m, pll3_80m, pll3_60m, twd, step, pll1_sw, + periph_pre, periph2_pre, periph_clk2_sel, periph2_clk2_sel, axi_sel, + esai_sel, asrc_sel, spdif_sel, gpu2d_axi, gpu3d_axi, gpu2d_core_sel, + gpu3d_core_sel, gpu3d_shader_sel, ipu1_sel, ipu2_sel, ldb_di0_sel, + ldb_di1_sel, ipu1_di0_pre_sel, ipu1_di1_pre_sel, ipu2_di0_pre_sel, + ipu2_di1_pre_sel, ipu1_di0_sel, ipu1_di1_sel, ipu2_di0_sel, + ipu2_di1_sel, hsi_tx_sel, pcie_axi_sel, ssi1_sel, ssi2_sel, ssi3_sel, + usdhc1_sel, usdhc2_sel, usdhc3_sel, usdhc4_sel, enfc_sel, emi_sel, + emi_slow_sel, vdo_axi_sel, vpu_axi_sel, cko1_sel, periph, periph2, + periph_clk2, periph2_clk2, ipg, ipg_per, esai_pred, esai_podf, + asrc_pred, asrc_podf, spdif_pred, spdif_podf, can_root, ecspi_root, + gpu2d_core_podf, gpu3d_core_podf, gpu3d_shader, ipu1_podf, ipu2_podf, + ldb_di0_podf, ldb_di1_podf, ipu1_di0_pre, ipu1_di1_pre, ipu2_di0_pre, + ipu2_di1_pre, hsi_tx_podf, ssi1_pred, ssi1_podf, ssi2_pred, ssi2_podf, + ssi3_pred, ssi3_podf, uart_serial_podf, usdhc1_podf, usdhc2_podf, + usdhc3_podf, usdhc4_podf, enfc_pred, enfc_podf, emi_podf, + emi_slow_podf, vpu_axi_podf, cko1_podf, axi, mmdc_ch0_axi_podf, + mmdc_ch1_axi_podf, arm, ahb, apbh_dma, asrc, can1_ipg, can1_serial, + can2_ipg, can2_serial, ecspi1, ecspi2, ecspi3, ecspi4, ecspi5, enet, + esai, gpt_ipg, gpt_ipg_per, gpu2d_core, gpu3d_core, hdmi_iahb, + hdmi_isfr, i2c1, i2c2, i2c3, iim, enfc, ipu1, ipu1_di0, ipu1_di1, ipu2, + ipu2_di0, ldb_di0, ldb_di1, ipu2_di1, hsi_tx, mlb, mmdc_ch0_axi, + mmdc_ch1_axi, ocram, openvg_axi, pcie_axi, pwm1, pwm2, pwm3, pwm4, + gpmi_bch_apb, gpmi_bch, gpmi_io, gpmi_apb, sata, sdma, spba, ssi1, + ssi2, ssi3, uart_ipg, uart_serial, usboh3, usdhc1, usdhc2, usdhc3, + usdhc4, vdo_axi, vpu_axi, cko1, pll1_sys, pll2_bus, pll3_usb_otg, + pll4_audio, pll5_video, pll6_mlb, pll7_usb_host, pll8_enet, ssi1_ipg, + ssi2_ipg, ssi3_ipg, rom, + + But generally, only the clocks that peripheral nodes have a phandle pointing + to need to be listed there. + +Examples: + +clks: ccm at 020c4000 { + compatible = "fsl,imx6q-ccm"; + reg = <0x020c4000 0x4000>; + interrupts = <0 87 0x04 0 88 0x04>; + #clock-cells = <1>; + clock-output-names = "uart_ipg", + "uart_serial", + "usdhc1", + "usdhc2", + "usdhc3", + "usdhc4"; +};
arch/arm/boot/dts/imx6q.dtsi +26 -1
@@ -169,6 +169,8 @@ compatible = "fsl,imx6q-uart", "fsl,imx21-uart"; reg = <0x02020000 0x4000>; interrupts = <0 26 0x04>; + clocks = <&clks 0>, <&clks 1>; + clock-names = "ipg", "per"; status = "disabled"; };
@@ -348,10 +350,17 @@ status = "disabled"; }; - ccm at 020c4000 { + clks: ccm at 020c4000 { compatible = "fsl,imx6q-ccm"; reg = <0x020c4000 0x4000>; interrupts = <0 87 0x04 0 88 0x04>; + #clock-cells = <1>; + clock-output-names = "uart_ipg", + "uart_serial", + "usdhc1", + "usdhc2", + "usdhc3", + "usdhc4"; }; anatop at 020c8000 {
@@ -589,6 +598,8 @@ compatible = "fsl,imx6q-usdhc"; reg = <0x02190000 0x4000>; interrupts = <0 22 0x04>; + clocks = <&clks 2>, <&clks 2>, <&clks 2>; + clock-names = "ipg", "ahb", "per"; status = "disabled"; };
@@ -596,6 +607,8 @@ compatible = "fsl,imx6q-usdhc"; reg = <0x02194000 0x4000>; interrupts = <0 23 0x04>; + clocks = <&clks 3>, <&clks 3>, <&clks 3>; + clock-names = "ipg", "ahb", "per"; status = "disabled"; };
@@ -603,6 +616,8 @@ compatible = "fsl,imx6q-usdhc"; reg = <0x02198000 0x4000>; interrupts = <0 24 0x04>; + clocks = <&clks 4>, <&clks 4>, <&clks 4>; + clock-names = "ipg", "ahb", "per"; status = "disabled"; };
@@ -610,6 +625,8 @@ compatible = "fsl,imx6q-usdhc"; reg = <0x0219c000 0x4000>; interrupts = <0 25 0x04>; + clocks = <&clks 5>, <&clks 5>, <&clks 5>; + clock-names = "ipg", "ahb", "per"; status = "disabled"; };
@@ -700,6 +717,8 @@ compatible = "fsl,imx6q-uart", "fsl,imx21-uart"; reg = <0x021e8000 0x4000>; interrupts = <0 27 0x04>; + clocks = <&clks 0>, <&clks 1>; + clock-names = "ipg", "per"; status = "disabled"; };
@@ -707,6 +726,8 @@ compatible = "fsl,imx6q-uart", "fsl,imx21-uart"; reg = <0x021ec000 0x4000>; interrupts = <0 28 0x04>; + clocks = <&clks 0>, <&clks 1>; + clock-names = "ipg", "per"; status = "disabled"; };
@@ -714,6 +735,8 @@ compatible = "fsl,imx6q-uart", "fsl,imx21-uart"; reg = <0x021f0000 0x4000>; interrupts = <0 29 0x04>; + clocks = <&clks 0>, <&clks 1>; + clock-names = "ipg", "per"; status = "disabled"; };
@@ -721,6 +744,8 @@ compatible = "fsl,imx6q-uart", "fsl,imx21-uart"; reg = <0x021f4000 0x4000>; interrupts = <0 30 0x04>; + clocks = <&clks 0>, <&clks 1>; + clock-names = "ipg", "per"; status = "disabled"; }; };
arch/arm/mach-imx/clk-imx6q.c +17 -14
@@ -57,6 +57,21 @@ static void __iomem *ccm_base;void __init imx6q_clock_map_io (void ) { } +static struct clk *imx_clk_src_get (struct of_phandle_args *clkspec, +void *data) +{ +const char *clk_name; +int idx = clkspec->args[0 ]; +int ret; + + ret =of_property_read_string_index (clkspec->np,"clock-output-names" , + idx, &clk_name); +if (ret <0 ) +return ERR_PTR (ret); + +return __clk_lookup (clk_name); +} +int imx6q_set_lpm (enum mxc_cpu_pwr_mode mode) {u32 val =readl_relaxed (ccm_base +CLPCR );
@@ -391,21 +406,7 @@ int __init mx6q_clocks_init(void)clk_register_clkdev (clk[gpt_ipg],"ipg" ,"imx-gpt.0" );clk_register_clkdev (clk[gpt_ipg_per],"per" ,"imx-gpt.0" );clk_register_clkdev (clk[twd],NULL ,"smp_twd" ); -clk_register_clkdev (clk[uart_serial],"per" ,"2020000.serial" ); -clk_register_clkdev (clk[uart_ipg],"ipg" ,"2020000.serial" ); -clk_register_clkdev (clk[uart_serial],"per" ,"21e8000.serial" ); -clk_register_clkdev (clk[uart_ipg],"ipg" ,"21e8000.serial" ); -clk_register_clkdev (clk[uart_serial],"per" ,"21ec000.serial" ); -clk_register_clkdev (clk[uart_ipg],"ipg" ,"21ec000.serial" ); -clk_register_clkdev (clk[uart_serial],"per" ,"21f0000.serial" ); -clk_register_clkdev (clk[uart_ipg],"ipg" ,"21f0000.serial" ); -clk_register_clkdev (clk[uart_serial],"per" ,"21f4000.serial" ); -clk_register_clkdev (clk[uart_ipg],"ipg" ,"21f4000.serial" );clk_register_clkdev (clk[enet],NULL ,"2188000.ethernet" ); -clk_register_clkdev (clk[usdhc1],NULL ,"2190000.usdhc" ); -clk_register_clkdev (clk[usdhc2],NULL ,"2194000.usdhc" ); -clk_register_clkdev (clk[usdhc3],NULL ,"2198000.usdhc" ); -clk_register_clkdev (clk[usdhc4],NULL ,"219c000.usdhc" );clk_register_clkdev (clk[i2c1],NULL ,"21a0000.i2c" );clk_register_clkdev (clk[i2c2],NULL ,"21a4000.i2c" );clk_register_clkdev (clk[i2c3],NULL ,"21a8000.i2c" );
@@ -422,6 +423,8 @@ int __init mx6q_clocks_init(void)clk_register_clkdev (clk[ahb],"ahb" ,NULL );clk_register_clkdev (clk[cko1],"cko1" ,NULL ); +of_clk_add_provider (np, imx_clk_src_get,NULL ); +for (i =0 ; i <ARRAY_SIZE (clks_init_on); i++)clk_prepare_enable (clk[clks_init_on[i]]);
Signature · 1 line
1.7.5.4
Quoted 13 lines · Shawn Guo
Quoted 4 lines · Rob Herring
I'm posting this again to solicit further review. There has been some discussion[1], but no definite path forward. This series is not changed from the last post other than rebasing to v3.5-rc2.
Hi Rob,
Per your comment[1], the patch below takes imx6q as example to define single CCM node with a whole bunch of outputs to support clk lookup with device tree. (Only uart and usdhc clocks are being put there for demonstration.) Though it seems working, going through the patch you will see a couple problems which may need to be solved to make the binding useful for cases like imx.
...
Quoted 6 lines
* phandle argument is not easy for engineering
As we will have a whole bunch of clock outputs listed in ccm node, which will be referenced by peripheral phandle in form of <&clks index>. When the list gets long, it becomes hard for people to find the correct index number for the clock referenced.
You can (well, probably /should/) list the clock IDs as well as names in the binding documentation. Then, there's no guess-work or counting involved. I did this in a very early Tegra clock binding proposal:
http://patchwork.ozlabs.org/patch/141359/
Hopefully, dtc will grow named-constants or a pre-processor sometime and help this too.
Quoted 45 lines · Shawn Guo
Quoted 4 lines · Rob Herring
I'm posting this again to solicit further review. There has been some discussion[1], but no definite path forward. This series is not changed from the last post other than rebasing to v3.5-rc2.
Hi Rob,
Per your comment[1], the patch below takes imx6q as example to define single CCM node with a whole bunch of outputs to support clk lookup with device tree. (Only uart and usdhc clocks are being put there for demonstration.) Though it seems working, going through the patch you will see a couple problems which may need to be solved to make the binding useful for cases like imx.
* Different clk matching behavior between DT and non-DT lookup
Let's take usdhc example (
drivers/mmc/host/sdhci-esdhc-imx.c) to elaborate the problem. The driver is being shared by several imx SoCs, imx25, imx35, imx5 and imx6. It needs to get 3 clocks below.imx_data->clk_ipg = devm_clk_get(&pdev->dev, "ipg"); imx_data->clk_ahb = devm_clk_get(&pdev->dev, "ahb"); imx_data->clk_per = devm_clk_get(&pdev->dev, "per");But not every single imx SoC clock tree implementation has all these 3 clocks for sdhc available. The imx6q happens to have only "per" clock, and clk-imx6q driver registers one clkdev for each usdhc instance with
con_idbeing NULL.clk_register_clkdev(clk[usdhc1], NULL, "2190000.usdhc"); clk_register_clkdev(clk[usdhc2], NULL, "2194000.usdhc"); clk_register_clkdev(clk[usdhc3], NULL, "2198000.usdhc"); clk_register_clkdev(clk[usdhc4], NULL, "219c000.usdhc");The non-DT lookup will match all those three
clk_getcalls to that single clkdev entry. That said, the NULLcon_idis treated as wildcard. When translating aboveclk_register_clkdevcalls into DT lookup, I would expect having "clock-names" absent should just work with all those 3clk_getcalls to get <&clks 2>.usdhc at 02190000 { ... clocks = <&clks 2>; };
Quoted 8 lines
But with the current
of_clkimplementation, we will have to have something like below to keep the usdhc behavior same as non-DT lookup.usdhc at 02190000 { ... clocks = <&clks 2>, <&clks 2>, <&clks 2>; clock-names = "ipg", "ahb", "per"; };
This looks correct to me. The module always has 3 clock inputs, but some chips may hook up the same clock to all 3 inputs. If you had different versions of the module, then that would imply different compatible strings.
Which clock input is which should be defined by the order listed and the names are supposed to be optional, but the current clk_get API doesn't have any way to specify the index. So either you have to put in the names or convert the driver to lookup by index. It's really no different than irqs.
Quoted 11 lines
Can we force all the
clk_getcalls with differentcon_idto get the only clk listed in "clocks" when "clock-names" is absent, so that we can somehow match the behavior with non-DT lookup?* phandle argument is not easy for engineering
As we will have a whole bunch of clock outputs listed in ccm node, which will be referenced by peripheral phandle in form of <&clks index>. When the list gets long, it becomes hard for people to find the correct index number for the clock referenced.
As Stephen pointed out, you just have to document them until we have defines in dts.
Rob
Quoted 238 lines
Regards,
Shawn[1] http://thread.gmane.org/gmane.linux.kernel/1300259/focus=1301107
8<---
.../devicetree/bindings/clk/clk-imx6q.txt | 58 ++++++++++++++++++++ arch/arm/boot/dts/imx6q.dtsi | 27 +++++++++- arch/arm/mach-imx/clk-imx6q.c | 31 ++++++----- 3 files changed, 101 insertions(+), 15 deletions(-) create mode 100644 Documentation/devicetree/bindings/clk/clk-imx6q.txtDocumentation/devicetree/bindings/clk/clk-imx6q.txt new file +58
-0@@ -0,0 +1,58 @@ +* Clock bindings for Freescale i.MX6Q + +Required properties: +- compatible: Should be "fsl,imx6q-ccm" +- reg: Address and length of the register set +- interrupts: Should contain CCM interrupt +- #clock-cells: Should be <1> +- clock-output-names: A list of clock output that CCM provides. The valid + clock names include: + + dummy, ckil, ckih, osc, pll2_pfd0_352m, pll2_pfd1_594m, pll2_pfd2_396m, + pll3_pfd0_720m, pll3_pfd1_540m, pll3_pfd2_508m, pll3_pfd3_454m, + pll2_198m, pll3_120m, pll3_80m, pll3_60m, twd, step, pll1_sw, + periph_pre, periph2_pre, periph_clk2_sel, periph2_clk2_sel, axi_sel, + esai_sel, asrc_sel, spdif_sel, gpu2d_axi, gpu3d_axi, gpu2d_core_sel, + gpu3d_core_sel, gpu3d_shader_sel, ipu1_sel, ipu2_sel, ldb_di0_sel, + ldb_di1_sel, ipu1_di0_pre_sel, ipu1_di1_pre_sel, ipu2_di0_pre_sel, + ipu2_di1_pre_sel, ipu1_di0_sel, ipu1_di1_sel, ipu2_di0_sel, + ipu2_di1_sel, hsi_tx_sel, pcie_axi_sel, ssi1_sel, ssi2_sel, ssi3_sel, + usdhc1_sel, usdhc2_sel, usdhc3_sel, usdhc4_sel, enfc_sel, emi_sel, + emi_slow_sel, vdo_axi_sel, vpu_axi_sel, cko1_sel, periph, periph2, + periph_clk2, periph2_clk2, ipg, ipg_per, esai_pred, esai_podf, + asrc_pred, asrc_podf, spdif_pred, spdif_podf, can_root, ecspi_root, + gpu2d_core_podf, gpu3d_core_podf, gpu3d_shader, ipu1_podf, ipu2_podf, + ldb_di0_podf, ldb_di1_podf, ipu1_di0_pre, ipu1_di1_pre, ipu2_di0_pre, + ipu2_di1_pre, hsi_tx_podf, ssi1_pred, ssi1_podf, ssi2_pred, ssi2_podf, + ssi3_pred, ssi3_podf, uart_serial_podf, usdhc1_podf, usdhc2_podf, + usdhc3_podf, usdhc4_podf, enfc_pred, enfc_podf, emi_podf, + emi_slow_podf, vpu_axi_podf, cko1_podf, axi, mmdc_ch0_axi_podf, + mmdc_ch1_axi_podf, arm, ahb, apbh_dma, asrc, can1_ipg, can1_serial, + can2_ipg, can2_serial, ecspi1, ecspi2, ecspi3, ecspi4, ecspi5, enet, + esai, gpt_ipg, gpt_ipg_per, gpu2d_core, gpu3d_core, hdmi_iahb, + hdmi_isfr, i2c1, i2c2, i2c3, iim, enfc, ipu1, ipu1_di0, ipu1_di1, ipu2, + ipu2_di0, ldb_di0, ldb_di1, ipu2_di1, hsi_tx, mlb, mmdc_ch0_axi, + mmdc_ch1_axi, ocram, openvg_axi, pcie_axi, pwm1, pwm2, pwm3, pwm4, + gpmi_bch_apb, gpmi_bch, gpmi_io, gpmi_apb, sata, sdma, spba, ssi1, + ssi2, ssi3, uart_ipg, uart_serial, usboh3, usdhc1, usdhc2, usdhc3, + usdhc4, vdo_axi, vpu_axi, cko1, pll1_sys, pll2_bus, pll3_usb_otg, + pll4_audio, pll5_video, pll6_mlb, pll7_usb_host, pll8_enet, ssi1_ipg, + ssi2_ipg, ssi3_ipg, rom, + + But generally, only the clocks that peripheral nodes have a phandle pointing + to need to be listed there. + +Examples: + +clks: ccm at 020c4000 { + compatible = "fsl,imx6q-ccm"; + reg = <0x020c4000 0x4000>; + interrupts = <0 87 0x04 0 88 0x04>; + #clock-cells = <1>; + clock-output-names = "uart_ipg", + "uart_serial", + "usdhc1", + "usdhc2", + "usdhc3", + "usdhc4"; +};arch/arm/boot/dts/imx6q.dtsi +26
-1@@ -169,6 +169,8 @@ compatible = "fsl,imx6q-uart", "fsl,imx21-uart"; reg = <0x02020000 0x4000>; interrupts = <0 26 0x04>; + clocks = <&clks 0>, <&clks 1>; + clock-names = "ipg", "per"; status = "disabled"; };@@ -348,10 +350,17 @@ status = "disabled"; }; - ccm at 020c4000 { + clks: ccm at 020c4000 { compatible = "fsl,imx6q-ccm"; reg = <0x020c4000 0x4000>; interrupts = <0 87 0x04 0 88 0x04>; + #clock-cells = <1>; + clock-output-names = "uart_ipg", + "uart_serial", + "usdhc1", + "usdhc2", + "usdhc3", + "usdhc4"; }; anatop at 020c8000 {@@ -589,6 +598,8 @@ compatible = "fsl,imx6q-usdhc"; reg = <0x02190000 0x4000>; interrupts = <0 22 0x04>; + clocks = <&clks 2>, <&clks 2>, <&clks 2>; + clock-names = "ipg", "ahb", "per"; status = "disabled"; };@@ -596,6 +607,8 @@ compatible = "fsl,imx6q-usdhc"; reg = <0x02194000 0x4000>; interrupts = <0 23 0x04>; + clocks = <&clks 3>, <&clks 3>, <&clks 3>; + clock-names = "ipg", "ahb", "per"; status = "disabled"; };@@ -603,6 +616,8 @@ compatible = "fsl,imx6q-usdhc"; reg = <0x02198000 0x4000>; interrupts = <0 24 0x04>; + clocks = <&clks 4>, <&clks 4>, <&clks 4>; + clock-names = "ipg", "ahb", "per"; status = "disabled"; };@@ -610,6 +625,8 @@ compatible = "fsl,imx6q-usdhc"; reg = <0x0219c000 0x4000>; interrupts = <0 25 0x04>; + clocks = <&clks 5>, <&clks 5>, <&clks 5>; + clock-names = "ipg", "ahb", "per"; status = "disabled"; };@@ -700,6 +717,8 @@ compatible = "fsl,imx6q-uart", "fsl,imx21-uart"; reg = <0x021e8000 0x4000>; interrupts = <0 27 0x04>; + clocks = <&clks 0>, <&clks 1>; + clock-names = "ipg", "per"; status = "disabled"; };@@ -707,6 +726,8 @@ compatible = "fsl,imx6q-uart", "fsl,imx21-uart"; reg = <0x021ec000 0x4000>; interrupts = <0 28 0x04>; + clocks = <&clks 0>, <&clks 1>; + clock-names = "ipg", "per"; status = "disabled"; };@@ -714,6 +735,8 @@ compatible = "fsl,imx6q-uart", "fsl,imx21-uart"; reg = <0x021f0000 0x4000>; interrupts = <0 29 0x04>; + clocks = <&clks 0>, <&clks 1>; + clock-names = "ipg", "per"; status = "disabled"; };@@ -721,6 +744,8 @@ compatible = "fsl,imx6q-uart", "fsl,imx21-uart"; reg = <0x021f4000 0x4000>; interrupts = <0 30 0x04>; + clocks = <&clks 0>, <&clks 1>; + clock-names = "ipg", "per"; status = "disabled"; }; };arch/arm/mach-imx/clk-imx6q.c +17
-14@@ -57,6 +57,21 @@ static void __iomem *ccm_base;void __init imx6q_clock_map_io (void ) { } +static struct clk *imx_clk_src_get (struct of_phandle_args *clkspec, +void *data) +{ +const char *clk_name; +int idx = clkspec->args[0 ]; +int ret; + + ret =of_property_read_string_index (clkspec->np,"clock-output-names" , + idx, &clk_name); +if (ret <0 ) +return ERR_PTR (ret); + +return __clk_lookup (clk_name); +} +int imx6q_set_lpm (enum mxc_cpu_pwr_mode mode) {u32 val =readl_relaxed (ccm_base +CLPCR );@@ -391,21 +406,7 @@ int __init mx6q_clocks_init(void)clk_register_clkdev (clk[gpt_ipg],"ipg" ,"imx-gpt.0" );clk_register_clkdev (clk[gpt_ipg_per],"per" ,"imx-gpt.0" );clk_register_clkdev (clk[twd],NULL ,"smp_twd" ); -clk_register_clkdev (clk[uart_serial],"per" ,"2020000.serial" ); -clk_register_clkdev (clk[uart_ipg],"ipg" ,"2020000.serial" ); -clk_register_clkdev (clk[uart_serial],"per" ,"21e8000.serial" ); -clk_register_clkdev (clk[uart_ipg],"ipg" ,"21e8000.serial" ); -clk_register_clkdev (clk[uart_serial],"per" ,"21ec000.serial" ); -clk_register_clkdev (clk[uart_ipg],"ipg" ,"21ec000.serial" ); -clk_register_clkdev (clk[uart_serial],"per" ,"21f0000.serial" ); -clk_register_clkdev (clk[uart_ipg],"ipg" ,"21f0000.serial" ); -clk_register_clkdev (clk[uart_serial],"per" ,"21f4000.serial" ); -clk_register_clkdev (clk[uart_ipg],"ipg" ,"21f4000.serial" );clk_register_clkdev (clk[enet],NULL ,"2188000.ethernet" ); -clk_register_clkdev (clk[usdhc1],NULL ,"2190000.usdhc" ); -clk_register_clkdev (clk[usdhc2],NULL ,"2194000.usdhc" ); -clk_register_clkdev (clk[usdhc3],NULL ,"2198000.usdhc" ); -clk_register_clkdev (clk[usdhc4],NULL ,"219c000.usdhc" );clk_register_clkdev (clk[i2c1],NULL ,"21a0000.i2c" );clk_register_clkdev (clk[i2c2],NULL ,"21a4000.i2c" );clk_register_clkdev (clk[i2c3],NULL ,"21a8000.i2c" );@@ -422,6 +423,8 @@ int __init mx6q_clocks_init(void)clk_register_clkdev (clk[ahb],"ahb" ,NULL );clk_register_clkdev (clk[cko1],"cko1" ,NULL ); +of_clk_add_provider (np, imx_clk_src_get,NULL ); +for (i =0 ; i <ARRAY_SIZE (clks_init_on); i++)clk_prepare_enable (clk[clks_init_on[i]]);
Hi Rob,
Quoted 7 lines · Rob Herring
This series defines clock bindings for Device-Tree and adds kernel support using the common clock infrastructure. The last patch enables DT clock support for the Calxeda Highbank platform.
I'm posting this again to solicit further review. There has been some discussion[1], but no definite path forward. This series is not changed from the last post other than rebasing to v3.5-rc2.
This is a very useful patchset, thanks! Mitch Bradley and I have been hooking up the mach-mmp ARM subarchitecture to it and we needed to make a few changes, mainly because mach-mmp doesn't use COMMON_CLK. The changes are:
1) Remove the COMMON_CLK dependency that the of_clk_* functions have by moving them into a new file, drivers/clk/clk-of.c. The OF functions in drivers/clk/clk.c aren't dependent on anything else in clk.c (since you pass them a struct clk) so they can be moved out easily. The of_clk_* entries in clk-provider.h move to clk.h.
2) Use alloc_bootmem() instead of kzalloc() in of_clk_add_provider(), because we need to set up clocks during .init_early on ARM (which happens pre-slab) so that they are available for platform init.
I'll send our patches in a reply to this mail. Are you okay with these changes, and would you like to include them in your patchset?
Thanks very much,
- Chris.
Signature · 2 lines
Chris Ball <[email protected]> <http://printf.net/> One Laptop Per Child
PATCH 1/2 clk: Refactor of_clk_* into a new clk-of.c
This removes the dependency on CONFIG_COMMON_CLK, which is unnecessary for the of_clk_* functions -- these functions are passed a struct clk, which can either be a COMMON_CLK struct or an arch-specific one.
--- drivers/clk/Makefile | 2 + drivers/clk/clk-of.c | 167 ++++++++++++++++++++++++++++++++++++++++++ drivers/clk/clk.c | 139 ----------------------------------- include/linux/clk-provider.h | 14 ---- include/linux/clk.h | 17 ++++- 5 files changed, 184 insertions(+), 155 deletions(-) create mode 100644 drivers/clk/clk-of.c
drivers/clk/Makefile +2 -0
@@ -2,6 +2,8 @@ obj-$(CONFIG_CLKDEV_LOOKUP) += clkdev.o obj-$(CONFIG_COMMON_CLK) += clk.o clk-fixed-rate.o clk-gate.o \ clk-mux.o clk-divider.o clk-fixed-factor.o +obj-$(CONFIG_OF) += clk-of.o + # SoCs specific obj-$(CONFIG_ARCH_MXS) += mxs/ obj-$(CONFIG_PLAT_SPEAR) += spear/
drivers/clk/clk-of.c new file +167 -0
167 lines not on this page · raw message
drivers/clk/clk.c +0 -139
142 lines not on this page · raw message
include/linux/clk-provider.h +0 -14
@@ -336,19 +336,5 @@ void __clk_unprepare(struct clk *clk);void __clk_reparent (struct clk *clk,struct clk *new_parent);unsigned long __clk_round_rate (struct clk *clk,unsigned long rate); -struct of_device_id ; - -typedef void (*of_clk_init_cb_t )(struct device_node *); - -int of_clk_add_provider (struct device_node *np, -struct clk *(*clk_src_get)(struct of_phandle_args *args, -void *data), -void *data); -void of_clk_del_provider (struct device_node *np); -struct clk *of_clk_src_simple_get (struct of_phandle_args *clkspec, -void *data); -const char *of_clk_get_parent_name (struct device_node *np,int index); -void of_clk_init (const struct of_device_id *matches); -#endif /* CONFIG_COMMON_CLK */ #endif /* CLK_PROVIDER_H */
include/linux/clk.h +15 -2
@@ -312,8 +312,21 @@ int clk_add_alias(const char *alias, const char *alias_dev_name, char *id,struct device_node ;struct of_phandle_args ; +struct of_device_id ;#ifdef CONFIG_OF +typedef void (*of_clk_init_cb_t )(struct device_node *); + +int of_clk_add_provider (struct device_node *np, +struct clk *(*clk_src_get)(struct of_phandle_args *args, +void *data), +void *data); +void of_clk_del_provider (struct device_node *np); +struct clk *of_clk_src_simple_get (struct of_phandle_args *clkspec, +void *data); +const char *of_clk_get_parent_name (struct device_node *np,int index); +void of_clk_init (const struct of_device_id *matches); +struct clk *of_clk_get (struct device_node *np,int index);struct clk *of_clk_get_by_name (struct device_node *np,const char *name);struct clk *of_clk_get_from_provider (struct of_phandle_args *clkspec);
@@ -327,6 +340,6 @@ static inline struct clk *of_clk_get_by_name(struct device_node *np, {return NULL ; } -#endif +#endif /* CONFIG_OF */ -#endif +#endif /* __LINUX_CLK_H */
Signature · 2 lines
Chris Ball <[email protected]> <http://printf.net/> One Laptop Per Child
PATCH 2/2 clk: clk-of: Use alloc_bootmem() instead of kzalloc()
In reply to Chris BallWhen of_clk_add_provider() is used at boot time (during .init_early on ARM, in the motivating case for this patch), kzalloc() cannot be used because slab isn't up yet.
--- drivers/clk/clk-of.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/clk/clk-of.c +2 -1
@@ -22,6 +22,7 @@#include <linux/list.h> #include <linux/slab.h> #include <linux/of.h> +#include <linux/bootmem.h> #ifdef CONFIG_OF
@@ -67,7 +68,7 @@ int of_clk_add_provider(struct device_node *np, {struct of_clk_provider *cp; - cp =kzalloc (sizeof (struct of_clk_provider ),GFP_KERNEL ); + cp =alloc_bootmem (sizeof (struct of_clk_provider ));if (!cp)return -ENOMEM ;
Signature · 2 lines
Chris Ball <[email protected]> <http://printf.net/> One Laptop Per Child
Re: [PATCH 02/02] clk: clk-of: Use alloc_bootmem() instead of kzalloc()
Quoted 5 lines · Chris Ball
When
of_clk_add_provider()is used at boot time (during .init_early on ARM, in the motivating case for this patch),kzalloc()cannot be used because slab isn't up yet.Signed-off-by Chris Ball <[email protected]>
I wouldn't use alloc_bootmem() unconditionally for this, as you have no idea where other platforms may wire the call site up. slab is also available a lot earlier now than it used to be, so many places that required bootmem pages previously can get away with slab allocations now.
You could rework this as:
if (slab_is_available()) cp = kzalloc(...) else cp = alloc_bootmem(...)
and then it doesn't matter when and where it gets called.
Chris,
Quoted 16 lines · Chris Ball
Hi Rob,
Quoted 7 lines · Rob Herring
This series defines clock bindings for Device-Tree and adds kernel support using the common clock infrastructure. The last patch enables DT clock support for the Calxeda Highbank platform.
I'm posting this again to solicit further review. There has been some discussion[1], but no definite path forward. This series is not changed from the last post other than rebasing to v3.5-rc2.
This is a very useful patchset, thanks! Mitch Bradley and I have been hooking up the mach-mmp ARM subarchitecture to it and we needed to make a few changes, mainly because mach-mmp doesn't use
COMMON_CLK. The changes are:
Quoted 5 lines
1) Remove the
COMMON_CLKdependency that the of_clk_* functions have by moving them into a new file,drivers/clk/clk-of.c. The OF functions indrivers/clk/clk.caren't dependent on anything else inclk.c(since you pass them a struct clk) so they can be moved out easily. The of_clk_* entries inclk-provider.hmove toclk.h.
Well, if you look at prior versions, it was separate being in drivers/of/. I'm not sure we want to encourage/allow not converting to COMMON_CLK. I would consider not using COMMON_CLK is deprecated. Having a single definition of struct clk is the primary reason for COMMON_CLK. What is the reason for not converting mmp to COMMON_CLK? It appears to be a pretty trivial clock implementation, and will be required for multi-platform kernels.
Quoted 4 lines
2) Use
alloc_bootmem()instead ofkzalloc()inof_clk_add_provider(), because we need to set up clocks during .init_early on ARM (which happens pre-slab) so that they are available for platform init.
This depends on 1 as the common clock code would have the same issue. Generally, the first place clocks are needed is the timer init. At that point, you can call kzalloc. This is where all the clock init used to be done until init_early was added and some platforms have moved their clock init. I don't think there was really ever much reason to move it other than to make the timer init function only deal with timer setup.
Is alloc_bootmem available on all arches? There's 3 occurrences in drivers/* which tells me that is the wrong thing to do.
Rob
Quoted 6 lines
I'll send our patches in a reply to this mail. Are you okay with these changes, and would you like to include them in your patchset?
Thanks very much,
- Chris.
Quoted 6 lines · Rob Herring
Quoted 4 lines · Chris Ball
2) Use
alloc_bootmem()instead ofkzalloc()inof_clk_add_provider(), because we need to set up clocks during .init_early on ARM (which happens pre-slab) so that they are available for platform init.
Quoted 7 lines
This depends on 1 as the common clock code would have the same issue. Generally, the first place clocks are needed is the timer init. At that point, you can call kzalloc. This is where all the clock init used to be done until
init_earlywas added and some platforms have moved their clock init. I don't think there was really ever much reason to move it other than to make the timer init function only deal with timer setup.
Hi Rob,
Just FYI I've been looking at using alloc_bootmem in the common clk code as a way to get rid of the static initialization stuff (which only existed due to very early initialization of timers).
The suggested change above to of_clk_add_provider would jive well with my change to the common clk code.
Regards,
Mike
Quoted 15 lines · Mike Turquette
Quoted 6 lines · Rob Herring
Quoted 4 lines · Chris Ball
2) Use
alloc_bootmem()instead ofkzalloc()inof_clk_add_provider(), because we need to set up clocks during .init_early on ARM (which happens pre-slab) so that they are available for platform init.Quoted 7 lines
This depends on 1 as the common clock code would have the same issue. Generally, the first place clocks are needed is the timer init. At that point, you can call kzalloc. This is where all the clock init used to be done until
init_earlywas added and some platforms have moved their clock init. I don't think there was really ever much reason to move it other than to make the timer init function only deal with timer setup.
Quoted 5 lines
Hi Rob,
Just FYI I've been looking at using
alloc_bootmemin the common clk code as a way to get rid of the static initialization stuff (which only existed due to very early initialization of timers).
The slab is up at the time timers are initialized. The only real problem is mixing clock init into the timer init functions and clk init in init_early is cleaner in that regard.
Quoted 2 lines
The suggested change above to
of_clk_add_providerwould jive well with my change to the common clk code.
Are you planning this for 3.6? If not, then this can be addressed at the time the clk framework supports bootmem.
I'm not so sure more users of bootmem are desired. There seems to be some effort/desire to remove it:
http://lists.linux-foundation.org/pipermail/ksummit-2012-discuss/2012-June/000562.html
https://lkml.org/lkml/2012/3/13/586
http://lists.infradead.org/pipermail/linux-arm-kernel/2011-December/074886.html
Rob
Quoted 18 lines · Rob Herring
Quoted 17 lines · Mike Turquette
Quoted 6 lines · Rob Herring
Quoted 4 lines · Chris Ball
2) Use
alloc_bootmem()instead ofkzalloc()inof_clk_add_provider(), ? ?because we need to set up clocks during .init_early on ARM (which ? ?happens pre-slab) so that they are available for platform init.Quoted 7 lines
This depends on 1 as the common clock code would have the same issue. Generally, the first place clocks are needed is the timer init. At that point, you can call kzalloc. This is where all the clock init used to be done until
init_earlywas added and some platforms have moved their clock init. I don't think there was really ever much reason to move it other than to make the timer init function only deal with timer setup.Hi Rob,
Quoted 8 lines
Quoted 3 lines
Just FYI I've been looking at using
alloc_bootmemin the common clk code as a way to get rid of the static initialization stuff (which only existed due to very early initialization of timers).The slab is up at the time timers are initialized. The only real problem is mixing clock init into the timer init functions and clk init in
init_earlyis cleaner in that regard.
Hi Rob,
I'm afraid I don't follow you when you say "slab is up at the time timers are initialized". On some platforms I believe that timer initialization takes place before slab is ready (.early_init).
Quoted 6 lines
Quoted 2 lines
The suggested change above to
of_clk_add_providerwould jive well with my change to the common clk code.Are you planning this for 3.6? If not, then this can be addressed at the time the clk framework supports bootmem.
Quoted 6 lines
I'm not so sure more users of bootmem are desired. There seems to be some effort/desire to remove it:
http://lists.linux-foundation.org/pipermail/ksummit-2012-discuss/2012-June/000562.html
https://lkml.org/lkml/2012/3/13/586
http://lists.infradead.org/pipermail/linux-arm-kernel/2011-December/074886.html
So after looking at those mails it seems using memblock_alloc would be a better alternative. Did I get that right?
Thanks,
Mike
Quoted 2 lines
Rob
Mike,
Quoted 28 lines · Mike
Quoted 18 lines · Rob Herring
Quoted 17 lines · Mike Turquette
Quoted 6 lines · Rob Herring
Quoted 4 lines · Chris Ball
2) Use
alloc_bootmem()instead ofkzalloc()inof_clk_add_provider(), because we need to set up clocks during .init_early on ARM (which happens pre-slab) so that they are available for platform init.Quoted 7 lines
This depends on 1 as the common clock code would have the same issue. Generally, the first place clocks are needed is the timer init. At that point, you can call kzalloc. This is where all the clock init used to be done until
init_earlywas added and some platforms have moved their clock init. I don't think there was really ever much reason to move it other than to make the timer init function only deal with timer setup.Hi Rob,
Quoted 8 lines
Quoted 3 lines
Just FYI I've been looking at using
alloc_bootmemin the common clk code as a way to get rid of the static initialization stuff (which only existed due to very early initialization of timers).The slab is up at the time timers are initialized. The only real problem is mixing clock init into the timer init functions and clk init in
init_earlyis cleaner in that regard.
Quoted 5 lines
Hi Rob,
I'm afraid I don't follow you when you say "slab is up at the time timers are initialized". On some platforms I believe that timer initialization takes place before slab is ready (.early_init).
Timers are not initialized in the .timer init hook? The clock init is definitely in init_early in some cases, but I don't think the actual timer init can be.
Quoted 9 lines
Quoted 9 lines
Quoted 2 lines
The suggested change above to
of_clk_add_providerwould jive well with my change to the common clk code.Are you planning this for 3.6? If not, then this can be addressed at the time the clk framework supports bootmem.
I'm not so sure more users of bootmem are desired. There seems to be some effort/desire to remove it:
Quoted 6 lines
Quoted 3 lines
http://lists.linux-foundation.org/pipermail/ksummit-2012-discuss/2012-June/000562.html
https://lkml.org/lkml/2012/3/13/586
http://lists.infradead.org/pipermail/linux-arm-kernel/2011-December/074886.htmlSo after looking at those mails it seems using
memblock_allocwould be a better alternative. Did I get that right?
Seems so, but my view is avoiding it altogether is the right way to me just based on how few users there are of either within drivers/.
Anyway, this is all really besides the point for this series. If and when the clock framework supports bootmem or memblock, then the DT clk code can easily be adapted as part of that. It's just a matter of timing and if you're not planning bootmem/memblock for 3.6 we can add this later.
Rob
Quoted 6 lines
Thanks,
MikeQuoted 2 lines
Rob