Re: [PATCH v2 3/3] arm64: dts: freescale: add Nodebox v3 CPU Module

Hi M. Bot,

I'll submit a v3 update tomorrow.

See below for the few other topics,

quoted

+&fspi {
+	pinctrl-names = "default";
+	pinctrl-0 = <&fspi_dqs_sck_cs10_pins>, <&fspi_data30_pins>;
+};

[Severity: High]
Does this pinmux configuration conflict with the thermtrip-hog defined on gpio1
pin 23?

During boot, the pin controller configures the RCWPMUXCR0 register (via
fspi_dqs_sck_cs10_pins) to route pins 20-23 to the FSPI (XSPI1) controller en
masse. Concurrently, the GPIO driver applies the thermtrip-hog to pin 23 in
fsl-lx2160a-nbxv3.dtsi, attempting to drive it low as a GPIO.

Correct, it is something I should have cleaned sooner. I'll be removed into the
next update.

quoted

+&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]
Will this force the kernel into an inefficient polling mode for the Ethernet
PHYs?

The hardware possesses dedicated interrupt lines for the PHYs, as the gpio2
node explicitly defines the line names 'lan8023-mdint' and
'u1300-rtl8211fd-int#'. Without interrupts and interrupt-parent properties in
the PHY nodes (lan8023_phy_s0, lan8023_phy_s1, and rtl8211fd), the PHY driver
will fall back to periodically polling the MDIO bus to check for link state
changes.

The lines do exist and gpio2 is an interrupt-controller, so this can be
added. I'd prefer to keep it for a future update.

[Severity: Medium]
Should this node also include a reset-gpios property to ensure the PHY is
properly initialized?

The u800 I2C GPIO expander explicitly defines line 15 as 'lan8023-rst'.
Without claiming this GPIO (e.g., reset-gpios = <&u800 15 GPIO_ACTIVE_LOW>),
the Ethernet driver will not execute a hardware reset sequence during probing,
leaving the hardware dependent on the state the bootloader left it in and
potentially failing to recover from a suspended state.

It is a single reset shared by both slices of the one LAN8023, so it cannot be
claimed by both.
The reset is owned by the bring-up userland application of the lan8023.

quoted

+&fspi {
+	status = "okay";
+
+	/* GigaDevice GD55LB02GF, 256 MiB boot NOR */
+	flash@0 {
+		compatible = "jedec,spi-nor";
+		reg = <0>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		spi-max-frequency = <150000000>;
+		spi-rx-bus-width = <1>;
+		spi-tx-bus-width = <1>;
+	};
+};

[Severity: Medium]
Does this artificially restrict the FSPI flash to a 1-bit bus width despite the
hardware being wired for 4-bit Quad SPI?

The pinctrl-0 property in the .dts file includes <&fspi_data30_pins>, which
configures four data lines (DATA[3:0]) for the FSPI controller. However, this
flash@0 node sets the spi-rx-bus-width and spi-tx-bus-width to 1. This mismatch
forces the SPI NOR flash driver to operate in single-bit mode rather than
quad mode, needlessly degrading I/O performance.

Deliberately on purpose for this board ! for the time being.

Thanks,
  Vincent
 

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907-for-upstream-nbxv3-dts-v2-0-6b4afd36180b@free.fr?part=3