Re: [Qemu-devel] [PATCH v3 4/5] ast2400: create SPI flash slaves

[Peter Maydell]

On 23 June 2016 at 17:33, Cédric Le Goater <address@hidden> wrote:
> A set of SPI flash slaves is attached under the flash controllers of
> the palmetto platform. "n25q256a" flash modules are used for the BMC
> and "mx25l25635e" for the host. These types are common in the
> OpenPower ecosystem.
>
> The segment addresses used for the memory mappings are the defaults
> provided by the specs. They can be changed with the Segment Address
> Register but this is not supported in the current implementation.
>
> Signed-off-by: Cédric Le Goater <address@hidden>
> ---

> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
> index d97c077565c3..6be911ed36d0 100644
> --- a/hw/ssi/aspeed_smc.c
> +++ b/hw/ssi/aspeed_smc.c
> @@ -98,13 +98,32 @@
>  #define R_SPI_MISC_CRTL   (0x10 / 4)
>  #define R_SPI_TIMINGS     (0x14 / 4)
>
> +/*
> + * Default segments mappings and size for each slave
> + */
> +static const AspeedSegments aspeed_segments_legacy[] = {
> +    { 0x14000000, 32 * 1024 * 1024 },
> +};
> +
> +static const AspeedSegments aspeed_segments_fmc[] = {
> +    { 0x20000000, 64 * 1024 * 1024 },
> +    { 0x24000000, 32 * 1024 * 1024 },
> +    { 0x26000000, 32 * 1024 * 1024 },
> +    { 0x28000000, 32 * 1024 * 1024 },
> +    { 0x2A000000, 32 * 1024 * 1024 }
> +};
> +
> +static const AspeedSegments aspeed_segments_spi[] = {
> +    { 0x30000000, 64 * 1024 * 1024 },
> +};
> +
>  static const AspeedSMCController controllers[] = {
>      { "aspeed.smc.smc", R_CONF, R_CE_CTRL, R_CTRL0, R_TIMINGS,
> -      CONF_ENABLE_W0, 5 },
> +      CONF_ENABLE_W0, 5, aspeed_segments_legacy },
>      { "aspeed.smc.fmc", R_CONF, R_CE_CTRL, R_CTRL0, R_TIMINGS,
> -      CONF_ENABLE_W0, 5 },
> +      CONF_ENABLE_W0, 5, aspeed_segments_fmc },
>      { "aspeed.smc.spi", R_SPI_CONF, 0xff, R_SPI_CTRL0, R_SPI_TIMINGS,
> -      SPI_CONF_ENABLE_W0, 1 },
> +      SPI_CONF_ENABLE_W0, 1, aspeed_segments_spi },
>  };
>
>  static bool aspeed_smc_is_ce_stop_active(AspeedSMCState *s, int cs)
> @@ -217,6 +236,8 @@ static const MemoryRegionOps aspeed_smc_ops = {
>      .valid.unaligned = true,
>  };
>
> +static const MemoryRegionOps aspeed_smc_flash_ops;
> +
>  static void aspeed_smc_realize(DeviceState *dev, Error **errp)
>  {
>      SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> @@ -254,6 +275,27 @@ static void aspeed_smc_realize(DeviceState *dev, Error 
> **errp)
>      memory_region_init_io(&s->mmio, OBJECT(s), &aspeed_smc_ops, s,
>                            s->ctrl->name, ASPEED_SMC_R_MAX * 4);
>      sysbus_init_mmio(sbd, &s->mmio);
> +
> +    s->flashes = g_new0(AspeedSMCFlashState *, s->num_cs);
> +
> +    for (i = 0; i < s->num_cs; ++i) {
> +        Object *obj = object_new(TYPE_ASPEED_SMC_FLASH);
> +        AspeedSMCFlashState *fl = ASPEED_SMC_FLASH(obj);
> +        char name[32];
> +
> +        s->flashes[i] = fl;
> +
> +        snprintf(name, sizeof(name), "%s.%d", s->ctrl->name, i);
> +
> +        fl->id = i;
> +        fl->controller = s;
> +        fl->size = s->ctrl->segments[i].size;
> +
> +        memory_region_init_io(&fl->mmio, obj, &aspeed_smc_flash_ops, fl, 
> name,
> +                              fl->size);
> +        sysbus_init_mmio(SYS_BUS_DEVICE(fl), &fl->mmio);
> +        sysbus_mmio_map(SYS_BUS_DEVICE(fl), 0, s->ctrl->segments[i].addr);

Device code shouldn't call sysbus_mmio_map() -- the system memory map is
the board's responsibility.

thanks
-- PMM