| Message ID | 20190328062222.8409-3-joel@jms.id.au |
|---|---|
| State | New |
| Headers | show |
| Series | arm: aspeed: Add RTC Model | expand |
On Thu, 28 Mar 2019 at 06:22, Joel Stanley <joel@jms.id.au> wrote: > > Signed-off-by: Joel Stanley <joel@jms.id.au> > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > hw/arm/aspeed_soc.c | 15 +++++++++++++++ > include/hw/arm/aspeed_soc.h | 2 ++ > 2 files changed, 17 insertions(+) > > diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c > index a27233d4876b..628ec633c91a 100644 > --- a/hw/arm/aspeed_soc.c > +++ b/hw/arm/aspeed_soc.c > @@ -32,6 +32,7 @@ > #define ASPEED_SOC_SDMC_BASE 0x1E6E0000 > #define ASPEED_SOC_SCU_BASE 0x1E6E2000 > #define ASPEED_SOC_SRAM_BASE 0x1E720000 > +#define ASPEED_SOC_RTC_BASE 0x1E781000 > #define ASPEED_SOC_TIMER_BASE 0x1E782000 > #define ASPEED_SOC_WDT_BASE 0x1E785000 > #define ASPEED_SOC_I2C_BASE 0x1E78A000 > @@ -135,6 +136,10 @@ static void aspeed_soc_init(Object *obj) > object_property_add_child(obj, "i2c", OBJECT(&s->i2c), NULL); > qdev_set_parent_bus(DEVICE(&s->i2c), sysbus_get_default()); > > + object_initialize(&s->rtc, sizeof(s->rtc), TYPE_ASPEED_RTC); > + object_property_add_child(obj, "rtc", OBJECT(&s->rtc), NULL); > + qdev_set_parent_bus(DEVICE(&s->rtc), sysbus_get_default()); These three lines should be written sysbus_init_child_obj(obj, "rtc", &s->rtc, sizeof(s->rtc), TYPE_ASPEED_RTC); which is both shorter and also avoids leaking a reference to the child object. (We should also at some point fix the existing uses of this pattern in this file.) thanks -- PMM
On 4/11/19 5:28 PM, Peter Maydell wrote: > On Thu, 28 Mar 2019 at 06:22, Joel Stanley <joel@jms.id.au> wrote: >> >> Signed-off-by: Joel Stanley <joel@jms.id.au> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> --- >> hw/arm/aspeed_soc.c | 15 +++++++++++++++ >> include/hw/arm/aspeed_soc.h | 2 ++ >> 2 files changed, 17 insertions(+) >> >> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c >> index a27233d4876b..628ec633c91a 100644 >> --- a/hw/arm/aspeed_soc.c >> +++ b/hw/arm/aspeed_soc.c >> @@ -32,6 +32,7 @@ >> #define ASPEED_SOC_SDMC_BASE 0x1E6E0000 >> #define ASPEED_SOC_SCU_BASE 0x1E6E2000 >> #define ASPEED_SOC_SRAM_BASE 0x1E720000 >> +#define ASPEED_SOC_RTC_BASE 0x1E781000 >> #define ASPEED_SOC_TIMER_BASE 0x1E782000 >> #define ASPEED_SOC_WDT_BASE 0x1E785000 >> #define ASPEED_SOC_I2C_BASE 0x1E78A000 >> @@ -135,6 +136,10 @@ static void aspeed_soc_init(Object *obj) >> object_property_add_child(obj, "i2c", OBJECT(&s->i2c), NULL); >> qdev_set_parent_bus(DEVICE(&s->i2c), sysbus_get_default()); >> >> + object_initialize(&s->rtc, sizeof(s->rtc), TYPE_ASPEED_RTC); >> + object_property_add_child(obj, "rtc", OBJECT(&s->rtc), NULL); >> + qdev_set_parent_bus(DEVICE(&s->rtc), sysbus_get_default()); > > These three lines should be written > sysbus_init_child_obj(obj, "rtc", &s->rtc, sizeof(s->rtc), > TYPE_ASPEED_RTC); > > which is both shorter and also avoids leaking a reference to > the child object. > > (We should also at some point fix the existing uses of this > pattern in this file.) yes. and there are a few. I will do that. Thanks, C.
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c index a27233d4876b..628ec633c91a 100644 --- a/hw/arm/aspeed_soc.c +++ b/hw/arm/aspeed_soc.c @@ -32,6 +32,7 @@ #define ASPEED_SOC_SDMC_BASE 0x1E6E0000 #define ASPEED_SOC_SCU_BASE 0x1E6E2000 #define ASPEED_SOC_SRAM_BASE 0x1E720000 +#define ASPEED_SOC_RTC_BASE 0x1E781000 #define ASPEED_SOC_TIMER_BASE 0x1E782000 #define ASPEED_SOC_WDT_BASE 0x1E785000 #define ASPEED_SOC_I2C_BASE 0x1E78A000 @@ -135,6 +136,10 @@ static void aspeed_soc_init(Object *obj) object_property_add_child(obj, "i2c", OBJECT(&s->i2c), NULL); qdev_set_parent_bus(DEVICE(&s->i2c), sysbus_get_default()); + object_initialize(&s->rtc, sizeof(s->rtc), TYPE_ASPEED_RTC); + object_property_add_child(obj, "rtc", OBJECT(&s->rtc), NULL); + qdev_set_parent_bus(DEVICE(&s->rtc), sysbus_get_default()); + object_initialize(&s->fmc, sizeof(s->fmc), sc->info->fmc_typename); object_property_add_child(obj, "fmc", OBJECT(&s->fmc), NULL); qdev_set_parent_bus(DEVICE(&s->fmc), sysbus_get_default()); @@ -231,6 +236,16 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp) sysbus_connect_irq(SYS_BUS_DEVICE(&s->timerctrl), i, irq); } + /* RTC */ + object_property_set_bool(OBJECT(&s->rtc), true, "realized", &err); + if (err) { + error_propagate(errp, err); + return; + } + sysbus_mmio_map(SYS_BUS_DEVICE(&s->rtc), 0, ASPEED_SOC_RTC_BASE); + sysbus_connect_irq(SYS_BUS_DEVICE(&s->rtc), 0, + qdev_get_gpio_in(DEVICE(&s->vic), 22)); + /* UART - attach an 8250 to the IO space as our UART5 */ if (serial_hd(0)) { qemu_irq uart5 = qdev_get_gpio_in(DEVICE(&s->vic), uart_irqs[4]); diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h index 11ec0179db50..a5f8f219e404 100644 --- a/include/hw/arm/aspeed_soc.h +++ b/include/hw/arm/aspeed_soc.h @@ -17,6 +17,7 @@ #include "hw/misc/aspeed_scu.h" #include "hw/misc/aspeed_sdmc.h" #include "hw/timer/aspeed_timer.h" +#include "hw/timer/aspeed_rtc.h" #include "hw/i2c/aspeed_i2c.h" #include "hw/ssi/aspeed_smc.h" #include "hw/watchdog/wdt_aspeed.h" @@ -33,6 +34,7 @@ typedef struct AspeedSoCState { ARMCPU cpu; MemoryRegion sram; AspeedVICState vic; + AspeedRtcState rtc; AspeedTimerCtrlState timerctrl; AspeedI2CState i2c; AspeedSCUState scu;