From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nicolas Ferre Subject: Re: [PATCH] spi: atmel: fix handling of cs_change set on non-last xfer Date: Tue, 26 Jan 2016 18:01:01 +0100 Message-ID: <56A7A64D.1070601@atmel.com> References: <1453080351-29593-1-git-send-email-mans@mansr.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: , To: Mans Rullgard , "Yang, Wenyou" , Mark Brown Return-path: In-Reply-To: <1453080351-29593-1-git-send-email-mans-2StjZFpD7GcAvxtiuMwx3w@public.gmane.org> Sender: linux-spi-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-ID: Le 18/01/2016 02:25, Mans Rullgard a =E9crit : > 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 where= as > it should be briefly toggling CS off and on again. >=20 > 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. >=20 > Fixes: 8090d6d1a415 ("spi: atmel: Refactor spi-atmel to use SPI frame= work queue") > Signed-off-by: Mans Rullgard Hi Mans, Yes, it seems to be a sensible thing to do: Acked-by: Nicolas Ferre Thanks, best regards. > --- > drivers/spi/spi-atmel.c | 10 +++------- > 1 file changed, 3 insertions(+), 7 deletions(-) >=20 > 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; > =20 > bool keep_cs; > - bool cs_active; > =20 > u32 fifo_size; > }; > @@ -1406,11 +1405,9 @@ static int atmel_spi_one_transfer(struct spi_m= aster *master, > &msg->transfers)) { > as->keep_cs =3D true; > } else { > - as->cs_active =3D !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); > } > } > =20 > @@ -1433,7 +1430,6 @@ static int atmel_spi_transfer_one_message(struc= t spi_master *master, > atmel_spi_lock(as); > cs_activate(as, spi); > =20 > - as->cs_active =3D true; > as->keep_cs =3D false; > =20 > msg->status =3D 0; >=20 --=20 Nicolas Ferre -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934655AbcAZRBL (ORCPT ); Tue, 26 Jan 2016 12:01:11 -0500 Received: from eusmtp01.atmel.com ([212.144.249.243]:55052 "EHLO eusmtp01.atmel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932284AbcAZRBG (ORCPT ); Tue, 26 Jan 2016 12:01:06 -0500 Subject: Re: [PATCH] spi: atmel: fix handling of cs_change set on non-last xfer To: Mans Rullgard , "Yang, Wenyou" , Mark Brown References: <1453080351-29593-1-git-send-email-mans@mansr.com> CC: , From: Nicolas Ferre Organization: atmel Message-ID: <56A7A64D.1070601@atmel.com> Date: Tue, 26 Jan 2016 18:01:01 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <1453080351-29593-1-git-send-email-mans@mansr.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 8bit X-Originating-IP: [10.161.30.18] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 Hi Mans, Yes, it seems to be a sensible thing to do: Acked-by: Nicolas Ferre 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