qemu-arm
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-arm] [PATCH v2 15/21] aspeed/smc: add support for DMAs


From: Cédric Le Goater
Subject: Re: [Qemu-arm] [PATCH v2 15/21] aspeed/smc: add support for DMAs
Date: Mon, 1 Jul 2019 15:50:25 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.2

On 01/07/2019 15:06, Peter Maydell wrote:
> On Tue, 18 Jun 2019 at 17:55, Cédric Le Goater <address@hidden> wrote:
>>
>> The FMC controller on the Aspeed SoCs support DMA to access the flash
>> modules. It can operate in a normal mode, to copy to or from the flash
>> module mapping window, or in a checksum calculation mode, to evaluate
>> the best clock settings for reads.
>>
>> The model introduces two custom address spaces for DMAs: one for the
>> AHB window of the FMC flash devices and one for the DRAM. The latter
>> is populated using a "dram" link set from the machine with the RAM
>> container region.
>>
>> Signed-off-by: Cédric Le Goater <address@hidden>
>> Acked-by: Joel Stanley <address@hidden>
>> +/*
>> + * Accumulate the result of the reads to provide a checksum that will
>> + * be used to validate the read timing settings.
>> + */
>> +static void aspeed_smc_dma_checksum(AspeedSMCState *s)
>> +{
>> +    MemTxResult result;
>> +    uint32_t data;
>> +
>> +    if (s->regs[R_DMA_CTRL] & DMA_CTRL_WRITE) {
>> +        qemu_log_mask(LOG_GUEST_ERROR,
>> +                      "%s: invalid direction for DMA checksum\n",  
>> __func__);
>> +        return;
>> +    }
>> +
>> +    while (s->regs[R_DMA_LEN]) {
>> +        result = address_space_read(&s->flash_as, s->regs[R_DMA_FLASH_ADDR],
>> +                                    MEMTXATTRS_UNSPECIFIED,
>> +                                    (uint8_t *)&data, 4);
> 
> This does a byte-by-byte read into a local uint32_t...
> 
>> +        if (result != MEMTX_OK) {
>> +            qemu_log_mask(LOG_GUEST_ERROR, "%s: Flash read failed @%08x\n",
>> +                          __func__, s->regs[R_DMA_FLASH_ADDR]);
>> +            return;
>> +        }
>> +
>> +        /*
>> +         * When the DMA is on-going, the DMA registers are updated
>> +         * with the current working addresses and length.
>> +         */
>> +        s->regs[R_DMA_CHECKSUM] += data;
> 
> ...which we then use as a (host-endian) 32-bit value.
> 
> This will behave differently on big endian and little endian hosts.
> If the h/w behaviour is to to load a 32-bit data type you probably
> want address_space_ldl_le() (or _be() if it's doing a big-endian load).
> 
>> +        s->regs[R_DMA_FLASH_ADDR] += 4;
>> +        s->regs[R_DMA_LEN] -= 4;
>> +    }
>> +}
>> +
>> +static void aspeed_smc_dma_rw(AspeedSMCState *s)
>> +{
>> +    MemTxResult result;
>> +    uint32_t data;
>> +
>> +    while (s->regs[R_DMA_LEN]) {
>> +        if (s->regs[R_DMA_CTRL] & DMA_CTRL_WRITE) {
>> +            result = address_space_read(&s->dram_as, 
>> s->regs[R_DMA_DRAM_ADDR],
>> +                                        MEMTXATTRS_UNSPECIFIED,
>> +                                        (uint8_t *)&data, 4);
>> +            if (result != MEMTX_OK) {
>> +                qemu_log_mask(LOG_GUEST_ERROR, "%s: DRAM read failed 
>> @%08x\n",
>> +                              __func__, s->regs[R_DMA_DRAM_ADDR]);
>> +                return;
>> +            }
>> +
>> +            result = address_space_write(&s->flash_as,
>> +                                         s->regs[R_DMA_FLASH_ADDR],
>> +                                         MEMTXATTRS_UNSPECIFIED,
>> +                                         (uint8_t *)&data, 4);
>> +            if (result != MEMTX_OK) {
>> +                qemu_log_mask(LOG_GUEST_ERROR, "%s: Flash write failed 
>> @%08x\n",
>> +                              __func__, s->regs[R_DMA_FLASH_ADDR]);
>> +                return;
>> +            }
>> +        } else {
>> +            result = address_space_read(&s->flash_as, 
>> s->regs[R_DMA_FLASH_ADDR],
>> +                                        MEMTXATTRS_UNSPECIFIED,
>> +                                        (uint8_t *)&data, 4);
>> +            if (result != MEMTX_OK) {
>> +                qemu_log_mask(LOG_GUEST_ERROR, "%s: Flash read failed 
>> @%08x\n",
>> +                              __func__, s->regs[R_DMA_FLASH_ADDR]);
>> +                return;
>> +            }
>> +
>> +            result = address_space_write(&s->dram_as, 
>> s->regs[R_DMA_DRAM_ADDR],
>> +                                         MEMTXATTRS_UNSPECIFIED,
>> +                                         (uint8_t *)&data, 4);
>> +            if (result != MEMTX_OK) {
>> +                qemu_log_mask(LOG_GUEST_ERROR, "%s: DRAM write failed 
>> @%08x\n",
>> +                              __func__, s->regs[R_DMA_DRAM_ADDR]);
>> +                return;
>> +            }
>> +        }
> 
> Since the code here doesn't do anything with the data the
> address_space_read/write here aren't wrong, but you might
> prefer to use the ldl and stl functions to avoid the casts
> to uint8_t* and need to specify the length.

yes. I will check with the HW guys to know precisely how the values are
read and accumulated.

Thanks,

C. 



reply via email to

[Prev in Thread] Current Thread [Next in Thread]