Re: [PATCH v2 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: Incorrect interrupt polarity for PCAL6524 GPIO expanders causes interrupt storm
- [High] dts: Incorrect reset GPIO polarity holds KSZ9131 Ethernet PHYs in reset
- [Low] dts: Use of deprecated singular `reset-gpio` and `gpio` properties
- [Low] dts: Typo in the 5.0V fixed regulator node name (`regulator-0p0v`)
--

commit 88c0d25818385c2749a5ff45278aa5930847be84
Author: Marek Vasut <marex@nabladev.com>

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

This commit adds support for the Data Modul i.MX95 eDM SBC evaluation
board, including its serial console, ethernet, eMMC, SD, SPI NOR, USB
host, and PCIe components.

> 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..391bf6e28e9c6
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx95-data-modul-edm-sbc.dts
> @@ -0,0 +1,1111 @@

[ ... ]

> +	reg_5p0v: regulator-0p0v {
> +		compatible = "regulator-fixed";
> +		regulator-max-microvolt = <5000000>;
> +		regulator-min-microvolt = <5000000>;
> +		regulator-name = "VDD_5V0";
> +	};

[Severity: Low]
Is the node name regulator-0p0v a typo? It contradicts the defined output of
5,000,000 microvolts and could cause confusion or schema warnings.

[ ... ]

> +	reg_pcie0: regulator-pcie0 {
> +		compatible = "regulator-fixed";
> +		regulator-max-microvolt = <3300000>;
> +		regulator-min-microvolt = <3300000>;
> +		regulator-name = "PCIE_M2-B_EN";
> +		gpio = <&i2c2_gpio_expander_20 6 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +	};

[Severity: Low]
Does this node need to use the gpios property instead of the deprecated
singular gpio property?

[ ... ]

> +	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 configuration cause an interrupt storm? The PCAL6524 INT output is
an open-drain, active-low signal. Configuring it as IRQ_TYPE_LEVEL_HIGH could
mean the kernel immediately and continuously perceives the idle state as an
asserted interrupt.

[ ... ]

> +	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 seems to have the same interrupt polarity issue as the first expander,
where the active-low signal is configured as active-high.

[ ... ]

> +	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 seems to have the same interrupt polarity issue as the other two
expanders, where the active-low signal is configured as active-high.

[ ... ]

> +&netc_emdio {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_emdio>;
> +	status = "okay";
> +
> +	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]
Will this hold the ethernet PHY in reset? The KSZ9131 has an active-low
hardware reset pin (RESET_N). By specifying GPIO_ACTIVE_HIGH, the logical
polarity is inverted, causing the GPIO subsystem to drive the pin low when
it attempts to deassert the 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 seems to have the same issue as ethphy0, incorrectly specifying
active-high reset polarity for the active-low RESET_N pin.

[ ... ]

> +&pcie0 {	/* M.2 Key-B */
> +	pinctrl-0 = <&pinctrl_pcie0>;
> +	pinctrl-names = "default";
> +	reset-gpio = <&gpio5 13 GPIO_ACTIVE_LOW>;

[Severity: Low]
Does this node need to use the reset-gpios property instead of the deprecated
singular reset-gpio property?

> +	vpcie-supply = <&reg_pcie0>;
> +	status = "okay";
> +};
> +
> +&pcie1 {	/* M.2 Key-M */
> +	pinctrl-0 = <&pinctrl_pcie1>;
> +	pinctrl-names = "default";
> +	reset-gpio = <&gpio5 16 GPIO_ACTIVE_LOW>;

[Severity: Low]
Does this node need to use the reset-gpios property instead of the deprecated
singular reset-gpio property?

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