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