linux-kernel - Re: [PATCH v2 2/4] spi: add software implementation for SPI_CS

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]

Message-ID: <20180916123222.67136773@archlinux>
Date:   Sun, 16 Sep 2018 12:32:22 +0100
From:   Jonathan Cameron <jic23@...nel.org>
To:     David Lechner <david@...hnology.com>
Cc:     linux-spi@...r.kernel.org, linux-iio@...r.kernel.org,
        Hartmut Knaack <knaack.h@....de>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Peter Meerwald-Stadler <pmeerw@...erw.net>,
        Mark Brown <broonie@...nel.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/4] spi: add software implementation for SPI_CS_WORD

On Wed, 12 Sep 2018 19:39:18 -0500
David Lechner <david@...hnology.com> wrote:

> This adds a default software implementation for the SPI_CS_WORD flag for
> controllers that don't have such a feature.
> 
> The SPI_CS_WORD flag indicates that the CS line should be toggled
> between each word sent, not just between each transfer. The
> implementation works by using existing functions to split transfers into
> one-word-sized transfers and sets the cs_change bit for each of the
> new transfers.
> 
> Signed-off-by: David Lechner <david@...hnology.com>
Looks good to me but I'm not that familiar with this code..

Acked-by: Jonathan Cameron <Jonathan.Cameron@...wei.com>

> ---
>  drivers/spi/spi.c | 31 +++++++++++++++++++++++++++++++-
>  1 file changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index 9da0bc5a036c..84518ed58872 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -2783,8 +2783,10 @@ int spi_setup(struct spi_device *spi)
>  		return -EINVAL;
>  	/* help drivers fail *cleanly* when they need options
>  	 * that aren't supported with their current controller
> +	 * SPI_CS_WORD has a fallback software implementation,
> +	 * so it is ignored here.
>  	 */
> -	bad_bits = spi->mode & ~spi->controller->mode_bits;
> +	bad_bits = spi->mode & ~(spi->controller->mode_bits | SPI_CS_WORD);
>  	ugly_bits = bad_bits &
>  		    (SPI_TX_DUAL | SPI_TX_QUAD | SPI_RX_DUAL | SPI_RX_QUAD);
>  	if (ugly_bits) {
> @@ -2838,6 +2840,33 @@ static int __spi_validate(struct spi_device *spi, struct spi_message *message)
>  	if (list_empty(&message->transfers))
>  		return -EINVAL;
>  
> +	/* If an SPI controller does not support toggling the CS line on each
> +	 * transfer (indicated by the SPI_CS_WORD flag), we can emulate it by
> +	 * splitting transfers into one-word transfers and ensuring that
> +	 * cs_change is set for each transfer.
> +	 */
> +	if ((spi->mode & SPI_CS_WORD) && !(ctlr->mode_bits & SPI_CS_WORD)) {
> +		size_t maxsize;
> +		int ret;
> +
> +		maxsize = (spi->bits_per_word + 7) / 8;
> +
> +		/* spi_split_transfers_maxsize() requires message->spi */
> +		message->spi = spi;
> +
> +		ret = spi_split_transfers_maxsize(ctlr, message, maxsize,
> +						  GFP_KERNEL);
> +		if (ret)
> +			return ret;
> +
> +		list_for_each_entry(xfer, &message->transfers, transfer_list) {
> +			/* don't change cs_change on the last entry in the list */
> +			if (list_is_last(&xfer->transfer_list, &message->transfers))
> +				break;
> +			xfer->cs_change = 1;
> +		}
> +	}
> +
>  	/* Half-duplex links include original MicroWire, and ones with
>  	 * only one data pin like SPI_3WIRE (switches direction) or where
>  	 * either MOSI or MISO is missing.  They can also be caused by