Skip to contentSkip to conversation

dt-bindings: add Marvell core PLL and clock divider PMU documentation

PATCH Superseded Russell King 5 participants 7 messages Nov 26, 2015 – Nov 30, 2015

Subsystems (2)

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/3 dt-bindings: add Marvell core PLL and clock divider PMU documentation

Add documentation for the Marvell clock divider driver, which is used to source clocks for the AXI bus, video decoder, GPU and LCD blocks.

---
 .../bindings/clock/dove-divider-clock.txt          | 28 ++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/dove-divider-clock.txt
Documentation/devicetree/bindings/clock/dove-divider-clock.txt new file +28 -0
new file mode 100644
index 000000000000..0c602de279e5
@@ -0,0 +1,28 @@
+PLl divider based Dove clocks
+
+Marvell Dove has a 2GHz PLL, which feeds into a set of dividers to provide
+high speed clocks for a number of peripherals.  These dividers are part of
+the PMU, and thus this node should be a child of the PMU node.
+
+The following clocks are provided:
+
+ID	Clock
+-------------
+0	AXI bus clock
+1	GPU clock
+2	VMeta clock
+3	LCD clock
+
+Required properties:
+- compatible : shall be "marvell,dove-divider-clock"
+- reg : shall be the register address of the Core PLL and Clock Divider
+   Control 0 register.  This will cover that register, as well as the
+   Core PLL and Clock Divider Control 1 register.  Thus, it will have
+   a size of 8.
+- #clock-cells : from common clock binding; shall be set to 1
+
+divider_clk: core-clock at 0064 {
+	compatible = "marvell,dove-divider-clock";
+	reg = <0x0064 0x8>;
+	#clock-cells = <1>;
+};
Signature · 1 line
2.1.0
#1 lore raw

PATCH 1/3 dt-bindings: add Marvell core PLL and clock divider PMU documentation

Quoted 15 lines · Russell King

Add documentation for the Marvell clock divider driver, which is used to source clocks for the AXI bus, video decoder, GPU and LCD blocks.

---
 .../bindings/clock/dove-divider-clock.txt          | 28 ++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/dove-divider-clock.txt
Documentation/devicetree/bindings/clock/dove-divider-clock.txt new file +0 -0
new file mode 100644
index 000000000000..0c602de279e5

Documentation/devicetree/bindings/clock/dove-divider-clock.txt line 1

Quoted 1 line
Documentation/devicetree/bindings/clock/dove-divider-clock.txt new file +1 -0
@@ -0,0 +1,28 @@
+PLl divider based Dove clocks

Nit pick. It would be nice if the second L of PLL was also a capital.

Andrew

maintainer
#2 lore raw

PATCH 1/3 dt-bindings: add Marvell core PLL and clock divider PMU documentation

In reply to Russell King
Quoted 15 lines · Russell King

Add documentation for the Marvell clock divider driver, which is used to source clocks for the AXI bus, video decoder, GPU and LCD blocks.

---
 .../bindings/clock/dove-divider-clock.txt          | 28 ++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/dove-divider-clock.txt
Documentation/devicetree/bindings/clock/dove-divider-clock.txt new file +0 -0
new file mode 100644
index 000000000000..0c602de279e5

Documentation/devicetree/bindings/clock/dove-divider-clock.txt line 5

Quoted 5 lines
Documentation/devicetree/bindings/clock/dove-divider-clock.txt new file +5 -0
@@ -0,0 +1,28 @@
+PLl divider based Dove clocks
+
+Marvell Dove has a 2GHz PLL, which feeds into a set of dividers to provide
+high speed clocks for a number of peripherals.  These dividers are part of
+the PMU, and thus this node should be a child of the PMU node.

It seems a bit strange to just be documenting these clocks. What about the rest of the SOC clocks?

Quoted 26 lines
Documentation/devicetree/bindings/clock/dove-divider-clock.txt new file +23 -0
+
+The following clocks are provided:
+
+ID	Clock
+-------------
+0	AXI bus clock
+1	GPU clock
+2	VMeta clock
+3	LCD clock
+
+Required properties:
+- compatible : shall be "marvell,dove-divider-clock"
+- reg : shall be the register address of the Core PLL and Clock Divider
+   Control 0 register.  This will cover that register, as well as the
+   Core PLL and Clock Divider Control 1 register.  Thus, it will have
+   a size of 8.
+- #clock-cells : from common clock binding; shall be set to 1
+
+divider_clk: core-clock at 0064 {
+	compatible = "marvell,dove-divider-clock";
+	reg = <0x0064 0x8>;
+	#clock-cells = <1>;
+};
Signature · 1 line
2.1.0
#3 lore raw

PATCH 1/3 dt-bindings: add Marvell core PLL and clock divider PMU documentation

Quoted 6 lines

Documentation/devicetree/bindings/clock/dove-divider-clock.txt line 5

Quoted 3 lines
Documentation/devicetree/bindings/clock/dove-divider-clock.txt new file +3 -0
+Marvell Dove has a 2GHz PLL, which feeds into a set of dividers to provide
+high speed clocks for a number of peripherals.  These dividers are part of
+the PMU, and thus this node should be a child of the PMU node.

It seems a bit strange to just be documenting these clocks. What about the rest of the SOC clocks?

$ ls Documentation/devicetree/bindings/clock/mvebu*
Documentation/devicetree/bindings/clock/mvebu-core-clock.txt
Documentation/devicetree/bindings/clock/mvebu-corediv-clock.txt
Documentation/devicetree/bindings/clock/mvebu-cpu-clock.txt
Documentation/devicetree/bindings/clock/mvebu-gated-clock.txt

Dove is a member of mvebu, and gets most of its clocks from these drivers. But it has additional clocks which are not shared with other members of mvebu.

Andrew

#4 lore raw

PATCH 1/3 dt-bindings: add Marvell core PLL and clock divider PMU documentation

In reply to Rob Herring
Quoted 10 lines · Rob Herring
Quoted 9 lines · Russell King

Add documentation for the Marvell clock divider driver, which is used to source clocks for the AXI bus, video decoder, GPU and LCD blocks.

---
 .../bindings/clock/dove-divider-clock.txt          | 28 ++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/dove-divider-clock.txt
Quoted 14 lines

Documentation/devicetree/bindings/clock/dove-divider-clock.txt line 5

Quoted 11 lines
Documentation/devicetree/bindings/clock/dove-divider-clock.txt new file +5 -0
new file mode 100644
index 000000000000..0c602de279e5
@@ -0,0 +1,28 @@
+PLl divider based Dove clocks
+
+Marvell Dove has a 2GHz PLL, which feeds into a set of dividers to provide
+high speed clocks for a number of peripherals.  These dividers are part of
+the PMU, and thus this node should be a child of the PMU node.

It seems a bit strange to just be documenting these clocks. What about the rest of the SOC clocks?

This is all that this pair of registers provide.

The SoC has other clocks handled by other DT nodes - for Dove, we now have:

        gate_clk: clock-gating-ctrl at 0038 {
                compatible = "marvell,dove-gating-clock";
                reg = <0x0038 0x4>;
                clocks = <&core_clk 0>;
                #clock-cells = <1>;
        };
        divider_clk: core-clock at 0064 {
                compatible = "marvell,dove-divider-clock";
                reg = <0x0064 0x8>;
                #clock-cells = <1>;
        };
        core_clk: core-clocks at 0214 {
                compatible = "marvell,dove-core-clock";
                reg = <0x0214 0x4>;
                #clock-cells = <1>;
        };

and all three of these are part of the PMU register block. I'm not sure why the mvebu maintainers decided to minutely describe the PMU like this, but unfortunately that's the structure we have.

What's more silly is that the "dove-core-clock" appears to disagree in terminology with the manual - there's a "Core PLL" which supplies the dividers in the "divider_clk" block, and is entirely separate from the CPU PLL which "core_clk" is describing.

IMHO, it would've been cleaner to have these components registered separately from a central PMU driver (as I'm doing with the power domains, resets and IRQs that are part of the PMU), but my view is limited to Dove and not the other mvebu clocks, so there may be a good reason for it.

Signature · 2 lines
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
#5 lore raw

PATCH 1/3 dt-bindings: add Marvell core PLL and clock divider PMU documentation

Quoted 57 lines · Russell King - ARM Linux
Quoted 10 lines · Rob Herring
Quoted 9 lines · Russell King

Add documentation for the Marvell clock divider driver, which is used to source clocks for the AXI bus, video decoder, GPU and LCD blocks.

---
 .../bindings/clock/dove-divider-clock.txt          | 28 ++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/dove-divider-clock.txt
Quoted 14 lines

Documentation/devicetree/bindings/clock/dove-divider-clock.txt line 5

Quoted 11 lines
Documentation/devicetree/bindings/clock/dove-divider-clock.txt new file +5 -0
new file mode 100644
index 000000000000..0c602de279e5
@@ -0,0 +1,28 @@
+PLl divider based Dove clocks
+
+Marvell Dove has a 2GHz PLL, which feeds into a set of dividers to provide
+high speed clocks for a number of peripherals.  These dividers are part of
+the PMU, and thus this node should be a child of the PMU node.

It seems a bit strange to just be documenting these clocks. What about the rest of the SOC clocks?

This is all that this pair of registers provide.

The SoC has other clocks handled by other DT nodes - for Dove, we now have:

        gate_clk: clock-gating-ctrl at 0038 {
                compatible = "marvell,dove-gating-clock";
                reg = <0x0038 0x4>;
                clocks = <&core_clk 0>;
                #clock-cells = <1>;
        };
        divider_clk: core-clock at 0064 {
                compatible = "marvell,dove-divider-clock";
                reg = <0x0064 0x8>;
                #clock-cells = <1>;
        };
        core_clk: core-clocks at 0214 {
                compatible = "marvell,dove-core-clock";
                reg = <0x0214 0x4>;
                #clock-cells = <1>;
        };

and all three of these are part of the PMU register block. I'm not sure why the mvebu maintainers decided to minutely describe the PMU like this, but unfortunately that's the structure we have.

What's more silly is that the "dove-core-clock" appears to disagree in terminology with the manual - there's a "Core PLL" which supplies the dividers in the "divider_clk" block, and is entirely separate from the CPU PLL which "core_clk" is describing.

Quoted 6 lines

IMHO, it would've been cleaner to have these components registered separately from a central PMU driver (as I'm doing with the power domains, resets and IRQs that are part of the PMU), but my view is limited to Dove and not the other mvebu clocks, so there may be a good reason for it.

Rob, Russell,

the main reason why dove clocks are the way they are is that back when we thought of the bindings for mvebu SoC clocks, CCF and DT was in its very beginning. Looking back, some decisions shouldn't have been made that way. Also, we tried to describe Dove and the other SoCs similarily, but Dove is always a little odd.

Quoted 1 line

From todays point of view, I certainly agree with Russell that all

PMU related stuff is best kept in a single PMU node.

Thanks for providing the patches, I appreciate the extra work of separating those from your working trees.

Sebastian

maintainer
#6 lore raw

PATCH 1/3 dt-bindings: add Marvell core PLL and clock divider PMU documentation

In reply to Russell King - ARM Linux
Quoted 48 lines · Russell King - ARM Linux
Quoted 10 lines · Rob Herring
Quoted 9 lines · Russell King

Add documentation for the Marvell clock divider driver, which is used to source clocks for the AXI bus, video decoder, GPU and LCD blocks.

---
 .../bindings/clock/dove-divider-clock.txt          | 28 ++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/dove-divider-clock.txt
Quoted 14 lines

Documentation/devicetree/bindings/clock/dove-divider-clock.txt line 5

Quoted 11 lines
Documentation/devicetree/bindings/clock/dove-divider-clock.txt new file +5 -0
new file mode 100644
index 000000000000..0c602de279e5
@@ -0,0 +1,28 @@
+PLl divider based Dove clocks
+
+Marvell Dove has a 2GHz PLL, which feeds into a set of dividers to provide
+high speed clocks for a number of peripherals.  These dividers are part of
+the PMU, and thus this node should be a child of the PMU node.

It seems a bit strange to just be documenting these clocks. What about the rest of the SOC clocks?

This is all that this pair of registers provide.

The SoC has other clocks handled by other DT nodes - for Dove, we now have:

        gate_clk: clock-gating-ctrl at 0038 {
                compatible = "marvell,dove-gating-clock";
                reg = <0x0038 0x4>;
                clocks = <&core_clk 0>;
                #clock-cells = <1>;
        };
        divider_clk: core-clock at 0064 {
                compatible = "marvell,dove-divider-clock";
                reg = <0x0064 0x8>;
                #clock-cells = <1>;
        };
        core_clk: core-clocks at 0214 {
                compatible = "marvell,dove-core-clock";
                reg = <0x0214 0x4>;
                #clock-cells = <1>;
        };
Quoted 3 lines

and all three of these are part of the PMU register block. I'm not sure why the mvebu maintainers decided to minutely describe the PMU like this, but unfortunately that's the structure we have.

Okay,

Quoted 5 lines

IMHO, it would've been cleaner to have these components registered separately from a central PMU driver (as I'm doing with the power domains, resets and IRQs that are part of the PMU), but my view is limited to Dove and not the other mvebu clocks, so there may be a good reason for it.

Yes, we debated which way to go early on and decided generally to go this direction, but it seems Dove was before that. We could try to migrate Dove, but it is probably not worth the pain at this point.

Rob