Re: [PATCH 2/3] arm64, ft-1500a: Add initial dts for Phytium FT-1500A SoC

2015-02-09 Thread Mark Rutland
On Sat, Feb 07, 2015 at 05:07:15AM +, Chen Baozi wrote:
> Add initial device tree nodes for Phytium FT-1500A SoC with support of
> 16 cores, gicv3 interrupt controller, serial port, PCIe host and
> on-chip GMAC ethernet controller.
> 
> Signed-off-by: Chen Baozi 
> ---
>  arch/arm64/boot/dts/Makefile  |   1 +
>  arch/arm64/boot/dts/phytium/Makefile  |   5 +
>  arch/arm64/boot/dts/phytium/ft-1500a.dtsi | 269 
> ++
>  arch/arm64/boot/dts/phytium/ft1500a-v2-dsk-v2.dts |  39 
>  4 files changed, 314 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/phytium/Makefile
>  create mode 100644 arch/arm64/boot/dts/phytium/ft-1500a.dtsi
>  create mode 100644 arch/arm64/boot/dts/phytium/ft1500a-v2-dsk-v2.dts
> 
> diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile
> index c62b0f4..e7e9e3d 100644
> --- a/arch/arm64/boot/dts/Makefile
> +++ b/arch/arm64/boot/dts/Makefile
> @@ -2,5 +2,6 @@ dts-dirs += amd
>  dts-dirs += apm
>  dts-dirs += arm
>  dts-dirs += cavium
> +dts-dirs += phytium
>  
>  subdir-y := $(dts-dirs)
> diff --git a/arch/arm64/boot/dts/phytium/Makefile 
> b/arch/arm64/boot/dts/phytium/Makefile
> new file mode 100644
> index 000..12a22c6
> --- /dev/null
> +++ b/arch/arm64/boot/dts/phytium/Makefile
> @@ -0,0 +1,5 @@
> +dtb-$(CONFIG_ARCH_PHYTIUM) += ft1500a-v2-dsk-v2.dtb
> +
> +always   := $(dtb-y)
> +subdir-y := $(dts-dirs)
> +clean-files  := *.dtb
> diff --git a/arch/arm64/boot/dts/phytium/ft-1500a.dtsi 
> b/arch/arm64/boot/dts/phytium/ft-1500a.dtsi
> new file mode 100644
> index 000..9005389
> --- /dev/null
> +++ b/arch/arm64/boot/dts/phytium/ft-1500a.dtsi
> @@ -0,0 +1,269 @@
> +/*
> + * DTS file for Phytium FT-1500A SoC
> + *
> + * Copyright (C) 2015, Phytium Technology Co., Ltd.
> + *
> + * This file is licensed under a dual GPLv2 or BSD license.
> + */
> +
> +/memreserve/0x8000 0x10;

Please add a comment describing what this protects. It looks like it's
for the spin-table implementation?

> +/ {
> + compatible = "phytium,ft-1500a";
> + interrupt-parent = <>;
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + aliases {
> + ethernet0 = 
> + ethernet1 = 
> + };

No serial aliases? With a serial0 alias here, it would allow you to
have: /chosen/stdout-path = "serial0:${RATE}";

That would make the user's experience with serial _far_ nicer as the
rate would be guaranteed to be consistent, and you can get earlycon
really easily.

> +
> + cpus {
> + #address-cells = <2>;
> + #size-cells = <0>;
> +
> + cpu@0 {
> + device_type = "cpu";
> + compatible = "arm,armv8";

Please prepend the compatible string for the actual CPU you are using.

> + reg = <0x0 0x000>;
> + enable-method = "spin-table";
> + cpu-release-addr = <0x0 0x8007fff0>;
> + };

It's a shame that you are using spin-table. Is there no chance of PSCI?

It's also a great shame that every CPU shares the same release address;
it was a mistake that we made with the early model bringup, and not
something that should be copied where possible to avoid.

[...]

> + cpu@15 {
> + device_type = "cpu";
> + compatible = "arm,armv8";
> + reg = <0x0 0x303>;
> + enable-method = "spin-table";
> + cpu-release-addr = <0x0 0x8007fff0>;
> + };

The unit-address should match the reg, so this should be cpu@303
(likewise the other CPU nodes need to be updated).

> + };
> +
> + gic: interrupt-controller@2980 {
> + compatible = "arm,gic-v3";
> + #interrupt-cells = <3>;
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> + interrupt-controller;
> + reg = <0x0 0x2980 0 0x1>,   /* GICD */
> +   <0x0 0x29a0 0 0x20>,  /* GICR */
> +   <0x0 0x29c0 0 0x1>,   /* GICC */
> +   <0x0 0x29c1 0 0x1>,   /* GICH */
> +   <0x0 0x29c2 0 0x1>;   /* GICV */
> + interrupts = <1 9 4>;
> +
> + its: gic-its@2982 {
> + compatible = "arm,gic-v3-its";
> + msi-controller;
> + reg = <0x0 0x2982 0x0 0x2>;
> + };
> + };
> +
> + timer {
> + compatible = "arm,armv8-timer";
> + interrupts = <1 13 8>,
> +  <1 14 8>,
> +  <1 11 8>,
> +  <1 10 8>;
> + clock-frequency = <5000>;

Please fix your firmware to configure CNTFRQ on _all_ CPUs.

Are CPUs booted at EL2, or at EL1?

If you're booting at EL2, 

Re: [PATCH 2/3] arm64, ft-1500a: Add initial dts for Phytium FT-1500A SoC

2015-02-09 Thread Mark Rutland
On Sat, Feb 07, 2015 at 05:07:15AM +, Chen Baozi wrote:
 Add initial device tree nodes for Phytium FT-1500A SoC with support of
 16 cores, gicv3 interrupt controller, serial port, PCIe host and
 on-chip GMAC ethernet controller.
 
 Signed-off-by: Chen Baozi chenba...@kylinos.com.cn
 ---
  arch/arm64/boot/dts/Makefile  |   1 +
  arch/arm64/boot/dts/phytium/Makefile  |   5 +
  arch/arm64/boot/dts/phytium/ft-1500a.dtsi | 269 
 ++
  arch/arm64/boot/dts/phytium/ft1500a-v2-dsk-v2.dts |  39 
  4 files changed, 314 insertions(+)
  create mode 100644 arch/arm64/boot/dts/phytium/Makefile
  create mode 100644 arch/arm64/boot/dts/phytium/ft-1500a.dtsi
  create mode 100644 arch/arm64/boot/dts/phytium/ft1500a-v2-dsk-v2.dts
 
 diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile
 index c62b0f4..e7e9e3d 100644
 --- a/arch/arm64/boot/dts/Makefile
 +++ b/arch/arm64/boot/dts/Makefile
 @@ -2,5 +2,6 @@ dts-dirs += amd
  dts-dirs += apm
  dts-dirs += arm
  dts-dirs += cavium
 +dts-dirs += phytium
  
  subdir-y := $(dts-dirs)
 diff --git a/arch/arm64/boot/dts/phytium/Makefile 
 b/arch/arm64/boot/dts/phytium/Makefile
 new file mode 100644
 index 000..12a22c6
 --- /dev/null
 +++ b/arch/arm64/boot/dts/phytium/Makefile
 @@ -0,0 +1,5 @@
 +dtb-$(CONFIG_ARCH_PHYTIUM) += ft1500a-v2-dsk-v2.dtb
 +
 +always   := $(dtb-y)
 +subdir-y := $(dts-dirs)
 +clean-files  := *.dtb
 diff --git a/arch/arm64/boot/dts/phytium/ft-1500a.dtsi 
 b/arch/arm64/boot/dts/phytium/ft-1500a.dtsi
 new file mode 100644
 index 000..9005389
 --- /dev/null
 +++ b/arch/arm64/boot/dts/phytium/ft-1500a.dtsi
 @@ -0,0 +1,269 @@
 +/*
 + * DTS file for Phytium FT-1500A SoC
 + *
 + * Copyright (C) 2015, Phytium Technology Co., Ltd.
 + *
 + * This file is licensed under a dual GPLv2 or BSD license.
 + */
 +
 +/memreserve/0x8000 0x10;

Please add a comment describing what this protects. It looks like it's
for the spin-table implementation?

 +/ {
 + compatible = phytium,ft-1500a;
 + interrupt-parent = gic;
 + #address-cells = 2;
 + #size-cells = 2;
 +
 + aliases {
 + ethernet0 = gmac0;
 + ethernet1 = gmac1;
 + };

No serial aliases? With a serial0 alias here, it would allow you to
have: /chosen/stdout-path = serial0:${RATE};

That would make the user's experience with serial _far_ nicer as the
rate would be guaranteed to be consistent, and you can get earlycon
really easily.

 +
 + cpus {
 + #address-cells = 2;
 + #size-cells = 0;
 +
 + cpu@0 {
 + device_type = cpu;
 + compatible = arm,armv8;

Please prepend the compatible string for the actual CPU you are using.

 + reg = 0x0 0x000;
 + enable-method = spin-table;
 + cpu-release-addr = 0x0 0x8007fff0;
 + };

It's a shame that you are using spin-table. Is there no chance of PSCI?

It's also a great shame that every CPU shares the same release address;
it was a mistake that we made with the early model bringup, and not
something that should be copied where possible to avoid.

[...]

 + cpu@15 {
 + device_type = cpu;
 + compatible = arm,armv8;
 + reg = 0x0 0x303;
 + enable-method = spin-table;
 + cpu-release-addr = 0x0 0x8007fff0;
 + };

The unit-address should match the reg, so this should be cpu@303
(likewise the other CPU nodes need to be updated).

 + };
 +
 + gic: interrupt-controller@2980 {
 + compatible = arm,gic-v3;
 + #interrupt-cells = 3;
 + #address-cells = 2;
 + #size-cells = 2;
 + ranges;
 + interrupt-controller;
 + reg = 0x0 0x2980 0 0x1,   /* GICD */
 +   0x0 0x29a0 0 0x20,  /* GICR */
 +   0x0 0x29c0 0 0x1,   /* GICC */
 +   0x0 0x29c1 0 0x1,   /* GICH */
 +   0x0 0x29c2 0 0x1;   /* GICV */
 + interrupts = 1 9 4;
 +
 + its: gic-its@2982 {
 + compatible = arm,gic-v3-its;
 + msi-controller;
 + reg = 0x0 0x2982 0x0 0x2;
 + };
 + };
 +
 + timer {
 + compatible = arm,armv8-timer;
 + interrupts = 1 13 8,
 +  1 14 8,
 +  1 11 8,
 +  1 10 8;
 + clock-frequency = 5000;

Please fix your firmware to configure CNTFRQ on _all_ CPUs.

Are CPUs booted at EL2, or at EL1?

If you're booting at EL2, virtualisation will not function correctly
unless you configure CNTFRQ.

If you're booting at EL1, you must configure CNTVOFF to a consistent
value on all 

[PATCH 2/3] arm64, ft-1500a: Add initial dts for Phytium FT-1500A SoC

2015-02-06 Thread Chen Baozi
Add initial device tree nodes for Phytium FT-1500A SoC with support of
16 cores, gicv3 interrupt controller, serial port, PCIe host and
on-chip GMAC ethernet controller.

Signed-off-by: Chen Baozi 
---
 arch/arm64/boot/dts/Makefile  |   1 +
 arch/arm64/boot/dts/phytium/Makefile  |   5 +
 arch/arm64/boot/dts/phytium/ft-1500a.dtsi | 269 ++
 arch/arm64/boot/dts/phytium/ft1500a-v2-dsk-v2.dts |  39 
 4 files changed, 314 insertions(+)
 create mode 100644 arch/arm64/boot/dts/phytium/Makefile
 create mode 100644 arch/arm64/boot/dts/phytium/ft-1500a.dtsi
 create mode 100644 arch/arm64/boot/dts/phytium/ft1500a-v2-dsk-v2.dts

diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile
index c62b0f4..e7e9e3d 100644
--- a/arch/arm64/boot/dts/Makefile
+++ b/arch/arm64/boot/dts/Makefile
@@ -2,5 +2,6 @@ dts-dirs += amd
 dts-dirs += apm
 dts-dirs += arm
 dts-dirs += cavium
+dts-dirs += phytium
 
 subdir-y   := $(dts-dirs)
diff --git a/arch/arm64/boot/dts/phytium/Makefile 
b/arch/arm64/boot/dts/phytium/Makefile
new file mode 100644
index 000..12a22c6
--- /dev/null
+++ b/arch/arm64/boot/dts/phytium/Makefile
@@ -0,0 +1,5 @@
+dtb-$(CONFIG_ARCH_PHYTIUM) += ft1500a-v2-dsk-v2.dtb
+
+always := $(dtb-y)
+subdir-y   := $(dts-dirs)
+clean-files:= *.dtb
diff --git a/arch/arm64/boot/dts/phytium/ft-1500a.dtsi 
b/arch/arm64/boot/dts/phytium/ft-1500a.dtsi
new file mode 100644
index 000..9005389
--- /dev/null
+++ b/arch/arm64/boot/dts/phytium/ft-1500a.dtsi
@@ -0,0 +1,269 @@
+/*
+ * DTS file for Phytium FT-1500A SoC
+ *
+ * Copyright (C) 2015, Phytium Technology Co., Ltd.
+ *
+ * This file is licensed under a dual GPLv2 or BSD license.
+ */
+
+/memreserve/0x8000 0x10;
+/ {
+   compatible = "phytium,ft-1500a";
+   interrupt-parent = <>;
+   #address-cells = <2>;
+   #size-cells = <2>;
+
+   aliases {
+   ethernet0 = 
+   ethernet1 = 
+   };
+
+   cpus {
+   #address-cells = <2>;
+   #size-cells = <0>;
+
+   cpu@0 {
+   device_type = "cpu";
+   compatible = "arm,armv8";
+   reg = <0x0 0x000>;
+   enable-method = "spin-table";
+   cpu-release-addr = <0x0 0x8007fff0>;
+   };
+
+   cpu@1 {
+   device_type = "cpu";
+   compatible = "arm,armv8";
+   reg = <0x0 0x001>;
+   enable-method = "spin-table";
+   cpu-release-addr = <0x0 0x8007fff0>;
+   };
+
+   cpu@2 {
+   device_type = "cpu";
+   compatible = "arm,armv8";
+   reg = <0x0 0x002>;
+   enable-method = "spin-table";
+   cpu-release-addr = <0x0 0x8007fff0>;
+   };
+
+   cpu@3 {
+   device_type = "cpu";
+   compatible = "arm,armv8";
+   reg = <0x0 0x003>;
+   enable-method = "spin-table";
+   cpu-release-addr = <0x0 0x8007fff0>;
+   };
+
+   cpu@4 {
+   device_type = "cpu";
+   compatible = "arm,armv8";
+   reg = <0x0 0x100>;
+   enable-method = "spin-table";
+   cpu-release-addr = <0x0 0x8007fff0>;
+   };
+
+   cpu@5 {
+   device_type = "cpu";
+   compatible = "arm,armv8";
+   reg = <0x0 0x101>;
+   enable-method = "spin-table";
+   cpu-release-addr = <0x0 0x8007fff0>;
+   };
+
+   cpu@6 {
+   device_type = "cpu";
+   compatible = "arm,armv8";
+   reg = <0x0 0x102>;
+   enable-method = "spin-table";
+   cpu-release-addr = <0x0 0x8007fff0>;
+   };
+
+   cpu@7 {
+   device_type = "cpu";
+   compatible = "arm,armv8";
+   reg = <0x0 0x103>;
+   enable-method = "spin-table";
+   cpu-release-addr = <0x0 0x8007fff0>;
+   };
+
+   cpu@8 {
+   device_type = "cpu";
+   compatible = "arm,armv8";
+   reg = <0x0 0x200>;
+   enable-method = "spin-table";
+   cpu-release-addr = <0x0 0x8007fff0>;
+   };
+
+   cpu@9 {
+   device_type = "cpu";
+   compatible = "arm,armv8";
+   reg = <0x0 0x201>;
+   enable-method = "spin-table";
+ 

[PATCH 2/3] arm64, ft-1500a: Add initial dts for Phytium FT-1500A SoC

2015-02-06 Thread Chen Baozi
Add initial device tree nodes for Phytium FT-1500A SoC with support of
16 cores, gicv3 interrupt controller, serial port, PCIe host and
on-chip GMAC ethernet controller.

Signed-off-by: Chen Baozi chenba...@kylinos.com.cn
---
 arch/arm64/boot/dts/Makefile  |   1 +
 arch/arm64/boot/dts/phytium/Makefile  |   5 +
 arch/arm64/boot/dts/phytium/ft-1500a.dtsi | 269 ++
 arch/arm64/boot/dts/phytium/ft1500a-v2-dsk-v2.dts |  39 
 4 files changed, 314 insertions(+)
 create mode 100644 arch/arm64/boot/dts/phytium/Makefile
 create mode 100644 arch/arm64/boot/dts/phytium/ft-1500a.dtsi
 create mode 100644 arch/arm64/boot/dts/phytium/ft1500a-v2-dsk-v2.dts

diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile
index c62b0f4..e7e9e3d 100644
--- a/arch/arm64/boot/dts/Makefile
+++ b/arch/arm64/boot/dts/Makefile
@@ -2,5 +2,6 @@ dts-dirs += amd
 dts-dirs += apm
 dts-dirs += arm
 dts-dirs += cavium
+dts-dirs += phytium
 
 subdir-y   := $(dts-dirs)
diff --git a/arch/arm64/boot/dts/phytium/Makefile 
b/arch/arm64/boot/dts/phytium/Makefile
new file mode 100644
index 000..12a22c6
--- /dev/null
+++ b/arch/arm64/boot/dts/phytium/Makefile
@@ -0,0 +1,5 @@
+dtb-$(CONFIG_ARCH_PHYTIUM) += ft1500a-v2-dsk-v2.dtb
+
+always := $(dtb-y)
+subdir-y   := $(dts-dirs)
+clean-files:= *.dtb
diff --git a/arch/arm64/boot/dts/phytium/ft-1500a.dtsi 
b/arch/arm64/boot/dts/phytium/ft-1500a.dtsi
new file mode 100644
index 000..9005389
--- /dev/null
+++ b/arch/arm64/boot/dts/phytium/ft-1500a.dtsi
@@ -0,0 +1,269 @@
+/*
+ * DTS file for Phytium FT-1500A SoC
+ *
+ * Copyright (C) 2015, Phytium Technology Co., Ltd.
+ *
+ * This file is licensed under a dual GPLv2 or BSD license.
+ */
+
+/memreserve/0x8000 0x10;
+/ {
+   compatible = phytium,ft-1500a;
+   interrupt-parent = gic;
+   #address-cells = 2;
+   #size-cells = 2;
+
+   aliases {
+   ethernet0 = gmac0;
+   ethernet1 = gmac1;
+   };
+
+   cpus {
+   #address-cells = 2;
+   #size-cells = 0;
+
+   cpu@0 {
+   device_type = cpu;
+   compatible = arm,armv8;
+   reg = 0x0 0x000;
+   enable-method = spin-table;
+   cpu-release-addr = 0x0 0x8007fff0;
+   };
+
+   cpu@1 {
+   device_type = cpu;
+   compatible = arm,armv8;
+   reg = 0x0 0x001;
+   enable-method = spin-table;
+   cpu-release-addr = 0x0 0x8007fff0;
+   };
+
+   cpu@2 {
+   device_type = cpu;
+   compatible = arm,armv8;
+   reg = 0x0 0x002;
+   enable-method = spin-table;
+   cpu-release-addr = 0x0 0x8007fff0;
+   };
+
+   cpu@3 {
+   device_type = cpu;
+   compatible = arm,armv8;
+   reg = 0x0 0x003;
+   enable-method = spin-table;
+   cpu-release-addr = 0x0 0x8007fff0;
+   };
+
+   cpu@4 {
+   device_type = cpu;
+   compatible = arm,armv8;
+   reg = 0x0 0x100;
+   enable-method = spin-table;
+   cpu-release-addr = 0x0 0x8007fff0;
+   };
+
+   cpu@5 {
+   device_type = cpu;
+   compatible = arm,armv8;
+   reg = 0x0 0x101;
+   enable-method = spin-table;
+   cpu-release-addr = 0x0 0x8007fff0;
+   };
+
+   cpu@6 {
+   device_type = cpu;
+   compatible = arm,armv8;
+   reg = 0x0 0x102;
+   enable-method = spin-table;
+   cpu-release-addr = 0x0 0x8007fff0;
+   };
+
+   cpu@7 {
+   device_type = cpu;
+   compatible = arm,armv8;
+   reg = 0x0 0x103;
+   enable-method = spin-table;
+   cpu-release-addr = 0x0 0x8007fff0;
+   };
+
+   cpu@8 {
+   device_type = cpu;
+   compatible = arm,armv8;
+   reg = 0x0 0x200;
+   enable-method = spin-table;
+   cpu-release-addr = 0x0 0x8007fff0;
+   };
+
+   cpu@9 {
+   device_type = cpu;
+   compatible = arm,armv8;
+   reg = 0x0 0x201;
+   enable-method = spin-table;
+   cpu-release-addr = 0x0 0x8007fff0;
+