Make the subject specific, not just "appropriately." I think you're marking something *removable*, so include that. On Fri, Feb 11, 2022 at 01:32:45PM -0600, Mario Limonciello wrote: > Discrete USB4 controllers won't have ACPI nodes specifying which > PCIe or XHCI port they are linked with. > > In order to set the removable attribute appropriately, use the > USB4 DVSEC extended capabability set on these root ports to determine > if they are located on a discrete USB4 controller. s/capabability/capability/ > Suggested-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx> > Link: https://usb.org/sites/default/files/USB4%20Specification%2020211116.zip > Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx> > --- > drivers/pci/probe.c | 33 +++++++++++++++++++++++++++++++++ > include/linux/pci_ids.h | 2 ++ > 2 files changed, 35 insertions(+) > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index 67ca33188cba..1ed3e24db11e 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -25,6 +25,8 @@ > #define CARDBUS_LATENCY_TIMER 176 /* secondary latency timer */ > #define CARDBUS_RESERVE_BUSNR 3 > > +#define PCI_DVSEC_ID_USB4 0x23 > + > static struct resource busn_resource = { > .name = "PCI busn", > .start = 0, > @@ -1590,6 +1592,36 @@ static void set_pcie_untrusted(struct pci_dev *dev) > dev->untrusted = true; > } > > +static bool pci_is_discrete_usb4(struct pci_dev *dev) > +{ > + int dvsec_val = 0, pos; > + u32 hdr; > + > + /* USB4 spec says vendors can use either */ > + pos = pci_find_dvsec_capability(dev, > + PCI_VENDOR_ID_INTEL, > + PCI_DVSEC_ID_USB4); > + if (pos) { > + dvsec_val = 0x06; > + } else { > + pos = pci_find_dvsec_capability(dev, > + PCI_VENDOR_ID_USB_IF, > + PCI_DVSEC_ID_USB4); > + if (pos) > + dvsec_val = 0x01; > + } > + if (!dvsec_val) > + return false; > + > + pci_read_config_dword(dev, pos + PCI_DVSEC_HEADER2, &hdr); > + if ((hdr & GENMASK(15, 0)) != dvsec_val) > + return false; > + /* this port is used for either NHI/PCIe tunnel/USB tunnel */ Capitalize comment like others in this file. Spec reference would be helpful, too. I don't know how to verify any of these values. The Link: above is great, but name, revision, section number would be even better. > + if (hdr & GENMASK(18, 16)) > + return true; > + return false; > +} > + > static void pci_set_removable(struct pci_dev *dev) > { > struct pci_dev *parent = pci_upstream_bridge(dev); > @@ -1612,6 +1644,7 @@ static void pci_set_removable(struct pci_dev *dev) > if (vsec || > dev->class == PCI_CLASS_SERIAL_USB_USB4 || > pci_acpi_is_usb4(dev) || > + pci_is_discrete_usb4(dev) || > (parent && > (parent->external_facing || dev_is_removable(&parent->dev)))) > dev_set_removable(&dev->dev, DEVICE_REMOVABLE); > diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h > index 61b161d914f0..271326e058b9 100644 > --- a/include/linux/pci_ids.h > +++ b/include/linux/pci_ids.h > @@ -3097,4 +3097,6 @@ > > #define PCI_VENDOR_ID_NCUBE 0x10ff > > +#define PCI_VENDOR_ID_USB_IF 0x1EC0 This file is supposed to be sorted by Vendor ID. PCI_VENDOR_ID_XEN, PCI_VENDOR_ID_OCZ, and PCI_VENDOR_ID_NCUBE screwed up, but you can put USB_IF in the correct spot. It's not 100%, but most entries use lower-case hex.
