Re: [PATCH v4 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 4 potential issue(s) to consider:
- [High] The CMA allocation range starting at 0xC0000000 falls entirely outside the statically defined 1GB memory node bounds.
- [High] Assigning a 3.3V regulator to the 1.8V WM8904 codec supply pins violates absolute maximum hardware ratings.
- [Medium] Delegating WiFi/Bluetooth hardware initialization to user space scripts bypasses kernel power management and is a known DT anti-pattern.
- [Medium] sai7 targets the wrong LPCG clock index (IPG clock instead of MCLK) for its assigned clocks, leading to incorrect audio clock configuration.
--

--- Patch [2]: [PATCH v4 2/3] arm64: dts: freescale: Add support for Variscite VAR-SOM-MX8 QuadMax ---
commit 62ffa3edc88c7bf11df029d0ed1df55635d9355b
Author: Stefano Radaelli <stefano.r@variscite.com>

arm64: dts: freescale: Add support for Variscite VAR-SOM-MX8 QuadMax

This adds device tree support for the Variscite VAR-SOM-MX8 QuadMax system
on module. It enables module-specific peripherals like the i.MX8 QuadMax MPU,
LPDDR4 memory, eMMC, Ethernet, WM8904 codec audio, and WIFI6/Bluetooth.
Carrier board interfaces are left disabled by default.

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
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8qm-var-som.dtsi
> @@ -0,0 +1,531 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
[ ... ]
> +	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";
> +			/* VAR-SOM-MX8QM doesn't have 1GB variant */
> +			alloc-ranges = <0 0xc0000000 0 0x3c000000>;

[Severity: High]
Does this CMA allocation range fall entirely outside the statically defined
system RAM?

The memory@80000000 node defined earlier establishes 1GB of memory starting
at 0x80000000 and ending at 0xC0000000. However, this alloc-ranges property
starts exactly at 0xC0000000. If the bootloader does not dynamically patch
the memory node, could this cause CMA initialization to fail or trigger
panics?

> +			size = <0 0x3c000000>;
> +			linux,cma-default;
> +			reusable;
> +		};
> +	};
[ ... ]
> +&i2c0 {
> +	clock-frequency = <100000>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_i2c0>;
> +	status = "okay";
> +
> +	wm8904: codec@1a {
> +		compatible = "wlf,wm8904";
> +		reg = <0x1a>;
[ ... ]
> +		AVDD-supply = <&reg_audio_supply>;
> +		CPVDD-supply = <&reg_audio_supply>;
> +		DBVDD-supply = <&reg_audio_supply>;
> +		DCVDD-supply = <&reg_audio_supply>;

[Severity: High]
Does assigning the 3.3V reg_audio_supply to these 1.8V WM8904 domains violate
the absolute maximum hardware ratings?

The reg_audio_supply is defined as a fixed 3.3V regulator, but the maximum
voltage for WM8904 AVDD/CPVDD is 2.0V and DCVDD is 1.98V. Does this hardware
description misrepresent the physical setup, or is the hardware actually
supplied with 3.3V on these pins?

> +		MICVDD-supply = <&reg_audio_supply>;
[ ... ]
> +&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>;

[Severity: Medium]
Is sai7 targeting the wrong LPCG clock index here?

This assigns a parent to <&sai7_lpcg 0> (the IPG clock, IMX_LPCG_CLK_0)
instead of the MCLK (IMX_LPCG_CLK_4). The sai6 node correctly uses
IMX_LPCG_CLK_4 for reparenting. Does this break the MCLK configuration
for sai7?

> +	assigned-clock-parents = <&aud_pll_div1_lpcg 0>;
> +	assigned-clock-rates = <0>, <786432000>, <98304000>, <24576000>, <98304000>;
> +	fsl,sai-asynchronous;
> +	status = "okay";
> +};
[ ... ]
> +	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
> +		>;
> +	};

[Severity: Medium]
Does delegating the WiFi and Bluetooth hardware initialization to user space
scripts bypass kernel power management?

This pinctrl group explicitly reserves pins to be toggled from user space.
Should the power sequence be modeled in the device tree via mmc-pwrseq-simple
or fixed regulators so the kernel can manage power sequencing autonomously
instead?

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