All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] spi: davinci: request cs_gpio's from probe
@ 2014-09-12 14:54 ` Grygorii Strashko
  0 siblings, 0 replies; 12+ messages in thread
From: Grygorii Strashko @ 2014-09-12 14:54 UTC (permalink / raw)
  To: santosh.shilimkar-l0cyMroinI0, Mark Brown,
	linux-spi-u79uwXL29TY76Z2rM5mHXA
  Cc: m-karicheri2-l0cyMroinI0,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/,
	Grygorii Strashko

Now CS GPIOs are requested from struct spi_master.setup() callback
and that causes failures when Client SPI device is getting accessed
through SPIDEV driver. The failure happens, because .setup() callback
may be called many times from IOCTL handler and when it's called
second time gpio_request() will fail and return -EBUSY.

Hence, fix it by moving CS GPIOs requesting code in .probe().

Reported-by: Murali Karicheri <m-karicheri2-l0cyMroinI0@public.gmane.org>
Signed-off-by: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>
---
Tested using spidev_test utility.

 drivers/spi/spi-davinci.c | 34 +++++++++++++++++++++++++---------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/drivers/spi/spi-davinci.c b/drivers/spi/spi-davinci.c
index 48f1d26..d493507 100644
--- a/drivers/spi/spi-davinci.c
+++ b/drivers/spi/spi-davinci.c
@@ -397,24 +397,21 @@ static int davinci_spi_setup(struct spi_device *spi)
 	struct spi_master *master = spi->master;
 	struct device_node *np = spi->dev.of_node;
 	bool internal_cs = true;
-	unsigned long flags = GPIOF_DIR_OUT;
 
 	dspi = spi_master_get_devdata(spi->master);
 	pdata = &dspi->pdata;
 
-	flags |= (spi->mode & SPI_CS_HIGH) ? GPIOF_INIT_LOW : GPIOF_INIT_HIGH;
-
 	if (!(spi->mode & SPI_NO_CS)) {
 		if (np && (master->cs_gpios != NULL) && (spi->cs_gpio >= 0)) {
-			retval = gpio_request_one(spi->cs_gpio,
-						  flags, dev_name(&spi->dev));
+			retval = gpio_direction_output(
+				      spi->cs_gpio, !(spi->mode & SPI_CS_HIGH));
 			internal_cs = false;
 		} else if (pdata->chip_sel &&
 			   spi->chip_select < pdata->num_chipselect &&
 			   pdata->chip_sel[spi->chip_select] != SPI_INTERN_CS) {
 			spi->cs_gpio = pdata->chip_sel[spi->chip_select];
-			retval = gpio_request_one(spi->cs_gpio,
-						  flags, dev_name(&spi->dev));
+			retval = gpio_direction_output(
+				      spi->cs_gpio, !(spi->mode & SPI_CS_HIGH));
 			internal_cs = false;
 		}
 
@@ -441,8 +438,6 @@ static int davinci_spi_setup(struct spi_device *spi)
 
 static void davinci_spi_cleanup(struct spi_device *spi)
 {
-	if (spi->cs_gpio >= 0)
-		gpio_free(spi->cs_gpio);
 }
 
 static int davinci_spi_check_error(struct davinci_spi *dspi, int int_status)
@@ -967,6 +962,27 @@ static int davinci_spi_probe(struct platform_device *pdev)
 	if (dspi->version == SPI_VERSION_2)
 		dspi->bitbang.flags |= SPI_READY;
 
+	if (pdev->dev.of_node) {
+		int i;
+
+		for (i = 0; i < pdata->num_chipselect; i++) {
+			int cs_gpio = of_get_named_gpio(pdev->dev.of_node,
+							"cs-gpios", i);
+
+			if (cs_gpio == -EPROBE_DEFER) {
+				ret = cs_gpio;
+				goto free_clk;
+			}
+
+			if (gpio_is_valid(cs_gpio)) {
+				ret = devm_gpio_request(&pdev->dev, cs_gpio,
+							dev_name(&pdev->dev));
+				if (ret)
+					goto free_clk;
+			}
+		}
+	}
+
 	r = platform_get_resource(pdev, IORESOURCE_DMA, 0);
 	if (r)
 		dma_rx_chan = r->start;
-- 
1.9.1

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH] spi: davinci: request cs_gpio's from probe
@ 2014-09-12 14:54 ` Grygorii Strashko
  0 siblings, 0 replies; 12+ messages in thread
From: Grygorii Strashko @ 2014-09-12 14:54 UTC (permalink / raw)
  To: linux-arm-kernel

Now CS GPIOs are requested from struct spi_master.setup() callback
and that causes failures when Client SPI device is getting accessed
through SPIDEV driver. The failure happens, because .setup() callback
may be called many times from IOCTL handler and when it's called
second time gpio_request() will fail and return -EBUSY.

Hence, fix it by moving CS GPIOs requesting code in .probe().

Reported-by: Murali Karicheri <m-karicheri2@ti.com>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
Tested using spidev_test utility.

 drivers/spi/spi-davinci.c | 34 +++++++++++++++++++++++++---------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/drivers/spi/spi-davinci.c b/drivers/spi/spi-davinci.c
index 48f1d26..d493507 100644
--- a/drivers/spi/spi-davinci.c
+++ b/drivers/spi/spi-davinci.c
@@ -397,24 +397,21 @@ static int davinci_spi_setup(struct spi_device *spi)
 	struct spi_master *master = spi->master;
 	struct device_node *np = spi->dev.of_node;
 	bool internal_cs = true;
-	unsigned long flags = GPIOF_DIR_OUT;
 
 	dspi = spi_master_get_devdata(spi->master);
 	pdata = &dspi->pdata;
 
-	flags |= (spi->mode & SPI_CS_HIGH) ? GPIOF_INIT_LOW : GPIOF_INIT_HIGH;
-
 	if (!(spi->mode & SPI_NO_CS)) {
 		if (np && (master->cs_gpios != NULL) && (spi->cs_gpio >= 0)) {
-			retval = gpio_request_one(spi->cs_gpio,
-						  flags, dev_name(&spi->dev));
+			retval = gpio_direction_output(
+				      spi->cs_gpio, !(spi->mode & SPI_CS_HIGH));
 			internal_cs = false;
 		} else if (pdata->chip_sel &&
 			   spi->chip_select < pdata->num_chipselect &&
 			   pdata->chip_sel[spi->chip_select] != SPI_INTERN_CS) {
 			spi->cs_gpio = pdata->chip_sel[spi->chip_select];
-			retval = gpio_request_one(spi->cs_gpio,
-						  flags, dev_name(&spi->dev));
+			retval = gpio_direction_output(
+				      spi->cs_gpio, !(spi->mode & SPI_CS_HIGH));
 			internal_cs = false;
 		}
 
@@ -441,8 +438,6 @@ static int davinci_spi_setup(struct spi_device *spi)
 
 static void davinci_spi_cleanup(struct spi_device *spi)
 {
-	if (spi->cs_gpio >= 0)
-		gpio_free(spi->cs_gpio);
 }
 
 static int davinci_spi_check_error(struct davinci_spi *dspi, int int_status)
@@ -967,6 +962,27 @@ static int davinci_spi_probe(struct platform_device *pdev)
 	if (dspi->version == SPI_VERSION_2)
 		dspi->bitbang.flags |= SPI_READY;
 
+	if (pdev->dev.of_node) {
+		int i;
+
+		for (i = 0; i < pdata->num_chipselect; i++) {
+			int cs_gpio = of_get_named_gpio(pdev->dev.of_node,
+							"cs-gpios", i);
+
+			if (cs_gpio == -EPROBE_DEFER) {
+				ret = cs_gpio;
+				goto free_clk;
+			}
+
+			if (gpio_is_valid(cs_gpio)) {
+				ret = devm_gpio_request(&pdev->dev, cs_gpio,
+							dev_name(&pdev->dev));
+				if (ret)
+					goto free_clk;
+			}
+		}
+	}
+
 	r = platform_get_resource(pdev, IORESOURCE_DMA, 0);
 	if (r)
 		dma_rx_chan = r->start;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] spi: davinci: request cs_gpio's from probe
  2014-09-12 14:54 ` Grygorii Strashko
@ 2014-09-13 16:28     ` Mark Brown
  -1 siblings, 0 replies; 12+ messages in thread
From: Mark Brown @ 2014-09-13 16:28 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: santosh.shilimkar-l0cyMroinI0, linux-spi-u79uwXL29TY76Z2rM5mHXA,
	m-karicheri2-l0cyMroinI0,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/

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

On Fri, Sep 12, 2014 at 05:54:00PM +0300, Grygorii Strashko wrote:

>  static void davinci_spi_cleanup(struct spi_device *spi)
>  {
> -	if (spi->cs_gpio >= 0)
> -		gpio_free(spi->cs_gpio);
>  }

This function is now empty so should be removed.  I've applied for now
but please send a followup fixing this.

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] spi: davinci: request cs_gpio's from probe
@ 2014-09-13 16:28     ` Mark Brown
  0 siblings, 0 replies; 12+ messages in thread
From: Mark Brown @ 2014-09-13 16:28 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Sep 12, 2014 at 05:54:00PM +0300, Grygorii Strashko wrote:

>  static void davinci_spi_cleanup(struct spi_device *spi)
>  {
> -	if (spi->cs_gpio >= 0)
> -		gpio_free(spi->cs_gpio);
>  }

This function is now empty so should be removed.  I've applied for now
but please send a followup fixing this.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140913/07d71215/attachment.sig>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] spi: davinci: request cs_gpio's from probe
  2014-09-13 16:28     ` Mark Brown
@ 2014-09-15 13:06         ` Grygorii Strashko
  -1 siblings, 0 replies; 12+ messages in thread
From: Grygorii Strashko @ 2014-09-15 13:06 UTC (permalink / raw)
  To: Mark Brown
  Cc: santosh.shilimkar-l0cyMroinI0, linux-spi-u79uwXL29TY76Z2rM5mHXA,
	m-karicheri2-l0cyMroinI0,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/

On 09/13/2014 07:28 PM, Mark Brown wrote:
> On Fri, Sep 12, 2014 at 05:54:00PM +0300, Grygorii Strashko wrote:
> 
>>   static void davinci_spi_cleanup(struct spi_device *spi)
>>   {
>> -	if (spi->cs_gpio >= 0)
>> -		gpio_free(spi->cs_gpio);
>>   }
> 
> This function is now empty so should be removed.  I've applied for now
> but please send a followup fixing this.
> 

The function davinci_spi_cleanup() will be reused by following patch
"[PATCH v3] spi: davinci: add support for adding delay between word's transmissions"
http://www.spinics.net/lists/devicetree/msg49141.html

So, in broonie/spi.git/for-next it will not be empty.

Again, I've missed description of this dependency, sorry for that.

Regards,
-grygorii

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] spi: davinci: request cs_gpio's from probe
@ 2014-09-15 13:06         ` Grygorii Strashko
  0 siblings, 0 replies; 12+ messages in thread
From: Grygorii Strashko @ 2014-09-15 13:06 UTC (permalink / raw)
  To: linux-arm-kernel

On 09/13/2014 07:28 PM, Mark Brown wrote:
> On Fri, Sep 12, 2014 at 05:54:00PM +0300, Grygorii Strashko wrote:
> 
>>   static void davinci_spi_cleanup(struct spi_device *spi)
>>   {
>> -	if (spi->cs_gpio >= 0)
>> -		gpio_free(spi->cs_gpio);
>>   }
> 
> This function is now empty so should be removed.  I've applied for now
> but please send a followup fixing this.
> 

The function davinci_spi_cleanup() will be reused by following patch
"[PATCH v3] spi: davinci: add support for adding delay between word's transmissions"
http://www.spinics.net/lists/devicetree/msg49141.html

So, in broonie/spi.git/for-next it will not be empty.

Again, I've missed description of this dependency, sorry for that.

Regards,
-grygorii

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] spi: davinci: request cs_gpio's from probe
  2014-09-15 13:06         ` Grygorii Strashko
@ 2014-09-15 16:38             ` Mark Brown
  -1 siblings, 0 replies; 12+ messages in thread
From: Mark Brown @ 2014-09-15 16:38 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: santosh.shilimkar-l0cyMroinI0, linux-spi-u79uwXL29TY76Z2rM5mHXA,
	m-karicheri2-l0cyMroinI0,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/

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

On Mon, Sep 15, 2014 at 04:06:42PM +0300, Grygorii Strashko wrote:

> The function davinci_spi_cleanup() will be reused by following patch
> "[PATCH v3] spi: davinci: add support for adding delay between word's transmissions"
> http://www.spinics.net/lists/devicetree/msg49141.html

> So, in broonie/spi.git/for-next it will not be empty.

> Again, I've missed description of this dependency, sorry for that.

Don't do things like this, delete the function and then re-add it - it
makes the review simpler and it means that if the second patch doesn't
get applied (as will happen here for v3.17) then the code still looks
good.

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] spi: davinci: request cs_gpio's from probe
@ 2014-09-15 16:38             ` Mark Brown
  0 siblings, 0 replies; 12+ messages in thread
From: Mark Brown @ 2014-09-15 16:38 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Sep 15, 2014 at 04:06:42PM +0300, Grygorii Strashko wrote:

> The function davinci_spi_cleanup() will be reused by following patch
> "[PATCH v3] spi: davinci: add support for adding delay between word's transmissions"
> http://www.spinics.net/lists/devicetree/msg49141.html

> So, in broonie/spi.git/for-next it will not be empty.

> Again, I've missed description of this dependency, sorry for that.

Don't do things like this, delete the function and then re-add it - it
makes the review simpler and it means that if the second patch doesn't
get applied (as will happen here for v3.17) then the code still looks
good.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140915/88c49bd2/attachment.sig>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] spi: davinci: remove empty function davinci_spi_cleanup
  2014-09-15 16:38             ` Mark Brown
@ 2014-09-16 11:14                 ` Grygorii Strashko
  -1 siblings, 0 replies; 12+ messages in thread
From: Grygorii Strashko @ 2014-09-16 11:14 UTC (permalink / raw)
  To: santosh.shilimkar-l0cyMroinI0, Mark Brown,
	linux-spi-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/,
	Grygorii Strashko

Remove empty function davinci_spi_cleanup().

Signed-off-by: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>
---
 drivers/spi/spi-davinci.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/spi/spi-davinci.c b/drivers/spi/spi-davinci.c
index d493507..134fb6e 100644
--- a/drivers/spi/spi-davinci.c
+++ b/drivers/spi/spi-davinci.c
@@ -436,10 +436,6 @@ static int davinci_spi_setup(struct spi_device *spi)
 	return retval;
 }
 
-static void davinci_spi_cleanup(struct spi_device *spi)
-{
-}
-
 static int davinci_spi_check_error(struct davinci_spi *dspi, int int_status)
 {
 	struct device *sdev = dspi->bitbang.master->dev.parent;
@@ -951,7 +947,6 @@ static int davinci_spi_probe(struct platform_device *pdev)
 	master->num_chipselect = pdata->num_chipselect;
 	master->bits_per_word_mask = SPI_BPW_RANGE_MASK(2, 16);
 	master->setup = davinci_spi_setup;
-	master->cleanup = davinci_spi_cleanup;
 
 	dspi->bitbang.chipselect = davinci_spi_chipselect;
 	dspi->bitbang.setup_transfer = davinci_spi_setup_transfer;
-- 
1.9.1

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH] spi: davinci: remove empty function davinci_spi_cleanup
@ 2014-09-16 11:14                 ` Grygorii Strashko
  0 siblings, 0 replies; 12+ messages in thread
From: Grygorii Strashko @ 2014-09-16 11:14 UTC (permalink / raw)
  To: linux-arm-kernel

Remove empty function davinci_spi_cleanup().

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/spi/spi-davinci.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/spi/spi-davinci.c b/drivers/spi/spi-davinci.c
index d493507..134fb6e 100644
--- a/drivers/spi/spi-davinci.c
+++ b/drivers/spi/spi-davinci.c
@@ -436,10 +436,6 @@ static int davinci_spi_setup(struct spi_device *spi)
 	return retval;
 }
 
-static void davinci_spi_cleanup(struct spi_device *spi)
-{
-}
-
 static int davinci_spi_check_error(struct davinci_spi *dspi, int int_status)
 {
 	struct device *sdev = dspi->bitbang.master->dev.parent;
@@ -951,7 +947,6 @@ static int davinci_spi_probe(struct platform_device *pdev)
 	master->num_chipselect = pdata->num_chipselect;
 	master->bits_per_word_mask = SPI_BPW_RANGE_MASK(2, 16);
 	master->setup = davinci_spi_setup;
-	master->cleanup = davinci_spi_cleanup;
 
 	dspi->bitbang.chipselect = davinci_spi_chipselect;
 	dspi->bitbang.setup_transfer = davinci_spi_setup_transfer;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] spi: davinci: remove empty function davinci_spi_cleanup
  2014-09-16 11:14                 ` Grygorii Strashko
@ 2014-09-16 17:31                     ` Mark Brown
  -1 siblings, 0 replies; 12+ messages in thread
From: Mark Brown @ 2014-09-16 17:31 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: santosh.shilimkar-l0cyMroinI0, linux-spi-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/

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

On Tue, Sep 16, 2014 at 02:14:23PM +0300, Grygorii Strashko wrote:
> Remove empty function davinci_spi_cleanup().

Applied, thanks.

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] spi: davinci: remove empty function davinci_spi_cleanup
@ 2014-09-16 17:31                     ` Mark Brown
  0 siblings, 0 replies; 12+ messages in thread
From: Mark Brown @ 2014-09-16 17:31 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Sep 16, 2014 at 02:14:23PM +0300, Grygorii Strashko wrote:
> Remove empty function davinci_spi_cleanup().

Applied, thanks.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140916/f2c2a7a7/attachment.sig>

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2014-09-16 17:31 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-12 14:54 [PATCH] spi: davinci: request cs_gpio's from probe Grygorii Strashko
2014-09-12 14:54 ` Grygorii Strashko
     [not found] ` <1410533640-21469-1-git-send-email-grygorii.strashko-l0cyMroinI0@public.gmane.org>
2014-09-13 16:28   ` Mark Brown
2014-09-13 16:28     ` Mark Brown
     [not found]     ` <20140913162858.GG7960-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2014-09-15 13:06       ` Grygorii Strashko
2014-09-15 13:06         ` Grygorii Strashko
     [not found]         ` <5416E462.7010406-l0cyMroinI0@public.gmane.org>
2014-09-15 16:38           ` Mark Brown
2014-09-15 16:38             ` Mark Brown
     [not found]             ` <20140915163841.GU7960-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2014-09-16 11:14               ` [PATCH] spi: davinci: remove empty function davinci_spi_cleanup Grygorii Strashko
2014-09-16 11:14                 ` Grygorii Strashko
     [not found]                 ` <1410866063-13393-1-git-send-email-grygorii.strashko-l0cyMroinI0@public.gmane.org>
2014-09-16 17:31                   ` Mark Brown
2014-09-16 17:31                     ` Mark Brown

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.