linux-kernel - Re: [PATCH] spi: atmel: fix handling of 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] [day] [month] [year] [list]

Date:	Tue, 26 Jan 2016 18:01:01 +0100
From:	Nicolas Ferre <nicolas.ferre@...el.com>
To:	Mans Rullgard <mans@...sr.com>,
	"Yang, Wenyou" <Wenyou.Yang@...el.com>,
	Mark Brown <broonie@...nel.org>
CC:	<linux-spi@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] spi: atmel: fix handling of cs_change set on non-last
 xfer

Le 18/01/2016 02:25, Mans Rullgard a écrit :
> The driver does the wrong thing when cs_change is set on a non-last
> xfer in a message.  When cs_change is set, the driver deactivates the
> CS and leaves it off until a later xfer again has cs_change set whereas
> it should be briefly toggling CS off and on again.
> 
> This patch brings the behaviour of the driver back in line with the
> documentation and common sense.  The delay of 10 us is the same as is
> used by the default spi_transfer_one_message() function in spi.c.
> 
> Fixes: 8090d6d1a415 ("spi: atmel: Refactor spi-atmel to use SPI framework queue")
> Signed-off-by: Mans Rullgard <mans@...sr.com>

Hi Mans,

Yes, it seems to be a sensible thing to do:
Acked-by: Nicolas Ferre <nicolas.ferre@...el.com>

Thanks, best regards.

> ---
>  drivers/spi/spi-atmel.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
> index aebad36391c9..4b8ccf1f897e 100644
> --- a/drivers/spi/spi-atmel.c
> +++ b/drivers/spi/spi-atmel.c
> @@ -315,7 +315,6 @@ struct atmel_spi {
>  	struct atmel_spi_dma	dma;
>  
>  	bool			keep_cs;
> -	bool			cs_active;
>  
>  	u32			fifo_size;
>  };
> @@ -1406,11 +1405,9 @@ static int atmel_spi_one_transfer(struct spi_master *master,
>  				 &msg->transfers)) {
>  			as->keep_cs = true;
>  		} else {
> -			as->cs_active = !as->cs_active;
> -			if (as->cs_active)
> -				cs_activate(as, msg->spi);
> -			else
> -				cs_deactivate(as, msg->spi);
> +			cs_deactivate(as, msg->spi);
> +			udelay(10);
> +			cs_activate(as, msg->spi);
>  		}
>  	}
>  
> @@ -1433,7 +1430,6 @@ static int atmel_spi_transfer_one_message(struct spi_master *master,
>  	atmel_spi_lock(as);
>  	cs_activate(as, spi);
>  
> -	as->cs_active = true;
>  	as->keep_cs = false;
>  
>  	msg->status = 0;
> 


-- 
Nicolas Ferre