Re: [PATCH v2 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 2 potential issue(s) to consider:
- [High] The PMIC thermal zone's cooling map omits the high-power Cortex-A72 cores, severely limiting thermal mitigation.
- [Medium] WiFi and Bluetooth enable/reset GPIOs are configured in pinctrl but never bound to a driver, breaking power management and suspend/resume.
--

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

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 includes an i.MX8 QuadMax processor, RAM, eMMC, Ethernet,
audio codec, and WiFi/Bluetooth.

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..e805c51eb27df
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8qm-var-som.dtsi
[ ... ]
> +/* Bluetooth */
> +&lpuart1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_lpuart1>;
> +	uart-has-rtscts;
> +	status = "okay";
> +};

[Severity: Medium]
Since this UART is used for Bluetooth, should it have a serdev child node
(like compatible = "brcm,bcm4329-bt") with an 'enable-gpios' property?

Without an explicit driver binding to manage the module, the Bluetooth
device's power state cannot be controlled across suspend and resume.

[ ... ]
> +&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]
Is it intentional to omit the Cortex-A72 cores (<&A72_0> and <&A72_1>) from
this cooling map?

By only throttling the A53 cores during an overheat scenario, the system might
fail to cool the PMIC sufficiently. This increases the risk of reaching the
125°C threshold and triggering 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>;
> +	};
> +};

[ ... ]
> +	pinctrl_wifi: wifigrp {
> +		fsl,pins = <
> +			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]
Are these WiFi enable/reset GPIOs left uncontrolled without an active driver
binding?

While pinctrl_wifi is attached to usdhc3, there is no mmc-pwrseq node
consuming these GPIOs to control the power and reset sequencing. Relying on
the bootloader for initialization prevents the kernel from managing the power
state during suspend and resume.

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