Skip to contentSkip to conversation

arm64, thunder: Enable Cavium Thunder SoC Family

PATCH Superseded Robert Richter 7 participants 35 messages Jul 30, 2014 – Sep 5, 2014

Subsystems (4)

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

cover arm64, thunder: Enable Cavium Thunder SoC Family

From: Robert Richter <[email protected]>

This initial patches enable Cavium Thunder SoC Family. The patches add Kconfig and devicetree support and then add Thunder to the defconfig. The last patch is unrelated to Thunder and enables the tmpfs mount option for a more convinient use of defconfig with distros.

The Thunder system needs more enablement patches for subsystems and devices, this includes network, ahci, gicv3/gicv3-its, pci, smmu, kvm. We will send separate patch sets for these. All of them base on this initial patches.

Patches are available here:

git://git.kernel.org/pub/scm/linux/kernel/git/rric/linux.git thunder/init

Radha Mohan Chintakuntla (3):
arm64, thunder: Add Kconfig option for Cavium Thunder SoC Family arm64, thunder: Add initial dts for Cavium Thunder SoC arm64, thunder: document devicetree bindings for Cavium Thunder SoC

Robert Richter (2):
arm64, defconfig: Enable Cavium Thunder SoC in defconfig arm64, defconfig: Enable tmpfs mount option

 .../devicetree/bindings/arm/cavium-thunder.txt     |  10 +
 Documentation/devicetree/bindings/arm/cpus.txt     |   1 +
 arch/arm64/Kconfig                                 |   6 +
 arch/arm64/boot/dts/Makefile                       |   1 +
 arch/arm64/boot/dts/thunder-88xx.dts               | 387 +++++++++++++++++++++
 arch/arm64/configs/defconfig                       |   2 +
 6 files changed, 407 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/cavium-thunder.txt
 create mode 100644 arch/arm64/boot/dts/thunder-88xx.dts
Signature · 1 line
2.0.1
#1 lore raw

PATCH 1/5 arm64, thunder: Add Kconfig option for Cavium Thunder SoC Family

From: Radha Mohan Chintakuntla <[email protected]>

Increase maximum numbers of cpus to 32. This relates to current maximal possible cpu number. Increasing this to 64 cpus will be a separate patch not part of this enablement patches.

---
 arch/arm64/Kconfig | 6 ++++++
 1 file changed, 6 insertions(+)
arch/arm64/Kconfig +6 -0
index a474de346be6..e989d8cab8c7 100644
@@ -128,6 +128,11 @@ source "kernel/Kconfig.freezer"
 
 menu "Platform selection"
 
+config ARCH_THUNDER
+	bool "Cavium Inc. Thunder SoC Family"
+	help
+	  This enables support for Cavium's Thunder Family of SoCs.
+
 config ARCH_VEXPRESS
 	bool "ARMv8 software model (Versatile Express)"
 	select ARCH_REQUIRE_GPIOLIB
@@ -201,6 +206,7 @@ config NR_CPUS
 	range 2 32
 	depends on SMP
 	# These have to remain sorted largest to smallest
+	default "32" if ARCH_THUNDER
 	default "8"
 
 config HOTPLUG_CPU
Signature · 1 line
2.0.1
#2 lore raw

PATCH 2/5 arm64, thunder: Add initial dts for Cavium Thunder SoC

In reply to Robert Richter

From: Radha Mohan Chintakuntla <[email protected]>

Add initial device tree nodes for Cavium Thunder SoCs with support of 48 cores and gicv3. The dts file requires further changes, esp. for pci, gicv3-its and smmu. This changes will be added later together with the device drivers.

---
 arch/arm64/boot/dts/Makefile         |   1 +
 arch/arm64/boot/dts/thunder-88xx.dts | 387 +++++++++++++++++++++++++++++++++++
 2 files changed, 388 insertions(+)
 create mode 100644 arch/arm64/boot/dts/thunder-88xx.dts
arch/arm64/boot/dts/Makefile +1 -0
index c52bdb051f66..f8001a62029c 100644
@@ -1,3 +1,4 @@
+dtb-$(CONFIG_ARCH_THUNDER) += thunder-88xx.dtb
 dtb-$(CONFIG_ARCH_VEXPRESS) += rtsm_ve-aemv8a.dtb foundation-v8.dtb
 dtb-$(CONFIG_ARCH_XGENE) += apm-mustang.dtb
 
arch/arm64/boot/dts/thunder-88xx.dts new file +387 -0

387 lines not on this page · raw message

Signature · 1 line
2.0.1
reviewer
#3 lore raw

Re: [PATCH 2/5] arm64, thunder: Add initial dts for Cavium Thunder SoC

Hi,

Quoted 34 lines · Robert Richter

From: Radha Mohan Chintakuntla <[email protected]>

Add initial device tree nodes for Cavium Thunder SoCs with support of 48 cores and gicv3. The dts file requires further changes, esp. for pci, gicv3-its and smmu. This changes will be added later together with the device drivers.

---
 arch/arm64/boot/dts/Makefile         |   1 +
 arch/arm64/boot/dts/thunder-88xx.dts | 387 +++++++++++++++++++++++++++++++++++
 2 files changed, 388 insertions(+)
 create mode 100644 arch/arm64/boot/dts/thunder-88xx.dts
arch/arm64/boot/dts/Makefile +1 -0
index c52bdb051f66..f8001a62029c 100644
@@ -1,3 +1,4 @@
+dtb-$(CONFIG_ARCH_THUNDER) += thunder-88xx.dtb
 dtb-$(CONFIG_ARCH_VEXPRESS) += rtsm_ve-aemv8a.dtb foundation-v8.dtb
 dtb-$(CONFIG_ARCH_XGENE) += apm-mustang.dtb
 
arch/arm64/boot/dts/thunder-88xx.dts new file +4 -0
new file mode 100644
index 000000000000..4cf20ac9138b
@@ -0,0 +1,387 @@
+/*
+ * Cavium Thunder DTS file
+ *
+ * Copyright (C) 2013, Cavium Inc.

arch/arm64/boot/dts/thunder-88xx.dts line 14

Quoted 10 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +10 -0
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ */
+/dts-v1/;
+
+/* Reserving first 12MB of DDR for firmware */
+/memreserve/ 0x00000000 0x00c00000;

What exactly is this memreserve intended to protect at runtime?

The only item of runtime firmware I see in use below is PSCI on the secure side.

How is the kernel booted on this platform? UEFI?

arch/arm64/boot/dts/thunder-88xx.dts line 18

Quoted 3 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +3 -0
+/ {
+	model = "Cavium ThunderX CN88XX Family";
+	compatible = "cavium,thunder-88xx";

Please don't use wildcards in compatible strings. Give this an absolute name, and override as necessary.

arch/arm64/boot/dts/thunder-88xx.dts line 31

Quoted 13 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +13 -0
+	interrupt-parent = <&gic0>;
+	#address-cells = <2>;
+	#size-cells = <2>;
+
+	aliases {
+		serial0 = &uaa0;
+		serial1 = &uaa1;
+	};
+
+	psci {
+		compatible = "arm,psci-0.2";
+		method = "smc";
+	};

Nice!

arch/arm64/boot/dts/thunder-88xx.dts line 42

Quoted 11 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +11 -0
+
+	cpus {
+		#address-cells = <2>;
+		#size-cells = <0>;
+
+		cpu at 000 {
+			device_type = "cpu";
+			compatible = "cavium,thunder", "arm,armv8";
+			reg = <0x0 0x000>;
+			enable-method = "psci";
+		};

Just to check: both the SoC and CPU are called thunder?

[...]

arch/arm64/boot/dts/thunder-88xx.dts line 332

Quoted 7 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +7 -0
+
+	memory at 00000000 {
+		device_type = "memory";
+		reg = <0x0 0x00000000 0x0 0x80000000>;
+	};
+
+	gic0: interrupt-controller at 801000000000 {

To make this easier to read, please place a comma between 32-bit portions of the unit address (e.g. here have 8010,00000000).

arch/arm64/boot/dts/thunder-88xx.dts line 337

Quoted 5 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +5 -0
+		compatible = "arm,gic-v3";
+		#interrupt-cells = <3>;
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;

This has no children, so why have ranges, #address-cells, and #size-cells?

Quoted 22 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +22 -0
+		interrupt-controller;
+		reg = <0x8010 0x00000000 0x0 0x010000>, /* GICD */
+		      <0x8010 0x80000000 0x0 0x200000>; /* GICR */
+		interrupts = <1 9 0xf04>;
+	};
+
+	timer {
+		compatible = "arm,armv8-timer";
+		interrupts = <1 13 0xff01>,
+		             <1 14 0xff01>,
+		             <1 11 0xff01>,
+		             <1 10 0xff01>;
+	};
+
+	soc {
+		compatible = "simple-bus";
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		clocks {
+			#address-cells = <2>;

arch/arm64/boot/dts/thunder-88xx.dts line 369

Quoted 10 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +10 -0
+			#size-cells = <2>;
+			ranges;
+
+			refclk50mhz: refclk50mhz {
+				compatible = "fixed-clock";
+				#clock-cells = <0>;
+				clock-frequency = <50000000>;
+				clock-output-names = "refclk50mhz";
+			};
+		};

Please get rid of the clocks node and just put the clocks here.

arch/arm64/boot/dts/thunder-88xx.dts line 376

Quoted 7 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +7 -0
+
+		uaa0: serial at 87e024000000 {
+			compatible = "arm,pl011", "arm,primecell";
+			reg = <0x87e0 0x24000000 0x0 0x1000>;
+			interrupts = <1 21 4>;
+			clocks = <&refclk50mhz>;
+			clock-names = "apb_pclk";

Is this actually the apb_pclk, or is the the uartclk? I assume it's the latter.

arch/arm64/boot/dts/thunder-88xx.dts line 384

Quoted 8 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +8 -0
+		};
+
+		uaa1: serial at 87e025000000 {
+			compatible = "arm,pl011", "arm,primecell";
+			reg = <0x87e0 0x25000000 0x0 0x1000>;
+			interrupts = <1 22 4>;
+			clocks = <&refclk50mhz>;
+			clock-names = "apb_pclk";

Similarly?

Thanks,
Mark.

#4 lore raw

Re: [PATCH 2/5] arm64, thunder: Add initial dts for Cavium Thunder SoC

Quoted 27 lines · Mark Rutland

Hi,

Quoted 24 lines · Robert Richter

From: Radha Mohan Chintakuntla <[email protected]>

Add initial device tree nodes for Cavium Thunder SoCs with support of 48 cores and gicv3. The dts file requires further changes, esp. for pci, gicv3-its and smmu. This changes will be added later together with the device drivers.

---
 arch/arm64/boot/dts/Makefile         |   1 +
 arch/arm64/boot/dts/thunder-88xx.dts | 387 +++++++++++++++++++++++++++++++++++
 2 files changed, 388 insertions(+)
 create mode 100644 arch/arm64/boot/dts/thunder-88xx.dts
arch/arm64/boot/dts/Makefile +1 -0
index c52bdb051f66..f8001a62029c 100644
@@ -1,3 +1,4 @@
+dtb-$(CONFIG_ARCH_THUNDER) += thunder-88xx.dtb
 dtb-$(CONFIG_ARCH_VEXPRESS) += rtsm_ve-aemv8a.dtb foundation-v8.dtb
 dtb-$(CONFIG_ARCH_XGENE) += apm-mustang.dtb
 
Quoted 16 lines
Quoted 6 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +0 -0
new file mode 100644
index 000000000000..4cf20ac9138b

arch/arm64/boot/dts/thunder-88xx.dts line 10

Quoted 10 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +10 -0
@@ -0,0 +1,387 @@
+/*
+ * Cavium Thunder DTS file
+ *
+ * Copyright (C) 2013, Cavium Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ */

You may want to reconsider if this should be BSD.

Quoted 51 lines

arch/arm64/boot/dts/thunder-88xx.dts line 14

Quoted 4 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +4 -0
+/dts-v1/;
+
+/* Reserving first 12MB of DDR for firmware */
+/memreserve/ 0x00000000 0x00c00000;

What exactly is this memreserve intended to protect at runtime?

The only item of runtime firmware I see in use below is PSCI on the secure side.

How is the kernel booted on this platform? UEFI?

arch/arm64/boot/dts/thunder-88xx.dts line 18

Quoted 3 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +3 -0
+/ {
+     model = "Cavium ThunderX CN88XX Family";
+     compatible = "cavium,thunder-88xx";

Please don't use wildcards in compatible strings. Give this an absolute name, and override as necessary.

arch/arm64/boot/dts/thunder-88xx.dts line 31

Quoted 13 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +13 -0
+     interrupt-parent = <&gic0>;
+     #address-cells = <2>;
+     #size-cells = <2>;
+
+     aliases {
+             serial0 = &uaa0;
+             serial1 = &uaa1;
+     };
+
+     psci {
+             compatible = "arm,psci-0.2";
+             method = "smc";
+     };

Nice!

arch/arm64/boot/dts/thunder-88xx.dts line 42

Quoted 11 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +11 -0
+
+     cpus {
+             #address-cells = <2>;
+             #size-cells = <0>;
+
+             cpu at 000 {
+                     device_type = "cpu";
+                     compatible = "cavium,thunder", "arm,armv8";
+                     reg = <0x0 0x000>;
+                     enable-method = "psci";
+             };

Just to check: both the SoC and CPU are called thunder?

[...]

Quoted 10 lines

arch/arm64/boot/dts/thunder-88xx.dts line 332

Quoted 7 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +7 -0
+
+     memory at 00000000 {
+             device_type = "memory";
+             reg = <0x0 0x00000000 0x0 0x80000000>;
+     };
+
+     gic0: interrupt-controller at 801000000000 {

To make this easier to read, please place a comma between 32-bit portions of the unit address (e.g. here have 8010,00000000).

Mark, perhaps a dtc or checkpatch.pl check for this?

This should also be under a bus node.

Quoted 52 lines

arch/arm64/boot/dts/thunder-88xx.dts line 337

Quoted 5 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +5 -0
+             compatible = "arm,gic-v3";
+             #interrupt-cells = <3>;
+             #address-cells = <2>;
+             #size-cells = <2>;
+             ranges;

This has no children, so why have ranges, #address-cells, and #size-cells?

Quoted 22 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +22 -0
+             interrupt-controller;
+             reg = <0x8010 0x00000000 0x0 0x010000>, /* GICD */
+                   <0x8010 0x80000000 0x0 0x200000>; /* GICR */
+             interrupts = <1 9 0xf04>;
+     };
+
+     timer {
+             compatible = "arm,armv8-timer";
+             interrupts = <1 13 0xff01>,
+                          <1 14 0xff01>,
+                          <1 11 0xff01>,
+                          <1 10 0xff01>;
+     };
+
+     soc {
+             compatible = "simple-bus";
+             #address-cells = <2>;
+             #size-cells = <2>;
+             ranges;
+
+             clocks {
+                     #address-cells = <2>;

arch/arm64/boot/dts/thunder-88xx.dts line 369

Quoted 10 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +10 -0
+                     #size-cells = <2>;
+                     ranges;
+
+                     refclk50mhz: refclk50mhz {
+                             compatible = "fixed-clock";
+                             #clock-cells = <0>;
+                             clock-frequency = <50000000>;
+                             clock-output-names = "refclk50mhz";
+                     };
+             };

Please get rid of the clocks node and just put the clocks here.

arch/arm64/boot/dts/thunder-88xx.dts line 376

Quoted 7 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +7 -0
+
+             uaa0: serial at 87e024000000 {
+                     compatible = "arm,pl011", "arm,primecell";
+                     reg = <0x87e0 0x24000000 0x0 0x1000>;
+                     interrupts = <1 21 4>;
+                     clocks = <&refclk50mhz>;
+                     clock-names = "apb_pclk";
Quoted 2 lines

Is this actually the apb_pclk, or is the the uartclk? I assume it's the latter.

Shouldn't new bindings have both clocks here? A single clock was a mistake I think (mine in fact).

Rob

reviewer
#5 lore raw

Re: [PATCH 2/5] arm64, thunder: Add initial dts for Cavium Thunder SoC

[...]

Quoted 6 lines
Quoted 4 lines
Quoted 1 line

+ gic0: interrupt-controller at 801000000000 {

To make this easier to read, please place a comma between 32-bit portions of the unit address (e.g. here have 8010,00000000).

Mark, perhaps a dtc or checkpatch.pl check for this?

Sure. Dodgy first atttempt at checkpatch below.

---->8----

scripts/checkpatch.pl +8 -1
index 182be0f..8aee3f5 100755
@@ -2136,6 +2136,14 @@ sub process {
                        }
                }
 
+# check for difficult-to-read unit-addresses
+               if (defined $root &&
+                       ($realfile =~ /\.dtsi?$/ && $line =~ /([a-z0-9._\-+]++@([0-9a-f]+))\s*{/gi) &&
+                       (length($2) > 8)) {
+                               WARN("LONG_DT_UNIT_ADDRESS",
+                                    "Consider splitting long unit address \"$2\" with a comma between cells\n" . $herecurr);
+               }
+
 # check we are in a valid source file if not then ignore this hunk
                next if ($realfile !~ /\.(h|c|s|S|pl|sh)$/);
----8<----

It would also be nice to check matching unit-address and reg, but doing that correctly requires knowing #address-cells, which sounds a little painful.

I'm not sure where I picked up the comma convention, as it doesn't seem to be in ePAPR. It does seem common though, and is my personal preference:

[mark at leverpostej:~/src/linux]% git grep '@[a-z0-9]\+,[a-b0-9]\+' \
-- arch/arm/boot/dts | wc -l
254
[mark at leverpostej:~/src/linux]% git grep '@[a-z0-9]\+,[a-b0-9]\+' \
-- arch/powerpc/boot/dts | wc -l
370

[mark@leverpostej:~/src/linux]% git grep '@[a-z0-9]\+,[a-b0-9]\+' \
        -- arch/*/boot/dts | wc -l

631

[...]

Quoted 7 lines
Quoted 7 lines
Quoted 6 lines
(quoted diff) +6 -0
+             uaa0: serial at 87e024000000 {
+                     compatible = "arm,pl011", "arm,primecell";
+                     reg = <0x87e0 0x24000000 0x0 0x1000>;
+                     interrupts = <1 21 4>;
+                     clocks = <&refclk50mhz>;
+                     clock-names = "apb_pclk";
Quoted 5 lines
Quoted 2 lines

Is this actually the apb_pclk, or is the the uartclk? I assume it's the latter.

Shouldn't new bindings have both clocks here? A single clock was a mistake I think (mine in fact).

I don't think we fixed it up in the end. It made drivers look a bit messy and it dropped off my priority queue.

Cheers,
Mark.

#6 lore raw

Re: [PATCH 2/5] arm64, thunder: Add initial dts for Cavium Thunder SoC

In reply to Rob Herring

Rob and Arnd,

Quoted 11 lines · Rob Herring
Quoted 10 lines
Quoted 10 lines

+/*
+ * Cavium Thunder DTS file
+ *
+ * Copyright (C) 2013, Cavium Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ */

Quoted 1 line

You may want to reconsider if this should be BSD.

I understand that this is an issue for the inclusion in firmware binaries. Though, we just followed common practice here which is gplv2.

I looked through existing dts files in arch/arm*/dts/* and about 590 of 720 have a gplv2 statement in. The other files don't have any and thus following the COPYING file which is gplv2 too.

It is my understanding that the kernel requires licensing under gplv2. So an option would be to dual license the dts file, e.g. with BSD. How is this typically handled? Are there other repositories where dts files are included with a different license model than gplv2?

Thanks,

-Robert

#7 lore raw

Re: [PATCH 2/5] arm64, thunder: Add initial dts for Cavium Thunder SoC

In reply to Rob Herring
Quoted 10 lines · Rob Herring
Quoted 9 lines · Mark Rutland

arch/arm64/boot/dts/thunder-88xx.dts line 18

Quoted 5 lines · Robert Richter

From: Radha Mohan Chintakuntla <[email protected]>

arch/arm64/boot/dts/thunder-88xx.dts new file +3 -0
+/ {
+     model = "Cavium ThunderX CN88XX Family";
+     compatible = "cavium,thunder-88xx";

Please don't use wildcards in compatible strings. Give this an absolute name, and override as necessary.

The naming 88xx refers to the processor family and arn't actually wildcards. In the future we might need another dts file for 87xx, but so far all SoCs of 88xx family should use the same dts files. In this sense the naming is very specific.

Quoted 11 lines
Quoted 11 lines
Quoted 10 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +10 -0
+     cpus {
+             #address-cells = <2>;
+             #size-cells = <0>;
+
+             cpu at 000 {
+                     device_type = "cpu";
+                     compatible = "cavium,thunder", "arm,armv8";
+                     reg = <0x0 0x000>;
+                     enable-method = "psci";
+             };
Quoted 1 line
Quoted 1 line

Just to check: both the SoC and CPU are called thunder?

The soc is called thunder-88xx, the cpu thunder. E.g. an 87xx soc will have the same core in which is thunder.

Quoted 9 lines
Quoted 9 lines

arch/arm64/boot/dts/thunder-88xx.dts line 332

Quoted 6 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +6 -0
+     memory at 00000000 {
+             device_type = "memory";
+             reg = <0x0 0x00000000 0x0 0x80000000>;
+     };
+
+     gic0: interrupt-controller at 801000000000 {

To make this easier to read, please place a comma between 32-bit portions of the unit address (e.g. here have 8010,00000000).

Changed this.

Quoted 4 lines

Mark, perhaps a dtc or checkpatch.pl check for this?

This should also be under a bus node.

Will do.

Quoted 9 lines
Quoted 9 lines

arch/arm64/boot/dts/thunder-88xx.dts line 337

Quoted 6 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +5 -0
+             compatible = "arm,gic-v3";
+             #interrupt-cells = <3>;
+             #address-cells = <2>;
+             #size-cells = <2>;
+             ranges;

This has no children, so why have ranges, #address-cells, and #size-cells?

Right, this is a leftover from a change in a follow on patch that introduces a child for its. Will remove #address-cells, #size-cells and ranges in this patch and move the change to the later patch.

Quoted 20 lines
Quoted 20 lines
Quoted 19 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +17 -0
+             interrupt-controller;
+             reg = <0x8010 0x00000000 0x0 0x010000>, /* GICD */
+                   <0x8010 0x80000000 0x0 0x200000>; /* GICR */
+             interrupts = <1 9 0xf04>;
+     };
 
+             clocks {
+                     #address-cells = <2>;
+                     #size-cells = <2>;
+                     ranges;
+
+                     refclk50mhz: refclk50mhz {
+                             compatible = "fixed-clock";
+                             #clock-cells = <0>;
+                             clock-frequency = <50000000>;
+                             clock-output-names = "refclk50mhz";
+                     };
+             };
Quoted 1 line
Quoted 1 line

Please get rid of the clocks node and just put the clocks here.

Will do.

Quoted 9 lines
Quoted 9 lines
Quoted 8 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +7 -0
+
+             uaa0: serial at 87e024000000 {
+                     compatible = "arm,pl011", "arm,primecell";
+                     reg = <0x87e0 0x24000000 0x0 0x1000>;
+                     interrupts = <1 21 4>;
+                     clocks = <&refclk50mhz>;
+                     clock-names = "apb_pclk";
Quoted 5 lines
Quoted 2 lines

Is this actually the apb_pclk, or is the the uartclk? I assume it's the latter.

Shouldn't new bindings have both clocks here? A single clock was a mistake I think (mine in fact).

Do you mean

			clock-names = "uartclk", "apb_pclk";

here?

Thanks,

-Robert

#8 lore raw

Re: [PATCH 2/5] arm64, thunder: Add initial dts for Cavium Thunder SoC

Quoted 9 lines · Robert Richter
Quoted 8 lines · Rob Herring
Quoted 7 lines · Mark Rutland
Quoted 5 lines · Robert Richter

From: Radha Mohan Chintakuntla <[email protected]>

arch/arm64/boot/dts/thunder-88xx.dts new file +3 -0
+/ {
+     model = "Cavium ThunderX CN88XX Family";
+     compatible = "cavium,thunder-88xx";
Quoted 7 lines
Quoted 2 lines
Quoted 2 lines

Please don't use wildcards in compatible strings. Give this an absolute name, and override as necessary.

The naming 88xx refers to the processor family and arn't actually wildcards. In the future we might need another dts file for 87xx, but so far all SoCs of 88xx family should use the same dts files. In this sense the naming is very specific.

Yes, but each implementation can have its own errata. You might not need to distinguish them now, but you could in the future.

However, if the family is really all the same die and different parts are just marketing, then the name is fine. Or if you can easily probe the exact part and revision it's probably fine.

Quoted 13 lines
Quoted 13 lines
Quoted 13 lines
Quoted 12 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +10 -0
+     cpus {
+             #address-cells = <2>;
+             #size-cells = <0>;
+
+             cpu at 000 {
+                     device_type = "cpu";
+                     compatible = "cavium,thunder", "arm,armv8";
+                     reg = <0x0 0x000>;
+                     enable-method = "psci";
+             };
Quoted 4 lines
Quoted 1 line
Quoted 1 line

Just to check: both the SoC and CPU are called thunder?

The soc is called thunder-88xx, the cpu thunder. E.g. an 87xx soc will have the same core in which is thunder.

And the next version of the core would be called something else? thunder-v2? lightning? As long as they are distinguishable they should be fine.

Rob

Quoted 71 lines
Quoted 11 lines
Quoted 11 lines

arch/arm64/boot/dts/thunder-88xx.dts line 332

Quoted 8 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +6 -0
+     memory at 00000000 {
+             device_type = "memory";
+             reg = <0x0 0x00000000 0x0 0x80000000>;
+     };
+
+     gic0: interrupt-controller at 801000000000 {

To make this easier to read, please place a comma between 32-bit portions of the unit address (e.g. here have 8010,00000000).

Changed this.

Quoted 4 lines

Mark, perhaps a dtc or checkpatch.pl check for this?

This should also be under a bus node.

Will do.

Quoted 9 lines
Quoted 9 lines

arch/arm64/boot/dts/thunder-88xx.dts line 337

Quoted 6 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +5 -0
+             compatible = "arm,gic-v3";
+             #interrupt-cells = <3>;
+             #address-cells = <2>;
+             #size-cells = <2>;
+             ranges;

This has no children, so why have ranges, #address-cells, and #size-cells?

Right, this is a leftover from a change in a follow on patch that introduces a child for its. Will remove #address-cells, #size-cells and ranges in this patch and move the change to the later patch.

Quoted 20 lines
Quoted 20 lines
Quoted 19 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +17 -0
+             interrupt-controller;
+             reg = <0x8010 0x00000000 0x0 0x010000>, /* GICD */
+                   <0x8010 0x80000000 0x0 0x200000>; /* GICR */
+             interrupts = <1 9 0xf04>;
+     };
 
+             clocks {
+                     #address-cells = <2>;
+                     #size-cells = <2>;
+                     ranges;
+
+                     refclk50mhz: refclk50mhz {
+                             compatible = "fixed-clock";
+                             #clock-cells = <0>;
+                             clock-frequency = <50000000>;
+                             clock-output-names = "refclk50mhz";
+                     };
+             };
Quoted 1 line
Quoted 1 line

Please get rid of the clocks node and just put the clocks here.

Will do.

Quoted 12 lines
Quoted 11 lines

arch/arm64/boot/dts/thunder-88xx.dts line 376

Quoted 8 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +7 -0
+
+             uaa0: serial at 87e024000000 {
+                     compatible = "arm,pl011", "arm,primecell";
+                     reg = <0x87e0 0x24000000 0x0 0x1000>;
+                     interrupts = <1 21 4>;
+                     clocks = <&refclk50mhz>;
+                     clock-names = "apb_pclk";

Is this actually the apb_pclk, or is the the uartclk? I assume it's the latter.

Quoted 6 lines
Quoted 2 lines

Shouldn't new bindings have both clocks here? A single clock was a mistake I think (mine in fact).

Do you mean

                        clock-names = "uartclk", "apb_pclk";

here?

Yes, but Mark said this change never happened so maybe it is fine. In any case, follow the pl011 binding documentation.

Rob

#9 lore raw

Re: [PATCH 2/5] arm64, thunder: Add initial dts for Cavium Thunder SoC

Quoted 18 lines · Rob Herring
Quoted 9 lines · Robert Richter
Quoted 8 lines · Rob Herring
Quoted 7 lines · Mark Rutland
Quoted 5 lines · Robert Richter

From: Radha Mohan Chintakuntla <[email protected]>

arch/arm64/boot/dts/thunder-88xx.dts new file +3 -0
+/ {
+     model = "Cavium ThunderX CN88XX Family";
+     compatible = "cavium,thunder-88xx";
Quoted 7 lines
Quoted 2 lines
Quoted 2 lines

Please don't use wildcards in compatible strings. Give this an absolute name, and override as necessary.

The naming 88xx refers to the processor family and arn't actually wildcards. In the future we might need another dts file for 87xx, but so far all SoCs of 88xx family should use the same dts files. In this sense the naming is very specific.

Quoted 6 lines

Yes, but each implementation can have its own errata. You might not need to distinguish them now, but you could in the future.

However, if the family is really all the same die and different parts are just marketing, then the name is fine. Or if you can easily probe the exact part and revision it's probably fine.

Yes, this relates to the same soc.

Quoted 14 lines
Quoted 14 lines
Quoted 14 lines
Quoted 14 lines
Quoted 13 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +10 -0
+     cpus {
+             #address-cells = <2>;
+             #size-cells = <0>;
+
+             cpu at 000 {
+                     device_type = "cpu";
+                     compatible = "cavium,thunder", "arm,armv8";
+                     reg = <0x0 0x000>;
+                     enable-method = "psci";
+             };
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
Quoted 1 line

Just to check: both the SoC and CPU are called thunder?

The soc is called thunder-88xx, the cpu thunder. E.g. an 87xx soc will have the same core in which is thunder.

And the next version of the core would be called something else? thunder-v2? lightning? As long as they are distinguishable they should be fine.

Same here, the name relates to the same core.

-Robert

#10 lore raw

Re: [PATCH 2/5] arm64, thunder: Add initial dts for Cavium Thunder SoC

In reply to Rob Herring
Quoted 15 lines · Mark Rutland
Quoted 7 lines
Quoted 7 lines
Quoted 6 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +6 -0
+             uaa0: serial at 87e024000000 {
+                     compatible = "arm,pl011", "arm,primecell";
+                     reg = <0x87e0 0x24000000 0x0 0x1000>;
+                     interrupts = <1 21 4>;
+                     clocks = <&refclk50mhz>;
+                     clock-names = "apb_pclk";
Quoted 5 lines
Quoted 2 lines

Is this actually the apb_pclk, or is the the uartclk? I assume it's the latter.

Shouldn't new bindings have both clocks here? A single clock was a mistake I think (mine in fact).

I don't think we fixed it up in the end. It made drivers look a bit messy and it dropped off my priority queue.

Quoted 18 lines · Rob Herring
Quoted 17 lines · Robert Richter
Quoted 15 lines · Rob Herring
Quoted 11 lines · Mark Rutland

arch/arm64/boot/dts/thunder-88xx.dts line 376

Quoted 7 lines · Robert Richter

From: Radha Mohan Chintakuntla <[email protected]>

arch/arm64/boot/dts/thunder-88xx.dts new file +6 -0
+             uaa0: serial at 87e024000000 {
+                     compatible = "arm,pl011", "arm,primecell";
+                     reg = <0x87e0 0x24000000 0x0 0x1000>;
+                     interrupts = <1 21 4>;
+                     clocks = <&refclk50mhz>;
+                     clock-names = "apb_pclk";

Is this actually the apb_pclk, or is the the uartclk? I assume it's the latter.

Shouldn't new bindings have both clocks here? A single clock was a mistake I think (mine in fact).

Quoted 6 lines
Quoted 3 lines

Do you mean

                        clock-names = "uartclk", "apb_pclk";

here?

Yes, but Mark said this change never happened so maybe it is fine. In any case, follow the pl011 binding documentation.

Right:

Documentation/devicetree/bindings/arm/primecell.txt:
- clocks : From common clock binding. First clock is phandle to clock for apb pclk. Additional clocks are optional and specific to those peripherals.
- clock-names : From common clock binding. Shall be "apb_pclk" for first clock.

Documentation/devicetree/bindings/serial/pl011.txt
- clocks: When present, must refer to exactly one clock named

           "apb_pclk"

There is no binding documentation at all for "uartclk".

It seems we should should leave clock-names = "apb_pclk" as it is.

Mark, Rob, would this be ok?

Thanks,

-Robert

#11 lore raw

Re: [PATCH 2/5] arm64, thunder: Add initial dts for Cavium Thunder SoC

In reply to Mark Rutland
Quoted 55 lines · Mark Rutland

Hi,

Quoted 7 lines · Robert Richter

From: Radha Mohan Chintakuntla <[email protected]>

Add initial device tree nodes for Cavium Thunder SoCs with support of 48 cores and gicv3. The dts file requires further changes, esp. for pci, gicv3-its and smmu. This changes will be added later together with the device drivers.

Quoted 5 lines
---
 arch/arm64/boot/dts/Makefile         |   1 +
 arch/arm64/boot/dts/thunder-88xx.dts | 387

+++++++++++++++++++++++++++++++++++

Review

Quoted 13 lines
 2 files changed, 388 insertions(+)
 create mode 100644 arch/arm64/boot/dts/thunder-88xx.dts
arch/arm64/boot/dts/Makefile +1 -0
index c52bdb051f66..f8001a62029c 100644
@@ -1,3 +1,4 @@
+dtb-$(CONFIG_ARCH_THUNDER) += thunder-88xx.dtb
 dtb-$(CONFIG_ARCH_VEXPRESS) += rtsm_ve-aemv8a.dtb foundation-v8.dtb
 dtb-$(CONFIG_ARCH_XGENE) += apm-mustang.dtb
 
(quoted diff) +0 -0

b/arch/arm64/boot/dts/thunder-88xx.dts

Quoted 9 lines

new file mode 100644
index 000000000000..4cf20ac9138b

arch/arm64/boot/dts/thunder-88xx.dts new file +4 -0
@@ -0,0 +1,387 @@
+/*
+ * Cavium Thunder DTS file
+ *
+ * Copyright (C) 2013, Cavium Inc.

arch/arm64/boot/dts/thunder-88xx.dts line 14

Quoted 10 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +10 -0
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ */
+/dts-v1/;
+
+/* Reserving first 12MB of DDR for firmware */
+/memreserve/ 0x00000000 0x00c00000;

What exactly is this memreserve intended to protect at runtime? Yes, this 12 MB is reserved for ATF and UEFI boot and run-time services.

Quoted 5 lines

The only item of runtime firmware I see in use below is PSCI on the secure side.

How is the kernel booted on this platform? UEFI?

Boot sequence tried is ATF->UEFI->Linux and ATF->UEFI->GRUB->Linux

Quoted 116 lines

arch/arm64/boot/dts/thunder-88xx.dts line 18

Quoted 3 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +3 -0
+/ {
+     model = "Cavium ThunderX CN88XX Family";
+     compatible = "cavium,thunder-88xx";

Please don't use wildcards in compatible strings. Give this an absolute name, and override as necessary.

arch/arm64/boot/dts/thunder-88xx.dts line 31

Quoted 13 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +13 -0
+     interrupt-parent = <&gic0>;
+     #address-cells = <2>;
+     #size-cells = <2>;
+
+     aliases {
+             serial0 = &uaa0;
+             serial1 = &uaa1;
+     };
+
+     psci {
+             compatible = "arm,psci-0.2";
+             method = "smc";
+     };

Nice!

arch/arm64/boot/dts/thunder-88xx.dts line 42

Quoted 11 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +11 -0
+
+     cpus {
+             #address-cells = <2>;
+             #size-cells = <0>;
+
+             cpu@000 {
+                     device_type = "cpu";
+                     compatible = "cavium,thunder", "arm,armv8";
+                     reg = <0x0 0x000>;
+                     enable-method = "psci";
+             };

Just to check: both the SoC and CPU are called thunder?

[...]

arch/arm64/boot/dts/thunder-88xx.dts

Quoted 7 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +7 -0
+
+     memory@00000000 {
+             device_type = "memory";
+             reg = <0x0 0x00000000 0x0 0x80000000>;
+     };
+
+     gic0: interrupt-controller@801000000000 {

To make this easier to read, please place a comma between 32-bit portions of the unit address (e.g. here have 8010,00000000).

arch/arm64/boot/dts/thunder-88xx.dts line 337

Quoted 5 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +5 -0
+             compatible = "arm,gic-v3";
+             #interrupt-cells = <3>;
+             #address-cells = <2>;
+             #size-cells = <2>;
+             ranges;

This has no children, so why have ranges, #address-cells, and #size-cells?

Quoted 22 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +22 -0
+             interrupt-controller;
+             reg = <0x8010 0x00000000 0x0 0x010000>, /* GICD */
+                   <0x8010 0x80000000 0x0 0x200000>; /* GICR */
+             interrupts = <1 9 0xf04>;
+     };
+
+     timer {
+             compatible = "arm,armv8-timer";
+             interrupts = <1 13 0xff01>,
+                          <1 14 0xff01>,
+                          <1 11 0xff01>,
+                          <1 10 0xff01>;
+     };
+
+     soc {
+             compatible = "simple-bus";
+             #address-cells = <2>;
+             #size-cells = <2>;
+             ranges;
+
+             clocks {
+                     #address-cells = <2>;

arch/arm64/boot/dts/thunder-88xx.dts line 369

Quoted 10 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +10 -0
+                     #size-cells = <2>;
+                     ranges;
+
+                     refclk50mhz: refclk50mhz {
+                             compatible = "fixed-clock";
+                             #clock-cells = <0>;
+                             clock-frequency = <50000000>;
+                             clock-output-names = "refclk50mhz";
+                     };
+             };

Please get rid of the clocks node and just put the clocks here.

arch/arm64/boot/dts/thunder-88xx.dts line 376

Quoted 7 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +7 -0
+
+             uaa0: serial@87e024000000 {
+                     compatible = "arm,pl011", "arm,primecell";
+                     reg = <0x87e0 0x24000000 0x0 0x1000>;
+                     interrupts = <1 21 4>;
+                     clocks = <&refclk50mhz>;
+                     clock-names = "apb_pclk";

Is this actually the apb_pclk, or is the the uartclk? I assume it's the latter.

arch/arm64/boot/dts/thunder-88xx.dts line 384

Quoted 8 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +8 -0
+             };
+
+             uaa1: serial@87e025000000 {
+                     compatible = "arm,pl011", "arm,primecell";
+                     reg = <0x87e0 0x25000000 0x0 0x1000>;
+                     interrupts = <1 22 4>;
+                     clocks = <&refclk50mhz>;
+                     clock-names = "apb_pclk";

Similarly?

Quoted 8 lines

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

thanks
Ganapat

reviewer
#12 lore raw

Re: [PATCH 2/5] arm64, thunder: Add initial dts for Cavium Thunder SoC

Quoted 47 lines · Ganapatrao Kulkarni
   On Wed, Jul 30, 2014 at 9:16 PM, Mark Rutland <[1][email protected]>
   wrote:

     Hi,
     On Wed, Jul 30, 2014 at 04:06:31PM +0100, Robert Richter wrote:
     > From: Radha Mohan Chintakuntla <[2][email protected]>
     >
     > Add initial device tree nodes for Cavium Thunder SoCs with support of
     > 48 cores and gicv3. The dts file requires further changes, esp. for
     > pci, gicv3-its and smmu. This changes will be added later together
     > with the device drivers.
     >
     > Signed-off-by: Radha Mohan Chintakuntla <[3][email protected]>
     > Signed-off-by: Robert Richter <[4][email protected]>
     > ---
     > ?arch/arm64/boot/dts/Makefile ? ? ? ? | ? 1 +
     > ?arch/arm64/boot/dts/thunder-88xx.dts | 387
     +++++++++++++++++++++++++++++++++++
     > ?2 files changed, 388 insertions(+)
     > ?create mode 100644 arch/arm64/boot/dts/thunder-88xx.dts
     >
     > diff --git a/arch/arm64/boot/dts/Makefile
     b/arch/arm64/boot/dts/Makefile
     > index c52bdb051f66..f8001a62029c 100644
     > --- a/arch/arm64/boot/dts/Makefile
     > +++ b/arch/arm64/boot/dts/Makefile
     > @@ -1,3 +1,4 @@
     > +dtb-$(CONFIG_ARCH_THUNDER) += thunder-88xx.dtb
     > ?dtb-$(CONFIG_ARCH_VEXPRESS) += rtsm_ve-aemv8a.dtb foundation-v8.dtb
     > ?dtb-$(CONFIG_ARCH_XGENE) += apm-mustang.dtb
     >
     > diff --git a/arch/arm64/boot/dts/thunder-88xx.dts
     b/arch/arm64/boot/dts/thunder-88xx.dts
     > new file mode 100644
     > index 000000000000..4cf20ac9138b
     > --- /dev/null
     > +++ b/arch/arm64/boot/dts/thunder-88xx.dts
     > @@ -0,0 +1,387 @@
     > +/*
     > + * Cavium Thunder DTS file
     > + *
     > + * Copyright (C) 2013, Cavium Inc.
     > + *
     > + * This program is free software; you can redistribute it and/or
     > + * modify it under the terms of the GNU General Public License as
     > + * published by the Free Software Foundation; either version 2 of
     > + * the License, or (at your option) any later version.
Quoted 8 lines
     > + */
     > +/dts-v1/;
     > +
     > +/* Reserving first 12MB of DDR for firmware */
     > +/memreserve/ 0x00000000 0x00c00000;

     What exactly is this memreserve intended to protect at runtime?
     Yes, this 12 MB is reserved for ATF and UEFI boot and run-time services.

If booted as an EFI application Linux will use the UEFI memory map. Anything UEFI needs to have kept around will be marked as such, so there's no need to memreserve that.

I was under the impression that the ARM Trusted Firmware didn't need anything resident on the non-secure side, so I don't see why that needs a memreserve -- Linux should not be able to address anything it has resident.

Quoted 6 lines

The only item of runtime firmware I see in use below is PSCI on the secure side.

How is the kernel booted on this platform? UEFI?

Boot sequence tried is ATF->UEFI->Linux? and ATF->UEFI->GRUB->Linux?

As I've just had it explained to me, in either of those cases we should enter Linux via the EFI stub. So we should be using the UEFI memory map regardless.

Thanks,
Mark.

#13 lore raw

Re: [PATCH 2/5] arm64, thunder: Add initial dts for Cavium Thunder SoC

Quoted 61 lines · Mark Rutland
Quoted 1 line · Ganapatrao Kulkarni

On Wed, Jul 30, 2014 at 9:16 PM, Mark Rutland <[1]

[email protected]>

Quoted 7 lines

wrote:

     Hi,
     On Wed, Jul 30, 2014 at 04:06:31PM +0100, Robert Richter wrote:
     > From: Radha Mohan Chintakuntla <[2][email protected]>
     >
     > Add initial device tree nodes for Cavium Thunder SoCs with

support of

Quoted 1 line

> 48 cores and gicv3. The dts file requires further changes, esp.

for

Quoted 4 lines

> pci, gicv3-its and smmu. This changes will be added later together > with the device drivers.
>
> Signed-off-by: Radha Mohan Chintakuntla <[3]

[email protected]>

Quoted 8 lines
     > Signed-off-by: Robert Richter <[4][email protected]>
     > ---
     >  arch/arm64/boot/dts/Makefile         |   1 +
     >  arch/arm64/boot/dts/thunder-88xx.dts | 387
     +++++++++++++++++++++++++++++++++++
     >  2 files changed, 388 insertions(+)
     >  create mode 100644 arch/arm64/boot/dts/thunder-88xx.dts
     >
Quoted 8 lines
     > diff --git a/arch/arm64/boot/dts/Makefile
     b/arch/arm64/boot/dts/Makefile
     > index c52bdb051f66..f8001a62029c 100644
     > --- a/arch/arm64/boot/dts/Makefile
     > +++ b/arch/arm64/boot/dts/Makefile
     > @@ -1,3 +1,4 @@
     > +dtb-$(CONFIG_ARCH_THUNDER) += thunder-88xx.dtb
     >  dtb-$(CONFIG_ARCH_VEXPRESS) += rtsm_ve-aemv8a.dtb

foundation-v8.dtb

Quoted 18 lines
     >  dtb-$(CONFIG_ARCH_XGENE) += apm-mustang.dtb
     >
     > diff --git a/arch/arm64/boot/dts/thunder-88xx.dts
     b/arch/arm64/boot/dts/thunder-88xx.dts
     > new file mode 100644
     > index 000000000000..4cf20ac9138b
     > --- /dev/null
     > +++ b/arch/arm64/boot/dts/thunder-88xx.dts
     > @@ -0,0 +1,387 @@
     > +/*
     > + * Cavium Thunder DTS file
     > + *
     > + * Copyright (C) 2013, Cavium Inc.
     > + *
     > + * This program is free software; you can redistribute it and/or
     > + * modify it under the terms of the GNU General Public License as
     > + * published by the Free Software Foundation; either version 2 of
     > + * the License, or (at your option) any later version.
Quoted 8 lines
     > + */
     > +/dts-v1/;
     > +
     > +/* Reserving first 12MB of DDR for firmware */
     > +/memreserve/ 0x00000000 0x00c00000;

     What exactly is this memreserve intended to protect at runtime?
     Yes, this 12 MB is reserved for ATF and UEFI boot and run-time
Quoted 7 lines

services.

If booted as an EFI application Linux will use the UEFI memory map. Anything UEFI needs to have kept around will be marked as such, so there's no need to memreserve that.

We are loading ATF and UEFI from flash(which is not XIP) within 12MB of

DDR.
I dont see UEFI stub freeing RAM address where UEFI image is loaded.

Quoted 5 lines

I was under the impression that the ARM Trusted Firmware didn't need anything resident on the non-secure side, so I don't see why that needs a memreserve -- Linux should not be able to address anything it has resident.

We mark RAM used by ATF as secure-RAM, however we don't support secure/non-secure address aliasing.
i.e, a DRAM address that can be referenced from both a secure PA and a non-secure PA is not allowed.

Quoted 15 lines
Quoted 7 lines

The only item of runtime firmware I see in use below is PSCI on the secure side.

How is the kernel booted on this platform? UEFI?

Boot sequence tried is ATF->UEFI->Linux and ATF->UEFI->GRUB->Linux

As I've just had it explained to me, in either of those cases we should enter Linux via the EFI stub. So we should be using the UEFI memory map regardless.

Thanks,
Mark.

reviewer
#14 lore raw

Re: [PATCH 2/5] arm64, thunder: Add initial dts for Cavium Thunder SoC

Please send plain text rather than HTML email; this is becoming painful to reply to.

Quoted 70 lines · Ganapatrao Kulkarni
   On Thu, Jul 31, 2014 at 3:23 PM, Mark Rutland <[1][email protected]>
   wrote:

     On Thu, Jul 31, 2014 at 09:41:10AM +0100, Ganapatrao Kulkarni wrote:
     > ? ?On Wed, Jul 30, 2014 at 9:16 PM, Mark Rutland
     <[1][2][email protected]>
     > ? ?wrote:
     >
     > ? ? ?Hi,
     > ? ? ?On Wed, Jul 30, 2014 at 04:06:31PM +0100, Robert Richter wrote:
     > ? ? ?> From: Radha Mohan Chintakuntla <[2][3][email protected]>
     > ? ? ?>
     > ? ? ?> Add initial device tree nodes for Cavium Thunder SoCs with
     support of
     > ? ? ?> 48 cores and gicv3. The dts file requires further changes, esp.
     for
     > ? ? ?> pci, gicv3-its and smmu. This changes will be added later
     together
     > ? ? ?> with the device drivers.
     > ? ? ?>
     > ? ? ?> Signed-off-by: Radha Mohan Chintakuntla
     <[3][4][email protected]>
     > ? ? ?> Signed-off-by: Robert Richter <[4][5][email protected]>
     > ? ? ?> ---
     > ? ? ?> ?arch/arm64/boot/dts/Makefile ? ? ? ? | ? 1 +
     > ? ? ?> ?arch/arm64/boot/dts/thunder-88xx.dts | 387
     > ? ? ?+++++++++++++++++++++++++++++++++++
     > ? ? ?> ?2 files changed, 388 insertions(+)
     > ? ? ?> ?create mode 100644 arch/arm64/boot/dts/thunder-88xx.dts
     > ? ? ?>
     > ? ? ?> diff --git a/arch/arm64/boot/dts/Makefile
     > ? ? ?b/arch/arm64/boot/dts/Makefile
     > ? ? ?> index c52bdb051f66..f8001a62029c 100644
     > ? ? ?> --- a/arch/arm64/boot/dts/Makefile
     > ? ? ?> +++ b/arch/arm64/boot/dts/Makefile
     > ? ? ?> @@ -1,3 +1,4 @@
     > ? ? ?> +dtb-$(CONFIG_ARCH_THUNDER) += thunder-88xx.dtb
     > ? ? ?> ?dtb-$(CONFIG_ARCH_VEXPRESS) += rtsm_ve-aemv8a.dtb
     foundation-v8.dtb
     > ? ? ?> ?dtb-$(CONFIG_ARCH_XGENE) += apm-mustang.dtb
     > ? ? ?>
     > ? ? ?> diff --git a/arch/arm64/boot/dts/thunder-88xx.dts
     > ? ? ?b/arch/arm64/boot/dts/thunder-88xx.dts
     > ? ? ?> new file mode 100644
     > ? ? ?> index 000000000000..4cf20ac9138b
     > ? ? ?> --- /dev/null
     > ? ? ?> +++ b/arch/arm64/boot/dts/thunder-88xx.dts
     > ? ? ?> @@ -0,0 +1,387 @@
     > ? ? ?> +/*
     > ? ? ?> + * Cavium Thunder DTS file
     > ? ? ?> + *
     > ? ? ?> + * Copyright (C) 2013, Cavium Inc.
     > ? ? ?> + *
     > ? ? ?> + * This program is free software; you can redistribute it
     and/or
     > ? ? ?> + * modify it under the terms of the GNU General Public License
     as
     > ? ? ?> + * published by the Free Software Foundation; either version 2
     of
     > ? ? ?> + * the License, or (at your option) any later version.
     > ? ? ?> + */
     > ? ? ?> +/dts-v1/;
     > ? ? ?> +
     > ? ? ?> +/* Reserving first 12MB of DDR for firmware */
     > ? ? ?> +/memreserve/ 0x00000000 0x00c00000;
     >
     > ? ? ?What exactly is this memreserve intended to protect at runtime?
     > ? ? ?Yes, this 12 MB is reserved for ATF and UEFI boot and run-time
     services.
Quoted 7 lines
     If booted as an EFI application Linux will use the UEFI memory map.
     Anything UEFI needs to have kept around will be marked as such, so
     there's no need to memreserve that.

   We are loading ATF and UEFI from flash(which is not XIP) within 12MB of
   DDR.
   ?I dont see UEFI stub freeing RAM address where UEFI? image is loaded.

Therefore we don't need the memreserve, then?

Quoted 9 lines

I was under the impression that the ARM Trusted Firmware didn't need anything resident on the non-secure side, so I don't see why that needs a memreserve -- Linux should not be able to address anything it has resident.

We mark RAM used by ATF as secure-RAM, however we don't support secure/non-secure address aliasing.
i.e, a DRAM address that can be referenced from both a secure PA and a non-secure PA is not allowed.

What exactly do you mean by "not allowed"?

If Linux maps that memory, what happens?

What if Linux tried to read or write to it?

If Linux should not map that memory, it should not be described in the memory map to begin with.

Thanks,
Mark.

#15 lore raw

Re: [PATCH 2/5] arm64, thunder: Add initial dts for Cavium Thunder SoC

Mark,

Quoted 7 lines · Mark Rutland
Quoted 4 lines · Ganapatrao Kulkarni

We mark RAM used by ATF as secure-RAM, however we don't support secure/non-secure address aliasing.
i.e, a DRAM address that can be referenced from both a secure PA and a non-secure PA is not allowed.

What exactly do you mean by "not allowed"?

It actually means "not possible" since secure and non-secure memory is kept in separate address ranges.

Quoted 6 lines

If Linux maps that memory, what happens?

What if Linux tried to read or write to it?

If Linux should not map that memory, it should not be described in the memory map to begin with.

Linux never will see secure-RAM. Firmware must be sure to report the correct non-secure memory ranges to the OS (e.g. unsecure mem size = total size - secure mem size).

-Robert

reviewer
#16 lore raw

Re: [PATCH 2/5] arm64, thunder: Add initial dts for Cavium Thunder SoC

Quoted 1 line · Robert Richter

Mark,

Hi Robert,

Quoted 11 lines · Mark Rutland
Quoted 7 lines · Ganapatrao Kulkarni
Quoted 4 lines

We mark RAM used by ATF as secure-RAM, however we don't support secure/non-secure address aliasing.
i.e, a DRAM address that can be referenced from both a secure PA and a non-secure PA is not allowed.

What exactly do you mean by "not allowed"?

It actually means "not possible" since secure and non-secure memory is kept in separate address ranges.

I understand that the two are separate physical address spaces, but Ganapatrao's reply was somewhat ambiguous and it wasn't clear to me that the memory was actually marked as secure.

Quoted 10 lines
Quoted 6 lines

If Linux maps that memory, what happens?

What if Linux tried to read or write to it?

If Linux should not map that memory, it should not be described in the memory map to begin with.

Linux never will see secure-RAM. Firmware must be sure to report the correct non-secure memory ranges to the OS (e.g. unsecure mem size = total size - secure mem size).

Ok, that's what I had hoped for and that makes sense.

The issue was that the memory node contained an address range that was supposedly secure-only (which Linux could attempt to map), which was 'protected' with a /memreserve/ (which does not stop it from being mapped).

Given they are unnecessary (unless you want to bypass EFI for some reason) they can be dropped.

Thanks,
Mark.

#17 lore raw

Re: [PATCH 2/5] arm64, thunder: Add initial dts for Cavium Thunder SoC

In reply to Mark Rutland

Mark,

Quoted 23 lines · Mark Rutland
Quoted 10 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +4 -0
new file mode 100644
index 000000000000..4cf20ac9138b
@@ -0,0 +1,387 @@
+/*
+ * Cavium Thunder DTS file
+ *
+ * Copyright (C) 2013, Cavium Inc.

arch/arm64/boot/dts/thunder-88xx.dts line 14

Quoted 10 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +10 -0
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ */
+/dts-v1/;
+
+/* Reserving first 12MB of DDR for firmware */
+/memreserve/ 0x00000000 0x00c00000;

What exactly is this memreserve intended to protect at runtime?

Quoted 4 lines

The only item of runtime firmware I see in use below is PSCI on the secure side.

How is the kernel booted on this platform? UEFI?

We checked this and it seems we don't need memreserve. Mem is either not visible to the kernel (ATF) or it is marked reserved by firmware (UEFI). At the moment we don't see any other use cases. We will remove this memreserve.

Thanks,

-Robert

#18 lore raw

Re: [PATCH 2/5] arm64, thunder: Add initial dts for Cavium Thunder SoC

In reply to Robert Richter
Quoted 38 lines · Robert Richter

From: Radha Mohan Chintakuntla <[email protected]>

Add initial device tree nodes for Cavium Thunder SoCs with support of 48 cores and gicv3. The dts file requires further changes, esp. for pci, gicv3-its and smmu. This changes will be added later together with the device drivers.

---
 arch/arm64/boot/dts/Makefile         |   1 +
 arch/arm64/boot/dts/thunder-88xx.dts | 387 +++++++++++++++++++++++++++++++++++
 2 files changed, 388 insertions(+)
 create mode 100644 arch/arm64/boot/dts/thunder-88xx.dts
arch/arm64/boot/dts/Makefile +1 -0
index c52bdb051f66..f8001a62029c 100644
@@ -1,3 +1,4 @@
+dtb-$(CONFIG_ARCH_THUNDER) += thunder-88xx.dtb
 dtb-$(CONFIG_ARCH_VEXPRESS) += rtsm_ve-aemv8a.dtb foundation-v8.dtb
 dtb-$(CONFIG_ARCH_XGENE) += apm-mustang.dtb
 
arch/arm64/boot/dts/thunder-88xx.dts new file +8 -0
new file mode 100644
index 000000000000..4cf20ac9138b
@@ -0,0 +1,387 @@
+/*
+ * Cavium Thunder DTS file
+ *
+ * Copyright (C) 2013, Cavium Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of

arch/arm64/boot/dts/thunder-88xx.dts line 18

Quoted 10 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +10 -0
+ * the License, or (at your option) any later version.
+ */
+/dts-v1/;
+
+/* Reserving first 12MB of DDR for firmware */
+/memreserve/ 0x00000000 0x00c00000;
+
+/ {
+       model = "Cavium ThunderX CN88XX Family";
+       compatible = "cavium,thunder-88xx";

Hmm. I take it this platform is compliant with some level of SBSA? We should probably have a compatible value for that so that platforms can communicate it.

Mark, are there already plans for this?

-Olof

reviewer
#19 lore raw

Re: [PATCH 2/5] arm64, thunder: Add initial dts for Cavium Thunder SoC

Quoted 54 lines · Olof Johansson
Quoted 38 lines · Robert Richter

From: Radha Mohan Chintakuntla <[email protected]>

Add initial device tree nodes for Cavium Thunder SoCs with support of 48 cores and gicv3. The dts file requires further changes, esp. for pci, gicv3-its and smmu. This changes will be added later together with the device drivers.

---
 arch/arm64/boot/dts/Makefile         |   1 +
 arch/arm64/boot/dts/thunder-88xx.dts | 387 +++++++++++++++++++++++++++++++++++
 2 files changed, 388 insertions(+)
 create mode 100644 arch/arm64/boot/dts/thunder-88xx.dts
arch/arm64/boot/dts/Makefile +1 -0
index c52bdb051f66..f8001a62029c 100644
@@ -1,3 +1,4 @@
+dtb-$(CONFIG_ARCH_THUNDER) += thunder-88xx.dtb
 dtb-$(CONFIG_ARCH_VEXPRESS) += rtsm_ve-aemv8a.dtb foundation-v8.dtb
 dtb-$(CONFIG_ARCH_XGENE) += apm-mustang.dtb
 
arch/arm64/boot/dts/thunder-88xx.dts new file +8 -0
new file mode 100644
index 000000000000..4cf20ac9138b
@@ -0,0 +1,387 @@
+/*
+ * Cavium Thunder DTS file
+ *
+ * Copyright (C) 2013, Cavium Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of

arch/arm64/boot/dts/thunder-88xx.dts line 18

Quoted 10 lines
arch/arm64/boot/dts/thunder-88xx.dts new file +10 -0
+ * the License, or (at your option) any later version.
+ */
+/dts-v1/;
+
+/* Reserving first 12MB of DDR for firmware */
+/memreserve/ 0x00000000 0x00c00000;
+
+/ {
+       model = "Cavium ThunderX CN88XX Family";
+       compatible = "cavium,thunder-88xx";

Hmm. I take it this platform is compliant with some level of SBSA? We should probably have a compatible value for that so that platforms can communicate it.

Quoted 1 line

Mark, are there already plans for this?

Not so far, but my monitor now has a sticky note telling me to look into it. :)

Many of the useful properties provided by SBSA are already described elsewhere (e.g. page and ASID size can be found in ID registers), and hardware properties it mandates (e.g. PPI assignment for level 2 compliant systems) will be described elsewhere.

I'll have to read the spec in more detail to figure out what we can derive from an SBSA compliance property. Regardless it probably makes sense to have a compatible string or other property on the root node; at worst it only costs us a few bytes.

Thanks,
Mark.

#20 lore raw

Re: [PATCH 2/5] arm64, thunder: Add initial dts for Cavium Thunder SoC

In reply to Robert Richter
Quoted 20 lines · Robert Richter

From: Radha Mohan Chintakuntla <[email protected]>

Add initial device tree nodes for Cavium Thunder SoCs with support of 48 cores and gicv3. The dts file requires further changes, esp. for pci, gicv3-its and smmu. This changes will be added later together with the device drivers.

---
 arch/arm64/boot/dts/Makefile         |   1 +
 arch/arm64/boot/dts/thunder-88xx.dts | 387 +++++++++++++++++++++++++++++++++++
 2 files changed, 388 insertions(+)
 create mode 100644 arch/arm64/boot/dts/thunder-88xx.dts
arch/arm64/boot/dts/Makefile +0 -0
index c52bdb051f66..f8001a62029c 100644

arch/arm64/boot/dts/Makefile line 3

Quoted 3 lines
arch/arm64/boot/dts/Makefile +1 -0
@@ -1,3 +1,4 @@
+dtb-$(CONFIG_ARCH_THUNDER) += thunder-88xx.dtb
 dtb-$(CONFIG_ARCH_VEXPRESS) += rtsm_ve-aemv8a.dtb foundation-v8.dtb
 dtb-$(CONFIG_ARCH_XGENE) += apm-mustang.dtb

Oh, and:

Let's fix the mistake we made on 32-bit here (that we can't undo unless we move all dts files now, which is too much churn), and put each SoC vendor in its own subdirectory.

So, please add a arch/arm64/boot/dts/cavium/ directory, add a Makefile and the dts there.

We should move vexpress under arm/ and xgene under apm/ too.

#21 lore raw

Re: [PATCH 2/5] arm64, thunder: Add initial dts for Cavium Thunder SoC

Olof,

Quoted 27 lines · Olof Johansson
Quoted 20 lines · Robert Richter

From: Radha Mohan Chintakuntla <[email protected]>

Add initial device tree nodes for Cavium Thunder SoCs with support of 48 cores and gicv3. The dts file requires further changes, esp. for pci, gicv3-its and smmu. This changes will be added later together with the device drivers.

---
 arch/arm64/boot/dts/Makefile         |   1 +
 arch/arm64/boot/dts/thunder-88xx.dts | 387 +++++++++++++++++++++++++++++++++++
 2 files changed, 388 insertions(+)
 create mode 100644 arch/arm64/boot/dts/thunder-88xx.dts
arch/arm64/boot/dts/Makefile +0 -0
index c52bdb051f66..f8001a62029c 100644

arch/arm64/boot/dts/Makefile line 3

Quoted 3 lines
arch/arm64/boot/dts/Makefile +1 -0
@@ -1,3 +1,4 @@
+dtb-$(CONFIG_ARCH_THUNDER) += thunder-88xx.dtb
 dtb-$(CONFIG_ARCH_VEXPRESS) += rtsm_ve-aemv8a.dtb foundation-v8.dtb
 dtb-$(CONFIG_ARCH_XGENE) += apm-mustang.dtb

Oh, and:

Quoted 8 lines

Let's fix the mistake we made on 32-bit here (that we can't undo unless we move all dts files now, which is too much churn), and put each SoC vendor in its own subdirectory.

So, please add a arch/arm64/boot/dts/cavium/ directory, add a Makefile and the dts there.

We should move vexpress under arm/ and xgene under apm/ too.

I well take a look at this.

-Robert

#22 lore raw

Re: [PATCH 2/5] arm64, thunder: Add initial dts for Cavium Thunder SoC

In reply to Olof Johansson

Olof,

Quoted 27 lines · Olof Johansson
Quoted 20 lines · Robert Richter

From: Radha Mohan Chintakuntla <[email protected]>

Add initial device tree nodes for Cavium Thunder SoCs with support of 48 cores and gicv3. The dts file requires further changes, esp. for pci, gicv3-its and smmu. This changes will be added later together with the device drivers.

---
 arch/arm64/boot/dts/Makefile         |   1 +
 arch/arm64/boot/dts/thunder-88xx.dts | 387 +++++++++++++++++++++++++++++++++++
 2 files changed, 388 insertions(+)
 create mode 100644 arch/arm64/boot/dts/thunder-88xx.dts
arch/arm64/boot/dts/Makefile +0 -0
index c52bdb051f66..f8001a62029c 100644

arch/arm64/boot/dts/Makefile line 3

Quoted 3 lines
arch/arm64/boot/dts/Makefile +1 -0
@@ -1,3 +1,4 @@
+dtb-$(CONFIG_ARCH_THUNDER) += thunder-88xx.dtb
 dtb-$(CONFIG_ARCH_VEXPRESS) += rtsm_ve-aemv8a.dtb foundation-v8.dtb
 dtb-$(CONFIG_ARCH_XGENE) += apm-mustang.dtb

Oh, and:

Quoted 8 lines

Let's fix the mistake we made on 32-bit here (that we can't undo unless we move all dts files now, which is too much churn), and put each SoC vendor in its own subdirectory.

So, please add a arch/arm64/boot/dts/cavium/ directory, add a Makefile and the dts there.

We should move vexpress under arm/ and xgene under apm/ too.

I was looking into this. It turned out that this would introduce more pain than it solves. The problem with introducing vendor subdirs for .dts files is that this will require also subdirs for .dtb files to avoid name conflicts. This means that dtb files are no longer in a single directory. Current build scripts and makefiles rely on this. E.g.:

scripts/Makefile.lib:$(obj)/%.dtb.S: $(obj)/%.dtb

Thus, the introduction of vendor subdirs is not easy to implement and may break existing workflows. Also, no other arch has vendor subdirs.

A solution would be to have a naming scheme for dts files which requires the vendor in the file name, such as cavium-thunder-88xx.dtb. Though, these long names may look a bit ugly.

Hmm... Any thoughts?

Thanks,

-Robert

reviewer
#23 lore raw

Re: [PATCH 2/5] arm64, thunder: Add initial dts for Cavium Thunder SoC

Quoted 48 lines · Robert Richter

Olof,

Quoted 27 lines · Olof Johansson
Quoted 20 lines · Robert Richter

From: Radha Mohan Chintakuntla <[email protected]>

Add initial device tree nodes for Cavium Thunder SoCs with support of 48 cores and gicv3. The dts file requires further changes, esp. for pci, gicv3-its and smmu. This changes will be added later together with the device drivers.

---
 arch/arm64/boot/dts/Makefile         |   1 +
 arch/arm64/boot/dts/thunder-88xx.dts | 387 +++++++++++++++++++++++++++++++++++
 2 files changed, 388 insertions(+)
 create mode 100644 arch/arm64/boot/dts/thunder-88xx.dts
arch/arm64/boot/dts/Makefile +0 -0
index c52bdb051f66..f8001a62029c 100644

arch/arm64/boot/dts/Makefile line 3

Quoted 3 lines
arch/arm64/boot/dts/Makefile +1 -0
@@ -1,3 +1,4 @@
+dtb-$(CONFIG_ARCH_THUNDER) += thunder-88xx.dtb
 dtb-$(CONFIG_ARCH_VEXPRESS) += rtsm_ve-aemv8a.dtb foundation-v8.dtb
 dtb-$(CONFIG_ARCH_XGENE) += apm-mustang.dtb

Oh, and:

Quoted 8 lines

Let's fix the mistake we made on 32-bit here (that we can't undo unless we move all dts files now, which is too much churn), and put each SoC vendor in its own subdirectory.

So, please add a arch/arm64/boot/dts/cavium/ directory, add a Makefile and the dts there.

We should move vexpress under arm/ and xgene under apm/ too.

I was looking into this. It turned out that this would introduce more pain than it solves. The problem with introducing vendor subdirs for .dts files is that this will require also subdirs for .dtb files to avoid name conflicts. This means that dtb files are no longer in a single directory. Current build scripts and makefiles rely on this. E.g.:

scripts/Makefile.lib:$(obj)/%.dtb.S: $(obj)/%.dtb

Quoted 8 lines

Thus, the introduction of vendor subdirs is not easy to implement and may break existing workflows. Also, no other arch has vendor subdirs.

A solution would be to have a naming scheme for dts files which requires the vendor in the file name, such as cavium-thunder-88xx.dtb. Though, these long names may look a bit ugly.

Hmm... Any thoughts?

That would make it possible to search, but the number of files in one place might become somewhat overwelming (which I believe was Olof's concern).

Olof, thoughts?

Mark.

#24 lore raw

Re: [PATCH 2/5] arm64, thunder: Add initial dts for Cavium Thunder SoC

Quoted 56 lines · Mark Rutland
Quoted 55 lines · Robert Richter

Olof,

Quoted 27 lines · Olof Johansson
Quoted 20 lines · Robert Richter

From: Radha Mohan Chintakuntla <[email protected]>

Add initial device tree nodes for Cavium Thunder SoCs with support of 48 cores and gicv3. The dts file requires further changes, esp. for pci, gicv3-its and smmu. This changes will be added later together with the device drivers.

---
 arch/arm64/boot/dts/Makefile         |   1 +
 arch/arm64/boot/dts/thunder-88xx.dts | 387 +++++++++++++++++++++++++++++++++++
 2 files changed, 388 insertions(+)
 create mode 100644 arch/arm64/boot/dts/thunder-88xx.dts
arch/arm64/boot/dts/Makefile +0 -0
index c52bdb051f66..f8001a62029c 100644

arch/arm64/boot/dts/Makefile line 3

Quoted 3 lines
arch/arm64/boot/dts/Makefile +1 -0
@@ -1,3 +1,4 @@
+dtb-$(CONFIG_ARCH_THUNDER) += thunder-88xx.dtb
 dtb-$(CONFIG_ARCH_VEXPRESS) += rtsm_ve-aemv8a.dtb foundation-v8.dtb
 dtb-$(CONFIG_ARCH_XGENE) += apm-mustang.dtb

Oh, and:

Quoted 8 lines

Let's fix the mistake we made on 32-bit here (that we can't undo unless we move all dts files now, which is too much churn), and put each SoC vendor in its own subdirectory.

So, please add a arch/arm64/boot/dts/cavium/ directory, add a Makefile and the dts there.

We should move vexpress under arm/ and xgene under apm/ too.

I was looking into this. It turned out that this would introduce more pain than it solves. The problem with introducing vendor subdirs for .dts files is that this will require also subdirs for .dtb files to avoid name conflicts. This means that dtb files are no longer in a single directory. Current build scripts and makefiles rely on this. E.g.:

scripts/Makefile.lib:$(obj)/%.dtb.S: $(obj)/%.dtb

Thus, the introduction of vendor subdirs is not easy to implement and may break existing workflows. Also, no other arch has vendor subdirs.

A solution would be to have a naming scheme for dts files which requires the vendor in the file name, such as cavium-thunder-88xx.dtb. Though, these long names may look a bit ugly.

Quoted 7 lines
Quoted 1 line

Hmm... Any thoughts?

That would make it possible to search, but the number of files in one place might become somewhat overwelming (which I believe was Olof's concern).

Olof, thoughts?

Yeah, on 32 bit we've got an ever-growing and very large directory today that is becoming a headache. Carving it up in directories to reduce plain directory size would come a long way.

I think we would catch name conflicts quickly with the automatic builders we have today -- and it's likely that the files will have a platform prefix anyway so it shouldn't be a big issue. I think we can live with that.

-Olof

#25 lore raw

Re: [PATCH 2/5] arm64, thunder: Add initial dts for Cavium Thunder SoC

Quoted 65 lines · Olof Johansson
Quoted 56 lines · Mark Rutland
Quoted 55 lines · Robert Richter

Olof,

Quoted 27 lines · Olof Johansson
Quoted 20 lines · Robert Richter

From: Radha Mohan Chintakuntla <[email protected]>

Add initial device tree nodes for Cavium Thunder SoCs with support of 48 cores and gicv3. The dts file requires further changes, esp. for pci, gicv3-its and smmu. This changes will be added later together with the device drivers.

---
 arch/arm64/boot/dts/Makefile         |   1 +
 arch/arm64/boot/dts/thunder-88xx.dts | 387 +++++++++++++++++++++++++++++++++++
 2 files changed, 388 insertions(+)
 create mode 100644 arch/arm64/boot/dts/thunder-88xx.dts
arch/arm64/boot/dts/Makefile +0 -0
index c52bdb051f66..f8001a62029c 100644

arch/arm64/boot/dts/Makefile line 3

Quoted 3 lines
arch/arm64/boot/dts/Makefile +1 -0
@@ -1,3 +1,4 @@
+dtb-$(CONFIG_ARCH_THUNDER) += thunder-88xx.dtb
 dtb-$(CONFIG_ARCH_VEXPRESS) += rtsm_ve-aemv8a.dtb foundation-v8.dtb
 dtb-$(CONFIG_ARCH_XGENE) += apm-mustang.dtb

Oh, and:

Quoted 8 lines

Let's fix the mistake we made on 32-bit here (that we can't undo unless we move all dts files now, which is too much churn), and put each SoC vendor in its own subdirectory.

So, please add a arch/arm64/boot/dts/cavium/ directory, add a Makefile and the dts there.

We should move vexpress under arm/ and xgene under apm/ too.

I was looking into this. It turned out that this would introduce more pain than it solves. The problem with introducing vendor subdirs for .dts files is that this will require also subdirs for .dtb files to avoid name conflicts. This means that dtb files are no longer in a single directory. Current build scripts and makefiles rely on this. E.g.:

scripts/Makefile.lib:$(obj)/%.dtb.S: $(obj)/%.dtb

Thus, the introduction of vendor subdirs is not easy to implement and may break existing workflows. Also, no other arch has vendor subdirs.

A solution would be to have a naming scheme for dts files which requires the vendor in the file name, such as cavium-thunder-88xx.dtb. Though, these long names may look a bit ugly.

Quoted 7 lines
Quoted 1 line

Hmm... Any thoughts?

That would make it possible to search, but the number of files in one place might become somewhat overwelming (which I believe was Olof's concern).

Olof, thoughts?

Quoted 8 lines

Yeah, on 32 bit we've got an ever-growing and very large directory today that is becoming a headache. Carving it up in directories to reduce plain directory size would come a long way.

I think we would catch name conflicts quickly with the automatic builders we have today -- and it's likely that the files will have a platform prefix anyway so it shouldn't be a big issue. I think we can live with that.

So you mean, having the source files with subdirectories, but the dtb files in a single obj dir?

-Robert

#26 lore raw

Re: [PATCH 2/5] arm64, thunder: Add initial dts for Cavium Thunder SoC

Quoted 70 lines · Robert Richter
Quoted 69 lines · Olof Johansson
Quoted 56 lines · Mark Rutland
Quoted 55 lines · Robert Richter

Olof,

Quoted 27 lines · Olof Johansson
Quoted 20 lines · Robert Richter

From: Radha Mohan Chintakuntla <[email protected]>

Add initial device tree nodes for Cavium Thunder SoCs with support of 48 cores and gicv3. The dts file requires further changes, esp. for pci, gicv3-its and smmu. This changes will be added later together with the device drivers.

---
 arch/arm64/boot/dts/Makefile         |   1 +
 arch/arm64/boot/dts/thunder-88xx.dts | 387 +++++++++++++++++++++++++++++++++++
 2 files changed, 388 insertions(+)
 create mode 100644 arch/arm64/boot/dts/thunder-88xx.dts
arch/arm64/boot/dts/Makefile +0 -0
index c52bdb051f66..f8001a62029c 100644

arch/arm64/boot/dts/Makefile line 3

Quoted 3 lines
arch/arm64/boot/dts/Makefile +1 -0
@@ -1,3 +1,4 @@
+dtb-$(CONFIG_ARCH_THUNDER) += thunder-88xx.dtb
 dtb-$(CONFIG_ARCH_VEXPRESS) += rtsm_ve-aemv8a.dtb foundation-v8.dtb
 dtb-$(CONFIG_ARCH_XGENE) += apm-mustang.dtb

Oh, and:

Quoted 8 lines

Let's fix the mistake we made on 32-bit here (that we can't undo unless we move all dts files now, which is too much churn), and put each SoC vendor in its own subdirectory.

So, please add a arch/arm64/boot/dts/cavium/ directory, add a Makefile and the dts there.

We should move vexpress under arm/ and xgene under apm/ too.

I was looking into this. It turned out that this would introduce more pain than it solves. The problem with introducing vendor subdirs for .dts files is that this will require also subdirs for .dtb files to avoid name conflicts. This means that dtb files are no longer in a single directory. Current build scripts and makefiles rely on this. E.g.:

scripts/Makefile.lib:$(obj)/%.dtb.S: $(obj)/%.dtb

Thus, the introduction of vendor subdirs is not easy to implement and may break existing workflows. Also, no other arch has vendor subdirs.

A solution would be to have a naming scheme for dts files which requires the vendor in the file name, such as cavium-thunder-88xx.dtb. Though, these long names may look a bit ugly.

Quoted 7 lines
Quoted 1 line

Hmm... Any thoughts?

That would make it possible to search, but the number of files in one place might become somewhat overwelming (which I believe was Olof's concern).

Olof, thoughts?

Yeah, on 32 bit we've got an ever-growing and very large directory today that is becoming a headache. Carving it up in directories to reduce plain directory size would come a long way.

Quoted 7 lines
Quoted 4 lines

I think we would catch name conflicts quickly with the automatic builders we have today -- and it's likely that the files will have a platform prefix anyway so it shouldn't be a big issue. I think we can live with that.

So you mean, having the source files with subdirectories, but the dtb files in a single obj dir?

Thinking about it a bit more, the even more obvious solution that I for some reason didn't think of at the time, is to have the dtbs_install target create the appropriate subdirectories on the install side, so that the structure is preserved.

That comes a bit closer to how modules are handled as well.

-Olof

#27 lore raw

Re: [PATCH 2/5] arm64, thunder: Add initial dts for Cavium Thunder SoC

Quoted 71 lines · Olof Johansson
Quoted 70 lines · Robert Richter
Quoted 69 lines · Olof Johansson
Quoted 56 lines · Mark Rutland
Quoted 55 lines · Robert Richter

Olof,

Quoted 27 lines · Olof Johansson
Quoted 20 lines · Robert Richter

From: Radha Mohan Chintakuntla <[email protected]>

Add initial device tree nodes for Cavium Thunder SoCs with support of 48 cores and gicv3. The dts file requires further changes, esp. for pci, gicv3-its and smmu. This changes will be added later together with the device drivers.

---
 arch/arm64/boot/dts/Makefile         |   1 +
 arch/arm64/boot/dts/thunder-88xx.dts | 387 +++++++++++++++++++++++++++++++++++
 2 files changed, 388 insertions(+)
 create mode 100644 arch/arm64/boot/dts/thunder-88xx.dts
arch/arm64/boot/dts/Makefile +0 -0
index c52bdb051f66..f8001a62029c 100644

arch/arm64/boot/dts/Makefile line 3

Quoted 3 lines
arch/arm64/boot/dts/Makefile +1 -0
@@ -1,3 +1,4 @@
+dtb-$(CONFIG_ARCH_THUNDER) += thunder-88xx.dtb
 dtb-$(CONFIG_ARCH_VEXPRESS) += rtsm_ve-aemv8a.dtb foundation-v8.dtb
 dtb-$(CONFIG_ARCH_XGENE) += apm-mustang.dtb

Oh, and:

Quoted 8 lines

Let's fix the mistake we made on 32-bit here (that we can't undo unless we move all dts files now, which is too much churn), and put each SoC vendor in its own subdirectory.

So, please add a arch/arm64/boot/dts/cavium/ directory, add a Makefile and the dts there.

We should move vexpress under arm/ and xgene under apm/ too.

I was looking into this. It turned out that this would introduce more pain than it solves. The problem with introducing vendor subdirs for .dts files is that this will require also subdirs for .dtb files to avoid name conflicts. This means that dtb files are no longer in a single directory. Current build scripts and makefiles rely on this. E.g.:

scripts/Makefile.lib:$(obj)/%.dtb.S: $(obj)/%.dtb

Thus, the introduction of vendor subdirs is not easy to implement and may break existing workflows. Also, no other arch has vendor subdirs.

A solution would be to have a naming scheme for dts files which requires the vendor in the file name, such as cavium-thunder-88xx.dtb. Though, these long names may look a bit ugly.

Quoted 7 lines
Quoted 1 line

Hmm... Any thoughts?

That would make it possible to search, but the number of files in one place might become somewhat overwelming (which I believe was Olof's concern).

Olof, thoughts?

Yeah, on 32 bit we've got an ever-growing and very large directory today that is becoming a headache. Carving it up in directories to reduce plain directory size would come a long way.

Quoted 7 lines
Quoted 7 lines
Quoted 4 lines

I think we would catch name conflicts quickly with the automatic builders we have today -- and it's likely that the files will have a platform prefix anyway so it shouldn't be a big issue. I think we can live with that.

So you mean, having the source files with subdirectories, but the dtb files in a single obj dir?

It seems it will be easier to have also subdirs for objs.

Quoted 6 lines

Thinking about it a bit more, the even more obvious solution that I for some reason didn't think of at the time, is to have the dtbs_install target create the appropriate subdirectories on the install side, so that the structure is preserved.

That comes a bit closer to how modules are handled as well.

Yes, dtbs_install target would be useful.

Will come with a solution for this too.

-Robert

#28 lore raw

PATCH 1/1 arm64, dts: Add dtbs_install make target

Olof,

Quoted 11 lines · Robert Richter
Quoted 6 lines · Olof Johansson

Thinking about it a bit more, the even more obvious solution that I for some reason didn't think of at the time, is to have the dtbs_install target create the appropriate subdirectories on the install side, so that the structure is preserved.

That comes a bit closer to how modules are handled as well.

Yes, dtbs_install target would be useful.

Will come with a solution for this too.

below is a patch that adds the dtbs_install target to arm64. Taken from arm where this was already implemented.

-Robert

Quoted 1 line

From bbb09a3775a385f4a549b1c4b8518658bcbc382d Mon Sep 17 00:00:00 2001

From: Robert Richter <[email protected]>
Date: Fri, 29 Aug 2014 14:17:02 +0200
Subject: [PATCH] arm64, dts: Add dtbs_install make target

This adds the dtbs_install make target to arm64. The target has been introduced already to arch/arm with the following commit:

f4d4ffc03efc kbuild: dtbs_install: new make target

Implementation for arm64 is the same as for arm.

With 'dtbs_install' all config enabled dtb files are installed to either the INSTALL_DTBS_PATH directory or the default location:

$INSTALL_PATH/dtbs/$KERNELRELEASE

---
 arch/arm64/Makefile          | 6 ++++--
 arch/arm64/boot/dts/Makefile | 4 +++-
 2 files changed, 7 insertions(+), 3 deletions(-)
arch/arm64/Makefile +4 -2
index 8185a913c5ed..c68efa07c5f3 100644
@@ -66,8 +66,9 @@ zinstall install: vmlinux
 %.dtb: scripts
 	$(Q)$(MAKE) $(build)=$(boot)/dts $(boot)/dts/$@
 
-dtbs: scripts
-	$(Q)$(MAKE) $(build)=$(boot)/dts dtbs
+PHONY += dtbs dtbs_install
+dtbs dtbs_install: prepare scripts
+	$(Q)$(MAKE) $(build)=$(boot)/dts $@
 
 PHONY += vdso_install
 vdso_install:
@@ -81,6 +82,7 @@ define archhelp
   echo  '* Image.gz      - Compressed kernel image (arch/$(ARCH)/boot/Image.gz)'
   echo  '  Image         - Uncompressed kernel image (arch/$(ARCH)/boot/Image)'
   echo  '* dtbs          - Build device tree blobs for enabled boards'
+  echo  '  dtbs_install  - Install dtbs to $(INSTALL_DTBS_PATH)'
   echo  '  install       - Install uncompressed kernel'
   echo  '  zinstall      - Install compressed kernel'
   echo  '                  Install using (your) ~/bin/installkernel or'
arch/arm64/boot/dts/Makefile +3 -1
index f8001a62029c..3a75cdc2d087 100644
@@ -2,9 +2,11 @@ dtb-$(CONFIG_ARCH_THUNDER) += thunder-88xx.dtb
 dtb-$(CONFIG_ARCH_VEXPRESS) += rtsm_ve-aemv8a.dtb foundation-v8.dtb
 dtb-$(CONFIG_ARCH_XGENE) += apm-mustang.dtb
 
-targets += dtbs
+targets += dtbs dtbs_install
 targets += $(dtb-y)
 
 dtbs: $(addprefix $(obj)/, $(dtb-y))
 
 clean-files := *.dtb
+
+dtbs_install: $(addsuffix _dtbinst_, $(dtb-y))
Signature · 1 line
2.0.1
#29 lore raw

Re: [PATCH] arm64, dts: Add dtbs_install make target

Olof,

Quoted 7 lines · Robert Richter
Quoted 6 lines · Robert Richter
Quoted 5 lines · Olof Johansson

Thinking about it a bit more, the even more obvious solution that I for some reason didn't think of at the time, is to have the dtbs_install target create the appropriate subdirectories on the install side, so that the structure is preserved.

Quoted 8 lines
Quoted 5 lines
Quoted 1 line

That comes a bit closer to how modules are handled as well.

Yes, dtbs_install target would be useful.

Will come with a solution for this too.

below is a patch that adds the dtbs_install target to arm64. Taken from arm where this was already implemented.

please note that I sent a separate patch set with a complete implementation of this with the title:

[PATCH 0/6] dts, kbuild: Implement support for dtb vendor subdirs

this patch is included there too.

-Robert

maintainer
#30 lore raw

Re: [PATCH 2/5] arm64, thunder: Add initial dts for Cavium Thunder SoC

In reply to Robert Richter
Quoted 10 lines · Robert Richter

+/*
+ * Cavium Thunder DTS file
+ *
+ * Copyright (C) 2013, Cavium Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ */

I think it makes sense to use a permissive license for these files here, since board vendors may want to include a copy of the binary in their firmware, which isn't necessarily GPL compatible.

What is Cavium's interest in making this GPL?

Arnd

#31 lore raw

Re: [PATCH 2/5] arm64, thunder: Add initial dts for Cavium Thunder SoC

Quoted 12 lines · Arnd Bergmann
Quoted 10 lines · Robert Richter

+/*
+ * Cavium Thunder DTS file
+ *
+ * Copyright (C) 2013, Cavium Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ */

Quoted 5 lines

I think it makes sense to use a permissive license for these files here, since board vendors may want to include a copy of the binary in their firmware, which isn't necessarily GPL compatible.

What is Cavium's interest in making this GPL?

See my response to Rob's mail on this.

-Robert

#32 lore raw

PATCH 3/5 arm64, thunder: document devicetree bindings for Cavium Thunder SoC

In reply to Robert Richter

From: Radha Mohan Chintakuntla <[email protected]>

This patch adds documentation for the devicetree bindings used by the DT files of Cavium Thunder SoC platforms.

---
 Documentation/devicetree/bindings/arm/cavium-thunder.txt | 10 ++++++++++
 Documentation/devicetree/bindings/arm/cpus.txt           |  1 +
 2 files changed, 11 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/cavium-thunder.txt
Documentation/devicetree/bindings/arm/cavium-thunder.txt new file +10 -0
new file mode 100644
index 000000000000..6f63a5866902
@@ -0,0 +1,10 @@
+Cavium Thunder platform device tree bindings
+--------------------------------------------
+
+Boards with Cavium's Thunder SoC shall have following properties.
+
+Root Node
+---------
+Required root node properties:
+
+  - compatible = "cavium,thunder-88xx";
Documentation/devicetree/bindings/arm/cpus.txt +1 -0
index 1fe72a0778cd..922e4970309d 100644
@@ -163,6 +163,7 @@ nodes to be present and contain the properties described below.
 			    "arm,cortex-r4"
 			    "arm,cortex-r5"
 			    "arm,cortex-r7"
+			    "cavium,thunder"
 			    "faraday,fa526"
 			    "intel,sa110"
 			    "intel,sa1100"
Signature · 1 line
2.0.1
#33 lore raw

PATCH 4/5 arm64, defconfig: Enable Cavium Thunder SoC in defconfig

In reply to Robert Richter

From: Robert Richter <[email protected]>

This patch enables Thunder SoCs in the arm64 defconfig. This is esp. useful to add Thunder platforms to automated builds based on arm64 defconfig.

---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)
arch/arm64/configs/defconfig +1 -0
index 3421f316f5dc..26c3ab582db5 100644
@@ -32,6 +32,7 @@ CONFIG_MODULES=y
 CONFIG_MODULE_UNLOAD=y
 # CONFIG_BLK_DEV_BSG is not set
 # CONFIG_IOSCHED_DEADLINE is not set
+CONFIG_ARCH_THUNDER=y
 CONFIG_ARCH_VEXPRESS=y
 CONFIG_ARCH_XGENE=y
 CONFIG_SMP=y
Signature · 1 line
2.0.1
#34 lore raw

PATCH 5/5 arm64, defconfig: Enable tmpfs mount option

In reply to Robert Richter

From: Robert Richter <[email protected]>

Making it more convinient to run the arm64 default kernel as distros like Ubuntu need this option.

---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)
arch/arm64/configs/defconfig +1 -0
index 26c3ab582db5..50ea69d51bb4 100644
@@ -55,6 +55,7 @@ CONFIG_IP_PNP_BOOTP=y
 # CONFIG_WIRELESS is not set
 CONFIG_UEVENT_HELPER_PATH="/sbin/hotplug"
 CONFIG_DEVTMPFS=y
+CONFIG_DEVTMPFS_MOUNT=y
 CONFIG_DMA_CMA=y
 CONFIG_BLK_DEV_LOOP=y
 CONFIG_VIRTIO_BLK=y
Signature · 1 line
2.0.1