Re: [PATCH v3 5/7] arm64: dts: imx8dxl-evk: Add onboard GPIO mux controller

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] arm64: dts: imx8dxl-evk: spi0_mux and m4_debug_mux driven low on probe
--

commit a3a65e29ad3fd8c46c72a9be8f8c719076ef18d5
Author: Frank Li <Frank.Li@nxp.com>

arm64: dts: imx8dxl-evk: Add onboard GPIO mux controller

This commit adds four gpio-mux controllers and a pinctrl-multiplexer node
to describe the onboard signal multiplexer on imx8dxl-evk, and removes the
regulator-m2uart1sel node.

> diff --git a/arch/arm64/boot/dts/freescale/imx8dxl-evk.dts b/arch/arm64/boot/dts/freescale/imx8dxl-evk.dts
> index 0ede0c7ed422c..0ee2a028ebff6 100644
> --- a/arch/arm64/boot/dts/freescale/imx8dxl-evk.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8dxl-evk.dts
> @@ -97,14 +97,68 @@ vdevbuffer: memory-vdevbuffer@90400000 {
>  		};
>  	};
>  
> -	m2_uart1_sel: regulator-m2uart1sel {
> -		compatible = "regulator-fixed";
> -		regulator-min-microvolt = <3300000>;
> -		regulator-max-microvolt = <3300000>;
> -		regulator-name = "m2_uart1_sel";
> -		gpio = <&pca6416_1 6 GPIO_ACTIVE_HIGH>;
> -		enable-active-high;
> -		regulator-always-on;
> +	m4_debug_mux: mux-controller-0 {
> +		compatible = "gpio-mux";
> +		#mux-control-cells = <0>;
> +		#mux-state-cells = <1>;
> +		mux-gpios = <&pca6416_1 3 GPIO_ACTIVE_HIGH>;
> +	};
> +
> +	spi3_mux: mux-controller-1 {
> +		compatible = "gpio-mux";
> +		#mux-control-cells = <0>;
> +		#mux-state-cells = <1>;
> +		mux-gpios = <&pca6416_1 4 GPIO_ACTIVE_HIGH>;
> +	};
> +
> +	uart1_mux: mux-controller-2 {
> +		compatible = "gpio-mux";
> +		#mux-control-cells = <0>;
> +		#mux-state-cells = <1>;
> +		mux-gpios = <&pca6416_1 6 GPIO_ACTIVE_HIGH>;
> +	};
> +
> +	spi0_mux: mux-controller-3 {
> +		compatible = "gpio-mux";
> +		#mux-control-cells = <0>;
> +		#mux-state-cells = <1>;
> +		mux-gpios = <&pca6416_1 7 GPIO_ACTIVE_HIGH>;
> +	};
> +
> +	pinctrl-gpiomux {
> +		compatible = "pinctrl-multiplexer";
> +
> +		m4_debug_fun: cm4-uart-grp {
> +			mux-states = <&m4_debug_mux 1>;
> +		};
> +
> +		spi0_fun: spi0-grp {
> +			mux-states = <&spi0_mux 1>;
> +		};
> +
> +		spi3_fun: spi3-grp {
> +			mux-states = <&spi3_mux 1>;
> +		};
> +
> +		uart1_fun: uart1-grp {
> +			mux-states = <&uart1_mux 1>;
> +		};
> +
> +		lcd_0_fun: lcd0-grp {
> +			mux-states = <&spi0_mux 0>;
> +		};
> +
> +		lcd_1_fun: lcd1-grp {
> +			mux-states = <&spi3_mux 0>;
> +		};
> +
> +		lcd_2_fun: lcd2-grp {
> +			mux-states = <&uart1_mux 0>;
> +		};
> +
> +		lcd_3_fun: lcd3-grp {
> +			mux-states = <&m4_debug_mux 0>;
> +		};
>  	};
>  
>  	mux3_en: regulator-0 {

[ ... ]

> @@ -658,7 +712,7 @@ &cm40_intmux {
>  
>  &cm40_lpuart {
>  	pinctrl-names = "default";
> -	pinctrl-0 = <&pinctrl_cm40_lpuart>;
> +	pinctrl-0 = <&pinctrl_cm40_lpuart>, <&m4_debug_fun>;
>  	status = "disabled";
>  };
>  

[Severity: High]
Does adding spi0_mux and m4_debug_mux without an active consumer or
idle-state = <1> cause those muxes to be driven low (state 0, lcd_0_fun
and lcd_3_fun) at boot instead of keeping the hardware pull-up default
(state 1)?

When mux_gpio_probe() in drivers/mux/gpio.c probes each gpio-mux node, it
requests the mux gpios as outputs initialized low (state 0):

drivers/mux/gpio.c:mux_gpio_probe() {
	...
	mux_gpio->gpios = devm_gpiod_get_array(dev, "mux", GPIOD_OUT_LOW);
	...
}

Because neither spi0_mux nor m4_debug_mux specifies idle-state = <1>, both
lines remain driven low unless an enabled device selects spi0_fun or
m4_debug_fun:

1. In arch/arm64/boot/dts/freescale/imx8dxl-evk.dts, spi0_fun is defined in
pinctrl-gpiomux, but has no consumer. Meanwhile, &sai0 is enabled (used by
sound-bt-sco) and muxes its signals onto the spi0 pads via pinctrl_sai0
without referencing spi0_fun:

	&sai0 {
		pinctrl-names = "default";
		pinctrl-0 = <&pinctrl_sai0>;
		...
		status = "okay";
	};

	pinctrl_sai0: sai0grp {
		fsl,pins = <
			IMX8DXL_SPI0_CS0_ADMA_SAI0_RXD		0x06000060
			IMX8DXL_SPI0_CS1_ADMA_SAI0_RXC		0x06000040
			IMX8DXL_SPI0_SCK_ADMA_SAI0_TXC		0x06000060
			IMX8DXL_SPI0_SDI_ADMA_SAI0_TXD		0x06000060
			IMX8DXL_SPI0_SDO_ADMA_SAI0_TXFS		0x06000040
		>;
	};

2. For m4_debug_mux, m4_debug_fun is only attached to &cm40_lpuart, which has
status = "disabled", while &adc0 and imx8dxl-cm4 are enabled and share the
pads controlled by m4_debug_mux (&pca6416_1 3).

Should &sai0 include <&spi0_fun> in pinctrl-0, and should the mux controllers
specify idle-state = <1> (or have m4_debug_fun selected by an enabled
consumer) so that sai0 and the m4 / adc0 signals are not disconnected on
probe?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261020-dxl_lcdif-v3-0-adc944659e14@nxp.com?part=5