Re: [Qemu-devel] [PATCH v4 4/8] STM32F2xx: Add the SPI device

[Alistair Francis]

On Mon, Jul 25, 2016 at 9:42 AM, Peter Maydell <address@hidden> wrote:
> On 23 July 2016 at 17:42, Alistair Francis <address@hidden> wrote:
>> Add the STM32F2xx SPI device.
>>
>> Signed-off-by: Alistair Francis <address@hidden>
>> ---
>
>> +static uint64_t stm32f2xx_spi_read(void *opaque, hwaddr addr,
>> +                                     unsigned int size)
>> +{
>> +    STM32F2XXSPIState *s = opaque;
>> +    uint32_t retval;
>> +
>> +    DB_PRINT("Address: 0x%" HWADDR_PRIx "\n", addr);
>> +
>> +    switch (addr) {
>> +    case STM_SPI_CR1:
>> +        return s->spi_cr1;
>> +    case STM_SPI_CR2:
>> +        qemu_log_mask(LOG_UNIMP, "%s: Interrupts and DMA are not 
>> implemented\n",
>> +                      __func__);
>> +        return s->spi_cr2;
>> +    case STM_SPI_SR:
>> +        retval = s->spi_sr;
>> +        return retval;
>
> Why not just 'return s->spi_sr;' like the other cases?

Not sure, I have fixed this.

>
>> +    case STM_SPI_DR:
>> +        stm32f2xx_spi_transfer(s);
>> +        s->spi_sr &= ~STM_SPI_SR_RXNE;
>> +        return s->spi_dr;
>> +    case STM_SPI_CRCPR:
>> +        qemu_log_mask(LOG_UNIMP, "%s: CRC is not implemented, the registers 
>> " \
>> +                      "are included for compatibility\n", __func__);
>> +        return s->spi_crcpr;
>> +    case STM_SPI_RXCRCR:
>> +        qemu_log_mask(LOG_UNIMP, "%s: CRC is not implemented, the registers 
>> " \
>> +                      "are included for compatibility\n", __func__);
>> +        return s->spi_rxcrcr;
>> +    case STM_SPI_TXCRCR:
>> +        qemu_log_mask(LOG_UNIMP, "%s: CRC is not implemented, the registers 
>> " \
>> +                      "are included for compatibility\n", __func__);
>> +        return s->spi_txcrcr;
>> +    case STM_SPI_I2SCFGR:
>> +        qemu_log_mask(LOG_UNIMP, "%s: I2S is not implemented, the registers 
>> " \
>> +                      "are included for compatibility\n", __func__);
>> +        return s->spi_i2scfgr;
>> +    case STM_SPI_I2SPR:
>> +        qemu_log_mask(LOG_UNIMP, "%s: I2S is not implemented, the registers 
>> " \
>> +                      "are included for compatibility\n", __func__);
>> +        return s->spi_i2spr;
>> +    default:
>> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIx 
>> "\n",
>> +                      __func__, addr);
>> +    }
>> +
>> +    return 0;
>> +}
>
> Otherwise
> Reviewed-by: Peter Maydell <address@hidden>

Thanks,

Alistair

>
> thanks
> -- PMM