Re: [PATCH v3 2/2] ARM: dts: aspeed: add asrock x470d4u bmc

[Tan Siewert]

Hi Zev,

Many thanks for your review!

On 16.08.26 06:49, Zev Weiss wrote:
> On Wed, Aug 12, 2026 at 01:28:27PM PDT, Tan Siewert wrote:
> > diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x470d4u- 
> > data64.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x470d4u- 
> > data64.dts
> > new file mode 100644
> > index 000000000000..6f11aeab087c
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x470d4u-data64.dts
> > @@ -0,0 +1,15 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +
> > +#include "aspeed-bmc-asrock-x470d4u.dts"
> > +
> > +/ {
> > +    compatible = "asrock,x470d4u-data64-bmc", "aspeed,ast2500";
> > +    model = "Asrock Rack X470D4U-series BMC (64MB store)";
>
> Nit: for consistency with how they spell it, and (at least *most* of) 
> the other aspeed-bmc-asrock-*.dts files, I'd suggest "ASRock" instead of 
> "Asrock".  (And IMO s/store/flash/ would be a touch clearer.)

Agreed on both, will fix it in v4.

> Also, looks like the other nearby DTS files pretty consistently have 
> compatible & model in the opposite order, so matching that might be nice 
> too.

This is something that was reported by `scripts/dtc/dt-style-check 
--mode strict`. Depending on how much time I have next week I'll look 
into fixing the dtc offsenses for arm/dts/aspeed (tho that'd come from 
my employer email).

> > +};
> > +
> > +&fmc {
> > +    flash@0 {
> > +        /delete-node/partitions;
> > +#include "openbmc-flash-layout-64.dtsi"
> > +    };
> > +};
> > diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x470d4u.dts b/ 
> > arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x470d4u.dts
> > new file mode 100644
> > index 000000000000..80fb2b7c1e0c
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x470d4u.dts
>
> <snip>
>
> > +
> > +    fan@3 {
> > +        /* FAN4 (6-pin) */
> > +        reg = <0x03>;
> > +        aspeed,fan-tach-ch = /bits/ 8 <0x03 0x0b>;
> > +    };
> > +
> > +    fan@4 {
> > +        /* FAN5 (6-pin) */
> > +        reg = <0x04>;
> > +        aspeed,fan-tach-ch = /bits/ 8 <0x04 0x0d>;
> > +    };
> > +
> > +    fan@5 {
> > +        /* FAN6 (6-pin) */
> > +        reg = <0x05>;
> > +        aspeed,fan-tach-ch = /bits/ 8 <0x05 0x0e>;
> > +    };
> > +};
>
> Just to confirm, skipping channel 0xc here is intentional I assume? 
> (Most other related platforms have them contiguously numbered, but not 
> all, so another slight oddity wouldn't be too shocking.)

The skip is intentional. That is what I found out during reverse 
engineering the SDR file in the vendor firmware.

Cheers,
Tan