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

[Frank Li <Frank.li@oss.nxp.com>]

On Mon, Aug 24, 2026 at 10:41:40AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
> - [High] Overriding `assigned-clocks` in `&sai1` drops essential `SAI1_ROOT_CLK` configuration, breaking audio.
> - [Low] Missing device tree binding headers in `imx7d-var-som-mx7customboard.dtsi` causing undefined macros.
> - [Low] The SoM `.dtsi` file incorrectly includes a `/dts-v1/;` directive, leading to duplicate directives in `.dts` files.
> - [Low] The `usdhc1` and `usdhc3` nodes specify `no-1-8-v` while defining unreachable 100MHz/200MHz pinctrl states.
> - [Low] Device Tree node names contain underscores, violating DT specifications.
> --
>
> --- Patch [2]: [PATCH v2 2/3] ARM: dts: imx7d: add Variscite VAR-SOM-MX7 support ---
> commit f0d7c3577a1db2d9fa7ddee6134d561982409054
> Author: Stefano Radaelli <stefano.r@variscite.com>
>
> ARM: dts: imx7d: add Variscite VAR-SOM-MX7 support
>
> Add device tree support for the Variscite VAR-SOM-MX7 system on module.
> The module is based on the NXP i.MX7Dual SoC and is available in
> multiple configurations and hardware revisions.
>
> 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-emmc.dtsi b/arch/arm/boot/dts/nxp/imx/imx7d-var-som-emmc.dtsi
> > new file mode 100644
> > index 0000000000000..2f8185f8c526e
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/nxp/imx/imx7d-var-som-emmc.dtsi
> > @@ -0,0 +1,99 @@
> [ ... ]
> > +&usdhc3 {
> > +	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_sleep>;
> > +	vmmc-supply = <&vgen3_reg>;
> > +	assigned-clocks = <&clks IMX7D_USDHC3_ROOT_CLK>;
> > +	assigned-clock-rates = <400000000>;
> > +	no-1-8-v;
>
> [Severity: Low]
> Does this no-1-8-v property conflict with the 100MHz and 200MHz pinctrl states
> defined above it?
>
> By forcing the MMC core to stay at 3.3V, UHS-I and high-speed modes that
> require 1.8V might be disabled, making those pinctrl states unreachable dead
> code.

Make sense, please check it

>
> > 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
> > new file mode 100644
> > index 0000000000000..ecd4601c12bfb
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/nxp/imx/imx7d-var-som-mx7customboard.dtsi
> > @@ -0,0 +1,382 @@
> [ ... ]
> > +	gpio-keys {
> > +		compatible = "gpio-keys";
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&pinctrl_gpio_keys>;
> > +
> > +		key-back {
> > +			label = "Back";
> > +			gpios = <&gpio1 11 GPIO_ACTIVE_LOW>;
> > +			linux,code = <KEY_BACK>;
>
> [Severity: Low]
> Are the KEY_BACK, KEY_MENU, and KEY_HOME macros undefined here?
>
> It looks like the dt-bindings/input/input.h header is missing from the
> includes, which could cause a dtc compilation failure.
>
> [ ... ]
> > +	reg_hsic_hub_connect: regulator_hsic_hub_connect {
> > +		compatible = "regulator-fixed";
>
> [Severity: Low]
> Could this node name and regulator_hsic_hub_pwr_on below use hyphens instead
> of underscores?

Yes, node name should use - instead of _

>
> Using underscores in node names violates the device tree specification and
> triggers node_name_chars_strict compiler warnings.
>
> [ ... ]
> > +&usdhc1 {
> > +	pinctrl-names = "default", "state_100mhz", "state_200mhz";
> > +	pinctrl-0 = <&pinctrl_usdhc1>, <&pinctrl_usdhc1_gpio>;
> > +	pinctrl-1 = <&pinctrl_usdhc1_100mhz>, <&pinctrl_usdhc1_gpio>;
> > +	pinctrl-2 = <&pinctrl_usdhc1_200mhz>, <&pinctrl_usdhc1_gpio>;
> > +	cd-gpios = <&gpio5 0 GPIO_ACTIVE_LOW>;
> > +	no-1-8-v;
>
> [Severity: Low]
> Does this no-1-8-v property similarly prevent the high-speed pinctrl states
> from being used on this interface?
>
> > 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
> > new file mode 100644
> > index 0000000000000..af5dd6d459ad2
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/nxp/imx/imx7d-var-som-wm8731.dtsi
> > @@ -0,0 +1,64 @@
> [ ... ]
> > +&sai1 {
> > +	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>, <36864000>;
> > +	/delete-property/ fsl,sai-mclk-direction-output;
> > +};
>
> [Severity: High]
> Does overriding the assigned-clocks here silently drop the required SAI1 root
> clock configuration defined in the base dtsi?

Please check this also

Frank
>
> In imx7d-var-som.dtsi, the assigned-clocks property sets IMX7D_SAI1_ROOT_SRC
> and IMX7D_SAI1_ROOT_CLK. By overriding it without retaining the original
> assignments, the SAI1 block might lose its required clock configuration, and
> deleting fsl,sai-mclk-direction-output forces the pad to become an input,
> which could break the audio interface entirely.
>
> > diff --git a/arch/arm/boot/dts/nxp/imx/imx7d-var-som.dtsi b/arch/arm/boot/dts/nxp/imx/imx7d-var-som.dtsi
> > new file mode 100644
> > index 0000000000000..486e9360dea52
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/nxp/imx/imx7d-var-som.dtsi
> > @@ -0,0 +1,637 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +/*
> > + * Common dtsi for Variscite VAR-SOM-MX7
> > + *
> > + * Link: https://variscite.com/system-on-module-som/i-mx-7/var-som-mx7/
> > + *
> > + * Copyright (C) 2026 Variscite Ltd. - https://www.variscite.com/
> > + *
> > + */
> > +
> > +/dts-v1/;
>
> [Severity: Low]
> Will this /dts-v1/; directive cause a duplicate version directive syntax error?
>
> When this dtsi file is included by carrier board dts files that also declare
> /dts-v1/;, the expanded source will contain duplicate version directives.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/cover.1787566980.git.stefano.r@variscite.com?part=2
>