Devicetree
 help / color / mirror / Atom feed
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

  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