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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 30 Jul 2018 16:30:34 +0100
From:   Mark Brown <broonie@...nel.org>
To:     Andreas Färber <afaerber@...e.de>
Cc:     linux-mips@...ux-mips.org, Ralf Baechle <ralf@...ux-mips.org>,
        Paul Burton <paul.burton@...s.com>,
        James Hogan <jhogan@...nel.org>, linux-kernel@...r.kernel.org,
        Ionela Voinescu <ionela.voinescu@...tec.com>,
        Ezequiel Garcia <ezequiel.garcia@...tec.com>,
        linux-spi@...r.kernel.org
Subject: Re: [PATCH 10/15] spi: img-spfi: Implement dual and quad mode

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.  

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)