[PATCH v3 09/10] arch: arm: boot: dts: Introduce HPE GXP Device tree

[Arnd Bergmann]

On Thu, Mar 10, 2022 at 8:52 PM <nick.hawkins at hpe.com> wrote:
>
> From: Nick Hawkins <nick.hawkins at hpe.com>
>
> The HPE SoC is new to linux. This patch
> creates the basic device tree layout with minimum required
> for linux to boot. This includes timer and watchdog
> support.
>
> Signed-off-by: Nick Hawkins <nick.hawkins at hpe.com>
> +/ {
> +       #address-cells = <1>;
> +       #size-cells = <1>;
> +       compatible = "hpe,gxp";
> +       model = "Hewlett Packard Enterprise ProLiant dl360 Gen10";
> +
> +       chosen {
> +               bootargs = "earlyprintk console=ttyS2,115200";
> +       };

Please drop the bootargs here, you definitely should not have 'earlyprintk'
in the bootargs because that is incompatible with cross-platform kernels.

Instead of passing the console in the bootargs, use the "stdout-path"
property.

The "compatible" property should be a list that contains at least the specific
SoC variant and an identifier for the board. "hpe,gxp" is way too generic
to be the only property here.
> +       gxp-init at cefe0010 {
> +               compatible = "hpe,gxp-cpu-init";
> +               reg = <0xcefe0010 0x04>;
> +       };
> +
> +       memory at 40000000 {
> +               device_type = "memory";
> +               reg = <0x40000000 0x20000000>;
> +       };
> +
> +       ahb {
> +               compatible = "simple-bus";
> +               #address-cells = <1>;
> +               #size-cells = <1>;
> +               device_type = "soc";
> +               ranges;
> +
> +               vic0: interrupt-controller at ceff0000 {
> +                       compatible = "arm,pl192-vic";
> +                       interrupt-controller;
> +                       reg = <0xceff0000 0x1000>;
> +                       #interrupt-cells = <1>;
> +               };

I don't think this represents the actual hierarchy of the devices:
the register range of the "vic0" and the "gxp-init" is very close
together, which usually indicates that they are also on the same
bus.



> +               vic1: interrupt-controller at 80f00000 {
> +                       compatible = "arm,pl192-vic";
> +                       interrupt-controller;
> +                       reg = <0x80f00000 0x1000>;
> +                       #interrupt-cells = <1>;
> +               };
> +
> +               timer0: timer at c0000080 {
> +                       compatible = "hpe,gxp-timer";
> +                       reg = <0xc0000080 0x1>, <0xc0000094 0x01>, <0xc0000088 0x08>;
> +                       interrupts = <0>;
> +                       interrupt-parent = <&vic0>;
> +                       clock-frequency = <400000000>;
> +               };
> +
> +               uarta: serial at c00000e0 {
> +                       compatible = "ns16550a";
> +                       reg = <0xc00000e0 0x8>;
> +                       interrupts = <17>;
> +                       interrupt-parent = <&vic0>;
> +                       clock-frequency = <1846153>;
> +                       reg-shift = <0>;
> +               };

In turn, you seem to have a lot of other devices on low addresses
of the 0xc0000000 range, which would be an indication that these
are on a different bus from the others.

Please see if you can find an internal datasheet that describes the
actual device hierarchy, and then try to model this in the device tree.

Use non-empty "ranges" properties to describe the address ranges
and how they get translated between these buses, and add
"dma-ranges" for any high-speed buses that have DMA master
capable devices like USB on them.

> +               i2cg: syscon at c00000f8 {
> +                       compatible = "simple-mfd", "syscon";
> +                       reg = <0xc00000f8 0x08>;
> +               };
> +       };

It's odd to have a "syscon" device that only has 8 bytes worth of registers.

What are the contents of this? Is it possible to have a proper abstraction
for it as something that has a specific purpose rather than being a
random collection of individual bits?

       Arnd