Re: [PATCH v2 06/17] arm64: dts: imx93-14x14-evk: Add board muxes to select between FlexCAN1 and MQS1

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The amplifier enable GPIO for MQS1 is incorrectly modeled as a multiplexer to hijack the pinctrl subsystem.
--

--- Patch [6]: [PATCH v2 06/17] arm64: dts: imx93-14x14-evk: Add board muxes to select between FlexCAN1 and MQS1 ---
commit cd0b6d22bf4426d5f5510c7f3a258a4b50405fb8
Author: Chancel Liu <[email protected]>

arm64: dts: imx93-14x14-evk: Add board muxes to select between FlexCAN1 and MQS1

FlexCAN1 has pin conflict with MQS1. Add GPIO-controlled board muxes to
route shared signals between FlexCAN1 and MQS1.

> diff --git a/arch/arm64/boot/dts/freescale/imx93-14x14-evk.dts b/arch/arm64/boot/dts/freescale/imx93-14x14-evk.dts
> index ec78c03f4788b..6dcfbca65cbd5 100644
> --- a/arch/arm64/boot/dts/freescale/imx93-14x14-evk.dts
> +++ b/arch/arm64/boot/dts/freescale/imx93-14x14-evk.dts
> @@ -37,6 +37,36 @@ chosen {
>  		stdout-path = &lpuart1;
>  	};
>  
[ ... ]
> +	mqs1_amp_en_mux: mux-controller-1 {
> +		compatible = "gpio-mux";
> +		#mux-control-cells = <0>;
> +		#mux-state-cells = <1>;
> +		mux-gpios = <&pcal6524_2 1 GPIO_ACTIVE_HIGH>;
> +	};

[Severity: Medium]
Is it appropriate to use the gpio-mux binding to control an amplifier enable
pin?

Device Tree generally describes the hardware structure. A hardware multiplexer
routes signals between inputs and outputs, whereas an amplifier enable pin is
usually a binary control.

Would it be better to model this with an audio-amplifier or regulator-fixed
node so that standard power management can dynamically toggle the amplifier
during audio playback and suspend, rather than statically evaluating it as a
mux state?

> +
> +	pinctrl-gpiomux {
> +		compatible = "pinctrl-multiplexer";
> +
[ ... ]
> +		mqs1_amp_en: mqs1-amp-en-grp {
> +			mux-states = <&mqs1_amp_en_mux 1>;
> +		};

[Severity: Medium]
Does modeling the amplifier enable as a pinctrl multiplexer state bypass the
expected power management abstractions?

It appears this uses the pinctrl subsystem to assert a basic power enable GPIO,
which might prevent the system from turning off the amplifier when not in use.

> +
> +		mqs1_fun: mqs1-grp {
> +			mux-states = <&can1_mqs1_mux 1>;
> +		};
> +	};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6