| Message ID | 20180831103816.13479-8-clg@kaod.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | aspeed: misc fixes and enhancements (SMC) | expand |
On 31 August 2018 at 11:38, Cédric Le Goater <clg@kaod.org> wrote: > The setting of the DRAM address of the DMA transaction depends on the > DRAM base address and the maximun DRAM size of the SoC. Let's add a > couple of properties to give this information to the SMC controller > model. In hardware, does the SMC controller really know the base address of DRAM, or is it actually emitting transactions that the bus fabric in the SoC sends to the right place? That is, would it be more accurate to model it by passing the SMC controller a MemoryRegion to use for emitting DMA transactions which was an alias into the right part of the address space ? thanks -- PMM
On 09/18/2018 08:47 PM, Peter Maydell wrote: > On 31 August 2018 at 11:38, Cédric Le Goater <clg@kaod.org> wrote: >> The setting of the DRAM address of the DMA transaction depends on the >> DRAM base address and the maximun DRAM size of the SoC. Let's add a >> couple of properties to give this information to the SMC controller >> model. > > In hardware, does the SMC controller really know the base address > of DRAM, Here is the definition of the DMA DRAM Side Address register On the AST2500 : 31:30 reserved 0x2 29:2 DRAM side start address. Only 4 bytes boundary. The valid range is 0x80000000-0xBFFFFFFF 1:0 reserved On the AST2400 : 31:29 reserved 0x2 28:2 DRAM side start address. Only 4 bytes boundary. The valid range is 0x40000000-0x5FFFFFFF 1:0 reserved The DRAM address ranges are hardwired to fit the DRAM base address of the SoC and its maximum size, depending on the revision. Same for the flash address. The value just wraps around when the maximum is reached. > or is it actually emitting transactions that the bus fabric in > the SoC sends to the right place? The address is limited by the register definition before sending the transaction AFAICT. > That is, would > it be more accurate to model it by passing the SMC controller a > MemoryRegion to use for emitting DMA transactions which was an > alias into the right part of the address space ? I think you already proposed that a while ago. That would mean adding an address space and a memory region acting as a translating proxy. OK. I will look into it. Thanks, C.
diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h index 1f557313fa93..d7090bb5e9b7 100644 --- a/include/hw/ssi/aspeed_smc.h +++ b/include/hw/ssi/aspeed_smc.h @@ -97,6 +97,10 @@ typedef struct AspeedSMCState { uint8_t r_timings; uint8_t conf_enable_w0; + /* for DMA support */ + uint64_t sdram_base; + uint64_t max_ram_size; + AspeedSMCFlash *flashes; } AspeedSMCState; diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c index 2cbacb4430bb..bbc05d172fe1 100644 --- a/hw/arm/aspeed_soc.c +++ b/hw/arm/aspeed_soc.c @@ -209,6 +209,14 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp) } sysbus_mmio_map(SYS_BUS_DEVICE(&s->scu), 0, ASPEED_SOC_SCU_BASE); + /* SDMC - SDRAM Memory Controller */ + object_property_set_bool(OBJECT(&s->sdmc), true, "realized", &err); + if (err) { + error_propagate(errp, err); + return; + } + sysbus_mmio_map(SYS_BUS_DEVICE(&s->sdmc), 0, ASPEED_SOC_SDMC_BASE); + /* VIC */ object_property_set_bool(OBJECT(&s->vic), true, "realized", &err); if (err) { @@ -252,7 +260,17 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp) qdev_get_gpio_in(DEVICE(&s->vic), 12)); /* FMC, The number of CS is set at the board level */ - object_property_set_bool(OBJECT(&s->fmc), true, "realized", &err); + object_property_set_int(OBJECT(&s->fmc), sc->info->sdram_base, "sdram-base", + &err); + object_property_set_int(OBJECT(&s->fmc), s->sdmc.max_ram_size, + "max-ram-size", &local_err); + error_propagate(&err, local_err); + if (err) { + error_propagate(errp, err); + return; + } + object_property_set_bool(OBJECT(&s->fmc), true, "realized", &local_err); + error_propagate(&err, local_err); if (err) { error_propagate(errp, err); return; @@ -278,14 +296,6 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp) s->spi[i].ctrl->flash_window_base); } - /* SDMC - SDRAM Memory Controller */ - object_property_set_bool(OBJECT(&s->sdmc), true, "realized", &err); - if (err) { - error_propagate(errp, err); - return; - } - sysbus_mmio_map(SYS_BUS_DEVICE(&s->sdmc), 0, ASPEED_SOC_SDMC_BASE); - /* Watch dog */ for (i = 0; i < sc->info->wdts_num; i++) { object_property_set_bool(OBJECT(&s->wdt[i]), true, "realized", &err); diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c index 6045ca11b969..500de6d16d09 100644 --- a/hw/ssi/aspeed_smc.c +++ b/hw/ssi/aspeed_smc.c @@ -800,6 +800,8 @@ static const VMStateDescription vmstate_aspeed_smc = { static Property aspeed_smc_properties[] = { DEFINE_PROP_UINT32("num-cs", AspeedSMCState, num_cs, 1), + DEFINE_PROP_UINT64("sdram-base", AspeedSMCState, sdram_base, 0), + DEFINE_PROP_UINT64("max-ram-size", AspeedSMCState, max_ram_size, 0), DEFINE_PROP_END_OF_LIST(), };
The setting of the DRAM address of the DMA transaction depends on the DRAM base address and the maximun DRAM size of the SoC. Let's add a couple of properties to give this information to the SMC controller model. Also, move the SDRAM Memory controller realization before the other controllers which need it. Signed-off-by: Cédric Le Goater <clg@kaod.org> --- include/hw/ssi/aspeed_smc.h | 4 ++++ hw/arm/aspeed_soc.c | 28 +++++++++++++++++++--------- hw/ssi/aspeed_smc.c | 2 ++ 3 files changed, 25 insertions(+), 9 deletions(-)