Re: [PATCH v6 3/3] arm64: dts: freescale: add Nodebox v3 CPU Module
From: sashiko-bot@kernel.org
Date: 2026-09-13 12:45:49
Also in:
imx
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] Ethernet PHY nodes lack 'interrupts' and 'reset-gpios' properties despite physical lines being explicitly defined on the board.
- [Medium] Regulator polarity mismatch for `ethsw-mdio-en`.
- [Low] Duplicate GPIO line name `usb01-oc#` in `gpio3` controller.
- [Low] Missing Devicetree binding documentation for the new board compatible.
--
commit 8433b3c3d1e5258b1e8b5fc1b405c85d777065d4
Author: Vincent Jardin [off-list ref]
arm64: dts: freescale: add Nodebox v3 CPU Module
The Nodebox v3 is a Free Mobile CPU module built around the NXP
LX2160A (16x Cortex-A72, DPAA2).
quoted hunk ↗ jump to hunk
diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a-nbxv3.dts b/arch/arm64/boot/dts/freescale/fsl-lx2160a-nbxv3.dts new file mode 100644 index 0000000000000..cc02204e9d1b4 --- /dev/null +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a-nbxv3.dts
[ ... ]
+/ {
+ compatible = "freemobile,nbxv3", "fsl,lx2160a";[Severity: Low] Are the devicetree bindings for this new board compatible string documented? New compatible strings must be documented to pass make dt_binding_check validation, but this patch does not appear to include the required updates to the Devicetree bindings documentation.
+ model = "Free Mobile Nodebox v3 CPU Module (LX2160A)"; +}; +
[ ... ]
quoted hunk ↗ jump to hunk
diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a-nbxv3.dtsi b/arch/arm64/boot/dts/freescale/fsl-lx2160a-nbxv3.dtsi new file mode 100644 index 0000000000000..ca4fb5329c0b2 --- /dev/null +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a-nbxv3.dtsi
[ ... ]
+ gpio-restart {
+ compatible = "gpio-restart";
+ gpios = <&gpio1 8 GPIO_ACTIVE_LOW>;
+ priority = <128>;
+ };
+
+ reg_ethsw_mdio: regulator-ethsw-mdio {
+ compatible = "regulator-fixed";
+ gpio = <&gpio3 25 GPIO_ACTIVE_LOW>;
+ regulator-always-on;
+ regulator-boot-on;
+ regulator-name = "ethsw-mdio-en";[Severity: Medium] Is ethsw-mdio-en truly an active-low signal? The reg_ethsw_mdio regulator specifies GPIO_ACTIVE_LOW and regulator-always-on. The kernel gpiolib will apply a logical-to-physical inversion, driving the physical pin to 0V. However, the board's strict naming convention uses a '#' or '-b' suffix for active-low signals (e.g., ethsw-reset#, usb01-oc#). The absence of this suffix on ethsw-mdio-en strongly implies it is an active-high signal. If it is active-high, driving it low will erroneously disable the MDIO bus to the Ethernet switch.
+ }; +}; +
[ ... ]
+&emdio1 {
+ status = "okay";
+
+ /* Microchip LAN8023 dual 10G PCS/retimer, Clause 45 only, one address per slice */
+ lan8023_phy_s0: ethernet-phy@2 {
+ compatible = "ethernet-phy-ieee802.3-c45";
+ reg = <2>;
+ };[Severity: Medium] Should this node include the interrupts and reset-gpios mappings? The lan8023_phy_s0 node omits the interrupts (lan8023-mdint) and reset-gpios (lan8023-rst) mappings, even though these physical lines are explicitly defined on the board. Without the interrupts property, the kernel's PHY state machine is forced to use inefficient polling mode instead of hardware interrupts, increasing CPU overhead and latency. Missing the reset-gpios property prevents the kernel from managing the PHY reset lifecycle. If the I2C expander (u800) initializes to a state that holds the PHYs in reset, the PHY probe will fail completely.
+
+ lan8023_phy_s1: ethernet-phy@3 {
+ compatible = "ethernet-phy-ieee802.3-c45";
+ reg = <3>;
+ };
+
+ rtl8211fd: ethernet-phy@6 {
+ compatible = "ethernet-phy-id001c.c916";
+ reg = <6>;
+ };[Severity: Medium] Similar to the lan8023_phy_s0 node, should this node include the interrupts mapping? The rtl8211fd node omits the interrupts (u1300-rtl8211fd-int#) mapping. Missing this also forces the kernel to use polling mode for the PHY state machine.
+};
[ ... ]
+&gpio3 {
+ gpio-line-names =
+ "altfn:ec1-txd3", "altfn:ec1-txd2",
+ "altfn:ec1-txd1", "altfn:ec1-txd0",
+ "altfn:ec1-tx-en", "altfn:ec1-gtx-clk",
+ "altfn:ec1-rxd3", "altfn:ec1-rxd2",
+ "altfn:ec1-rxd1", "altfn:ec1-rxd0",
+ "altfn:ec1-rx-clk", "altfn:ec1-rx-dv",
+ "", "",
+ "ptp-refclk-in",
+ "", "", "", "", "", "", "", "",
+ "u1700-zl30733-pps-out",
+ "",
+ "ethsw-mdio-en",
+ "usb01-oc#", /* USB1_PWRFAULT and USB2_PWRFAULT share one net */
+ "",
+ "usb01-oc#",[Severity: Low] Are these duplicate line names intentional? Device tree GPIO line names must be unique within a controller. Defining usb01-oc# twice causes ambiguity; any in-kernel consumer or userspace tool (via libgpiod) attempting to lookup the GPIO by name will only ever match the first occurrence. This violates DT schema rules and makes the second pin inaccessible by name.
+ "altfn:spi3-sck", + "", ""; + + status = "okay"; +};
-- Sashiko AI review · https://sashiko.dev/#/patchset/20260913-for-upstream-nbxv3-dts-v6-0-058566b93ba1@free.fr?part=3