[PATCH 1/2] spi: img-spfi: check for timeout error before proceeding

[PATCH 1/2] spi: img-spfi: check for timeout error before proceeding

[Sifan Naeem]

Calling spfi_wait_all_done is not required if the transfer has timed
out before all data is transferred.

spfi_wait_all_done polls for Alldone interrupt which is triggered to
mark the transfer as complete and to indicate it is now safe to issue
a new transfer.

Fixes: 8c2c8c0 ("spi: img-spfi: Control CS lines with GPIO")
Signed-off-by: Sifan Naeem <sifan.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
Cc: Stable kernel (v4.1) <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
---
 drivers/spi/spi-img-spfi.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-img-spfi.c b/drivers/spi/spi-img-spfi.c
index 00f1ac3..f27d9d5 100644
--- a/drivers/spi/spi-img-spfi.c
+++ b/drivers/spi/spi-img-spfi.c
@@ -267,15 +267,15 @@ static int img_spfi_start_pio(struct spi_master *master,
 		cpu_relax();
 	}
 
-	ret = spfi_wait_all_done(spfi);
-	if (ret < 0)
-		return ret;
-
 	if (rx_bytes > 0 || tx_bytes > 0) {
 		dev_err(spfi->dev, "PIO transfer timed out\n");
 		return -ETIMEDOUT;
 	}
 
+	ret = spfi_wait_all_done(spfi);
+	if (ret < 0)
+		return ret;
+
 	return 0;
 }
 
-- 
1.7.9.5

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

[PATCH 2/2] spi: img-spfi: fix spfi_setup by removing gpio_request_one

[Sifan Naeem]

spfi_setup may be called many times bye the spi framework, but
gpio_request_one can only be called once without freeing, repeatedly
calling gpio_request_one will cause an error to be thrown, which causes
the request to spi_setup to be marked as failed.

We can simply use gpio_direction_output to set the direction of the
gpio instead of gpio_request_one to put the gpio in to initial state,
after which the spi framework can control the chipselect line via gpio
using gpio_set_value.

Fixes: 8c2c8c0 ("spi: img-spfi: Control CS lines with GPIO")
Signed-off-by: Sifan Naeem <sifan.naeem@imgtec.com>
Cc: Stable kernel (v4.1) <stable@vger.kernel.org>
---
 drivers/spi/spi-img-spfi.c |   16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/spi-img-spfi.c b/drivers/spi/spi-img-spfi.c
index f27d9d5..a7f3d6a 100644
--- a/drivers/spi/spi-img-spfi.c
+++ b/drivers/spi/spi-img-spfi.c
@@ -442,13 +442,15 @@ static int img_spfi_setup(struct spi_device *spi)
 {
 	int ret;
 
-	ret = gpio_request_one(spi->cs_gpio, (spi->mode & SPI_CS_HIGH) ?
-			       GPIOF_OUT_INIT_LOW : GPIOF_OUT_INIT_HIGH,
-			       dev_name(&spi->dev));
-	if (ret)
-		dev_err(&spi->dev, "can't request chipselect gpio %d\n",
-				spi->cs_gpio);
-
+	if (gpio_is_valid(spi->cs_gpio)) {
+		int mode = ((spi->mode & SPI_CS_HIGH) ?
+			     GPIOF_OUT_INIT_LOW : GPIOF_OUT_INIT_HIGH);
+
+		ret = gpio_direction_output(spi->cs_gpio, mode);
+		if (ret)
+			dev_err(&spi->dev, "chipselect gpio %d setup failed (%d)\n",
+				spi->cs_gpio, ret);
+	}
 	return ret;
 }
 
-- 
1.7.9.5

Re: [PATCH 2/2] spi: img-spfi: fix spfi_setup by removing gpio_request_one

[Andrew Bresticker]

Hi Sifan,

On Mon, Jul 27, 2015 at 5:11 AM, Sifan Naeem <sifan.naeem@imgtec.com> wrote:
> spfi_setup may be called many times bye the spi framework, but
> gpio_request_one can only be called once without freeing, repeatedly
> calling gpio_request_one will cause an error to be thrown, which causes
> the request to spi_setup to be marked as failed.
>
> We can simply use gpio_direction_output to set the direction of the
> gpio instead of gpio_request_one to put the gpio in to initial state,
> after which the spi framework can control the chipselect line via gpio
> using gpio_set_value.

I don't think we want to leave the CS GPIOs un-requested.  Instead, we
could either request them all at probe() time (and then set direction
in setup()) or have a per-spi_device flag that indicates whether or
not the GPIO has been requested.

-Andrew

Re: [PATCH 2/2] spi: img-spfi: fix spfi_setup by removing gpio_request_one

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1010 bytes --]

On Mon, Jul 27, 2015 at 09:34:41AM -0700, Andrew Bresticker wrote:
> On Mon, Jul 27, 2015 at 5:11 AM, Sifan Naeem <sifan.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org> wrote:

> > spfi_setup may be called many times bye the spi framework, but
> > gpio_request_one can only be called once without freeing, repeatedly
> > calling gpio_request_one will cause an error to be thrown, which causes
> > the request to spi_setup to be marked as failed.

> > We can simply use gpio_direction_output to set the direction of the
> > gpio instead of gpio_request_one to put the gpio in to initial state,
> > after which the spi framework can control the chipselect line via gpio
> > using gpio_set_value.

> I don't think we want to leave the CS GPIOs un-requested.  Instead, we
> could either request them all at probe() time (and then set direction
> in setup()) or have a per-spi_device flag that indicates whether or
> not the GPIO has been requested.

Not requesting the GPIOs at all would just be a straight up bug.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

RE: [PATCH 2/2] spi: img-spfi: fix spfi_setup by removing gpio_request_one

[Sifan Naeem]

Thanks Mark, Andrew.

I'll rework this and submit a new patch.

Thanks,
Sifan

> -----Original Message-----
> From: Mark Brown [mailto:broonie@kernel.org]
> Sent: 27 July 2015 17:40
> To: Andrew Bresticker
> Cc: Sifan Naeem; linux-spi@vger.kernel.org; Stable kernel (v4.1)
> Subject: Re: [PATCH 2/2] spi: img-spfi: fix spfi_setup by removing
> gpio_request_one
> 
> On Mon, Jul 27, 2015 at 09:34:41AM -0700, Andrew Bresticker wrote:
> > On Mon, Jul 27, 2015 at 5:11 AM, Sifan Naeem <sifan.naeem@imgtec.com>
> wrote:
> 
> > > spfi_setup may be called many times bye the spi framework, but
> > > gpio_request_one can only be called once without freeing, repeatedly
> > > calling gpio_request_one will cause an error to be thrown, which
> > > causes the request to spi_setup to be marked as failed.
> 
> > > We can simply use gpio_direction_output to set the direction of the
> > > gpio instead of gpio_request_one to put the gpio in to initial
> > > state, after which the spi framework can control the chipselect line
> > > via gpio using gpio_set_value.
> 
> > I don't think we want to leave the CS GPIOs un-requested.  Instead, we
> > could either request them all at probe() time (and then set direction
> > in setup()) or have a per-spi_device flag that indicates whether or
> > not the GPIO has been requested.
> 
> Not requesting the GPIOs at all would just be a straight up bug.

RE: [PATCH 2/2] spi: img-spfi: fix spfi_setup by removing gpio_request_one

[Sifan Naeem]

Please find the new patch in http://marc.info/?l=linux-spi&m=143816718611912&w=2

Thanks,
Sifan

> -----Original Message-----
> From: Sifan Naeem
> Sent: 27 July 2015 17:54
> To: 'Mark Brown'; Andrew Bresticker
> Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Stable kernel (v4.1)
> Subject: RE: [PATCH 2/2] spi: img-spfi: fix spfi_setup by removing
> gpio_request_one
> 
> Thanks Mark, Andrew.
> 
> I'll rework this and submit a new patch.
> 
> Thanks,
> Sifan
> 
> > -----Original Message-----
> > From: Mark Brown [mailto:broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org]
> > Sent: 27 July 2015 17:40
> > To: Andrew Bresticker
> > Cc: Sifan Naeem; linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Stable kernel (v4.1)
> > Subject: Re: [PATCH 2/2] spi: img-spfi: fix spfi_setup by removing
> > gpio_request_one
> >
> > On Mon, Jul 27, 2015 at 09:34:41AM -0700, Andrew Bresticker wrote:
> > > On Mon, Jul 27, 2015 at 5:11 AM, Sifan Naeem
> > > <sifan.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
> > wrote:
> >
> > > > spfi_setup may be called many times bye the spi framework, but
> > > > gpio_request_one can only be called once without freeing,
> > > > repeatedly calling gpio_request_one will cause an error to be
> > > > thrown, which causes the request to spi_setup to be marked as failed.
> >
> > > > We can simply use gpio_direction_output to set the direction of
> > > > the gpio instead of gpio_request_one to put the gpio in to initial
> > > > state, after which the spi framework can control the chipselect
> > > > line via gpio using gpio_set_value.
> >
> > > I don't think we want to leave the CS GPIOs un-requested.  Instead,
> > > we could either request them all at probe() time (and then set
> > > direction in setup()) or have a per-spi_device flag that indicates
> > > whether or not the GPIO has been requested.
> >
> > Not requesting the GPIOs at all would just be a straight up bug.
--
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

Re: [PATCH 2/2] spi: img-spfi: fix spfi_setup by removing gpio_request_one

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 316 bytes --]

On Wed, Jul 29, 2015 at 11:05:05AM +0000, Sifan Naeem wrote:
> Please find the new patch in http://marc.info/?l=linux-spi&m=143816718611912&w=2
> 
> Thanks,
> Sifan
> 
> > -----Original Message-----

Please don't top post and please submit patches using the normal process
documented in SubmittingPatches.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

RE: [PATCH 2/2] spi: img-spfi: fix spfi_setup by removing gpio_request_one

[Sifan Naeem]

Hi Mark,

> On Wed, Jul 29, 2015 at 11:05:05AM +0000, Sifan Naeem wrote:
> > Please find the new patch in
> > http://marc.info/?l=linux-spi&m=143816718611912&w=2
> >
> > Thanks,
> > Sifan
> >
> > > -----Original Message-----
> 
> Please don't top post and please submit patches using the normal process
> documented in SubmittingPatches.

Noted, I did submit the modified patch following the normal process, but as the patch title changed I mentioned it here.

[PATCH v2] spi: img-spfi: fix multiple calls to request gpio

Thanks,
Sifan 
--
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

Re: [PATCH 1/2] spi: img-spfi: check for timeout error before proceeding

[Andrew Bresticker]

Hi Sifan,

On Mon, Jul 27, 2015 at 5:11 AM, Sifan Naeem <sifan.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org> wrote:
> Calling spfi_wait_all_done is not required if the transfer has timed
> out before all data is transferred.
>
> spfi_wait_all_done polls for Alldone interrupt which is triggered to
> mark the transfer as complete and to indicate it is now safe to issue
> a new transfer.
>
> Fixes: 8c2c8c0 ("spi: img-spfi: Control CS lines with GPIO")
> Signed-off-by: Sifan Naeem <sifan.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
> Cc: Stable kernel (v4.1) <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>

Reviewed-by: Andrew Bresticker <abrestic-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
--
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

Re: [PATCH 1/2] spi: img-spfi: check for timeout error before proceeding

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 244 bytes --]

On Mon, Jul 27, 2015 at 01:11:15PM +0100, Sifan Naeem wrote:

> Fixes: 8c2c8c0 ("spi: img-spfi: Control CS lines with GPIO")

Please use at least 12 characters of the hash, six is likely to generate
collisions on large repositories like Linux.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

RE: [PATCH 1/2] spi: img-spfi: check for timeout error before proceeding

[Sifan Naeem]

Hi Mark,

> -----Original Message-----
> From: Mark Brown [mailto:broonie@kernel.org]
> Sent: 29 July 2015 14:53
> To: Sifan Naeem
> Cc: linux-spi@vger.kernel.org; abrestic@chromium.org; Stable kernel (v4.1)
> Subject: Re: [PATCH 1/2] spi: img-spfi: check for timeout error before
> proceeding
> 
> On Mon, Jul 27, 2015 at 01:11:15PM +0100, Sifan Naeem wrote:
> 
> > Fixes: 8c2c8c0 ("spi: img-spfi: Control CS lines with GPIO")
> 
> Please use at least 12 characters of the hash, six is likely to generate collisions
> on large repositories like Linux.

v2 posted with 12 digit hash.

Thanks,
Sifan

Applied "spi: img-spfi: check for timeout error before proceeding" to the spi tree

[Mark Brown]

The patch

   spi: img-spfi: check for timeout error before proceeding

has been applied to the spi tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 011710e2ab659c7ad6e5e554806414bd7a9508be Mon Sep 17 00:00:00 2001
From: Sifan Naeem <sifan.naeem@imgtec.com>
Date: Mon, 27 Jul 2015 13:11:15 +0100
Subject: [PATCH] spi: img-spfi: check for timeout error before proceeding

Calling spfi_wait_all_done is not required if the transfer has timed
out before all data is transferred.

spfi_wait_all_done polls for Alldone interrupt which is triggered to
mark the transfer as complete and to indicate it is now safe to issue
a new transfer.

Fixes: 8c2c8c0 ("spi: img-spfi: Control CS lines with GPIO")
Signed-off-by: Sifan Naeem <sifan.naeem@imgtec.com>
Reviewed-by: Andrew Bresticker <abrestic@chromium.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
Cc: stable@vger.kernel.org
---
 drivers/spi/spi-img-spfi.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-img-spfi.c b/drivers/spi/spi-img-spfi.c
index 788e2b1..83b9741 100644
--- a/drivers/spi/spi-img-spfi.c
+++ b/drivers/spi/spi-img-spfi.c
@@ -266,15 +266,15 @@ static int img_spfi_start_pio(struct spi_master *master,
 		cpu_relax();
 	}
 
-	ret = spfi_wait_all_done(spfi);
-	if (ret < 0)
-		return ret;
-
 	if (rx_bytes > 0 || tx_bytes > 0) {
 		dev_err(spfi->dev, "PIO transfer timed out\n");
 		return -ETIMEDOUT;
 	}
 
+	ret = spfi_wait_all_done(spfi);
+	if (ret < 0)
+		return ret;
+
 	return 0;
 }
 
-- 
2.1.4