Re: [PATCH v1 2/3] ARM: dts: imx7d: add Variscite VAR-SOM-MX7 support

On Fri, Aug 21, 2026 at 12:41:07PM +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
> - [High] The Ethernet PHY reset pin for `ethphy1` is muxed on the `fec2` node instead of `fec1`, causing PHY detection to fail during MDIO bus probe.
> - [Medium] Use of `IRQ_TYPE_NONE` (0) for touchscreen controller interrupts.
> - [Low] Duplicate `assigned-clock-rates` property in the `&sai1` node.
> - [Low] Attempting to delete a non-existent child node `bcrmf@1` instead of `wifi@1`.
> - [Low] Missing required `color` property in the GPIO LED definition.
> - [Low] Use of deprecated `reset-gpio` property instead of `reset-gpios` in the PCIe node.
> --
>
> --- Patch [2]: [PATCH v1 2/3] ARM: dts: imx7d: add Variscite VAR-SOM-MX7 support ---
> commit 910d1859fce3905900b424cb29918719c63275b0
> Author: Stefano Radaelli <[email protected]>
>
> ARM: dts: imx7d: add Variscite VAR-SOM-MX7 support
>
> This commit adds device tree support for the Variscite VAR-SOM-MX7 system on
> module. It provides the common SoM description along with configurations for
> eMMC, NAND, WM8731 audio codec, and the V2 hardware revision.
>
> Link: https://variscite.com/system-on-module-som/i-mx-7/var-som-mx7/
>
> > diff --git a/arch/arm/boot/dts/nxp/imx/imx7d-var-som-mx7customboard.dtsi b/arch/arm/boot/dts/nxp/imx/imx7d-var-som-mx7customboard.dtsi
> > index 0000000000000..82814535fb93e 100644
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/nxp/imx/imx7d-var-som-mx7customboard.dtsi
> > @@ -0,0 +1,378 @@
> [ ... ]
> > +	leds {
> > +		compatible = "gpio-leds";
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&pinctrl_gpio_leds>;
> > +
> > +		led-d10 {
> > +			function = LED_FUNCTION_STATUS;
> > +			gpios = <&gpio1 13 GPIO_ACTIVE_HIGH>;
> > +			linux,default-trigger = "heartbeat";
> > +		};
> > +	};
>
> [Severity: Low]
> Does this led-d10 node also need a color property? When function is
> specified, modern LED bindings generally require the color property to form
> sysfs directories in the format color:function.
>
> [ ... ]
> > +&i2c4 {
> > +	clock-frequency = <100000>;
> > +	pinctrl-names = "default", "gpio";
> > +	pinctrl-0 = <&pinctrl_i2c4>;
> > +	pinctrl-1 = <&pinctrl_i2c4_gpio>;
> > +	scl-gpios = <&gpio4 14 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> > +	sda-gpios = <&gpio4 15 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> > +	status = "okay";
> > +
> > +	ft5x06_ts: touchscreen@38 {
> > +		compatible = "edt,edt-ft5206";
> > +		reg = <0x38>;
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&pinctrl_touch>;
> > +		interrupt-parent = <&gpio2>;
> > +		interrupts = <30 0>;
>
> [Severity: Medium]
> Is it intentional to use 0 (IRQ_TYPE_NONE) for the touchscreen interrupt
> trigger type here? This can cause the interrupt to be unconfigured or
> incorrectly configured by default, which may lead to missed touch events or
> an interrupt storm.
>
> [ ... ]
> > +&pcie {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&pinctrl_pcie>;
> > +	reset-gpio = <&gpio2 31 GPIO_ACTIVE_LOW>;
>
> [Severity: Low]
> Could this use the standard reset-gpios property instead? The reset-gpio
> property is deprecated, and dtbs_check validation may emit warnings for it.

Yes, please add reset-gpios under pci port (&pcie0_port0) for new dts.

>
> > diff --git a/arch/arm/boot/dts/nxp/imx/imx7d-var-som-v2.dtsi b/arch/arm/boot/dts/nxp/imx/imx7d-var-som-v2.dtsi
> > index 0000000000000..724aa6fb36de2 100644
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/nxp/imx/imx7d-var-som-v2.dtsi
> > @@ -0,0 +1,64 @@
> [ ... ]
> > +/* WiFi */
> > +&usdhc2 {
> > +	mmc-pwrseq = <&wifi_pwrseq>;
> > +	/delete-node/ bcrmf@1;
> > +};
>
> [Severity: Low]
> Is there a typo in the node name here? The node defined in the base dtsi is
> wifi@1 (with the label brcmf). Trying to delete bcrmf@1 will cause the
> device tree compiler to fail since the node doesn't exist.

Now don't prefer delete-node.

>
> > diff --git a/arch/arm/boot/dts/nxp/imx/imx7d-var-som-wm8731.dtsi b/arch/arm/boot/dts/nxp/imx/imx7d-var-som-wm8731.dtsi
> > index 0000000000000..5fab164a64820 100644
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/nxp/imx/imx7d-var-som-wm8731.dtsi
> > @@ -0,0 +1,65 @@
> [ ... ]
> > +&sai1 {
> > +	assigned-clock-rates = <0>, <36864000>;
> > +	assigned-clocks = <&clks IMX7D_AUDIO_MCLK_ROOT_SRC>,
> > +			  <&clks IMX7D_AUDIO_MCLK_ROOT_DIV>;
> > +	assigned-clock-parents = <&clks IMX7D_PLL_AUDIO_POST_DIV>;
> > +	assigned-clock-rates = <0>, <12288000>;
>
> [Severity: Low]
> Does this node intend to duplicate the assigned-clock-rates property? The
> second definition (<0>, <12288000>) will silently overwrite the first one
> (<0>, <36864000>), which will cause DTC to emit a duplicate property warning
> and the intended 36.864MHz clock rate configuration to be lost.

Yes, please fix it.


>
> > +	/delete-property/ fsl,sai-mclk-direction-output;
> > +};
> > diff --git a/arch/arm/boot/dts/nxp/imx/imx7d-var-som.dtsi b/arch/arm/boot/dts/nxp/imx/imx7d-var-som.dtsi
> > index 0000000000000..cbd6252bd18f3 100644
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/nxp/imx/imx7d-var-som.dtsi
> > @@ -0,0 +1,633 @@
> [ ... ]
> > +&ecspi1 {
> > +	pinctrl-names = "default", "sleep";
> > +	pinctrl-0 = <&pinctrl_ecspi1>;
> > +	pinctrl-1 = <&pinctrl_ecspi1_sleep>;
> > +	cs-gpios = <&gpio4 19 GPIO_ACTIVE_LOW>;
> > +	status = "okay";
> > +
> > +	/* Resistive touch controller */
> > +	ads7846_ts: ads7846@0 {
> > +		reg = <0>;
> > +		compatible = "ti,ads7846";
> > +		pinctrl-names ="default";
> > +		pinctrl-0 = <&pinctrl_restouch>;
> > +		interrupt-parent = <&gpio1>;
> > +		interrupts = <9 0>;
>
> [Severity: Medium]
> Is it intentional to use 0 (IRQ_TYPE_NONE) for this touch controller interrupt
> trigger type? This might result in the interrupt being unconfigured by default.

why type is NONE?

Frank
>
> [ ... ]
> > +&fec1 {
> > +	pinctrl-names = "default","sleep";
> > +	pinctrl-0 = <&pinctrl_enet1>, <&pinctrl_enet1_rst>;
> [ ... ]
> > +	mdio {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		ethphy0: ethernet-phy@0 {
> > +			compatible = "ethernet-phy-ieee802.3-c22";
> > +			reg = <0>;
> > +			reset-gpios = <&gpio5 11 GPIO_ACTIVE_LOW>;
> > +			reset-assert-us = <20000>;
> > +			reset-deassert-us = <80000>;
> > +			vddio-supply = <&reg_phy_vddio>;
> > +		};
> > +
> > +		ethphy1: ethernet-phy@1 {
> > +			compatible = "ethernet-phy-ieee802.3-c22";
> > +			reg = <1>;
> > +			reset-gpios = <&gpio4 3 GPIO_ACTIVE_LOW>;
> > +			reset-assert-us = <20000>;
> > +			reset-deassert-us = <80000>;
> > +			vddio-supply = <&reg_phy_vddio>;
> > +		};
> > +	};
> > +};
> > +
> > +&fec2 {
> > +	pinctrl-names = "default","sleep";
> > +	pinctrl-0 = <&pinctrl_enet2>, <&pinctrl_enet2_rst>;
>
> [Severity: High]
> Does ethphy1 fail to reset during the MDIO bus probe on fec1?
>
> Since ethphy1 is a child of the fec1 MDIO bus, fec1 will attempt to toggle
> its reset GPIO (&gpio4 3) when it probes. However, the pinctrl for this GPIO
> (&pinctrl_enet2_rst) is attached to fec2 instead of fec1. Since fec2 has
> not probed yet, the reset pin may not be muxed properly, preventing the reset
> toggle from reaching the PHY and causing detection to fail.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2