Re: [SeaBIOS] [PATCH v5 1/3] pci: refactor pci_find_capapibilty to get bdf as the first argument instead of the whole pci_device

[Aleksandr Bezzubikov]

>From the previous versions:

Reviewed-by: Marcel Apfelbaum 

2017-08-11 2:21 GMT+03:00 Aleksandr Bezzubikov :
> Refactor pci_find_capability function to get bdf instead of
> a whole pci_device* as the only necessary field for this function
> is still bdf.
>
> Signed-off-by: Aleksandr Bezzubikov 
> ---
>  src/fw/pciinit.c|  4 ++--
>  src/hw/pci.c| 25 +
>  src/hw/pci.h|  1 +
>  src/hw/pcidevice.c  | 24 
>  src/hw/pcidevice.h  |  1 -
>  src/hw/virtio-pci.c |  6 +++---
>  6 files changed, 31 insertions(+), 30 deletions(-)
>
> diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
> index 08221e6..864954f 100644
> --- a/src/fw/pciinit.c
> +++ b/src/fw/pciinit.c
> @@ -762,7 +762,7 @@ static int pci_bus_hotplug_support(struct pci_bus *bus, 
> u8 pcie_cap)
>  return downstream_port && slot_implemented;
>  }
>
> -shpc_cap = pci_find_capability(bus->bus_dev, PCI_CAP_ID_SHPC, 0);
> +shpc_cap = pci_find_capability(bus->bus_dev->bdf, PCI_CAP_ID_SHPC, 0);
>  return !!shpc_cap;
>  }
>
> @@ -844,7 +844,7 @@ static int pci_bios_check_devices(struct pci_bus *busses)
>   */
>  parent = [0];
>  int type;
> -u8 pcie_cap = pci_find_capability(s->bus_dev, PCI_CAP_ID_EXP, 0);
> +u8 pcie_cap = pci_find_capability(s->bus_dev->bdf, PCI_CAP_ID_EXP, 
> 0);
>  int hotplug_support = pci_bus_hotplug_support(s, pcie_cap);
>  for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) {
>  u64 align = (type == PCI_REGION_TYPE_IO) ?
> diff --git a/src/hw/pci.c b/src/hw/pci.c
> index 8e3d617..50d9d2d 100644
> --- a/src/hw/pci.c
> +++ b/src/hw/pci.c
> @@ -58,6 +58,30 @@ pci_config_maskw(u16 bdf, u32 addr, u16 off, u16 on)
>  pci_config_writew(bdf, addr, val);
>  }
>
> +u8 pci_find_capability(u16 bdf, u8 cap_id, u8 cap)
> +{
> +int i;
> +u16 status = pci_config_readw(bdf, PCI_STATUS);
> +
> +if (!(status & PCI_STATUS_CAP_LIST))
> +return 0;
> +
> +if (cap == 0) {
> +/* find first */
> +cap = pci_config_readb(bdf, PCI_CAPABILITY_LIST);
> +} else {
> +/* find next */
> +cap = pci_config_readb(bdf, cap + PCI_CAP_LIST_NEXT);
> +}
> +for (i = 0; cap && i <= 0xff; i++) {
> +if (pci_config_readb(bdf, cap + PCI_CAP_LIST_ID) == cap_id)
> +return cap;
> +cap = pci_config_readb(bdf, cap + PCI_CAP_LIST_NEXT);
> +}
> +
> +return 0;
> +}
> +
>  // Helper function for foreachbdf() macro - return next device
>  int
>  pci_next(int bdf, int bus)
> @@ -107,3 +131,4 @@ pci_reboot(void)
>  outb(v|6, PORT_PCI_REBOOT); /* Actually do the reset */
>  udelay(50);
>  }
> +
> diff --git a/src/hw/pci.h b/src/hw/pci.h
> index ee6e196..2e30e28 100644
> --- a/src/hw/pci.h
> +++ b/src/hw/pci.h
> @@ -39,6 +39,7 @@ u32 pci_config_readl(u16 bdf, u32 addr);
>  u16 pci_config_readw(u16 bdf, u32 addr);
>  u8 pci_config_readb(u16 bdf, u32 addr);
>  void pci_config_maskw(u16 bdf, u32 addr, u16 off, u16 on);
> +u8 pci_find_capability(u16 bdf, u8 cap_id, u8 cap);
>  int pci_next(int bdf, int bus);
>  int pci_probe_host(void);
>  void pci_reboot(void);
> diff --git a/src/hw/pcidevice.c b/src/hw/pcidevice.c
> index cfebf66..8853cf7 100644
> --- a/src/hw/pcidevice.c
> +++ b/src/hw/pcidevice.c
> @@ -134,30 +134,6 @@ pci_find_init_device(const struct pci_device_id *ids, 
> void *arg)
>  return NULL;
>  }
>
> -u8 pci_find_capability(struct pci_device *pci, u8 cap_id, u8 cap)
> -{
> -int i;
> -u16 status = pci_config_readw(pci->bdf, PCI_STATUS);
> -
> -if (!(status & PCI_STATUS_CAP_LIST))
> -return 0;
> -
> -if (cap == 0) {
> -/* find first */
> -cap = pci_config_readb(pci->bdf, PCI_CAPABILITY_LIST);
> -} else {
> -/* find next */
> -cap = pci_config_readb(pci->bdf, cap + PCI_CAP_LIST_NEXT);
> -}
> -for (i = 0; cap && i <= 0xff; i++) {
> -if (pci_config_readb(pci->bdf, cap + PCI_CAP_LIST_ID) == cap_id)
> -return cap;
> -cap = pci_config_readb(pci->bdf, cap + PCI_CAP_LIST_NEXT);
> -}
> -
> -return 0;
> -}
> -
>  // Enable PCI bus-mastering (ie, DMA) support on a pci device
>  void
>  pci_enable_busmaster(struct pci_device *pci)
> diff --git a/src/hw/pcidevice.h b/src/hw/pcidevice.h
> index 354b549..225d545 100644
> --- a/src/hw/pcidevice.h
> +++ b/src/hw/pcidevice.h
> @@ -69,7 +69,6 @@ int pci_init_device(const struct pci_device_id *ids
>  , struct pci_device *pci, void *arg);
>  struct pci_device *pci_find_init_device(const struct pci_device_id *ids
>  , void *arg);
> -u8 pci_find_capability(struct pci_device *pci, u8 cap_id, u8 cap);
>  void pci_enable_busmaster(struct pci_device *pci);
>  u16 pci_enable_iobar(struct pci_device *pci, u32 addr);
>  void *pci_enable_membar(struct pci_device *pci, u32 

[SeaBIOS] [PATCH v5 1/3] pci: refactor pci_find_capapibilty to get bdf as the first argument instead of the whole pci_device

[Aleksandr Bezzubikov]

Refactor pci_find_capability function to get bdf instead of
a whole pci_device* as the only necessary field for this function
is still bdf.

Signed-off-by: Aleksandr Bezzubikov 
---
 src/fw/pciinit.c|  4 ++--
 src/hw/pci.c| 25 +
 src/hw/pci.h|  1 +
 src/hw/pcidevice.c  | 24 
 src/hw/pcidevice.h  |  1 -
 src/hw/virtio-pci.c |  6 +++---
 6 files changed, 31 insertions(+), 30 deletions(-)

diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
index 08221e6..864954f 100644
--- a/src/fw/pciinit.c
+++ b/src/fw/pciinit.c
@@ -762,7 +762,7 @@ static int pci_bus_hotplug_support(struct pci_bus *bus, u8 
pcie_cap)
 return downstream_port && slot_implemented;
 }
 
-shpc_cap = pci_find_capability(bus->bus_dev, PCI_CAP_ID_SHPC, 0);
+shpc_cap = pci_find_capability(bus->bus_dev->bdf, PCI_CAP_ID_SHPC, 0);
 return !!shpc_cap;
 }
 
@@ -844,7 +844,7 @@ static int pci_bios_check_devices(struct pci_bus *busses)
  */
 parent = [0];
 int type;
-u8 pcie_cap = pci_find_capability(s->bus_dev, PCI_CAP_ID_EXP, 0);
+u8 pcie_cap = pci_find_capability(s->bus_dev->bdf, PCI_CAP_ID_EXP, 0);
 int hotplug_support = pci_bus_hotplug_support(s, pcie_cap);
 for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) {
 u64 align = (type == PCI_REGION_TYPE_IO) ?
diff --git a/src/hw/pci.c b/src/hw/pci.c
index 8e3d617..50d9d2d 100644
--- a/src/hw/pci.c
+++ b/src/hw/pci.c
@@ -58,6 +58,30 @@ pci_config_maskw(u16 bdf, u32 addr, u16 off, u16 on)
 pci_config_writew(bdf, addr, val);
 }
 
+u8 pci_find_capability(u16 bdf, u8 cap_id, u8 cap)
+{
+int i;
+u16 status = pci_config_readw(bdf, PCI_STATUS);
+
+if (!(status & PCI_STATUS_CAP_LIST))
+return 0;
+
+if (cap == 0) {
+/* find first */
+cap = pci_config_readb(bdf, PCI_CAPABILITY_LIST);
+} else {
+/* find next */
+cap = pci_config_readb(bdf, cap + PCI_CAP_LIST_NEXT);
+}
+for (i = 0; cap && i <= 0xff; i++) {
+if (pci_config_readb(bdf, cap + PCI_CAP_LIST_ID) == cap_id)
+return cap;
+cap = pci_config_readb(bdf, cap + PCI_CAP_LIST_NEXT);
+}
+
+return 0;
+}
+
 // Helper function for foreachbdf() macro - return next device
 int
 pci_next(int bdf, int bus)
@@ -107,3 +131,4 @@ pci_reboot(void)
 outb(v|6, PORT_PCI_REBOOT); /* Actually do the reset */
 udelay(50);
 }
+
diff --git a/src/hw/pci.h b/src/hw/pci.h
index ee6e196..2e30e28 100644
--- a/src/hw/pci.h
+++ b/src/hw/pci.h
@@ -39,6 +39,7 @@ u32 pci_config_readl(u16 bdf, u32 addr);
 u16 pci_config_readw(u16 bdf, u32 addr);
 u8 pci_config_readb(u16 bdf, u32 addr);
 void pci_config_maskw(u16 bdf, u32 addr, u16 off, u16 on);
+u8 pci_find_capability(u16 bdf, u8 cap_id, u8 cap);
 int pci_next(int bdf, int bus);
 int pci_probe_host(void);
 void pci_reboot(void);
diff --git a/src/hw/pcidevice.c b/src/hw/pcidevice.c
index cfebf66..8853cf7 100644
--- a/src/hw/pcidevice.c
+++ b/src/hw/pcidevice.c
@@ -134,30 +134,6 @@ pci_find_init_device(const struct pci_device_id *ids, void 
*arg)
 return NULL;
 }
 
-u8 pci_find_capability(struct pci_device *pci, u8 cap_id, u8 cap)
-{
-int i;
-u16 status = pci_config_readw(pci->bdf, PCI_STATUS);
-
-if (!(status & PCI_STATUS_CAP_LIST))
-return 0;
-
-if (cap == 0) {
-/* find first */
-cap = pci_config_readb(pci->bdf, PCI_CAPABILITY_LIST);
-} else {
-/* find next */
-cap = pci_config_readb(pci->bdf, cap + PCI_CAP_LIST_NEXT);
-}
-for (i = 0; cap && i <= 0xff; i++) {
-if (pci_config_readb(pci->bdf, cap + PCI_CAP_LIST_ID) == cap_id)
-return cap;
-cap = pci_config_readb(pci->bdf, cap + PCI_CAP_LIST_NEXT);
-}
-
-return 0;
-}
-
 // Enable PCI bus-mastering (ie, DMA) support on a pci device
 void
 pci_enable_busmaster(struct pci_device *pci)
diff --git a/src/hw/pcidevice.h b/src/hw/pcidevice.h
index 354b549..225d545 100644
--- a/src/hw/pcidevice.h
+++ b/src/hw/pcidevice.h
@@ -69,7 +69,6 @@ int pci_init_device(const struct pci_device_id *ids
 , struct pci_device *pci, void *arg);
 struct pci_device *pci_find_init_device(const struct pci_device_id *ids
 , void *arg);
-u8 pci_find_capability(struct pci_device *pci, u8 cap_id, u8 cap);
 void pci_enable_busmaster(struct pci_device *pci);
 u16 pci_enable_iobar(struct pci_device *pci, u32 addr);
 void *pci_enable_membar(struct pci_device *pci, u32 addr);
diff --git a/src/hw/virtio-pci.c b/src/hw/virtio-pci.c
index e5c2c33..96f9c6b 100644
--- a/src/hw/virtio-pci.c
+++ b/src/hw/virtio-pci.c
@@ -19,7 +19,7 @@
 #include "malloc.h" // free
 #include "output.h" // dprintf
 #include "pci.h" // pci_config_readl
-#include "pcidevice.h" // pci_find_capability
+#include "pcidevice.h" // struct pci_device
 #include "pci_regs.h" // PCI_BASE_ADDRESS_0
 #include