Re: [PATCH v9 2/2] ARM: dts: aspeed: ventura2: Add Meta ventura2 BMC

[Kyle Hsieh]

Hi Andrew,

On Fri, Aug 21, 2026 at 2:31 PM Kyle Hsieh <kylehsieh1995@xxxxxxxxx> wrote:
>
> Hi Tan,
>
> Thanks for the review.
>
> On Fri, Aug 21, 2026 at 2:04 AM Tan Siewert <tan.siewert@xxxxxxxxxxxxx> wrote:
> >
> > Hi Kyle,
> >
> > > Ventura2 is a Rack Management Controller. It is a modular
> > > device designed to manage liquid cooling systems and monitor hardware
> > > states within an IT rack. The system uses an AST2600 BMC for management.
> > >
> > > RMCv2 serves several critical roles:
> > > - Detects liquid leakage at both tray and rack levels.
> > > - Communicates with and controls liquid cooling equipment.
> > > - Manages leakage events and executes system recovery protocols.
> > >
> > > Key hardware features include:
> > > - An extensive I2C and IO expander topology to support comprehensive
> > >   sensor monitoring and backward compatibility with legacy trays.
> > > - MCTP over I2C support for asynchronous device communications.
> > > - A dual-flash design for BMC firmware redundancy.
> > >
> > > Signed-off-by: Kyle Hsieh <kylehsieh1995@xxxxxxxxx>
> >
> > *snip*
> >
> > > diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-ventura2.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-ventura2.dts
> > > new file mode 100644
> > > index 000000000000..f1273ca31bf8
> > > --- /dev/null
> > > +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-ventura2.dts
> > > @@ -0,0 +1,3005 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +// Copyright (c) 2023 Facebook Inc.
> > > +/dts-v1/;
> > > +
> > > +#include "aspeed-g6.dtsi"
> > > +#include <dt-bindings/i2c/i2c.h>
> > > +#include <dt-bindings/gpio/aspeed-gpio.h>
> > > +
> > > +/ {
> > > +     compatible = "facebook,ventura2-rmc", "aspeed,ast2600";
> > > +     model = "Facebook Ventura2 RMC";
> > > +
> > > +     aliases {
> > > +             /*
> > > +             * Pre-allocate I2C bus aliases for userspace predictability.
> > > +             * Several I2C channels are intentionally left empty in this
> > > +             * DTS as they are strictly reserved for future hardware
> > > +             * feature expansions and add-on boards that will interface
> > > +             * with these busses.
> > > +             */
> > > +             /*
> > > +             * i2c switch 0-0077, pca9548, 8 child channels assigned
> > > +             * with bus number 16-23.
> > > +             */
> >
> > This alignment is one of the things that seem to be a false-positive for
> > dt-check-style in strict mode (value-whitespace). See [0] where Krzysztof said
> > that it may report false-positives, and "Review and investigate warnings from
> > 'strict' mode, but do not fix them automatically without thinking".
> >
> > @Krzysztof wdyt about this one?
>
> You're right, and this is a regression I introduced in v9. In v8 these comment
> blocks used the standard kernel style. While running dt-check-style, I applied
> the strict-mode output too mechanically and dropped the space before '*' in all
> 38 comment blocks in the file. I will restore to the standard style in v10.

This is the part I'd like your input on, since it goes against your v8
review where you asked me to fix the 49 mixed-indent-chars reported in
relaxed mode.

Looking at the output again, all 52 hits on the current file are on comment
lines. The standard kernel comment style '\t * ' is a tab followed by a
space, so the tool flags every correctly formatted multi-line comment. The
49 you saw in v8 were the same thing. [property-order] on the spi node
behaves the same way - it wants #address-cells and #size-cells split apart,
which is what Tan asked me to change back.

So I can either restore the kernel comment style and note these as false
positives in the cover letter, or keep the v9 form that satisfies the tool.
Which would you prefer for v10?

Thanks,
Kyle Hsieh