Re: [PATCH 5/5] arm64: dts: imx8mq-phanbell: Enable the rt5645 analog audio

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Incomplete DAPM routing for the external Headphone Microphone breaks headset recording.
- [High] Using `system-clock-direction-out` without an explicit clock misconfigures the Audio PLL and breaks audio playback.
- [Low] Missing required regulator supply properties `avdd-supply` and `cpvdd-supply` in the `rt5645` codec node.
--

--- Patch [5]: [PATCH 5/5] arm64: dts: imx8mq-phanbell: Enable the rt5645 analog audio ---
commit c3b167f1c120ceab271f952ec9169de906983534
Author: Rudi Heitbaum <[email protected]>

arm64: dts: imx8mq-phanbell: Enable the rt5645 analog audio

The board has an rt5645 on i2c3 driving the 3.5 mm headphone jack, its
microphone and two on-board DMICs, fed by sai2.

> diff --git a/arch/arm64/boot/dts/freescale/imx8mq-phanbell.dts b/arch/arm64/boot/dts/freescale/imx8mq-phanbell.dts
> index 2d134eefb7659..b819e644fa54e 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mq-phanbell.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mq-phanbell.dts
> @@ -46,6 +46,41 @@ fan: gpio-fan {
>  		pinctrl-0 = <&pinctrl_gpio_fan>;
>  		status = "okay";
>  	};
> +
> +	sound-analog {
> +		compatible = "simple-audio-card";

[ ... ]

> +		simple-audio-card,routing = "Headphone Jack", "HPOL",
> +					    "Headphone Jack", "HPOR",
> +					    "Headphone Mic", "micbias1",
> +					    "DMIC L1", "Internal Mic",
> +					    "DMIC R1", "Internal Mic";

[Severity: High]
Is there a missing route here to connect Headphone Mic to a codec input pin,
such as IN1P or IN2P?

Without this route, will ALSA DAPM see the microphone as a dead end, failing
to power up the codec's ADC and breaking headset microphone recording?

> +
> +		/*
> +		 * 32 bit slots regardless of the sample width. sai2 divides its
> +		 * 24.576 MHz root by an even integer only, so a 24 bit frame at
> +		 * 48 kHz would ask for 2.304 MHz, a ratio of 10.67.
> +		 */
> +		sound_analog_cpu: simple-audio-card,cpu {
> +			sound-dai = <&sai2>;
> +			system-clock-direction-out;

[Severity: High]
By specifying system-clock-direction-out here without an explicit clocks or
system-clock-frequency property, does this cause simple-card to fall back to
the child node's first clock (IMX8MQ_CLK_SAI2_IPG at 133MHz)?

If so, simple_init_dai() will set the Audio PLL to 133MHz during probe.
Because fsl_sai_set_dai_sysclk() blocks clock changes once playback starts
(when sai->mclk_streams > 0), the PLL remains stuck at 133MHz.

Will this cause fsl_sai_hw_params() to fail to derive valid BCLK dividers
from 133MHz, breaking audio playback completely?

> +			dai-tdm-slot-num = <2>;
> +			dai-tdm-slot-width = <32>;
> +		};

[ ... ]

> @@ -272,6 +307,18 @@ &i2c3 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_i2c3>;
>  	status = "okay";
> +
> +	rt5645: audio-codec@1a {
> +		compatible = "realtek,rt5645";
> +		reg = <0x1a>;
> +		pinctrl-names = "default";

[Severity: Low]
Are the avdd-supply and cpvdd-supply properties required for this codec node?

Will omitting them cause device tree schema validation (dtbs_check) to fail?

> +		pinctrl-0 = <&pinctrl_hp_det>;
> +		#sound-dai-cells = <0>;
> +		interrupt-parent = <&gpio5>;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5