RE: [PATCH v3 2/2] fpga: dfl: look for vendor specific capability

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On Wed, 2 Dec 2020, Wu, Hao wrote:

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. : )

I think this makes sense. I can change the name of the variable to bir to match the documentation.


Thanks
Hao




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux