Re: [PATCH v5] powerpc/dts: Add Device Tree for WatchGuard Firebox T30 (v5)

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] powerpc/dts: firebox-t30.dts placed in OpenWrt path and missing from Makefile
- [Low] powerpc/dts: undocumented watchguard vendor prefix and firebox-t30 compatible
--

commit f6c5f758b9931c759d889087f336848b7b3fbdff
Author: Lefteris Koutrakis <lefteris.koutrakis@gmail.com>

powerpc/dts: Add Device Tree for WatchGuard Firebox T30 (v5)

This commit adds a device tree source file for the WatchGuard Firebox T30
board based on the Freescale P1020 SoC.

>     powerpc/dts: Add Device Tree for WatchGuard Firebox T30 (v5)
>     
>     This commit adds initial device tree support for the WatchGuard Firebox T30
>     board and enables building firebox-t30.dtb under CONFIG_MPC85xx_RDB in
>     arch/powerpc/boot/dts/fsl/Makefile.
>     
>     Signed-off-by: Lefteris Koutrakis <lefteris.koutrakis@gmail.com>
> 
> diff --git a/target/linux/mpc85xx/files/arch/powerpc/boot/dts/firebox-t30.dts b/target/linux/mpc85xx/files/arch/powerpc/boot/dts/firebox-t30.dts
> new file mode 100644
> index 0000000000000..ec6cf911c66f4
> --- /dev/null
> +++ b/target/linux/mpc85xx/files/arch/powerpc/boot/dts/firebox-t30.dts
> @@ -0,0 +1,386 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
> +/*
> + * Device Tree for WatchGuard Firebox T30
> + * Author: Lefteris Koutrakis <lefteris.koutrakis@gmail.com>
> + */
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/leds/common.h>
> +
> +/include/ "p1020si-pre.dtsi"

[Severity: Medium]
Was target/linux/mpc85xx/files/arch/powerpc/boot/dts/firebox-t30.dts meant to
be placed at arch/powerpc/boot/dts/fsl/firebox-t30.dts instead of an OpenWrt
target directory?

The commit message states that building firebox-t30.dtb is enabled under
CONFIG_MPC85xx_RDB in arch/powerpc/boot/dts/fsl/Makefile, but that Makefile
is not modified by this patch:

arch/powerpc/boot/dts/fsl/Makefile:
    dtb-$(CONFIG_MPC85xx_RDB) += p1020mbg-pc_32b.dtb p1020mbg-pc_36b.dtb
    dtb-$(CONFIG_MPC85xx_RDB) += p1020rdb_36b.dtb p1020rdb.dtb p1020rdb-pc_32b.dtb
    dtb-$(CONFIG_MPC85xx_RDB) += p1020rdb-pc_36b.dtb p1020rdb-pc_camp_core0.dtb
    dtb-$(CONFIG_MPC85xx_RDB) += p1020rdb-pc_camp_core1.dtb p1020rdb-pd.dtb

As a result, Kbuild never builds firebox-t30.dtb when CONFIG_MPC85xx_RDB or
CONFIG_OF_ALL_DTBS is enabled, and the relative /include/ directives for
p1020si-pre.dtsi and p1020si-post.dtsi in firebox-t30.dts cannot resolve
outside arch/powerpc/boot/dts/fsl/.

> +
> +/ {
> +	model = "Watchguard Firebox T30";
> +	compatible = "watchguard,firebox-t30", "fsl,P1020RDB";

[Severity: Low]
Should the watchguard vendor prefix and watchguard,firebox-t30 compatible
string be documented in the devicetree bindings before adding them to the root
node in firebox-t30.dts?

Currently, patternProperties in
Documentation/devicetree/bindings/vendor-prefixes.yaml has no entry for
"^watchguard,.*":

Documentation/devicetree/bindings/vendor-prefixes.yaml:
      "^wand,.*":
        description: Wandbord (Technexion)
      "^waveshare,.*":
        description: Waveshare Electronics

Running checkpatch or dtbs_check will report undocumented DT string and schema
validation failures without the vendor prefix and board binding documented.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261028185407.26952-1-lefteris.koutrakis@gmail.com?part=1