ARM: Versatile irq and clock DT support
Subsystems (7)
cover ARM: Versatile irq and clock DT support
From: Rob Herring <[email protected]>
This series is some preparation work to fully convert Versatile platform to DT. This converts the IRQ initialization to DT based init and adds clocks to the Versatile DT. This doesn't actually enable using common clock framework, but that is only a matter of removing the old clock code and enabling the appropriate Kconfig options. This will be done when Versatile can be completely converted over to DT boot. I have that working in my tree, but full DT conversion is also dependent on DT PCI support.
I've tested under QEMU and Linus Walleij has tested an earlier version on Versatile AB h/w.
Rob
Rob Herring (10):
dt/bindings: add passthru-mask property to versatile-fpga-irq
dts: versatile: add missing irq controller properties
irqchip: versatile-fpga: add pass-thru enable support
irqchip: versatile-fpga: add DT init
ARM: versatile: remove init_irq hook for DT boot
dts: versatile: add pl180 compatible strings dt/bindings: arm-boards: add binding for Versatile core module dt/bindings: add compatible string for versatile osc clock
dts: versatile: add clock tree
clk: versatile: add versatile OSC support
Documentation/devicetree/bindings/arm/arm-boards | 6 ++ .../devicetree/bindings/arm/versatile-fpga-irq.txt | 3 + .../devicetree/bindings/clock/arm-integrator.txt | 4 +- arch/arm/boot/dts/versatile-ab.dts | 81 +++++++++++++++++++++- arch/arm/boot/dts/versatile-pb.dts | 12 +++- arch/arm/mach-versatile/versatile_dt.c | 1 - drivers/clk/versatile/Makefile | 3 +- drivers/clk/versatile/clk-integrator.c | 35 +++++++++- drivers/irqchip/irq-versatile-fpga.c | 9 +++ 9 files changed, 145 insertions(+), 9 deletions(-)
Signature · 1 line
1.9.1
PATCH 1/10 dt/bindings: add passthru-mask property to versatile-fpga-irq
From: Rob Herring <[email protected]>
Add a passthru-mask property for setting interrupts which are passed through directly to a primary controller.
--- Documentation/devicetree/bindings/arm/versatile-fpga-irq.txt | 3 +++ 1 file changed, 3 insertions(+)
Documentation/devicetree/bindings/arm/versatile-fpga-irq.txt +3 -0
@@ -34,3 +34,6 @@ Optional properties: - interrupts: if the FPGA IRQ controller is cascaded, i.e. if its IRQ output is simply connected to the input of another IRQ controller, then the parent IRQ shall be specified in this property. +- passthru-mask: a u32 number representing a bit mas determining which of + the interrupts are directly passed through to the primary interrupt + controller.
Signature · 1 line
1.9.1
PATCH 1/10 dt/bindings: add passthru-mask property to versatile-fpga-irq
Quoted 19 lines · Rob Herring
From: Rob Herring <[email protected]>
Add a passthru-mask property for setting interrupts which are passed through directly to a primary controller.
Signed-off-by Rob Herring <[email protected]>Cc Pawel Moll <[email protected]>Cc Mark Rutland <[email protected]>Cc Ian Campbell <[email protected]>Cc Kumar Gala <[email protected]>--- Documentation/devicetree/bindings/arm/versatile-fpga-irq.txt | 3 +++ 1 file changed, 3 insertions(+)Documentation/devicetree/bindings/arm/versatile-fpga-irq.txt +0
-0
Documentation/devicetree/bindings/arm/versatile-fpga-irq.txt Optional properties: · line 37
Quoted 4 lines
Documentation/devicetree/bindings/arm/versatile-fpga-irq.txt +1
-0@@ -34,3 +34,6 @@ Optional properties: - interrupts: if the FPGA IRQ controller is cascaded, i.e. if its IRQ output is simply connected to the input of another IRQ controller, then the parent IRQ shall be specified in this property. +- passthru-mask: a u32 number representing a bit mas determining which of
bit mask
(speling)
Documentation/devicetree/bindings/arm/versatile-fpga-irq.txt Optional properties: · line 39
Quoted 2 lines
Documentation/devicetree/bindings/arm/versatile-fpga-irq.txt +2
-0+ the interrupts are directly passed through to the primary interrupt + controller.
This is very confusing on the Integrators. The FPGA IRQ controller *is* the primary interrupt controller on these.
(Further on Integrators with an IM-PD1 expansion board the VIC is actually the secondary controller visavis the FPGA IRQ controller which is the primary one.)
So on the versatile the FPGA IRQ controller is *not* cascaded off one line of the VIC but rather connected in parallel or something?
Care to elaborate a bit on how things are cascaded here? And specify that this is for some special usecase or something?
I'll read through the patches and see if I can wrap my head around this.
Yours,
Linus Walleij
PATCH 1/10 dt/bindings: add passthru-mask property to versatile-fpga-irq
Quoted 28 lines · Linus Walleij
Quoted 19 lines · Rob Herring
From: Rob Herring <[email protected]>
Add a passthru-mask property for setting interrupts which are passed through directly to a primary controller.
Signed-off-by Rob Herring <[email protected]>Cc Pawel Moll <[email protected]>Cc Mark Rutland <[email protected]>Cc Ian Campbell <[email protected]>Cc Kumar Gala <[email protected]>--- Documentation/devicetree/bindings/arm/versatile-fpga-irq.txt | 3 +++ 1 file changed, 3 insertions(+)Documentation/devicetree/bindings/arm/versatile-fpga-irq.txt +0
-0Documentation/devicetree/bindings/arm/versatile-fpga-irq.txt Optional properties: · line 37
Quoted 4 lines
Documentation/devicetree/bindings/arm/versatile-fpga-irq.txt +1
-0@@ -34,3 +34,6 @@ Optional properties: - interrupts: if the FPGA IRQ controller is cascaded, i.e. if its IRQ output is simply connected to the input of another IRQ controller, then the parent IRQ shall be specified in this property. +- passthru-mask: a u32 number representing a bit mas determining which ofbit mask
Quoted 7 lines
(speling)
Documentation/devicetree/bindings/arm/versatile-fpga-irq.txt Optional properties: · line 39
Quoted 2 lines
Documentation/devicetree/bindings/arm/versatile-fpga-irq.txt +2
-0+ the interrupts are directly passed through to the primary interrupt + controller.This is very confusing on the Integrators. The FPGA IRQ controller *is* the primary interrupt controller on these.
Yes. Really, the h/w is not the same and we should have different compatible strings. Unfortunately, it is Integrator that should change.
Quoted 8 lines
(Further on Integrators with an IM-PD1 expansion board the VIC is actually the secondary controller visavis the FPGA IRQ controller which is the primary one.)
So on the versatile the FPGA IRQ controller is *not* cascaded off one line of the VIC but rather connected in parallel or something?
See figure in 3.10 of the user guide:
It is both chained and direct connection. I guess we could just use chained mode to simplify things.
Quoted 3 lines
Care to elaborate a bit on how things are cascaded here? And specify that this is for some special usecase or something?
I'll add a reference to the user guide to the binding.
Rob
PATCH 1/10 dt/bindings: add passthru-mask property to versatile-fpga-irq
Quoted 34 lines · Rob Herring
Quoted 33 lines · Linus Walleij
Quoted 19 lines · Rob Herring
From: Rob Herring <[email protected]>
Add a passthru-mask property for setting interrupts which are passed through directly to a primary controller.
Signed-off-by Rob Herring <[email protected]>Cc Pawel Moll <[email protected]>Cc Mark Rutland <[email protected]>Cc Ian Campbell <[email protected]>Cc Kumar Gala <[email protected]>--- Documentation/devicetree/bindings/arm/versatile-fpga-irq.txt | 3 +++ 1 file changed, 3 insertions(+)Documentation/devicetree/bindings/arm/versatile-fpga-irq.txt +0
-0Documentation/devicetree/bindings/arm/versatile-fpga-irq.txt Optional properties: · line 37
Quoted 4 lines
Documentation/devicetree/bindings/arm/versatile-fpga-irq.txt +1
-0@@ -34,3 +34,6 @@ Optional properties: - interrupts: if the FPGA IRQ controller is cascaded, i.e. if its IRQ output is simply connected to the input of another IRQ controller, then the parent IRQ shall be specified in this property. +- passthru-mask: a u32 number representing a bit mas determining which ofbit mask
(speling)
Quoted 2 lines
Documentation/devicetree/bindings/arm/versatile-fpga-irq.txt +2
-0+ the interrupts are directly passed through to the primary interrupt + controller.
Quoted 6 lines
Quoted 2 lines
This is very confusing on the Integrators. The FPGA IRQ controller *is* the primary interrupt controller on these.
Yes. Really, the h/w is not the same and we should have different compatible strings. Unfortunately, it is Integrator that should change.
I don't know what I was thinking... The compatible strings are different as Versatile uses arm,versatile-sic, so I can drop this passthru-mask.
Rob
PATCH 2/10 dts: versatile: add missing irq controller properties
In reply to Rob HerringFrom: Rob Herring <[email protected]>
Add valid-mask, clear-mask and passthru-mask properties to the dts so the platform code doing the same thing can be removed.
--- arch/arm/boot/dts/versatile-ab.dts | 5 +++++ 1 file changed, 5 insertions(+)
arch/arm/boot/dts/versatile-ab.dts +5 -0
@@ -59,6 +59,8 @@ interrupt-controller; #interrupt-cells = <1>; reg = <0x10140000 0x1000>; + clear-mask = <0xffffffff>; + valid-mask = <0xffffffff>; }; sic: intc at 10003000 {
@@ -68,6 +70,9 @@ reg = <0x10003000 0x1000>; interrupt-parent = <&vic>; interrupts = <31>; /* Cascaded to vic */ + clear-mask = <0xffffffff>; + valid-mask = <0xffc203f8>; + passthru-mask = <0xffd00000>; }; dma at 10130000 {
Signature · 1 line
1.9.1
PATCH 3/10 irqchip: versatile-fpga: add pass-thru enable support
In reply to Rob HerringFrom: Rob Herring <[email protected]>
Set the PIC_ENABLES register when the passthru-mask property is present. This enables interrupts on the secondary controller to be passed thru directly to the primary controller.
--- drivers/irqchip/irq-versatile-fpga.c | 6 ++++++ 1 file changed, 6 insertions(+)
drivers/irqchip/irq-versatile-fpga.c +6 -0
@@ -26,6 +26,8 @@#define FIQ_ENABLE_SET 0x28#define FIQ_ENABLE_CLEAR 0x2C +#define PIC_ENABLES 0x20/* set interrupt pass through bits */ +/** * struct fpga_irq_data - irq data container for the FPGA IRQ controller * @base: memory offset in virtual memory
@@ -185,6 +187,7 @@ int __init fpga_irq_of_init(struct device_node *node,void __iomem *base;u32 clear_mask;u32 valid_mask; +u32 passthru_mask;int parent_irq;if (WARN_ON (!node))
@@ -209,6 +212,9 @@ int __init fpga_irq_of_init(struct device_node *node,writel (clear_mask, base +IRQ_ENABLE_CLEAR );writel (clear_mask, base +FIQ_ENABLE_CLEAR ); +if (!of_property_read_u32 (node,"passthru-mask" , &passthru_mask)) +writel (passthru_mask, base +PIC_ENABLES ); + return 0; }#endif
Signature · 1 line
1.9.1
PATCH 3/10 irqchip: versatile-fpga: add pass-thru enable support
Rob,
Quoted 6 lines · Rob Herring
From: Rob Herring <[email protected]>
Set the
PIC_ENABLESregister when the passthru-mask property is present. This enables interrupts on the secondary controller to be passed thru directly to the primary controller.
Quoted 6 lines
Signed-off-by Rob Herring <[email protected]>Cc Thomas Gleixner <[email protected]>Cc Jason Cooper <[email protected]>--- drivers/irqchip/irq-versatile-fpga.c | 6 ++++++ 1 file changed, 6 insertions(+)
Patches 3 and 4 are good to go. Shall I take them, or are they a depended on by later patches?
thx,
Jason.
PATCH 3/10 irqchip: versatile-fpga: add pass-thru enable support
Quoted 16 lines · Jason Cooper
Rob,
Quoted 6 lines · Rob Herring
From: Rob Herring <[email protected]>
Set the
PIC_ENABLESregister when the passthru-mask property is present. This enables interrupts on the secondary controller to be passed thru directly to the primary controller.Quoted 6 lines
Signed-off-by Rob Herring <[email protected]>Cc Thomas Gleixner <[email protected]>Cc Jason Cooper <[email protected]>--- drivers/irqchip/irq-versatile-fpga.c | 6 ++++++ 1 file changed, 6 insertions(+)
Quoted 2 lines
Patches 3 and 4 are good to go. Shall I take them, or are they a depended on by later patches?
Thanks. There are dependencies, so I plan to send it all thru arm-soc.
Rob
PATCH 3/10 irqchip: versatile-fpga: add pass-thru enable support
Quoted 17 lines · Rob Herring
Quoted 16 lines · Jason Cooper
Rob,
Quoted 12 lines · Rob Herring
From: Rob Herring <[email protected]>
Set the
PIC_ENABLESregister when the passthru-mask property is present. This enables interrupts on the secondary controller to be passed thru directly to the primary controller.Signed-off-by Rob Herring <[email protected]>Cc Thomas Gleixner <[email protected]>Cc Jason Cooper <[email protected]>--- drivers/irqchip/irq-versatile-fpga.c | 6 ++++++ 1 file changed, 6 insertions(+)
Quoted 4 lines
Quoted 2 lines
Patches 3 and 4 are good to go. Shall I take them, or are they a depended on by later patches?
Thanks. There are dependencies, so I plan to send it all thru arm-soc.
Very well. For both:
thx,
Jason.
PATCH 3/10 irqchip: versatile-fpga: add pass-thru enable support
In reply to Rob HerringQuoted 9 lines · Rob Herring
From: Rob Herring <[email protected]>
Set the
PIC_ENABLESregister when the passthru-mask property is present. This enables interrupts on the secondary controller to be passed thru directly to the primary controller.Signed-off-by Rob Herring <[email protected]>Cc Thomas Gleixner <[email protected]>Cc Jason Cooper <[email protected]>
(...)
Quoted 1 line
+#define
PIC_ENABLES0x20/* set interrupt pass through bits */
What register is this? In this hardware register 0x20 is
#define FIQ_STATUS 0x20
Do you mean that this FPGA IRQ controller is an augmented version with the FIQ portions removed and instead it has some special routing register at 0x20?
In that case it should have a different compatible-string should it not?
I was under the impression that this was just a simple cascaded IRQ controller cascaded off bit 31 of the VIC, which is much simpler to handle, maybe in the manner of commits e641b987c20832dfaaa51d7792ed928c2b2d2dbf
"irqchip: support cascaded VICs" f6da9fe45c3074b909084ae9da5f55034ebffeb4
"irqchip: vic: Properly chain the cascaded IRQs"
But for the fpga IRQ driver. Atleast that is how the comments in the device tree says it should work so something is odd here :-/
I really want to know how this special register works.
Yours,
Linus Walleij
PATCH 4/10 irqchip: versatile-fpga: add DT init
In reply to Rob HerringFrom: Rob Herring <[email protected]>
Add IRQCHIP_DECLARE call for the versatile-fpga interrupt controller so that initialization from DT will work.
--- drivers/irqchip/irq-versatile-fpga.c | 3 +++ 1 file changed, 3 insertions(+)
drivers/irqchip/irq-versatile-fpga.c +3 -0
@@ -14,6 +14,8 @@#include <asm/exception.h> #include <asm/mach/irq.h> +#include "irqchip.h" +#define IRQ_STATUS 0x00#define IRQ_RAW_STATUS 0x04#define IRQ_ENABLE_SET 0x08
@@ -217,4 +219,5 @@ int __init fpga_irq_of_init(struct device_node *node, return 0; } +IRQCHIP_DECLARE (arm_fpga,"arm,versatile-sic" , fpga_irq_of_init);#endif
Signature · 1 line
1.9.1
PATCH 4/10 irqchip: versatile-fpga: add DT init
Quoted 8 lines · Rob Herring
From: Rob Herring <[email protected]>
Add
IRQCHIP_DECLAREcall for the versatile-fpga interrupt controller so that initialization from DT will work.Signed-off-by Rob Herring <[email protected]>Cc Thomas Gleixner <[email protected]>Cc Jason Cooper <[email protected]>
If the different compatible string indicated that the hardware is different that string must be used to augment the runpath, right?
It seems to me that the special routing register will be used whenever the DT has this special passthrough property set, but that should be *refused* on all other variants, as that regiser has a different meaning on the integrator (used for FIQ).
Yours,
Linus Walleij
PATCH 5/10 ARM: versatile: remove init_irq hook for DT boot
In reply to Rob HerringFrom: Rob Herring <[email protected]>
Now that versatile's irqchips are initialized from DT, the init_irq hook can be removed.
--- arch/arm/mach-versatile/versatile_dt.c | 1 - 1 file changed, 1 deletion(-)
arch/arm/mach-versatile/versatile_dt.c +0 -1
@@ -44,7 +44,6 @@ static const char *versatile_dt_match[] __initconst = {DT_MACHINE_START (VERSATILE_PB ,"ARM-Versatile (Device Tree Support)" ) .map_io = versatile_map_io, .init_early = versatile_init_early, - .init_irq = versatile_init_irq, .init_machine = versatile_dt_init, .dt_compat = versatile_dt_match, .restart = versatile_restart,
Signature · 1 line
1.9.1
PATCH 6/10 dts: versatile: add pl180 compatible strings
In reply to Rob HerringFrom: Rob Herring <[email protected]>
While not needed for probing, add the "arm,pl180" compatible string for completeness.
--- arch/arm/boot/dts/versatile-ab.dts | 2 +- arch/arm/boot/dts/versatile-pb.dts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
arch/arm/boot/dts/versatile-ab.dts +1 -1
@@ -188,7 +188,7 @@ interrupts = <24>; }; mmc at 5000 { - compatible = "arm,primecell"; + compatible = "arm,pl180", "arm,primecell"; reg = < 0x5000 0x1000>; interrupts-extended = <&vic 22 &sic 2>; };
arch/arm/boot/dts/versatile-pb.dts +1 -1
@@ -39,7 +39,7 @@ interrupts = <5>; }; mmc at b000 { - compatible = "arm,primecell"; + compatible = "arm,pl180", "arm,primecell"; reg = <0xb000 0x1000>; interrupts-extended = <&vic 23 &sic 2>; };
Signature · 1 line
1.9.1
PATCH 6/10 dts: versatile: add pl180 compatible strings
Quoted 6 lines · Rob Herring
From: Rob Herring <[email protected]>
While not needed for probing, add the "arm,pl180" compatible string for completeness.
Signed-off-by Rob Herring <[email protected]>
Yours,
Linus Walleij
PATCH 7/10 dt/bindings: arm-boards: add binding for Versatile core module
In reply to Rob HerringFrom: Rob Herring <[email protected]>
Add binding for the core module found on ARM versatile AB and PB boards.
--- Documentation/devicetree/bindings/arm/arm-boards | 6 ++++++ 1 file changed, 6 insertions(+)
Documentation/devicetree/bindings/arm/arm-boards +6 -0
@@ -86,3 +86,9 @@ Interrupt controllers: compatible = "arm,versatile-sic"; interrupt-controller; #interrupt-cells = <1>; + +Required nodes: + +- core-module: the root node to the Versatile platforms must have + a core-module with regs and the compatible string + "arm,core-module-versatile"
Signature · 1 line
1.9.1
PATCH 7/10 dt/bindings: arm-boards: add binding for Versatile core module
Quoted 6 lines · On Tuesday 20 May 2014 16:09:33 Rob Herring wrote:
Documentation/devicetree/bindings/arm/arm-boards +0
-0
Documentation/devicetree/bindings/arm/arm-boards Interrupt controllers: · line 94
Quoted 9 lines
Documentation/devicetree/bindings/arm/arm-boards +6
-0@@ -86,3 +86,9 @@ Interrupt controllers: compatible = "arm,versatile-sic"; interrupt-controller; #interrupt-cells = <1>; + +Required nodes: + +- core-module: the root node to the Versatile platforms must have + a core-module with regs and the compatible string + "arm,core-module-versatile"
Should we mark this compatible="syscon" as well, so we can move the accesses into syscon references later if we want to?
ARnd
PATCH 7/10 dt/bindings: arm-boards: add binding for Versatile core module
Quoted 12 lines · Arnd Bergmann
Quoted 11 lines · On Tuesday 20 May 2014 16:09:33 Rob Herring wrote:
Documentation/devicetree/bindings/arm/arm-boards +3
-0@@ -86,3 +86,9 @@ Interrupt controllers: compatible = "arm,versatile-sic"; interrupt-controller; #interrupt-cells = <1>; + +Required nodes: +
Quoted 8 lines
Documentation/devicetree/bindings/arm/arm-boards Interrupt controllers: · line 94
Quoted 4 lines
Documentation/devicetree/bindings/arm/arm-boards +3
-0+- core-module: the root node to the Versatile platforms must have + a core-module with regs and the compatible string + "arm,core-module-versatile"Should we mark this compatible="syscon" as well, so we can move the accesses into syscon references later if we want to?
Yes, probably so. That should be done on Integrator and Realview as well if not already.
Rob
PATCH 8/10 dt/bindings: add compatible string for versatile osc clock
In reply to Rob HerringFrom: Rob Herring <[email protected]>
--- Documentation/devicetree/bindings/clock/arm-integrator.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/clock/arm-integrator.txt +2 -2
@@ -1,4 +1,4 @@ -Clock bindings for ARM Integrator Core Module clocks +Clock bindings for ARM Integrator and Versatile Core Module clocks Auxilary Oscillator Clock
@@ -12,7 +12,7 @@ parent node. Required properties: -- compatible: must be "arm,integrator-cm-auxosc" +- compatible: must be "arm,integrator-cm-auxosc" or "arm,versatile-cm-auxosc" - #clock-cells: must be <0> Optional properties:
Signature · 1 line
1.9.1
PATCH 8/10 dt/bindings: add compatible string for versatile osc clock
Quoted 7 lines · Rob Herring
From: Rob Herring <[email protected]>
Signed-off-by Rob Herring <[email protected]>Cc Pawel Moll <[email protected]>Cc Mark Rutland <[email protected]>Cc Ian Campbell <[email protected]>Cc Kumar Gala <[email protected]>
Yours,
Linus Walleij
PATCH 9/10 dts: versatile: add clock tree
In reply to Rob HerringFrom: Rob Herring <[email protected]>
The versatile dts is missing any clock data. Add the clocks.
It is not clear from the documentation where pclk comes from, so for now it is a dummy clock which is sufficient for things to work.
--- arch/arm/boot/dts/versatile-ab.dts | 74 ++++++++++++++++++++++++++++++++++++++ arch/arm/boot/dts/versatile-pb.dts | 10 ++++++ 2 files changed, 84 insertions(+)
arch/arm/boot/dts/versatile-ab.dts +74 -0
@@ -19,6 +19,40 @@ reg = <0x0 0x08000000>; }; + core-module at 10000000 { + compatible = "arm,core-module-versatile"; + reg = <0x10000000 0x200>; + + osc24M: oscillator at 24M { + #clock-cells = <0>; + compatible = "fixed-clock"; + clock-frequency = <24000000>; + }; + + /* OSC1 on AB, OSC4 on PB */ + osc1: cm_aux_osc at 24M { + #clock-cells = <0>; + compatible = "arm,versatile-cm-auxosc"; + clocks = <&osc24M>; + }; + + /* The timer clock is the 24 MHz oscillator divided to 1MHz */ + timclk: timclk at 1M { + #clock-cells = <0>; + compatible = "fixed-factor-clock"; + clock-div = <24>; + clock-mult = <1>; + clocks = <&osc24M>; + }; + + /* Actually hclk ? */ + pclk: pclk at 0 { + #clock-cells = <0>; + compatible = "fixed-clock"; + clock-frequency = <0>; + }; + }; + flash at 34000000 { compatible = "arm,versatile-flash"; reg = <0x34000000 0x4000000>;
@@ -79,63 +113,85 @@ compatible = "arm,pl081", "arm,primecell"; reg = <0x10130000 0x1000>; interrupts = <17>; + clocks = <&pclk>; + clock-names = "apb_pclk"; }; uart0: uart at 101f1000 { compatible = "arm,pl011", "arm,primecell"; reg = <0x101f1000 0x1000>; interrupts = <12>; + clocks = <&osc24M>, <&pclk>; + clock-names = "uartclk", "apb_pclk"; }; uart1: uart at 101f2000 { compatible = "arm,pl011", "arm,primecell"; reg = <0x101f2000 0x1000>; interrupts = <13>; + clocks = <&osc24M>, <&pclk>; + clock-names = "uartclk", "apb_pclk"; }; uart2: uart at 101f3000 { compatible = "arm,pl011", "arm,primecell"; reg = <0x101f3000 0x1000>; interrupts = <14>; + clocks = <&osc24M>, <&pclk>; + clock-names = "uartclk", "apb_pclk"; }; smc at 10100000 { compatible = "arm,primecell"; reg = <0x10100000 0x1000>; + clocks = <&pclk>; + clock-names = "apb_pclk"; }; mpmc at 10110000 { compatible = "arm,primecell"; reg = <0x10110000 0x1000>; + clocks = <&pclk>; + clock-names = "apb_pclk"; }; display at 10120000 { compatible = "arm,pl110", "arm,primecell"; reg = <0x10120000 0x1000>; interrupts = <16>; + clocks = <&osc1>, <&pclk>; + clock-names = "clcd", "apb_pclk"; }; sctl at 101e0000 { compatible = "arm,primecell"; reg = <0x101e0000 0x1000>; + clocks = <&pclk>; + clock-names = "apb_pclk"; }; watchdog at 101e1000 { compatible = "arm,primecell"; reg = <0x101e1000 0x1000>; interrupts = <0>; + clocks = <&pclk>; + clock-names = "apb_pclk"; }; timer at 101e2000 { compatible = "arm,sp804", "arm,primecell"; reg = <0x101e2000 0x1000>; interrupts = <4>; + clocks = <&timclk>, <&pclk>; + clock-names = "tmrclk", "apb_pclk"; }; timer at 101e3000 { compatible = "arm,sp804", "arm,primecell"; reg = <0x101e3000 0x1000>; interrupts = <5>; + clocks = <&timclk>, <&pclk>; + clock-names = "tmrclk", "apb_pclk"; }; gpio0: gpio at 101e4000 {
@@ -146,6 +202,8 @@ #gpio-cells = <2>; interrupt-controller; #interrupt-cells = <2>; + clocks = <&pclk>; + clock-names = "apb_pclk"; }; gpio1: gpio at 101e5000 {
@@ -156,24 +214,32 @@ #gpio-cells = <2>; interrupt-controller; #interrupt-cells = <2>; + clocks = <&pclk>; + clock-names = "apb_pclk"; }; rtc at 101e8000 { compatible = "arm,pl030", "arm,primecell"; reg = <0x101e8000 0x1000>; interrupts = <10>; + clocks = <&pclk>; + clock-names = "apb_pclk"; }; sci at 101f0000 { compatible = "arm,primecell"; reg = <0x101f0000 0x1000>; interrupts = <15>; + clocks = <&pclk>; + clock-names = "apb_pclk"; }; ssp at 101f4000 { compatible = "arm,pl022", "arm,primecell"; reg = <0x101f4000 0x1000>; interrupts = <11>; + clocks = <&osc24M>, <&pclk>; + clock-names = "sspclk", "apb_pclk"; }; fpga {
@@ -186,23 +252,31 @@ compatible = "arm,primecell"; reg = <0x4000 0x1000>; interrupts = <24>; + clocks = <&pclk>; + clock-names = "apb_pclk"; }; mmc at 5000 { compatible = "arm,pl180", "arm,primecell"; reg = < 0x5000 0x1000>; interrupts-extended = <&vic 22 &sic 2>; + clocks = <&osc24M>, <&pclk>; + clock-names = "mclk", "apb_pclk"; }; kmi at 6000 { compatible = "arm,pl050", "arm,primecell"; reg = <0x6000 0x1000>; interrupt-parent = <&sic>; interrupts = <3>; + clocks = <&osc24M>, <&pclk>; + clock-names = "KMIREFCLK", "apb_pclk"; }; kmi at 7000 { compatible = "arm,pl050", "arm,primecell"; reg = <0x7000 0x1000>; interrupt-parent = <&sic>; interrupts = <4>; + clocks = <&osc24M>, <&pclk>; + clock-names = "KMIREFCLK", "apb_pclk"; }; }; };
arch/arm/boot/dts/versatile-pb.dts +10 -0
@@ -13,6 +13,8 @@ #gpio-cells = <2>; interrupt-controller; #interrupt-cells = <2>; + clocks = <&pclk>; + clock-names = "apb_pclk"; }; gpio3: gpio at 101e7000 {
@@ -23,6 +25,8 @@ #gpio-cells = <2>; interrupt-controller; #interrupt-cells = <2>; + clocks = <&pclk>; + clock-names = "apb_pclk"; }; fpga {
@@ -31,17 +35,23 @@ reg = <0x9000 0x1000>; interrupt-parent = <&sic>; interrupts = <6>; + clocks = <&osc24M>, <&pclk>; + clock-names = "uartclk", "apb_pclk"; }; sci at a000 { compatible = "arm,primecell"; reg = <0xa000 0x1000>; interrupt-parent = <&sic>; interrupts = <5>; + clocks = <&osc24M>; + clock-names = "apb_pclk"; }; mmc at b000 { compatible = "arm,pl180", "arm,primecell"; reg = <0xb000 0x1000>; interrupts-extended = <&vic 23 &sic 2>; + clocks = <&osc24M>, <&pclk>; + clock-names = "mclk", "apb_pclk"; }; }; };
Signature · 1 line
1.9.1
PATCH 9/10 dts: versatile: add clock tree
Quoted 6 lines · Rob Herring
From: Rob Herring <[email protected]>
The versatile dts is missing any clock data. Add the clocks.
It is not clear from the documentation where pclk comes from, so for now it is a dummy clock which is sufficient for things to work.
AFAICT (from experiments and measurements on some boards, during which I destroyed some boards) that is actually just the 24MHz clock right off.
arch/arm/boot/dts/versatile-ab.dts line 30
Quoted 11 lines
Signed-off-by Rob Herring <[email protected]>arch/arm/boot/dts/versatile-ab.dts +9
-0+ core-module at 10000000 { + compatible = "arm,core-module-versatile"; + reg = <0x10000000 0x200>; + + osc24M: oscillator at 24M { + #clock-cells = <0>; + compatible = "fixed-clock"; + clock-frequency = <24000000>; + };
Please follow the naming convention from the Integrator DTS, I am pretty sure this is a chrystal:
/* 24 MHz chrystal on the core module */
xtal24mhz: xtal24mhz at 24M {
#clock-cells = <0>;
compatible = "fixed-clock";
clock-frequency = <24000000>;
};arch/arm/boot/dts/versatile-ab.dts line 37
Quoted 6 lines
arch/arm/boot/dts/versatile-ab.dts +6
-0+ /* OSC1 on AB, OSC4 on PB */ + osc1: cm_aux_osc at 24M { + #clock-cells = <0>; + compatible = "arm,versatile-cm-auxosc"; + clocks = <&osc24M>; + };
Name xtal, also: why is this inside the core module node? You're explicitly saying it is on the PB (platform baseboard) and *not* on the core module!
Quoted 5 lines
arch/arm/boot/dts/versatile-ab.dts +5
-0+ /* The timer clock is the 24 MHz oscillator divided to 1MHz */ + timclk: timclk at 1M { + #clock-cells = <0>; + compatible = "fixed-factor-clock"; + clock-div = <24>;
arch/arm/boot/dts/versatile-ab.dts line 53
Quoted 10 lines
arch/arm/boot/dts/versatile-ab.dts +10
-0+ clock-mult = <1>; + clocks = <&osc24M>; + }; + + /* Actually hclk ? */ + pclk: pclk at 0 { + #clock-cells = <0>; + compatible = "fixed-clock"; + clock-frequency = <0>; + };
I strongly suspect it's like this:
pclk: pclk at 0 {
#clock-cells = <0>;
compatible = "fixed-factor-clock";
clock-div = <1>;
clock-mult = <1>;
clocks = <&xtal24mhz>;
};arch/arm/boot/dts/versatile-ab.dts line 187
Quoted 7 lines
timer at 101e2000 { compatible = "arm,sp804", "arm,primecell"; reg = <0x101e2000 0x1000>; interrupts = <4>;arch/arm/boot/dts/versatile-ab.dts +2
-0+ clocks = <&timclk>, <&pclk>; + clock-names = "tmrclk", "apb_pclk"; };
We recently had some fight over the names of these clocks. The DT bindings say they should be named "timer0" "timer1" etc, see Documentation/devicetree/bindings/timer/arm,sp804.txt
Review
Quoted 6 lines
timer at 101e3000 { compatible = "arm,sp804", "arm,primecell"; reg = <0x101e3000 0x1000>; interrupts = <5>;(quoted diff) +2
-0+ clocks = <&timclk>, <&pclk>; + clock-names = "tmrclk", "apb_pclk";
Dito.
arch/arm/boot/dts/versatile-ab.dts line 242
Quoted 6 lines
ssp at 101f4000 { compatible = "arm,pl022", "arm,primecell"; reg = <0x101f4000 0x1000>; interrupts = <11>;arch/arm/boot/dts/versatile-ab.dts +2
-0+ clocks = <&osc24M>, <&pclk>; + clock-names = "sspclk", "apb_pclk";
Should be SSPCLK all capitals.
Yours,
Linus Walleij
PATCH 9/10 dts: versatile: add clock tree
Quoted 27 lines · Linus Walleij
Quoted 6 lines · Rob Herring
From: Rob Herring <[email protected]>
The versatile dts is missing any clock data. Add the clocks.
It is not clear from the documentation where pclk comes from, so for now it is a dummy clock which is sufficient for things to work.
AFAICT (from experiments and measurements on some boards, during which I destroyed some boards) that is actually just the 24MHz clock right off.
arch/arm/boot/dts/versatile-ab.dts line 30
Quoted 11 lines
Signed-off-by Rob Herring <[email protected]>arch/arm/boot/dts/versatile-ab.dts +9
-0+ core-module at 10000000 { + compatible = "arm,core-module-versatile"; + reg = <0x10000000 0x200>; + + osc24M: oscillator at 24M { + #clock-cells = <0>; + compatible = "fixed-clock"; + clock-frequency = <24000000>; + };Please follow the naming convention from the Integrator DTS, I am pretty sure this is a chrystal:
Quoted 6 lines
/* 24 MHz chrystal on the core module */
xtal24mhz: xtal24mhz at 24M {
#clock-cells = <0>;
compatible = "fixed-clock";
clock-frequency = <24000000>;
};I'm not too sure I'm happy about this naming convention (yours or mine). Following that node names are generic, it should probably be just clk or clock. The unit address is a bit of an abuse as there is no reg property, but we do need some way to make it unique. I don't see the point of having the frequency in the name twice either.
Quoted 10 lines
arch/arm/boot/dts/versatile-ab.dts line 37
Quoted 6 lines
arch/arm/boot/dts/versatile-ab.dts +6
-0+ /* OSC1 on AB, OSC4 on PB */ + osc1: cm_aux_osc at 24M { + #clock-cells = <0>; + compatible = "arm,versatile-cm-auxosc"; + clocks = <&osc24M>; + };Name xtal, also: why is this inside the core module node? You're explicitly saying it is on the PB (platform baseboard) and *not* on the core module!
But it is not a crystal. It is an output of the clock chip. This follows what you have for integrator in name and location and it is part of the AB core-module.
Rob
PATCH 10/10 clk: versatile: add versatile OSC support
In reply to Rob HerringFrom: Rob Herring <[email protected]>
Versatile platforms share the same OSC programming model as Integrator platforms. Add the necessary parameters and init functions for Versatile.
--- drivers/clk/versatile/Makefile | 3 +-- drivers/clk/versatile/clk-integrator.c | 35 ++++++++++++++++++++++++++++++++-- 2 files changed, 34 insertions(+), 4 deletions(-)
drivers/clk/versatile/Makefile +1 -2
@@ -1,6 +1,5 @@ # Makefile for Versatile-specific clocks -obj-$(CONFIG_ICST) += clk-icst.o -obj-$(CONFIG_ARCH_INTEGRATOR) += clk-integrator.o +obj-$(CONFIG_ICST) += clk-icst.o clk-integrator.o obj-$(CONFIG_INTEGRATOR_IMPD1) += clk-impd1.o obj-$(CONFIG_ARCH_REALVIEW) += clk-realview.o obj-$(CONFIG_ARCH_VEXPRESS) += clk-vexpress.o clk-sp810.o
drivers/clk/versatile/clk-integrator.c +33 -2
@@ -17,6 +17,9 @@#define INTEGRATOR_HDR_LOCK_OFFSET 0x14 +#define VERSATILE_SYS_OSCCLCD_OFFSET 0x1c +#define VERSATILE_SYS_LOCK_OFFSET 0x20 +/* Base offset for the core module */ static void __iomem *cm_base;
@@ -37,11 +40,27 @@ static const struct clk_icst_desc __initdata cm_auxosc_desc = { .lock_offset = INTEGRATOR_HDR_LOCK_OFFSET, }; -static void __init of_integrator_cm_osc_setup (struct device_node *np) +static const struct icst_params versatile_auxosc_params = { + .vco_max =ICST307_VCO_MAX , + .vco_min =ICST307_VCO_MIN , + .vd_min =4 +8 , + .vd_max =511 +8 , + .rd_min =1 +2 , + .rd_max =127 +2 , + .s2div = icst307_s2div, + .idx2s = icst307_idx2s, +}; + +static const struct clk_icst_desc versatile_auxosc_desc__initconst = { + .params = &versatile_auxosc_params, + .vco_offset =VERSATILE_SYS_OSCCLCD_OFFSET , + .lock_offset =VERSATILE_SYS_LOCK_OFFSET , +}; +static void __init cm_osc_setup (struct device_node *np, +const struct clk_icst_desc *desc) {struct clk *clk =ERR_PTR (-EINVAL );const char *clk_name = np->name; -const struct clk_icst_desc *desc = &cm_auxosc_desc;const char *parent_name;if (!cm_base) {
@@ -65,5 +84,17 @@ static void __init of_integrator_cm_osc_setup(struct device_node *np)if (!IS_ERR (clk))of_clk_add_provider (np, of_clk_src_simple_get, clk); } + +static void __init of_integrator_cm_osc_setup (struct device_node *np) +{ +cm_osc_setup (np, &cm_auxosc_desc); +}CLK_OF_DECLARE (integrator_cm_auxosc_clk,"arm,integrator-cm-auxosc" , of_integrator_cm_osc_setup); + +static void __init of_versatile_cm_osc_setup (struct device_node *np) +{ +cm_osc_setup (np, &versatile_auxosc_desc); +} +CLK_OF_DECLARE (versatile_cm_auxosc_clk, +"arm,versatile-cm-auxosc" , of_versatile_cm_osc_setup);
Signature · 1 line
1.9.1
PATCH 10/10 clk: versatile: add versatile OSC support
Quoted 10 lines · Rob Herring
From: Rob Herring <[email protected]>
Versatile platforms share the same OSC programming model as Integrator platforms. Add the necessary parameters and init functions for Versatile.
Signed-off-by Rob Herring <[email protected]>Cc Mike Turquette <[email protected]>--- drivers/clk/versatile/Makefile | 3 +-- drivers/clk/versatile/clk-integrator.c | 35 ++++++++++++++++++++++++++++++++--
So then this is a good opportunity to rename that file from clk-integrator.c to something else, right? Usually we use "versatile" to refer to the whole family of boards but other names are possible I guess.
Apart from that it looks good :-)
Yours,
Linus Walleij
PATCH 10/10 clk: versatile: add versatile OSC support
Quoting Linus Walleij (2014-05-23 06:31:26)
Quoted 13 lines · Rob Herring
Quoted 11 lines
From: Rob Herring <[email protected]>
Versatile platforms share the same OSC programming model as Integrator platforms. Add the necessary parameters and init functions for Versatile.
Signed-off-by Rob Herring <[email protected]>Cc Mike Turquette <[email protected]>--- drivers/clk/versatile/Makefile | 3 +-- drivers/clk/versatile/clk-integrator.c | 35 ++++++++++++++++++++++++++++++++--
Quoted 5 lines
So then this is a good opportunity to rename that file from
clk-integrator.cto something else, right? Usually we use "versatile" to refer to the whole family of boards but other names are possible I guess.Apart from that it looks good :-)
Looks good to me too. Rob will you incorporate the file name change?
Regards,
Mike
Quoted 3 lines
Yours,
Linus Walleij
cover ARM: Versatile irq and clock DT support
In reply to Rob HerringQuoted 11 lines · On Tuesday 20 May 2014 16:09:26 Rob Herring wrote:
From: Rob Herring <[email protected]>
This series is some preparation work to fully convert Versatile platform to DT. This converts the IRQ initialization to DT based init and adds clocks to the Versatile DT. This doesn't actually enable using common clock framework, but that is only a matter of removing the old clock code and enabling the appropriate
Kconfigoptions. This will be done when Versatile can be completely converted over to DT boot. I have that working in my tree, but full DT conversion is also dependent on DT PCI support.
Quoted 2 lines
I've tested under QEMU and Linus Walleij has tested an earlier version on Versatile AB h/w.
Looks great to me.
Should we merge it directly into arm-soc or are you planning to send a pull request?
Arnd
cover ARM: Versatile irq and clock DT support
Quoted 15 lines · Arnd Bergmann
Quoted 11 lines · On Tuesday 20 May 2014 16:09:26 Rob Herring wrote:
From: Rob Herring <[email protected]>
This series is some preparation work to fully convert Versatile platform to DT. This converts the IRQ initialization to DT based init and adds clocks to the Versatile DT. This doesn't actually enable using common clock framework, but that is only a matter of removing the old clock code and enabling the appropriate
Kconfigoptions. This will be done when Versatile can be completely converted over to DT boot. I have that working in my tree, but full DT conversion is also dependent on DT PCI support.Quoted 2 lines
I've tested under QEMU and Linus Walleij has tested an earlier version on Versatile AB h/w.
Quoted 6 lines
Looks great to me.
Acked-by Arnd Bergmann <[email protected]>Should we merge it directly into arm-soc or are you planning to send a pull request?
I'll do a pull req. I'll give it a few more days for review.
Rob