[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v9 08/13] xilinx_spips: Make tx/rx_data_bytes mo
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [Qemu-devel] [PATCH v9 08/13] xilinx_spips: Make tx/rx_data_bytes more generic and reusable |
Date: |
Mon, 27 Nov 2017 00:52:27 -0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 |
Hi Francisco,
On 11/26/2017 08:16 PM, Francisco Iglesias wrote:
> Make tx/rx_data_bytes more generic so they can be reused (when adding
> support for the Zynqmp Generic QSPI).
>
> Signed-off-by: Francisco Iglesias <address@hidden>
> Reviewed-by: Edgar E. Iglesias <address@hidden>
> Tested-by: Edgar E. Iglesias <address@hidden>
> ---
> hw/ssi/xilinx_spips.c | 64
> +++++++++++++++++++++++++++++----------------------
> 1 file changed, 37 insertions(+), 27 deletions(-)
>
> diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
> index 691d48d..4621dbb 100644
> --- a/hw/ssi/xilinx_spips.c
> +++ b/hw/ssi/xilinx_spips.c
> @@ -47,7 +47,7 @@
> /* config register */
> #define R_CONFIG (0x00 / 4)
> #define IFMODE (1U << 31)
> -#define ENDIAN (1 << 26)
> +#define R_CONFIG_ENDIAN (1 << 26)
> #define MODEFAIL_GEN_EN (1 << 17)
> #define MAN_START_COM (1 << 16)
> #define MAN_START_EN (1 << 15)
> @@ -450,13 +450,28 @@ static void xilinx_spips_flush_txfifo(XilinxSPIPS *s)
> }
> }
>
> -static inline void rx_data_bytes(XilinxSPIPS *s, uint8_t *value, int max)
> +static inline void tx_data_bytes(Fifo8 *fifo, uint32_t value, int num, bool
> be)
> {
> int i;
> + for (i = 0; i < num && !fifo8_is_full(fifo); ++i) {
> + if (be) {
> + fifo8_push(fifo, (uint8_t)(value >> 24));
> + value <<= 8;
> + } else {
> + fifo8_push(fifo, (uint8_t)value);
> + value >>= 8;
> + }
> + }
> +}
>
> - for (i = 0; i < max && !fifo8_is_empty(&s->rx_fifo); ++i) {
> - value[i] = fifo8_pop(&s->rx_fifo);
> +static inline int rx_data_bytes(Fifo8 *fifo, uint8_t *value, int max)
> +{
> + int i;
> +
> + for (i = 0; i < max && !fifo8_is_empty(fifo); ++i) {
> + value[i] = fifo8_pop(fifo);
> }
> + return max - i;
> }
>
> static uint64_t xilinx_spips_read(void *opaque, hwaddr addr,
> @@ -466,6 +481,7 @@ static uint64_t xilinx_spips_read(void *opaque, hwaddr
> addr,
> uint32_t mask = ~0;
> uint32_t ret;
> uint8_t rx_buf[4];
> + int shortfall;
>
> addr >>= 2;
> switch (addr) {
> @@ -496,9 +512,13 @@ static uint64_t xilinx_spips_read(void *opaque, hwaddr
> addr,
> break;
> case R_RX_DATA:
> memset(rx_buf, 0, sizeof(rx_buf));
> - rx_data_bytes(s, rx_buf, s->num_txrx_bytes);
> - ret = s->regs[R_CONFIG] & ENDIAN ? cpu_to_be32(*(uint32_t *)rx_buf)
> - : cpu_to_le32(*(uint32_t *)rx_buf);
> + shortfall = rx_data_bytes(&s->rx_fifo, rx_buf, s->num_txrx_bytes);
About this part,
> + ret = s->regs[R_CONFIG] & R_CONFIG_ENDIAN ?
> + cpu_to_be32(*(uint32_t *)rx_buf) :
> + cpu_to_le32(*(uint32_t *)rx_buf);
> + if (!(s->regs[R_CONFIG] & R_CONFIG_ENDIAN)) {
> + ret <<= 8 * shortfall;
> + }
The following looks easier to read to me, probably matter of taste, but
you don't have to wonder the cpu <-> device direction and it remove the
casts:
if (s->regs[R_CONFIG] & R_CONFIG_ENDIAN) {
ret = ldl_be_p(rx_buf);
} else {
ret = ldl_le_p(rx_buf) << (8 * shortfall);
}
What do you think? (before respining)
> DB_PRINT_L(0, "addr=" TARGET_FMT_plx " = %x\n", addr * 4, ret);
> xilinx_spips_update_ixr(s);
> return ret;
> @@ -509,20 +529,6 @@ static uint64_t xilinx_spips_read(void *opaque, hwaddr
> addr,
>
> }
>
> -static inline void tx_data_bytes(XilinxSPIPS *s, uint32_t value, int num)
> -{
> - int i;
> - for (i = 0; i < num && !fifo8_is_full(&s->tx_fifo); ++i) {
> - if (s->regs[R_CONFIG] & ENDIAN) {
> - fifo8_push(&s->tx_fifo, (uint8_t)(value >> 24));
> - value <<= 8;
> - } else {
> - fifo8_push(&s->tx_fifo, (uint8_t)value);
> - value >>= 8;
> - }
> - }
> -}
> -
> static void xilinx_spips_write(void *opaque, hwaddr addr,
> uint64_t value, unsigned size)
> {
> @@ -563,16 +569,20 @@ static void xilinx_spips_write(void *opaque, hwaddr
> addr,
> mask = 0;
> break;
> case R_TX_DATA:
> - tx_data_bytes(s, (uint32_t)value, s->num_txrx_bytes);
> + tx_data_bytes(&s->tx_fifo, (uint32_t)value, s->num_txrx_bytes,
> + s->regs[R_CONFIG] & R_CONFIG_ENDIAN);
> goto no_reg_update;
> case R_TXD1:
> - tx_data_bytes(s, (uint32_t)value, 1);
> + tx_data_bytes(&s->tx_fifo, (uint32_t)value, 1,
> + s->regs[R_CONFIG] & R_CONFIG_ENDIAN);
> goto no_reg_update;
> case R_TXD2:
> - tx_data_bytes(s, (uint32_t)value, 2);
> + tx_data_bytes(&s->tx_fifo, (uint32_t)value, 2,
> + s->regs[R_CONFIG] & R_CONFIG_ENDIAN);
> goto no_reg_update;
> case R_TXD3:
> - tx_data_bytes(s, (uint32_t)value, 3);
> + tx_data_bytes(&s->tx_fifo, (uint32_t)value, 3,
> + s->regs[R_CONFIG] & R_CONFIG_ENDIAN);
> goto no_reg_update;
> }
> s->regs[addr] = (s->regs[addr] & ~mask) | (value & mask);
> @@ -682,11 +692,11 @@ static void lqspi_load_cache(void *opaque, hwaddr addr)
>
> while (cache_entry < LQSPI_CACHE_SIZE) {
> for (i = 0; i < 64; ++i) {
> - tx_data_bytes(s, 0, 1);
> + tx_data_bytes(&s->tx_fifo, 0, 1, false);
> }
> xilinx_spips_flush_txfifo(s);
> for (i = 0; i < 64; ++i) {
> - rx_data_bytes(s, &q->lqspi_buf[cache_entry++], 1);
> + rx_data_bytes(&s->rx_fifo, &q->lqspi_buf[cache_entry++], 1);
> }
> }
>
>
- [Qemu-devel] [PATCH v9 01/13] m25p80: Add support for continuous read out of RDSR and READ_FSR, (continued)
- [Qemu-devel] [PATCH v9 01/13] m25p80: Add support for continuous read out of RDSR and READ_FSR, Francisco Iglesias, 2017/11/26
- [Qemu-devel] [PATCH v9 02/13] m25p80: Add support for SST READ ID 0x90/0xAB commands, Francisco Iglesias, 2017/11/26
- [Qemu-devel] [PATCH v9 03/13] m25p80: Add support for BRRD/BRWR and BULK_ERASE (0x60), Francisco Iglesias, 2017/11/26
- [Qemu-devel] [PATCH v9 04/13] m25p80: Add support for n25q512a11 and n25q512a13, Francisco Iglesias, 2017/11/26
- [Qemu-devel] [PATCH v9 05/13] xilinx_spips: Move FlashCMD, XilinxQSPIPS and XilinxSPIPSClass, Francisco Iglesias, 2017/11/26
- [Qemu-devel] [PATCH v9 07/13] xilinx_spips: Add support for RX discard and RX drain, Francisco Iglesias, 2017/11/26
- [Qemu-devel] [PATCH v9 06/13] xilinx_spips: Update striping to be big-endian bit order, Francisco Iglesias, 2017/11/26
- [Qemu-devel] [PATCH v9 09/13] xilinx_spips: Add support for zero pumping, Francisco Iglesias, 2017/11/26
- [Qemu-devel] [PATCH v9 08/13] xilinx_spips: Make tx/rx_data_bytes more generic and reusable, Francisco Iglesias, 2017/11/26
- Re: [Qemu-devel] [PATCH v9 08/13] xilinx_spips: Make tx/rx_data_bytes more generic and reusable,
Philippe Mathieu-Daudé <=
- [Qemu-devel] [PATCH v9 10/13] xilinx_spips: Add support for 4 byte addresses in the LQSPI, Francisco Iglesias, 2017/11/26
- [Qemu-devel] [PATCH v9 11/13] xilinx_spips: Don't set TX FIFO UNDERFLOW at cmd done, Francisco Iglesias, 2017/11/26
- [Qemu-devel] [PATCH v9 13/13] xlnx-zcu102: Add support for the ZynqMP QSPI, Francisco Iglesias, 2017/11/26
- [Qemu-devel] [PATCH v9 12/13] xilinx_spips: Add support for the ZynqMP Generic QSPI, Francisco Iglesias, 2017/11/26
- Re: [Qemu-devel] [PATCH v9 00/13] Add support for the ZynqMP Generic QSPI, Peter Maydell, 2017/11/28