Re: [PATCH 2/2] arm64: dts: freescale: add i.MX937 FRDM board support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] arm64: dts: freescale: shared pinctrl-multiplexer on flexcan2/3 fails probe
- [Medium] arm64: dts: freescale: mismatch between imx937.dtsi GPU OPP and commit message
--

commit 27f9d906883e5428d8c5dbb12d877982379dd7f9
Author: Joy Zou <joy.zou@oss.nxp.com>

arm64: dts: freescale: add i.MX937 FRDM board support

This commit adds device tree support for the NXP i.MX937 SoC and i.MX937
FRDM board based on the i.MX952 device trees.

>     arm64: dts: freescale: add i.MX937 FRDM board support
>     
>     The i.MX937 is a derivative of the i.MX952 SoC, so it reuses the
>     i.MX952 device tree as its base. Add imx937.dtsi based on imx952.dtsi
>     and imx937-frdm.dts based on imx952-frdm.dtsi to support the NXP
>     i.MX937 FRDM board.
>     
>     Compared to the i.MX952, the i.MX937 drops the local dimming and ISP
>     blocks and runs its Cortex-A55 cores at a lower maximum frequency.
>     
>     The PCAL6416 GPIO expander is at I2C address 0x21 on i.MX937 FRDM,
>     unlike 0x20 on i.MX952 FRDM, so disable the inherited node and add
>     the expander at 0x21.
>     
>     Signed-off-by: Joy Zou <joy.zou@oss.nxp.com>

> diff --git a/arch/arm64/boot/dts/freescale/imx937-frdm.dts b/arch/arm64/boot/dts/freescale/imx937-frdm.dts
> new file mode 100644
> index 0000000000000..d6be48c810573
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx937-frdm.dts
> @@ -0,0 +1,111 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright 2026 NXP
> + */
> +
> +/dts-v1/;
> +
> +#include "imx937.dtsi"
> +#include "imx952-frdm.dtsi"
> +
> +/ {
> +	model = "NXP i.MX937 FRDM board";
> +	compatible = "fsl,imx937-frdm", "fsl,imx937", "fsl,imx952";
> +
> +	can_mux: mux-controller-0 {
> +		compatible = "gpio-mux";
> +		#mux-control-cells = <0>;
> +		#mux-state-cells = <1>;
> +		/*
> +		 * PCAL6416 (0x21) pin 4 selects the CAN/GPIO mux:
> +		 * low selects CAN, high selects GPIO. With GPIO_ACTIVE_HIGH,
> +		 * mux state 0 drives the pin low (CAN) and state 1 high (GPIO).
> +		 */
> +		mux-gpios = <&pcal6416_21 4 GPIO_ACTIVE_HIGH>;
> +	};
> +
> +	can_mux_pinctrl: pinctrl-gpiomux {
> +		compatible = "pinctrl-multiplexer";
> +
> +		can_fun: can-grp {
> +			mux-states = <&can_mux 0>;
> +		};
> +
> +		gpio_fun: gpio-grp {
> +			mux-states = <&can_mux 1>;
> +		};
> +	};
> +};
> +
> +/* Select CAN on the shared CAN/GPIO mux when FlexCAN is enabled. */
> +&flexcan2 {
> +	pinctrl-0 = <&pinctrl_flexcan2>, <&can_fun>;
> +};
> +
> +/*
> + * The CAN transceiver silent-mode line is on the 0x21 PCAL6416 on
> + * i.MX937 FRDM (the inherited 0x20 expander is disabled below).
> + */
> +&flexcan2_phy {
> +	silent-gpios = <&pcal6416_21 6 GPIO_ACTIVE_HIGH>;
> +};
> +
> +&flexcan3 {
> +	pinctrl-0 = <&pinctrl_flexcan3>, <&can_fun>;
> +};

[Severity: High]
Will the second FlexCAN controller to probe fail with -EBUSY when both
flexcan2 and flexcan3 reference <&can_fun> in pinctrl-0?

Both flexcan2 and flexcan3 inherit status = "okay" from
arch/arm64/boot/dts/freescale/imx952-frdm.dtsi:

arch/arm64/boot/dts/freescale/imx952-frdm.dtsi:
	&flexcan2 {
		pinctrl-names = "default";
		pinctrl-0 = <&pinctrl_flexcan2>;
		phys = <&flexcan2_phy>;
		status = "okay";
	};

	&flexcan3 {
		pinctrl-names = "default";
		pinctrl-0 = <&pinctrl_flexcan3>;
		phys = <&flexcan3_phy>;
		status = "okay";
	};

When the first FlexCAN device probes, the call chain

  really_probe() ->
    pinctrl_bind_pins() ->
      pinctrl_select_state() ->
        pinmux_enable_setting() ->
          mux_pinmux_set_mux()

selects func->mux_state in drivers/pinctrl/pinctrl-generic-mux.c:

drivers/pinctrl/pinctrl-generic-mux.c:mux_pinmux_set_mux() {
	...
	ret = mux_state_try_select(func->mux_state);
	if (ret)
		return ret;
	...
}

which calls mux_control_try_select_delay() in drivers/mux/core.c and
acquires the exclusive binary semaphore mux->lock (initialized to 1 in
mux_chip_alloc()):

drivers/mux/core.c:mux_control_try_select_delay() {
	...
	if (down_trylock(&mux->lock))
		return -EBUSY;
	...
}

Because mux->lock remains held until mux_pinmux_release_mux() calls
mux_state_deselect(), when the second FlexCAN device probes and attempts
to select the same <&can_fun> state backed by &can_mux,
down_trylock(&mux->lock) fails with -EBUSY and aborts its probe.

[ ... ]

> diff --git a/arch/arm64/boot/dts/freescale/imx937.dtsi b/arch/arm64/boot/dts/freescale/imx937.dtsi
> new file mode 100644
> index 0000000000000..7912a9c282d8c
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx937.dtsi
> @@ -0,0 +1,12 @@
> +// SPDX-License-Identifier: (GPL-2.0-only OR MIT)
> +/*
> + * Copyright 2026 NXP
> + */
> +
> +#include "imx952.dtsi"
> +
> +&gpu_opp_table {
> +	opp-1000000000 {
> +		status = "disabled";
> +	};
> +};

[Severity: Medium]
Is disabling opp-1000000000 in &gpu_opp_table intentional here, or does the
commit message mismatch the change in imx937.dtsi?

The commit message states:

  "Compared to the i.MX952, the i.MX937 drops the local dimming and ISP
   blocks and runs its Cortex-A55 cores at a lower maximum frequency."

However, the only change in arch/arm64/boot/dts/freescale/imx937.dtsi
disables the 1.0 GHz operating point in &gpu_opp_table, which is the
operating-points-v2 table for the Mali GPU (gpu@4d900000) in
arch/arm64/boot/dts/freescale/imx952.dtsi:

arch/arm64/boot/dts/freescale/imx952.dtsi:
	gpu_opp_table: opp-table {
		compatible = "operating-points-v2";
		...
		opp-1000000000 {
			opp-hz = /bits/ 64 <1000000000>;
			opp-microvolt = <920000>;
		};
	};

while the Cortex-A55 cores in imx952.dtsi use SCMI performance domains
(<&scmi_perf IMX952_PERF_A55>).

If the i.MX937 reduces the maximum GPU frequency, should the commit message
describe that instead (or was this meant to target the CPU frequency)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019-b4-imx937-frdm-v1-0-76d949a61954@oss.nxp.com?part=2