Re: [PATCH v3 08/12] ARM: dts: imx6ul: Reuse Variscite DTSI files for Concerto

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] mmc: missing non-removable and no-1-8-v in SDIO Wi-Fi node
- [Medium] dt-bindings: removing Wi-Fi/BT hardware description for user-space management
- [Medium] mmc: missing high-speed pinctrl states for eMMC and SD card
- [Medium] ASoC: missing DAPM audio routing for the Microphone Jack
--

commit fe65cbeab7096b23be45d7585ae539a72bae3505
Author: Stefano Radaelli <stefano.r@variscite.com>

ARM: dts: imx6ul: Reuse Variscite DTSI files for Concerto

This commit converts the existing i.MX6UL and i.MX6ULL Concerto boards to
use shared module, option, and carrier DTSI files while maintaining the
existing DTB names and compatible strings. It replaces former components
with updated LCDIF display timings, userspace-managed Broadcom Wi-Fi
descriptions, and folds common resources into the shared configurations.

> The old Wi-Fi power sequence and Bluetooth nodes modeled enable and
> reset control in the kernel. On these SoMs those controls are handled
> in userspace, so use the shared Broadcom Wi-Fi description without the
> old kernel-managed nodes.

[Severity: Medium]
Device Tree is generally meant to describe the physical hardware topology
rather than software policy.

If these pins are physically wired to the enable lines, does removing the
physical Device Tree hardware description for the Wi-Fi reset
(usdhc1_pwrseq), power (reg_sd1_vmmc), and Bluetooth (shutdown-gpios)
break backward compatibility for older kernel versions expecting to
initialize the hardware?

> diff --git a/arch/arm/boot/dts/nxp/imx/imx6ul-var-som-common.dtsi b/arch/arm/boot/dts/nxp/imx/imx6ul-var-som-common.dtsi
> deleted file mode 100644
> --- a/arch/arm/boot/dts/nxp/imx/imx6ul-var-som-common.dtsi
> +++ /dev/null
[ ... ]
> -&usdhc2 {
> -	pinctrl-names = "default", "state_100mhz", "state_200mhz";
> -	pinctrl-0 = <&pinctrl_usdhc2>;
> -	pinctrl-1 = <&pinctrl_usdhc2_100mhz>;
> -	pinctrl-2 = <&pinctrl_usdhc2_200mhz>;

[Severity: Medium]
The original configuration files explicitly defined state_100mhz and
state_200mhz for the eMMC and SD card interfaces.

The i.MX ESDHC driver typically requires these states to adjust pin drive
strength and timing for higher-speed bus modes.

Does the newly introduced imx6ul-imx6ull-var-emmc.dtsi truncate the
pinctrl-names to just "default", which might artificially limit the SD and
eMMC performance to standard speeds?

> diff --git a/arch/arm/boot/dts/nxp/imx/imx6ul-var-som-concerto-full.dts b/arch/arm/boot/dts/nxp/imx/imx6ul-var-som-concerto-full.dts
> --- a/arch/arm/boot/dts/nxp/imx/imx6ul-var-som-concerto-full.dts
> +++ b/arch/arm/boot/dts/nxp/imx/imx6ul-var-som-concerto-full.dts
[ ... ]
> -#include "imx6ul-var-som-audio.dtsi"
> -#include "imx6ul-var-som-lvds-panel.dtsi"
> +#include "imx6ul-imx6ull-var-emmc.dtsi"
> +#include "imx6ul-imx6ull-var-wifi-brcm.dtsi"
> +#include "imx6ul-imx6ull-var-wm8904.dtsi"
> +#include "imx6ul-var-som-concerto-board.dtsi"

[Severity: Medium]
The newly included imx6ul-imx6ull-var-wm8904.dtsi defines a
simple-audio-card that lists "Microphone Jack" in its widgets.

Without a logical route connecting the Microphone Jack to the WM8904
codec's input pins, ALSA DAPM might not power up the microphone bias or
the ADC path, rendering the microphone input unusable on these boards.

Is the "Microphone Jack" missing from the simple-audio-card,routing list?

> diff --git a/arch/arm/boot/dts/nxp/imx/imx6ul-var-som-wifi.dtsi b/arch/arm/boot/dts/nxp/imx/imx6ul-var-som-wifi.dtsi
> deleted file mode 100644
> --- a/arch/arm/boot/dts/nxp/imx/imx6ul-var-som-wifi.dtsi
> +++ /dev/null
[ ... ]
> -&usdhc1 {
> -	#address-cells = <1>;
> -	#size-cells = <0>;
> -	pinctrl-names = "default", "state_100mhz", "state_200mhz";
> -	pinctrl-0 = <&pinctrl_usdhc1>, <&pinctrl_32k_clk>;
> -	pinctrl-1 = <&pinctrl_usdhc1_100mhz>, <&pinctrl_32k_clk>;
> -	pinctrl-2 = <&pinctrl_usdhc1_200mhz>, <&pinctrl_32k_clk>;
> -	no-1-8-v;
> -	non-removable;

[Severity: High]
Without non-removable and cd-gpios, the MMC core might wait for a card
detect state change and fail to probe the SDIO Wi-Fi device.

Furthermore, without no-1-8-v, the MMC core might attempt a 1.8V UHS
voltage switch on a 3.3V-only bus and cause communication errors.

By switching to the shared imx6ul-imx6ull-var-wifi-brcm.dtsi, does the new
SDIO Wi-Fi node omit the non-removable and no-1-8-v properties that were
present here?

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