Re: [PATCH v3 2/3] ARM: dts: aspeed: add device tree for ASRock Rack ALTRAD8 BMC

[Tan Siewert <tan.siewert@hetzner.com>]

[-- Attachment #1.1: Type: text/plain, Size: 4518 bytes --]

On 04.12.25 23:29, Tan Siewert wrote:
> On 04.12.25 22:02, Rebecca Cran wrote:
>> The ALTRAD8 BMC is an Aspeed AST2500-based BMC for the ASRock Rack
>> ALTRAD8UD-1L2T and ALTRAD8UD2-1L2Q boards.
>>
>> Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
>> ---
>>   arch/arm/boot/dts/aspeed/Makefile                      |   1 +
>>   arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-altrad8.dts | 612 +++++++ 
>> +++++++++++++
>>   2 files changed, 613 insertions(+)

*snip*

>> diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-altrad8.dts b/ 
>> arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-altrad8.dts
>> new file mode 100644
>> index 000000000000..2696a8692607
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-altrad8.dts

*snip*

>> +&gpio {
>> +    gpio-line-names =
>> +    /*A0-A7*/    "","","","bmc-ready","","","","",
>> +    /*B0-B7*/    "i2c-backup-sel","","","","","","","host0-shd-ack-n",
>> +    /*C0-C7*/    "","","","","","","","",
>> +    /*D0-D7*/    "power-button","power-out","button-reset",

I just noticed that I overlooked the power buttons in my recent review.

power-button = button-power-n (active low)

power-out = control-power-n (active low)

button-reset = button-reset-n (active low)

The other review comments remain.

-- Tan

>> +            "host0-sysreset-n","","","power-chassis-good","",
>> +    /*E0-E7*/    "","s0-vrd1-vddq0123-fault-n",
>> +            "s0-vrd1-vddq4567-fault-n","s0-vrd0-vddc-fault-n",
>> +            "s0-vrd3-p0v75-fault-n","","","",
>> +    /*F0-F7*/    "","","ps-atx-on-n","","","","","",
>> +    /*G0-G7*/    "led-identify","button-identify","","","uboot-ready",\
> 
> `button-identify` should be `button-identify-n` (active-low).
> 
> `led-system-fault-n` is GPIOG3.
> 
> `led-identify-n` _could_ be at GPIOQ6 (seems to be wired to the CPLD (?!)).
> 
>> +            "bmc-salt2-n","","",
>> +    /*H0-H7*/    "ps-pwr-ok","uart1-mode1","uart2-mode1","uart3-mode1",
>> +            "uart4-mode1","","led-bmc-heartbeat","",
> 
> GPIOH6 is `BMC_JTAG_SEL_S0`. The `led-bmc-hearbeat-n` should be GPIOG0.
> 
>> +    /*I0-I7*/    "","","","","","","","",
>> +    /*J0-J7*/    "s0-hightemp-n","","","","","","","",
>> +    /*K0-K7*/    "","","","","","","","",
>> +    /*L0-L7*/    "","","","","","","","",
>> +    /*M0-M7*/    "","","","","","s0-spi-auth-fail-n","","",
>> +    /*N0-N7*/    "","","","","","","","",
>> +    /*O0-O7*/    "","","","","","","","",
>> +    /*P0-P7*/    "","","cpld-disable-bmc","","","","","",
> 
> `cpld-disable-bmc` should be `cpld-disable-bmc-n` and at GPIOM0 
> (`CPLD_DISABLE_BMC_L` in schematic). GPIOP2 is a fan tacho.
> 
>> +    /*Q0-Q7*/    "","","ext-hightemp-n","","","","",
> 
> double assignment of `ext-hightemp-n`. Correct one is GPIOR2. Please 
> remove it.
> 
>> +            "chassis-intrusion",
> 
> `chassis-intrusion-n` (active-low).
> 
>> +    /*R0-R7*/    "","","ext-hightemp-n","spi0-program-sel","",
>> +            "hwm-bat-en","","",
> 
> `hwm-bat-en` could be prefixed with `output-`.
> 
>> +    /*S0-S7*/    "s0-vr-hot-n","","","bmc-sysreset-n","","","","",
> 
> GPIOS2 (`input-salt5-n`) _could_ be useful (comes from the X550 for WoL 
> (`LAN_3V3STBY_ALRT_N` in schematic)).
> 
>> +    /*T0-T7*/    "","","","","","","","",
>> +    /*U0-U7*/    "","","","","","","","",
>> +    /*V0-V7*/    "","","","","","","","",
>> +    /*W0-W7*/    "","","","","","","","",
>> +    /*X0-X7*/    "","","","","","","","",
>> +    /*Y0-Y7*/    "sio-s3","sio-s5","sio-pwr-req","sio-on- 
>> ctrl","","","","",
>> +    /*Z0-Z7*/    "","sio-pwr-good","","s0-rtc-lock","","","","",
> 
> System has no Super-IO. Please remove the `sio-` assignments.
> 
>> +    /*AA0-AA7*/    "rtc-int","","","","","pmbus-sel-n","","",
> 
> `rtc-int` should be `s0-rtc-int-n`.
> 
>> +    /*AB0-AB7*/    "host0-reboot-ack-n","s0-sys-auth-failure-n",
>> +            "","","","","","",
>> +    /*AC0-AC7*/    "s0-fault-alert","host0-ready","s0-overtemp-n",
>> +            "","bmc-ok","host0-special-boot","presence-cpu0",
>> +            "host0-shd-req-n";
>> +
>> +    status = "okay";
>> +};


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]