Re: [PATCH v2 04/15] PCI: Add pci_find_vsec_capability() to find a specific VSEC

[Lukas Wunner]

On Wed, Feb 03, 2021 at 09:11:03AM +0000, Gustavo Pimentel wrote:
> On Wed, Feb 3, 2021 at 7:51:3, Lukas Wunner <lukas@xxxxxxxxx> wrote:
> > On Wed, Feb 03, 2021 at 01:54:49AM +0000, Gustavo Pimentel wrote:
> > > On Tue, Feb 2, 2021 at 18:8:55, Lukas Wunner <lukas@xxxxxxxxx> wrote:
> > > > As the name implies, the capability is "vendor-specific", so it is
> > > > perfectly possible that two vendors use the same VSEC ID for different
> > > > things.
> > > > 
> > > > To make sure you're looking for the right capability, you need to pass
> > > > a u16 vendor into this function and bail out if dev->vendor is
> > > > different.
> > > 
> > > This function will be called by the driver that will pass the correct 
> > > device which will be already pointing to the config space associated with
> > > the endpoint for instance. Because the driver is already attached to the 
> > > endpoint through the vendor ID and device ID specified, there is no need 
> > > to do that validation, it will be redundant.
> > 
> > Okay.  Please amend the kernel-doc to make it explicit that it's the
> > caller's responsibility to check the vendor ID.
> 
> I don't think that would be necessary, as I said, the 'struct pci_dev *' 
> already points exclusively for the device' config space, which contains 
> all the capabilities for that particular device by his turn will be 
> attached to a specific driver by the Vendor and Device IDs to a specific 
> driver, that will know, firstly search for the specific device vendor ID, 
>  and then secondly how to decode it, and thirdly to do something with it.

The helper you're adding may not only be called from drivers but also
from generic PCI code (such as set_pcie_thunderbolt()).  In that case
the vendor ID is arbitrary.  Also, it doesn't *hurt* documenting this
requirement, does it?

Thanks,

Lukas