Re: [PATCH] usb: cdns3: attempt to fix Kconfig dependencies

Peter Chen (CIX)
2026-04-03T07:50:49Z

On 26-04-02 16:09:55, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> The way that dependencies between host and gadget mode, as well as cdns3
> and cdnsp were handled was rather fragile before commit 6076388ca1ed
> ("usb: cdns3: Add USBSSP platform driver support").
>
> After those changes, I get randconfig build failures:
>
> arm-linux-gnueabi-ld: drivers/usb/cdns3/cdnsp-gadget.o: in function `__cdnsp_gadget_init':
> cdnsp-gadget.c:(.text+0x12da): undefined reference to `cdns_drd_gadget_on'
> arm-linux-gnueabi-ld: drivers/usb/cdns3/cdnsp-gadget.o: in function `cdnsp_gadget_pullup':
> cdnsp-gadget.c:(.text+0x3030): undefined reference to `cdns_clear_vbus'
> arm-linux-gnueabi-ld: cdnsp-gadget.c:(.text+0x3138): undefined reference to `cdns_set_vbus'
> arm-linux-gnueabi-ld: drivers/usb/cdns3/cdnsp-gadget.o: in function `cdnsp_gadget_exit':
> cdnsp-gadget.c:(.text+0xe0): undefined reference to `cdns_drd_gadget_off'
>
> and I see additional configurations that are broken. The main problem
> here is that the 'common' module links against both host and gadget
> support if they are enabled, but there are insufficient protections
> agains it being built-in if only one of them is built-in and the other
> is in a loadable module, causing link failures.
>
> The use of IS_REACHABLE() in gadget-export.h works around a similar
> problem if one of cdns3 and cdnsp is built-in but the other one is
> =m. This one is worse because instead of a clear link failure, the
> logic just makes it not work at all despite support being enabled.
>
> To improve this mess, throw out both the Makefile hacks and the
> IS_REACHABLE() hack and replace these with regular Kconfig dependencies
> that ensure each driver is only enabled when its dependencies are there,
> as we do in most other drivers. The main downside here is that there is no
> good way to have built-in gadget support on cdn3 along with USB=m. Fixing
> this part proper would require cleaning up the code to turn the 'common'
> parts into a library module that only gets called by the other drivers
> but does not interact with either host or gadget support itself.
>
> Another problem that is not solved by this patch is the way that
> platform specific glue logic in this driver relies on having
> a soc specific device as the parent of a generic child, instead of
> the specific driver just calling into a common helper module.
> This may be impossible to fix without breaking the DT bindings.
>
> Fixes: 6076388ca1ed ("usb: cdns3: Add USBSSP platform driver support")

Hi Arnd,

Thanks for fixing it, I am sorry for taking your effort debug it.

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> TBH, I would be more comfortable with reverting 6076388ca1ed altogether
> and asking for a new version with the proper fixups included along
> with more testing for the next merge window.

It depends on Greg, I am okay for both ways. If Greg reverts the patch,
I will do below improvements and adapts for most of your changes for v3
patch.

cdns-usb-common.ko is a libary, and no USB/GADGET dependency, could builds in.
├── core.o
└── drd.o

cdns3-host.ko -> depends on USB/XHCI（it is m when USB = m
cdns3.ko (gadget) -> depends on USB_GADGET
cdnsp.ko (gadget) -> depends on USB_GADGET
cdns3-plat.ko -> assign host_init/gadget_init function pointer

In that way, below combination could work:
cdns-usb-common=y
cdns3-host=m
cdns3(p)-gadget=y

> @@ -10,12 +11,24 @@ config USB_CDNS_SUPPORT
>
> config USB_CDNS_HOST
> bool
> + depends on USB=y || USB=USB_CDNS_SUPPORT
> +
> +config CONFIG_USB_CDNS_PLATFORM

%s/CONFIG_USB_CDNS_PLATFORM/USB_CDNS_PLATFORM

Peter

> + tristate "Cadence USB3 generic platform support"
> + depends on USB_CDNSP || USB_CDNS3
> + depends on USB_CDNSP || !USB_CDNSP
> + depends on USB_CDNS3 || !USB_CDNS3
> + help
> + The platform driver support is needed on any SoC integrating
> + a variant of the Cadence USB3 or USBSSP dual-role controllers,
> + e.g. the TI K3 or NXP i.MX series of SoCs.
>
> if USB_CDNS_SUPPORT
>
> config USB_CDNS3
> tristate "Cadence USB3 Dual-Role Controller"
> depends on USB_CDNS_SUPPORT
> + select USB_CDNS_HOST if USB_CDNS3_HOST
> help
> Say Y here if your system has a Cadence USB3 dual-role controller.
> It supports: dual-role switch, Host-only, and Peripheral-only.
> @@ -23,8 +36,9 @@ config USB_CDNS3
> if USB_CDNS3
>
> config USB_CDNS3_GADGET
> - bool "Cadence USB3 device controller"
> + tristate "Cadence USB3 device controller"
> depends on USB_GADGET=y || USB_GADGET=USB_CDNS3
> + depends on USB_CDNS_SUPPORT
> help
> Say Y here to enable device controller functionality of the
> Cadence USBSS-DEV driver.
> @@ -34,8 +48,7 @@ config USB_CDNS3_GADGET
>
> config USB_CDNS3_HOST
> bool "Cadence USB3 host controller"
> - depends on USB=y || USB=USB_CDNS3
> - select USB_CDNS_HOST
> + depends on USB=y || USB=USB_CDNS_SUPPORT
> help
> Say Y here to enable host controller functionality of the
> Cadence driver.
> @@ -57,6 +70,7 @@ config USB_CDNS3_PCI_WRAP
> config USB_CDNS3_TI
> tristate "Cadence USB3 support on TI platforms"
> depends on ARCH_K3 || COMPILE_TEST
> + depends on USB_CDNS_PLATFORM
> default USB_CDNS3
> help
> Say 'Y' or 'M' here if you are building for Texas Instruments
> @@ -67,6 +81,7 @@ config USB_CDNS3_TI
> config USB_CDNS3_IMX
> tristate "Cadence USB3 support on NXP i.MX platforms"
> depends on ARCH_MXC || COMPILE_TEST
> + depends on USB_CDNS_PLATFORM
> default USB_CDNS3
> help
> Say 'Y' or 'M' here if you are building for NXP i.MX
> @@ -77,6 +92,7 @@ config USB_CDNS3_IMX
> config USB_CDNS3_STARFIVE
> tristate "Cadence USB3 support on StarFive SoC platforms"
> depends on ARCH_STARFIVE || COMPILE_TEST
> + depends on USB_CDNS_PLATFORM
> help
> Say 'Y' or 'M' here if you are building for StarFive SoCs
> platforms that contain Cadence USB3 controller core.
> @@ -91,6 +107,7 @@ endif # USB_CDNS3
> config USB_CDNSP
> tristate "Cadence USBSSP Dual-Role Controller"
> depends on USB_CDNS_SUPPORT
> + select USB_CDNS_HOST if USB_CDNSP_HOST
> help
> Say Y here if your system has a Cadence USBSSP dual-role controller.
> It supports: dual-role switch, Host-only, and Peripheral-only.
> @@ -115,8 +132,7 @@ config USB_CDNSP_GADGET
>
> config USB_CDNSP_HOST
> bool "Cadence USBSSP host controller"
> - depends on USB=y || USB=USB_CDNSP
> - select USB_CDNS_HOST
> + depends on USB=y || USB=USB_CDNS_SUPPORT
> help
> Say Y here to enable host controller functionality of the
> Cadence driver.
> diff --git a/drivers/usb/cdns3/Makefile b/drivers/usb/cdns3/Makefile
> index 63484f145bb9..f1e00ab3d729 100644
> --- a/drivers/usb/cdns3/Makefile
> +++ b/drivers/usb/cdns3/Makefile
> @@ -5,32 +5,25 @@ CFLAGS_cdnsp-trace.o := -I$(src)
>
> cdns-usb-common-y := core.o drd.o
>
> -ifeq ($(CONFIG_USB),m)
> -obj-m += cdns-usb-common.o
> -obj-m += cdns3-plat.o
> -else
> obj-$(CONFIG_USB_CDNS_SUPPORT) += cdns-usb-common.o
> -obj-$(CONFIG_USB_CDNS_SUPPORT) += cdns3-plat.o
> -endif
> +
> +obj-$(CONFIG_USB_CDNS_PLATFORM) += cdns3-plat.o
>
> cdns-usb-common-$(CONFIG_USB_CDNS_HOST) += host.o
>
> # For CDNS3 gadget
> -ifneq ($(CONFIG_USB_CDNS3_GADGET),)
> cdns3-y := cdns3-gadget.o cdns3-ep0.o
> cdns3-$(CONFIG_TRACING) += cdns3-trace.o
> -obj-$(CONFIG_USB_CDNS3) += cdns3.o
> -endif
> +obj-$(CONFIG_USB_CDNS3_GADGET) += cdns3.o
> +
> obj-$(CONFIG_USB_CDNS3_PCI_WRAP) += cdns3-pci-wrap.o
> obj-$(CONFIG_USB_CDNS3_TI) += cdns3-ti.o
> obj-$(CONFIG_USB_CDNS3_IMX) += cdns3-imx.o
> obj-$(CONFIG_USB_CDNS3_STARFIVE) += cdns3-starfive.o
>
> # For CDNSP gadget
> -ifneq ($(CONFIG_USB_CDNSP_GADGET),)
> cdnsp-y := cdnsp-ring.o cdnsp-gadget.o \
> cdnsp-mem.o cdnsp-ep0.o
> cdnsp-$(CONFIG_TRACING) += cdnsp-trace.o
> -obj-$(CONFIG_USB_CDNSP) += cdnsp.o
> -endif
> +obj-$(CONFIG_USB_CDNSP_GADGET) += cdnsp.o
> obj-$(CONFIG_USB_CDNSP_PCI) += cdnsp-pci.o
> diff --git a/drivers/usb/cdns3/gadget-export.h b/drivers/usb/cdns3/gadget-export.h
> index 0cb600e2b5d2..c37b6269b001 100644
> --- a/drivers/usb/cdns3/gadget-export.h
> +++ b/drivers/usb/cdns3/gadget-export.h
> @@ -10,7 +10,7 @@
> #ifndef __LINUX_CDNS3_GADGET_EXPORT
> #define __LINUX_CDNS3_GADGET_EXPORT
>
> -#if defined(CONFIG_USB_CDNSP_GADGET) && IS_REACHABLE(CONFIG_USB_CDNSP)
> +#if IS_ENABLED(CONFIG_USB_CDNSP_GADGET)
>
> int cdnsp_gadget_init(struct cdns *cdns);
> #else
> @@ -22,7 +22,7 @@ static inline int cdnsp_gadget_init(struct cdns *cdns)
>
> #endif /* CONFIG_USB_CDNSP_GADGET */
>
> -#if defined(CONFIG_USB_CDNS3_GADGET) && IS_REACHABLE(CONFIG_USB_CDNS3)
> +#if IS_ENABLED(CONFIG_USB_CDNS3_GADGET)
>
> int cdns3_gadget_init(struct cdns *cdns);
> #else
> --
> 2.39.5
>

--

Best regards,
Peter