On 23 July 2016 at 17:42, Alistair Francis <alistai...@gmail.com> wrote: > Add the STM32F2xx SPI device. > > Signed-off-by: Alistair Francis <alist...@alistair23.me> > ---
> +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 <peter.mayd...@linaro.org> thanks -- PMM