On 06/23/2016 08:48 PM, Peter Maydell wrote: > On 23 June 2016 at 17:33, Cédric Le Goater <[email protected]> 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 <[email protected]> >> --- > >> 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.
ok. It's here because it was easy to reuse the address segments[i].addr which should not change, even if is reconfigurable at run time by the guest. I will rethink the structures and find way. Thanks, C.
