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 <[email protected]>
> Signed-off-by: Cédric Le Goater <[email protected]>
> ---
> 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?
