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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] WM8731 codec MCLK pin is left undriven due to the deletion of 'fsl,sai-mclk-direction-output' without an alternative clock output route.
- [Medium] Onboard Wi-Fi and Bluetooth power sequences are improperly delegated to user-space scripts instead of kernel device drivers.
--

commit 3caa578f0cdaf021ef9c9ff617d60ff3b372369e
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.
This module is based on the NXP i.MX7Dual SoC and is available in multiple
configurations.
    
Link: https://variscite.com/system-on-module-som/i-mx-7/var-som-mx7/

> 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..92dda8d36478e
> --- /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>;
> +		clock-names = "mclk";

[ ... ]

> +&sai1 {
> +	assigned-clock-rates = <0>, <36864000>;
> +	/delete-property/ fsl,sai-mclk-direction-output;
> +};

[Severity: High]
Does removing the 'fsl,sai-mclk-direction-output' property break the master
clock for the WM8731 codec?

The wm8731 node configures the codec to expect an internal SoC master clock
by setting clocks = <&clks IMX7D_AUDIO_MCLK_ROOT_DIV>.

By deleting the 'fsl,sai-mclk-direction-output' property from the sai1
node, the SAI1_MCLK pad is configured as an input instead of driving the
clock out to the codec.

Will this prevent the WM8731 codec from functioning on this board variant
because it never receives a 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..158158afc3391
> --- /dev/null
> +++ b/arch/arm/boot/dts/nxp/imx/imx7d-var-som.dtsi

[ ... ]

> +&iomuxc {
> +	/* GPIO Used in User Space init script */
> +	pinctrl_bt: btgrp {
> +		fsl,pins = <
> +			MX7D_PAD_GPIO1_IO14__GPIO1_IO14			0x80000000
> +		>;
> +	};

[Severity: Medium]
Are these Bluetooth power sequences improperly delegated to user-space
scripts instead of kernel device drivers?

The pinctrl_bt node defines the BT_REG_ON (GPIO1_IO14) pin strictly as a
pinctrl definition with no corresponding power sequencer or regulator
binding.

Since the comment explicitly notes it is used in a user space init script,
will this prevent the kernel from autonomously enumerating the UART Bluetooth
device during boot?

[ ... ]

> +&iomuxc_lpsr {

[ ... ]

> +	/* GPIO Used in User Space init script */
> +	pinctrl_wlan: wlangrp {
> +		fsl,pins = <
> +			MX7D_PAD_LPSR_GPIO1_IO04__GPIO1_IO4		0x09
> +		>;
> +	};

[Severity: Medium]
Is this Wi-Fi power sequence improperly delegated to user-space scripts
instead of an MMC power sequence?

The pinctrl_wlan node defines the WL_REG_ON (GPIO1_IO4) pin strictly as a
pinctrl definition without a corresponding power sequencer or regulator
binding.

Will this prevent the kernel from autonomously enumerating the SDIO Wi-Fi
device during boot?

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