From: Rashmica Gupta <rashmica.g@gmail.com>
To: "Cédric Le Goater" <clg@kaod.org>,
peter.maydell@linaro.org, qemu-arm@nongnu.org
Cc: andrew@aj.id.au, aik@ozlabs.ru, qemu-devel@nongnu.org, joel@jms.id.au
Subject: Re: [Qemu-devel] [PATCH v4 3/3] hw/gpio: Add in AST2600 specific implementation
Date: Fri, 16 Aug 2019 11:16:04 +1000 [thread overview]
Message-ID: <186994ccaaaa1c07015426ef2cc84bcefb411b09.camel@gmail.com> (raw)
In-Reply-To: <88221198-9699-6ec4-14f7-ee3c375a57e4@kaod.org>
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
> >
next prev parent reply other threads:[~2019-08-16 1:17 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-14 7:14 [Qemu-devel] [PATCH v4 0/3] Add Aspeed GPIO controller model Rashmica Gupta
2019-08-14 7:14 ` [Qemu-devel] [PATCH v4 1/3] hw/gpio: Add basic Aspeed GPIO model for AST2400 and AST2500 Rashmica Gupta
2019-08-14 12:27 ` Cédric Le Goater
2019-08-14 7:14 ` [Qemu-devel] [PATCH v4 2/3] aspeed: add a GPIO controller to the SoC Rashmica Gupta
2019-08-14 12:30 ` Cédric Le Goater
2019-08-14 7:14 ` [Qemu-devel] [PATCH v4 3/3] hw/gpio: Add in AST2600 specific implementation Rashmica Gupta
2019-08-14 12:37 ` Cédric Le Goater
2019-08-16 1:16 ` Rashmica Gupta [this message]
2019-08-14 13:47 ` [Qemu-devel] [PATCH v4 0/3] Add Aspeed GPIO controller model no-reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=186994ccaaaa1c07015426ef2cc84bcefb411b09.camel@gmail.com \
--to=rashmica.g@gmail.com \
--cc=aik@ozlabs.ru \
--cc=andrew@aj.id.au \
--cc=clg@kaod.org \
--cc=joel@jms.id.au \
--cc=peter.maydell@linaro.org \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).