Re: [EXT] Re: [PATCH 2/2] arm64: dts: freescale: add i.MX952 FRDM Board support device tree

[Frank Li <Frank.li@oss.nxp.com>]

On Wed, Jul 22, 2026 at 07:57:58AM +0000, Bough Chen wrote:
> > -----Original Message-----
> > From: Frank Li <Frank.li@oss.nxp.com>
> > Sent: 2026年7月20日 23:07
> > To: Joseph Guo <qijian.guo@nxp.com>
> > Cc: sashiko-reviews@lists.linux.dev; robh@kernel.org; Frank.Li@kernel.org;
> > conor+dt@kernel.org; imx@lists.linux.dev; devicetree@vger.kernel.org
> > Subject: Re: [EXT] Re: [PATCH 2/2] arm64: dts: freescale: add i.MX952 FRDM
> > Board support device tree
> >
> > On Mon, Jul 20, 2026 at 03:03:20PM +0800, Joseph Guo wrote:
> > > On 7/20/2026 2:55 PM, sashiko-bot@kernel.org wrote:
> > > > [You don't often get email from sashiko-bot@kernel.org. Learn why
> > > > this is important at https://aka.ms/LearnAboutSenderIdentification ]
> > > >
> > > > Caution: This is an external email. Please take care when clicking
> > > > links or opening attachments. When in doubt, report the message
> > > > using the 'Report this email' button
> > > >
> > > >
> > > > Thank you for your contribution! Sashiko AI review found 2 potential
> > issue(s) to consider:
> > > > - [High] Shared GPIO requested exclusively by two CAN transceivers
> > causes probe failure for the second transceiver.
> > > > - [Low] Use of undocumented, downstream-only device tree property
> > `fsl,cd-gpio-wakeup-disable`.
> > > > --
> > > >
> > > > --- Patch [2]: [PATCH 2/2] arm64: dts: freescale: add i.MX952 FRDM
> > > > Board support device tree --- commit
> > > > 8bbcb28d08a05e0f4c57023333492b4e023f7caf
> > > > Author: Joseph Guo <qijian.guo@nxp.com>
> > > >
> > > > arm64: dts: freescale: add i.MX952 FRDM Board support device tree
> > > >
> > > > Add basic device tree support for i.MX952 FRDM Board. The
> > > > board-common hardware description is placed in imx952-frdm.dtsi so
> > > > that it can be shared with future i.MX952-based FRDM derivative boards.
> > > >
> > > >> diff --git a/arch/arm64/boot/dts/freescale/imx952-frdm.dtsi
> > > >> b/arch/arm64/boot/dts/freescale/imx952-frdm.dtsi
> > > >> new file mode 100644
> > > >> index 0000000000000..b9ef6bec2bd5a
> > > >> --- /dev/null
> > > >> +++ b/arch/arm64/boot/dts/freescale/imx952-frdm.dtsi
> > > >> @@ -0,0 +1,727 @@
> > > > [ ... ]
> > > >> +     flexcan2_phy: can-phy0 {
> > > >> +             compatible = "nxp,tja1051";
> > > >> +             #phy-cells = <0>;
> > > >> +             max-bitrate = <8000000>;
> > > >> +             silent-gpios = <&pcal6416 6 GPIO_ACTIVE_HIGH>;
> > > >> +     };
> > > >> +
> > > >> +     flexcan3_phy: can-phy1 {
> > > >> +             compatible = "nxp,tja1051";
> > > >> +             #phy-cells = <0>;
> > > >> +             max-bitrate = <8000000>;
> > > >> +             silent-gpios = <&pcal6416 6 GPIO_ACTIVE_HIGH>;
> > > >
> > > > [Severity: High]
> > > > Does this cause a probe failure for the second transceiver?
> > > >
> > > > The first PHY node successfully requests this GPIO line, taking
> > > > exclusive ownership. Because the phy-can-transceiver driver uses
> > > > standard exclusive requests (without GPIOD_FLAGS_BIT_NONEXCLUSIVE),
> > > > the second request for the exact same GPIO line fails with -EBUSY.
> > > >
> > > > This prevents the flexcan3_phy and the flexcan3 controller from
> > > > probing and functioning.
> > > >
> > >
> > > This two CAN PHY did share same pin as their silent-gpio on hardware.
> >
> > Hardware share gpio, but driver may not handle this case. Are you sure
> >
> > flexcan3_phy probed if can-phy0 already probed?
>
>
> Hi Frank
>
> I dug into this. Both of you and sashiko are effectively right, but the outcome depends entirely on kernel config, so let me lay out the facts.
>
> The driver side confirms Frank's/Sashiko's concern. In drivers/phy/phy-can-transceiver.c the silent line is requested exclusively:
>
> silent_gpio = devm_gpiod_get_index_optional(dev, "silent", i,
> 					    GPIOD_OUT_LOW);
> No GPIOD_FLAGS_BIT_NONEXCLUSIVE, and tja1051_drvdata carries CAN_TRANSCEIVER_SILENT_PRESENT, so the line really is requested. On a plain build, the second phy referencing the same pin would hit test_and_set_bit(GPIOD_FLAG_REQUESTED) in gpiod_request_commit() and get -EBUSY, failing the second transceiver and its flexcan controller. And NONEXCLUSIVE cannot be set from DT - the DT gpio flags cell only encodes ACTIVE_LOW / OPEN_DRAIN / PULL etc. So DTS tweaks alone cannot fix it.
>
> Yet on the i.MX95 15x15 FRDM (same design: flexcan2_phy and flexcan5_phy both use silent-gpios = <&pcal6524 7 ...>) I tested it and both can0/can1 probe and pass traffic. The reason is the gpio-shared-proxy framework (drivers/gpio/gpiolib-shared.c, gpio-shared-proxy.c). At postcore_initcall gpiolib scans the DT; when the same controller+offset is referenced by
>
> 1 consumer via a "*-gpios" property, it claims the real line once (marked "shared") and hands each consumer its own proxy descriptor with vote-based arbitration - so the second probe no longer returns -EBUSY, with no DTS change.
>
> The important subtlety is how that framework got enabled in my build. It is gated by HAVE_SHARED_GPIOS, and in both your BSP tree(frank.li/linux.git) and current linux-next, the only selector is ARCH_QCOM (commit e511d484cbe4, "arm64: select HAVE_SHARED_GPIOS for ARCH_QCOM"). ARCH_MXC does NOT select it. My kernel has CONFIG_GPIO_SHARED=y because CONFIG_ARCH_QCOM=y is also set (as in the arm64 defconfig), which selects HAVE_SHARED_GPIOS -> GPIO_SHARED=y globally. Since the framework scans the whole DT regardless of SoC, our i.MX CAN nodes benefit from a config that Qualcomm pulled in. It works by coincidence of the multi-platform build, not because i.MX enables it.
>
> Consequences for upstreaming this DTS:
>
> Pure i.MX config (ARCH_MXC=y, no ARCH_QCOM): nothing selects HAVE_SHARED_GPIOS, GPIO_SHARED=n, the framework is compiled out, and the second CAN transceiver fails with -EBUSY. This is exactly Sashiko's scenario, and it is real.
>
> arm64 defconfig (QCOM + i.MX): framework is pulled in by QCOM and it works - what I tested.
>
> So the DTS is fine as written, but to make the shared silent-gpio robust for any i.MX-only configuration we should send a small prerequisite patch mirroring the Qualcomm one:
>
> --- a/arch/arm64/Kconfig.platforms
> +++ b/arch/arm64/Kconfig.platforms
> @@ config ARCH_MXC
> 	 select IMX_GPCV2
> 	 select IMX_GPCV2_PM_DOMAINS
> +	 select HAVE_SHARED_GPIOS
> 	 select PM
> 	 select PM_GENERIC_DOMAINS
> (and CONFIG_GPIO_SHARED_PROXY should be built-in/module in the i.MX defconfig; it is default m). With that in place, the shared silent-gpio in the FRDM DTSI probes correctly regardless of whether ARCH_QCOM is also enabled. If you think this method is okay, then I think Joseph can post this Kconfig patch alongside the board DTS.
>

Thanks you for detail information. It is quite helpful.

Frank

> Separately, on the [Low] finding:  Joseph, please drop the downstream-only "fsl,cd-gpio-wakeup-disable" from &usdhc2 in the upstream DTSI - there is no upstream binding and no driver consumes it, so it is dead code and would trip make dtbs_check
>
> Regards
> Haibo Chen
>
> >
> > Frank
> >
> > >
> > > The same design on i.MX95 15x15 FRDM. This should be fine.
> > >
> > > Regards,
> > > Joseph
> > >
> > > >> +     };
> > > > [ ... ]
> > > >> +&usdhc2 {
> > > >> +     bus-width = <4>;
> > > >> +     cd-gpios = <&gpio3 0 GPIO_ACTIVE_LOW>;
> > > >> +     pinctrl-0 = <&pinctrl_usdhc2>, <&pinctrl_usdhc2_gpio>;
> > > >> +     pinctrl-1 = <&pinctrl_usdhc2_100mhz>, <&pinctrl_usdhc2_gpio>;
> > > >> +     pinctrl-2 = <&pinctrl_usdhc2_200mhz>, <&pinctrl_usdhc2_gpio>;
> > > >> +     pinctrl-3 = <&pinctrl_usdhc2>, <&pinctrl_usdhc2_gpio>;
> > > >> +     pinctrl-names = "default", "state_100mhz", "state_200mhz",
> > "sleep";
> > > >> +     vmmc-supply = <&reg_usdhc2_vmmc>;
> > > >> +     fsl,cd-gpio-wakeup-disable;
> > > >
> > > > [Severity: Low]
> > > > Is this out-of-tree vendor property invalid in the upstream kernel?
> > > >
> > > > The property fsl,cd-gpio-wakeup-disable does not exist in any
> > > > upstream device tree bindings (such as fsl-imx-esdhc.yaml) and is
> > > > not handled by any driver code in the upstream Linux tree. This acts
> > > > as dead code and will trigger `make dtbs_check` validation warnings.
> > > >
> > > >> +     status = "okay";
> > > >> +};
> > > >
> > > > --
> > > > Sashiko AI review ・
> > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsa
> > > >
> > shiko.dev%2F%23%2Fpatchset%2F20260720-imx952_frdm-v1-0-6d145eaaa5
> > c4%
> > > >
> > 40nxp.com%3Fpart%3D2&data=05%7C02%7Chaibo.chen%40nxp.com%7C29
> > e9bc792
> > > >
> > 4f2455f2cdd08dee67565f4%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0
> > %7C0%7
> > > >
> > C639201589065857811%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGk
> > iOnRydWUs
> > > >
> > IlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D
> > > > %7C0%7C%7C%7C&sdata=hQfdYq0rrk8kvruJk%2FGc32LxLfMMmRAJn7kG
> > bT5BMjA%3D
> > > > &reserved=0
> > >
> > > --
> > > Best Regards,
> > > Joseph
>