Re: [PATCH v4 02/12] ARM: dts: aspeed: anacapa: Add EVT1 devicetree and point wrapper to it

Colin Huang <[email protected]> Thu, 6 Aug 2026 15:47:37 +0800
Newsgroups dev.linux.lists.sashiko-reviews,org.kernel.vger.linux-devicetree
Message-ID <CAPBH0A9Op2U7THS6APB+oMvRJQp28sAU9+NaUTFoxLpDfEQBRg@mail.gmail.com>
<[email protected]> 於 2026年8月5日週三 下午9:18寫道:
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] The wrapper Device Tree file includes a `.dts` file directly instead of a `.dtsi` and drops the generic board compatible string.
> - [Low] The commit message ends with an incomplete sentence.
> --
>
> --- Patch [2]: [PATCH v4 02/12] ARM: dts: aspeed: anacapa: Add EVT1 devicetree and point wrapper to it ---
> commit 0f5bca30c4e664b3bfec688906a2d297fbe5f884
> Author: Colin Huang <[email protected]>
>
> ARM: dts: aspeed: anacapa: Add EVT1 devicetree and point wrapper to it
>
> This commit introduces an EVT1 development-phase devicetree for the Facebook
> Anacapa BMC. It updates the top-level wrapper DTS to include the EVT1
> hardware revision file.
>
> > A dedicated EVT1 DTS is added for revision-specific hardware while
> > keeping a single, Anacapa entrypoint used by the build and deployment
> > flow. The top-level aspeed-bmc-facebook-anacapa.dts
>
> [Severity: Low]
> This isn't a bug, but it looks like this sentence ends abruptly. Could the
> rest of the thought be included in the commit message?
>
If I need to send v5, I will provide more message.

> > diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-anacapa.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-anacapa.dts
> > index 1fddf3a5d1383..209eef65cedbc 100644
> > --- a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-anacapa.dts
> > +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-anacapa.dts
> > @@ -1,1079 +1,4 @@
> >  // SPDX-License-Identifier: GPL-2.0-or-later
> >
> > -/dts-v1/;
> > -#include "aspeed-g6.dtsi"
> > -#include <dt-bindings/gpio/aspeed-gpio.h>
> > -#include <dt-bindings/i2c/i2c.h>
> > +#include "aspeed-bmc-facebook-anacapa-evt1.dts"
> >
> > -/ {
> > -     model = "Facebook Anacapa BMC";
> > -     compatible = "facebook,anacapa-bmc", "aspeed,ast2600";
>
> [Severity: Medium]
> Does this change unintentionally break the device tree ABI for bootloaders or
> userspace tooling that might rely on the generic compatible string?

Thanks for the review.

The removal of the generic "facebook,anacapa-bmc" compatible is
intentional.

The EVT1, EVT2 and DVT boards are treated as distinct
development-phase hardware revisions and are expected to use their
own revision-specific compatible strings. We checked the kernel tree
and did not find any in-tree drivers matching on the generic
"facebook,anacapa-bmc" compatible.

We therefore prefer to keep the current compatible hierarchy and
schema definition rather than adding the generic compatible as a
fallback.

Regarding the DTS include structure, the use of DTS includes is
intentional and models the hardware revision inheritance used by the
Anacapa platform (EVT1 -> EVT2 -> DVT).



>
> By including a .dts file directly instead of a shared .dtsi and replacing the
> wrapper's contents entirely, the generic "facebook,anacapa-bmc" compatible
> string is dropped. The resulting device tree blob will now only present the
> "facebook,anacapa-bmc-evt1" string, which might cause downstream failures if
> environments expect the generic identifier. Device tree conventions also
> typically recommend keeping shared components in .dtsi files rather than
> including another top-level .dts.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260805-anacapa-devlop-phase-devicetree-v4-0-5d67e0efa6c5@gmail.com?part=2