Messages in this thread | | | Date | Sun, 14 Nov 2021 04:31:43 +0100 | From | Krzysztof Wilczyński <> | Subject | Re: [PATCH v1 1/1] PCI: probe: Use pci_find_vsec_capability() when looking for TBT devices |
| |
Hi Andy,
Nice find! There might one more driver that leverages the vendor-specific capabilities that seems to be also open coding pci_find_vsec_capability(), as per:
139-static int find_dfls_by_vsec(struct pci_dev *pcidev, struct dfl_fpga_enum_info *info) 140-{ 141- u32 bir, offset, vndr_hdr, dfl_cnt, dfl_res; 142- int dfl_res_off, i, bars, voff = 0; 143- resource_size_t start, len; 144- 145- while ((voff = pci_find_next_ext_capability(pcidev, voff, PCI_EXT_CAP_ID_VNDR))) { 146- vndr_hdr = 0; 147: pci_read_config_dword(pcidev, voff + PCI_VNDR_HEADER, &vndr_hdr); 148- 149: if (PCI_VNDR_HEADER_ID(vndr_hdr) == PCI_VSEC_ID_INTEL_DFLS && 150- pcidev->vendor == PCI_VENDOR_ID_INTEL) 151- break; 152- } 153- 154- if (!voff) { 155- dev_dbg(&pcidev->dev, "%s no DFL VSEC found\n", __func__); 156- return -ENODEV; 157- }
Do you think that it would be worthwhile to also update this other driver to use pci_find_vsec_capability() at the same time? I might be nice to rid of the other open coded implementation too.
> Currently the set_pcie_thunderbolt() opens code pci_find_vsec_capability().
I would write it as "open codes" in the above.
> static void set_pcie_thunderbolt(struct pci_dev *dev) > { > - int vsec = 0; > - u32 header; > + u16 vsec; > > - while ((vsec = pci_find_next_ext_capability(dev, vsec, > - PCI_EXT_CAP_ID_VNDR))) { > - pci_read_config_dword(dev, vsec + PCI_VNDR_HEADER, &header); > - > - /* Is the device part of a Thunderbolt controller? */ > - if (dev->vendor == PCI_VENDOR_ID_INTEL && > - PCI_VNDR_HEADER_ID(header) == PCI_VSEC_ID_INTEL_TBT) { > - dev->is_thunderbolt = 1; > - return; > - } > - } > + vsec = pci_find_vsec_capability(dev, PCI_VENDOR_ID_INTEL, PCI_VSEC_ID_INTEL_TBT); > + if (vsec) > + dev->is_thunderbolt = 1; > }
Thank you!
Reviewed-by: Krzysztof Wilczyński <kw@linux.com>
Krzysztof
| |