Re: [Qemu-devel] Re: [PATCH 0/10] qdev patches.

[Gerd Hoffmann <kraxel@redhat.com>]

On 06/19/09 19:51, Paul Brook wrote:
> * qdev: update pci device registration
>
> I dislike passing an {array,length} pair. Especially when it requires every
> user to manually get the right length.

qemu has a ARRAY_SIZE macro which can be used like this:
(from uhci patch):

     pci_qdev_register(uhci_info, ARRAY_SIZE(uhci_info));

to get the right length, so I don't see this as a problem.

I can create pci_qdev_register_{single,array} macros to hide the length 
parameter.  I can also just drop the length argument and just use 
multiple calls in the (few) places where multiple drivers are registered 
at once.  What do you prefer?

> * qdev/core: bus list
>
> I don't seen any good reason for this. In fact I think it is a major step
> backwards. A bus is uniquely identified by its name and parent device.

For a static device tree you don't need that indeed.  I've added the 
patch recently while hacking usb, the reason is hotplugging devices. 
When adding devices to busses using monitor commands you'll need some 
way to specify the bus and to find it.  This gets the job done without 
having to walk the whole device tree.

> * qdev/pci: misc fixes.
>
> All uses of the second argument to savevm should go away, not introduce new
> ones.

This just maintains current behavior (PCI busses are numbered starting 
zero).

> I'm unconvinced by the dev->name change. If we're using the same value
> then why does it exist at all?

It is redundant indeed.  I've considered dropping it right away, then 
decided to better wait with that step until all PCI drivers are 
converted to qdev.

> * qdev/pci: hook up i440fx
>
> i440fx_init should not exist. c.f. versatile_pci.c

Indeed.  That is the long-term plan, I'm just not there yet.

> * qdev: update pci device registration

Guess this was supposed to say "qdev: convert piix-ide." ?

> This is exactly the sort of fake conversion that I don't like, because you
> still require use of the old hardcoded initialization functions.

I'd prefer to call them "incomplete" instead of "fake".  I know that 
more work needs to be done to get the drivers into shape for a dt-driven 
machine creation.

> Convenience wrappers like smc91c111_init are fine

Yes, that is the plan for all drivers.  So the old, hardcoded way of 
doing things keeps working while we are busy making the dt-driven 
machine creation work.

I'm just not there yet in all cases.  Basically every old *_init() call 
which has more than a single $bus_create_simple(...) call needs more work.

cheers,
   Gerd