Re: [PATCH 10/15] spi: img-spfi: Implement dual and quad mode

From: Mark Brown
Date: Mon Jul 30 2018 - 11:30:44 EST


On Sun, Jul 22, 2018 at 11:20:05PM +0200, Andreas Färber wrote:

> #define SPFI_CONTROL_GET_DMA BIT(9)
> -#define SPFI_CONTROL_SE BIT(8)
> +#define SPFI_CONTROL_SE BIT(8)
> +#define SPFI_CONTROL_TX_RX BIT(1)

Random reindent of _SE there?

> + /*
> + * Disable SPFI for it not to interfere with
> + * pending transactions
> + */
> + spfi_writel(spfi, spfi_readl(spfi, SPFI_CONTROL)
> + & ~SPFI_CONTROL_SPFI_EN, SPFI_CONTROL);
> return 0;

The indentation on the second line of the write is very confusing, it
should be indented relative to the first line.

> + if (!list_is_last(&xfer->transfer_list, &master->cur_msg->transfers) &&
> + /*
> + * For duplex mode (both the tx and rx buffers are !NULL) the
> + * CMD, ADDR, and DUMMY byte parts of the transaction register
> + * should always be 0 and therefore the pending transfer
> + * technique cannot be used.
> + */
> + (xfer->tx_buf) && (!xfer->rx_buf) &&
> + (xfer->len <= SPFI_DATA_REQUEST_MAX_SIZE) && !is_pending) {
> + transact = (1 & SPFI_TRANSACTION_CMD_MASK) <<

This is again *really* hard to read - having the comment in the middle
of the condidional for the if statement, then indenting the code within
the if statement to the same depth is just super confusing.

Attachment: signature.asc
Description: PGP signature