From: Frank Li <Frank.li@oss.nxp.com>
To: sashiko-reviews@lists.linux.dev
Cc: Stefano Radaelli <stefano.radaelli21@gmail.com>,
Frank.Li@kernel.org, imx@lists.linux.dev, conor+dt@kernel.org,
robh@kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH v2 2/3] arm64: dts: freescale: Add support for Variscite DART-MX8M-MINI
Date: Mon, 13 Jul 2026 10:20:48 -0500 [thread overview]
Message-ID: <alUCUKa7UbyCSnzG@SMW015318> (raw)
In-Reply-To: <20260627150042.8F9A91F000E9@smtp.kernel.org>
On Sat, Jun 27, 2026 at 03:00:42PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [Medium] Simultaneous assertion/deassertion of Wi-Fi power enable and reset pins in `wifi_pwrseq`, violating typical SDIO Wi-Fi power sequencing requirements.
> - [Medium] Bluetooth GPIOs configured in `pinctrl_bt` are not assigned to or claimed by the Bluetooth device node, leaving them as unmanaged floating inputs.
> - [Medium] WM8904 codec configuration contains contradictory settings blindly copied from binding examples, likely breaking microphone functionality.
> --
>
> commit 4642338fef518c3eba73b9681507dd2dd37ee845
> Author: Stefano Radaelli <stefano.r@variscite.com>
>
> arm64: dts: freescale: Add support for Variscite DART-MX8M-MINI
>
> Add device tree support for the Variscite DART-MX8MM system on module.
> This SOM is designed to be used with various carrier boards. Only
> SOM-specific peripherals are enabled by default.
>
> Link: https://variscite.com/system-on-module-som/i-mx-8/i-mx-8m-mini/dart-mx8m-mini/
>
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mm-var-dart.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-var-dart.dtsi
> > new file mode 100644
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/freescale/imx8mm-var-dart.dtsi
>
> [ ... ]
>
> > + sound-wm8904 {
> > + compatible = "simple-audio-card";
> > + simple-audio-card,bitclock-master = <&codec_dai>;
> > + simple-audio-card,format = "i2s";
> > + simple-audio-card,frame-master = <&codec_dai>;
> > + simple-audio-card,mclk-fs = <256>;
> > + simple-audio-card,name = "wm8904-audio";
> > + simple-audio-card,routing =
> > + "Headphone Jack", "HPOUTL",
> > + "Headphone Jack", "HPOUTR",
> > + "IN2L", "Line In Jack",
> > + "IN2R", "Line In Jack",
> > + "IN1L", "Microphone Jack",
> > + "IN1R", "Microphone Jack";
>
> [Severity: Medium]
> Is this analog microphone routing missing a connection to MICBIAS?
>
> The simple-audio-card,routing property routes the Microphone Jack to the
> codec's analog inputs (IN1L/IN1R), but without a connection to the codec's
> MICBIAS pin, the analog microphone will likely not receive the required bias
> voltage to function.
>
> > + simple-audio-card,widgets =
> > + "Microphone", "Microphone Jack",
> > + "Headphone", "Headphone Jack",
> > + "Line", "Line In Jack";
>
> [ ... ]
>
> > + wifi_pwrseq: wifi-pwrseq {
> > + compatible = "mmc-pwrseq-simple";
> > + post-power-on-delay-ms = <100>;
> > + power-off-delay-us = <10000>;
> > + reset-gpios = <&gpio2 10 GPIO_ACTIVE_LOW>, /* WIFI_RESET */
> > + <&gpio2 20 GPIO_ACTIVE_LOW>; /* WIFI_PWR_EN */
> > + };
>
> [Severity: Medium]
> Does grouping both the power enable and reset pins in this single reset-gpios
> array violate SDIO Wi-Fi power sequencing requirements?
>
> The mmc-pwrseq-simple driver toggles all GPIOs in the reset-gpios array
> simultaneously. During power-on, the Wi-Fi chip's power enable and reset lines
> will be driven high at the exact same moment.
>
> Standard Wi-Fi power sequencing typically requires the power supply to be
> fully ramped and stable before the reset line is released.
>
> [ ... ]
>
> > +&i2c3 {
> > + clock-frequency = <100000>;
> > + pinctrl-names = "default", "gpio";
> > + pinctrl-0 = <&pinctrl_i2c3>;
> > + pinctrl-1 = <&pinctrl_i2c3_gpio>;
> > + scl-gpios = <&gpio5 18 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> > + sda-gpios = <&gpio5 19 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> > + status = "okay";
> > +
> > + wm8904: audio-codec@1a {
> > + compatible = "wlf,wm8904";
> > + reg = <0x1a>;
>
> [ ... ]
>
> > + /* GPIO1 = DMIC_CLK, don't touch others */
> > + wlf,gpio-cfg = <0x0018>, <0xffff>, <0xffff>, <0xffff>;
> > + };
> > +};
>
> [Severity: Medium]
> Does this digital microphone clock configuration contradict the analog audio
> routing defined above?
>
> Configuring GPIO1 as a digital microphone clock appears to be a direct copy
> from the wlf,wm8904 binding example, but it contradicts the analog routing for
> the Microphone Jack to IN1L/IN1R.
>
> This might expose unintended side-effects by driving a digital clock out of
> GPIO1 when it's not needed for an analog microphone setup.
Please check it?
>
> [ ... ]
>
> > +/* BT module */
> > +&uart4 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pinctrl_uart4>, <&pinctrl_bt>;
> > + assigned-clocks = <&clk IMX8MM_CLK_UART4>;
> > + assigned-clock-parents = <&clk IMX8MM_SYS_PLL1_80M>;
> > + uart-has-rtscts;
> > + status = "okay";
> > +
> > + bluetooth_iw61x: bluetooth {
> > + compatible = "nxp,88w8987-bt";
> > + };
> > +};
>
> [Severity: Medium]
> Are the Bluetooth GPIOs defined in pinctrl_bt missing their assignments here?
>
> The pinctrl_bt group defines GPIO2_IO6 and GPIO5_IO5 with floating pad
> configurations (0xc1), and the parent uart4 node applies them. However,
> because this bluetooth node doesn't map them to expected driver properties
> (such as device-wakeup-gpios or host-wakeup-gpios), they are left as
> unmanaged floating inputs.
>
> If one of these acts as an enable or reset pin, floating inputs can cause
> erratic behavior due to voltage drift.
Please double check this also
>
> > +/* WIFI */
> > +&usdhc1 {
> > + pinctrl-names = "default", "state_100mhz", "state_200mhz";
> > + pinctrl-0 = <&pinctrl_usdhc1>, <&pinctrl_wifi>;
> > + pinctrl-1 = <&pinctrl_usdhc1_100mhz>, <&pinctrl_wifi>;
> > + pinctrl-2 = <&pinctrl_usdhc1_200mhz>, <&pinctrl_wifi>;
> > + bus-width = <4>;
> > + keep-power-in-suspend;
> > + mmc-pwrseq = <&wifi_pwrseq>;
> > + non-removable;
> > + status = "okay";
> > +};
>
> [Severity: Medium]
> Should this node include a vmmc-supply linking to a regulator for the Wi-Fi
> power enable?
>
> Modeling WIFI_PWR_EN as a regulator-fixed assigned to vmmc-supply in this
> usdhc1 node, while leaving only WIFI_RESET in the mmc-pwrseq-simple node,
> would ensure the power sequencing is handled correctly.
Please double check this also
Frank
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/cover.1782571383.git.stefano.r@variscite.com?part=2
next prev parent reply other threads:[~2026-07-13 15:20 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-27 14:46 [PATCH v2 0/3] Add support for Variscite DART-MX8M-MINI and Sonata board Stefano Radaelli
2026-06-27 14:46 ` [PATCH v2 1/3] dt-bindings: arm: fsl: add Variscite DART-MX8M-MINI Boards Stefano Radaelli
2026-06-27 14:46 ` [PATCH v2 2/3] arm64: dts: freescale: Add support for Variscite DART-MX8M-MINI Stefano Radaelli
2026-06-27 15:00 ` sashiko-bot
2026-07-13 15:20 ` Frank Li [this message]
2026-06-27 14:46 ` [PATCH v2 3/3] arm64: dts: imx8mm-var-dart: Add support for Variscite Sonata board Stefano Radaelli
2026-06-27 14:57 ` sashiko-bot
2026-07-13 15:17 ` Frank Li
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=alUCUKa7UbyCSnzG@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=sashiko-reviews@lists.linux.dev \
--cc=stefano.radaelli21@gmail.com \
/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