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

[sashiko-bot@kernel.org]

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.

> 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?

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?

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