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@xxxxxxxxx>
Krzysztof
