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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] The VDD_ARM regulator (sw1a) max voltage constraint is dangerously set to 3.3V.
- [High] MCLK audio clock routing is missing required direction properties and incorrectly assigned to the wrong CCF clock node.
- [Low] Misplaced header inclusion for LED macros creates brittle dependencies.
- [Low] The commit message fails to list support for the WM8904 audio codec.
--

commit 0f005e2caf9fb13eb2178194911af6c97acfe6fc
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.
    
    Add the common SoM description together with support for:
    - eMMC storage
    - NAND storage
    - WM8731 audio codec
    - VAR-SOM-MX7 V2 hardware revision

Link: https://variscite.com/system-on-module-som/i-mx-7/var-som-mx7/

>     Add the common SoM description together with support for:
>     - eMMC storage
>     - NAND storage
>     - WM8731 audio codec
>     - VAR-SOM-MX7 V2 hardware revision

[Severity: Low]
Does this list in the commit message omit the WM8904 audio codec, which is
also introduced by this patch?

> 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..d798b021f8e05
> --- /dev/null
> +++ b/arch/arm/boot/dts/nxp/imx/imx7d-var-som-mx7customboard.dtsi

[ ... ]

> +	leds {
> +		compatible = "gpio-leds";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_gpio_leds>;
> +
> +		led-d10 {
> +			function = LED_FUNCTION_STATUS;
> +			color = <LED_COLOR_ID_GREEN>;

[Severity: Low]
Are the LED_FUNCTION_STATUS and LED_COLOR_ID_GREEN macros used here without
explicitly including the required <dt-bindings/leds/common.h> header? It
looks like the header was mistakenly included in imx7d-var-som.dtsi instead,
creating a brittle dependency based on implicit inclusions.

> 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..ab05de43cc53a
> --- /dev/null
> +++ b/arch/arm/boot/dts/nxp/imx/imx7d-var-som-wm8731.dtsi

[ ... ]

> +&i2c2 {
> +	wm8731: audio-codec@1a {
> +		compatible = "wlf,wm8731";
> +		reg = <0x1a>;
> +		#sound-dai-cells = <0>;
> +		AVDD-supply = <&vgen6_reg>;
> +		DBVDD-supply = <&vgen6_reg>;
> +		DCVDD-supply = <&vgen6_reg>;
> +		HPVDD-supply = <&vgen6_reg>;
> +		clocks = <&clks IMX7D_AUDIO_MCLK_ROOT_DIV>;

[Severity: High]
Does the codec driver incorrectly bind to IMX7D_AUDIO_MCLK_ROOT_DIV instead of
the physical SAI1 root clock driving the pin, causing the ASoC driver to read
an incorrect frequency?

> +		clock-names = "mclk";
> +		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>;
> +	};
> +};
> +
> +&sai1 {
> +	pinctrl-names = "default", "sleep";
> +	pinctrl-0 = <&pinctrl_sai1>, <&pinctrl_sai1_mclk>;
> +	pinctrl-1 = <&pinctrl_sai1_sleep>, <&pinctrl_sai1_mclk_sleep>;
> +	assigned-clocks = <&clks IMX7D_SAI1_ROOT_SRC>,
> +			  <&clks IMX7D_SAI1_ROOT_CLK>;
> +	assigned-clock-parents = <&clks IMX7D_PLL_AUDIO_POST_DIV>;
> +	assigned-clock-rates = <0>, <36864000>;
> +	status = "okay";
> +};

[Severity: High]
Is the sai1 node missing the fsl,sai-mclk-direction-output property, leaving
the codec's MCLK pin undriven?

> diff --git a/arch/arm/boot/dts/nxp/imx/imx7d-var-som-wm8904.dtsi b/arch/arm/boot/dts/nxp/imx/imx7d-var-som-wm8904.dtsi
> new file mode 100644
> index 0000000000000..b56f3680edcff
> --- /dev/null
> +++ b/arch/arm/boot/dts/nxp/imx/imx7d-var-som-wm8904.dtsi

[ ... ]

> +&i2c2 {
> +	wm8904: audio-codec@1a {
> +		compatible = "wlf,wm8904";
> +		reg = <0x1a>;
> +		#sound-dai-cells = <0>;
> +		clocks = <&clks IMX7D_AUDIO_MCLK_ROOT_DIV>;

[Severity: High]
Similar to the wm8731 codec, does this codec driver also incorrectly bind to
IMX7D_AUDIO_MCLK_ROOT_DIV instead of the physical SAI1 root clock?

> 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..255cf44900771
> --- /dev/null
> +++ b/arch/arm/boot/dts/nxp/imx/imx7d-var-som.dtsi
> @@ -0,0 +1,533 @@
> +// 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/
> + *
> + */
> +
> +#include <dt-bindings/leds/common.h>

[Severity: Low]
Is this header included but not used in this file? It appears the LED macros
are used in imx7d-var-som-mx7customboard.dtsi instead.

[ ... ]

> +&i2c1 {
> +	clock-frequency = <100000>;
> +	pinctrl-names = "default", "gpio";
> +	pinctrl-0 = <&pinctrl_i2c1>;
> +	pinctrl-1 = <&pinctrl_i2c1_gpio>;
> +	scl-gpios = <&gpio4 8 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> +	sda-gpios = <&gpio4 9 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> +	status = "okay";
> +
> +	pfuze300: pmic@8 {
> +		compatible = "fsl,pfuze3000";
> +		reg = <0x08>;
> +
> +		regulators {
> +			sw1a_reg: sw1a {
> +				regulator-min-microvolt = <700000>;
> +				regulator-max-microvolt = <3300000>;

[Severity: High]
Is it safe to set regulator-max-microvolt to 3.3V for the VDD_ARM regulator?
Since this could allow the PMIC to supply up to 3.3V to the core, does this
exceed the i.MX7Dual hardware absolute maximum tolerances (around 1.3V)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1788180142.git.stefano.r@variscite.com?part=2