On 23 July 2016 at 17:42, Alistair Francis <[email protected]> wrote:
> Add the STM32F2xx SPI device.
>
> Signed-off-by: Alistair Francis <[email protected]>
> ---
> +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 <[email protected]>
thanks
-- PMM
