Re: [PATCH v6 3/3] arm64: dts: imx95: Add support for Data Modul i.MX95 eDM SBC

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] dts: imx95: incorrect PCAL6524 interrupt polarity causes IRQ storm
- [High] dts: imx95: incorrect reset-gpios polarity holds Ethernet PHYs in reset
- [Medium] dts: imx95: missing power supply regulator for M.2 Key-E slot
- [Low] dts: imx95: unused pinctrl_panel_backlight_lvds group left in device tree
--

commit 7f31e6dad5b9c7665d8920ec3ddbd34978190a69
Author: Marek Vasut <marex@nabladev.com>

arm64: dts: imx95: Add support for Data Modul i.MX95 eDM SBC

Add support for Data Modul i.MX95 eDM SBC board. This is an
evaluation board for various custom display units. Currently
supported are serial console, ethernet, eMMC, SD, SPI NOR,
USB host and PCIe.

diff --git a/arch/arm64/boot/dts/freescale/imx95-data-modul-edm-sbc.dts b/arch/arm64/boot/dts/freescale/imx95-data-modul-edm-sbc.dts
new file mode 100644
index 0000000000000..5ca26c6ab665f
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx95-data-modul-edm-sbc.dts
@@ -0,0 +1,1079 @@
[ ... ]
> +	i2c2_gpio_expander_20: gpio@20 {	/* U12 */
> +		compatible = "nxp,pcal6524";
> +		reg = <0x20>;
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +		#interrupt-cells = <2>;
> +		interrupt-controller;
> +		interrupts-extended = <&gpio5 11 IRQ_TYPE_LEVEL_HIGH>;

[Severity: High]
Is the interrupt polarity configured correctly? Since open-drain lines
typically rest HIGH due to pull-ups and assert LOW, wouldn't setting the
polarity to IRQ_TYPE_LEVEL_HIGH cause the interrupt controller to see the
line as asserted immediately, leading to an interrupt storm?

This same configuration is also used in i2c2_gpio_expander_21 and
i2c2_gpio_expander_22.

[ ... ]
> +	ethphy0: ethernet-phy@1 {	/* KSZ9131RNXI U20 */
> +		compatible = "ethernet-phy-id0022.1640";
> +		reg = <1>;
> +		interrupts-extended = <&gpio1 10 IRQ_TYPE_LEVEL_LOW>;
> +		reset-assert-us = <1000>;
> +		/* RESET_N signal rise time ~100ms */
> +		reset-deassert-us = <120000>;
> +		reset-gpios = <&i2c2_gpio_expander_22 4 GPIO_ACTIVE_HIGH>;
> +	};
> +
> +	ethphy1: ethernet-phy@2 {	/* KSZ9131RNXI U22 */
> +		compatible = "ethernet-phy-id0022.1640";
> +		reg = <2>;
> +		interrupts-extended = <&gpio1 10 IRQ_TYPE_LEVEL_LOW>;
> +		reset-assert-us = <1000>;
> +		/* RESET_N signal rise time ~100ms */
> +		reset-deassert-us = <120000>;
> +		reset-gpios = <&i2c2_gpio_expander_22 5 GPIO_ACTIVE_HIGH>;
> +	};

[Severity: High]
Will setting GPIO_ACTIVE_HIGH hold the PHYs in reset? The core drives the line
LOW to deassert reset when active-high is used, but the physical RESET_N pin
is active-low, meaning a LOW signal actually holds the PHY in reset during
normal operation.

[ ... ]
> +&usdhc3 {	/* M.2 Key-E */
> +	pinctrl-names = "default", "state_100mhz", "state_200mhz", "sleep";
> +	pinctrl-0 = <&pinctrl_usdhc3>;
> +	pinctrl-1 = <&pinctrl_usdhc3_100mhz>;
> +	pinctrl-2 = <&pinctrl_usdhc3_200mhz>;
> +	pinctrl-3 = <&pinctrl_usdhc3>;
> +	bus-width = <4>;
> +	status = "okay";
> +};

[Severity: Medium]
Is a regulator missing for the M.2 Key-E slot?

The usdhc3 interface is enabled, but the DT lacks a regulator node for
VDD_3V3_M2-E_EN (pin 5 of i2c2_gpio_expander_20) and lacks a vmmc-supply
assignment. Will the slot remain physically unpowered because the enable
GPIO is never driven active by the kernel?

[ ... ]
> +	pinctrl_panel_backlight_lvds: backlightlvdsgrp {
> +		fsl,pins = <
> +			IMX95_PAD_GPIO_IO27__GPIO2_IO_BIT27			0x31e
> +		>;
> +	};

[Severity: Low]
Is this pinctrl group orphaned? The backlight supply nodes were removed in
V6 of the patch, but the associated pinctrl_panel_backlight_lvds group
appears to have been left in the device tree.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930214729.809649-1-marex@nabladev.com?part=3