[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 6/9] pci.c: factor out while(bus) bus->next loop
From: |
Isaku Yamahata |
Subject: |
Re: [Qemu-devel] [PATCH 6/9] pci.c: factor out while(bus) bus->next loop logic into pci_find_bus_from(). |
Date: |
Thu, 1 Oct 2009 12:29:34 +0900 |
User-agent: |
Mutt/1.5.6i |
On Wed, Sep 30, 2009 at 01:45:06PM +0200, Michael S. Tsirkin wrote:
> On Wed, Jul 15, 2009 at 08:15:06PM +0900, Isaku Yamahata wrote:
> > factor out while(bus) bus->next loop logic into pci_find_bus_from()
> > which will be used later.
> >
> > Signed-off-by: Isaku Yamahata <address@hidden>
> > ---
> > hw/pci.c | 26 ++++++++++++++------------
> > 1 files changed, 14 insertions(+), 12 deletions(-)
> >
> > diff --git a/hw/pci.c b/hw/pci.c
> > index 9639a32..f06e1da 100644
> > --- a/hw/pci.c
> > +++ b/hw/pci.c
> > @@ -574,6 +574,16 @@ void pci_default_write_config(PCIDevice *d, uint32_t
> > addr, uint32_t val, int l)
> > pci_update_mappings(d);
> > }
> >
> > +static PCIBus *pci_find_bus_from(PCIBus *from, int bus_num)
>
> Why what is "from"? pci_find_parent_bus a better name?
Its intention is to go down from a given parent bus
to find child bus of a given bus number, bus_num.
The current implementation arranges PCIBus in a single linked list,
and a single linked list doesn't represent pci bus topology well.
So it may find a pci bus which isn't child of a given bus.
So far it hasn't been a issue because only a single bus PCI.0 is supported.
Given that several people including me want multiple PCI bus,
your question arises that the linked list should be changed/enhanced to
some kind of tree structure to represent PCI bus topology accurately.
Does it sound a good idea?
> > +{
> > + PCIBus *s = from;
> > +
> > + while (s && s->bus_num != bus_num)
>
> No space after while.
>
> > + s = s->next;
> > +
> > + return s;
> > +}
> > +
> > void pci_data_write(void *opaque, uint32_t addr, uint32_t val, int len)
> > {
> > PCIBus *s = opaque;
> > @@ -585,8 +595,7 @@ void pci_data_write(void *opaque, uint32_t addr,
> > uint32_t val, int len)
> > addr, val, len);
> > #endif
> > bus_num = (addr >> 16) & 0xff;
> > - while (s && s->bus_num != bus_num)
> > - s = s->next;
> > + s = pci_find_bus_from(s, bus_num);
> > if (!s)
> > return;
> > pci_dev = s->devices[(addr >> 8) & 0xff];
> > @@ -607,8 +616,7 @@ uint32_t pci_data_read(void *opaque, uint32_t addr, int
> > len)
> > uint32_t val;
> >
> > bus_num = (addr >> 16) & 0xff;
> > - while (s && s->bus_num != bus_num)
> > - s= s->next;
> > + s = pci_find_bus_from(s, bus_num);
> > if (!s)
> > goto fail;
> > pci_dev = s->devices[(addr >> 8) & 0xff];
> > @@ -792,8 +800,7 @@ void pci_for_each_device(int bus_num, void
> > (*fn)(PCIDevice *d))
> > PCIDevice *d;
> > int devfn;
> >
> > - while (bus && bus->bus_num != bus_num)
> > - bus = bus->next;
> > + bus = pci_find_bus_from(bus, bus_num);
> > if (bus) {
> > for(devfn = 0; devfn < 256; devfn++) {
> > d = bus->devices[devfn];
> > @@ -891,12 +898,7 @@ static void pci_bridge_write_config(PCIDevice *d,
> >
> > PCIBus *pci_find_bus(int bus_num)
> > {
> > - PCIBus *bus = first_bus;
> > -
> > - while (bus && bus->bus_num != bus_num)
> > - bus = bus->next;
> > -
> > - return bus;
> > + return pci_find_bus_from(first_bus, bus_num);
> > }
> >
> > PCIDevice *pci_find_device(int bus_num, int slot, int function)
>
--
yamahata