Message ID | 22b9e550138022caa0368f12c12c55f1675420a5.1403588925.git.alistair.francis@xilinx.com |
---|---|
State | New |
Headers | show |
On Tue, Jun 24, 2014 at 4:06 PM, Alistair Francis <alistair.francis@xilinx.com> wrote: > SysBusDevice::init is deprecated. Convert to Object::init and > Device::realize as prescribed by QOM conventions. > > Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> CC Peter for target-arm. Regards, Peter > --- > > hw/char/cadence_uart.c | 29 ++++++++++++++++------------- > 1 files changed, 16 insertions(+), 13 deletions(-) > > diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c > index bf0c853..5a22a72 100644 > --- a/hw/char/cadence_uart.c > +++ b/hw/char/cadence_uart.c > @@ -468,27 +468,30 @@ static void cadence_uart_reset(DeviceState *dev) > uart_update_status(s); > } > > -static int cadence_uart_init(SysBusDevice *dev) > +static void candence_uart_realize(DeviceState *dev, Error **errp) > { > UartState *s = CADENCE_UART(dev); > > - memory_region_init_io(&s->iomem, OBJECT(s), &uart_ops, s, "uart", 0x1000); > - sysbus_init_mmio(dev, &s->iomem); > - sysbus_init_irq(dev, &s->irq); > - > - s->fifo_trigger_handle = timer_new_ns(QEMU_CLOCK_VIRTUAL, > - (QEMUTimerCB *)fifo_trigger_update, s); > - > - s->char_tx_time = (get_ticks_per_sec() / 9600) * 10; > - > s->chr = qemu_char_get_next_serial(); > > if (s->chr) { > qemu_chr_add_handlers(s->chr, uart_can_receive, uart_receive, > uart_event, s); > } > +} > > - return 0; > +static void cadence_uart_init(Object *obj) > +{ > + UartState *s = CADENCE_UART(obj); > + > + memory_region_init_io(&s->iomem, obj, &uart_ops, s, "uart", 0x1000); > + sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem); > + sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq); > + > + s->fifo_trigger_handle = timer_new_ns(QEMU_CLOCK_VIRTUAL, > + (QEMUTimerCB *)fifo_trigger_update, s); > + > + s->char_tx_time = (get_ticks_per_sec() / 9600) * 10; > } > > static int cadence_uart_post_load(void *opaque, int version_id) > @@ -520,9 +523,8 @@ static const VMStateDescription vmstate_cadence_uart = { > static void cadence_uart_class_init(ObjectClass *klass, void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > - SysBusDeviceClass *sdc = SYS_BUS_DEVICE_CLASS(klass); > > - sdc->init = cadence_uart_init; > + dc->realize = candence_uart_realize; > dc->vmsd = &vmstate_cadence_uart; > dc->reset = cadence_uart_reset; > } > @@ -531,6 +533,7 @@ static const TypeInfo cadence_uart_info = { > .name = TYPE_CADENCE_UART, > .parent = TYPE_SYS_BUS_DEVICE, > .instance_size = sizeof(UartState), > + .instance_init = cadence_uart_init, > .class_init = cadence_uart_class_init, > }; > > -- > 1.7.1 > >
On 27 June 2014 01:11, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote: > On Tue, Jun 24, 2014 at 4:06 PM, Alistair Francis > <alistair.francis@xilinx.com> wrote: >> SysBusDevice::init is deprecated. Convert to Object::init and >> Device::realize as prescribed by QOM conventions. >> >> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> > > Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> > > CC Peter for target-arm. I think at this point given we're quite close to hardfreeze I'd prefer not to take this, since it's just cleanup. thanks -- PMM
On 24 June 2014 07:06, Alistair Francis <alistair.francis@xilinx.com> wrote: > SysBusDevice::init is deprecated. Convert to Object::init and > Device::realize as prescribed by QOM conventions. > > Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> > --- > > hw/char/cadence_uart.c | 29 ++++++++++++++++------------- > 1 files changed, 16 insertions(+), 13 deletions(-) > > diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c > index bf0c853..5a22a72 100644 > --- a/hw/char/cadence_uart.c > +++ b/hw/char/cadence_uart.c > @@ -468,27 +468,30 @@ static void cadence_uart_reset(DeviceState *dev) > uart_update_status(s); > } > > -static int cadence_uart_init(SysBusDevice *dev) > +static void candence_uart_realize(DeviceState *dev, Error **errp) Typo in your new function name :-) Otherwise Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
Am 03.07.2014 18:46, schrieb Peter Maydell: > On 24 June 2014 07:06, Alistair Francis <alistair.francis@xilinx.com> wrote: >> SysBusDevice::init is deprecated. Convert to Object::init and Note that there is no Object::init, only TypeInfo::instance_init. >> Device::realize as prescribed by QOM conventions. >> >> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> >> --- >> >> hw/char/cadence_uart.c | 29 ++++++++++++++++------------- >> 1 files changed, 16 insertions(+), 13 deletions(-) >> >> diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c >> index bf0c853..5a22a72 100644 >> --- a/hw/char/cadence_uart.c >> +++ b/hw/char/cadence_uart.c >> @@ -468,27 +468,30 @@ static void cadence_uart_reset(DeviceState *dev) >> uart_update_status(s); >> } >> >> -static int cadence_uart_init(SysBusDevice *dev) >> +static void candence_uart_realize(DeviceState *dev, Error **errp) > > Typo in your new function name :-) > > Otherwise > Reviewed-by: Peter Maydell <peter.maydell@linaro.org> LGTM, but I wonder whether the work Alex is doing on SysBusDevice requires us to introduce SysBusDevice::realize, called from Device::realize in SysBusDevice code? Regards, Andreas
On Fri, Jul 4, 2014 at 2:59 AM, Andreas Färber <afaerber@suse.de> wrote: > Am 03.07.2014 18:46, schrieb Peter Maydell: >> On 24 June 2014 07:06, Alistair Francis <alistair.francis@xilinx.com> wrote: >>> SysBusDevice::init is deprecated. Convert to Object::init and > > Note that there is no Object::init, only TypeInfo::instance_init. > So I've been using these C++ish ::s in commentry liberally without worrying about their literal correctness. TypeInfo requires a bit of QOM internal understanding to get the point, whearas a much wider audience has awarness of the Object type and the fact that is has an init. If anything I would just soften to plain english - "object init". >>> Device::realize as prescribed by QOM conventions. >>> >>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> >>> --- >>> >>> hw/char/cadence_uart.c | 29 ++++++++++++++++------------- >>> 1 files changed, 16 insertions(+), 13 deletions(-) >>> >>> diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c >>> index bf0c853..5a22a72 100644 >>> --- a/hw/char/cadence_uart.c >>> +++ b/hw/char/cadence_uart.c >>> @@ -468,27 +468,30 @@ static void cadence_uart_reset(DeviceState *dev) >>> uart_update_status(s); >>> } >>> >>> -static int cadence_uart_init(SysBusDevice *dev) >>> +static void candence_uart_realize(DeviceState *dev, Error **errp) >> >> Typo in your new function name :-) >> Will fix v2 (i'll respin this one) >> Otherwise >> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> > Thanks, Regards, Peter > LGTM, but I wonder whether the work Alex is doing on SysBusDevice > requires us to introduce SysBusDevice::realize, called from > Device::realize in SysBusDevice code? > > Regards, > Andreas > > -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg >
On 03.07.14 18:59, Andreas Färber wrote: > Am 03.07.2014 18:46, schrieb Peter Maydell: >> On 24 June 2014 07:06, Alistair Francis <alistair.francis@xilinx.com> wrote: >>> SysBusDevice::init is deprecated. Convert to Object::init and > Note that there is no Object::init, only TypeInfo::instance_init. > >>> Device::realize as prescribed by QOM conventions. >>> >>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> >>> --- >>> >>> hw/char/cadence_uart.c | 29 ++++++++++++++++------------- >>> 1 files changed, 16 insertions(+), 13 deletions(-) >>> >>> diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c >>> index bf0c853..5a22a72 100644 >>> --- a/hw/char/cadence_uart.c >>> +++ b/hw/char/cadence_uart.c >>> @@ -468,27 +468,30 @@ static void cadence_uart_reset(DeviceState *dev) >>> uart_update_status(s); >>> } >>> >>> -static int cadence_uart_init(SysBusDevice *dev) >>> +static void candence_uart_realize(DeviceState *dev, Error **errp) >> Typo in your new function name :-) >> >> Otherwise >> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> > LGTM, but I wonder whether the work Alex is doing on SysBusDevice > requires us to introduce SysBusDevice::realize, called from > Device::realize in SysBusDevice code? I don't think we need a realize function with my patch set - all work happens either during the creation phase (generate hint properties) or in the machine, after realize has successfully passed. Alex
On Fri, Jul 4, 2014 at 9:17 AM, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote: > On Fri, Jul 4, 2014 at 2:59 AM, Andreas Färber <afaerber@suse.de> wrote: >> Am 03.07.2014 18:46, schrieb Peter Maydell: >>> On 24 June 2014 07:06, Alistair Francis <alistair.francis@xilinx.com> wrote: >>>> SysBusDevice::init is deprecated. Convert to Object::init and >> >> Note that there is no Object::init, only TypeInfo::instance_init. >> > > So I've been using these C++ish ::s in commentry liberally without > worrying about their literal correctness. TypeInfo requires a bit of > QOM internal understanding to get the point, whearas a much wider > audience has awarness of the Object type and the fact that is has an > init. If anything I would just soften to plain english - "object > init". > >>>> Device::realize as prescribed by QOM conventions. >>>> >>>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> >>>> --- >>>> >>>> hw/char/cadence_uart.c | 29 ++++++++++++++++------------- >>>> 1 files changed, 16 insertions(+), 13 deletions(-) >>>> >>>> diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c >>>> index bf0c853..5a22a72 100644 >>>> --- a/hw/char/cadence_uart.c >>>> +++ b/hw/char/cadence_uart.c >>>> @@ -468,27 +468,30 @@ static void cadence_uart_reset(DeviceState *dev) >>>> uart_update_status(s); >>>> } >>>> >>>> -static int cadence_uart_init(SysBusDevice *dev) >>>> +static void candence_uart_realize(DeviceState *dev, Error **errp) >>> >>> Typo in your new function name :-) >>> > > Will fix v2 (i'll respin this one) What ever happened to this? I can fix the typo and resend > >>> Otherwise >>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> >> > > Thanks, > > Regards, > Peter > >> LGTM, but I wonder whether the work Alex is doing on SysBusDevice >> requires us to introduce SysBusDevice::realize, called from >> Device::realize in SysBusDevice code? >> >> Regards, >> Andreas >> >> -- >> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany >> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg >> >
diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c index bf0c853..5a22a72 100644 --- a/hw/char/cadence_uart.c +++ b/hw/char/cadence_uart.c @@ -468,27 +468,30 @@ static void cadence_uart_reset(DeviceState *dev) uart_update_status(s); } -static int cadence_uart_init(SysBusDevice *dev) +static void candence_uart_realize(DeviceState *dev, Error **errp) { UartState *s = CADENCE_UART(dev); - memory_region_init_io(&s->iomem, OBJECT(s), &uart_ops, s, "uart", 0x1000); - sysbus_init_mmio(dev, &s->iomem); - sysbus_init_irq(dev, &s->irq); - - s->fifo_trigger_handle = timer_new_ns(QEMU_CLOCK_VIRTUAL, - (QEMUTimerCB *)fifo_trigger_update, s); - - s->char_tx_time = (get_ticks_per_sec() / 9600) * 10; - s->chr = qemu_char_get_next_serial(); if (s->chr) { qemu_chr_add_handlers(s->chr, uart_can_receive, uart_receive, uart_event, s); } +} - return 0; +static void cadence_uart_init(Object *obj) +{ + UartState *s = CADENCE_UART(obj); + + memory_region_init_io(&s->iomem, obj, &uart_ops, s, "uart", 0x1000); + sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem); + sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq); + + s->fifo_trigger_handle = timer_new_ns(QEMU_CLOCK_VIRTUAL, + (QEMUTimerCB *)fifo_trigger_update, s); + + s->char_tx_time = (get_ticks_per_sec() / 9600) * 10; } static int cadence_uart_post_load(void *opaque, int version_id) @@ -520,9 +523,8 @@ static const VMStateDescription vmstate_cadence_uart = { static void cadence_uart_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); - SysBusDeviceClass *sdc = SYS_BUS_DEVICE_CLASS(klass); - sdc->init = cadence_uart_init; + dc->realize = candence_uart_realize; dc->vmsd = &vmstate_cadence_uart; dc->reset = cadence_uart_reset; } @@ -531,6 +533,7 @@ static const TypeInfo cadence_uart_info = { .name = TYPE_CADENCE_UART, .parent = TYPE_SYS_BUS_DEVICE, .instance_size = sizeof(UartState), + .instance_init = cadence_uart_init, .class_init = cadence_uart_class_init, };
SysBusDevice::init is deprecated. Convert to Object::init and Device::realize as prescribed by QOM conventions. Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> --- hw/char/cadence_uart.c | 29 ++++++++++++++++------------- 1 files changed, 16 insertions(+), 13 deletions(-)