On 12/12/2023 17:40, Ninad Palsule wrote: > This commit adds following devices to the device tree. > - GPIO pin assignements, GPIO expansion devices > - LED brinker devices > - Fan controllers > > Tested: > This board is tested using the simics simulator. > > Signed-off-by: Ninad Palsule <ninad@xxxxxxxxxxxxx> > --- > .../dts/aspeed/aspeed-bmc-ibm-system1.dts | 547 +++++++++++++++++- Squash it. > 1 file changed, 542 insertions(+), 5 deletions(-) > > diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-system1.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-system1.dts > index b8e7e52d4600..75562aa63701 100644 > --- a/arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-system1.dts > +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-system1.dts > @@ -114,6 +114,99 @@ vga_memory: region@bf000000 { > }; > }; > > + leds { > + compatible = "gpio-leds"; > + > + bmc-ready { It does not look like you tested the DTS against bindings. Please run `make dtbs_check W=1` (see Documentation/devicetree/bindings/writing-schema.rst or https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/ for instructions). > + gpios = <&gpio0 ASPEED_GPIO(L, 7) GPIO_ACTIVE_HIGH>; > + }; > + > + bmc-hb { None of these were tested. > /*A0-A7*/ "","","","","","","","", > - /*B0-B7*/ "","","","","","","","", > + /*B0-B7*/ "","","","","bmc-tpm-reset","","","", Really? You just added these lines. There is no point in adding a new line and immediately changing it. This points how your split is artificial and not helpful. ... > &i2c2 { > @@ -486,6 +744,20 @@ regulator@43 { > &i2c6 { > status = "okay"; > > + fan-controller@52 { > + compatible = "maxim,max31785a"; > + reg = <0x52>; > + #address-cells = <1>; > + #size-cells = <0>; Why do you need cells? > + }; > + > + fan-controller@54 { > + compatible = "maxim,max31785a"; > + reg = <0x54>; > + #address-cells = <1>; > + #size-cells = <0>; Why do you need cells? > + }; > + > i2c-mux@70 { > compatible = "nxp,pca9548"; > reg = <0x70>; > @@ -522,6 +794,48 @@ i2c6mux0chn4: i2c@4 { > #address-cells = <1>; > #size-cells = <0>; > reg = <4>; > + > + led-controller@60 { > + compatible = "nxp,pca9551"; > + reg = <0x60>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + gpio-controller; > + #gpio-cells = <2>; > + > + led@0 { > + label = "enclosure-id-led"; > + reg = <0>; > + retain-state-shutdown; > + default-state = "keep"; > + type = <PCA955X_TYPE_LED>; > + }; > + > + led@1 { > + label = "attention-led"; > + reg = <1>; > + retain-state-shutdown; > + default-state = "keep"; > + type = <PCA955X_TYPE_LED>; > + }; > + > + led@2 { > + label = "enclosure-fault-rollup-led"; > + reg = <2>; > + retain-state-shutdown; > + default-state = "keep"; > + type = <PCA955X_TYPE_LED>; > + }; > + > + led@3 { > + label = "power-on-led"; > + reg = <3>; > + retain-state-shutdown; > + default-state = "keep"; > + type = <PCA955X_TYPE_LED>; > + }; > + }; > }; > > i2c6mux0chn5: i2c@5 { > @@ -542,6 +856,44 @@ i2c6mux0chn7: i2c@7 { > reg = <7>; > }; > }; > + > + pca3: pca9539@74 { Node names should be generic. See also an explanation and list of examples (not exhaustive) in DT specification: https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > + compatible = "nxp,pca9539"; > + reg = <0x74>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + gpio-controller; > + #gpio-cells = <2>; > + }; > + > + pca4: pca9539@77 { Node names should be generic. See also an explanation and list of examples (not exhaustive) in DT specification: https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > + compatible = "nxp,pca9539"; > + reg = <0x77>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + gpio-controller; > + #gpio-cells = <2>; > + > + gpio-line-names = > + "PE_NVMED0_EXP_PRSNT_N", > + "PE_NVMED1_EXP_PRSNT_N", > + "PE_NVMED2_EXP_PRSNT_N", > + "PE_NVMED3_EXP_PRSNT_N", > + "LED_FAULT_NVMED0", > + "LED_FAULT_NVMED1", > + "LED_FAULT_NVMED2", > + "LED_FAULT_NVMED3", > + "FAN0_PRESENCE_R_N", > + "FAN1_PRESENCE_R_N", > + "FAN2_PRESENCE_R_N", > + "FAN3_PRESENCE_R_N", > + "FAN4_PRESENCE_R_N", > + "FAN5_PRESENCE_N", > + "FAN6_PRESENCE_N", > + ""; > + }; > }; > > &i2c7 { > @@ -809,6 +1161,191 @@ regulator@41 { > compatible = "infineon,ir38263"; > reg = <0x41>; > }; > + > + led-controller@61 { > + compatible = "nxp,pca9552"; > + reg = <0x61>; > + #address-cells = <1>; > + #size-cells = <0>; > + ... > + led@15 { > + label = "pe-cp-drv3-perst"; > + reg = <15>; > + retain-state-shutdown; > + default-state = "keep"; > + type = <PCA955X_TYPE_LED>; > + }; > + }; > + > + pca1: pca9539@75 { Node names should be generic. See also an explanation and list of examples (not exhaustive) in DT specification: https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation Best regards, Krzysztof