| Message ID | 1486997062-23227-2-git-send-email-clg@kaod.org |
|---|---|
| State | New |
| Headers | show |
On 13 February 2017 at 14:44, Cédric Le Goater <clg@kaod.org> wrote: > The setting of the DRAM address of the DMA transaction depends on the > DRAM base address of the SoC. Let's add a property to give this > information to the SMC controller model. > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > Reviewed-by: Joel Stanley <joel@jms.id.au> > Reviewed-by: Andrew Jeffery <andrew@aj.id.au> > -- This seems a bit weird -- what does it actually do in hardware? (Does it really have the ability to dma anywhere in the address space and just use an odd base address, or is it DMA'ing to a more restricted address space?) thanks -- PMM
On 02/20/2017 02:51 PM, Peter Maydell wrote: > On 13 February 2017 at 14:44, Cédric Le Goater <clg@kaod.org> wrote: >> The setting of the DRAM address of the DMA transaction depends on the >> DRAM base address of the SoC. Let's add a property to give this >> information to the SMC controller model. >> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> Reviewed-by: Joel Stanley <joel@jms.id.au> >> Reviewed-by: Andrew Jeffery <andrew@aj.id.au> >> -- > > This seems a bit weird -- what does it actually do in hardware? > (Does it really have the ability to dma anywhere in the > address space and just use an odd base address, or is it > DMA'ing to a more restricted address space?) For the AST2500, the valid address range for the DRAM side start address of the DMA is [ 0x80000000 - 0xBFFFFFFF ] which is the full 1024M SDRAM space. It will wrap back in case of overflow. For the AST2400, it is [ 0x40000000 - 0x5FFFFFFF ] which is the full 512M SDRAM space. And the length of the DMA goes from 4 bytes to 32MB [ O - 0x7FFFFF ] On real HW, the DRAM address of the DMA is taken as an offset from the beginning of the SDRAM address space. It does the same for the flash address btw. This is why we have the macros in the model : #define DMA_DRAM_ADDR(base, x) (((x) & ~0xE0000003) | base) #define DMA_FLASH_ADDR(x) (((x) & ~0xE0000003) | \ ASPEED_SOC_FMC_FLASH_BASE) Thanks, C.
On 20 February 2017 at 14:30, Cédric Le Goater <clg@kaod.org> wrote: > On 02/20/2017 02:51 PM, Peter Maydell wrote: >> On 13 February 2017 at 14:44, Cédric Le Goater <clg@kaod.org> wrote: >>> The setting of the DRAM address of the DMA transaction depends on the >>> DRAM base address of the SoC. Let's add a property to give this >>> information to the SMC controller model. >>> >>> Signed-off-by: Cédric Le Goater <clg@kaod.org> >>> Reviewed-by: Joel Stanley <joel@jms.id.au> >>> Reviewed-by: Andrew Jeffery <andrew@aj.id.au> >>> -- >> >> This seems a bit weird -- what does it actually do in hardware? >> (Does it really have the ability to dma anywhere in the >> address space and just use an odd base address, or is it >> DMA'ing to a more restricted address space?) > > For the AST2500, the valid address range for the DRAM side start > address of the DMA is [ 0x80000000 - 0xBFFFFFFF ] which is the > full 1024M SDRAM space. It will wrap back in case of overflow. > > For the AST2400, it is [ 0x40000000 - 0x5FFFFFFF ] which is the > full 512M SDRAM space. > > And the length of the DMA goes from 4 bytes to 32MB [ O - 0x7FFFFF ] So if I do a DMA access to 0x800000 that's the same as an access at 0x0, or does it fail? I ask because the other option for modelling this kind of thing would be that you make the DMA controller device take a link property which is a MemoryRegion*. Then inside the DMA device you create the AddressSpace* which you use to do DMA accesses, and in the board code you pass the DMA device a suitably constructed MemoryRegion that contains the things that the device can DMA to. You don't have to do it that way (and I don't think we have much in the way of examples of doing it like that, except for the ARM CPU handling of secure and nonsecure memory address spaces), but if it seems to fit better with what the hardware's doing it might be worth the effort. thanks -- PMM
On 02/20/2017 03:38 PM, Peter Maydell wrote: > On 20 February 2017 at 14:30, Cédric Le Goater <clg@kaod.org> wrote: >> On 02/20/2017 02:51 PM, Peter Maydell wrote: >>> On 13 February 2017 at 14:44, Cédric Le Goater <clg@kaod.org> wrote: >>>> The setting of the DRAM address of the DMA transaction depends on the >>>> DRAM base address of the SoC. Let's add a property to give this >>>> information to the SMC controller model. >>>> >>>> Signed-off-by: Cédric Le Goater <clg@kaod.org> >>>> Reviewed-by: Joel Stanley <joel@jms.id.au> >>>> Reviewed-by: Andrew Jeffery <andrew@aj.id.au> >>>> -- >>> >>> This seems a bit weird -- what does it actually do in hardware? >>> (Does it really have the ability to dma anywhere in the >>> address space and just use an odd base address, or is it >>> DMA'ing to a more restricted address space?) >> >> For the AST2500, the valid address range for the DRAM side start >> address of the DMA is [ 0x80000000 - 0xBFFFFFFF ] which is the >> full 1024M SDRAM space. It will wrap back in case of overflow. >> >> For the AST2400, it is [ 0x40000000 - 0x5FFFFFFF ] which is the >> full 512M SDRAM space. >> >> And the length of the DMA goes from 4 bytes to 32MB [ O - 0x7FFFFF ] In fact, the above range is from the spec and it is wrong. I suppose it is a left over from previous boards. Here is the current one : [ 0 - 0x1fffffc ] > So if I do a DMA access to 0x800000 that's the same as an > access at 0x0, or does it fail? The length value wraps back just like the addresses. > I ask because the other option for modelling this kind of thing > would be that you make the DMA controller device take a link > property which is a MemoryRegion*. Then inside the DMA device > you create the AddressSpace* which you use to do DMA accesses, > and in the board code you pass the DMA device a suitably > constructed MemoryRegion that contains the things that the > device can DMA to. ok. I think I understand. I have done something similar for the SCOM sideband bus of the PowerNV machine. It looks a like a good idea. The 'minus 4' on the DMA length might be a little annoying but I will give it a try. Thanks, C. > You don't have to do it that way (and I don't think we > have much in the way of examples of doing it like that, > except for the ARM CPU handling of secure and nonsecure > memory address spaces), but if it seems to fit better > with what the hardware's doing it might be worth the effort. > > thanks > -- PMM >
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c index 571e4f097b02..6df76382f007 100644 --- a/hw/arm/aspeed_soc.c +++ b/hw/arm/aspeed_soc.c @@ -258,7 +258,10 @@ 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_bool(OBJECT(&s->fmc), true, "realized", &local_err); + error_propagate(&err, local_err); if (err) { error_propagate(errp, err); return; diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c index cb515730c5ad..f6ecdc014436 100644 --- a/hw/ssi/aspeed_smc.c +++ b/hw/ssi/aspeed_smc.c @@ -798,6 +798,7 @@ static const VMStateDescription vmstate_aspeed_smc = { }; static Property aspeed_smc_properties[] = { + DEFINE_PROP_UINT64("sdram-base", AspeedSMCState, sdram_base, 0), DEFINE_PROP_UINT32("num-cs", AspeedSMCState, num_cs, 1), DEFINE_PROP_END_OF_LIST(), }; diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h index 1f557313fa93..2c375af7bcbb 100644 --- a/include/hw/ssi/aspeed_smc.h +++ b/include/hw/ssi/aspeed_smc.h @@ -97,6 +97,9 @@ typedef struct AspeedSMCState { uint8_t r_timings; uint8_t conf_enable_w0; + /* for DMA support */ + uint64_t sdram_base; + AspeedSMCFlash *flashes; } AspeedSMCState;