[PATCH v6 5/8] dt-bindings: timer: hpe,gxp-timer: Creation

[PATCH v6 5/8] dt-bindings: timer: hpe,gxp-timer: Creation

[nick.hawkins]

From: Nick Hawkins <nick.hawkins@hpe.com>

Add support for the HPE GXP Timer and Watchdog. There are multiple
timers on the SoC but only one is enabled at this time.

Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>

---
v6:
* Removed simple-mfd compatible, timer will create watchdog without
  watchdog node.
* Removed timer0 label
* Changed title from HPE GXP TIMER to HPE GXP Timer
* Changed clock name iopclk to iop
* Set additionalProperties to false
* Added space after ',' in the compatible list
* Changed subject to match the log better
* Altered description to take up 75 characters per line
* Changed description to better describe the patch
v5:
* Fix versioning
* Fixed typo time -> timer
v4:
* Made watchdog a child of timer
* Added reference clock
v3:
* Removed maintainer change from patch
* Verified there was no compilation errors
* Added reference code in separate patch of patchset
v2:
* Converted from txt to yaml
---
 .../bindings/timer/hpe,gxp-timer.yaml         | 47 +++++++++++++++++++
 1 file changed, 47 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/hpe,gxp-timer.yaml

diff --git a/Documentation/devicetree/bindings/timer/hpe,gxp-timer.yaml b/Documentation/devicetree/bindings/timer/hpe,gxp-timer.yaml
new file mode 100644
index 000000000000..d33d90f44d28
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/hpe,gxp-timer.yaml
@@ -0,0 +1,47 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/timer/hpe,gxp-timer.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: HPE GXP Timer
+
+maintainers:
+  - Nick Hawkins <nick.hawkins@hpe.com>
+  - Jean-Marie Verdun <verdun@hpe.com>
+
+properties:
+  compatible:
+    const: hpe,gxp-timer
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    const: iop
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+
+additionalProperties: false
+
+examples:
+  - |
+    timer@c0000000 {
+        compatible = "hpe,gxp-timer";
+        reg = <0x80 0x16>;
+        interrupts = <0>;
+        interrupt-parent = <&vic0>;
+        clocks = <&iopclk>;
+        clock-names = "iop";
+    };
-- 
2.17.1

Re: [PATCH v6 5/8] dt-bindings: timer: hpe,gxp-timer: Creation

[Krzysztof Kozlowski]

On 02/05/2022 22:40, nick.hawkins@hpe.com wrote:
> From: Nick Hawkins <nick.hawkins@hpe.com>
> 
> Add support for the HPE GXP Timer and Watchdog. There are multiple
> timers on the SoC but only one is enabled at this time.
> 
> Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>
> 
> ---
> v6:
> * Removed simple-mfd compatible, timer will create watchdog without
>   watchdog node.
> * Removed timer0 label
> * Changed title from HPE GXP TIMER to HPE GXP Timer
> * Changed clock name iopclk to iop
> * Set additionalProperties to false
> * Added space after ',' in the compatible list
> * Changed subject to match the log better

The subject after prefixes does not match it better. What is "creation"?
"Add HPE GXP Timer and Watchdog"

Rest is okay, so this could be fixed while applying (if you are going to
resend, don't loose that comment and don't loose the tag):

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v6 5/8] dt-bindings: timer: hpe,gxp-timer: Creation

[Rob Herring]

On Mon, 02 May 2022 15:40:47 -0500, nick.hawkins@hpe.com wrote:
> From: Nick Hawkins <nick.hawkins@hpe.com>
> 
> Add support for the HPE GXP Timer and Watchdog. There are multiple
> timers on the SoC but only one is enabled at this time.
> 
> Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>
> 
> ---
> v6:
> * Removed simple-mfd compatible, timer will create watchdog without
>   watchdog node.
> * Removed timer0 label
> * Changed title from HPE GXP TIMER to HPE GXP Timer
> * Changed clock name iopclk to iop
> * Set additionalProperties to false
> * Added space after ',' in the compatible list
> * Changed subject to match the log better
> * Altered description to take up 75 characters per line
> * Changed description to better describe the patch
> v5:
> * Fix versioning
> * Fixed typo time -> timer
> v4:
> * Made watchdog a child of timer
> * Added reference clock
> v3:
> * Removed maintainer change from patch
> * Verified there was no compilation errors
> * Added reference code in separate patch of patchset
> v2:
> * Converted from txt to yaml
> ---
>  .../bindings/timer/hpe,gxp-timer.yaml         | 47 +++++++++++++++++++
>  1 file changed, 47 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/timer/hpe,gxp-timer.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH v6 6/8] dt-bindings: arm: hpe: add GXP Support

[nick.hawkins]

From: Nick Hawkins <nick.hawkins@hpe.com>

Add support for HPE GXP. The GXP is based on the cortex a9 processor and
supports arm7.

Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>

---
v6:
* Changed subject to match others in log.
* Changed the commit description.
* Changed the title in the .yaml from HPE BMC GXP SoC Driver to HPE BMC
  GXP platforms.
* Added the oneOf attribute
* Fixed commit description to be close to 75 characters per line
v5:
* Fix version log
v4:
* Removed gxp.yaml
* Created hpe,gxp.yaml based on reviewer input
v3:
* Created gxp.yaml
v2:
* No change
---
 .../devicetree/bindings/arm/hpe,gxp.yaml      | 27 +++++++++++++++++++
 1 file changed, 27 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/hpe,gxp.yaml

diff --git a/Documentation/devicetree/bindings/arm/hpe,gxp.yaml b/Documentation/devicetree/bindings/arm/hpe,gxp.yaml
new file mode 100644
index 000000000000..224bbcb93f95
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/hpe,gxp.yaml
@@ -0,0 +1,27 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/hpe,gxp.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: HPE BMC GXP platforms
+
+maintainers:
+  - Nick Hawkins <nick.hawkins@hpe.com>
+  - Jean-Marie Verdun <verdun@hpe.com>
+
+properties:
+  compatible:
+    oneOf:
+      - description: GXP Based Boards
+        items:
+          - enum:
+              - hpe,gxp-dl360gen10
+          - const: hpe,gxp
+
+required:
+  - compatible
+
+additionalProperties: true
+
+...
-- 
2.17.1

Re: [PATCH v6 6/8] dt-bindings: arm: hpe: add GXP Support

[Krzysztof Kozlowski]

On 02/05/2022 22:40, nick.hawkins@hpe.com wrote:
> From: Nick Hawkins <nick.hawkins@hpe.com>
> 
> Add support for HPE GXP. The GXP is based on the cortex a9 processor and
> supports arm7.
> 
> Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

Re: [PATCH v6 6/8] dt-bindings: arm: hpe: add GXP Support

[Rob Herring]

On Mon, 02 May 2022 15:40:48 -0500, nick.hawkins@hpe.com wrote:
> From: Nick Hawkins <nick.hawkins@hpe.com>
> 
> Add support for HPE GXP. The GXP is based on the cortex a9 processor and
> supports arm7.
> 
> Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>
> 
> ---
> v6:
> * Changed subject to match others in log.
> * Changed the commit description.
> * Changed the title in the .yaml from HPE BMC GXP SoC Driver to HPE BMC
>   GXP platforms.
> * Added the oneOf attribute
> * Fixed commit description to be close to 75 characters per line
> v5:
> * Fix version log
> v4:
> * Removed gxp.yaml
> * Created hpe,gxp.yaml based on reviewer input
> v3:
> * Created gxp.yaml
> v2:
> * No change
> ---
>  .../devicetree/bindings/arm/hpe,gxp.yaml      | 27 +++++++++++++++++++
>  1 file changed, 27 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/hpe,gxp.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH v6 7/8] ARM: dts: Introduce HPE GXP Device tree

[nick.hawkins]

From: Nick Hawkins <nick.hawkins@hpe.com>

The HPE SoC is new to linux. A basic device tree layout with minimum
required for linux to boot including a timer and watchdog support has
been created.

The dts file is empty at this point but will be updated in subsequent
updates as board specific features are enabled.

Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>

---
v6:
* Added cache-controller to CPU
* Removed hpe,gxp-wdt and removed simple-mfd from hpe,gxp-timer
* Added space after ',' in compatible lists containing more than one
  item
* Switched clock name iopclk to iop based on feedback from
  hpe,gxp-timer.yaml
* Added clock labels clock-0 and clock-1
* Added dma-ranges to ahb
* Changed subject to better match ones in the log
* Changed description to fit 75 characters per line
v5:
* Fixed commit message to show previous changes
* Fixed typo ehci -> echi
v4:
* Removed hpe,gxp-cpu-init as it was no longer necessary
* Removed bootargs as requested
* Removed empty ahb node
* Moved reg after compatible, everywhere
* Removed osc and memclk
* Removed syscon@c00000f8 as it was not necessary for boot
* Fixed Alphabetical issue in dts/Makefile
* Added specific board binding for dl360gen10
* Removed empty node
* Added Accurate Clock Architecture
* Fixed generic-echi and generic-ochi issues
* Removed i2cg
v3:
* Fixed issues with warnings
* Used proper patchset format
v2:
* Reduced size of dtsi to essential components
* Followed the proper format for having a dtsi and
  dts
---
 arch/arm/boot/dts/Makefile               |   2 +
 arch/arm/boot/dts/hpe-bmc-dl360gen10.dts |  13 +++
 arch/arm/boot/dts/hpe-gxp.dtsi           | 132 +++++++++++++++++++++++
 3 files changed, 147 insertions(+)
 create mode 100644 arch/arm/boot/dts/hpe-bmc-dl360gen10.dts
 create mode 100644 arch/arm/boot/dts/hpe-gxp.dtsi

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 7c16f8a2b738..293717719c70 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -255,6 +255,8 @@ dtb-$(CONFIG_ARCH_HISI) += \
 	hi3519-demb.dtb
 dtb-$(CONFIG_ARCH_HIX5HD2) += \
 	hisi-x5hd2-dkb.dtb
+dtb-$(CONFIG_ARCH_HPE_GXP) += \
+	hpe-bmc-dl360gen10.dtb
 dtb-$(CONFIG_ARCH_INTEGRATOR) += \
 	integratorap.dtb \
 	integratorap-im-pd1.dtb \
diff --git a/arch/arm/boot/dts/hpe-bmc-dl360gen10.dts b/arch/arm/boot/dts/hpe-bmc-dl360gen10.dts
new file mode 100644
index 000000000000..69e9c6672ea8
--- /dev/null
+++ b/arch/arm/boot/dts/hpe-bmc-dl360gen10.dts
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Device Tree file for HPE DL360Gen10
+ */
+
+/include/ "hpe-gxp.dtsi"
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+	compatible = "hpe,gxp-dl360gen10","hpe,gxp";
+	model = "Hewlett Packard Enterprise ProLiant dl360 Gen10";
+};
diff --git a/arch/arm/boot/dts/hpe-gxp.dtsi b/arch/arm/boot/dts/hpe-gxp.dtsi
new file mode 100644
index 000000000000..7a99e174e4b3
--- /dev/null
+++ b/arch/arm/boot/dts/hpe-gxp.dtsi
@@ -0,0 +1,132 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Device Tree file for HPE GXP
+ */
+
+/dts-v1/;
+/ {
+	model = "Hewlett Packard Enterprise GXP BMC";
+	compatible = "hpe,gxp", "hpe,gxp-dl360gen10";
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		cpu@0 {
+			compatible = "arm,cortex-a9";
+			reg = <0>;
+			device_type = "cpu";
+			next-level-cache = <&L2>;
+		};
+	};
+
+	clocks {
+		pll: clock-0 {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <1600000000>;
+		};
+
+		iopclk: clock-1 {
+			compatible = "fixed-factor-clock";
+			#clock-cells = <0>;
+			clock-div = <4>;
+			clock-mult = <1>;
+			clocks = <&pll>;
+		};
+	};
+
+	memory@40000000 {
+		device_type = "memory";
+		reg = <0x40000000 0x20000000>;
+	};
+
+	axi {
+		compatible = "simple-bus";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+		dma-ranges;
+
+		L2: cache-controller@b0040000 {
+			compatible = "arm,pl310-cache";
+			reg = <0xb0040000 0xFFC>;
+			cache-unified;
+			cache-level = <2>;
+		};
+
+		ahb@c0000000 {
+			compatible = "simple-bus";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges = <0x0 0xc0000000 0x30000000>;
+			dma-ranges;
+
+			vic0: interrupt-controller@eff0000 {
+				compatible = "arm,pl192-vic";
+				reg = <0xeff0000 0x1000>;
+				interrupt-controller;
+				#interrupt-cells = <1>;
+			};
+
+			vic1: interrupt-controller@80f00000 {
+				compatible = "arm,pl192-vic";
+				reg = <0x80f00000 0x1000>;
+				interrupt-controller;
+				#interrupt-cells = <1>;
+			};
+
+			uarta: serial@e0 {
+				compatible = "ns16550a";
+				reg = <0xe0 0x8>;
+				interrupts = <17>;
+				interrupt-parent = <&vic0>;
+				clock-frequency = <1846153>;
+				reg-shift = <0>;
+			};
+
+			uartb: serial@e8 {
+				compatible = "ns16550a";
+				reg = <0xe8 0x8>;
+				interrupts = <18>;
+				interrupt-parent = <&vic0>;
+				clock-frequency = <1846153>;
+				reg-shift = <0>;
+			};
+
+			uartc: serial@f0 {
+				compatible = "ns16550a";
+				reg = <0xf0 0x8>;
+				interrupts = <19>;
+				interrupt-parent = <&vic0>;
+				clock-frequency = <1846153>;
+				reg-shift = <0>;
+			};
+
+			usb0: usb@efe0000 {
+				compatible = "hpe,gxp-ehci", "generic-ehci";
+				reg = <0xefe0000 0x100>;
+				interrupts = <7>;
+				interrupt-parent = <&vic0>;
+			};
+
+			st: timer@80 {
+				compatible = "hpe,gxp-timer";
+				reg = <0x80 0x16>;
+				interrupts = <0>;
+				interrupt-parent = <&vic0>;
+				clocks = <&iopclk>;
+				clock-names = "iop";
+			};
+
+			usb1: usb@efe0100 {
+				compatible = "hpe,gxp-ohci", "generic-ohci";
+				reg = <0xefe0100 0x110>;
+				interrupts = <6>;
+				interrupt-parent = <&vic0>;
+			};
+		};
+	};
+};
-- 
2.17.1

Re: [PATCH v6 7/8] ARM: dts: Introduce HPE GXP Device tree

[Krzysztof Kozlowski]

On 02/05/2022 22:40, nick.hawkins@hpe.com wrote:
> From: Nick Hawkins <nick.hawkins@hpe.com>
> 
> The HPE SoC is new to linux. A basic device tree layout with minimum
> required for linux to boot including a timer and watchdog support has
> been created.
> 
> The dts file is empty at this point but will be updated in subsequent
> updates as board specific features are enabled.
> 
> Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>
> 
> ---
> v6:
> * Added cache-controller to CPU
> * Removed hpe,gxp-wdt and removed simple-mfd from hpe,gxp-timer
> * Added space after ',' in compatible lists containing more than one
>   item

Hmmm....

>  	integratorap-im-pd1.dtb \
> diff --git a/arch/arm/boot/dts/hpe-bmc-dl360gen10.dts b/arch/arm/boot/dts/hpe-bmc-dl360gen10.dts
> new file mode 100644
> index 000000000000..69e9c6672ea8
> --- /dev/null
> +++ b/arch/arm/boot/dts/hpe-bmc-dl360gen10.dts
> @@ -0,0 +1,13 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Device Tree file for HPE DL360Gen10
> + */
> +
> +/include/ "hpe-gxp.dtsi"
> +
> +/ {
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +	compatible = "hpe,gxp-dl360gen10","hpe,gxp";

Missing space after ','

With this fixed:
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

RE: [PATCH v6 7/8] ARM: dts: Introduce HPE GXP Device tree

[Hawkins, Nick]

On 02/05/2022 22:40, nick.hawkins@hpe.com wrote:
> > From: Nick Hawkins <nick.hawkins@hpe.com>
> > 
> > The HPE SoC is new to linux. A basic device tree layout with minimum 
> > required for linux to boot including a timer and watchdog support has 
> > been created.
> > 
> > The dts file is empty at this point but will be updated in subsequent 
> > updates as board specific features are enabled.
> > 
> > Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>
> > 
> > ---
> > v6:
> > * Added cache-controller to CPU
> > * Removed hpe,gxp-wdt and removed simple-mfd from hpe,gxp-timer
> > * Added space after ',' in compatible lists containing more than one
> >   item

> Hmmm....

> >  	integratorap-im-pd1.dtb \
> > diff --git a/arch/arm/boot/dts/hpe-bmc-dl360gen10.dts 
> > b/arch/arm/boot/dts/hpe-bmc-dl360gen10.dts
> > new file mode 100644
> > index 000000000000..69e9c6672ea8
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/hpe-bmc-dl360gen10.dts
> > @@ -0,0 +1,13 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Device Tree file for HPE DL360Gen10  */
> > +
> > +/include/ "hpe-gxp.dtsi"
> > +
> > +/ {
> > +	#address-cells = <1>;
> > +	#size-cells = <1>;
> > +	compatible = "hpe,gxp-dl360gen10","hpe,gxp";

> Missing space after ','

> With this fixed:
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

I forgot to check this file while fixing the spacing. Thank you for your review and apologies about missing this.

Thanks,

-Nick Hawkins

Re: [PATCH v6 7/8] ARM: dts: Introduce HPE GXP Device tree

[Arnd Bergmann]

On Mon, May 2, 2022 at 10:40 PM <nick.hawkins@hpe.com> wrote:

> --- /dev/null
> +++ b/arch/arm/boot/dts/hpe-bmc-dl360gen10.dts
> @@ -0,0 +1,13 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Device Tree file for HPE DL360Gen10
> + */
> +
> +/include/ "hpe-gxp.dtsi"
> +
> +/ {
> +       #address-cells = <1>;
> +       #size-cells = <1>;
> +       compatible = "hpe,gxp-dl360gen10","hpe,gxp";
> +       model = "Hewlett Packard Enterprise ProLiant dl360 Gen10";
> +};

In the board specific file, you normally have "aliases", "chosen" and "memory"
nodes that define e.g. which uart is used for the console.

> diff --git a/arch/arm/boot/dts/hpe-gxp.dtsi b/arch/arm/boot/dts/hpe-gxp.dtsi
> new file mode 100644
> index 000000000000..7a99e174e4b3
> --- /dev/null
> +++ b/arch/arm/boot/dts/hpe-gxp.dtsi
> @@ -0,0 +1,132 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Device Tree file for HPE GXP
> + */
> +
> +/dts-v1/;
> +/ {
> +       model = "Hewlett Packard Enterprise GXP BMC";
> +       compatible = "hpe,gxp", "hpe,gxp-dl360gen10";

I think "hpe,gxp-dl360gen10" is misplaced here, if that is the identifier
of a particular machine rather than the SoC.

> +       memory@40000000 {
> +               device_type = "memory";
> +               reg = <0x40000000 0x20000000>;
> +       };

If the memory is outside of the SoC, it should not be here.

> +       axi {
> +               compatible = "simple-bus";
> +               #address-cells = <1>;
> +               #size-cells = <1>;
> +               ranges;
> +               dma-ranges;
> +
> +               L2: cache-controller@b0040000 {
> +                       compatible = "arm,pl310-cache";
> +                       reg = <0xb0040000 0xFFC>;
> +                       cache-unified;
> +                       cache-level = <2>;
> +               };

The length of the register area looks wrong, this is usually the full
4KB area, rather than leaving out the last four bytes.

        Arnd

RE: [PATCH v6 7/8] ARM: dts: Introduce HPE GXP Device tree

[Hawkins, Nick]

On Mon, May 2, 2022 at 10:40 PM <nick.hawkins@hpe.com> wrote:


> > +++ b/arch/arm/boot/dts/hpe-bmc-dl360gen10.dts

> In the board specific file, you normally have "aliases", "chosen" and "memory"
> nodes that define e.g. which uart is used for the console.

This is what I have come up with.

/ {
        #address-cells = <1>;
        #size-cells = <1>;
        compatible = "hpe,gxp-dl360gen10", "hpe,gxp";
        model = "Hewlett Packard Enterprise ProLiant dl360 Gen10";

        aliases {
                serial2 = &uartc;
        };

        chosen {
                bootargs = "console=ttyS2,115200";
        };

        memory@40000000 {
                device_type = "memory";
                reg = <0x40000000 0x20000000>;
        };
};

> > +/dts-v1/;
> > +/ {
> > +       model = "Hewlett Packard Enterprise GXP BMC";
> > +       compatible = "hpe,gxp", "hpe,gxp-dl360gen10";

> I think "hpe,gxp-dl360gen10" is misplaced here, if that is the identifier of a particular machine rather than the SoC.

Yes that does not look correct there it will now be compatible = "hpe,gxp";

> > +       memory@40000000 {
> > +               device_type = "memory";
> > +               reg = <0x40000000 0x20000000>;
> > +       };

> If the memory is outside of the SoC, it should not be here.

It is part of the SoC. Should I put it in the axi bus instead? If so would I need to create an axi for the .dts as well?

Thanks for the review,

-Nick Hawkins

Re: [PATCH v6 7/8] ARM: dts: Introduce HPE GXP Device tree

[Arnd Bergmann]

On Tue, May 3, 2022 at 5:07 PM Hawkins, Nick <nick.hawkins@hpe.com> wrote:
> On Mon, May 2, 2022 at 10:40 PM <nick.hawkins@hpe.com> wrote:
> > > +++ b/arch/arm/boot/dts/hpe-bmc-dl360gen10.dts
>
> > In the board specific file, you normally have "aliases", "chosen" and "memory"
> > nodes that define e.g. which uart is used for the console.
>
> This is what I have come up with.
>
> / {
>         #address-cells = <1>;
>         #size-cells = <1>;
>         compatible = "hpe,gxp-dl360gen10", "hpe,gxp";
>         model = "Hewlett Packard Enterprise ProLiant dl360 Gen10";
>
>         aliases {
>                 serial2 = &uartc;
>         };

It's a bit unusual to have a serial2 uart when there is no serial0 or serial1.
Most boards that only have a single uart label it either '0' or '1'.
>
>         chosen {
>                 bootargs = "console=ttyS2,115200";
>         };

Use 'stdout-path' to pick the console device here, not the argument
string.

>
>         memory@40000000 {
>                 device_type = "memory";
>                 reg = <0x40000000 0x20000000>;
>         };
> };


> > > +       memory@40000000 {
> > > +               device_type = "memory";
> > > +               reg = <0x40000000 0x20000000>;
> > > +       };
>
> > If the memory is outside of the SoC, it should not be here.
>
> It is part of the SoC. Should I put it in the axi bus instead? If so would
> I need to create an axi for the .dts as well?

That's fine then. The memory node has to be at the root of the DT, not in a
child node. It's just that most SoCs have the memory in a separate chip
on the board, rather than inside of the SoC itself, so it would be part
of the board file. If the gxp package includes the memory and it's not
an option for the board design, you can leave it in the .dtsi file.

         Arnd