devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] Adds Corstone500 DeviceTree
@ 2022-12-14 13:24 Emekcan Aras
  2022-12-14 13:24 ` [PATCH v2 1/2] arm64: dts: Add Arm corstone500 platform support Emekcan Aras
  2022-12-14 13:24 ` [PATCH v2 2/2] dt-bindings: Add Arm corstone500 platform Emekcan Aras
  0 siblings, 2 replies; 9+ messages in thread
From: Emekcan Aras @ 2022-12-14 13:24 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski
  Cc: linux-arm-kernel, devicetree, Emekcan Aras

From: Emekcan Aras <Emekcan.Aras@arm.com>

Adding Corstone500 devicetree and its bindings.

Emekcan Aras (2):
  arm64: dts: Add Arm corstone500 platform support
  dt-bindings: Add Arm corstone500 platform

 .../bindings/arm/arm,corstone500.yaml         |  30 +++
 arch/arm/boot/dts/corstone500.dts             | 181 ++++++++++++++++++
 2 files changed, 211 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/arm,corstone500.yaml
 create mode 100644 arch/arm/boot/dts/corstone500.dts

-- 
2.25.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 1/2] arm64: dts: Add Arm corstone500 platform support
  2022-12-14 13:24 [PATCH v2 0/2] Adds Corstone500 DeviceTree Emekcan Aras
@ 2022-12-14 13:24 ` Emekcan Aras
  2022-12-14 13:41   ` Krzysztof Kozlowski
  2022-12-14 16:37   ` Rob Herring
  2022-12-14 13:24 ` [PATCH v2 2/2] dt-bindings: Add Arm corstone500 platform Emekcan Aras
  1 sibling, 2 replies; 9+ messages in thread
From: Emekcan Aras @ 2022-12-14 13:24 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski
  Cc: linux-arm-kernel, devicetree, Emekcan Aras, Emekcan Aras

From: Emekcan Aras <Emekcan.Aras@arm.com>

Corstone500[0] is a platform from arm, which includes Cortex-A cores and
ideal starting point for feature rich System on Chip (SoC) designs
based on the Cortex-A5 core.

These device trees contains the necessary bits to support the
Corstone 500 FVP (Fixed Virtual Platform) and the
FPGA MPS3 board.

0: https://developer.arm.com/documentation/102262/0000

Signed-off-by: Emekcan Aras <emekcan.aras@arm.com>
---
 arch/arm/boot/dts/corstone500.dts | 181 ++++++++++++++++++++++++++++++
 1 file changed, 181 insertions(+)
 create mode 100644 arch/arm/boot/dts/corstone500.dts

diff --git a/arch/arm/boot/dts/corstone500.dts b/arch/arm/boot/dts/corstone500.dts
new file mode 100644
index 000000000000..976aa333ffbc
--- /dev/null
+++ b/arch/arm/boot/dts/corstone500.dts
@@ -0,0 +1,181 @@
+// SPDX-License-Identifier: GPL-2.0 or MIT
+/*
+ * Copyright (c) 2022, Arm Limited. All rights reserved.
+ *
+ */
+
+
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+
+/ {
+	model = "ARM Corstone500";
+	compatible = "arm,corstone500";
+	interrupt-parent = <&gic>;
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	aliases {
+		serial0 = &uart0;
+		serial1 = &uart1;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+
+	psci {
+		compatible = "arm,psci-1.0", "arm,psci-0.2", "arm,psci";
+		method = "smc";
+		cpu_on = <0x84000003>;
+	};
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		enable-method = "psci";
+
+		cpu@0 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a5";
+			reg = <0>;
+			next-level-cache = <&L2>;
+		};
+
+		cpu@1 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a5";
+			reg = <1>;
+			next-level-cache = <&L2>;
+		};
+
+		cpu@2 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a5";
+			reg = <2>;
+			next-level-cache = <&L2>;
+		};
+
+		cpu@3 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a5";
+			reg = <3>;
+			next-level-cache = <&L2>;
+		};
+	};
+
+	memory@80000000 {
+		device_type = "memory";
+		reg = <0x80000000 0x7f000000>;
+	};
+
+	L2: cache-controller@1c010000 {
+		compatible = "arm,pl310-cache";
+		reg = <0x1c010000 0x1000>;
+		interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>;
+		cache-level = <2>;
+		cache-unified;
+		arm,data-latency = <1 1 1>;
+		arm,tag-latency = <1 1 1>;
+	};
+
+	refclk7500khz: refclk7500khz {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <7500000>;
+		clock-output-names = "apb_pclk";
+	};
+
+	refclk24mhz: refclk24mhz {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <24000000>;
+		clock-output-names = "apb_pclk";
+	};
+
+	smbclk: refclk24mhzx2 {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <48000000>;
+		clock-output-names = "smclk";
+	};
+
+	rtc@1a220000 {
+		compatible = "arm,pl031", "arm,primecell";
+		reg = <0x1a220000 0x1000>;
+		clocks = <&refclk24mhz>;
+		interrupts = <GIC_SPI 6 (GIC_CPU_MASK_SIMPLE(4) |
+			IRQ_TYPE_LEVEL_HIGH)>;
+		clock-names = "apb_pclk";
+	};
+
+	gic: interrupt-controller@1c001000 {
+		compatible = "arm,cortex-a5-gic";
+		#interrupt-cells = <3>;
+		#address-cells = <0>;
+		interrupt-controller;
+		reg =	<0x1c001000 0x1000>,
+			<0x1c000100 0x100>;
+		interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) |
+			IRQ_TYPE_LEVEL_HIGH)>;
+	};
+
+	soc{
+		compatible = "simple-bus";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		clock_frequency = <50000000>;
+		interrupt-parent = <&gic>;
+		ranges;
+
+		uart0: serial@1a200000 {
+			compatible = "arm,pl011", "arm,primecell";
+			reg = <0x1a200000 0x1000>;
+			interrupts = <GIC_SPI 8 (GIC_CPU_MASK_SIMPLE(4) |
+				IRQ_TYPE_LEVEL_HIGH)>;
+			clocks = <&refclk7500khz>;
+			clock-names = "apb_pclk";
+		};
+
+		uart1: serial@1a210000 {
+			compatible = "arm,pl011", "arm,primecell";
+			reg = <0x1a210000 0x1000>;
+			interrupts = <GIC_SPI 9 (GIC_CPU_MASK_SIMPLE(4) |
+				IRQ_TYPE_LEVEL_HIGH)>;
+			clocks = <&refclk7500khz>;
+			clock-names = "apb_pclk";
+		};
+
+		timer0: timer@1a040000 {
+			compatible = "arm,armv7-timer-mem";
+			reg = <0x1a040000 0x1000>;
+			clock-frequency = <7500000>;
+
+			frame@1a050000 {
+				frame-number = <0>;
+				interrupts = <GIC_SPI 2 (GIC_CPU_MASK_SIMPLE(4) |
+				IRQ_TYPE_LEVEL_HIGH)>;
+				reg = <0x1a050000 0x1000>;
+			};
+		};
+
+		smsc: ethernet@4020000 {
+			compatible = "smsc,lan9220", "smsc,lan9115";
+			reg = <0x40200000 0x10000>;
+			interrupts = <GIC_SPI 43 (GIC_CPU_MASK_SIMPLE(4) |
+				IRQ_TYPE_LEVEL_HIGH)>;
+			reg-io-width = <4>;
+			phy-mode = "mii";
+			smsc,irq-active-high;
+			vdd33a-supply = <&v2m_fixed_3v3>;
+			vddvario-supply = <&v2m_fixed_3v3>;
+		};
+	};
+
+	v2m_fixed_3v3: fixed-regulator-0 {
+		compatible = "regulator-fixed";
+		regulator-name = "3V3";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-always-on;
+	};
+};
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 2/2] dt-bindings: Add Arm corstone500 platform
  2022-12-14 13:24 [PATCH v2 0/2] Adds Corstone500 DeviceTree Emekcan Aras
  2022-12-14 13:24 ` [PATCH v2 1/2] arm64: dts: Add Arm corstone500 platform support Emekcan Aras
@ 2022-12-14 13:24 ` Emekcan Aras
  2022-12-14 13:44   ` Krzysztof Kozlowski
  1 sibling, 1 reply; 9+ messages in thread
From: Emekcan Aras @ 2022-12-14 13:24 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski
  Cc: linux-arm-kernel, devicetree, Emekcan Aras, Emekcan Aras

From: Emekcan Aras <Emekcan.Aras@arm.com>

Add bindings to describe implementation of
the ARM Corstone500 platform.

Signed-off-by: Emekcan Aras <emekcan.aras@arm.com>
---
 .../bindings/arm/arm,corstone500.yaml         | 30 +++++++++++++++++++
 1 file changed, 30 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/arm,corstone500.yaml

diff --git a/Documentation/devicetree/bindings/arm/arm,corstone500.yaml b/Documentation/devicetree/bindings/arm/arm,corstone500.yaml
new file mode 100644
index 000000000000..cfe41f7760fd
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/arm,corstone500.yaml
@@ -0,0 +1,30 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/arm,corstone500.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ARM Corstone500
+
+maintainers:
+  - Emekcan Aras <emekcan.aras@arm.com>
+  - Rui Miguel Silva <rui.silva@linaro.org>
+
+description: |+
+  Corstone-500 is an ideal starting point for feature rich System on Chip
+  (SoC) designs based on the Cortex-A5 core. These designs can be used in
+  Internet of Things (IoT) and embedded products.
+
+  Corstone-500 includes most of the Arm IP in the SSE-500 subsystem and
+  example integration layer, an FPGA, and access to modelling options.
+
+properties:
+  $nodename:
+    const: '/'
+  compatible:
+    items:
+      - const: arm,corstone500
+
+additionalProperties: true
+
+...
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/2] arm64: dts: Add Arm corstone500 platform support
  2022-12-14 13:24 ` [PATCH v2 1/2] arm64: dts: Add Arm corstone500 platform support Emekcan Aras
@ 2022-12-14 13:41   ` Krzysztof Kozlowski
  2022-12-14 14:54     ` Krzysztof Kozlowski
  2022-12-14 16:37   ` Rob Herring
  1 sibling, 1 reply; 9+ messages in thread
From: Krzysztof Kozlowski @ 2022-12-14 13:41 UTC (permalink / raw)
  To: Emekcan Aras, Rob Herring, Krzysztof Kozlowski
  Cc: linux-arm-kernel, devicetree

On 14/12/2022 14:24, Emekcan Aras wrote:
> From: Emekcan Aras <Emekcan.Aras@arm.com>
> 

Use subject prefixes matching the subsystem (git log --oneline -- ...).
You already got such comment for v1... so do not ignore it but implement
instead.

> Corstone500[0] is a platform from arm, which includes Cortex-A cores and
> ideal starting point for feature rich System on Chip (SoC) designs
> based on the Cortex-A5 core.
> 
> These device trees contains the necessary bits to support the
> Corstone 500 FVP (Fixed Virtual Platform) and the
> FPGA MPS3 board.
> 
> 0: https://developer.arm.com/documentation/102262/0000
> 
> Signed-off-by: Emekcan Aras <emekcan.aras@arm.com>
> ---
>  arch/arm/boot/dts/corstone500.dts | 181 ++++++++++++++++++++++++++++++
>  1 file changed, 181 insertions(+)
>  create mode 100644 arch/arm/boot/dts/corstone500.dts

How do you test it? I do not see a way to compile it...

You need to include it in maintainers entry and CC relevant maintainers.

> 
> diff --git a/arch/arm/boot/dts/corstone500.dts b/arch/arm/boot/dts/corstone500.dts
> new file mode 100644
> index 000000000000..976aa333ffbc
> --- /dev/null
> +++ b/arch/arm/boot/dts/corstone500.dts
> @@ -0,0 +1,181 @@
> +// SPDX-License-Identifier: GPL-2.0 or MIT
> +/*
> + * Copyright (c) 2022, Arm Limited. All rights reserved.
> + *
> + */
> +
> +

Just one blank line

> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +/ {
> +	model = "ARM Corstone500";
> +	compatible = "arm,corstone500";

No DTSI? No board or platform compatible? How is it going to be used by
anyone else?

> +	interrupt-parent = <&gic>;
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +
> +	aliases {
> +		serial0 = &uart0;
> +		serial1 = &uart1;
> +	};
> +
> +	chosen {
> +		stdout-path = "serial0:115200n8";
> +	};
> +
> +	psci {
> +		compatible = "arm,psci-1.0", "arm,psci-0.2", "arm,psci";
> +		method = "smc";
> +		cpu_on = <0x84000003>;
> +	};
> +
> +	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		enable-method = "psci";
> +
> +		cpu@0 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a5";
> +			reg = <0>;
> +			next-level-cache = <&L2>;
> +		};
> +
> +		cpu@1 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a5";
> +			reg = <1>;
> +			next-level-cache = <&L2>;
> +		};
> +
> +		cpu@2 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a5";
> +			reg = <2>;
> +			next-level-cache = <&L2>;
> +		};
> +
> +		cpu@3 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a5";
> +			reg = <3>;
> +			next-level-cache = <&L2>;
> +		};
> +	};
> +
> +	memory@80000000 {
> +		device_type = "memory";
> +		reg = <0x80000000 0x7f000000>;
> +	};
> +
> +	L2: cache-controller@1c010000 {
> +		compatible = "arm,pl310-cache";
> +		reg = <0x1c010000 0x1000>;
> +		interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>;
> +		cache-level = <2>;
> +		cache-unified;
> +		arm,data-latency = <1 1 1>;
> +		arm,tag-latency = <1 1 1>;
> +	};
> +
> +	refclk7500khz: refclk7500khz {

Node names should be generic, so at least generic prefix or suffix
(clock-, -clock).
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <7500000>;
> +		clock-output-names = "apb_pclk";
> +	};
> +
> +	refclk24mhz: refclk24mhz {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <24000000>;
> +		clock-output-names = "apb_pclk";
> +	};
> +
> +	smbclk: refclk24mhzx2 {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <48000000>;
> +		clock-output-names = "smclk";
> +	};
> +
> +	rtc@1a220000 {
> +		compatible = "arm,pl031", "arm,primecell";
> +		reg = <0x1a220000 0x1000>;
> +		clocks = <&refclk24mhz>;
> +		interrupts = <GIC_SPI 6 (GIC_CPU_MASK_SIMPLE(4) |
> +			IRQ_TYPE_LEVEL_HIGH)>;
> +		clock-names = "apb_pclk";
> +	};
> +
> +	gic: interrupt-controller@1c001000 {
> +		compatible = "arm,cortex-a5-gic";
> +		#interrupt-cells = <3>;
> +		#address-cells = <0>;
> +		interrupt-controller;
> +		reg =	<0x1c001000 0x1000>,
> +			<0x1c000100 0x100>;
> +		interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) |
> +			IRQ_TYPE_LEVEL_HIGH)>;
> +	};
> +
> +	soc{
> +		compatible = "simple-bus";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		clock_frequency = <50000000>;
> +		interrupt-parent = <&gic>;
> +		ranges;
> +
> +		uart0: serial@1a200000 {
> +			compatible = "arm,pl011", "arm,primecell";
> +			reg = <0x1a200000 0x1000>;
> +			interrupts = <GIC_SPI 8 (GIC_CPU_MASK_SIMPLE(4) |
> +				IRQ_TYPE_LEVEL_HIGH)>;
> +			clocks = <&refclk7500khz>;
> +			clock-names = "apb_pclk";
> +		};
> +
> +		uart1: serial@1a210000 {
> +			compatible = "arm,pl011", "arm,primecell";
> +			reg = <0x1a210000 0x1000>;
> +			interrupts = <GIC_SPI 9 (GIC_CPU_MASK_SIMPLE(4) |
> +				IRQ_TYPE_LEVEL_HIGH)>;
> +			clocks = <&refclk7500khz>;
> +			clock-names = "apb_pclk";
> +		};
> +
> +		timer0: timer@1a040000 {
> +			compatible = "arm,armv7-timer-mem";
> +			reg = <0x1a040000 0x1000>;
> +			clock-frequency = <7500000>;
> +
> +			frame@1a050000 {
> +				frame-number = <0>;
> +				interrupts = <GIC_SPI 2 (GIC_CPU_MASK_SIMPLE(4) |
> +				IRQ_TYPE_LEVEL_HIGH)>;
> +				reg = <0x1a050000 0x1000>;
> +			};
> +		};
> +
> +		smsc: ethernet@4020000 {
> +			compatible = "smsc,lan9220", "smsc,lan9115";
> +			reg = <0x40200000 0x10000>;
> +			interrupts = <GIC_SPI 43 (GIC_CPU_MASK_SIMPLE(4) |
> +				IRQ_TYPE_LEVEL_HIGH)>;
> +			reg-io-width = <4>;
> +			phy-mode = "mii";
> +			smsc,irq-active-high;
> +			vdd33a-supply = <&v2m_fixed_3v3>;
> +			vddvario-supply = <&v2m_fixed_3v3>;
> +		};
> +	};
> +
> +	v2m_fixed_3v3: fixed-regulator-0 {

You have weird ordering of nodes. If it is by name, then this should be
regulator-0. Anyway drop "fixed" prefix as it is not relevant.


Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/2] dt-bindings: Add Arm corstone500 platform
  2022-12-14 13:24 ` [PATCH v2 2/2] dt-bindings: Add Arm corstone500 platform Emekcan Aras
@ 2022-12-14 13:44   ` Krzysztof Kozlowski
  2022-12-14 14:55     ` Krzysztof Kozlowski
  2022-12-14 14:57     ` Krzysztof Kozlowski
  0 siblings, 2 replies; 9+ messages in thread
From: Krzysztof Kozlowski @ 2022-12-14 13:44 UTC (permalink / raw)
  To: Emekcan Aras, Rob Herring, Krzysztof Kozlowski
  Cc: linux-arm-kernel, devicetree

On 14/12/2022 14:24, Emekcan Aras wrote:
> From: Emekcan Aras <Emekcan.Aras@arm.com>

1. Your From does not match SoB exactly. Avoid it and make it consistent.

2. Use subject prefixes matching the subsystem (git log --oneline -- ...).

3. Please use scripts/get_maintainers.pl to get a list of necessary
people and lists to CC.  It might happen, that command when run on an
older kernel, gives you outdated entries.  Therefore please be sure you
base your patches on recent Linux kernel.


> 
> Add bindings to describe implementation of
> the ARM Corstone500 platform.

Please wrap commit message according to Linux coding style / submission
process:
https://elixir.bootlin.com/linux/v5.18-rc4/source/Documentation/process/submitting-patches.rst#L586

> 
> Signed-off-by: Emekcan Aras <emekcan.aras@arm.com>
> ---
>  .../bindings/arm/arm,corstone500.yaml         | 30 +++++++++++++++++++
>  1 file changed, 30 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/arm,corstone500.yaml
> 
> diff --git a/Documentation/devicetree/bindings/arm/arm,corstone500.yaml b/Documentation/devicetree/bindings/arm/arm,corstone500.yaml
> new file mode 100644
> index 000000000000..cfe41f7760fd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/arm,corstone500.yaml
> @@ -0,0 +1,30 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/arm/arm,corstone500.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ARM Corstone500
> +
> +maintainers:
> +  - Emekcan Aras <emekcan.aras@arm.com>
> +  - Rui Miguel Silva <rui.silva@linaro.org>
> +
> +description: |+
> +  Corstone-500 is an ideal starting point for feature rich System on Chip
> +  (SoC) designs based on the Cortex-A5 core. These designs can be used in
> +  Internet of Things (IoT) and embedded products.

Yet you do not allow to re-use your DTS, so how this re-usage of design
will happen?

> +
> +  Corstone-500 includes most of the Arm IP in the SSE-500 subsystem and
> +  example integration layer, an FPGA, and access to modelling options.
> +
> +properties:
> +  $nodename:
> +    const: '/'
> +  compatible:
> +    items:

You have one item, so drop "items"

> +      - const: arm,corstone500
> +

required:
  - compatible

> +additionalProperties: true
> +
> +...

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/2] arm64: dts: Add Arm corstone500 platform support
  2022-12-14 13:41   ` Krzysztof Kozlowski
@ 2022-12-14 14:54     ` Krzysztof Kozlowski
  0 siblings, 0 replies; 9+ messages in thread
From: Krzysztof Kozlowski @ 2022-12-14 14:54 UTC (permalink / raw)
  To: Emekcan Aras, Rob Herring, Krzysztof Kozlowski
  Cc: linux-arm-kernel, devicetree

On 14/12/2022 14:41, Krzysztof Kozlowski wrote:
> On 14/12/2022 14:24, Emekcan Aras wrote:
>> From: Emekcan Aras <Emekcan.Aras@arm.com>
>>
> 
> Use subject prefixes matching the subsystem (git log --oneline -- ...).
> You already got such comment for v1... so do not ignore it but implement
> instead.
> 
>> Corstone500[0] is a platform from arm, which includes Cortex-A cores and
>> ideal starting point for feature rich System on Chip (SoC) designs
>> based on the Cortex-A5 core.
>>
>> These device trees contains the necessary bits to support the
>> Corstone 500 FVP (Fixed Virtual Platform) and the
>> FPGA MPS3 board.
>>
>> 0: https://developer.arm.com/documentation/102262/0000
>>
>> Signed-off-by: Emekcan Aras <emekcan.aras@arm.com>
>> ---
>>  arch/arm/boot/dts/corstone500.dts | 181 ++++++++++++++++++++++++++++++
>>  1 file changed, 181 insertions(+)
>>  create mode 100644 arch/arm/boot/dts/corstone500.dts
> 
> How do you test it? I do not see a way to compile it...
> 
> You need to include it in maintainers entry and CC relevant maintainers.
> 
>>
>> diff --git a/arch/arm/boot/dts/corstone500.dts b/arch/arm/boot/dts/corstone500.dts
>> new file mode 100644
>> index 000000000000..976aa333ffbc
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/corstone500.dts
>> @@ -0,0 +1,181 @@
>> +// SPDX-License-Identifier: GPL-2.0 or MIT
>> +/*
>> + * Copyright (c) 2022, Arm Limited. All rights reserved.
>> + *
>> + */
>> +
>> +
> 
> Just one blank line
> 
>> +#include <dt-bindings/interrupt-controller/arm-gic.h>
>> +
>> +/ {
>> +	model = "ARM Corstone500";
>> +	compatible = "arm,corstone500";
> 
> No DTSI? No board or platform compatible? How is it going to be used by
> anyone else?

I guess this is the same as concept as Corstone 1000.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/2] dt-bindings: Add Arm corstone500 platform
  2022-12-14 13:44   ` Krzysztof Kozlowski
@ 2022-12-14 14:55     ` Krzysztof Kozlowski
  2022-12-14 14:57     ` Krzysztof Kozlowski
  1 sibling, 0 replies; 9+ messages in thread
From: Krzysztof Kozlowski @ 2022-12-14 14:55 UTC (permalink / raw)
  To: Emekcan Aras, Rob Herring, Krzysztof Kozlowski
  Cc: linux-arm-kernel, devicetree

On 14/12/2022 14:44, Krzysztof Kozlowski wrote:
> On 14/12/2022 14:24, Emekcan Aras wrote:
>> From: Emekcan Aras <Emekcan.Aras@arm.com>
> 
> 1. Your From does not match SoB exactly. Avoid it and make it consistent.
> 
> 2. Use subject prefixes matching the subsystem (git log --oneline -- ...).
> 
> 3. Please use scripts/get_maintainers.pl to get a list of necessary
> people and lists to CC.  It might happen, that command when run on an
> older kernel, gives you outdated entries.  Therefore please be sure you
> base your patches on recent Linux kernel.
> 
> 
>>
>> Add bindings to describe implementation of
>> the ARM Corstone500 platform.
> 
> Please wrap commit message according to Linux coding style / submission
> process:
> https://elixir.bootlin.com/linux/v5.18-rc4/source/Documentation/process/submitting-patches.rst#L586
> 
>>
>> Signed-off-by: Emekcan Aras <emekcan.aras@arm.com>
>> ---
>>  .../bindings/arm/arm,corstone500.yaml         | 30 +++++++++++++++++++
>>  1 file changed, 30 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/arm/arm,corstone500.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/arm/arm,corstone500.yaml b/Documentation/devicetree/bindings/arm/arm,corstone500.yaml
>> new file mode 100644
>> index 000000000000..cfe41f7760fd
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/arm,corstone500.yaml
>> @@ -0,0 +1,30 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/arm/arm,corstone500.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: ARM Corstone500
>> +
>> +maintainers:
>> +  - Emekcan Aras <emekcan.aras@arm.com>
>> +  - Rui Miguel Silva <rui.silva@linaro.org>
>> +
>> +description: |+
>> +  Corstone-500 is an ideal starting point for feature rich System on Chip
>> +  (SoC) designs based on the Cortex-A5 core. These designs can be used in
>> +  Internet of Things (IoT) and embedded products.
> 
> Yet you do not allow to re-use your DTS, so how this re-usage of design
> will happen?
> 

OK, so similar as Corstone 1000, but please tell me - any reason why you
keep them separate in the bindings? Next time new file for Corstone
1001, 1002, 2000, 2221 etc.?

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/2] dt-bindings: Add Arm corstone500 platform
  2022-12-14 13:44   ` Krzysztof Kozlowski
  2022-12-14 14:55     ` Krzysztof Kozlowski
@ 2022-12-14 14:57     ` Krzysztof Kozlowski
  1 sibling, 0 replies; 9+ messages in thread
From: Krzysztof Kozlowski @ 2022-12-14 14:57 UTC (permalink / raw)
  To: Emekcan Aras, Rob Herring, Krzysztof Kozlowski
  Cc: linux-arm-kernel, devicetree

On 14/12/2022 14:44, Krzysztof Kozlowski wrote:

> 
>> +      - const: arm,corstone500
>> +
> 
> required:
>   - compatible

And this is actually not needed. Most of boards skip it as root-node
schema requires it. This comment can be ignored.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/2] arm64: dts: Add Arm corstone500 platform support
  2022-12-14 13:24 ` [PATCH v2 1/2] arm64: dts: Add Arm corstone500 platform support Emekcan Aras
  2022-12-14 13:41   ` Krzysztof Kozlowski
@ 2022-12-14 16:37   ` Rob Herring
  1 sibling, 0 replies; 9+ messages in thread
From: Rob Herring @ 2022-12-14 16:37 UTC (permalink / raw)
  To: Emekcan Aras; +Cc: Krzysztof Kozlowski, linux-arm-kernel, devicetree

On Wed, Dec 14, 2022 at 01:24:03PM +0000, Emekcan Aras wrote:
> From: Emekcan Aras <Emekcan.Aras@arm.com>
> 
> Corstone500[0] is a platform from arm, which includes Cortex-A cores and
> ideal starting point for feature rich System on Chip (SoC) designs
> based on the Cortex-A5 core.
> 
> These device trees contains the necessary bits to support the
> Corstone 500 FVP (Fixed Virtual Platform) and the
> FPGA MPS3 board.
> 
> 0: https://developer.arm.com/documentation/102262/0000
> 
> Signed-off-by: Emekcan Aras <emekcan.aras@arm.com>
> ---
>  arch/arm/boot/dts/corstone500.dts | 181 ++++++++++++++++++++++++++++++
>  1 file changed, 181 insertions(+)
>  create mode 100644 arch/arm/boot/dts/corstone500.dts
> 
> diff --git a/arch/arm/boot/dts/corstone500.dts b/arch/arm/boot/dts/corstone500.dts
> new file mode 100644
> index 000000000000..976aa333ffbc
> --- /dev/null
> +++ b/arch/arm/boot/dts/corstone500.dts
> @@ -0,0 +1,181 @@
> +// SPDX-License-Identifier: GPL-2.0 or MIT
> +/*
> + * Copyright (c) 2022, Arm Limited. All rights reserved.
> + *
> + */
> +
> +
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +/ {
> +	model = "ARM Corstone500";
> +	compatible = "arm,corstone500";
> +	interrupt-parent = <&gic>;
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +
> +	aliases {
> +		serial0 = &uart0;
> +		serial1 = &uart1;
> +	};
> +
> +	chosen {
> +		stdout-path = "serial0:115200n8";
> +	};
> +
> +	psci {
> +		compatible = "arm,psci-1.0", "arm,psci-0.2", "arm,psci";

You really need to be compatible with all these PSCI versions? 
'arm,psci' means you have the function IDs in DT before they were 
standardized (in 2013 IIRC). You need to support OS's from that time?

> +		method = "smc";
> +		cpu_on = <0x84000003>;

And here is one, but the rest? Really, it should be dropped along with 
'arm,psci'.

> +	};
> +
> +	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		enable-method = "psci";
> +
> +		cpu@0 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a5";
> +			reg = <0>;
> +			next-level-cache = <&L2>;
> +		};
> +
> +		cpu@1 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a5";
> +			reg = <1>;
> +			next-level-cache = <&L2>;
> +		};
> +
> +		cpu@2 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a5";
> +			reg = <2>;
> +			next-level-cache = <&L2>;
> +		};
> +
> +		cpu@3 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a5";
> +			reg = <3>;
> +			next-level-cache = <&L2>;
> +		};
> +	};
> +
> +	memory@80000000 {
> +		device_type = "memory";
> +		reg = <0x80000000 0x7f000000>;
> +	};
> +
> +	L2: cache-controller@1c010000 {
> +		compatible = "arm,pl310-cache";
> +		reg = <0x1c010000 0x1000>;
> +		interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>;
> +		cache-level = <2>;
> +		cache-unified;
> +		arm,data-latency = <1 1 1>;
> +		arm,tag-latency = <1 1 1>;
> +	};
> +
> +	refclk7500khz: refclk7500khz {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <7500000>;
> +		clock-output-names = "apb_pclk";
> +	};
> +
> +	refclk24mhz: refclk24mhz {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <24000000>;
> +		clock-output-names = "apb_pclk";
> +	};
> +
> +	smbclk: refclk24mhzx2 {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <48000000>;
> +		clock-output-names = "smclk";
> +	};
> +
> +	rtc@1a220000 {

Why is this one not under 'soc'. Really, all MMIO nodes should be.


> +		compatible = "arm,pl031", "arm,primecell";
> +		reg = <0x1a220000 0x1000>;
> +		clocks = <&refclk24mhz>;
> +		interrupts = <GIC_SPI 6 (GIC_CPU_MASK_SIMPLE(4) |
> +			IRQ_TYPE_LEVEL_HIGH)>;
> +		clock-names = "apb_pclk";
> +	};
> +
> +	gic: interrupt-controller@1c001000 {
> +		compatible = "arm,cortex-a5-gic";
> +		#interrupt-cells = <3>;
> +		#address-cells = <0>;
> +		interrupt-controller;
> +		reg =	<0x1c001000 0x1000>,
> +			<0x1c000100 0x100>;
> +		interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) |
> +			IRQ_TYPE_LEVEL_HIGH)>;
> +	};
> +
> +	soc{
> +		compatible = "simple-bus";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		clock_frequency = <50000000>;
> +		interrupt-parent = <&gic>;
> +		ranges;
> +
> +		uart0: serial@1a200000 {
> +			compatible = "arm,pl011", "arm,primecell";
> +			reg = <0x1a200000 0x1000>;
> +			interrupts = <GIC_SPI 8 (GIC_CPU_MASK_SIMPLE(4) |
> +				IRQ_TYPE_LEVEL_HIGH)>;
> +			clocks = <&refclk7500khz>;
> +			clock-names = "apb_pclk";
> +		};
> +
> +		uart1: serial@1a210000 {
> +			compatible = "arm,pl011", "arm,primecell";
> +			reg = <0x1a210000 0x1000>;
> +			interrupts = <GIC_SPI 9 (GIC_CPU_MASK_SIMPLE(4) |
> +				IRQ_TYPE_LEVEL_HIGH)>;
> +			clocks = <&refclk7500khz>;
> +			clock-names = "apb_pclk";
> +		};
> +
> +		timer0: timer@1a040000 {
> +			compatible = "arm,armv7-timer-mem";
> +			reg = <0x1a040000 0x1000>;
> +			clock-frequency = <7500000>;
> +
> +			frame@1a050000 {
> +				frame-number = <0>;
> +				interrupts = <GIC_SPI 2 (GIC_CPU_MASK_SIMPLE(4) |
> +				IRQ_TYPE_LEVEL_HIGH)>;
> +				reg = <0x1a050000 0x1000>;
> +			};
> +		};
> +
> +		smsc: ethernet@4020000 {
> +			compatible = "smsc,lan9220", "smsc,lan9115";
> +			reg = <0x40200000 0x10000>;
> +			interrupts = <GIC_SPI 43 (GIC_CPU_MASK_SIMPLE(4) |
> +				IRQ_TYPE_LEVEL_HIGH)>;
> +			reg-io-width = <4>;
> +			phy-mode = "mii";
> +			smsc,irq-active-high;
> +			vdd33a-supply = <&v2m_fixed_3v3>;
> +			vddvario-supply = <&v2m_fixed_3v3>;
> +		};
> +	};
> +
> +	v2m_fixed_3v3: fixed-regulator-0 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "3V3";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		regulator-always-on;
> +	};
> +};
> -- 
> 2.25.1
> 
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-12-14 16:37 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-14 13:24 [PATCH v2 0/2] Adds Corstone500 DeviceTree Emekcan Aras
2022-12-14 13:24 ` [PATCH v2 1/2] arm64: dts: Add Arm corstone500 platform support Emekcan Aras
2022-12-14 13:41   ` Krzysztof Kozlowski
2022-12-14 14:54     ` Krzysztof Kozlowski
2022-12-14 16:37   ` Rob Herring
2022-12-14 13:24 ` [PATCH v2 2/2] dt-bindings: Add Arm corstone500 platform Emekcan Aras
2022-12-14 13:44   ` Krzysztof Kozlowski
2022-12-14 14:55     ` Krzysztof Kozlowski
2022-12-14 14:57     ` Krzysztof Kozlowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).