Re: [Qemu-devel] [PATCH 07/22] gpio/puv3_gpio: Convert sysbus initfunction to realize function

[maozy]

On 11/19/18 10:31 PM, Peter Maydell wrote:
> On 19 November 2018 at 12:08, Mao Zhongyi
> <address@hidden> wrote:
> > Use DeviceClass rather than SysBusDeviceClass in
> > puv3_gpio_class_init().
> >
> > Cc: address@hidden
> >
> > Signed-off-by: Mao Zhongyi <address@hidden>
> > Signed-off-by: Zhang Shengju <address@hidden>
> > ---
> >   hw/gpio/puv3_gpio.c | 28 +++++++++++++---------------
> >   1 file changed, 13 insertions(+), 15 deletions(-)
> >
> > diff --git a/hw/gpio/puv3_gpio.c b/hw/gpio/puv3_gpio.c
> > index 445afccf9f..bd6fc43aae 100644
> > --- a/hw/gpio/puv3_gpio.c
> > +++ b/hw/gpio/puv3_gpio.c
> > @@ -99,7 +99,7 @@ static const MemoryRegionOps puv3_gpio_ops = {
> >       .endianness = DEVICE_NATIVE_ENDIAN,
> >   };
> >
> > -static int puv3_gpio_init(SysBusDevice *dev)
> > +static void puv3_gpio_realize(DeviceState *dev, Error **errp)
> >   {
> >       PUV3GPIOState *s = PUV3_GPIO(dev);
> >
> > @@ -107,28 +107,26 @@ static int puv3_gpio_init(SysBusDevice *dev)
> >       s->reg_GPDR = 0;
> >
> >       /* FIXME: these irqs not handled yet */
> > -    sysbus_init_irq(dev, &s->irq[PUV3_IRQS_GPIOLOW0]);
> > -    sysbus_init_irq(dev, &s->irq[PUV3_IRQS_GPIOLOW1]);
> > -    sysbus_init_irq(dev, &s->irq[PUV3_IRQS_GPIOLOW2]);
> > -    sysbus_init_irq(dev, &s->irq[PUV3_IRQS_GPIOLOW3]);
> > -    sysbus_init_irq(dev, &s->irq[PUV3_IRQS_GPIOLOW4]);
> > -    sysbus_init_irq(dev, &s->irq[PUV3_IRQS_GPIOLOW5]);
> > -    sysbus_init_irq(dev, &s->irq[PUV3_IRQS_GPIOLOW6]);
> > -    sysbus_init_irq(dev, &s->irq[PUV3_IRQS_GPIOLOW7]);
> > -    sysbus_init_irq(dev, &s->irq[PUV3_IRQS_GPIOHIGH]);
> > +    sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq[PUV3_IRQS_GPIOLOW0]);
> > +    sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq[PUV3_IRQS_GPIOLOW1]);
> > +    sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq[PUV3_IRQS_GPIOLOW2]);
> > +    sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq[PUV3_IRQS_GPIOLOW3]);
> > +    sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq[PUV3_IRQS_GPIOLOW4]);
> > +    sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq[PUV3_IRQS_GPIOLOW5]);
> > +    sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq[PUV3_IRQS_GPIOLOW6]);
> > +    sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq[PUV3_IRQS_GPIOLOW7]);
> > +    sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq[PUV3_IRQS_GPIOHIGH]);
> >
> >       memory_region_init_io(&s->iomem, OBJECT(s), &puv3_gpio_ops, s, 
> > "puv3_gpio",
> >               PUV3_REGS_OFFSET);
> > -    sysbus_init_mmio(dev, &s->iomem);
> > -
> > -    return 0;
> > +    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->iomem);
> >   }
>
> The SYS_BUS_DEVICE() cast is not free (it does type
> checking). It's better to do it once, ie
>     SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> and use the variable, if we're going to be using
> it several times.

Wow, sure，thanks a lot, will fix it in the next.

mao

> thanks
> -- PMM