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

[sashiko-bot@kernel.org]

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? 

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). 

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.

> +		>;
> +	};
[ ... ]
> +	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