Re: [PATCH v2 3/7] thunderbolt: Make PCIe NHI support opt-in

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] thunderbolt: missing default USB4 on CONFIG_USB4_PCI breaks
  existing configs
- [Low] thunderbolt: misplaced EXPORT_SYMBOL_GPL(tb_xdomain_remove) in
  xdomain.c
--

commit 20bc8cfb6a438440bc1447d0b676bf03b1cdb691
Author: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

thunderbolt: Make PCIe NHI support opt-in

This commit decouples PCIe NHI and ICM support from the core USB4/Thunderbolt
framework into a separate thunderbolt_pci module controlled by CONFIG_USB4_PCI.

> diff --git a/drivers/thunderbolt/Kconfig b/drivers/thunderbolt/Kconfig
> index 294b3227a5458..605af440457a1 100644
> --- a/drivers/thunderbolt/Kconfig
> +++ b/drivers/thunderbolt/Kconfig
> @@ -1,7 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  menuconfig USB4
>  	tristate "Unified support for USB4 and Thunderbolt"
> -	depends on PCI
>  	select APPLE_PROPERTIES if EFI_STUB && X86
>  	select CRC32
>  	select CRYPTO_LIB_SHA256
> @@ -18,6 +17,16 @@ menuconfig USB4
>  
>  if USB4
>  
> +config USB4_PCI
> +	depends on PCI
> +	tristate "Support for PCIe-attached USB4 and Thunderbolt controllers"
> +	help
> +	  Say Y here to add support for the USB4 and Thunderbolt host
> +	  routers attached over the PCIe bus, commonly found on x86 PCs.
> +
> +	  To compile this driver a module, choose M here. The module will be
> +	  called thunderbolt_pci.

[Severity: High]
Should USB4_PCI default to USB4?

Without a default, make olddefconfig on existing configs with CONFIG_USB4
enabled (as well as in-tree defconfigs such as loongson64_defconfig) will set
CONFIG_USB4_PCI=n and drop PCIe controller support.  On ACPI systems with a
PCIe host router, having CONFIG_USB4 enabled while CONFIG_USB4_PCI is disabled
still causes acpi_bus_osc_negotiate_platform_control() to claim native OS
USB4 control via _OSC (disabling the firmware Connection Manager) with no OS
NHI driver bound, and causes typec_has_usb4_host_interface() to wait for a
usb4_port component that is never registered.

[ ... ]

> diff --git a/drivers/thunderbolt/xdomain.c b/drivers/thunderbolt/xdomain.c
> index 16e818c1af1c0..d2dd6dec5b474 100644
> --- a/drivers/thunderbolt/xdomain.c
> +++ b/drivers/thunderbolt/xdomain.c

[ ... ]

> @@ -2270,6 +2273,7 @@ void tb_xdomain_unregister(struct tb_xdomain *xd)
>  	dev_info(&xd->dev, "host disconnected\n");
>  	device_unregister(&xd->dev);
>  }
> +EXPORT_SYMBOL_GPL(tb_xdomain_remove);

[Severity: Low]
EXPORT_SYMBOL_GPL(tb_xdomain_remove) is placed after tb_xdomain_unregister()
instead of immediately after tb_xdomain_remove().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110114925.722014-2-chunfeng.yun@mediatek.com?part=3