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

[Rudi Heitbaum <rudi@heitbaum.com>]

On Fri, Aug 07, 2026 at 12:06:37PM -0500, Frank Li wrote:
> On Fri, Aug 07, 2026 at 02:15:22PM +0000, sashiko-bot@kernel.org wrote:
> > 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 <rudi@heitbaum.com>
> >
> > 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?

Headphone Mic route - added in v2 as "IN1P", "Headphone Mic". The jack
is a TRRS part wired to CTIA from the datasheet.

> > > +
> > > +		/*
> > > +		 * 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)?
> 
> Is it true?

system-clock-direction-out without a clocks property - the fallback is real,
but the conclusion is not, and the suggested fix is what breaks playback.

Falling back to the DAI node's first clock, the IPG clock, sets that as the
system clock once at card init. It is replaced at hw_params with
rate * mclk-fs before any stream starts, so the init-time value never
reaches a running stream.

I tested both ways on the board. With no clocks property, 44.1 kHz and
48 kHz both play natively. With clocks = <&clk IMX8MQ_CLK_SAI2_ROOT> on the
cpu dai, the mclk never moves to the 11.025 kHz PLL family and 44.1 kHz
fails in hw_params.

So I have left the property out, and said why in the v2 commit message.
 
> >
> > 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?

avdd-supply/cpvdd-supply - correct, both are in the binding's required
list, so dtbs_check did fail. v2 adds them and the fixed regulator they
need. CHECK_DTBS is clean now.

> Please check this also
> 
> Frank

Hi Frank,

Thanks for the review. I have posted a v2 including these fixes and explanation.

Regards
Rudi

> > > +		pinctrl-0 = <&pinctrl_hp_det>;
> > > +		#sound-dai-cells = <0>;
> > > +		interrupt-parent = <&gpio5>;
> >
> > --
> > Sashiko AI review · https://sashiko.dev/#/patchset/cover.1786107925.git.rudi@heitbaum.com?part=5