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

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1308 bytes --]

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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]