Re: [PATCH 1/8] pci: Define pci_bus_dev_fn type

[Peter Xu]

On Thu, Oct 21, 2021 at 01:15:03PM +0200, Eric Auger wrote:
> Hi Peter,
> 
> On 10/21/21 12:42 PM, Peter Xu wrote:
> > It's used in quite a few places of pci.c and also in the rest of the code 
> > base.
> > Define such a hook so that it doesn't need to be defined all over the 
> > places.
> >
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >  hw/pci/pci.c         | 14 ++++----------
> >  include/hw/pci/pci.h |  7 ++++---
> >  2 files changed, 8 insertions(+), 13 deletions(-)
> >
> > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > index 186758ee11..1ab2b78321 100644
> > --- a/hw/pci/pci.c
> > +++ b/hw/pci/pci.c
> > @@ -1655,9 +1655,7 @@ static const pci_class_desc pci_class_descriptions[] =
> >  };
> >  
> >  static void pci_for_each_device_under_bus_reverse(PCIBus *bus,
> > -                                                  void (*fn)(PCIBus *b,
> > -                                                             PCIDevice *d,
> > -                                                             void *opaque),
> > +                                                  pci_bus_dev_fn fn,
> >                                                    void *opaque)
> >  {
> >      PCIDevice *d;
> > @@ -1672,8 +1670,7 @@ static void 
> > pci_for_each_device_under_bus_reverse(PCIBus *bus,
> >  }
> >  
> >  void pci_for_each_device_reverse(PCIBus *bus, int bus_num,
> > -                         void (*fn)(PCIBus *b, PCIDevice *d, void *opaque),
> > -                         void *opaque)
> > +                                 pci_bus_dev_fn fn, void *opaque)
> >  {
> >      bus = pci_find_bus_nr(bus, bus_num);
> >  
> > @@ -1683,9 +1680,7 @@ void pci_for_each_device_reverse(PCIBus *bus, int 
> > bus_num,
> >  }
> >  
> >  static void pci_for_each_device_under_bus(PCIBus *bus,
> > -                                          void (*fn)(PCIBus *b, PCIDevice 
> > *d,
> > -                                                     void *opaque),
> > -                                          void *opaque)
> > +                                          pci_bus_dev_fn fn, void *opaque)
> >  {
> >      PCIDevice *d;
> >      int devfn;
> > @@ -1699,8 +1694,7 @@ static void pci_for_each_device_under_bus(PCIBus *bus,
> >  }
> >  
> >  void pci_for_each_device(PCIBus *bus, int bus_num,
> > -                         void (*fn)(PCIBus *b, PCIDevice *d, void *opaque),
> > -                         void *opaque)
> > +                         pci_bus_dev_fn fn, void *opaque)
> >  {
> >      bus = pci_find_bus_nr(bus, bus_num);
> >  
> > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> > index 7fc90132cf..8e2d80860b 100644
> > --- a/include/hw/pci/pci.h
> > +++ b/include/hw/pci/pci.h
> > @@ -401,6 +401,8 @@ typedef PCIINTxRoute (*pci_route_irq_fn)(void *opaque, 
> > int pin);
> >  OBJECT_DECLARE_TYPE(PCIBus, PCIBusClass, PCI_BUS)
> >  #define TYPE_PCIE_BUS "PCIE"
> >  
> > +typedef void (*pci_bus_dev_fn)(PCIBus *b, PCIDevice *d, void *opaque);
> > +
> >  bool pci_bus_is_express(PCIBus *bus);
> >  
> >  void pci_root_bus_init(PCIBus *bus, size_t bus_size, DeviceState *parent,
> > @@ -458,11 +460,10 @@ static inline int pci_dev_bus_num(const PCIDevice 
> > *dev)
> >  
> >  int pci_bus_numa_node(PCIBus *bus);
> >  void pci_for_each_device(PCIBus *bus, int bus_num,
> > -                         void (*fn)(PCIBus *bus, PCIDevice *d, void 
> > *opaque),
> > +                         pci_bus_dev_fn fn,
> >                           void *opaque);
> >  void pci_for_each_device_reverse(PCIBus *bus, int bus_num,
> > -                                 void (*fn)(PCIBus *bus, PCIDevice *d,
> > -                                            void *opaque),
> > +                                 pci_bus_dev_fn fn,
> >                                   void *opaque);
> >  void pci_for_each_bus_depth_first(PCIBus *bus,
> >                                    void *(*begin)(PCIBus *bus, void 
> > *parent_state),
> There is another candidate in
> hw/ppc/pegasos2.c:    void (*dtf)(PCIBus *bus, PCIDevice *d, FDTInfo *fi);
> but this may be coverted later by the maintainer of this file.

That one has the last parameter a specific type, rather than "void *".

> 
> Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks!

-- 
Peter Xu