On 07/01/2016 06:24 PM, Peter Maydell wrote:
> On 28 June 2016 at 19:24, Cédric Le Goater <c...@kaod.org> wrote:
>> Each controller on the ast2400 has a memory range on which it maps its
>> flash module slaves. Each slave is assigned a memory segment for its
>> mapping that can be changed at bootime with the Segment Address
>> Register. This is not supported in the current implementation so we
>> are using the defaults provided by the specs.
>>
>> Each SPI flash slave can then be accessed in two modes: Command and
>> User. When in User mode, accesses to the memory segment of the slaves
>> are translated in SPI transfers. When in Command mode, the HW
>> generates the SPI commands automatically and the memory segment is
>> accessed as if doing a MMIO. Other SPI controllers call that mode
>> linear addressing mode.
>>
>> For this purpose, we are adding below each crontoller an array of
>> structs gathering for each SPI flash module, a segment rank, a
>> MemoryRegion to handle the memory accesses and the associated SPI
>> slave device, which should be a m25p80.
>>
>> Only the User mode is supported for now but we are preparing ground
>> for the Command mode. The framework is sufficient to support Linux.
>>
>> Signed-off-by: Cédric Le Goater <c...@kaod.org>
>
>> +/*
>> + * Default segments mapping addresses and size for each slave per
>> + * controller. These can be changed when board is initialized with the
>> + * Segment Address Registers but they don't seem do be used on the
>> + * field.
>> + */
>> +static const AspeedSegments aspeed_segments_legacy[] = {
>> + { 0x10000000, 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 },
>> +};
>
> If I understand this correctly, and the Segment Address Registers
> are part of the SMC controller, then eventually if we want to
> implement making these writable then the correct approach is
> probably to have a container memory region which the SMC controller
> provides to the board (and which the board then maps into the
> system memory space), and then the controller is responsible for
> mapping and unmapping the individual memory regions inside that
> container. This is basically how we do PCI controllers, which also
> allow guests to write to PCI BARs to map and unmap bits of memory.
OK. I will take a look at the approach. The default setting seems to
satisfy the different implementations I know of.
> This will be fine for now, though.
>
>> static bool aspeed_smc_is_ce_stop_active(const AspeedSMCState *s, int cs)
>> @@ -237,6 +353,8 @@ static void aspeed_smc_realize(DeviceState *dev, Error
>> **errp)
>> AspeedSMCState *s = ASPEED_SMC(dev);
>> AspeedSMCClass *mc = ASPEED_SMC_GET_CLASS(s);
>> int i;
>> + char name[32];
>> + hwaddr offset = 0;
>>
>> s->ctrl = mc->ctrl;
>>
>> @@ -270,6 +388,32 @@ 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);
>> +
>> + /*
>> + * Memory region where flash modules are remapped
>> + */
>> + snprintf(name, sizeof(name), "%s.flash", s->ctrl->name);
>> +
>> + memory_region_init_io(&s->mmio_flash, OBJECT(s),
>> + &aspeed_smc_flash_default_ops, s, name,
>> + s->ctrl->mapping_window_size);
>> + sysbus_init_mmio(sbd, &s->mmio_flash);
>> +
>> + s->flashes = g_malloc0(sizeof(AspeedSMCFlash) * s->num_cs);
>
> This should be g_new0(AspeedSMCFlash, s->num_cs); -- multiplying
> in a g_malloc0() is usually a sign you should use g_new0 instead.
ah yes. I have changed that back and forth and kept the wrong one ...
> Otherwise
> Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
>
> so I'll just fix that when I put the series in target-arm.next.
I have some extra patches to use a rom device and boot from flash0.
That is for next week.
Thanks,
C.
> thanks
> -- PMM
>
