Re: [PATCH] spi: atmel: fix handling of cs_change set on non-last xfer

[Nicolas Ferre]

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@xxxxxxxxx>

Hi Mans,

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

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