Re: [PATCH v3 04/12] PCI: Drop the `is_thunderbolt` attribute from PCI core

[Lukas Wunner <lukas@wunner.de>]

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