Re: [PATCH v5 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] dt: imx95-data-modul: PCAL6524 expander interrupts cause IRQ storm due to active-high polarity
- [High] dt: imx95-data-modul: KSZ9131 Ethernet PHYs permanently held in reset due to active-high polarity
- [Medium] dt: imx95-data-modul: reg_panel_vcc defaults to active-low, breaking panel power
- [Medium] dt: imx95-data-modul: M.2 Key-E slot unpowered due to missing regulator for VDD_3V3_M2-E_EN
--

commit 88cbe063476bce8eddcc6ed5fd22c93a72c1ced5
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.

> 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..0181262eb8a98
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx95-data-modul-edm-sbc.dts

[ ... ]

> +	reg_panel_vcc: regulator-panel-vcc {
> +		compatible = "regulator-fixed";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		regulator-name = "PANEL_VCC";
> +		gpios = <&i2c2_gpio_expander_20 17 0>;
> +		/* Disabled by default, unless display board plugged in. */
> +		status = "disabled";
> +	};

[Severity: Medium]
Does this regulator invert the panel power logic?

Because this fixed regulator omits the enable-active-high boolean
property, the gpiolib-of quirk implementation specifically ignores the phandle
polarity flag (0 / GPIO_ACTIVE_HIGH) and forces the polarity to active-low.
This results in inverted enable logic for the display panel when it should be
on or off.

[ ... ]

> +	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]
Does this interrupt configuration cause an immediate IRQ storm?

The NXP PCAL6524 INT output is an open-drain, active-low signal, and
the expanders share the same gpio5 11 line. By configuring the interrupt
with IRQ_TYPE_LEVEL_HIGH, the GPIO interrupt controller continuously
perceives the natural high idle state (maintained by a pull-up) as an
active interrupt, rendering all the GPIO expanders inoperable.

> +		vcc-supply = <&reg_3p3v>;
> +		/*
> +		 * reset-gpios cannot be used because the reset
> +		 * line is shared by multiple PCAL6524 consumers.
> +		 */
> +		gpio-line-names =
> +			/* Port 0 */
> +			"UART8_RS232-485_MODE0", "UART8_RS232-485_MODE1",
> +			"UART8_RS232-485_Term_TX", "UART8_RS232-485_Term_RX",
> +			"UART8_RS232-485_EN", "VDD_3V3_M2-E_EN",

[Severity: Medium]
Will the M.2 Key-E slot remain unpowered?

The board features a dedicated power enable pin for the Key-E slot
("VDD_3V3_M2-E_EN" here on the PCAL6524 expander). However, unlike the Key-B
and Key-M PCIe slots, there is no fixed regulator defined for this pin, and
the usdhc3 node has no vmmc-supply. Since the expander powers up in input
mode, the enable pin is never actively driven high, leaving the slot unpowered
and any SDIO device failing to initialize.

[ ... ]

> +	i2c2_gpio_expander_21: gpio@21 {	/* U14 */
> +		compatible = "nxp,pcal6524";
> +		reg = <0x21>;
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +		#interrupt-cells = <2>;
> +		interrupt-controller;
> +		interrupts-extended = <&gpio5 11 IRQ_TYPE_LEVEL_HIGH>;

[Severity: High]
This is a second instance of the active-high interrupt polarity issue on the
PCAL6524 INT pin, which will also contribute to an IRQ storm on gpio5 11.

[ ... ]

> +	i2c2_gpio_expander_22: gpio@22 {	/* U13 */
> +		compatible = "nxp,pcal6524";
> +		reg = <0x22>;
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +		#interrupt-cells = <2>;
> +		interrupt-controller;
> +		interrupts-extended = <&gpio5 11 IRQ_TYPE_LEVEL_HIGH>;

[Severity: High]
This is a third instance of the active-high interrupt polarity issue on the
PCAL6524 INT pin, continuing to cause an IRQ storm on the shared line.

[ ... ]

> +	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>;
> +	};

[Severity: High]
Are the KSZ9131 Ethernet PHYs permanently held in reset?

The KSZ9131 PHY requires an active-low reset signal (RESET_N). By
specifying GPIO_ACTIVE_HIGH here for reset-gpios, the PHY framework will
drive the line high to assert reset, and low during normal operation, exactly
opposite of what the hardware expects, permanently holding the PHY in reset.

> +
> +	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]
This is the second instance of the reset-gpios polarity issue on the ethernet
PHY, also holding it permanently in reset.

[ ... ]

> +&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]
As mentioned earlier regarding the missing regulator for VDD_3V3_M2-E_EN,
usdhc3 misses a vmmc-supply to control the slot's power, leaving the
M.2 Key-E slot unpowered.

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