Devicetree
 help / color / mirror / Atom feed
From: Frank Li <Frank.li@oss.nxp.com>
To: sashiko-reviews@lists.linux.dev
Cc: Rudi Heitbaum <rudi@heitbaum.com>,
	imx@lists.linux.dev, devicetree@vger.kernel.org,
	conor+dt@kernel.org, Frank.Li@kernel.org, robh@kernel.org
Subject: Re: [PATCH 5/5] arm64: dts: imx8mq-phanbell: Enable the rt5645 analog audio
Date: Fri, 7 Aug 2026 12:06:37 -0500	[thread overview]
Message-ID: <anYQnd1jW2ecFq_n@SMW015318> (raw)
In-Reply-To: <20260807141524.179AB1F00A3A@smtp.kernel.org>

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?
>
> > +
> > +		/*
> > +		 * 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?

>
> 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?

Please check this also

Frank
>
> > +		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

      reply	other threads:[~2026-08-07 17:06 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-07 13:29 [PATCH 0/5] arm64: dts: imx8mq-phanbell: Google Coral Dev Board enablement Rudi Heitbaum
2026-08-07 13:30 ` [PATCH 1/5] arm64: dts: imx8mq-phanbell: Keep the GPU rail on Rudi Heitbaum
2026-08-07 13:30 ` [PATCH 2/5] arm64: dts: imx8mq-phanbell: Do not hardcode a cooling state that may not exist Rudi Heitbaum
2026-08-07 13:31 ` [PATCH 3/5] arm64: dts: imx8mq-phanbell: Enable i2c2 and i2c3 Rudi Heitbaum
2026-08-07 13:32 ` [PATCH 4/5] arm64: dts: imx8mq-phanbell: Mux the 32 kHz reference clock pad Rudi Heitbaum
2026-08-07 13:32 ` [PATCH 5/5] arm64: dts: imx8mq-phanbell: Enable the rt5645 analog audio Rudi Heitbaum
2026-08-07 14:15   ` sashiko-bot
2026-08-07 17:06     ` Frank Li [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=anYQnd1jW2ecFq_n@SMW015318 \
    --to=frank.li@oss.nxp.com \
    --cc=Frank.Li@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=imx@lists.linux.dev \
    --cc=robh@kernel.org \
    --cc=rudi@heitbaum.com \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox