Devicetree
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Stefano Radaelli" <stefano.radaelli21@gmail.com>
Cc: robh@kernel.org, conor+dt@kernel.org, Frank.Li@kernel.org,
	devicetree@vger.kernel.org, imx@lists.linux.dev
Subject: Re: [PATCH v5 08/13] ARM: dts: imx6ul: Add Variscite storage and wireless options
Date: Fri, 09 Oct 2026 19:49:26 +0000	[thread overview]
Message-ID: <sashiko-outbox-166015@kernel.org> (raw)
In-Reply-To: <cfcd21519e92c74761875912dab36762ae304e2f.1791487179.git.stefano.r@variscite.com>

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] dts: mmc: missing no-1-8-v property in iw61x SDIO configuration
- [Medium] dts: mmc: missing vmmc-supply linkage for usdhc1 SD card interface
- [Medium] arm: dts: imx: tsc disabled in IW61x variant without documented pin conflict
--

commit aa7d3a1b7e7dcf63c97315306517bc7b3d2e56da
Author: Stefano Radaelli <stefano.r@variscite.com>
Date:   Thu Oct 8 19:37:48 2026 +0000

ARM: dts: imx6ul: Add Variscite storage and wireless options
    
Extract usdhc2 and its pinctrl groups from the VAR-SOM-6UL common
file into imx6ul-imx6ull-var-emmc.dtsi. Add shared NAND and IW61x
descriptions for VAR-SOM-6UL and DART-6UL, and describe the fixed
3.3 V supply in the existing SD card option. Rename
imx6ul-var-som-wifi.dtsi to imx6ul-var-som-wifi-brcm.dtsi and
update both existing users to distinguish Broadcom from IW61x.

> diff --git a/arch/arm/boot/dts/nxp/imx/imx6ul-imx6ull-var-wifi-iw61x.dtsi b/arch/arm/boot/dts/nxp/imx/imx6ul-imx6ull-var-wifi-iw61x.dtsi
> new file mode 100644
> index 0000000000000..d2f116bc8d70f
> --- /dev/null
> +++ b/arch/arm/boot/dts/nxp/imx/imx6ul-imx6ull-var-wifi-iw61x.dtsi
> @@ -0,0 +1,43 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2026 Variscite Ltd. - https://www.variscite.com
> + */
> +
> +/ {
> +	reg_sd1_vmmc: regulator-sd1-vmmc {
> +		compatible = "regulator-fixed";
> +		regulator-name = "VSD_3V3";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +	};
> +
> +	iw61x_pwrseq: mmc-pwrseq {
> +		compatible = "mmc-pwrseq-simple";
> +		post-power-on-delay-ms = <100>;
> +		power-off-delay-us = <10000>;
> +		reset-gpios = <&gpio5 6 GPIO_ACTIVE_LOW>, /* WIFI_RESET */
> +			      <&gpio5 2 GPIO_ACTIVE_LOW>; /* WIFI_PWR_EN */
> +	};
> +};
> +
> +&tsc {
> +	status = "disabled";
> +};

[Severity: Medium]
Is it intentional to unconditionally disable the touch screen controller
(tsc) for the IW61x module?

In the Broadcom variant, tsc is disabled due to a hardware conflict where
GPIO1_IO03 is multiplexed as OSC32K_32K_OUT to supply the Wi-Fi module,
which conflicts with tsc's xnur-gpios. However, this IW61x variant does
not declare pinctrl_32k_clk or multiplex GPIO1_IO03.

Without a documented hardware or pin conflict, this removes touch
functionality for all users of the IW61x Wi-Fi variant. Could this be an
unintended copy-paste from the Broadcom variant?

> +
> +&uart2 {
> +	bluetooth_iw61x: bluetooth {
> +		compatible = "nxp,88w8987-bt";
> +	};
> +};
> +
> +&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>;
> +	keep-power-in-suspend;
> +	non-removable;
> +	vmmc-supply = <&reg_sd1_vmmc>;
> +	mmc-pwrseq = <&iw61x_pwrseq>;
> +	status = "okay";
> +};

[Severity: High]
Does this iw61x SDIO configuration need the no-1-8-v property?

The SDIO interface for the Wi-Fi module is powered by a fixed 3.3V
supply (reg_sd1_vmmc). Modern NXP Wi-Fi modules (such as IW61x)
advertise UHS capabilities (1.8V modes). Without no-1-8-v, the MMC core
will negotiate a 1.8V switch (CMD11) if the card advertises it.

Since the i.MX6UL SDHCI controller on this board lacks a mechanism to
physically drop the IO voltage, it will continue driving 3.3V signals to a
Wi-Fi module that has switched to 1.8V receivers, causing immediate CRC
errors and probe failure.

[ ... ]

> diff --git a/arch/arm/boot/dts/nxp/imx/imx6ul-var-som-sd.dtsi b/arch/arm/boot/dts/nxp/imx/imx6ul-var-som-sd.dtsi
> index 0e6d9b945eb4a..bef0a60abfa94 100644
> --- a/arch/arm/boot/dts/nxp/imx/imx6ul-var-som-sd.dtsi
> +++ b/arch/arm/boot/dts/nxp/imx/imx6ul-var-som-sd.dtsi
> @@ -4,8 +4,18 @@
>   *
>   * Copyright 2019-2024 Variscite Ltd.
>   * Copyright 2026 Dimonoff
> + * Copyright (C) 2026 Variscite Ltd.
>   */
>  
> +/ {
> +	reg_sd1_vmmc: regulator-sd1-vmmc {
> +		compatible = "regulator-fixed";
> +		regulator-name = "VSD_3V3";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +	};
> +};
> +

[Severity: Medium]
Does the newly added reg_sd1_vmmc need to be linked to the usdhc1 SD card
interface?

The patch adds reg_sd1_vmmc to explicitly describe the fixed 3.3V supply,
but it doesn't link this regulator to the usdhc1 controller node via the
vmmc-supply = <&reg_sd1_vmmc>; property.

Without this linkage, the MMC subsystem remains unaware of the supply,
rendering the newly added regulator definition practically dead code and
failing to fulfill the commit's stated intent.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1791487179.git.stefano.r@variscite.com?part=8

  reply	other threads:[~2026-10-09 19:49 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-08 19:37 [PATCH v5 00/13] ARM: dts: imx6ul: Extend VAR-SOM-6UL and add DART-6UL support Stefano Radaelli
2026-10-08 19:37 ` [PATCH v5 01/13] dt-bindings: arm: fsl: Add Variscite i.MX6UL board variants Stefano Radaelli
2026-10-08 19:37 ` [PATCH v5 02/13] dt-bindings: arm: fsl: Add Variscite i.MX6ULL " Stefano Radaelli
2026-10-08 19:37 ` [PATCH v5 03/13] dt-bindings: arm: fsl: Add Variscite i.MX6ULZ " Stefano Radaelli
2026-10-09 10:39   ` Krzysztof Kozlowski
2026-10-08 19:37 ` [PATCH v5 04/13] net: phy: micrel: Check RMII clock select property presence Stefano Radaelli
2026-10-08 19:37 ` [PATCH v5 05/13] dt-bindings: net: micrel: Fix RMII clock select property type Stefano Radaelli
2026-10-08 19:37 ` [PATCH v5 06/13] ARM: dts: imx6ul: Extend VAR-SOM-6UL and add DART-6UL files Stefano Radaelli
2026-10-09 19:49   ` sashiko-bot
2026-10-08 19:37 ` [PATCH v5 07/13] ARM: dts: imx6ul: Add Variscite WM8904 and WM8731 support Stefano Radaelli
2026-10-09 19:49   ` sashiko-bot
2026-10-08 19:37 ` [PATCH v5 08/13] ARM: dts: imx6ul: Add Variscite storage and wireless options Stefano Radaelli
2026-10-09 19:49   ` sashiko-bot [this message]
2026-10-08 19:37 ` [PATCH v5 09/13] ARM: dts: imx6ul: Extend Variscite carrier board descriptions Stefano Radaelli
2026-10-09 19:49   ` sashiko-bot
2026-10-08 19:37 ` [PATCH v5 10/13] ARM: dts: imx6ul: Share Variscite PHY clock and update PHY options Stefano Radaelli
2026-10-08 19:37 ` [PATCH v5 11/13] ARM: dts: imx6ul: Add Variscite i.MX6UL board variants Stefano Radaelli
2026-10-08 19:37 ` [PATCH v5 12/13] ARM: dts: imx6ull: Add Variscite i.MX6ULL " Stefano Radaelli
2026-10-08 19:37 ` [PATCH v5 13/13] ARM: dts: imx6ulz: Add Variscite i.MX6ULZ " Stefano Radaelli

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=sashiko-outbox-166015@kernel.org \
    --to=sashiko-bot@kernel.org \
    --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