Re: [PATCH] Add common PCIE capability list
> Date: Mon, 7 Sep 2020 13:33:14 -0500 > From: Jordan Hargrave > > Attaching the full diff ok kettenis@ > 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 > > > > > > > > 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)) > >
Re: [PATCH] Add common PCIE capability list
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 > > > > > > 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 *, > > > > intpci_mem_find(pci_chipset_tag_t, pcitag_t, int, bus_addr_t *, > > > > bus_size_t *, int *); > > > > > > > > +intpcie_get_capability(pci_chipset_tag_t, pcitag_t, int, > > > > + int *, pcireg_t *); > > > > intpci_get_capability(pci_chipset_tag_t, pcitag_t, int, > > > > int *, pcireg_t *); > > > > intpci_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, >
Re: [PATCH] Add common PCIE capability list
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 > > > > 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 *, intpci_mem_find(pci_chipset_tag_t, pcitag_t, int, bus_addr_t *, bus_size_t *, int *); -int
Re: [PATCH] Add common PCIE capability list
> Date: Wed, 2 Sep 2020 15:19:55 +1000 > From: Jonathan Gray > > 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 *, > > intpci_mem_find(pci_chipset_tag_t, pcitag_t, int, bus_addr_t *, > > bus_size_t *, int *); > > > > +intpcie_get_capability(pci_chipset_tag_t, pcitag_t, int, > > + int *, pcireg_t *); > > intpci_get_capability(pci_chipset_tag_t, pcitag_t, int, > > int *, pcireg_t *); > > intpci_get_ht_capability(pci_chipset_tag_t, pcitag_t, int, > > -- > > 2.26.2 > > > > > >
Re: [PATCH] Add common PCIE capability list
On Wed, Sep 02, 2020 at 03:19:55PM +1000, Jonathan Gray wrote: > 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. > > It is 'PCI Express' not 'PCIExpress' > > 'ofs & 3' test doesn't make sense when PCI_PCIE_ECAP_NEXT() always > masks those bits. > Ok fixed below... > > > > --- > > 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 */ > > + if (pci_get_capability(pc, tag, PCI_CAP_PCIEXPRESS, NULL, NULL) == 0) > > + return (0); > > + /* Scan PCIExpress capabilities */ > > + ofs = PCI_PCIE_ECAP; > > + while (ofs != 0) { > > + if ((ofs & 3) || (ofs < PCI_PCIE_ECAP)) > > + return (0); > > + 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..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 *, > > intpci_mem_find(pci_chipset_tag_t, pcitag_t, int, bus_addr_t *, > > bus_size_t *, int *); > > > > +intpcie_get_capability(pci_chipset_tag_t, pcitag_t, int, > > + int *, pcireg_t *); > > intpci_get_capability(pci_chipset_tag_t, pcitag_t, int, > > int *, pcireg_t *); > > intpci_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 bf75f875e..56a85453a 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 +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) { + if (ofs < PCI_PCIE_ECAP) + return (0); + 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 *); intpci_get_ht_capability(pci_chipset_tag_t, pcitag_t, int, int *, pcireg_t *); +intpci_get_ext_capability(pci_chipset_tag_t, pcitag_t, int, + int *, pcireg_t *); struct msix_vector;
Re: [PATCH] Add common PCIE capability list
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. 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 */ > + if (pci_get_capability(pc, tag, PCI_CAP_PCIEXPRESS, NULL, NULL) == 0) > + return (0); > + /* Scan PCIExpress capabilities */ > + ofs = PCI_PCIE_ECAP; > + while (ofs != 0) { > + if ((ofs & 3) || (ofs < PCI_PCIE_ECAP)) > + return (0); > + 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..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 > >
[PATCH] Add common PCIE capability list
This patch adds a common function for scanning PCIE Express Capability list The PCIE Capability list starts at 0x100 in extended PCI configuration space. --- 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 */ + if (pci_get_capability(pc, tag, PCI_CAP_PCIEXPRESS, NULL, NULL) == 0) + return (0); + /* Scan PCIExpress capabilities */ + ofs = PCI_PCIE_ECAP; + while (ofs != 0) { + if ((ofs & 3) || (ofs < PCI_PCIE_ECAP)) + return (0); + 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..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 *, intpci_mem_find(pci_chipset_tag_t, pcitag_t, int, bus_addr_t *, bus_size_t *, int *); +intpcie_get_capability(pci_chipset_tag_t, pcitag_t, int, + int *, pcireg_t *); intpci_get_capability(pci_chipset_tag_t, pcitag_t, int, int *, pcireg_t *); intpci_get_ht_capability(pci_chipset_tag_t, pcitag_t, int, -- 2.26.2