On Fri, Feb 11, 2022 at 01:32:42PM -0600, Mario Limonciello wrote:
> The `is_thunderbolt` attribute is currently a dumping ground for a
> variety of things.
It's not as arbitrary as it may seem. Quite a bit of thought went into
the current design.
> Instead use the driver core removable attribute to indicate the
> detail a device is attached to a thunderbolt or USB4 chain.
You're missing the point that "is_thunderbolt" is set on the *controller*
(i.e. its upstream and downstream ports).
The controller itself is *not* removable if it's the host controller.
However a device can be assumed to be removable if it has an ancestor
which has the "is_thunderbolt" flag set.
> static void pci_set_removable(struct pci_dev *dev)
> {
> struct pci_dev *parent = pci_upstream_bridge(dev);
> + u16 vsec;
> +
> + /* Is the device a Thunderbolt controller? */
> + vsec = pci_find_vsec_capability(dev, PCI_VENDOR_ID_INTEL, PCI_VSEC_ID_INTEL_TBT);
This doesn't make any sense because the host controller is not
removable.
> @@ -1860,7 +1855,6 @@ int pci_setup_device(struct pci_dev *dev)
> dev->cfg_size = pci_cfg_space_size(dev);
>
> /* Need to have dev->cfg_size ready */
> - set_pcie_thunderbolt(dev);
>
> set_pcie_untrusted(dev);
Either drop the blank line or drop the code comment if set_pcie_untrusted()
doesn't need dev->cfg_size.
> diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c
> index 57553f9b4d1d..04232fbc7d56 100644
> --- a/drivers/platform/x86/apple-gmux.c
> +++ b/drivers/platform/x86/apple-gmux.c
> @@ -596,7 +596,7 @@ static int gmux_resume(struct device *dev)
>
> static int is_thunderbolt(struct device *dev, void *data)
> {
> - return to_pci_dev(dev)->is_thunderbolt;
> + return pci_is_thunderbolt_attached(to_pci_dev(dev));
> }
No, the gmux driver changes its behavior if a Thunderbolt host
controller is present. Not if there's a Thunderbolt-attached
device present.
Thanks,
Lukas
