Re: [PATCH v3 2/3] arm64: dts: freescale: Add support for Variscite VAR-SOM-MX8 QuadMax

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

On Wed, Jul 15, 2026 at 11:21:10AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
> - [High] Audio master clocks (MCLK) are left unconfigured, and audio baud rates are erroneously applied to peripheral bus clocks (IPG) due to incorrect LPCG clock indices.
> - [High] Conflicting assigned clock rates for shared audio PLL MST_BUS.
> - [High] Codec MCLK pinmux connects to ESAI0 high-frequency clock instead of the configured LPCG clock.
> - [High] The Continuous Memory Allocator (CMA) pool allocation range is strictly out-of-bounds relative to the default declared system memory, breaking CMA initialization on 1GB SOM variants.
> - [High] The PMIC thermal mitigation map ignores the high-power Cortex-A72 cores, defeating effective thermal throttling.
> - [Medium] WiFi and Bluetooth enable/power sequencing is explicitly deferred to user space instead of being structurally modeled in the device tree.
> --
>
> --- Patch [2]: [PATCH v3 2/3] arm64: dts: freescale: Add support for Variscite VAR-SOM-MX8 QuadMax ---
> commit e14b5ef256af728dc56444b803ea5575d3814182
> Author: Stefano Radaelli <stefano.r@variscite.com>
> Date:   Wed Jul 15 11:08:11 2026 +0000
>
>     arm64: dts: freescale: Add support for Variscite VAR-SOM-MX8 QuadMax
>
>     Add device tree support for the Variscite VAR-SOM-MX8 QuadMax
>     system on module. This SOM is designed to be used with various carrier
>     boards.
>
>     Link: https://variscite.com/system-on-module-som/i-mx-8/i-mx-8-quadmax-quadplus/var-som-mx8/
>
> > diff --git a/arch/arm64/boot/dts/freescale/imx8qm-var-som.dtsi b/arch/arm64/boot/dts/freescale/imx8qm-var-som.dtsi
> > new file mode 100644
> > index 0000000000000..08b02173b780c
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/freescale/imx8qm-var-som.dtsi
> > @@ -0,0 +1,530 @@
> [ ... ]
> > +	memory@80000000 {
> > +		device_type = "memory";
> > +		reg = <0x00000000 0x80000000 0 0x40000000>;
> > +	};
> [ ... ]
> > +	reserved-memory {
> > +		#address-cells = <2>;
> > +		#size-cells = <2>;
> > +		ranges;
> [ ... ]
> > +		/* global autoconfigured region for contiguous allocations */
> > +		linux,cma {
> > +			compatible = "shared-dma-pool";
> > +			alloc-ranges = <0 0xc0000000 0 0x3c000000>;
>
> [Severity: High]
> Does this alloc-ranges declaration break the Continuous Memory Allocator (CMA)
> initialization on 1GB SOM variants?

Do your board have 1GB variant?

>
> The declared 1GB system memory ends at 0xc0000000, while this alloc-ranges
> property forces CMA to find memory strictly starting at 0xc0000000, which
> would sit completely outside physical RAM on a 1GB board.
>
> > +			size = <0 0x3c000000>;
> > +			linux,cma-default;
> > +			reusable;
> > +		};
> > +	};
> [ ... ]
> > +&esai0 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&pinctrl_esai0>;
> > +	#sound-dai-cells = <0>;
> > +	assigned-clocks = <&acm IMX_ADMA_ACM_ESAI0_MCLK_SEL>,
> > +			<&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_PLL>,
> > +			<&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_SLV_BUS>,
> > +			<&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_MST_BUS>,
> > +			<&esai0_lpcg 0>;
> > +	assigned-clock-parents = <&aud_pll_div0_lpcg 0>;
> > +	assigned-clock-rates = <0>, <786432000>, <49152000>, <24576000>, <49152000>;
>
> [Severity: High]
> Does this configuration incorrectly alter the IPG clock instead of the MCLK?
>
> The assigned-clocks list uses <&esai0_lpcg 0>, which is IMX_LPCG_CLK_0
> (the peripheral bus IPG clock). The master clock (MCLK) would normally be
> at index 16 (IMX_LPCG_CLK_4).

Yes, 0 is wrong.

>
> Additionally, is it safe to assign 24.576 MHz to IMX_SC_PM_CLK_MST_BUS here?
> Later in the file, the wm8904 codec driver assigns 12.288 MHz to this exact
> same shared audio PLL MST_BUS clock. Whichever device probes last will
> overwrite the other's frequency, likely causing audio distortion or failure.
>
> > +	status = "okay";
> > +};
> [ ... ]
> > +&i2c0 {
> > +	clock-frequency = <100000>;
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&pinctrl_i2c0>;
> > +	status = "okay";
> > +
> > +	wm8904: codec@1a {
> > +		compatible = "wlf,wm8904";
> > +		reg = <0x1a>;
> > +		#sound-dai-cells = <0>;
> > +		clocks = <&mclkout0_lpcg 0>;
> > +		clock-names = "mclk";
> > +		assigned-clocks = <&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_PLL>,
> > +				<&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_SLV_BUS>,
> > +				<&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_MST_BUS>,
> > +				<&mclkout0_lpcg 0>;
> > +		assigned-clock-rates = <786432000>, <49152000>, <12288000>, <12288000>;
>
> [Severity: High]
> This assigns 12.288 MHz to IMX_SC_PM_CLK_MST_BUS, which conflicts with the
> 24.576 MHz assigned in &esai0.
>
> Also, the codec driver expects to control its clock via <&mclkout0_lpcg 0>.
> However, looking down at pinctrl_esai0, the physical pin is muxed to
> AUD_ESAI0_TX_HF_CLK (ESAI0's internal clock) instead of AUD_ACM_MCLK_OUT0.
> Will this leave the codec without the expected master clock source?
>
> > +		AVDD-supply = <&reg_audio_supply>;
> [ ... ]
> > +/* Bluetooth */
> > +&lpuart1 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&pinctrl_lpuart1>;
> > +	uart-has-rtscts;
> > +	status = "okay";
> > +};
>
> [Severity: Medium]
> Since this UART is used for Bluetooth, does it need a 'bluetooth' child node
> to properly describe power and enable dependencies?
>
> Without it, kernel-level power management (like suspend/resume and rfkill)
> will not be able to power down the module.
>
> [ ... ]
> > +&sai6 {
> > +	assigned-clocks = <&acm IMX_ADMA_ACM_SAI6_MCLK_SEL>,
> > +			<&clk IMX_SC_R_AUDIO_PLL_1 IMX_SC_PM_CLK_PLL>,
> > +			<&clk IMX_SC_R_AUDIO_PLL_1 IMX_SC_PM_CLK_SLV_BUS>,
> > +			<&clk IMX_SC_R_AUDIO_PLL_1 IMX_SC_PM_CLK_MST_BUS>,
> > +			<&sai6_lpcg 0>;
> > +	assigned-clock-parents = <&aud_pll_div1_lpcg 0>;
> > +	assigned-clock-rates = <0>, <786432000>, <98304000>, <24576000>, <98304000>;
>
> [Severity: High]
> Similar to the esai0 node, this assigns the audio baud rate to clock index 0
> (IPG clock) instead of index 16 (MCLK). Could this cause bus instability?
>
> > +	fsl,sai-asynchronous;
> > +	status = "okay";
> > +};
> > +
> > +&sai7 {
> > +	assigned-clocks = <&acm IMX_ADMA_ACM_SAI7_MCLK_SEL>,
> > +			<&clk IMX_SC_R_AUDIO_PLL_1 IMX_SC_PM_CLK_PLL>,
> > +			<&clk IMX_SC_R_AUDIO_PLL_1 IMX_SC_PM_CLK_SLV_BUS>,
> > +			<&clk IMX_SC_R_AUDIO_PLL_1 IMX_SC_PM_CLK_MST_BUS>,
> > +			<&sai7_lpcg 0>;
> > +	assigned-clock-parents = <&aud_pll_div1_lpcg 0>;
> > +	assigned-clock-rates = <0>, <786432000>, <98304000>, <24576000>, <98304000>;
>
> [Severity: High]
> As with esai0 and sai6, this applies the 98.304 MHz rate to <&sai7_lpcg 0>
> (IPG clock) instead of index 16.
>
> > +	fsl,sai-asynchronous;
> > +	status = "okay";
> > +};
> > +
> > +&thermal_zones {
> > +	pmic-thermal {
> > +		polling-delay-passive = <250>;
> > +		polling-delay = <2000>;
> > +		thermal-sensors = <&tsens IMX_SC_R_PMIC_0>;
> > +		trips {
> > +			pmic_alert0: trip0 {
> > +				temperature = <110000>;
> > +				hysteresis = <2000>;
> > +				type = "passive";
> > +			};
> > +			pmic_crit0: trip1 {
> > +				temperature = <125000>;
> > +				hysteresis = <2000>;
> > +				type = "critical";
> > +			};
> > +		};
> > +		cooling-maps {
> > +			map0 {
> > +				trip = <&pmic_alert0>;
> > +				cooling-device = <&A53_0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > +						 <&A53_1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > +						 <&A53_2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > +						 <&A53_3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
>
> [Severity: High]
> Should the high-power Cortex-A72 cores (A72_0 and A72_1) be included in this
> cooling map?
>
> If the A72 cores are omitted, they will remain unthrottled during a PMIC
> thermal event. Throttling only the low-power A53 cores might fail to reduce
> current draw sufficiently to prevent a hardware thermal shutdown.
>
> > +			};
> > +		};
> > +	};
> > +};
> [ ... ]
> > +/* WIFI */
> > +&usdhc3 {
> > +	#address-cells = <1>;
> > +	#size-cells = <0>;
> > +	pinctrl-names = "default", "state_100mhz", "state_200mhz";
> > +	pinctrl-0 = <&pinctrl_usdhc3>, <&pinctrl_wifi>;
> > +	pinctrl-1 = <&pinctrl_usdhc3_100mhz>, <&pinctrl_wifi>;
> > +	pinctrl-2 = <&pinctrl_usdhc3_200mhz>, <&pinctrl_wifi>;
> > +	bus-width = <4>;
> > +	non-removable;
> > +	status = "okay";
> > +
> > +	brcmf: wifi@1 {
> > +		compatible = "brcm,bcm4329-fmac";
> > +		reg = <1>;
> > +	};
> > +};
>
> [Severity: Medium]
> Is it intended to leave this Wi-Fi SDIO bus without a vmmc-supply or
> mmc-pwrseq regulator?
>
> By leaving this out and explicitly reserving the enable GPIOs for user-space
> control (as seen below in pinctrl_wifi), kernel-level power management is
> bypassed. This will break suspend/resume and rfkill functionality because the
> MMC/SDIO core cannot power down or up the module itself.
>
> [ ... ]
> > +	pinctrl_esai0: esai0grp {
> > +		fsl,pins = <
> > +			IMX8QM_ESAI0_FSR_AUD_ESAI0_FSR				0xc6000040
> > +			IMX8QM_ESAI0_FST_AUD_ESAI0_FST				0xc6000040
> > +			IMX8QM_ESAI0_SCKR_AUD_ESAI0_SCKR			0xc6000040
> > +			IMX8QM_ESAI0_SCKT_AUD_ESAI0_SCKT			0xc6000040
> > +			IMX8QM_ESAI0_TX0_AUD_ESAI0_TX0				0xc6000040
> > +			IMX8QM_ESAI0_TX5_RX0_AUD_ESAI0_TX5_RX0			0xc6000040
> > +			IMX8QM_MCLK_OUT0_AUD_ESAI0_TX_HF_CLK			0xc600004c
>
> [Severity: High]
> As mentioned earlier, the wm8904 codec driver configures <&mclkout0_lpcg 0>
> for its clock. However, this pinmux connects the pin to AUD_ESAI0_TX_HF_CLK
> rather than AUD_ACM_MCLK_OUT0, meaning the codec will likely receive the
> wrong clock source.


Please double check this

Frank

>
> > +		>;
> > +	};
> [ ... ]
> > +	pinctrl_wifi: wifigrp {
> > +		fsl,pins = <
> > +			/* GPIOs used by user space WiFi init */
> > +			IMX8QM_SCU_GPIO0_07_SCU_DSC_RTC_CLOCK_OUTPUT_32K	0xc600004c
> > +			IMX8QM_SCU_GPIO0_03_LSIO_GPIO0_IO31			0x06000021
> > +			IMX8QM_SCU_GPIO0_02_LSIO_GPIO0_IO30			0x00000021
> > +			IMX8QM_QSPI1A_DATA0_LSIO_GPIO4_IO26			0x00000021
> > +		>;
> > +	};
> > +};
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/cover.1784113547.git.stefano.r@variscite.com?part=2