[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 3/4] aspeed/wdt: Add an alias for the MMIO region
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH 3/4] aspeed/wdt: Add an alias for the MMIO region |
Date: |
Mon, 18 Oct 2021 11:04:06 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.1.0 |
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?
- [PATCH 0/4] aspeed/smc: Improve support for the alternate boot function, Cédric Le Goater, 2021/10/04
- [PATCH 2/4] aspeed/smc: Dump address offset in trace events, Cédric Le Goater, 2021/10/04
- [PATCH 3/4] aspeed/wdt: Add an alias for the MMIO region, Cédric Le Goater, 2021/10/04
- Re: [PATCH 3/4] aspeed/wdt: Add an alias for the MMIO region,
Philippe Mathieu-Daudé <=
- [PATCH 1/4] aspeed/wdt: Add trace events, Cédric Le Goater, 2021/10/04
- [PATCH 4/4] aspeed/smc: Improve support for the alternate boot function, Cédric Le Goater, 2021/10/04
- Re: [PATCH 0/4] aspeed/smc: Improve support for the alternate boot function, Cédric Le Goater, 2021/10/18