> Subject: Re: [PATCH v3 2/2] fpga: dfl: look for vendor specific capability > > Hi Matthew, > > On Mon, Nov 30, 2020 at 04:45:20PM -0800, > matthew.gerlach@xxxxxxxxxxxxxxx wrote: > > > > > > On Sat, 28 Nov 2020, Wu, Hao wrote: > > > > > > Subject: [PATCH v3 2/2] fpga: dfl: look for vendor specific capability > > > > > > Maybe we can change the title a little bit, what about > > > fpga: dfl-pci: locate DFLs by PCIe vendor specific capability > > > > > > > > > > > From: Matthew Gerlach <matthew.gerlach@xxxxxxxxxxxxxxx> > > > > > > > > A DFL may not begin at offset 0 of BAR 0. A PCIe vendor > > > > specific capability can be used to specify the start of a > > > > number of DFLs. > > > > > > A PCIe vendor specific extended capability is introduced by Intel to > > > specify the start of a number of DFLs. > > > > Your suggestion is more precise. > > > > > > > > > > > > > > Signed-off-by: Matthew Gerlach <matthew.gerlach@xxxxxxxxxxxxxxx> > > > > --- > > > > v3: Add text and ascii art to documentation. > > > > Ensure not to exceed PCIe config space in loop. > > > > > > > > v2: Update documentation for clarity. > > > > Clean up macro names. > > > > Use GENMASK. > > > > Removed spurious blank lines. > > > > Changed some calls from dev_info to dev_dbg. > > > > Specifically check for VSEC not found, -ENODEV. > > > > Ensure correct pci vendor id. > > > > Remove check for page alignment. > > > > Rename find_dfl_in_cfg to find_dfls_by_vsec. > > > > Initialize target memory of pci_read_config_dword to invalid values > before > > > > use. > > > > --- > > > > Documentation/fpga/dfl.rst | 25 +++++++++++ > > > > drivers/fpga/dfl-pci.c | 91 > +++++++++++++++++++++++++++++++++++++- > > > > 2 files changed, 115 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/Documentation/fpga/dfl.rst b/Documentation/fpga/dfl.rst > > > > index 0404fe6ffc74..fa0da884a818 100644 > > > > --- a/Documentation/fpga/dfl.rst > > > > +++ b/Documentation/fpga/dfl.rst > > > > @@ -501,6 +501,31 @@ Developer only needs to provide a sub feature > > > > driver with matched feature id. > > > > FME Partial Reconfiguration Sub Feature driver (see drivers/fpga/dfl- > fme- > > > > pr.c) > > > > could be a reference. > > > > > > > > +Location of DFLs on a PCI Device > > > > +=========================== > > > > +There are two ways of locating DFLs on a PCI Device. The original > > > > > > I found this new VSEC is only for PCIe device, correct? If so, let's make > > > sure descriptions are accurate. E.g. default method for all devices > > > and a new method for PCIe device. > > > > Yes, the default method can be used with PCI and PCIe device, and the > VSEC > > approach is PCIe, only. Documentation can be made more precise. > > > > > > > > > +method assumed the start of the first DFL to offset 0 of bar 0. > > > > +If the first node of the DFL is an FME, then further DFLs > > > > +in the port(s) are specified in FME header registers. > > > > +Alternatively, a vendor specific capability structure can be used to > Maybe: a vendor specific extended capability (VSEC) ... > > > > +specify the location of all the DFLs on the device, providing flexibility > > > > +for the type of starting node in the DFL. Intel has reserved the > > > > +VSEC ID of 0x43 for this purpose. The vendor specific > > > > +data begins with a 4 byte vendor specific register for the number of > DFLs > > > > followed 4 byte > > > > +Offset/BIR vendor specific registers for each DFL. Bits 2:0 of Offset/BIR > > > > register > > > > > > Do we have a defined register name here? or it's named as Offset/BIR > register? > > > Sounds a little wired, and I see you defined it as DFLS_RES? > > > > The Offset/BIR terminology is also used in the MSI-X capability structure. > > Yeah, this intuitively made sense to me having worked with PCIe :) I just feel that it's better to use the same register name defined in the code below. So people can find matched information in both code and doc easily. : ) Thanks Hao
