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

[Peter Maydell]

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?

> +    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
-- PMM