lkml.org 
[lkml]   [2021]   [Nov]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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

    \
     
     \ /
      Last update: 2021-11-14 04:33    [W:3.208 / U:0.868 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site