Attaching the full diff
On Mon, Sep 07, 2020 at 01:09:12PM -0500, Jordan Hargrave wrote: > On Thu, Sep 03, 2020 at 08:37:56PM +0200, Mark Kettenis wrote: > > > Date: Wed, 2 Sep 2020 15:19:55 +1000 > > > From: Jonathan Gray <[email protected]> > > > > > > On Tue, Sep 01, 2020 at 11:44:03PM -0500, Jordan Hargrave wrote: > > > > This patch adds a common function for scanning PCIE Express Capability > > > > list > > > > The PCIE Capability list starts at 0x100 in extended PCI configuration > > > > space. > > > > > > This seems to only handle extended capabilities? > > > Something like pcie_get_ext_capability() would be a better name. > > > > I think it should be pci_get_ext_capability() which follows the > > pattern set by pci_get_ht_capability(). > > > > > > > > It is 'PCI Express' not 'PCIExpress' > > > > > > 'ofs & 3' test doesn't make sense when PCI_PCIE_ECAP_NEXT() always > > > masks those bits. > > > > > > > > > > > --- > > > > sys/dev/pci/pci.c | 28 ++++++++++++++++++++++++++++ > > > > sys/dev/pci/pcivar.h | 2 ++ > > > > 2 files changed, 30 insertions(+) > > > > > > > > diff --git a/sys/dev/pci/pci.c b/sys/dev/pci/pci.c > > > > index bf75f875e..8f9a5ef7a 100644 > > > > --- a/sys/dev/pci/pci.c > > > > +++ b/sys/dev/pci/pci.c > > > > @@ -677,6 +677,34 @@ pci_get_ht_capability(pci_chipset_tag_t pc, > > > > pcitag_t tag, int capid, > > > > return (0); > > > > } > > > > > > > > +int > > > > +pcie_get_capability(pci_chipset_tag_t pc, pcitag_t tag, int capid, > > > > + int *offset, pcireg_t *value) > > > > +{ > > > > + pcireg_t reg; > > > > + unsigned int ofs; > > > > + > > > > + /* Make sure we support PCIExpress device */ > > > > PCI Express like jsg@ already mentioned. Add a full stop at the end > > of the sentence. > > > > > > + if (pci_get_capability(pc, tag, PCI_CAP_PCIEXPRESS, NULL, NULL) > > > > == 0) > > > > + return (0); > > > > + /* Scan PCIExpress capabilities */ > > > > Drop this comment and replace with a blank line such that it matches > > pci_get_ht_capability(). > > > > > > + ofs = PCI_PCIE_ECAP; > > > > + while (ofs != 0) { > > > > + if ((ofs & 3) || (ofs < PCI_PCIE_ECAP)) > > > > + return (0); > > > > Make this check #ifdef DIAGNOSTIC like pci_get_ht_capability() doesn. > > Dropping the (ofs & 3) bit is indeed a good idea. > > > > > > + reg = pci_conf_read(pc, tag, ofs); > > > > + if (PCI_PCIE_ECAP_ID(reg) == capid) { > > > > + if (offset) > > > > + *offset = ofs; > > > > + if (value) > > > > + *value = reg; > > > > + return (1); > > > > + } > > > > + ofs = PCI_PCIE_ECAP_NEXT(reg); > > > > + } > > > > Blank line here please. > > > > > > + return (0); > > > > +} > > > > + > > > > uint16_t > > > > pci_requester_id(pci_chipset_tag_t pc, pcitag_t tag) > > > > { > > > > diff --git a/sys/dev/pci/pcivar.h b/sys/dev/pci/pcivar.h > > > > index bdfe0404f..0376ba992 100644 > > > > --- a/sys/dev/pci/pcivar.h > > > > +++ b/sys/dev/pci/pcivar.h > > > > @@ -233,6 +233,8 @@ int pci_io_find(pci_chipset_tag_t, pcitag_t, int, > > > > bus_addr_t *, > > > > int pci_mem_find(pci_chipset_tag_t, pcitag_t, int, bus_addr_t *, > > > > bus_size_t *, int *); > > > > > > > > +int pcie_get_capability(pci_chipset_tag_t, pcitag_t, int, > > > > + int *, pcireg_t *); > > > > int pci_get_capability(pci_chipset_tag_t, pcitag_t, int, > > > > int *, pcireg_t *); > > > > int pci_get_ht_capability(pci_chipset_tag_t, pcitag_t, int, > > > > -- > > > > 2.26.2 > > > > > > > > > > > > > > > diff --git a/sys/dev/pci/pci.c b/sys/dev/pci/pci.c > index 8f9a5ef7a..f649b3e79 100644 > --- a/sys/dev/pci/pci.c > +++ b/sys/dev/pci/pci.c > @@ -678,20 +678,22 @@ pci_get_ht_capability(pci_chipset_tag_t pc, pcitag_t > tag, int capid, > } > > int > -pcie_get_capability(pci_chipset_tag_t pc, pcitag_t tag, int capid, > +pci_get_ext_capability(pci_chipset_tag_t pc, pcitag_t tag, int capid, > int *offset, pcireg_t *value) > { > pcireg_t reg; > unsigned int ofs; > > - /* Make sure we support PCIExpress device */ > + /* Make sure we support PCI Express device */ > if (pci_get_capability(pc, tag, PCI_CAP_PCIEXPRESS, NULL, NULL) == 0) > return (0); > - /* Scan PCIExpress capabilities */ > + /* Scan PCI Express capabilities */ > ofs = PCI_PCIE_ECAP; > while (ofs != 0) { > +#ifdef DIAGNOSTIC > if ((ofs & 3) || (ofs < PCI_PCIE_ECAP)) > return (0); > +#endif > reg = pci_conf_read(pc, tag, ofs); > if (PCI_PCIE_ECAP_ID(reg) == capid) { > if (offset) > @@ -702,6 +704,7 @@ pcie_get_capability(pci_chipset_tag_t pc, pcitag_t tag, > int capid, > } > ofs = PCI_PCIE_ECAP_NEXT(reg); > } > + > return (0); > } > > diff --git a/sys/dev/pci/pcivar.h b/sys/dev/pci/pcivar.h > index 0376ba992..1a19996f5 100644 > --- a/sys/dev/pci/pcivar.h > +++ b/sys/dev/pci/pcivar.h > @@ -233,12 +233,12 @@ int pci_io_find(pci_chipset_tag_t, pcitag_t, int, > bus_addr_t *, > int pci_mem_find(pci_chipset_tag_t, pcitag_t, int, bus_addr_t *, > bus_size_t *, int *); > > -int pcie_get_capability(pci_chipset_tag_t, pcitag_t, int, > - int *, pcireg_t *); > int pci_get_capability(pci_chipset_tag_t, pcitag_t, int, > int *, pcireg_t *); > int pci_get_ht_capability(pci_chipset_tag_t, pcitag_t, int, > int *, pcireg_t *); > +int pci_get_ext_capability(pci_chipset_tag_t, pcitag_t, int, > + int *, pcireg_t *); > > struct msix_vector; > > >diff --git a/sys/dev/pci/pci.c b/sys/dev/pci/pci.c index bf75f875e..4e8d06389 100644 --- a/sys/dev/pci/pci.c +++ b/sys/dev/pci/pci.c @@ -677,6 +677,38 @@ pci_get_ht_capability(pci_chipset_tag_t pc, pcitag_t tag, int capid, return (0); } +int +pci_get_ext_capability(pci_chipset_tag_t pc, pcitag_t tag, int capid, + int *offset, pcireg_t *value) +{ + pcireg_t reg; + unsigned int ofs; + + /* Make sure we support PCI Express device */ + if (pci_get_capability(pc, tag, PCI_CAP_PCIEXPRESS, NULL, NULL) == 0) + return (0); + + /* Scan PCI Express capabilities */ + ofs = PCI_PCIE_ECAP; + while (ofs != 0) { +#ifdef DIAGNOSTIC + if ((ofs & 3) || (ofs < PCI_PCIE_ECAP)) + return (0); +#endif + reg = pci_conf_read(pc, tag, ofs); + if (PCI_PCIE_ECAP_ID(reg) == capid) { + if (offset) + *offset = ofs; + if (value) + *value = reg; + return (1); + } + ofs = PCI_PCIE_ECAP_NEXT(reg); + } + + return (0); +} + uint16_t pci_requester_id(pci_chipset_tag_t pc, pcitag_t tag) { diff --git a/sys/dev/pci/pcivar.h b/sys/dev/pci/pcivar.h index bdfe0404f..1a19996f5 100644 --- a/sys/dev/pci/pcivar.h +++ b/sys/dev/pci/pcivar.h @@ -237,6 +237,8 @@ int pci_get_capability(pci_chipset_tag_t, pcitag_t, int, int *, pcireg_t *); int pci_get_ht_capability(pci_chipset_tag_t, pcitag_t, int, int *, pcireg_t *); +int pci_get_ext_capability(pci_chipset_tag_t, pcitag_t, int, + int *, pcireg_t *); struct msix_vector;
