Initialize the region in the instance_init handler because we will
want to link this region in the FMC object before the WDT object is
realized.
Cc: Peter Delevoryas <pdel@fb.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
include/hw/watchdog/wdt_aspeed.h | 1 +
hw/watchdog/wdt_aspeed.c | 15 ++++++++++++---
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/include/hw/watchdog/wdt_aspeed.h b/include/hw/watchdog/wdt_aspeed.h
index f945cd6c5833..008e7d9b498c 100644
--- a/include/hw/watchdog/wdt_aspeed.h
+++ b/include/hw/watchdog/wdt_aspeed.h
@@ -29,6 +29,7 @@ struct AspeedWDTState {
/*< public >*/
MemoryRegion iomem;
+ MemoryRegion iomem_alias;
uint32_t regs[ASPEED_WDT_REGS_MAX];
AspeedSCUState *scu;
diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c
index 146ffcd71301..6426f3a77494 100644
--- a/hw/watchdog/wdt_aspeed.c
+++ b/hw/watchdog/wdt_aspeed.c
@@ -261,6 +261,16 @@ static void aspeed_wdt_timer_expired(void *dev)
#define PCLK_HZ 24000000
+static void aspeed_wdt_instance_init(Object *obj)
+{
+ AspeedWDTState *s = ASPEED_WDT(obj);
+
+ memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_wdt_ops, s,
+ TYPE_ASPEED_WDT, ASPEED_WDT_REGS_MAX * 4);
+ memory_region_init_alias(&s->iomem_alias, OBJECT(s),
+ TYPE_ASPEED_WDT ".alias",
+ &s->iomem, 0, ASPEED_WDT_REGS_MAX * 4);
+}
static void aspeed_wdt_realize(DeviceState *dev, Error **errp)
{
SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
@@ -275,9 +285,7 @@ static void aspeed_wdt_realize(DeviceState *dev, Error **errp)
*/
s->pclk_freq = PCLK_HZ;
- memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_wdt_ops, s,
- TYPE_ASPEED_WDT, ASPEED_WDT_REGS_MAX * 4);
- sysbus_init_mmio(sbd, &s->iomem);
+ sysbus_init_mmio(sbd, &s->iomem_alias);
}
static Property aspeed_wdt_properties[] = {
@@ -301,6 +309,7 @@ static void aspeed_wdt_class_init(ObjectClass *klass, void *data)
static const TypeInfo aspeed_wdt_info = {
.parent = TYPE_SYS_BUS_DEVICE,
.name = TYPE_ASPEED_WDT,
+ .instance_init = aspeed_wdt_instance_init,
.instance_size = sizeof(AspeedWDTState),
.class_init = aspeed_wdt_class_init,
.class_size = sizeof(AspeedWDTClass),
--
2.31.1
Hi Cédric,
On 10/4/21 17:46, Cédric Le Goater wrote:
> Initialize the region in the instance_init handler because we will
> want to link this region in the FMC object before the WDT object is
> realized.
>
> Cc: Peter Delevoryas <pdel@fb.com>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> include/hw/watchdog/wdt_aspeed.h | 1 +
> hw/watchdog/wdt_aspeed.c | 15 ++++++++++++---
> 2 files changed, 13 insertions(+), 3 deletions(-)
> diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c
> index 146ffcd71301..6426f3a77494 100644
> --- a/hw/watchdog/wdt_aspeed.c
> +++ b/hw/watchdog/wdt_aspeed.c
> @@ -261,6 +261,16 @@ static void aspeed_wdt_timer_expired(void *dev)
>
> #define PCLK_HZ 24000000
>
> +static void aspeed_wdt_instance_init(Object *obj)
> +{
> + AspeedWDTState *s = ASPEED_WDT(obj);
> +
> + memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_wdt_ops, s,
> + TYPE_ASPEED_WDT, ASPEED_WDT_REGS_MAX * 4);
> + memory_region_init_alias(&s->iomem_alias, OBJECT(s),
> + TYPE_ASPEED_WDT ".alias",
> + &s->iomem, 0, ASPEED_WDT_REGS_MAX * 4);
> +}
> static void aspeed_wdt_realize(DeviceState *dev, Error **errp)
> {
> SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> @@ -275,9 +285,7 @@ static void aspeed_wdt_realize(DeviceState *dev, Error **errp)
> */
> s->pclk_freq = PCLK_HZ;
>
> - memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_wdt_ops, s,
> - TYPE_ASPEED_WDT, ASPEED_WDT_REGS_MAX * 4);
> - sysbus_init_mmio(sbd, &s->iomem);
> + sysbus_init_mmio(sbd, &s->iomem_alias);
Exposing an alias that way seems odd. Don't you want to use/expose
a container instead?
Anyhow, by moving memory_region_init_io() in init(), the region is
available for linking before realize(), so why do you need an alias?
Hello Phil,
On 10/18/21 11:04, Philippe Mathieu-Daudé wrote:
> Hi Cédric,
>
> On 10/4/21 17:46, Cédric Le Goater wrote:
>> Initialize the region in the instance_init handler because we will
>> want to link this region in the FMC object before the WDT object is
>> realized.
>>
>> Cc: Peter Delevoryas <pdel@fb.com>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>> include/hw/watchdog/wdt_aspeed.h | 1 +
>> hw/watchdog/wdt_aspeed.c | 15 ++++++++++++---
>> 2 files changed, 13 insertions(+), 3 deletions(-)
>
>> diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c
>> index 146ffcd71301..6426f3a77494 100644
>> --- a/hw/watchdog/wdt_aspeed.c
>> +++ b/hw/watchdog/wdt_aspeed.c
>> @@ -261,6 +261,16 @@ static void aspeed_wdt_timer_expired(void *dev)
>>
>> #define PCLK_HZ 24000000
>>
>> +static void aspeed_wdt_instance_init(Object *obj)
>> +{
>> + AspeedWDTState *s = ASPEED_WDT(obj);
>> +
>> + memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_wdt_ops, s,
>> + TYPE_ASPEED_WDT, ASPEED_WDT_REGS_MAX * 4);
>> + memory_region_init_alias(&s->iomem_alias, OBJECT(s),
>> + TYPE_ASPEED_WDT ".alias",
>> + &s->iomem, 0, ASPEED_WDT_REGS_MAX * 4);
>> +}
>> static void aspeed_wdt_realize(DeviceState *dev, Error **errp)
>> {
>> SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>> @@ -275,9 +285,7 @@ static void aspeed_wdt_realize(DeviceState *dev, Error **errp)
>> */
>> s->pclk_freq = PCLK_HZ;
>>
>> - memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_wdt_ops, s,
>> - TYPE_ASPEED_WDT, ASPEED_WDT_REGS_MAX * 4);
>> - sysbus_init_mmio(sbd, &s->iomem);
>> + sysbus_init_mmio(sbd, &s->iomem_alias);
>
> Exposing an alias that way seems odd. Don't you want to use/expose
> a container instead?
I think I got confused by changes in e9c568dbc225 ("hw/arm/aspeed:
Do not sysbus-map mmio flash region directly, use alias") :)
You are right. This needs a container, not an alias. I will respin
and fix the aspeed-smc flash address space also.
> Anyhow, by moving memory_region_init_io() in init(), the region is
> available for linking before realize(), so why do you need an alias?
yes.
I still need to initialize the watchdog models before the FMC. There
will be a little change in the order.
Thanks,
C.
© 2016 - 2025 Red Hat, Inc.