This patch series adds device tree support for the Broadcom Northstar,
Northstar Plus, and Northstar 2 clocks.
Last sent as an RFC (see https://lkml.org/lkml/2015/10/13/882) due to
the inability to merge because of the driver dependencies. Those
necessary driver changes were merged into 4.4. All comments have been
addressed and it is ready to be pulled in.
Jon Mason (3):
ARM: dts: enable clock support for BCM5301X
ARM: dts: enable clock support for Broadcom NSP
ARM64: dts: enable clock support for Broadcom NS2
arch/arm/boot/dts/bcm-nsp.dtsi | 99 ++++++++++++++++++++++++++---------
arch/arm/boot/dts/bcm5301x.dtsi | 92 ++++++++++++++++++++++++--------
arch/arm64/boot/dts/broadcom/ns2.dtsi | 80 +++++++++++++++++++++++++++-
3 files changed, 225 insertions(+), 46 deletions(-)
--
1.9.1
Replace current device tree dummy clocks with real clock support for
Broadcom Northstar SoCs.
Signed-off-by: Jon Mason <[email protected]>
---
arch/arm/boot/dts/bcm5301x.dtsi | 92 +++++++++++++++++++++++++++++++----------
1 file changed, 71 insertions(+), 21 deletions(-)
diff --git a/arch/arm/boot/dts/bcm5301x.dtsi b/arch/arm/boot/dts/bcm5301x.dtsi
index 6f50f67..65a1309 100644
--- a/arch/arm/boot/dts/bcm5301x.dtsi
+++ b/arch/arm/boot/dts/bcm5301x.dtsi
@@ -8,6 +8,7 @@
* Licensed under the GNU/GPL. See COPYING for details.
*/
+#include <dt-bindings/clock/bcm-nsp.h>
#include <dt-bindings/gpio/gpio.h>
#include <dt-bindings/input/input.h>
#include <dt-bindings/interrupt-controller/irq.h>
@@ -27,7 +28,7 @@
compatible = "ns16550";
reg = <0x0300 0x100>;
interrupts = <GIC_SPI 85 IRQ_TYPE_LEVEL_HIGH>;
- clock-frequency = <100000000>;
+ clocks = <&iprocslow>;
status = "disabled";
};
@@ -35,48 +36,55 @@
compatible = "ns16550";
reg = <0x0400 0x100>;
interrupts = <GIC_SPI 85 IRQ_TYPE_LEVEL_HIGH>;
- clock-frequency = <100000000>;
+ clocks = <&iprocslow>;
status = "disabled";
};
};
mpcore {
compatible = "simple-bus";
- ranges = <0x00000000 0x19020000 0x00003000>;
+ ranges = <0x00000000 0x19000000 0x00023000>;
#address-cells = <1>;
#size-cells = <1>;
- scu@0000 {
+ a9pll: arm_clk@00000 {
+ #clock-cells = <0>;
+ compatible = "brcm,nsp-armpll";
+ clocks = <&osc>;
+ reg = <0x00000 0x1000>;
+ };
+
+ scu@20000 {
compatible = "arm,cortex-a9-scu";
- reg = <0x0000 0x100>;
+ reg = <0x20000 0x100>;
};
- timer@0200 {
+ timer@20200 {
compatible = "arm,cortex-a9-global-timer";
- reg = <0x0200 0x100>;
+ reg = <0x20200 0x100>;
interrupts = <GIC_PPI 11 IRQ_TYPE_LEVEL_HIGH>;
- clocks = <&clk_periph>;
+ clocks = <&periph_clk>;
};
- local-timer@0600 {
+ local-timer@20600 {
compatible = "arm,cortex-a9-twd-timer";
- reg = <0x0600 0x100>;
+ reg = <0x20600 0x100>;
interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_HIGH>;
- clocks = <&clk_periph>;
+ clocks = <&periph_clk>;
};
- gic: interrupt-controller@1000 {
+ gic: interrupt-controller@21000 {
compatible = "arm,cortex-a9-gic";
#interrupt-cells = <3>;
#address-cells = <0>;
interrupt-controller;
- reg = <0x1000 0x1000>,
- <0x0100 0x100>;
+ reg = <0x21000 0x1000>,
+ <0x20100 0x100>;
};
- L2: cache-controller@2000 {
+ L2: cache-controller@22000 {
compatible = "arm,pl310-cache";
- reg = <0x2000 0x1000>;
+ reg = <0x22000 0x1000>;
cache-unified;
arm,shared-override;
prefetch-data = <1>;
@@ -94,14 +102,37 @@
clocks {
#address-cells = <1>;
- #size-cells = <0>;
+ #size-cells = <1>;
+ ranges;
- /* As long as we do not have a real clock driver us this
- * fixed clock */
- clk_periph: periph {
+ osc: oscillator {
+ #clock-cells = <0>;
compatible = "fixed-clock";
+ clock-frequency = <25000000>;
+ };
+
+ iprocmed: iprocmed {
+ #clock-cells = <0>;
+ compatible = "fixed-factor-clock";
+ clocks = <&genpll BCM_NSP_GENPLL_IPROCFAST_CLK>;
+ clock-div = <2>;
+ clock-mult = <1>;
+ };
+
+ iprocslow: iprocslow {
+ #clock-cells = <0>;
+ compatible = "fixed-factor-clock";
+ clocks = <&genpll BCM_NSP_GENPLL_IPROCFAST_CLK>;
+ clock-div = <4>;
+ clock-mult = <1>;
+ };
+
+ periph_clk: periph_clk {
#clock-cells = <0>;
- clock-frequency = <400000000>;
+ compatible = "fixed-factor-clock";
+ clocks = <&a9pll>;
+ clock-div = <2>;
+ clock-mult = <1>;
};
};
@@ -178,6 +209,25 @@
};
};
+ lcpll0: lcpll0@1800c100 {
+ #clock-cells = <1>;
+ compatible = "brcm,nsp-lcpll0";
+ reg = <0x1800c100 0x14>;
+ clocks = <&osc>;
+ clock-output-names = "lcpll0", "pcie_phy", "sdio",
+ "ddr_phy";
+ };
+
+ genpll: genpll@1800c140 {
+ #clock-cells = <1>;
+ compatible = "brcm,nsp-genpll";
+ reg = <0x1800c140 0x24>;
+ clocks = <&osc>;
+ clock-output-names = "genpll", "phy", "ethernetclk",
+ "usbclk", "iprocfast", "sata1",
+ "sata2";
+ };
+
nand: nand@18028000 {
compatible = "brcm,nand-iproc", "brcm,brcmnand-v6.1", "brcm,brcmnand";
reg = <0x18028000 0x600>, <0x1811a408 0x600>, <0x18028f00 0x20>;
--
1.9.1
Replace current device tree dummy clocks with real clock support for
Broadcom Northstar Plus SoC
Signed-off-by: Jon Mason <[email protected]>
---
arch/arm/boot/dts/bcm-nsp.dtsi | 99 ++++++++++++++++++++++++++++++++----------
1 file changed, 75 insertions(+), 24 deletions(-)
diff --git a/arch/arm/boot/dts/bcm-nsp.dtsi b/arch/arm/boot/dts/bcm-nsp.dtsi
index 4bcdd28..f85a4f1 100644
--- a/arch/arm/boot/dts/bcm-nsp.dtsi
+++ b/arch/arm/boot/dts/bcm-nsp.dtsi
@@ -32,6 +32,7 @@
#include <dt-bindings/interrupt-controller/arm-gic.h>
#include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/clock/bcm-nsp.h>
#include "skeleton.dtsi"
@@ -42,7 +43,7 @@
mpcore {
compatible = "simple-bus";
- ranges = <0x00000000 0x19020000 0x00003000>;
+ ranges = <0x00000000 0x19000000 0x00023000>;
#address-cells = <1>;
#size-cells = <1>;
@@ -58,32 +59,23 @@
};
};
- L2: l2-cache {
- compatible = "arm,pl310-cache";
- reg = <0x2000 0x1000>;
- cache-unified;
- cache-level = <2>;
- };
-
- gic: interrupt-controller@19021000 {
- compatible = "arm,cortex-a9-gic";
- #interrupt-cells = <3>;
- #address-cells = <0>;
- interrupt-controller;
- reg = <0x1000 0x1000>,
- <0x0100 0x100>;
+ a9pll: arm_clk@19000000 {
+ #clock-cells = <0>;
+ compatible = "brcm,nsp-armpll";
+ clocks = <&osc>;
+ reg = <0x00000 0x1000>;
};
timer@19020200 {
compatible = "arm,cortex-a9-global-timer";
- reg = <0x0200 0x100>;
+ reg = <0x20200 0x100>;
interrupts = <GIC_PPI 11 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&periph_clk>;
};
twd-timer@19020600 {
compatible = "arm,cortex-a9-twd-timer";
- reg = <0x0600 0x20>;
+ reg = <0x20600 0x20>;
interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(2) |
IRQ_TYPE_LEVEL_HIGH)>;
clocks = <&periph_clk>;
@@ -91,11 +83,27 @@
twd-watchdog@19020620 {
compatible = "arm,cortex-a9-twd-wdt";
- reg = <0x0620 0x20>;
+ reg = <0x20620 0x20>;
interrupts = <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(2) |
IRQ_TYPE_LEVEL_HIGH)>;
clocks = <&periph_clk>;
};
+
+ gic: interrupt-controller@19021000 {
+ compatible = "arm,cortex-a9-gic";
+ #interrupt-cells = <3>;
+ #address-cells = <0>;
+ interrupt-controller;
+ reg = <0x21000 0x1000>,
+ <0x20100 0x100>;
+ };
+
+ L2: l2-cache {
+ compatible = "arm,pl310-cache";
+ reg = <0x22000 0x1000>;
+ cache-unified;
+ cache-level = <2>;
+ };
};
clocks {
@@ -103,10 +111,34 @@
#size-cells = <1>;
ranges;
- periph_clk: periph_clk {
+ osc: oscillator {
+ #clock-cells = <0>;
compatible = "fixed-clock";
+ clock-frequency = <25000000>;
+ };
+
+ iprocmed: iprocmed {
+ #clock-cells = <0>;
+ compatible = "fixed-factor-clock";
+ clocks = <&genpll BCM_NSP_GENPLL_IPROCFAST_CLK>;
+ clock-div = <2>;
+ clock-mult = <1>;
+ };
+
+ iprocslow: iprocslow {
+ #clock-cells = <0>;
+ compatible = "fixed-factor-clock";
+ clocks = <&genpll BCM_NSP_GENPLL_IPROCFAST_CLK>;
+ clock-div = <4>;
+ clock-mult = <1>;
+ };
+
+ periph_clk: periph_clk {
#clock-cells = <0>;
- clock-frequency = <500000000>;
+ compatible = "fixed-factor-clock";
+ clocks = <&a9pll>;
+ clock-div = <2>;
+ clock-mult = <1>;
};
};
@@ -118,17 +150,17 @@
uart0: serial@18000300 {
compatible = "ns16550a";
- reg = <0x0300 0x100>;
+ reg = <0x000300 0x100>;
interrupts = <GIC_SPI 85 IRQ_TYPE_LEVEL_HIGH>;
- clock-frequency = <62499840>;
+ clocks = <&osc>;
status = "disabled";
};
uart1: serial@18000400 {
compatible = "ns16550a";
- reg = <0x0400 0x100>;
+ reg = <0x000400 0x100>;
interrupts = <GIC_SPI 85 IRQ_TYPE_LEVEL_HIGH>;
- clock-frequency = <62499840>;
+ clocks = <&osc>;
status = "disabled";
};
@@ -217,5 +249,24 @@
brcm,nand-has-wp;
};
+
+ lcpll0: lcpll0@1803f100 {
+ #clock-cells = <1>;
+ compatible = "brcm,nsp-lcpll0";
+ reg = <0x03f100 0x14>;
+ clocks = <&osc>;
+ clock-output-names = "lcpll0", "pcie_phy", "sdio",
+ "ddr_phy";
+ };
+
+ genpll: genpll@1803f140 {
+ #clock-cells = <1>;
+ compatible = "brcm,nsp-genpll";
+ reg = <0x03f140 0x24>;
+ clocks = <&osc>;
+ clock-output-names = "genpll", "phy", "ethernetclk",
+ "usbclk", "iprocfast", "sata1",
+ "sata2";
+ };
};
};
--
1.9.1
Add device tree entries for clock support for Broadcom Northstar 2 SoC
Signed-off-by: Jon Mason <[email protected]>
---
arch/arm64/boot/dts/broadcom/ns2.dtsi | 80 ++++++++++++++++++++++++++++++++++-
1 file changed, 79 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/broadcom/ns2.dtsi b/arch/arm64/boot/dts/broadcom/ns2.dtsi
index 9610822..a510d3a 100644
--- a/arch/arm64/boot/dts/broadcom/ns2.dtsi
+++ b/arch/arm64/boot/dts/broadcom/ns2.dtsi
@@ -31,6 +31,7 @@
*/
#include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/clock/bcm-ns2.h>
/memreserve/ 0x84b00000 0x00000008;
@@ -109,6 +110,33 @@
<&A57_3>;
};
+ clocks {
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ osc: oscillator {
+ #clock-cells = <0>;
+ compatible = "fixed-clock";
+ clock-frequency = <25000000>;
+ };
+
+ iprocmed: iprocmed {
+ #clock-cells = <0>;
+ compatible = "fixed-factor-clock";
+ clocks = <&genpll_scr BCM_NS2_GENPLL_SCR_SCR_CLK>;
+ clock-div = <2>;
+ clock-mult = <1>;
+ };
+
+ iprocslow: iprocslow {
+ #clock-cells = <0>;
+ compatible = "fixed-factor-clock";
+ clocks = <&genpll_scr BCM_NS2_GENPLL_SCR_SCR_CLK>;
+ clock-div = <4>;
+ clock-mult = <1>;
+ };
+ };
+
soc: soc {
compatible = "simple-bus";
#address-cells = <1>;
@@ -156,6 +184,56 @@
mmu-masters;
};
+ lcpll_ddr: lcpll_ddr@6501d058 {
+ #clock-cells = <1>;
+ compatible = "brcm,ns2-lcpll-ddr";
+ reg = <0x6501d058 0x20>,
+ <0x6501c020 0x4>,
+ <0x6501d04c 0x4>;
+ clocks = <&osc>;
+ clock-output-names = "lcpll_ddr", "pcie_sata_usb",
+ "ddr", "ddr_ch2_unused",
+ "ddr_ch3_unused", "ddr_ch4_unused",
+ "ddr_ch5_unused";
+ };
+
+ lcpll_ports: lcpll_ports@6501d078 {
+ #clock-cells = <1>;
+ compatible = "brcm,ns2-lcpll-ports";
+ reg = <0x6501d078 0x20>,
+ <0x6501c020 0x4>,
+ <0x6501d054 0x4>;
+ clocks = <&osc>;
+ clock-output-names = "lcpll_ports", "wan", "rgmii",
+ "ports_ch2_unused",
+ "ports_ch3_unused",
+ "ports_ch4_unused",
+ "ports_ch5_unused";
+ };
+
+ genpll_scr: genpll_scr@6501d098 {
+ #clock-cells = <1>;
+ compatible = "brcm,ns2-genpll-scr";
+ reg = <0x6501d098 0x32>,
+ <0x6501c020 0x4>,
+ <0x6501d044 0x4>;
+ clocks = <&osc>;
+ clock-output-names = "genpll_scr", "scr", "fs",
+ "audio_ref", "scr_ch3_unused",
+ "scr_ch4_unused", "scr_ch5_unused";
+ };
+
+ genpll_sw: genpll_sw@6501d0c4 {
+ #clock-cells = <1>;
+ compatible = "brcm,ns2-genpll-sw";
+ reg = <0x6501d0c4 0x32>,
+ <0x6501c020 0x4>,
+ <0x6501d044 0x4>;
+ clocks = <&osc>;
+ clock-output-names = "genpll_sw", "rpe", "250", "nic",
+ "chimp", "port", "sdio";
+ };
+
crmu: crmu@65024000 {
compatible = "syscon";
reg = <0x65024000 0x100>;
@@ -204,7 +282,7 @@
interrupts = <GIC_SPI 393 IRQ_TYPE_LEVEL_HIGH>;
reg-shift = <2>;
reg-io-width = <4>;
- clock-frequency = <23961600>;
+ clocks = <&osc>;
status = "disabled";
};
--
1.9.1
Would this patch merge properly with the other NSP DT clean up patch +
I2C DT patch that you worked out internally but have not sent out?
I thought it's going to make the maintainers' life easier if you can
group DT changes per platform and send them out in the same series.
I also have some inline comments below.
On 11/18/2015 3:13 PM, Jon Mason wrote:
> Replace current device tree dummy clocks with real clock support for
> Broadcom Northstar Plus SoC
>
> Signed-off-by: Jon Mason <[email protected]>
> ---
> arch/arm/boot/dts/bcm-nsp.dtsi | 99 ++++++++++++++++++++++++++++++++----------
> 1 file changed, 75 insertions(+), 24 deletions(-)
>
> diff --git a/arch/arm/boot/dts/bcm-nsp.dtsi b/arch/arm/boot/dts/bcm-nsp.dtsi
> index 4bcdd28..f85a4f1 100644
> --- a/arch/arm/boot/dts/bcm-nsp.dtsi
> +++ b/arch/arm/boot/dts/bcm-nsp.dtsi
> @@ -32,6 +32,7 @@
>
> #include <dt-bindings/interrupt-controller/arm-gic.h>
> #include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/clock/bcm-nsp.h>
>
> #include "skeleton.dtsi"
>
> @@ -42,7 +43,7 @@
>
> mpcore {
> compatible = "simple-bus";
> - ranges = <0x00000000 0x19020000 0x00003000>;
> + ranges = <0x00000000 0x19000000 0x00023000>;
Why does this have anything to do with clocks? Shouldn't it be a
separate patch?
> #address-cells = <1>;
> #size-cells = <1>;
>
> @@ -58,32 +59,23 @@
> };
> };
>
> - L2: l2-cache {
> - compatible = "arm,pl310-cache";
> - reg = <0x2000 0x1000>;
> - cache-unified;
> - cache-level = <2>;
> - };
> -
> - gic: interrupt-controller@19021000 {
> - compatible = "arm,cortex-a9-gic";
> - #interrupt-cells = <3>;
> - #address-cells = <0>;
> - interrupt-controller;
> - reg = <0x1000 0x1000>,
> - <0x0100 0x100>;
> + a9pll: arm_clk@19000000 {
> + #clock-cells = <0>;
> + compatible = "brcm,nsp-armpll";
> + clocks = <&osc>;
> + reg = <0x00000 0x1000>;
> };
>
> timer@19020200 {
> compatible = "arm,cortex-a9-global-timer";
> - reg = <0x0200 0x100>;
> + reg = <0x20200 0x100>;
> interrupts = <GIC_PPI 11 IRQ_TYPE_LEVEL_HIGH>;
> clocks = <&periph_clk>;
> };
>
> twd-timer@19020600 {
> compatible = "arm,cortex-a9-twd-timer";
> - reg = <0x0600 0x20>;
> + reg = <0x20600 0x20>;
> interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(2) |
> IRQ_TYPE_LEVEL_HIGH)>;
> clocks = <&periph_clk>;
> @@ -91,11 +83,27 @@
>
> twd-watchdog@19020620 {
> compatible = "arm,cortex-a9-twd-wdt";
> - reg = <0x0620 0x20>;
> + reg = <0x20620 0x20>;
> interrupts = <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(2) |
> IRQ_TYPE_LEVEL_HIGH)>;
> clocks = <&periph_clk>;
> };
> +
> + gic: interrupt-controller@19021000 {
> + compatible = "arm,cortex-a9-gic";
> + #interrupt-cells = <3>;
> + #address-cells = <0>;
> + interrupt-controller;
> + reg = <0x21000 0x1000>,
> + <0x20100 0x100>;
> + };
> +
> + L2: l2-cache {
> + compatible = "arm,pl310-cache";
> + reg = <0x22000 0x1000>;
> + cache-unified;
> + cache-level = <2>;
> + };
From here and above, all labels are wrong. You are moving them into a
bus that has translated bus addresses, but you still use absolute
addresses for all labels.
And again, 1) Why is this change imbedded in a patch meant for adding DT
clock support according to the commit message; 2) how does the
dependency work with the other patches that you are about to send out?
> };
>
> clocks {
> @@ -103,10 +111,34 @@
> #size-cells = <1>;
> ranges;
>
> - periph_clk: periph_clk {
> + osc: oscillator {
> + #clock-cells = <0>;
> compatible = "fixed-clock";
> + clock-frequency = <25000000>;
> + };
> +
> + iprocmed: iprocmed {
> + #clock-cells = <0>;
> + compatible = "fixed-factor-clock";
> + clocks = <&genpll BCM_NSP_GENPLL_IPROCFAST_CLK>;
> + clock-div = <2>;
> + clock-mult = <1>;
> + };
> +
> + iprocslow: iprocslow {
> + #clock-cells = <0>;
> + compatible = "fixed-factor-clock";
> + clocks = <&genpll BCM_NSP_GENPLL_IPROCFAST_CLK>;
> + clock-div = <4>;
> + clock-mult = <1>;
> + };
> +
> + periph_clk: periph_clk {
> #clock-cells = <0>;
> - clock-frequency = <500000000>;
> + compatible = "fixed-factor-clock";
> + clocks = <&a9pll>;
> + clock-div = <2>;
> + clock-mult = <1>;
> };
> };
>
> @@ -118,17 +150,17 @@
>
> uart0: serial@18000300 {
> compatible = "ns16550a";
> - reg = <0x0300 0x100>;
> + reg = <0x000300 0x100>;
> interrupts = <GIC_SPI 85 IRQ_TYPE_LEVEL_HIGH>;
> - clock-frequency = <62499840>;
> + clocks = <&osc>;
> status = "disabled";
> };
>
> uart1: serial@18000400 {
> compatible = "ns16550a";
> - reg = <0x0400 0x100>;
> + reg = <0x000400 0x100>;
> interrupts = <GIC_SPI 85 IRQ_TYPE_LEVEL_HIGH>;
> - clock-frequency = <62499840>;
> + clocks = <&osc>;
> status = "disabled";
> };
>
> @@ -217,5 +249,24 @@
>
> brcm,nand-has-wp;
> };
> +
> + lcpll0: lcpll0@1803f100 {
wrong label
> + #clock-cells = <1>;
> + compatible = "brcm,nsp-lcpll0";
> + reg = <0x03f100 0x14>;
> + clocks = <&osc>;
> + clock-output-names = "lcpll0", "pcie_phy", "sdio",
> + "ddr_phy";
> + };
> +
> + genpll: genpll@1803f140 {
wrong label
> + #clock-cells = <1>;
> + compatible = "brcm,nsp-genpll";
> + reg = <0x03f140 0x24>;
> + clocks = <&osc>;
> + clock-output-names = "genpll", "phy", "ethernetclk",
> + "usbclk", "iprocfast", "sata1",
> + "sata2";
> + };
> };
> };
>
On 11/18/2015 3:13 PM, Jon Mason wrote:
> Add device tree entries for clock support for Broadcom Northstar 2 SoC
>
> Signed-off-by: Jon Mason <[email protected]>
> ---
> arch/arm64/boot/dts/broadcom/ns2.dtsi | 80 ++++++++++++++++++++++++++++++++++-
> 1 file changed, 79 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/broadcom/ns2.dtsi b/arch/arm64/boot/dts/broadcom/ns2.dtsi
> index 9610822..a510d3a 100644
> --- a/arch/arm64/boot/dts/broadcom/ns2.dtsi
> +++ b/arch/arm64/boot/dts/broadcom/ns2.dtsi
> @@ -31,6 +31,7 @@
> */
>
> #include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/clock/bcm-ns2.h>
>
> /memreserve/ 0x84b00000 0x00000008;
>
> @@ -109,6 +110,33 @@
> <&A57_3>;
> };
>
> + clocks {
Is this a new convention? That is, group all clocks without a base
register address in a node named "clocks", but at the same time, put all
other clocks with base register address under a bus node.
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + osc: oscillator {
> + #clock-cells = <0>;
> + compatible = "fixed-clock";
> + clock-frequency = <25000000>;
> + };
> +
> + iprocmed: iprocmed {
> + #clock-cells = <0>;
> + compatible = "fixed-factor-clock";
> + clocks = <&genpll_scr BCM_NS2_GENPLL_SCR_SCR_CLK>;
> + clock-div = <2>;
> + clock-mult = <1>;
> + };
> +
> + iprocslow: iprocslow {
> + #clock-cells = <0>;
> + compatible = "fixed-factor-clock";
> + clocks = <&genpll_scr BCM_NS2_GENPLL_SCR_SCR_CLK>;
> + clock-div = <4>;
> + clock-mult = <1>;
> + };
> + };
> +
> soc: soc {
> compatible = "simple-bus";
> #address-cells = <1>;
> @@ -156,6 +184,56 @@
> mmu-masters;
> };
>
> + lcpll_ddr: lcpll_ddr@6501d058 {
> + #clock-cells = <1>;
> + compatible = "brcm,ns2-lcpll-ddr";
> + reg = <0x6501d058 0x20>,
> + <0x6501c020 0x4>,
> + <0x6501d04c 0x4>;
> + clocks = <&osc>;
> + clock-output-names = "lcpll_ddr", "pcie_sata_usb",
> + "ddr", "ddr_ch2_unused",
> + "ddr_ch3_unused", "ddr_ch4_unused",
> + "ddr_ch5_unused";
> + };
> +
> + lcpll_ports: lcpll_ports@6501d078 {
> + #clock-cells = <1>;
> + compatible = "brcm,ns2-lcpll-ports";
> + reg = <0x6501d078 0x20>,
> + <0x6501c020 0x4>,
> + <0x6501d054 0x4>;
> + clocks = <&osc>;
> + clock-output-names = "lcpll_ports", "wan", "rgmii",
> + "ports_ch2_unused",
> + "ports_ch3_unused",
> + "ports_ch4_unused",
> + "ports_ch5_unused";
> + };
> +
> + genpll_scr: genpll_scr@6501d098 {
> + #clock-cells = <1>;
> + compatible = "brcm,ns2-genpll-scr";
> + reg = <0x6501d098 0x32>,
> + <0x6501c020 0x4>,
> + <0x6501d044 0x4>;
> + clocks = <&osc>;
> + clock-output-names = "genpll_scr", "scr", "fs",
> + "audio_ref", "scr_ch3_unused",
> + "scr_ch4_unused", "scr_ch5_unused";
> + };
> +
> + genpll_sw: genpll_sw@6501d0c4 {
> + #clock-cells = <1>;
> + compatible = "brcm,ns2-genpll-sw";
> + reg = <0x6501d0c4 0x32>,
> + <0x6501c020 0x4>,
> + <0x6501d044 0x4>;
> + clocks = <&osc>;
> + clock-output-names = "genpll_sw", "rpe", "250", "nic",
> + "chimp", "port", "sdio";
> + };
> +
> crmu: crmu@65024000 {
> compatible = "syscon";
> reg = <0x65024000 0x100>;
> @@ -204,7 +282,7 @@
> interrupts = <GIC_SPI 393 IRQ_TYPE_LEVEL_HIGH>;
> reg-shift = <2>;
> reg-io-width = <4>;
> - clock-frequency = <23961600>;
> + clocks = <&osc>;
> status = "disabled";
> };
>
>
On 18/11/15 16:03, Ray Jui wrote:
>
>
> On 11/18/2015 3:13 PM, Jon Mason wrote:
>> Add device tree entries for clock support for Broadcom Northstar 2 SoC
>>
>> Signed-off-by: Jon Mason <[email protected]>
>> ---
>> arch/arm64/boot/dts/broadcom/ns2.dtsi | 80
>> ++++++++++++++++++++++++++++++++++-
>> 1 file changed, 79 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/broadcom/ns2.dtsi
>> b/arch/arm64/boot/dts/broadcom/ns2.dtsi
>> index 9610822..a510d3a 100644
>> --- a/arch/arm64/boot/dts/broadcom/ns2.dtsi
>> +++ b/arch/arm64/boot/dts/broadcom/ns2.dtsi
>> @@ -31,6 +31,7 @@
>> */
>>
>> #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +#include <dt-bindings/clock/bcm-ns2.h>
>>
>> /memreserve/ 0x84b00000 0x00000008;
>>
>> @@ -109,6 +110,33 @@
>> <&A57_3>;
>> };
>>
>> + clocks {
>
> Is this a new convention? That is, group all clocks without a base
> register address in a node named "clocks", but at the same time, put all
> other clocks with base register address under a bus node.
I do not think that is new, lots of platforms do that. The clock
providers/controllers would typically be in the 'bus' nodes because it
has a register interface, while the synthetic clocks would be under
'clocks'.
--
Florian
On 11/18/2015 4:07 PM, Florian Fainelli wrote:
> On 18/11/15 16:03, Ray Jui wrote:
>>
>>
>> On 11/18/2015 3:13 PM, Jon Mason wrote:
>>> Add device tree entries for clock support for Broadcom Northstar 2 SoC
>>>
>>> Signed-off-by: Jon Mason <[email protected]>
>>> ---
>>> arch/arm64/boot/dts/broadcom/ns2.dtsi | 80
>>> ++++++++++++++++++++++++++++++++++-
>>> 1 file changed, 79 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/broadcom/ns2.dtsi
>>> b/arch/arm64/boot/dts/broadcom/ns2.dtsi
>>> index 9610822..a510d3a 100644
>>> --- a/arch/arm64/boot/dts/broadcom/ns2.dtsi
>>> +++ b/arch/arm64/boot/dts/broadcom/ns2.dtsi
>>> @@ -31,6 +31,7 @@
>>> */
>>>
>>> #include <dt-bindings/interrupt-controller/arm-gic.h>
>>> +#include <dt-bindings/clock/bcm-ns2.h>
>>>
>>> /memreserve/ 0x84b00000 0x00000008;
>>>
>>> @@ -109,6 +110,33 @@
>>> <&A57_3>;
>>> };
>>>
>>> + clocks {
>>
>> Is this a new convention? That is, group all clocks without a base
>> register address in a node named "clocks", but at the same time, put all
>> other clocks with base register address under a bus node.
>
> I do not think that is new, lots of platforms do that. The clock
> providers/controllers would typically be in the 'bus' nodes because it
> has a register interface, while the synthetic clocks would be under
> 'clocks'.
>
Okay that's very good to know. Thanks!
Ray
On Wed, Nov 18, 2015 at 03:57:36PM -0800, Ray Jui wrote:
> Would this patch merge properly with the other NSP DT clean up patch
> + I2C DT patch that you worked out internally but have not sent out?
>
> I thought it's going to make the maintainers' life easier if you can
> group DT changes per platform and send them out in the same series.
>
> I also have some inline comments below.
>
> On 11/18/2015 3:13 PM, Jon Mason wrote:
> >Replace current device tree dummy clocks with real clock support for
> >Broadcom Northstar Plus SoC
> >
> >Signed-off-by: Jon Mason <[email protected]>
> >---
> > arch/arm/boot/dts/bcm-nsp.dtsi | 99 ++++++++++++++++++++++++++++++++----------
> > 1 file changed, 75 insertions(+), 24 deletions(-)
> >
> >diff --git a/arch/arm/boot/dts/bcm-nsp.dtsi b/arch/arm/boot/dts/bcm-nsp.dtsi
> >index 4bcdd28..f85a4f1 100644
> >--- a/arch/arm/boot/dts/bcm-nsp.dtsi
> >+++ b/arch/arm/boot/dts/bcm-nsp.dtsi
> >@@ -32,6 +32,7 @@
> >
> > #include <dt-bindings/interrupt-controller/arm-gic.h>
> > #include <dt-bindings/interrupt-controller/irq.h>
> >+#include <dt-bindings/clock/bcm-nsp.h>
> >
> > #include "skeleton.dtsi"
> >
> >@@ -42,7 +43,7 @@
> >
> > mpcore {
> > compatible = "simple-bus";
> >- ranges = <0x00000000 0x19020000 0x00003000>;
> >+ ranges = <0x00000000 0x19000000 0x00023000>;
>
> Why does this have anything to do with clocks? Shouldn't it be a
> separate patch?
No, this is correct (though the patch is a little odd to look at).
The a9pll starts at 0x19000000 instead of 0x19020000. So, everything
needs to be adjusted.
>
> > #address-cells = <1>;
> > #size-cells = <1>;
> >
> >@@ -58,32 +59,23 @@
> > };
> > };
> >
> >- L2: l2-cache {
> >- compatible = "arm,pl310-cache";
> >- reg = <0x2000 0x1000>;
> >- cache-unified;
> >- cache-level = <2>;
> >- };
> >-
> >- gic: interrupt-controller@19021000 {
> >- compatible = "arm,cortex-a9-gic";
> >- #interrupt-cells = <3>;
> >- #address-cells = <0>;
> >- interrupt-controller;
> >- reg = <0x1000 0x1000>,
> >- <0x0100 0x100>;
> >+ a9pll: arm_clk@19000000 {
> >+ #clock-cells = <0>;
> >+ compatible = "brcm,nsp-armpll";
> >+ clocks = <&osc>;
> >+ reg = <0x00000 0x1000>;
> > };
> >
> > timer@19020200 {
> > compatible = "arm,cortex-a9-global-timer";
> >- reg = <0x0200 0x100>;
> >+ reg = <0x20200 0x100>;
> > interrupts = <GIC_PPI 11 IRQ_TYPE_LEVEL_HIGH>;
> > clocks = <&periph_clk>;
> > };
> >
> > twd-timer@19020600 {
> > compatible = "arm,cortex-a9-twd-timer";
> >- reg = <0x0600 0x20>;
> >+ reg = <0x20600 0x20>;
> > interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(2) |
> > IRQ_TYPE_LEVEL_HIGH)>;
> > clocks = <&periph_clk>;
> >@@ -91,11 +83,27 @@
> >
> > twd-watchdog@19020620 {
> > compatible = "arm,cortex-a9-twd-wdt";
> >- reg = <0x0620 0x20>;
> >+ reg = <0x20620 0x20>;
> > interrupts = <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(2) |
> > IRQ_TYPE_LEVEL_HIGH)>;
> > clocks = <&periph_clk>;
> > };
> >+
> >+ gic: interrupt-controller@19021000 {
> >+ compatible = "arm,cortex-a9-gic";
> >+ #interrupt-cells = <3>;
> >+ #address-cells = <0>;
> >+ interrupt-controller;
> >+ reg = <0x21000 0x1000>,
> >+ <0x20100 0x100>;
> >+ };
> >+
> >+ L2: l2-cache {
> >+ compatible = "arm,pl310-cache";
> >+ reg = <0x22000 0x1000>;
> >+ cache-unified;
> >+ cache-level = <2>;
> >+ };
>
> From here and above, all labels are wrong. You are moving them into
> a bus that has translated bus addresses, but you still use absolute
> addresses for all labels.
>
> And again, 1) Why is this change imbedded in a patch meant for
> adding DT clock support according to the commit message; 2) how does
> the dependency work with the other patches that you are about to
> send out?
This was already discussed in the original series. See
http://www.spinics.net/lists/arm-kernel/msg451580.html
>
>
> > };
> >
> > clocks {
> >@@ -103,10 +111,34 @@
> > #size-cells = <1>;
> > ranges;
> >
> >- periph_clk: periph_clk {
> >+ osc: oscillator {
> >+ #clock-cells = <0>;
> > compatible = "fixed-clock";
> >+ clock-frequency = <25000000>;
> >+ };
> >+
> >+ iprocmed: iprocmed {
> >+ #clock-cells = <0>;
> >+ compatible = "fixed-factor-clock";
> >+ clocks = <&genpll BCM_NSP_GENPLL_IPROCFAST_CLK>;
> >+ clock-div = <2>;
> >+ clock-mult = <1>;
> >+ };
> >+
> >+ iprocslow: iprocslow {
> >+ #clock-cells = <0>;
> >+ compatible = "fixed-factor-clock";
> >+ clocks = <&genpll BCM_NSP_GENPLL_IPROCFAST_CLK>;
> >+ clock-div = <4>;
> >+ clock-mult = <1>;
> >+ };
> >+
> >+ periph_clk: periph_clk {
> > #clock-cells = <0>;
> >- clock-frequency = <500000000>;
> >+ compatible = "fixed-factor-clock";
> >+ clocks = <&a9pll>;
> >+ clock-div = <2>;
> >+ clock-mult = <1>;
> > };
> > };
> >
> >@@ -118,17 +150,17 @@
> >
> > uart0: serial@18000300 {
> > compatible = "ns16550a";
> >- reg = <0x0300 0x100>;
> >+ reg = <0x000300 0x100>;
> > interrupts = <GIC_SPI 85 IRQ_TYPE_LEVEL_HIGH>;
> >- clock-frequency = <62499840>;
> >+ clocks = <&osc>;
> > status = "disabled";
> > };
> >
> > uart1: serial@18000400 {
> > compatible = "ns16550a";
> >- reg = <0x0400 0x100>;
> >+ reg = <0x000400 0x100>;
> > interrupts = <GIC_SPI 85 IRQ_TYPE_LEVEL_HIGH>;
> >- clock-frequency = <62499840>;
> >+ clocks = <&osc>;
> > status = "disabled";
> > };
> >
> >@@ -217,5 +249,24 @@
> >
> > brcm,nand-has-wp;
> > };
> >+
> >+ lcpll0: lcpll0@1803f100 {
>
> wrong label
>
> >+ #clock-cells = <1>;
> >+ compatible = "brcm,nsp-lcpll0";
> >+ reg = <0x03f100 0x14>;
> >+ clocks = <&osc>;
> >+ clock-output-names = "lcpll0", "pcie_phy", "sdio",
> >+ "ddr_phy";
> >+ };
> >+
> >+ genpll: genpll@1803f140 {
>
> wrong label
>
> >+ #clock-cells = <1>;
> >+ compatible = "brcm,nsp-genpll";
> >+ reg = <0x03f140 0x24>;
> >+ clocks = <&osc>;
> >+ clock-output-names = "genpll", "phy", "ethernetclk",
> >+ "usbclk", "iprocfast", "sata1",
> >+ "sata2";
> >+ };
> > };
> > };
> >
On 11/19/2015 7:48 AM, Jon Mason wrote:
> On Wed, Nov 18, 2015 at 03:57:36PM -0800, Ray Jui wrote:
>> Would this patch merge properly with the other NSP DT clean up patch
>> + I2C DT patch that you worked out internally but have not sent out?
>>
>> I thought it's going to make the maintainers' life easier if you can
>> group DT changes per platform and send them out in the same series.
>>
>> I also have some inline comments below.
>>
>> On 11/18/2015 3:13 PM, Jon Mason wrote:
>>> Replace current device tree dummy clocks with real clock support for
>>> Broadcom Northstar Plus SoC
>>>
>>> Signed-off-by: Jon Mason <[email protected]>
>>> ---
>>> arch/arm/boot/dts/bcm-nsp.dtsi | 99 ++++++++++++++++++++++++++++++++----------
>>> 1 file changed, 75 insertions(+), 24 deletions(-)
>>>
>>> diff --git a/arch/arm/boot/dts/bcm-nsp.dtsi b/arch/arm/boot/dts/bcm-nsp.dtsi
>>> index 4bcdd28..f85a4f1 100644
>>> --- a/arch/arm/boot/dts/bcm-nsp.dtsi
>>> +++ b/arch/arm/boot/dts/bcm-nsp.dtsi
>>> @@ -32,6 +32,7 @@
>>>
>>> #include <dt-bindings/interrupt-controller/arm-gic.h>
>>> #include <dt-bindings/interrupt-controller/irq.h>
>>> +#include <dt-bindings/clock/bcm-nsp.h>
>>>
>>> #include "skeleton.dtsi"
>>>
>>> @@ -42,7 +43,7 @@
>>>
>>> mpcore {
>>> compatible = "simple-bus";
>>> - ranges = <0x00000000 0x19020000 0x00003000>;
>>> + ranges = <0x00000000 0x19000000 0x00023000>;
>>
>> Why does this have anything to do with clocks? Shouldn't it be a
>> separate patch?
>
> No, this is correct (though the patch is a little odd to look at).
> The a9pll starts at 0x19000000 instead of 0x19020000. So, everything
> needs to be adjusted.
>
Okay.
>>
>>> #address-cells = <1>;
>>> #size-cells = <1>;
>>>
>>> @@ -58,32 +59,23 @@
>>> };
>>> };
>>>
>>> - L2: l2-cache {
>>> - compatible = "arm,pl310-cache";
>>> - reg = <0x2000 0x1000>;
>>> - cache-unified;
>>> - cache-level = <2>;
>>> - };
>>> -
>>> - gic: interrupt-controller@19021000 {
>>> - compatible = "arm,cortex-a9-gic";
>>> - #interrupt-cells = <3>;
>>> - #address-cells = <0>;
>>> - interrupt-controller;
>>> - reg = <0x1000 0x1000>,
>>> - <0x0100 0x100>;
>>> + a9pll: arm_clk@19000000 {
>>> + #clock-cells = <0>;
>>> + compatible = "brcm,nsp-armpll";
>>> + clocks = <&osc>;
>>> + reg = <0x00000 0x1000>;
>>> };
>>>
>>> timer@19020200 {
>>> compatible = "arm,cortex-a9-global-timer";
>>> - reg = <0x0200 0x100>;
>>> + reg = <0x20200 0x100>;
>>> interrupts = <GIC_PPI 11 IRQ_TYPE_LEVEL_HIGH>;
>>> clocks = <&periph_clk>;
>>> };
>>>
>>> twd-timer@19020600 {
>>> compatible = "arm,cortex-a9-twd-timer";
>>> - reg = <0x0600 0x20>;
>>> + reg = <0x20600 0x20>;
>>> interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(2) |
>>> IRQ_TYPE_LEVEL_HIGH)>;
>>> clocks = <&periph_clk>;
>>> @@ -91,11 +83,27 @@
>>>
>>> twd-watchdog@19020620 {
>>> compatible = "arm,cortex-a9-twd-wdt";
>>> - reg = <0x0620 0x20>;
>>> + reg = <0x20620 0x20>;
>>> interrupts = <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(2) |
>>> IRQ_TYPE_LEVEL_HIGH)>;
>>> clocks = <&periph_clk>;
>>> };
>>> +
>>> + gic: interrupt-controller@19021000 {
>>> + compatible = "arm,cortex-a9-gic";
>>> + #interrupt-cells = <3>;
>>> + #address-cells = <0>;
>>> + interrupt-controller;
>>> + reg = <0x21000 0x1000>,
>>> + <0x20100 0x100>;
>>> + };
>>> +
>>> + L2: l2-cache {
>>> + compatible = "arm,pl310-cache";
>>> + reg = <0x22000 0x1000>;
>>> + cache-unified;
>>> + cache-level = <2>;
>>> + };
>>
>> From here and above, all labels are wrong. You are moving them into
>> a bus that has translated bus addresses, but you still use absolute
>> addresses for all labels.
>>
Please fix all the labels.
>> And again, 1) Why is this change imbedded in a patch meant for
>> adding DT clock support according to the commit message; 2) how does
>> the dependency work with the other patches that you are about to
>> send out?
>
> This was already discussed in the original series. See
> http://www.spinics.net/lists/arm-kernel/msg451580.html
The discussion explains my first question. But what about my second
question? How does the dependency work with other NSP DT patches that
you have on hand? Will there be conflicts? If so, do you expect the
maintainers need to manually fix all the conflicts?
>>
>>
>>> };
>>>
>>> clocks {
>>> @@ -103,10 +111,34 @@
>>> #size-cells = <1>;
>>> ranges;
>>>
>>> - periph_clk: periph_clk {
>>> + osc: oscillator {
>>> + #clock-cells = <0>;
>>> compatible = "fixed-clock";
>>> + clock-frequency = <25000000>;
>>> + };
>>> +
>>> + iprocmed: iprocmed {
>>> + #clock-cells = <0>;
>>> + compatible = "fixed-factor-clock";
>>> + clocks = <&genpll BCM_NSP_GENPLL_IPROCFAST_CLK>;
>>> + clock-div = <2>;
>>> + clock-mult = <1>;
>>> + };
>>> +
>>> + iprocslow: iprocslow {
>>> + #clock-cells = <0>;
>>> + compatible = "fixed-factor-clock";
>>> + clocks = <&genpll BCM_NSP_GENPLL_IPROCFAST_CLK>;
>>> + clock-div = <4>;
>>> + clock-mult = <1>;
>>> + };
>>> +
>>> + periph_clk: periph_clk {
>>> #clock-cells = <0>;
>>> - clock-frequency = <500000000>;
>>> + compatible = "fixed-factor-clock";
>>> + clocks = <&a9pll>;
>>> + clock-div = <2>;
>>> + clock-mult = <1>;
>>> };
>>> };
>>>
>>> @@ -118,17 +150,17 @@
>>>
>>> uart0: serial@18000300 {
>>> compatible = "ns16550a";
>>> - reg = <0x0300 0x100>;
>>> + reg = <0x000300 0x100>;
>>> interrupts = <GIC_SPI 85 IRQ_TYPE_LEVEL_HIGH>;
>>> - clock-frequency = <62499840>;
>>> + clocks = <&osc>;
>>> status = "disabled";
>>> };
>>>
>>> uart1: serial@18000400 {
>>> compatible = "ns16550a";
>>> - reg = <0x0400 0x100>;
>>> + reg = <0x000400 0x100>;
>>> interrupts = <GIC_SPI 85 IRQ_TYPE_LEVEL_HIGH>;
>>> - clock-frequency = <62499840>;
>>> + clocks = <&osc>;
>>> status = "disabled";
>>> };
>>>
>>> @@ -217,5 +249,24 @@
>>>
>>> brcm,nand-has-wp;
>>> };
>>> +
>>> + lcpll0: lcpll0@1803f100 {
>>
>> wrong label
>>
Please fix
>>> + #clock-cells = <1>;
>>> + compatible = "brcm,nsp-lcpll0";
>>> + reg = <0x03f100 0x14>;
>>> + clocks = <&osc>;
>>> + clock-output-names = "lcpll0", "pcie_phy", "sdio",
>>> + "ddr_phy";
>>> + };
>>> +
>>> + genpll: genpll@1803f140 {
>>
>> wrong label
>>
Please fix
>>> + #clock-cells = <1>;
>>> + compatible = "brcm,nsp-genpll";
>>> + reg = <0x03f140 0x24>;
>>> + clocks = <&osc>;
>>> + clock-output-names = "genpll", "phy", "ethernetclk",
>>> + "usbclk", "iprocfast", "sata1",
>>> + "sata2";
>>> + };
>>> };
>>> };
>>>