Re: [Qemu-devel] [PATCH 01/26] ohci: use realize for ohci

[Peter Crosthwaite]

Hi Hu,

On Mon, Jun 24, 2013 at 4:11 PM, Hu Tao <address@hidden> wrote:
> On Mon, Jun 24, 2013 at 03:54:31PM +1000, Peter Crosthwaite wrote:
>> Hi Hu,
>>
>> On Sat, Jun 22, 2013 at 6:50 PM, Hu Tao <address@hidden> wrote:
>> > Cc: Gerd Hoffmann <address@hidden>
>> > Signed-off-by: Hu Tao <address@hidden>
>> > ---
>> >  hw/usb/hcd-ohci.c | 16 +++++++---------
>> >  1 file changed, 7 insertions(+), 9 deletions(-)
>> >
>> > diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
>> > index 51241cd..79ef41b 100644
>> > --- a/hw/usb/hcd-ohci.c
>> > +++ b/hw/usb/hcd-ohci.c
>> > @@ -1876,17 +1876,16 @@ typedef struct {
>> >      dma_addr_t dma_offset;
>> >  } OHCISysBusState;
>> >
>> > -static int ohci_init_pxa(SysBusDevice *dev)
>> > +static void ohci_realize_pxa(DeviceState *dev, Error **errp)
>> >  {
>> > -    OHCISysBusState *s = FROM_SYSBUS(OHCISysBusState, dev);
>> > +    OHCISysBusState *s = DO_UPCAST(OHCISysBusState, busdev.qdev, dev);
>>
>> I don't think this is an improvement. Until a QOM cast macro is
>> available, FROM_SYSBUS is preferable to a DO_UPCAST I think?
>
> patch 2 introduces QOM macro and replaces DO_UPCAST. Instead, we can
> also first do QOM then realize. Which one do you prefer?
>

Other way round I think make more sense, as no need to have this ugly
hunk for transition sake.

Squashing is another low effort option, one patch that just does it
all makes sense to me (and ive done this a few times already with
various devices).

Regards,
Peter

>>
>> > +    SysBusDevice *b = SYS_BUS_DEVICE(dev);
>> >
>> >      /* Cannot fail as we pass NULL for masterbus */
>> > -    usb_ohci_init(&s->ohci, &dev->qdev, s->num_ports, s->dma_offset, 
>> > NULL, 0,
>> > +    usb_ohci_init(&s->ohci, dev, s->num_ports, s->dma_offset, NULL, 0,
>> >                    &dma_context_memory);
>>
>> Rebase required due to Paolos IOMMU patches going in removing
>> dma_context_memory.
>
> Thanks for reminding. I'll do a rebase anyway, patches involving i440fx
> and q35 may conflict with your `pci cleanup' series, and ehci patches
> duplicates Andreas's work.
>
>