[PATCH v2] spi/imx: Fix spi-imx when the hardware SPI chipselects are used

linux-spi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH v2] spi/imx: Fix spi-imx when the hardware SPI chipselects are used
@ 2011-09-15 18:28 Fabio Estevam
       [not found] ` <1316111337-13485-1-git-send-email-fabio.estevam-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
  2011-09-15 21:01 ` Grant Likely
  0 siblings, 2 replies; 6+ messages in thread
From: Fabio Estevam @ 2011-09-15 18:28 UTC (permalink / raw)
  To: spi-devel-general
  Cc: shawn.guo, grant.likely, kernel, linux-arm-kernel, Fabio Estevam

commit 22a85e4cd51 (spi/imx: add device tree probe support) broke spi-imx usage
when the SPI chipselect is the one internal to the controller.

On a mx31pdk board the following error is seen:

Registering mxc_nand as whole device
------------[ cut here ]------------
WARNING: at drivers/gpio/gpiolib.c:101 gpio_ensure_requested+0x4c/0xf4()
autorequest GPIO-0
Modules linked in:
[<c0014410>] (unwind_backtrace+0x0/0xf4) from [<c0025754>] (warn_slowpath_common+0x4c/0x64)
[<c0025754>] (warn_slowpath_common+0x4c/0x64) from [<c0025800>] (warn_slowpath_fmt+0x30/0x40)
[<c0025800>] (warn_slowpath_fmt+0x30/0x40) from [<c0198688>] (gpio_ensure_requested+0x4c/0xf4)
[<c0198688>] (gpio_ensure_requested+0x4c/0xf4) from [<c01988c8>] (gpio_direction_output+0xa0/0x138)
[<c01988c8>] (gpio_direction_output+0xa0/0x138) from [<c01ed198>] (spi_imx_setup+0x38/0x4c)
[<c01ed198>] (spi_imx_setup+0x38/0x4c) from [<c01eb5d0>] (spi_setup+0x38/0x50)
[<c01eb5d0>] (spi_setup+0x38/0x50) from [<c01eb85c>] (spi_add_device+0x94/0x124)
[<c01eb85c>] (spi_add_device+0x94/0x124) from [<c01eb960>] (spi_new_device+0x74/0xac)
[<c01eb960>] (spi_new_device+0x74/0xac) from [<c01eb9b8>] (spi_match_master_to_boardinfo+0x20/0x40)
[<c01eb9b8>] (spi_match_master_to_boardinfo+0x20/0x40) from [<c01eba88>] (spi_register_master+0xb0/0x104)
[<c01eba88>] (spi_register_master+0xb0/0x104) from [<c01ec0b4>] (spi_bitbang_start+0x104/0x17c)
[<c01ec0b4>] (spi_bitbang_start+0x104/0x17c) from [<c02c2c4c>] (spi_imx_probe+0x2fc/0x404)
[<c02c2c4c>] (spi_imx_probe+0x2fc/0x404) from [<c01c2498>] (platform_drv_probe+0x18/0x1c)
[<c01c2498>] (platform_drv_probe+0x18/0x1c) from [<c01c1058>] (driver_probe_device+0x78/0x174)
[<c01c1058>] (driver_probe_device+0x78/0x174) from [<c01c11e0>] (__driver_attach+0x8c/0x90)
[<c01c11e0>] (__driver_attach+0x8c/0x90) from [<c01c0860>] (bus_for_each_dev+0x60/0x8c)
[<c01c0860>] (bus_for_each_dev+0x60/0x8c) from [<c01c0088>] (bus_add_driver+0xa0/0x288)
[<c01c0088>] (bus_add_driver+0xa0/0x288) from [<c01c179c>] (driver_register+0x78/0x18c)
[<c01c179c>] (driver_register+0x78/0x18c) from [<c0008490>] (do_one_initcall+0x34/0x178)
[<c0008490>] (do_one_initcall+0x34/0x178) from [<c03a5204>] (kernel_init+0x74/0x118)
[<c03a5204>] (kernel_init+0x74/0x118) from [<c000f65c>] (kernel_thread_exit+0x0/0x8)
---[ end trace 759f924b30fd5a44 ]---

Fix this issue by using the original chip select logic and make spi-imx to work again.

Tested on a mx31pdk that uses the hardware SPI chipselect pins and also
on a mx27pdk that uses GPIO as SPI chipselect.

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
Changes since v1:
- Fix the logic for the internal chip select case and
keep the usage of of_get_named_gpio for getting the cs_gpio

 drivers/spi/spi-imx.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index 8ac6542..d917fa3 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -784,11 +784,13 @@ static int __devinit spi_imx_probe(struct platform_device *pdev)
 
 	for (i = 0; i < master->num_chipselect; i++) {
 		int cs_gpio = of_get_named_gpio(np, "cs-gpios", i);
-		if (cs_gpio < 0)
+		if (cs_gpio < 0) {
 			cs_gpio = mxc_platform_info->chipselect[i];
+			spi_imx->chipselect[i] = cs_gpio;
+		}
 		if (cs_gpio < 0)
 			continue;
-		spi_imx->chipselect[i] = cs_gpio;
+
 		ret = gpio_request(spi_imx->chipselect[i], DRIVER_NAME);
 		if (ret) {
 			while (i > 0) {
-- 
1.7.1

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

[parent not found: <1316111337-13485-1-git-send-email-fabio.estevam-KZfg59tc24xl57MIdRCFDg@public.gmane.org>]

* Re: [PATCH v2] spi/imx: Fix spi-imx when the hardware SPI chipselects are used
       [not found] ` <1316111337-13485-1-git-send-email-fabio.estevam-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
@ 2011-09-15 19:52   ` Uwe Kleine-König
       [not found]     ` <20110915195243.GH11297-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
  0 siblings, 1 reply; 6+ messages in thread
From: Uwe Kleine-König @ 2011-09-15 19:52 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: shawn.guo-KZfg59tc24xl57MIdRCFDg,
	spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Hello Fabio,

On Thu, Sep 15, 2011 at 03:28:57PM -0300, Fabio Estevam wrote:
> diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
> index 8ac6542..d917fa3 100644
> --- a/drivers/spi/spi-imx.c
> +++ b/drivers/spi/spi-imx.c
> @@ -784,11 +784,13 @@ static int __devinit spi_imx_probe(struct platform_device *pdev)
>  
>  	for (i = 0; i < master->num_chipselect; i++) {
>  		int cs_gpio = of_get_named_gpio(np, "cs-gpios", i);
> -		if (cs_gpio < 0)
> +		if (cs_gpio < 0) {
>  			cs_gpio = mxc_platform_info->chipselect[i];
> +			spi_imx->chipselect[i] = cs_gpio;
> +		}
>  		if (cs_gpio < 0)
>  			continue;
> -		spi_imx->chipselect[i] = cs_gpio;
> +
>  		ret = gpio_request(spi_imx->chipselect[i], DRIVER_NAME);
>  		if (ret) {
>  			while (i > 0) {
I think this is wrong. In case of_get_named_gpio returns a gpio to use
spi_imx->chipselect[i] is unassigned.

I think you just need

diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index 8ac6542..fa594d6 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -786,9 +786,11 @@ static int __devinit spi_imx_probe(struct platform_device *pdev)
 		int cs_gpio = of_get_named_gpio(np, "cs-gpios", i);
 		if (cs_gpio < 0)
 			cs_gpio = mxc_platform_info->chipselect[i];
+
+		spi_imx->chipselect[i] = cs_gpio;
 		if (cs_gpio < 0)
 			continue;
-		spi_imx->chipselect[i] = cs_gpio;
+
 		ret = gpio_request(spi_imx->chipselect[i], DRIVER_NAME);
 		if (ret) {
 			while (i > 0) {

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

------------------------------------------------------------------------------
Doing More with Less: The Next Generation Virtual Desktop 
What are the key obstacles that have prevented many mid-market businesses
from deploying virtual desktops?   How do next-generation virtual desktops
provide companies an easier-to-deploy, easier-to-manage and more affordable
virtual desktop model.http://www.accelacomm.com/jaw/sfnl/114/51426474/

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

[parent not found: <20110915195243.GH11297-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>]

* Re: [PATCH v2] spi/imx: Fix spi-imx when the hardware SPI chipselects are used
       [not found]     ` <20110915195243.GH11297-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
@ 2011-09-15 20:19       ` Uwe Kleine-König
  2011-09-15 21:01         ` Grant Likely
  0 siblings, 1 reply; 6+ messages in thread
From: Uwe Kleine-König @ 2011-09-15 20:19 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ, shawn.guo-KZfg59tc24xl57MIdRCFDg,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Hello again,

On Thu, Sep 15, 2011 at 09:52:43PM +0200, Uwe Kleine-König wrote:
> diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
> index 8ac6542..fa594d6 100644
> --- a/drivers/spi/spi-imx.c
> +++ b/drivers/spi/spi-imx.c
> @@ -786,9 +786,11 @@ static int __devinit spi_imx_probe(struct platform_device *pdev)
>  		int cs_gpio = of_get_named_gpio(np, "cs-gpios", i);
>  		if (cs_gpio < 0)
>  			cs_gpio = mxc_platform_info->chipselect[i];
> +
> +		spi_imx->chipselect[i] = cs_gpio;
>  		if (cs_gpio < 0)
>  			continue;
> -		spi_imx->chipselect[i] = cs_gpio;
> +
>  		ret = gpio_request(spi_imx->chipselect[i], DRIVER_NAME);
>  		if (ret) {
>  			while (i > 0) {
Having said that I wonder how to specify to use an internal chipselect
via DT?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

------------------------------------------------------------------------------
Doing More with Less: The Next Generation Virtual Desktop 
What are the key obstacles that have prevented many mid-market businesses
from deploying virtual desktops?   How do next-generation virtual desktops
provide companies an easier-to-deploy, easier-to-manage and more affordable
virtual desktop model.http://www.accelacomm.com/jaw/sfnl/114/51426474/

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

* Re: [PATCH v2] spi/imx: Fix spi-imx when the hardware SPI chipselects are used
  2011-09-15 20:19       ` Uwe Kleine-König
@ 2011-09-15 21:01         ` Grant Likely
  0 siblings, 0 replies; 6+ messages in thread
From: Grant Likely @ 2011-09-15 21:01 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Fabio Estevam, spi-devel-general, kernel, shawn.guo,
	linux-arm-kernel

On Thu, Sep 15, 2011 at 10:19:09PM +0200, Uwe Kleine-König wrote:
> Hello again,
> 
> On Thu, Sep 15, 2011 at 09:52:43PM +0200, Uwe Kleine-König wrote:
> > diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
> > index 8ac6542..fa594d6 100644
> > --- a/drivers/spi/spi-imx.c
> > +++ b/drivers/spi/spi-imx.c
> > @@ -786,9 +786,11 @@ static int __devinit spi_imx_probe(struct platform_device *pdev)
> >  		int cs_gpio = of_get_named_gpio(np, "cs-gpios", i);
> >  		if (cs_gpio < 0)
> >  			cs_gpio = mxc_platform_info->chipselect[i];
> > +
> > +		spi_imx->chipselect[i] = cs_gpio;
> >  		if (cs_gpio < 0)
> >  			continue;
> > -		spi_imx->chipselect[i] = cs_gpio;
> > +
> >  		ret = gpio_request(spi_imx->chipselect[i], DRIVER_NAME);
> >  		if (ret) {
> >  			while (i > 0) {
> Having said that I wonder how to specify to use an internal chipselect
> via DT?

It would be done with a blank gpio specifier which is supported by the GPIO binding.

g.

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

* Re: [PATCH v2] spi/imx: Fix spi-imx when the hardware SPI chipselects are used
  2011-09-15 18:28 [PATCH v2] spi/imx: Fix spi-imx when the hardware SPI chipselects are used Fabio Estevam
       [not found] ` <1316111337-13485-1-git-send-email-fabio.estevam-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
@ 2011-09-15 21:01 ` Grant Likely
       [not found]   ` <20110915210108.GG3523-e0URQFbLeQY2iJbIjFUEsiwD8/FfD2ys@public.gmane.org>
  1 sibling, 1 reply; 6+ messages in thread
From: Grant Likely @ 2011-09-15 21:01 UTC (permalink / raw)
  To: Fabio Estevam; +Cc: shawn.guo, spi-devel-general, kernel, linux-arm-kernel

On Thu, Sep 15, 2011 at 03:28:57PM -0300, Fabio Estevam wrote:
> commit 22a85e4cd51 (spi/imx: add device tree probe support) broke spi-imx usage
> when the SPI chipselect is the one internal to the controller.
> 
> On a mx31pdk board the following error is seen:
> 
> Registering mxc_nand as whole device
> ------------[ cut here ]------------
> WARNING: at drivers/gpio/gpiolib.c:101 gpio_ensure_requested+0x4c/0xf4()
> autorequest GPIO-0
> Modules linked in:
> [<c0014410>] (unwind_backtrace+0x0/0xf4) from [<c0025754>] (warn_slowpath_common+0x4c/0x64)
> [<c0025754>] (warn_slowpath_common+0x4c/0x64) from [<c0025800>] (warn_slowpath_fmt+0x30/0x40)
> [<c0025800>] (warn_slowpath_fmt+0x30/0x40) from [<c0198688>] (gpio_ensure_requested+0x4c/0xf4)
> [<c0198688>] (gpio_ensure_requested+0x4c/0xf4) from [<c01988c8>] (gpio_direction_output+0xa0/0x138)
> [<c01988c8>] (gpio_direction_output+0xa0/0x138) from [<c01ed198>] (spi_imx_setup+0x38/0x4c)
> [<c01ed198>] (spi_imx_setup+0x38/0x4c) from [<c01eb5d0>] (spi_setup+0x38/0x50)
> [<c01eb5d0>] (spi_setup+0x38/0x50) from [<c01eb85c>] (spi_add_device+0x94/0x124)
> [<c01eb85c>] (spi_add_device+0x94/0x124) from [<c01eb960>] (spi_new_device+0x74/0xac)
> [<c01eb960>] (spi_new_device+0x74/0xac) from [<c01eb9b8>] (spi_match_master_to_boardinfo+0x20/0x40)
> [<c01eb9b8>] (spi_match_master_to_boardinfo+0x20/0x40) from [<c01eba88>] (spi_register_master+0xb0/0x104)
> [<c01eba88>] (spi_register_master+0xb0/0x104) from [<c01ec0b4>] (spi_bitbang_start+0x104/0x17c)
> [<c01ec0b4>] (spi_bitbang_start+0x104/0x17c) from [<c02c2c4c>] (spi_imx_probe+0x2fc/0x404)
> [<c02c2c4c>] (spi_imx_probe+0x2fc/0x404) from [<c01c2498>] (platform_drv_probe+0x18/0x1c)
> [<c01c2498>] (platform_drv_probe+0x18/0x1c) from [<c01c1058>] (driver_probe_device+0x78/0x174)
> [<c01c1058>] (driver_probe_device+0x78/0x174) from [<c01c11e0>] (__driver_attach+0x8c/0x90)
> [<c01c11e0>] (__driver_attach+0x8c/0x90) from [<c01c0860>] (bus_for_each_dev+0x60/0x8c)
> [<c01c0860>] (bus_for_each_dev+0x60/0x8c) from [<c01c0088>] (bus_add_driver+0xa0/0x288)
> [<c01c0088>] (bus_add_driver+0xa0/0x288) from [<c01c179c>] (driver_register+0x78/0x18c)
> [<c01c179c>] (driver_register+0x78/0x18c) from [<c0008490>] (do_one_initcall+0x34/0x178)
> [<c0008490>] (do_one_initcall+0x34/0x178) from [<c03a5204>] (kernel_init+0x74/0x118)
> [<c03a5204>] (kernel_init+0x74/0x118) from [<c000f65c>] (kernel_thread_exit+0x0/0x8)
> ---[ end trace 759f924b30fd5a44 ]---
> 
> Fix this issue by using the original chip select logic and make spi-imx to work again.
> 
> Tested on a mx31pdk that uses the hardware SPI chipselect pins and also
> on a mx27pdk that uses GPIO as SPI chipselect.
> 
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
> Changes since v1:
> - Fix the logic for the internal chip select case and
> keep the usage of of_get_named_gpio for getting the cs_gpio
> 
>  drivers/spi/spi-imx.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
> index 8ac6542..d917fa3 100644
> --- a/drivers/spi/spi-imx.c
> +++ b/drivers/spi/spi-imx.c
> @@ -784,11 +784,13 @@ static int __devinit spi_imx_probe(struct platform_device *pdev)
>  
>  	for (i = 0; i < master->num_chipselect; i++) {
>  		int cs_gpio = of_get_named_gpio(np, "cs-gpios", i);
> -		if (cs_gpio < 0)
> +		if (cs_gpio < 0) {
>  			cs_gpio = mxc_platform_info->chipselect[i];
> +			spi_imx->chipselect[i] = cs_gpio;
> +		}
>  		if (cs_gpio < 0)
>  			continue;
> -		spi_imx->chipselect[i] = cs_gpio;
> +

It looks like the removal of this line will break DT users.  Perhaps
the spi_imx->chipselect[i] line should just be moved before the
'if (cp_gpio < 0)' line.

g.


>  		ret = gpio_request(spi_imx->chipselect[i], DRIVER_NAME);
>  		if (ret) {
>  			while (i > 0) {
> -- 
> 1.7.1
> 
> 

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

[parent not found: <20110915210108.GG3523-e0URQFbLeQY2iJbIjFUEsiwD8/FfD2ys@public.gmane.org>]

* Re: [PATCH v2] spi/imx: Fix spi-imx when the hardware SPI chipselects are used
       [not found]   ` <20110915210108.GG3523-e0URQFbLeQY2iJbIjFUEsiwD8/FfD2ys@public.gmane.org>
@ 2011-09-15 21:24     ` Fabio Estevam
  0 siblings, 0 replies; 6+ messages in thread
From: Fabio Estevam @ 2011-09-15 21:24 UTC (permalink / raw)
  To: Grant Likely
  Cc: Fabio Estevam, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ, shawn.guo-KZfg59tc24xl57MIdRCFDg,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Grant,

On Thu, Sep 15, 2011 at 6:01 PM, Grant Likely <grant.likely@secretlab.ca> wrote:
...
> It looks like the removal of this line will break DT users.  Perhaps
> the spi_imx->chipselect[i] line should just be moved before the
> 'if (cp_gpio < 0)' line.

Yes, this is addressed in v3 now.

Thanks,

Fabio Estevam

------------------------------------------------------------------------------
Doing More with Less: The Next Generation Virtual Desktop 
What are the key obstacles that have prevented many mid-market businesses
from deploying virtual desktops?   How do next-generation virtual desktops
provide companies an easier-to-deploy, easier-to-manage and more affordable
virtual desktop model.http://www.accelacomm.com/jaw/sfnl/114/51426474/
_______________________________________________
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general

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

end of thread, other threads:[~2011-09-15 21:24 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-09-15 18:28 [PATCH v2] spi/imx: Fix spi-imx when the hardware SPI chipselects are used Fabio Estevam
     [not found] ` <1316111337-13485-1-git-send-email-fabio.estevam-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
2011-09-15 19:52   ` Uwe Kleine-König
     [not found]     ` <20110915195243.GH11297-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2011-09-15 20:19       ` Uwe Kleine-König
2011-09-15 21:01         ` Grant Likely
2011-09-15 21:01 ` Grant Likely
     [not found]   ` <20110915210108.GG3523-e0URQFbLeQY2iJbIjFUEsiwD8/FfD2ys@public.gmane.org>
2011-09-15 21:24     ` Fabio Estevam

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).