On 21/04/2022 21:21, nick.hawkins@xxxxxxx wrote: > From: Nick Hawkins <nick.hawkins@xxxxxxx> > Thank you for your patch. There is something to discuss/improve. > +/include/ "hpe-gxp.dtsi" > + > +/ { > + #address-cells = <1>; > + #size-cells = <1>; > + compatible = "hpe,gxp-dl360gen10","hpe,gxp"; Missing space after ','. > + 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..a3a082d21101 > --- /dev/null > +++ b/arch/arm/boot/dts/hpe-gxp.dtsi > @@ -0,0 +1,128 @@ > +// 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"; The same. > + #address-cells = <1>; > + #size-cells = <1>; > + > + cpus { > + #address-cells = <1>; > + #size-cells = <0>; > + > + cpu@0 { > + compatible = "arm,cortex-a9"; > + reg = <0>; > + device_type = "cpu"; > + }; > + }; > + > + clocks { > + No need for blank line. > + pll: pll { Generic node names, so either "clock-0" or "pll-clock" > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <1600000000>; > + }; > + > + iopclk: iopclk { "clock-1" or "iop-clock" > + compatible = "fixed-factor-clock"; > + #clock-cells = <0>; > + clock-div = <4>; > + clock-mult = <1>; > + clocks = <&pll>; > + }; > + }; (...) > + > + usb0: usb@efe0000 { > + compatible = "hpe,gxp-ehci","generic-ehci"; Here and in other places - always missing a space. > + reg = <0xefe0000 0x100>; > + interrupts = <7>; > + interrupt-parent = <&vic0>; > + }; > + > + st: timer@80 { > + compatible = "hpe,gxp-timer","simple-mfd"; > + reg = <0x80 0x16>; > + interrupts = <0>; > + interrupt-parent = <&vic0>; > + clocks = <&iopclk>; > + clock-names = "iopclk"; > + watchdog { > + compatible = "hpe,gxp-wdt"; > + }; > + }; > + > + usb1: usb@efe0100 { > + compatible = "hpe,gxp-ohci","generic-ohci"; > + reg = <0xefe0100 0x110>; > + interrupts = <6>; > + interrupt-parent = <&vic0>; > + }; > + }; > + }; > + No need for blank line. > +}; Best regards, Krzysztof