Message ID | d445aecddbb8eb8d2c6107c1c56e2e5421c12437.1371804804.git.hutao@cn.fujitsu.com |
---|---|
State | New |
Headers | show |
Hi Hu, On Sat, Jun 22, 2013 at 6:50 PM, Hu Tao <hutao@cn.fujitsu.com> wrote: > Cc: Gerd Hoffmann <kraxel@redhat.com> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> > --- > 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? > + 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. Regards, Peter > - sysbus_init_irq(dev, &s->ohci.irq); > - sysbus_init_mmio(dev, &s->ohci.mem); > - > - return 0; > + sysbus_init_irq(b, &s->ohci.irq); > + sysbus_init_mmio(b, &s->ohci.mem); > } > > static Property ohci_pci_properties[] = { > @@ -1926,9 +1925,8 @@ static Property ohci_sysbus_properties[] = { > static void ohci_sysbus_class_init(ObjectClass *klass, void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > - SysBusDeviceClass *sbc = SYS_BUS_DEVICE_CLASS(klass); > > - sbc->init = ohci_init_pxa; > + dc->realize = ohci_realize_pxa; > dc->desc = "OHCI USB Controller"; > dc->props = ohci_sysbus_properties; > } > -- > 1.8.3.1 > >
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 <hutao@cn.fujitsu.com> wrote: > > Cc: Gerd Hoffmann <kraxel@redhat.com> > > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> > > --- > > 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? > > > + 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.
Hi Hu, On Mon, Jun 24, 2013 at 4:11 PM, Hu Tao <hutao@cn.fujitsu.com> 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 <hutao@cn.fujitsu.com> wrote: >> > Cc: Gerd Hoffmann <kraxel@redhat.com> >> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> >> > --- >> > 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. > >
On Sat, Jun 22, 2013 at 04:50:13PM +0800, Hu Tao wrote: > Cc: Gerd Hoffmann <kraxel@redhat.com> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> > --- > 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); > + 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); This patch doesn't apply cleanly anymore, since "[PULL 00/25] Memory/IOMMU patches, part 3: IOMMU implementation" from Paolo was merged (commit 576156f) > - sysbus_init_irq(dev, &s->ohci.irq); > - sysbus_init_mmio(dev, &s->ohci.mem); > - > - return 0; > + sysbus_init_irq(b, &s->ohci.irq); > + sysbus_init_mmio(b, &s->ohci.mem); > } > > static Property ohci_pci_properties[] = { > @@ -1926,9 +1925,8 @@ static Property ohci_sysbus_properties[] = { > static void ohci_sysbus_class_init(ObjectClass *klass, void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > - SysBusDeviceClass *sbc = SYS_BUS_DEVICE_CLASS(klass); > > - sbc->init = ohci_init_pxa; > + dc->realize = ohci_realize_pxa; > dc->desc = "OHCI USB Controller"; > dc->props = ohci_sysbus_properties; > } > -- > 1.8.3.1 > >
On Mon, Jun 24, 2013 at 04:17:28PM +1000, Peter Crosthwaite wrote: > Hi Hu, > > On Mon, Jun 24, 2013 at 4:11 PM, Hu Tao <hutao@cn.fujitsu.com> 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 <hutao@cn.fujitsu.com> wrote: > >> > Cc: Gerd Hoffmann <kraxel@redhat.com> > >> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> > >> > --- > >> > 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. Agreed. > > 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). But I think it's easier to review to keep it in two steps. > > 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. > > > >
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); + 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); - sysbus_init_irq(dev, &s->ohci.irq); - sysbus_init_mmio(dev, &s->ohci.mem); - - return 0; + sysbus_init_irq(b, &s->ohci.irq); + sysbus_init_mmio(b, &s->ohci.mem); } static Property ohci_pci_properties[] = { @@ -1926,9 +1925,8 @@ static Property ohci_sysbus_properties[] = { static void ohci_sysbus_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); - SysBusDeviceClass *sbc = SYS_BUS_DEVICE_CLASS(klass); - sbc->init = ohci_init_pxa; + dc->realize = ohci_realize_pxa; dc->desc = "OHCI USB Controller"; dc->props = ohci_sysbus_properties; }
Cc: Gerd Hoffmann <kraxel@redhat.com> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> --- hw/usb/hcd-ohci.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)