On Wed, 2019-08-14 at 14:37 +0200, Cédric Le Goater wrote: > On 14/08/2019 09:14, Rashmica Gupta wrote:
... > > +static void aspeed_2600_gpio_realize(DeviceState *dev, Error > > **errp) > > +{ > > + AspeedGPIOState *s = ASPEED_GPIO(dev); > > + AspeedGPIOState *s_1_8, *s_3_6; > > + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); > > + AspeedGPIOClass *agc, *agc2; > > + int pin; > > + void *obj; > > + > > + /* Create and setup the 1.8V gpio state/class */ > > + obj = object_new(TYPE_ASPEED_GPIO "-ast2600"); > > + s_1_8 = ASPEED_GPIO(obj); > > + object_property_add_child(OBJECT(dev), TYPE_ASPEED_GPIO "- > > ast2600-1.8v", > > + obj, errp); > > + if (error_abort) { > > + error_propagate(errp, error_abort); > > + } > > This looks wrong. > > Shouldn't we instantiate 2 GPIOs devices under the AST2600 SoC with > different > types ? > We were trying to keep one GPIO device with two states, one state for 3.6V gpios and one for the 1.8V gpios. As you can see I couldn't find a nice way to do that. I'll have a go at just making two seperate devices. > C. > > > > + agc = ASPEED_GPIO_GET_CLASS(s_1_8); > > + agc->ctrl = (void *)&aspeed_gpio_ast2600_1_8v_controller; > > + aspeed_gpio_init(obj); > > + > > + /* Create and setup the 3.6V gpio state/class */ > > + obj = object_new(TYPE_ASPEED_GPIO "-ast2600"); > > + s_3_6 = ASPEED_GPIO(obj); > > + object_property_add_child(OBJECT(dev), TYPE_ASPEED_GPIO "- > > ast2600-3.6v", > > + obj, errp); > > + if (error_abort) { > > + error_propagate(errp, error_abort); > > + } > > + agc2 = ASPEED_GPIO_GET_CLASS(s_3_6); > > + agc2->ctrl = (void *)&aspeed_gpio_ast2600_3_6v_controller; > > + aspeed_gpio_init(obj); > > + > > + for (pin = 0; pin < agc->ctrl->nr_gpio_pins; pin++) { > > + sysbus_init_irq(sbd, &s->gpios[pin]); > > + } > > + > > + memory_region_init_io(&s_3_6->iomem, OBJECT(s_3_6), > > &aspeed_gpio_ops, s_3_6, > > + TYPE_ASPEED_GPIO, GPIO_3_6V_MEM_SIZE + > > GPIO_1_8V_MEM_SIZE); > > + s_3_6->lookup = aspeed_3_6v_gpios; > > + > > + memory_region_init_io(&s_1_8->iomem, OBJECT(s_1_8), > > &aspeed_gpio_ops, s_1_8, > > + TYPE_ASPEED_GPIO, GPIO_1_8V_MEM_SIZE); > > + memory_region_add_subregion(&s_3_6->iomem, > > GPIO_1_8V_REG_OFFSET, > > + &s_1_8->iomem); > > + s_1_8->lookup = aspeed_1_8v_gpios; > > + > > + sysbus_init_mmio(sbd, &s_3_6->iomem); > > + sysbus_init_mmio(sbd, &s_1_8->iomem); > > +} > > + > > static const VMStateDescription vmstate_gpio_regs = { > > .name = TYPE_ASPEED_GPIO"/regs", > > .version_id = 1, > > @@ -830,6 +991,16 @@ static const VMStateDescription > > vmstate_aspeed_gpio = { > > } > > }; > > > > +static void aspeed_gpio_ast2600_class_init(ObjectClass *klass, > > void *data) > > +{ > > + DeviceClass *dc = DEVICE_CLASS(klass); > > + > > + dc->realize = aspeed_2600_gpio_realize; > > + dc->reset = aspeed_gpio_reset; > > + dc->desc = "Aspeed GPIO Controller"; > > + dc->vmsd = &vmstate_aspeed_gpio; > > +} > > + > > static void aspeed_gpio_class_init(ObjectClass *klass, void *data) > > { > > DeviceClass *dc = DEVICE_CLASS(klass); > > @@ -866,11 +1037,18 @@ static const TypeInfo > > aspeed_gpio_ast2500_info = { > > .class_data = (void *)&aspeed_gpio_ast2500_controller, > > }; > > > > +static const TypeInfo aspeed_gpio_ast2600_info = { > > + .name = TYPE_ASPEED_GPIO "-ast2600", > > + .parent = TYPE_ASPEED_GPIO, > > + .class_init = aspeed_gpio_ast2600_class_init, > > +}; > > + > > static void aspeed_gpio_register_types(void) > > { > > type_register_static(&aspeed_gpio_info); > > type_register_static(&aspeed_gpio_ast2400_info); > > type_register_static(&aspeed_gpio_ast2500_info); > > + type_register_static(&aspeed_gpio_ast2600_info); > > } > > > > type_init(aspeed_gpio_register_types); > > diff --git a/slirp b/slirp > > index f0da672620..126c04acba 160000 > > --- a/slirp > > +++ b/slirp > > @@ -1 +1 @@ > > -Subproject commit f0da6726207b740f6101028b2992f918477a4b08 > > +Subproject commit 126c04acbabd7ad32c2b018fe10dfac2a3bc1210 > >