Re: [PATCH v2 4/5] hw/misc: Add a basic Aspeed LPC controller model

[Andrew Jeffery]

On Mon, 1 Mar 2021, at 11:36, Andrew Jeffery wrote:
> From: Cédric Le Goater <clg@kaod.org>
> 
> This is a very minimal framework to access registers which are used to
> configure the AHB memory mapping of the flash chips on the LPC HC
> Firmware address space.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> ---
>  docs/system/arm/aspeed.rst   |   2 +-
>  hw/arm/aspeed_ast2600.c      |  10 +++
>  hw/arm/aspeed_soc.c          |  10 +++
>  hw/misc/aspeed_lpc.c         | 131 +++++++++++++++++++++++++++++++++++
>  hw/misc/meson.build          |   7 +-
>  include/hw/arm/aspeed_soc.h  |   2 +
>  include/hw/misc/aspeed_lpc.h |  32 +++++++++
>  7 files changed, 192 insertions(+), 2 deletions(-)
>  create mode 100644 hw/misc/aspeed_lpc.c
>  create mode 100644 include/hw/misc/aspeed_lpc.h
> 
> diff --git a/docs/system/arm/aspeed.rst b/docs/system/arm/aspeed.rst
> index 690bada7842b..2f6fa8938d02 100644
> --- a/docs/system/arm/aspeed.rst
> +++ b/docs/system/arm/aspeed.rst
> @@ -48,6 +48,7 @@ Supported devices
>   * UART
>   * Ethernet controllers
>   * Front LEDs (PCA9552 on I2C bus)
> + * LPC Peripheral Controller (a subset of subdevices are supported)
>  
>  
>  Missing devices
> @@ -56,7 +57,6 @@ Missing devices
>   * Coprocessor support
>   * ADC (out of tree implementation)
>   * PWM and Fan Controller
> - * LPC Bus Controller
>   * Slave GPIO Controller
>   * Super I/O Controller
>   * Hash/Crypto Engine
> diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> index 2125a96ef317..60152de001e6 100644
> --- a/hw/arm/aspeed_ast2600.c
> +++ b/hw/arm/aspeed_ast2600.c
> @@ -211,6 +211,8 @@ static void aspeed_soc_ast2600_init(Object *obj)
>  
>      object_initialize_child(obj, "emmc-controller.sdhci", &s->emmc.slots[0],
>                              TYPE_SYSBUS_SDHCI);
> +
> +    object_initialize_child(obj, "lpc", &s->lpc, TYPE_ASPEED_LPC);
>  }
>  
>  /*
> @@ -469,6 +471,14 @@ static void aspeed_soc_ast2600_realize(DeviceState 
> *dev, Error **errp)
>      sysbus_mmio_map(SYS_BUS_DEVICE(&s->emmc), 0, 
> sc->memmap[ASPEED_DEV_EMMC]);
>      sysbus_connect_irq(SYS_BUS_DEVICE(&s->emmc), 0,
>                         aspeed_soc_get_irq(s, ASPEED_DEV_EMMC));
> +
> +    /* LPC */
> +    if (!sysbus_realize(SYS_BUS_DEVICE(&s->lpc), errp)) {
> +        return;
> +    }
> +    sysbus_mmio_map(SYS_BUS_DEVICE(&s->lpc), 0, 
> sc->memmap[ASPEED_DEV_LPC]);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(&s->emmc), 0,
> +                       aspeed_soc_get_irq(s, ASPEED_DEV_LPC));

Hah, this isn't right! We don't notice it wrt LPC devices because the 
LPC IRQ is unused right now, but it will impact the eMMC.

Let me do a v3.

Andrew