public inbox for linux-mmc@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers
  2011-08-16 10:11 [PATCH 0/4 v3] mmc: tmio, sdhi: provide multiple " Simon Horman
@ 2011-08-16 10:11 ` Simon Horman
  2011-08-16 11:11   ` Guennadi Liakhovetski
  0 siblings, 1 reply; 23+ messages in thread
From: Simon Horman @ 2011-08-16 10:11 UTC (permalink / raw)
  To: linux-mmc, linux-sh
  Cc: Chris Ball, Guennadi Liakhovetski, Magnus Damm, Simon Horman

Make use of per-source irq handles if the
platform (data) has multiple irq sources.

Also, as suggested by Guennadi Liakhovetski,
add and use defines the index or irqs in platform data.

Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Magnus Damm <magnus.damm@gmail.com>
Signed-off-by: Simon Horman <horms@verge.net.au>

---

v3
* Update for changes to "mmc: tmio: Provide separate interrupt handlers"
* As suggested by Guennadi Liakhovetski:
  - Merge in patch "mmc: sdhi: Add defines for platform irq indexes"
  - Use an enum instead of defines for irq indexes

v2
* Update for changes to "mmc: tmio: Provide separate interrupt handlers"
* Make use of defines provided by
  "mmc: sdhi: Add defines for platform irq indexes"
* As suggested by Guennadi Liakhovetski:
  - Don't use a loop to initialise irq handlers, the unrolled version
    is easier on the eyes (and exactly the same number of lines of code!)
---
 drivers/mmc/host/sh_mobile_sdhi.c  |   60 ++++++++++++++++++++++-------------
 include/linux/mmc/sh_mobile_sdhi.h |    7 ++++
 2 files changed, 45 insertions(+), 22 deletions(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 774f643..d792705 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -96,7 +96,9 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
 	struct sh_mobile_sdhi_info *p = pdev->dev.platform_data;
 	struct tmio_mmc_host *host;
 	char clk_name[8];
-	int i, irq, ret;
+	int irq, ret;
+	irqreturn_t (*f)(int irq, void *devid);
+	bool multi_irq = false;
 
 	priv = kzalloc(sizeof(struct sh_mobile_sdhi), GFP_KERNEL);
 	if (priv == NULL) {
@@ -153,27 +155,33 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto eprobe;
 
-	for (i = 0; i < 3; i++) {
-		irq = platform_get_irq(pdev, i);
-		if (irq < 0) {
-			if (i) {
-				continue;
-			} else {
-				ret = irq;
-				goto eirq;
-			}
-		}
-		ret = request_irq(irq, tmio_mmc_irq, 0,
+	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
+	if (irq >= 0) {
+		multi_irq = true;
+		ret = request_irq(irq, tmio_mmc_sdio_irq, 0,
 				  dev_name(&pdev->dev), host);
-		if (ret) {
-			while (i--) {
-				irq = platform_get_irq(pdev, i);
-				if (irq >= 0)
-					free_irq(irq, host);
-			}
-			goto eirq;
-		}
+		if (ret)
+			goto eirq_sdio;
 	}
+
+	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
+	if (irq >= 0) {
+		multi_irq = true;
+		ret = request_irq(irq, tmio_mmc_sdcard_irq, 0,
+				  dev_name(&pdev->dev), host);
+		if (ret)
+			goto eirq_card_detect;
+	} else if (multi_irq)
+		goto eirq_card_detect;
+
+	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
+	if (irq < 0)
+		goto eirq_sdcard;
+	f = multi_irq ? tmio_mmc_card_detect_irq : tmio_mmc_irq;
+	ret = request_irq(irq, f, 0, dev_name(&pdev->dev), host);
+	if (ret)
+		goto eirq_sdcard;
+
 	dev_info(&pdev->dev, "%s base at 0x%08lx clock rate %u MHz\n",
 		 mmc_hostname(host->mmc), (unsigned long)
 		 (platform_get_resource(pdev,IORESOURCE_MEM, 0)->start),
@@ -181,7 +189,15 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
 
 	return ret;
 
-eirq:
+eirq_sdcard:
+	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
+	if (irq >= 0)
+		free_irq(irq, host);
+eirq_card_detect:
+	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
+	if (irq >= 0)
+		free_irq(irq, host);
+eirq_sdio:
 	tmio_mmc_host_remove(host);
 eprobe:
 	clk_disable(priv->clk);
@@ -203,7 +219,7 @@ static int sh_mobile_sdhi_remove(struct platform_device *pdev)
 
 	tmio_mmc_host_remove(host);
 
-	for (i = 0; i < 3; i++) {
+	for (i = 0; i < SH_MOBILE_SDHI_IRQ_MAX; i++) {
 		irq = platform_get_irq(pdev, i);
 		if (irq >= 0)
 			free_irq(irq, host);
diff --git a/include/linux/mmc/sh_mobile_sdhi.h b/include/linux/mmc/sh_mobile_sdhi.h
index bd50b36..5182496 100644
--- a/include/linux/mmc/sh_mobile_sdhi.h
+++ b/include/linux/mmc/sh_mobile_sdhi.h
@@ -3,6 +3,13 @@
 
 #include <linux/types.h>
 
+enum {
+	SH_MOBILE_SDHI_IRQ_SDCARD = 0,
+	SH_MOBILE_SDHI_IRQ_CARD_DETECT,
+	SH_MOBILE_SDHI_IRQ_SDIO,
+	SH_MOBILE_SDHI_IRQ_MAX
+};
+
 struct platform_device;
 struct tmio_mmc_data;
 
-- 
1.7.5.4


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

* Re: [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers
  2011-08-16 10:11 ` [PATCH 3/4] mmc: sdhi: Make use of per-source " Simon Horman
@ 2011-08-16 11:11   ` Guennadi Liakhovetski
  2011-08-16 11:51     ` Simon Horman
  0 siblings, 1 reply; 23+ messages in thread
From: Guennadi Liakhovetski @ 2011-08-16 11:11 UTC (permalink / raw)
  To: Simon Horman; +Cc: linux-mmc, linux-sh, Chris Ball, Magnus Damm

On Tue, 16 Aug 2011, Simon Horman wrote:

> Make use of per-source irq handles if the
> platform (data) has multiple irq sources.
> 
> Also, as suggested by Guennadi Liakhovetski,
> add and use defines the index or irqs in platform data.
> 
> Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Cc: Magnus Damm <magnus.damm@gmail.com>
> Signed-off-by: Simon Horman <horms@verge.net.au>
> 
> ---
> 
> v3
> * Update for changes to "mmc: tmio: Provide separate interrupt handlers"
> * As suggested by Guennadi Liakhovetski:
>   - Merge in patch "mmc: sdhi: Add defines for platform irq indexes"
>   - Use an enum instead of defines for irq indexes
> 
> v2
> * Update for changes to "mmc: tmio: Provide separate interrupt handlers"
> * Make use of defines provided by
>   "mmc: sdhi: Add defines for platform irq indexes"
> * As suggested by Guennadi Liakhovetski:
>   - Don't use a loop to initialise irq handlers, the unrolled version
>     is easier on the eyes (and exactly the same number of lines of code!)
> ---
>  drivers/mmc/host/sh_mobile_sdhi.c  |   60 ++++++++++++++++++++++-------------
>  include/linux/mmc/sh_mobile_sdhi.h |    7 ++++
>  2 files changed, 45 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
> index 774f643..d792705 100644
> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> @@ -96,7 +96,9 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
>  	struct sh_mobile_sdhi_info *p = pdev->dev.platform_data;
>  	struct tmio_mmc_host *host;
>  	char clk_name[8];
> -	int i, irq, ret;
> +	int irq, ret;
> +	irqreturn_t (*f)(int irq, void *devid);
> +	bool multi_irq = false;
>  
>  	priv = kzalloc(sizeof(struct sh_mobile_sdhi), GFP_KERNEL);
>  	if (priv == NULL) {
> @@ -153,27 +155,33 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
>  	if (ret < 0)
>  		goto eprobe;
>  
> -	for (i = 0; i < 3; i++) {
> -		irq = platform_get_irq(pdev, i);
> -		if (irq < 0) {
> -			if (i) {
> -				continue;
> -			} else {
> -				ret = irq;
> -				goto eirq;
> -			}
> -		}
> -		ret = request_irq(irq, tmio_mmc_irq, 0,
> +	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
> +	if (irq >= 0) {
> +		multi_irq = true;
> +		ret = request_irq(irq, tmio_mmc_sdio_irq, 0,
>  				  dev_name(&pdev->dev), host);
> -		if (ret) {
> -			while (i--) {
> -				irq = platform_get_irq(pdev, i);
> -				if (irq >= 0)
> -					free_irq(irq, host);
> -			}
> -			goto eirq;
> -		}
> +		if (ret)
> +			goto eirq_sdio;
>  	}
> +
> +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
> +	if (irq >= 0) {
> +		multi_irq = true;
> +		ret = request_irq(irq, tmio_mmc_sdcard_irq, 0,
> +				  dev_name(&pdev->dev), host);

Sorry, I still don't understand why you check "CARD_DETECT" and request 
"sdcard_irq." Am I missing something or was my comment not clear enough in 
the previous review?

> +		if (ret)
> +			goto eirq_card_detect;
> +	} else if (multi_irq)
> +		goto eirq_card_detect;

? Sorry, have you maybe missed my review?

> +
> +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
> +	if (irq < 0)
> +		goto eirq_sdcard;
> +	f = multi_irq ? tmio_mmc_card_detect_irq : tmio_mmc_irq;

Same here

Thanks
Guennadi

> +	ret = request_irq(irq, f, 0, dev_name(&pdev->dev), host);
> +	if (ret)
> +		goto eirq_sdcard;
> +
>  	dev_info(&pdev->dev, "%s base at 0x%08lx clock rate %u MHz\n",
>  		 mmc_hostname(host->mmc), (unsigned long)
>  		 (platform_get_resource(pdev,IORESOURCE_MEM, 0)->start),
> @@ -181,7 +189,15 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
>  
>  	return ret;
>  
> -eirq:
> +eirq_sdcard:
> +	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
> +	if (irq >= 0)
> +		free_irq(irq, host);
> +eirq_card_detect:
> +	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
> +	if (irq >= 0)
> +		free_irq(irq, host);
> +eirq_sdio:
>  	tmio_mmc_host_remove(host);
>  eprobe:
>  	clk_disable(priv->clk);
> @@ -203,7 +219,7 @@ static int sh_mobile_sdhi_remove(struct platform_device *pdev)
>  
>  	tmio_mmc_host_remove(host);
>  
> -	for (i = 0; i < 3; i++) {
> +	for (i = 0; i < SH_MOBILE_SDHI_IRQ_MAX; i++) {
>  		irq = platform_get_irq(pdev, i);
>  		if (irq >= 0)
>  			free_irq(irq, host);
> diff --git a/include/linux/mmc/sh_mobile_sdhi.h b/include/linux/mmc/sh_mobile_sdhi.h
> index bd50b36..5182496 100644
> --- a/include/linux/mmc/sh_mobile_sdhi.h
> +++ b/include/linux/mmc/sh_mobile_sdhi.h
> @@ -3,6 +3,13 @@
>  
>  #include <linux/types.h>
>  
> +enum {
> +	SH_MOBILE_SDHI_IRQ_SDCARD = 0,
> +	SH_MOBILE_SDHI_IRQ_CARD_DETECT,
> +	SH_MOBILE_SDHI_IRQ_SDIO,
> +	SH_MOBILE_SDHI_IRQ_MAX
> +};
> +
>  struct platform_device;
>  struct tmio_mmc_data;
>  
> -- 
> 1.7.5.4
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

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

* Re: [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers
  2011-08-16 11:11   ` Guennadi Liakhovetski
@ 2011-08-16 11:51     ` Simon Horman
  2011-08-16 12:40       ` Guennadi Liakhovetski
  0 siblings, 1 reply; 23+ messages in thread
From: Simon Horman @ 2011-08-16 11:51 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: linux-mmc, linux-sh, Chris Ball, Magnus Damm

On Tue, Aug 16, 2011 at 01:11:42PM +0200, Guennadi Liakhovetski wrote:
> On Tue, 16 Aug 2011, Simon Horman wrote:
> 
> > Make use of per-source irq handles if the
> > platform (data) has multiple irq sources.
> > 
> > Also, as suggested by Guennadi Liakhovetski,
> > add and use defines the index or irqs in platform data.
> > 
> > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > Cc: Magnus Damm <magnus.damm@gmail.com>
> > Signed-off-by: Simon Horman <horms@verge.net.au>
> > 
> > ---
> > 
> > v3
> > * Update for changes to "mmc: tmio: Provide separate interrupt handlers"
> > * As suggested by Guennadi Liakhovetski:
> >   - Merge in patch "mmc: sdhi: Add defines for platform irq indexes"
> >   - Use an enum instead of defines for irq indexes
> > 
> > v2
> > * Update for changes to "mmc: tmio: Provide separate interrupt handlers"
> > * Make use of defines provided by
> >   "mmc: sdhi: Add defines for platform irq indexes"
> > * As suggested by Guennadi Liakhovetski:
> >   - Don't use a loop to initialise irq handlers, the unrolled version
> >     is easier on the eyes (and exactly the same number of lines of code!)
> > ---
> >  drivers/mmc/host/sh_mobile_sdhi.c  |   60 ++++++++++++++++++++++-------------
> >  include/linux/mmc/sh_mobile_sdhi.h |    7 ++++
> >  2 files changed, 45 insertions(+), 22 deletions(-)
> > 
> > diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
> > index 774f643..d792705 100644
> > --- a/drivers/mmc/host/sh_mobile_sdhi.c
> > +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> > @@ -96,7 +96,9 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
> >  	struct sh_mobile_sdhi_info *p = pdev->dev.platform_data;
> >  	struct tmio_mmc_host *host;
> >  	char clk_name[8];
> > -	int i, irq, ret;
> > +	int irq, ret;
> > +	irqreturn_t (*f)(int irq, void *devid);
> > +	bool multi_irq = false;
> >  
> >  	priv = kzalloc(sizeof(struct sh_mobile_sdhi), GFP_KERNEL);
> >  	if (priv == NULL) {
> > @@ -153,27 +155,33 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
> >  	if (ret < 0)
> >  		goto eprobe;
> >  
> > -	for (i = 0; i < 3; i++) {
> > -		irq = platform_get_irq(pdev, i);
> > -		if (irq < 0) {
> > -			if (i) {
> > -				continue;
> > -			} else {
> > -				ret = irq;
> > -				goto eirq;
> > -			}
> > -		}
> > -		ret = request_irq(irq, tmio_mmc_irq, 0,
> > +	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
> > +	if (irq >= 0) {
> > +		multi_irq = true;
> > +		ret = request_irq(irq, tmio_mmc_sdio_irq, 0,
> >  				  dev_name(&pdev->dev), host);
> > -		if (ret) {
> > -			while (i--) {
> > -				irq = platform_get_irq(pdev, i);
> > -				if (irq >= 0)
> > -					free_irq(irq, host);
> > -			}
> > -			goto eirq;
> > -		}
> > +		if (ret)
> > +			goto eirq_sdio;
> >  	}
> > +
> > +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
> > +	if (irq >= 0) {
> > +		multi_irq = true;
> > +		ret = request_irq(irq, tmio_mmc_sdcard_irq, 0,
> > +				  dev_name(&pdev->dev), host);
> 
> Sorry, I still don't understand why you check "CARD_DETECT" and request 
> "sdcard_irq." Am I missing something or was my comment not clear enough in 
> the previous review?
> 
> > +		if (ret)
> > +			goto eirq_card_detect;
> > +	} else if (multi_irq)
> > +		goto eirq_card_detect;
> 
> ? Sorry, have you maybe missed my review?
> 
> > +
> > +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
> > +	if (irq < 0)
> > +		goto eirq_sdcard;
> > +	f = multi_irq ? tmio_mmc_card_detect_irq : tmio_mmc_irq;
> 
> Same here

Sorry, that is a bug.

As the code seems to work on my board I think it should be fixed by
inverting the values of SH_MOBILE_SDHI_IRQ_SDCARD and
SH_MOBILE_SDHI_IRQ_CARD_DETECT.

I will check that and repost.


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

* Re: [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers
  2011-08-16 11:51     ` Simon Horman
@ 2011-08-16 12:40       ` Guennadi Liakhovetski
  2011-08-16 13:45         ` Simon Horman
  0 siblings, 1 reply; 23+ messages in thread
From: Guennadi Liakhovetski @ 2011-08-16 12:40 UTC (permalink / raw)
  To: Simon Horman; +Cc: linux-mmc, linux-sh, Chris Ball, Magnus Damm

On Tue, 16 Aug 2011, Simon Horman wrote:

> On Tue, Aug 16, 2011 at 01:11:42PM +0200, Guennadi Liakhovetski wrote:
> > On Tue, 16 Aug 2011, Simon Horman wrote:
> > 
> > > Make use of per-source irq handles if the
> > > platform (data) has multiple irq sources.
> > > 
> > > Also, as suggested by Guennadi Liakhovetski,
> > > add and use defines the index or irqs in platform data.
> > > 
> > > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > > Cc: Magnus Damm <magnus.damm@gmail.com>
> > > Signed-off-by: Simon Horman <horms@verge.net.au>
> > > 
> > > ---
> > > 
> > > v3
> > > * Update for changes to "mmc: tmio: Provide separate interrupt handlers"
> > > * As suggested by Guennadi Liakhovetski:
> > >   - Merge in patch "mmc: sdhi: Add defines for platform irq indexes"
> > >   - Use an enum instead of defines for irq indexes
> > > 
> > > v2
> > > * Update for changes to "mmc: tmio: Provide separate interrupt handlers"
> > > * Make use of defines provided by
> > >   "mmc: sdhi: Add defines for platform irq indexes"
> > > * As suggested by Guennadi Liakhovetski:
> > >   - Don't use a loop to initialise irq handlers, the unrolled version
> > >     is easier on the eyes (and exactly the same number of lines of code!)
> > > ---
> > >  drivers/mmc/host/sh_mobile_sdhi.c  |   60 ++++++++++++++++++++++-------------
> > >  include/linux/mmc/sh_mobile_sdhi.h |    7 ++++
> > >  2 files changed, 45 insertions(+), 22 deletions(-)
> > > 
> > > diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
> > > index 774f643..d792705 100644
> > > --- a/drivers/mmc/host/sh_mobile_sdhi.c
> > > +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> > > @@ -96,7 +96,9 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
> > >  	struct sh_mobile_sdhi_info *p = pdev->dev.platform_data;
> > >  	struct tmio_mmc_host *host;
> > >  	char clk_name[8];
> > > -	int i, irq, ret;
> > > +	int irq, ret;
> > > +	irqreturn_t (*f)(int irq, void *devid);
> > > +	bool multi_irq = false;
> > >  
> > >  	priv = kzalloc(sizeof(struct sh_mobile_sdhi), GFP_KERNEL);
> > >  	if (priv == NULL) {
> > > @@ -153,27 +155,33 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
> > >  	if (ret < 0)
> > >  		goto eprobe;
> > >  
> > > -	for (i = 0; i < 3; i++) {
> > > -		irq = platform_get_irq(pdev, i);
> > > -		if (irq < 0) {
> > > -			if (i) {
> > > -				continue;
> > > -			} else {
> > > -				ret = irq;
> > > -				goto eirq;
> > > -			}
> > > -		}
> > > -		ret = request_irq(irq, tmio_mmc_irq, 0,
> > > +	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
> > > +	if (irq >= 0) {
> > > +		multi_irq = true;
> > > +		ret = request_irq(irq, tmio_mmc_sdio_irq, 0,
> > >  				  dev_name(&pdev->dev), host);
> > > -		if (ret) {
> > > -			while (i--) {
> > > -				irq = platform_get_irq(pdev, i);
> > > -				if (irq >= 0)
> > > -					free_irq(irq, host);
> > > -			}
> > > -			goto eirq;
> > > -		}
> > > +		if (ret)
> > > +			goto eirq_sdio;
> > >  	}
> > > +
> > > +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
> > > +	if (irq >= 0) {
> > > +		multi_irq = true;
> > > +		ret = request_irq(irq, tmio_mmc_sdcard_irq, 0,
> > > +				  dev_name(&pdev->dev), host);
> > 
> > Sorry, I still don't understand why you check "CARD_DETECT" and request 
> > "sdcard_irq." Am I missing something or was my comment not clear enough in 
> > the previous review?
> > 
> > > +		if (ret)
> > > +			goto eirq_card_detect;
> > > +	} else if (multi_irq)
> > > +		goto eirq_card_detect;
> > 
> > ? Sorry, have you maybe missed my review?
> > 
> > > +
> > > +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
> > > +	if (irq < 0)
> > > +		goto eirq_sdcard;
> > > +	f = multi_irq ? tmio_mmc_card_detect_irq : tmio_mmc_irq;
> > 
> > Same here
> 
> Sorry, that is a bug.
> 
> As the code seems to work on my board I think it should be fixed by
> inverting the values of SH_MOBILE_SDHI_IRQ_SDCARD and
> SH_MOBILE_SDHI_IRQ_CARD_DETECT.
> 
> I will check that and repost.

Right, SDIO works, /proc/interrupts has

  96:          5  sh7372-intca-level     sh_mobile_sdhi.0
  97:    4116138  sh7372-intca-level     sh_mobile_sdhi.0
  98:      44822  sh7372-intca-level     sh_mobile_sdhi.0

So, indeed, #0 is card-detect, #1 is sdcard, #2 is sdio, your enum is 
wrong and request_irq() swapped ISRs compensate for that mistake.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

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

* Re: [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers
  2011-08-16 12:40       ` Guennadi Liakhovetski
@ 2011-08-16 13:45         ` Simon Horman
  0 siblings, 0 replies; 23+ messages in thread
From: Simon Horman @ 2011-08-16 13:45 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: linux-mmc, linux-sh, Chris Ball, Magnus Damm

On Tue, Aug 16, 2011 at 02:40:25PM +0200, Guennadi Liakhovetski wrote:
> On Tue, 16 Aug 2011, Simon Horman wrote:
> 
> > On Tue, Aug 16, 2011 at 01:11:42PM +0200, Guennadi Liakhovetski wrote:
> > > On Tue, 16 Aug 2011, Simon Horman wrote:

[snip]

> > > Sorry, I still don't understand why you check "CARD_DETECT" and request 
> > > "sdcard_irq." Am I missing something or was my comment not clear enough in 
> > > the previous review?
> > > 
> > > > +		if (ret)
> > > > +			goto eirq_card_detect;
> > > > +	} else if (multi_irq)
> > > > +		goto eirq_card_detect;
> > > 
> > > ? Sorry, have you maybe missed my review?
> > > 
> > > > +
> > > > +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
> > > > +	if (irq < 0)
> > > > +		goto eirq_sdcard;
> > > > +	f = multi_irq ? tmio_mmc_card_detect_irq : tmio_mmc_irq;
> > > 
> > > Same here
> > 
> > Sorry, that is a bug.
> > 
> > As the code seems to work on my board I think it should be fixed by
> > inverting the values of SH_MOBILE_SDHI_IRQ_SDCARD and
> > SH_MOBILE_SDHI_IRQ_CARD_DETECT.
> > 
> > I will check that and repost.
> 
> Right, SDIO works, /proc/interrupts has
> 
>   96:          5  sh7372-intca-level     sh_mobile_sdhi.0
>   97:    4116138  sh7372-intca-level     sh_mobile_sdhi.0
>   98:      44822  sh7372-intca-level     sh_mobile_sdhi.0
> 
> So, indeed, #0 is card-detect, #1 is sdcard, #2 is sdio, your enum is 
> wrong and request_irq() swapped ISRs compensate for that mistake.

Thanks.

I swapped around the enum, made the appropriate changes elsewhere
and things do seem to work correctly. Its a bit late now so I'll hold
off on reposting until tomorrow.

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

* [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers
  2011-08-17  0:50 [PATCH 0/4 v4] mmc: tmio, sdhi: provide multiple " Simon Horman
@ 2011-08-17  0:50 ` Simon Horman
  2011-08-17  8:20   ` Guennadi Liakhovetski
  0 siblings, 1 reply; 23+ messages in thread
From: Simon Horman @ 2011-08-17  0:50 UTC (permalink / raw)
  To: linux-mmc, linux-sh
  Cc: Chris Ball, Paul Mundt, Guennadi Liakhovetski, Magnus Damm,
	Simon Horman

Make use of per-source irq handles if the
platform (data) has multiple irq sources.

Also, as suggested by Guennadi Liakhovetski,
add and use defines the index or irqs in platform data.

Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Magnus Damm <magnus.damm@gmail.com>
Signed-off-by: Simon Horman <horms@verge.net.au>

---

v4
* As suggested by Guennadi Liakhovetski:
  - Correct inverted values of SH_MOBILE_SDHI_IRQ_SDCARD and
    SH_MOBILE_SDHI_IRQ_CARD_DETECT

v3
* Update for changes to "mmc: tmio: Provide separate interrupt handlers"
* As suggested by Guennadi Liakhovetski:
  - Merge in patch "mmc: sdhi: Add defines for platform irq indexes"
  - Use an enum instead of defines for irq indexes

v2
* Update for changes to "mmc: tmio: Provide separate interrupt handlers"
* Make use of defines provided by
  "mmc: sdhi: Add defines for platform irq indexes"
* As suggested by Guennadi Liakhovetski:
  - Don't use a loop to initialise irq handlers, the unrolled version
    is easier on the eyes (and exactly the same number of lines of code!)
---
 drivers/mmc/host/sh_mobile_sdhi.c  |   60 ++++++++++++++++++++++-------------
 include/linux/mmc/sh_mobile_sdhi.h |    7 ++++
 2 files changed, 45 insertions(+), 22 deletions(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 774f643..619df30 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -96,7 +96,9 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
 	struct sh_mobile_sdhi_info *p = pdev->dev.platform_data;
 	struct tmio_mmc_host *host;
 	char clk_name[8];
-	int i, irq, ret;
+	int irq, ret;
+	irqreturn_t (*f)(int irq, void *devid);
+	bool multi_irq = false;
 
 	priv = kzalloc(sizeof(struct sh_mobile_sdhi), GFP_KERNEL);
 	if (priv == NULL) {
@@ -153,27 +155,33 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto eprobe;
 
-	for (i = 0; i < 3; i++) {
-		irq = platform_get_irq(pdev, i);
-		if (irq < 0) {
-			if (i) {
-				continue;
-			} else {
-				ret = irq;
-				goto eirq;
-			}
-		}
-		ret = request_irq(irq, tmio_mmc_irq, 0,
+	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
+	if (irq >= 0) {
+		multi_irq = true;
+		ret = request_irq(irq, tmio_mmc_sdio_irq, 0,
 				  dev_name(&pdev->dev), host);
-		if (ret) {
-			while (i--) {
-				irq = platform_get_irq(pdev, i);
-				if (irq >= 0)
-					free_irq(irq, host);
-			}
-			goto eirq;
-		}
+		if (ret)
+			goto eirq_sdio;
 	}
+
+	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
+	if (irq >= 0) {
+		multi_irq = true;
+		ret = request_irq(irq, tmio_mmc_sdcard_irq, 0,
+				  dev_name(&pdev->dev), host);
+		if (ret)
+			goto eirq_sdcard;
+	} else if (multi_irq)
+		goto eirq_sdcard;
+
+	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
+	if (irq < 0)
+		goto eirq_card_detect;
+	f = multi_irq ? tmio_mmc_card_detect_irq : tmio_mmc_irq;
+	ret = request_irq(irq, f, 0, dev_name(&pdev->dev), host);
+	if (ret)
+		goto eirq_card_detect;
+
 	dev_info(&pdev->dev, "%s base at 0x%08lx clock rate %u MHz\n",
 		 mmc_hostname(host->mmc), (unsigned long)
 		 (platform_get_resource(pdev,IORESOURCE_MEM, 0)->start),
@@ -181,7 +189,15 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
 
 	return ret;
 
-eirq:
+eirq_card_detect:
+	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
+	if (irq >= 0)
+		free_irq(irq, host);
+eirq_sdcard:
+	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
+	if (irq >= 0)
+		free_irq(irq, host);
+eirq_sdio:
 	tmio_mmc_host_remove(host);
 eprobe:
 	clk_disable(priv->clk);
@@ -203,7 +219,7 @@ static int sh_mobile_sdhi_remove(struct platform_device *pdev)
 
 	tmio_mmc_host_remove(host);
 
-	for (i = 0; i < 3; i++) {
+	for (i = 0; i < SH_MOBILE_SDHI_IRQ_MAX; i++) {
 		irq = platform_get_irq(pdev, i);
 		if (irq >= 0)
 			free_irq(irq, host);
diff --git a/include/linux/mmc/sh_mobile_sdhi.h b/include/linux/mmc/sh_mobile_sdhi.h
index bd50b36..80d3629 100644
--- a/include/linux/mmc/sh_mobile_sdhi.h
+++ b/include/linux/mmc/sh_mobile_sdhi.h
@@ -3,6 +3,13 @@
 
 #include <linux/types.h>
 
+enum {
+	SH_MOBILE_SDHI_IRQ_CARD_DETECT = 0,
+	SH_MOBILE_SDHI_IRQ_SDCARD,
+	SH_MOBILE_SDHI_IRQ_SDIO,
+	SH_MOBILE_SDHI_IRQ_MAX
+};
+
 struct platform_device;
 struct tmio_mmc_data;
 
-- 
1.7.5.4


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

* Re: [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers
  2011-08-17  0:50 ` [PATCH 3/4] mmc: sdhi: Make use of per-source " Simon Horman
@ 2011-08-17  8:20   ` Guennadi Liakhovetski
  2011-08-17  9:49     ` Simon Horman
  0 siblings, 1 reply; 23+ messages in thread
From: Guennadi Liakhovetski @ 2011-08-17  8:20 UTC (permalink / raw)
  To: Simon Horman; +Cc: linux-mmc, linux-sh, Chris Ball, Paul Mundt, Magnus Damm

On Wed, 17 Aug 2011, Simon Horman wrote:

> Make use of per-source irq handles if the
> platform (data) has multiple irq sources.
> 
> Also, as suggested by Guennadi Liakhovetski,
> add and use defines the index or irqs in platform data.
> 
> Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Cc: Magnus Damm <magnus.damm@gmail.com>
> Signed-off-by: Simon Horman <horms@verge.net.au>
> 
> ---
> 
> v4
> * As suggested by Guennadi Liakhovetski:
>   - Correct inverted values of SH_MOBILE_SDHI_IRQ_SDCARD and
>     SH_MOBILE_SDHI_IRQ_CARD_DETECT
> 
> v3
> * Update for changes to "mmc: tmio: Provide separate interrupt handlers"
> * As suggested by Guennadi Liakhovetski:
>   - Merge in patch "mmc: sdhi: Add defines for platform irq indexes"
>   - Use an enum instead of defines for irq indexes
> 
> v2
> * Update for changes to "mmc: tmio: Provide separate interrupt handlers"
> * Make use of defines provided by
>   "mmc: sdhi: Add defines for platform irq indexes"
> * As suggested by Guennadi Liakhovetski:
>   - Don't use a loop to initialise irq handlers, the unrolled version
>     is easier on the eyes (and exactly the same number of lines of code!)
> ---
>  drivers/mmc/host/sh_mobile_sdhi.c  |   60 ++++++++++++++++++++++-------------
>  include/linux/mmc/sh_mobile_sdhi.h |    7 ++++
>  2 files changed, 45 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
> index 774f643..619df30 100644
> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> @@ -96,7 +96,9 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
>  	struct sh_mobile_sdhi_info *p = pdev->dev.platform_data;
>  	struct tmio_mmc_host *host;
>  	char clk_name[8];
> -	int i, irq, ret;
> +	int irq, ret;
> +	irqreturn_t (*f)(int irq, void *devid);
> +	bool multi_irq = false;
>  
>  	priv = kzalloc(sizeof(struct sh_mobile_sdhi), GFP_KERNEL);
>  	if (priv == NULL) {
> @@ -153,27 +155,33 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
>  	if (ret < 0)
>  		goto eprobe;
>  
> -	for (i = 0; i < 3; i++) {
> -		irq = platform_get_irq(pdev, i);
> -		if (irq < 0) {
> -			if (i) {
> -				continue;
> -			} else {
> -				ret = irq;
> -				goto eirq;
> -			}
> -		}
> -		ret = request_irq(irq, tmio_mmc_irq, 0,
> +	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
> +	if (irq >= 0) {
> +		multi_irq = true;
> +		ret = request_irq(irq, tmio_mmc_sdio_irq, 0,
>  				  dev_name(&pdev->dev), host);
> -		if (ret) {
> -			while (i--) {
> -				irq = platform_get_irq(pdev, i);
> -				if (irq >= 0)
> -					free_irq(irq, host);
> -			}
> -			goto eirq;
> -		}
> +		if (ret)
> +			goto eirq_sdio;
>  	}
> +
> +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
> +	if (irq >= 0) {
> +		multi_irq = true;
> +		ret = request_irq(irq, tmio_mmc_sdcard_irq, 0,
> +				  dev_name(&pdev->dev), host);
> +		if (ret)
> +			goto eirq_sdcard;
> +	} else if (multi_irq)
> +		goto eirq_sdcard;
> +
> +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
> +	if (irq < 0)
> +		goto eirq_card_detect;
> +	f = multi_irq ? tmio_mmc_card_detect_irq : tmio_mmc_irq;
> +	ret = request_irq(irq, f, 0, dev_name(&pdev->dev), host);
> +	if (ret)
> +		goto eirq_card_detect;
> +

I still don't see why a multi-IRQ configuration without a card-detect IRQ 
like

static struct resource sdhi_resources[] = {
	[0] = {
		.name	= "SDHI2",
		...,
	},
	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
		.start	= ...,
		.flags	= IORESOURCE_IRQ,
	},
	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
		.start	= ...,
		.flags	= IORESOURCE_IRQ,
	},
};

should be invalid. Especially since we actually want to avoid using the 
controller card-detect IRQ for power efficiency and use a GPIO instead.

Thanks
Guennadi

>  	dev_info(&pdev->dev, "%s base at 0x%08lx clock rate %u MHz\n",
>  		 mmc_hostname(host->mmc), (unsigned long)
>  		 (platform_get_resource(pdev,IORESOURCE_MEM, 0)->start),
> @@ -181,7 +189,15 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
>  
>  	return ret;
>  
> -eirq:
> +eirq_card_detect:
> +	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
> +	if (irq >= 0)
> +		free_irq(irq, host);
> +eirq_sdcard:
> +	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
> +	if (irq >= 0)
> +		free_irq(irq, host);
> +eirq_sdio:
>  	tmio_mmc_host_remove(host);
>  eprobe:
>  	clk_disable(priv->clk);
> @@ -203,7 +219,7 @@ static int sh_mobile_sdhi_remove(struct platform_device *pdev)
>  
>  	tmio_mmc_host_remove(host);
>  
> -	for (i = 0; i < 3; i++) {
> +	for (i = 0; i < SH_MOBILE_SDHI_IRQ_MAX; i++) {
>  		irq = platform_get_irq(pdev, i);
>  		if (irq >= 0)
>  			free_irq(irq, host);
> diff --git a/include/linux/mmc/sh_mobile_sdhi.h b/include/linux/mmc/sh_mobile_sdhi.h
> index bd50b36..80d3629 100644
> --- a/include/linux/mmc/sh_mobile_sdhi.h
> +++ b/include/linux/mmc/sh_mobile_sdhi.h
> @@ -3,6 +3,13 @@
>  
>  #include <linux/types.h>
>  
> +enum {
> +	SH_MOBILE_SDHI_IRQ_CARD_DETECT = 0,
> +	SH_MOBILE_SDHI_IRQ_SDCARD,
> +	SH_MOBILE_SDHI_IRQ_SDIO,
> +	SH_MOBILE_SDHI_IRQ_MAX
> +};
> +
>  struct platform_device;
>  struct tmio_mmc_data;
>  
> -- 
> 1.7.5.4
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

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

* Re: [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers
  2011-08-17  8:20   ` Guennadi Liakhovetski
@ 2011-08-17  9:49     ` Simon Horman
  2011-08-17 10:06       ` Guennadi Liakhovetski
  0 siblings, 1 reply; 23+ messages in thread
From: Simon Horman @ 2011-08-17  9:49 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: linux-mmc, linux-sh, Chris Ball, Paul Mundt, Magnus Damm

On Wed, Aug 17, 2011 at 10:20:24AM +0200, Guennadi Liakhovetski wrote:
> On Wed, 17 Aug 2011, Simon Horman wrote:

[snip ]

> > +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
> > +	if (irq >= 0) {
> > +		multi_irq = true;
> > +		ret = request_irq(irq, tmio_mmc_sdcard_irq, 0,
> > +				  dev_name(&pdev->dev), host);
> > +		if (ret)
> > +			goto eirq_sdcard;
> > +	} else if (multi_irq)
> > +		goto eirq_sdcard;
> > +
> > +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
> > +	if (irq < 0)
> > +		goto eirq_card_detect;
> > +	f = multi_irq ? tmio_mmc_card_detect_irq : tmio_mmc_irq;
> > +	ret = request_irq(irq, f, 0, dev_name(&pdev->dev), host);
> > +	if (ret)
> > +		goto eirq_card_detect;
> > +
> 
> I still don't see why a multi-IRQ configuration without a card-detect IRQ 
> like
> 
> static struct resource sdhi_resources[] = {
> 	[0] = {
> 		.name	= "SDHI2",
> 		...,
> 	},
> 	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
> 		.start	= ...,
> 		.flags	= IORESOURCE_IRQ,
> 	},
> 	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
> 		.start	= ...,
> 		.flags	= IORESOURCE_IRQ,
> 	},
> };
> 
> should be invalid. Especially since we actually want to avoid using the 
> controller card-detect IRQ for power efficiency and use a GPIO instead.

Ok, in this case you would like SH_MOBILE_SDHI_IRQ_SDCARD to use
tmio_mmc_sdcard_irq() ?

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

* Re: [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers
  2011-08-17  9:49     ` Simon Horman
@ 2011-08-17 10:06       ` Guennadi Liakhovetski
  2011-08-17 10:46         ` Simon Horman
  0 siblings, 1 reply; 23+ messages in thread
From: Guennadi Liakhovetski @ 2011-08-17 10:06 UTC (permalink / raw)
  To: Simon Horman; +Cc: linux-mmc, linux-sh, Chris Ball, Paul Mundt, Magnus Damm

On Wed, 17 Aug 2011, Simon Horman wrote:

> On Wed, Aug 17, 2011 at 10:20:24AM +0200, Guennadi Liakhovetski wrote:
> > On Wed, 17 Aug 2011, Simon Horman wrote:
> 
> [snip ]
> 
> > > +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
> > > +	if (irq >= 0) {
> > > +		multi_irq = true;
> > > +		ret = request_irq(irq, tmio_mmc_sdcard_irq, 0,
> > > +				  dev_name(&pdev->dev), host);
> > > +		if (ret)
> > > +			goto eirq_sdcard;
> > > +	} else if (multi_irq)
> > > +		goto eirq_sdcard;
> > > +
> > > +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
> > > +	if (irq < 0)
> > > +		goto eirq_card_detect;
> > > +	f = multi_irq ? tmio_mmc_card_detect_irq : tmio_mmc_irq;
> > > +	ret = request_irq(irq, f, 0, dev_name(&pdev->dev), host);
> > > +	if (ret)
> > > +		goto eirq_card_detect;
> > > +
> > 
> > I still don't see why a multi-IRQ configuration without a card-detect IRQ 
> > like
> > 
> > static struct resource sdhi_resources[] = {
> > 	[0] = {
> > 		.name	= "SDHI2",
> > 		...,
> > 	},
> > 	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
> > 		.start	= ...,
> > 		.flags	= IORESOURCE_IRQ,
> > 	},
> > 	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
> > 		.start	= ...,
> > 		.flags	= IORESOURCE_IRQ,
> > 	},
> > };
> > 
> > should be invalid. Especially since we actually want to avoid using the 
> > controller card-detect IRQ for power efficiency and use a GPIO instead.
> 
> Ok, in this case you would like SH_MOBILE_SDHI_IRQ_SDCARD to use
> tmio_mmc_sdcard_irq() ?

This is a good question. I think your erroring out is wrong, but I'm not 
sure what is best here. Using sdcard and sdio ISR in this case seems most 
logical to me. But I don't know if we ever can get hardware, where we 
indeed only have two SDHI interrupts - one for SDIO and one for SDCARD and 
CARD_DETECT. I'm not aware of such hardware so far, so, yes, I'd go with 
SDIO and SDCARD ISRs for now. In any case, this is SDHI internal decision, 
so, we can change it at any time, if we need to.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

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

* Re: [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers
  2011-08-17 10:06       ` Guennadi Liakhovetski
@ 2011-08-17 10:46         ` Simon Horman
  0 siblings, 0 replies; 23+ messages in thread
From: Simon Horman @ 2011-08-17 10:46 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: linux-mmc, linux-sh, Chris Ball, Paul Mundt, Magnus Damm

On Wed, Aug 17, 2011 at 12:06:56PM +0200, Guennadi Liakhovetski wrote:
> On Wed, 17 Aug 2011, Simon Horman wrote:
> 
> > On Wed, Aug 17, 2011 at 10:20:24AM +0200, Guennadi Liakhovetski wrote:
> > > On Wed, 17 Aug 2011, Simon Horman wrote:
> > 
> > [snip ]
> > 
> > > > +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
> > > > +	if (irq >= 0) {
> > > > +		multi_irq = true;
> > > > +		ret = request_irq(irq, tmio_mmc_sdcard_irq, 0,
> > > > +				  dev_name(&pdev->dev), host);
> > > > +		if (ret)
> > > > +			goto eirq_sdcard;
> > > > +	} else if (multi_irq)
> > > > +		goto eirq_sdcard;
> > > > +
> > > > +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
> > > > +	if (irq < 0)
> > > > +		goto eirq_card_detect;
> > > > +	f = multi_irq ? tmio_mmc_card_detect_irq : tmio_mmc_irq;
> > > > +	ret = request_irq(irq, f, 0, dev_name(&pdev->dev), host);
> > > > +	if (ret)
> > > > +		goto eirq_card_detect;
> > > > +
> > > 
> > > I still don't see why a multi-IRQ configuration without a card-detect IRQ 
> > > like
> > > 
> > > static struct resource sdhi_resources[] = {
> > > 	[0] = {
> > > 		.name	= "SDHI2",
> > > 		...,
> > > 	},
> > > 	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
> > > 		.start	= ...,
> > > 		.flags	= IORESOURCE_IRQ,
> > > 	},
> > > 	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
> > > 		.start	= ...,
> > > 		.flags	= IORESOURCE_IRQ,
> > > 	},
> > > };
> > > 
> > > should be invalid. Especially since we actually want to avoid using the 
> > > controller card-detect IRQ for power efficiency and use a GPIO instead.
> > 
> > Ok, in this case you would like SH_MOBILE_SDHI_IRQ_SDCARD to use
> > tmio_mmc_sdcard_irq() ?
> 
> This is a good question. I think your erroring out is wrong, but I'm not 
> sure what is best here. Using sdcard and sdio ISR in this case seems most 
> logical to me. But I don't know if we ever can get hardware, where we 
> indeed only have two SDHI interrupts - one for SDIO and one for SDCARD and 
> CARD_DETECT. I'm not aware of such hardware so far, so, yes, I'd go with 
> SDIO and SDCARD ISRs for now. In any case, this is SDHI internal decision, 
> so, we can change it at any time, if we need to.

Agreed.

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

* [PATCH 0/4 v5] mmc: tmio, sdhi: provide multiple irq handlers
@ 2011-08-17 10:59 Simon Horman
  2011-08-17 10:59 ` [PATCH 1/4] mmc: tmio: Cache interrupt masks Simon Horman
                   ` (3 more replies)
  0 siblings, 4 replies; 23+ messages in thread
From: Simon Horman @ 2011-08-17 10:59 UTC (permalink / raw)
  To: linux-mmc, linux-sh
  Cc: Chris Ball, Guennadi Liakhovetski, Magnus Damm, Paul Mundt,
	Simon Horman

The SDHI driver already supports making use of up to three interrupt
sources.

This series breaks up the existing interrupt handler into three handlers,
one for card access, one for card detect interrupts, and one for SDIO
interrupts.  A cover-all handler, which makes use of these new broken-out
handlers is provided for for the case where there is only one interrupt
source.

This series also wires up the broken-out irq handlers in the SDHI driver


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

* [PATCH 1/4] mmc: tmio: Cache interrupt masks
  2011-08-17 10:59 [PATCH 0/4 v5] mmc: tmio, sdhi: provide multiple irq handlers Simon Horman
@ 2011-08-17 10:59 ` Simon Horman
  2011-08-17 10:59 ` [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers Simon Horman
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 23+ messages in thread
From: Simon Horman @ 2011-08-17 10:59 UTC (permalink / raw)
  To: linux-mmc, linux-sh
  Cc: Chris Ball, Guennadi Liakhovetski, Magnus Damm, Paul Mundt,
	Simon Horman

This avoids the need to look up the masks each time an interrupt
is handled.

As suggested by Guennadi.

Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Magnus Damm <magnus.damm@gmail.com>
Signed-off-by: Simon Horman <horms@verge.net.au>

---

* SDCARD portion tested on AP4/Mackerel
* SDIO portion untested

v3
* As suggested by Guennadi Liakhovetski
  - Only read sdcard_irq_mask once and never read sdio_irq_mask,
    instead use the cached values as much as possible

v2
* Initial release
---
 drivers/mmc/host/tmio_mmc.h     |    4 ++++
 drivers/mmc/host/tmio_mmc_pio.c |   34 ++++++++++++++++++----------------
 2 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index eeaf643..1cf8db5 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -79,6 +79,10 @@ struct tmio_mmc_host {
 	struct delayed_work	delayed_reset_work;
 	struct work_struct	done;
 
+	/* Cache IRQ mask */
+	u32			sdcard_irq_mask;
+	u32			sdio_irq_mask;
+
 	spinlock_t		lock;		/* protect host private data */
 	unsigned long		last_req_ts;
 	struct mutex		ios_lock;	/* protect set_ios() context */
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 1f16357..f0c7830 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -48,14 +48,14 @@
 
 void tmio_mmc_enable_mmc_irqs(struct tmio_mmc_host *host, u32 i)
 {
-	u32 mask = sd_ctrl_read32(host, CTL_IRQ_MASK) & ~(i & TMIO_MASK_IRQ);
-	sd_ctrl_write32(host, CTL_IRQ_MASK, mask);
+	host->sdcard_irq_mask &= ~(i & TMIO_MASK_IRQ);
+	sd_ctrl_write32(host, CTL_IRQ_MASK, host->sdcard_irq_mask);
 }
 
 void tmio_mmc_disable_mmc_irqs(struct tmio_mmc_host *host, u32 i)
 {
-	u32 mask = sd_ctrl_read32(host, CTL_IRQ_MASK) | (i & TMIO_MASK_IRQ);
-	sd_ctrl_write32(host, CTL_IRQ_MASK, mask);
+	host->sdcard_irq_mask |= (i & TMIO_MASK_IRQ);
+	sd_ctrl_write32(host, CTL_IRQ_MASK, host->sdcard_irq_mask);
 }
 
 static void tmio_mmc_ack_mmc_irqs(struct tmio_mmc_host *host, u32 i)
@@ -127,11 +127,13 @@ static void tmio_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable)
 
 	if (enable) {
 		host->sdio_irq_enabled = 1;
+		host->sdio_irq_mask = TMIO_SDIO_MASK_ALL &
+					~TMIO_SDIO_STAT_IOIRQ;
 		sd_ctrl_write16(host, CTL_TRANSACTION_CTL, 0x0001);
-		sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK,
-			(TMIO_SDIO_MASK_ALL & ~TMIO_SDIO_STAT_IOIRQ));
+		sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK, host->sdio_irq_mask);
 	} else {
-		sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK, TMIO_SDIO_MASK_ALL);
+		host->sdio_irq_mask = TMIO_SDIO_MASK_ALL;
+		sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK, host->sdio_irq_mask);
 		sd_ctrl_write16(host, CTL_TRANSACTION_CTL, 0x0000);
 		host->sdio_irq_enabled = 0;
 	}
@@ -548,26 +550,25 @@ irqreturn_t tmio_mmc_irq(int irq, void *devid)
 	struct tmio_mmc_host *host = devid;
 	struct mmc_host *mmc = host->mmc;
 	struct tmio_mmc_data *pdata = host->pdata;
-	unsigned int ireg, irq_mask, status;
-	unsigned int sdio_ireg, sdio_irq_mask, sdio_status;
+	unsigned int ireg, status;
+	unsigned int sdio_ireg, sdio_status;
 
 	pr_debug("MMC IRQ begin\n");
 
 	status = sd_ctrl_read32(host, CTL_STATUS);
-	irq_mask = sd_ctrl_read32(host, CTL_IRQ_MASK);
-	ireg = status & TMIO_MASK_IRQ & ~irq_mask;
+	ireg = status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
 
 	sdio_ireg = 0;
 	if (!ireg && pdata->flags & TMIO_MMC_SDIO_IRQ) {
 		sdio_status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
-		sdio_irq_mask = sd_ctrl_read16(host, CTL_SDIO_IRQ_MASK);
-		sdio_ireg = sdio_status & TMIO_SDIO_MASK_ALL & ~sdio_irq_mask;
+		sdio_ireg = sdio_status & TMIO_SDIO_MASK_ALL &
+				~host->sdio_irq_mask;
 
 		sd_ctrl_write16(host, CTL_SDIO_STATUS, sdio_status & ~TMIO_SDIO_MASK_ALL);
 
 		if (sdio_ireg && !host->sdio_irq_enabled) {
 			pr_warning("tmio_mmc: Spurious SDIO IRQ, disabling! 0x%04x 0x%04x 0x%04x\n",
-				   sdio_status, sdio_irq_mask, sdio_ireg);
+				   sdio_status, host->sdio_irq_mask, sdio_ireg);
 			tmio_mmc_enable_sdio_irq(mmc, 0);
 			goto out;
 		}
@@ -623,9 +624,9 @@ irqreturn_t tmio_mmc_irq(int irq, void *devid)
 	}
 
 	pr_warning("tmio_mmc: Spurious irq, disabling! "
-		"0x%08x 0x%08x 0x%08x\n", status, irq_mask, ireg);
+		"0x%08x 0x%08x 0x%08x\n", status, host->sdcard_irq_mask, ireg);
 	pr_debug_status(status);
-	tmio_mmc_disable_mmc_irqs(host, status & ~irq_mask);
+	tmio_mmc_disable_mmc_irqs(host, status & ~host->sdcard_irq_mask);
 
 out:
 	return IRQ_HANDLED;
@@ -882,6 +883,7 @@ int __devinit tmio_mmc_host_probe(struct tmio_mmc_host **host,
 	tmio_mmc_clk_stop(_host);
 	tmio_mmc_reset(_host);
 
+	_host->sdcard_irq_mask = sd_ctrl_read32(_host, CTL_IRQ_MASK);
 	tmio_mmc_disable_mmc_irqs(_host, TMIO_MASK_ALL);
 	if (pdata->flags & TMIO_MMC_SDIO_IRQ)
 		tmio_mmc_enable_sdio_irq(mmc, 0);
-- 
1.7.5.4


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

* [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers
  2011-08-17 10:59 [PATCH 0/4 v5] mmc: tmio, sdhi: provide multiple irq handlers Simon Horman
  2011-08-17 10:59 ` [PATCH 1/4] mmc: tmio: Cache interrupt masks Simon Horman
@ 2011-08-17 10:59 ` Simon Horman
  2011-08-17 10:59 ` [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers Simon Horman
  2011-08-17 10:59 ` [PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index enum Simon Horman
  3 siblings, 0 replies; 23+ messages in thread
From: Simon Horman @ 2011-08-17 10:59 UTC (permalink / raw)
  To: linux-mmc, linux-sh
  Cc: Chris Ball, Guennadi Liakhovetski, Magnus Damm, Paul Mundt,
	Simon Horman

Provide separate interrupt handlers which may be used by platforms where
SDHI has three interrupt sources.

This patch also removes the commented-out handling of CRC and other errors.

Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Magnus Damm <magnus.damm@gmail.com>
Signed-off-by: Simon Horman <horms@verge.net.au>

---

* SDCARD portion tested on AP4/Mackerel
* SDIO portion untested

v4
* As suggested by Guennadi Liakhovetski
  - Use bool as return type for __tmio_mmc_sdcard_irq() and
    __tmio_mmc_card_detect_irq()

v3
* Rebase for updated "mmc: tmio: Cache interrupt masks"
* As suggested by Guennadi Liakhovetski
  - Do not alter logic to handle more than one interupt at once
  - Add missing "static" to declartion of __tmio_mmc_sdcard_irq()

v2
* As suggested by Guennadi Liakhovetski
  - Combine 3 patches into one
  - Reduce the number of __tmio_..._irq() functions
  - Rename "...card_access..." functions as "...sdcard..."
---
 drivers/mmc/host/tmio_mmc.h     |    3 +
 drivers/mmc/host/tmio_mmc_pio.c |  131 ++++++++++++++++++++++++--------------
 2 files changed, 86 insertions(+), 48 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 1cf8db5..3020f98 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -97,6 +97,9 @@ void tmio_mmc_do_data_irq(struct tmio_mmc_host *host);
 void tmio_mmc_enable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
 void tmio_mmc_disable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
 irqreturn_t tmio_mmc_irq(int irq, void *devid);
+irqreturn_t tmio_mmc_sdcard_irq(int irq, void *devid);
+irqreturn_t tmio_mmc_card_detect_irq(int irq, void *devid);
+irqreturn_t tmio_mmc_sdio_irq(int irq, void *devid);
 
 static inline char *tmio_mmc_kmap_atomic(struct scatterlist *sg,
 					 unsigned long *flags)
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index f0c7830..6275e3d 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -545,44 +545,20 @@ out:
 	spin_unlock(&host->lock);
 }
 
-irqreturn_t tmio_mmc_irq(int irq, void *devid)
+static void tmio_mmc_card_irq_status(struct tmio_mmc_host *host,
+				       int *ireg, int *status)
 {
-	struct tmio_mmc_host *host = devid;
-	struct mmc_host *mmc = host->mmc;
-	struct tmio_mmc_data *pdata = host->pdata;
-	unsigned int ireg, status;
-	unsigned int sdio_ireg, sdio_status;
-
-	pr_debug("MMC IRQ begin\n");
-
-	status = sd_ctrl_read32(host, CTL_STATUS);
-	ireg = status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
+	*status = sd_ctrl_read32(host, CTL_STATUS);
+	*ireg = *status & TMIO_MASK_IRQ & ~host->sdcard_irq_mask;
 
-	sdio_ireg = 0;
-	if (!ireg && pdata->flags & TMIO_MMC_SDIO_IRQ) {
-		sdio_status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
-		sdio_ireg = sdio_status & TMIO_SDIO_MASK_ALL &
-				~host->sdio_irq_mask;
-
-		sd_ctrl_write16(host, CTL_SDIO_STATUS, sdio_status & ~TMIO_SDIO_MASK_ALL);
-
-		if (sdio_ireg && !host->sdio_irq_enabled) {
-			pr_warning("tmio_mmc: Spurious SDIO IRQ, disabling! 0x%04x 0x%04x 0x%04x\n",
-				   sdio_status, host->sdio_irq_mask, sdio_ireg);
-			tmio_mmc_enable_sdio_irq(mmc, 0);
-			goto out;
-		}
-
-		if (mmc->caps & MMC_CAP_SDIO_IRQ &&
-			sdio_ireg & TMIO_SDIO_STAT_IOIRQ)
-			mmc_signal_sdio_irq(mmc);
-
-		if (sdio_ireg)
-			goto out;
-	}
+	pr_debug_status(*status);
+	pr_debug_status(*ireg);
+}
 
-	pr_debug_status(status);
-	pr_debug_status(ireg);
+static bool __tmio_mmc_card_detect_irq(struct tmio_mmc_host *host,
+				      int ireg, int status)
+{
+	struct mmc_host *mmc = host->mmc;
 
 	/* Card insert / remove attempts */
 	if (ireg & (TMIO_STAT_CARD_INSERT | TMIO_STAT_CARD_REMOVE)) {
@@ -592,43 +568,102 @@ irqreturn_t tmio_mmc_irq(int irq, void *devid)
 		     ((ireg & TMIO_STAT_CARD_INSERT) && !mmc->card)) &&
 		    !work_pending(&mmc->detect.work))
 			mmc_detect_change(host->mmc, msecs_to_jiffies(100));
-		goto out;
+		return true;
 	}
 
-	/* CRC and other errors */
-/*	if (ireg & TMIO_STAT_ERR_IRQ)
- *		handled |= tmio_error_irq(host, irq, stat);
- */
+	return false;
+}
+
+irqreturn_t tmio_mmc_card_detect_irq(int irq, void *devid)
+{
+	unsigned int ireg, status;
+	struct tmio_mmc_host *host = devid;
 
+	tmio_mmc_card_irq_status(host, &ireg, &status);
+	__tmio_mmc_card_detect_irq(host, ireg, status);
+
+	return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(tmio_mmc_card_detect_irq);
+
+static bool __tmio_mmc_sdcard_irq(struct tmio_mmc_host *host,
+				 int ireg, int status)
+{
 	/* Command completion */
 	if (ireg & (TMIO_STAT_CMDRESPEND | TMIO_STAT_CMDTIMEOUT)) {
 		tmio_mmc_ack_mmc_irqs(host,
 			     TMIO_STAT_CMDRESPEND |
 			     TMIO_STAT_CMDTIMEOUT);
 		tmio_mmc_cmd_irq(host, status);
-		goto out;
+		return true;
 	}
 
 	/* Data transfer */
 	if (ireg & (TMIO_STAT_RXRDY | TMIO_STAT_TXRQ)) {
 		tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_RXRDY | TMIO_STAT_TXRQ);
 		tmio_mmc_pio_irq(host);
-		goto out;
+		return true;
 	}
 
 	/* Data transfer completion */
 	if (ireg & TMIO_STAT_DATAEND) {
 		tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_DATAEND);
 		tmio_mmc_data_irq(host);
-		goto out;
+		return true;
 	}
 
-	pr_warning("tmio_mmc: Spurious irq, disabling! "
-		"0x%08x 0x%08x 0x%08x\n", status, host->sdcard_irq_mask, ireg);
-	pr_debug_status(status);
-	tmio_mmc_disable_mmc_irqs(host, status & ~host->sdcard_irq_mask);
+	return false;
+}
+
+irqreturn_t tmio_mmc_sdcard_irq(int irq, void *devid)
+{
+	unsigned int ireg, status;
+	struct tmio_mmc_host *host = devid;
+
+	tmio_mmc_card_irq_status(host, &ireg, &status);
+	__tmio_mmc_sdcard_irq(host, ireg, status);
+
+	return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(tmio_mmc_sdcard_irq);
+
+irqreturn_t tmio_mmc_sdio_irq(int irq, void *devid)
+{
+	struct tmio_mmc_host *host = devid;
+	struct mmc_host *mmc = host->mmc;
+	struct tmio_mmc_data *pdata = host->pdata;
+	unsigned int ireg, status;
+
+	if (!(pdata->flags & TMIO_MMC_SDIO_IRQ))
+		return IRQ_HANDLED;
+
+	status = sd_ctrl_read16(host, CTL_SDIO_STATUS);
+	ireg = status & TMIO_SDIO_MASK_ALL & ~host->sdcard_irq_mask;
+
+	sd_ctrl_write16(host, CTL_SDIO_STATUS, status & ~TMIO_SDIO_MASK_ALL);
+
+	if (mmc->caps & MMC_CAP_SDIO_IRQ && ireg & TMIO_SDIO_STAT_IOIRQ)
+		mmc_signal_sdio_irq(mmc);
+
+	return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(tmio_mmc_sdio_irq);
+
+irqreturn_t tmio_mmc_irq(int irq, void *devid)
+{
+	struct tmio_mmc_host *host = devid;
+	unsigned int ireg, status;
+
+	pr_debug("MMC IRQ begin\n");
+
+	tmio_mmc_card_irq_status(host, &ireg, &status);
+	if (__tmio_mmc_card_detect_irq(host, ireg, status))
+		return IRQ_HANDLED;
+	if (__tmio_mmc_sdcard_irq(host, ireg, status))
+		return IRQ_HANDLED;
+
+	tmio_mmc_sdio_irq(irq, devid);
 
-out:
 	return IRQ_HANDLED;
 }
 EXPORT_SYMBOL(tmio_mmc_irq);
-- 
1.7.5.4


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

* [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers
  2011-08-17 10:59 [PATCH 0/4 v5] mmc: tmio, sdhi: provide multiple irq handlers Simon Horman
  2011-08-17 10:59 ` [PATCH 1/4] mmc: tmio: Cache interrupt masks Simon Horman
  2011-08-17 10:59 ` [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers Simon Horman
@ 2011-08-17 10:59 ` Simon Horman
  2011-08-17 11:24   ` Guennadi Liakhovetski
  2011-08-17 10:59 ` [PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index enum Simon Horman
  3 siblings, 1 reply; 23+ messages in thread
From: Simon Horman @ 2011-08-17 10:59 UTC (permalink / raw)
  To: linux-mmc, linux-sh
  Cc: Chris Ball, Guennadi Liakhovetski, Magnus Damm, Paul Mundt,
	Simon Horman

Make use of per-source irq handles if the
platform (data) has multiple irq sources.

Also, as suggested by Guennadi Liakhovetski,
add and use defines the index or irqs in platform data.

Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Magnus Damm <magnus.damm@gmail.com>
Signed-off-by: Simon Horman <horms@verge.net.au>

---

v5
* As suggested by Guennadi Liakhovetski:
  - Allow only SH_MOBILE_SDHI_IRQ_SDCARD and
    SH_MOBILE_SDHI_IRQ_SDIO to be specified in platform data

v4
* As suggested by Guennadi Liakhovetski:
  - Correct inverted values of SH_MOBILE_SDHI_IRQ_SDCARD and
    SH_MOBILE_SDHI_IRQ_CARD_DETECT

v3
* Update for changes to "mmc: tmio: Provide separate interrupt handlers"
* As suggested by Guennadi Liakhovetski:
  - Merge in patch "mmc: sdhi: Add defines for platform irq indexes"
  - Use an enum instead of defines for irq indexes

v2
* Update for changes to "mmc: tmio: Provide separate interrupt handlers"
* Make use of defines provided by
  "mmc: sdhi: Add defines for platform irq indexes"
* As suggested by Guennadi Liakhovetski:
  - Don't use a loop to initialise irq handlers, the unrolled version
    is easier on the eyes (and exactly the same number of lines of code!)
---
 drivers/mmc/host/sh_mobile_sdhi.c  |   59 ++++++++++++++++++++++-------------
 include/linux/mmc/sh_mobile_sdhi.h |    7 ++++
 2 files changed, 44 insertions(+), 22 deletions(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 774f643..73ecadb 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -96,7 +96,9 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
 	struct sh_mobile_sdhi_info *p = pdev->dev.platform_data;
 	struct tmio_mmc_host *host;
 	char clk_name[8];
-	int i, irq, ret;
+	int irq, ret;
+	irqreturn_t (*f)(int irq, void *devid);
+	bool multi_irq = false;
 
 	priv = kzalloc(sizeof(struct sh_mobile_sdhi), GFP_KERNEL);
 	if (priv == NULL) {
@@ -153,27 +155,32 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto eprobe;
 
-	for (i = 0; i < 3; i++) {
-		irq = platform_get_irq(pdev, i);
-		if (irq < 0) {
-			if (i) {
-				continue;
-			} else {
-				ret = irq;
-				goto eirq;
-			}
-		}
-		ret = request_irq(irq, tmio_mmc_irq, 0,
+	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
+	if (irq >= 0) {
+		multi_irq = true;
+		ret = request_irq(irq, tmio_mmc_sdio_irq, 0,
 				  dev_name(&pdev->dev), host);
-		if (ret) {
-			while (i--) {
-				irq = platform_get_irq(pdev, i);
-				if (irq >= 0)
-					free_irq(irq, host);
-			}
-			goto eirq;
-		}
+		if (ret)
+			goto eirq_sdio;
+	}
+
+	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
+	if (irq >= 0) {
+		multi_irq = true;
+		ret = request_irq(irq, tmio_mmc_sdcard_irq, 0,
+				  dev_name(&pdev->dev), host);
+		if (ret)
+			goto eirq_sdcard;
 	}
+
+	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
+	if (irq < 0)
+		goto eirq_card_detect;
+	f = multi_irq ? tmio_mmc_card_detect_irq : tmio_mmc_irq;
+	ret = request_irq(irq, f, 0, dev_name(&pdev->dev), host);
+	if (ret)
+		goto eirq_card_detect;
+
 	dev_info(&pdev->dev, "%s base at 0x%08lx clock rate %u MHz\n",
 		 mmc_hostname(host->mmc), (unsigned long)
 		 (platform_get_resource(pdev,IORESOURCE_MEM, 0)->start),
@@ -181,7 +188,15 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
 
 	return ret;
 
-eirq:
+eirq_card_detect:
+	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
+	if (irq >= 0)
+		free_irq(irq, host);
+eirq_sdcard:
+	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
+	if (irq >= 0)
+		free_irq(irq, host);
+eirq_sdio:
 	tmio_mmc_host_remove(host);
 eprobe:
 	clk_disable(priv->clk);
@@ -203,7 +218,7 @@ static int sh_mobile_sdhi_remove(struct platform_device *pdev)
 
 	tmio_mmc_host_remove(host);
 
-	for (i = 0; i < 3; i++) {
+	for (i = 0; i < SH_MOBILE_SDHI_IRQ_MAX; i++) {
 		irq = platform_get_irq(pdev, i);
 		if (irq >= 0)
 			free_irq(irq, host);
diff --git a/include/linux/mmc/sh_mobile_sdhi.h b/include/linux/mmc/sh_mobile_sdhi.h
index bd50b36..80d3629 100644
--- a/include/linux/mmc/sh_mobile_sdhi.h
+++ b/include/linux/mmc/sh_mobile_sdhi.h
@@ -3,6 +3,13 @@
 
 #include <linux/types.h>
 
+enum {
+	SH_MOBILE_SDHI_IRQ_CARD_DETECT = 0,
+	SH_MOBILE_SDHI_IRQ_SDCARD,
+	SH_MOBILE_SDHI_IRQ_SDIO,
+	SH_MOBILE_SDHI_IRQ_MAX
+};
+
 struct platform_device;
 struct tmio_mmc_data;
 
-- 
1.7.5.4


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

* [PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index enum
  2011-08-17 10:59 [PATCH 0/4 v5] mmc: tmio, sdhi: provide multiple irq handlers Simon Horman
                   ` (2 preceding siblings ...)
  2011-08-17 10:59 ` [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers Simon Horman
@ 2011-08-17 10:59 ` Simon Horman
  3 siblings, 0 replies; 23+ messages in thread
From: Simon Horman @ 2011-08-17 10:59 UTC (permalink / raw)
  To: linux-mmc, linux-sh
  Cc: Chris Ball, Guennadi Liakhovetski, Magnus Damm, Paul Mundt,
	Simon Horman

This is intended to make it easier to correctly order IRQs.

As suggested by Guennadi Liakhovetski.

Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Magnus Damm <magnus.damm@gmail.com>
Cc: Paul Mundt <lethal@linux-sh.org>
Signed-off-by: Simon Horman <horms@verge.net.au>

---

Depends on "mmc: sdhi: Make use of per-source irq handlers"

v4
* Update for corrected ordering of SH_MOBILE_SDHI_IRQ_SDCARD and
  SH_MOBILE_SDHI_IRQ_CARD_DETECT

v2
* Initial release
---
 arch/arm/mach-shmobile/board-ag5evm.c   |   12 ++++++------
 arch/arm/mach-shmobile/board-mackerel.c |   18 +++++++++---------
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/arch/arm/mach-shmobile/board-ag5evm.c b/arch/arm/mach-shmobile/board-ag5evm.c
index ce5c251..0d543bb 100644
--- a/arch/arm/mach-shmobile/board-ag5evm.c
+++ b/arch/arm/mach-shmobile/board-ag5evm.c
@@ -352,15 +352,15 @@ static struct resource sdhi0_resources[] = {
 		.end	= 0xee1000ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= gic_spi(83),
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= gic_spi(84),
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
 		.start	= gic_spi(85),
 		.flags	= IORESOURCE_IRQ,
 	},
@@ -395,15 +395,15 @@ static struct resource sdhi1_resources[] = {
 		.end	= 0xee1200ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= gic_spi(87),
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= gic_spi(88),
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
 		.start	= gic_spi(89),
 		.flags	= IORESOURCE_IRQ,
 	},
diff --git a/arch/arm/mach-shmobile/board-mackerel.c b/arch/arm/mach-shmobile/board-mackerel.c
index d41c01f..f9d3a93 100644
--- a/arch/arm/mach-shmobile/board-mackerel.c
+++ b/arch/arm/mach-shmobile/board-mackerel.c
@@ -1022,15 +1022,15 @@ static struct resource sdhi0_resources[] = {
 		.end	= 0xe68500ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= evt2irq(0x0e00) /* SDHI0_SDHI0I0 */,
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= evt2irq(0x0e20) /* SDHI0_SDHI0I1 */,
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
 		.start	= evt2irq(0x0e40) /* SDHI0_SDHI0I2 */,
 		.flags	= IORESOURCE_IRQ,
 	},
@@ -1065,15 +1065,15 @@ static struct resource sdhi1_resources[] = {
 		.end	= 0xe68600ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= evt2irq(0x0e80), /* SDHI1_SDHI1I0 */
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= evt2irq(0x0ea0), /* SDHI1_SDHI1I1 */
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
 		.start	= evt2irq(0x0ec0), /* SDHI1_SDHI1I2 */
 		.flags	= IORESOURCE_IRQ,
 	},
@@ -1116,15 +1116,15 @@ static struct resource sdhi2_resources[] = {
 		.end	= 0xe68700ff,
 		.flags	= IORESOURCE_MEM,
 	},
-	[1] = {
+	[1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
 		.start	= evt2irq(0x1200), /* SDHI2_SDHI2I0 */
 		.flags	= IORESOURCE_IRQ,
 	},
-	[2] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
 		.start	= evt2irq(0x1220), /* SDHI2_SDHI2I1 */
 		.flags	= IORESOURCE_IRQ,
 	},
-	[3] = {
+	[1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
 		.start	= evt2irq(0x1240), /* SDHI2_SDHI2I2 */
 		.flags	= IORESOURCE_IRQ,
 	},
-- 
1.7.5.4


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

* Re: [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers
  2011-08-17 10:59 ` [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers Simon Horman
@ 2011-08-17 11:24   ` Guennadi Liakhovetski
  2011-08-17 11:41     ` Simon Horman
  0 siblings, 1 reply; 23+ messages in thread
From: Guennadi Liakhovetski @ 2011-08-17 11:24 UTC (permalink / raw)
  To: Simon Horman; +Cc: linux-mmc, linux-sh, Chris Ball, Magnus Damm, Paul Mundt

On Wed, 17 Aug 2011, Simon Horman wrote:

> Make use of per-source irq handles if the
> platform (data) has multiple irq sources.
> 
> Also, as suggested by Guennadi Liakhovetski,
> add and use defines the index or irqs in platform data.
> 
> Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Cc: Magnus Damm <magnus.damm@gmail.com>
> Signed-off-by: Simon Horman <horms@verge.net.au>
> 
> ---
> 
> v5
> * As suggested by Guennadi Liakhovetski:
>   - Allow only SH_MOBILE_SDHI_IRQ_SDCARD and
>     SH_MOBILE_SDHI_IRQ_SDIO to be specified in platform data

This means, CARD_DETECT is optional in a multi-irq configuration, agree.

> 
> v4
> * As suggested by Guennadi Liakhovetski:
>   - Correct inverted values of SH_MOBILE_SDHI_IRQ_SDCARD and
>     SH_MOBILE_SDHI_IRQ_CARD_DETECT
> 
> v3
> * Update for changes to "mmc: tmio: Provide separate interrupt handlers"
> * As suggested by Guennadi Liakhovetski:
>   - Merge in patch "mmc: sdhi: Add defines for platform irq indexes"
>   - Use an enum instead of defines for irq indexes
> 
> v2
> * Update for changes to "mmc: tmio: Provide separate interrupt handlers"
> * Make use of defines provided by
>   "mmc: sdhi: Add defines for platform irq indexes"
> * As suggested by Guennadi Liakhovetski:
>   - Don't use a loop to initialise irq handlers, the unrolled version
>     is easier on the eyes (and exactly the same number of lines of code!)
> ---
>  drivers/mmc/host/sh_mobile_sdhi.c  |   59 ++++++++++++++++++++++-------------
>  include/linux/mmc/sh_mobile_sdhi.h |    7 ++++
>  2 files changed, 44 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
> index 774f643..73ecadb 100644
> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> @@ -96,7 +96,9 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
>  	struct sh_mobile_sdhi_info *p = pdev->dev.platform_data;
>  	struct tmio_mmc_host *host;
>  	char clk_name[8];
> -	int i, irq, ret;
> +	int irq, ret;
> +	irqreturn_t (*f)(int irq, void *devid);
> +	bool multi_irq = false;
>  
>  	priv = kzalloc(sizeof(struct sh_mobile_sdhi), GFP_KERNEL);
>  	if (priv == NULL) {
> @@ -153,27 +155,32 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
>  	if (ret < 0)
>  		goto eprobe;
>  
> -	for (i = 0; i < 3; i++) {
> -		irq = platform_get_irq(pdev, i);
> -		if (irq < 0) {
> -			if (i) {
> -				continue;
> -			} else {
> -				ret = irq;
> -				goto eirq;
> -			}
> -		}
> -		ret = request_irq(irq, tmio_mmc_irq, 0,
> +	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
> +	if (irq >= 0) {
> +		multi_irq = true;
> +		ret = request_irq(irq, tmio_mmc_sdio_irq, 0,
>  				  dev_name(&pdev->dev), host);
> -		if (ret) {
> -			while (i--) {
> -				irq = platform_get_irq(pdev, i);
> -				if (irq >= 0)
> -					free_irq(irq, host);
> -			}
> -			goto eirq;
> -		}
> +		if (ret)
> +			goto eirq_sdio;
> +	}
> +
> +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
> +	if (irq >= 0) {
> +		multi_irq = true;
> +		ret = request_irq(irq, tmio_mmc_sdcard_irq, 0,
> +				  dev_name(&pdev->dev), host);
> +		if (ret)
> +			goto eirq_sdcard;
>  	}

This means SDCARD is optional in both single- and multi-irq 
configurations.

> +
> +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
> +	if (irq < 0)
> +		goto eirq_card_detect;

This means, CARD_DETECT is compulsory in all configurations.

One of us must be speaking klingonish today.

> +	f = multi_irq ? tmio_mmc_card_detect_irq : tmio_mmc_irq;
> +	ret = request_irq(irq, f, 0, dev_name(&pdev->dev), host);
> +	if (ret)
> +		goto eirq_card_detect;
> +
>  	dev_info(&pdev->dev, "%s base at 0x%08lx clock rate %u MHz\n",
>  		 mmc_hostname(host->mmc), (unsigned long)
>  		 (platform_get_resource(pdev,IORESOURCE_MEM, 0)->start),
> @@ -181,7 +188,15 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
>  
>  	return ret;
>  
> -eirq:
> +eirq_card_detect:
> +	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
> +	if (irq >= 0)
> +		free_irq(irq, host);
> +eirq_sdcard:
> +	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
> +	if (irq >= 0)
> +		free_irq(irq, host);
> +eirq_sdio:
>  	tmio_mmc_host_remove(host);
>  eprobe:
>  	clk_disable(priv->clk);
> @@ -203,7 +218,7 @@ static int sh_mobile_sdhi_remove(struct platform_device *pdev)
>  
>  	tmio_mmc_host_remove(host);
>  
> -	for (i = 0; i < 3; i++) {
> +	for (i = 0; i < SH_MOBILE_SDHI_IRQ_MAX; i++) {
>  		irq = platform_get_irq(pdev, i);
>  		if (irq >= 0)
>  			free_irq(irq, host);
> diff --git a/include/linux/mmc/sh_mobile_sdhi.h b/include/linux/mmc/sh_mobile_sdhi.h
> index bd50b36..80d3629 100644
> --- a/include/linux/mmc/sh_mobile_sdhi.h
> +++ b/include/linux/mmc/sh_mobile_sdhi.h
> @@ -3,6 +3,13 @@
>  
>  #include <linux/types.h>
>  
> +enum {
> +	SH_MOBILE_SDHI_IRQ_CARD_DETECT = 0,
> +	SH_MOBILE_SDHI_IRQ_SDCARD,
> +	SH_MOBILE_SDHI_IRQ_SDIO,
> +	SH_MOBILE_SDHI_IRQ_MAX
> +};
> +
>  struct platform_device;
>  struct tmio_mmc_data;
>  
> -- 
> 1.7.5.4
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

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

* Re: [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers
  2011-08-17 11:24   ` Guennadi Liakhovetski
@ 2011-08-17 11:41     ` Simon Horman
  2011-08-17 11:54       ` Guennadi Liakhovetski
  2011-08-17 12:07       ` Simon Horman
  0 siblings, 2 replies; 23+ messages in thread
From: Simon Horman @ 2011-08-17 11:41 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: linux-mmc, linux-sh, Chris Ball, Magnus Damm, Paul Mundt

On Wed, Aug 17, 2011 at 01:24:58PM +0200, Guennadi Liakhovetski wrote:
> On Wed, 17 Aug 2011, Simon Horman wrote:
> 
> > Make use of per-source irq handles if the
> > platform (data) has multiple irq sources.
> > 
> > Also, as suggested by Guennadi Liakhovetski,
> > add and use defines the index or irqs in platform data.
> > 
> > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > Cc: Magnus Damm <magnus.damm@gmail.com>
> > Signed-off-by: Simon Horman <horms@verge.net.au>
> > 
> > ---
> > 
> > v5
> > * As suggested by Guennadi Liakhovetski:
> >   - Allow only SH_MOBILE_SDHI_IRQ_SDCARD and
> >     SH_MOBILE_SDHI_IRQ_SDIO to be specified in platform data
> 
> This means, CARD_DETECT is optional in a multi-irq configuration, agree.

[snip]

> > +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
> > +	if (irq >= 0) {
> > +		multi_irq = true;
> > +		ret = request_irq(irq, tmio_mmc_sdcard_irq, 0,
> > +				  dev_name(&pdev->dev), host);
> > +		if (ret)
> > +			goto eirq_sdcard;
> >  	}
> 
> This means SDCARD is optional in both single- and multi-irq 
> configurations.
> 
> > +
> > +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
> > +	if (irq < 0)
> > +		goto eirq_card_detect;
> 
> This means, CARD_DETECT is compulsory in all configurations.
> 
> One of us must be speaking klingonish today.

Sorry, I messed that up a bit.

I will make CARD_DETECT optional in multi-irq configurations.
Likewise SDIO will be optional in multi-irq configurations.

But SDCARD will always be required.

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

* Re: [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers
  2011-08-17 11:41     ` Simon Horman
@ 2011-08-17 11:54       ` Guennadi Liakhovetski
  2011-08-17 12:07       ` Simon Horman
  1 sibling, 0 replies; 23+ messages in thread
From: Guennadi Liakhovetski @ 2011-08-17 11:54 UTC (permalink / raw)
  To: Simon Horman; +Cc: linux-mmc, linux-sh, Chris Ball, Magnus Damm, Paul Mundt

On Wed, 17 Aug 2011, Simon Horman wrote:

> On Wed, Aug 17, 2011 at 01:24:58PM +0200, Guennadi Liakhovetski wrote:
> > On Wed, 17 Aug 2011, Simon Horman wrote:
> > 
> > > Make use of per-source irq handles if the
> > > platform (data) has multiple irq sources.
> > > 
> > > Also, as suggested by Guennadi Liakhovetski,
> > > add and use defines the index or irqs in platform data.
> > > 
> > > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > > Cc: Magnus Damm <magnus.damm@gmail.com>
> > > Signed-off-by: Simon Horman <horms@verge.net.au>
> > > 
> > > ---
> > > 
> > > v5
> > > * As suggested by Guennadi Liakhovetski:
> > >   - Allow only SH_MOBILE_SDHI_IRQ_SDCARD and
> > >     SH_MOBILE_SDHI_IRQ_SDIO to be specified in platform data
> > 
> > This means, CARD_DETECT is optional in a multi-irq configuration, agree.
> 
> [snip]
> 
> > > +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
> > > +	if (irq >= 0) {
> > > +		multi_irq = true;
> > > +		ret = request_irq(irq, tmio_mmc_sdcard_irq, 0,
> > > +				  dev_name(&pdev->dev), host);
> > > +		if (ret)
> > > +			goto eirq_sdcard;
> > >  	}
> > 
> > This means SDCARD is optional in both single- and multi-irq 
> > configurations.
> > 
> > > +
> > > +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
> > > +	if (irq < 0)
> > > +		goto eirq_card_detect;
> > 
> > This means, CARD_DETECT is compulsory in all configurations.
> > 
> > One of us must be speaking klingonish today.
> 
> Sorry, I messed that up a bit.
> 
> I will make CARD_DETECT optional in multi-irq configurations.
> Likewise SDIO will be optional in multi-irq configurations.
> 
> But SDCARD will always be required.

No, wrong again. This would break existing single-IRQ configurations, 
which only use IRQ resource #0, which is CARD_DETECT.

Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

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

* Re: [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers
  2011-08-17 11:41     ` Simon Horman
  2011-08-17 11:54       ` Guennadi Liakhovetski
@ 2011-08-17 12:07       ` Simon Horman
  2011-08-17 12:22         ` Guennadi Liakhovetski
  1 sibling, 1 reply; 23+ messages in thread
From: Simon Horman @ 2011-08-17 12:07 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: linux-mmc, linux-sh, Chris Ball, Magnus Damm, Paul Mundt

On Wed, Aug 17, 2011 at 08:41:47PM +0900, Simon Horman wrote:
> On Wed, Aug 17, 2011 at 01:24:58PM +0200, Guennadi Liakhovetski wrote:
> > On Wed, 17 Aug 2011, Simon Horman wrote:
> > 
> > > Make use of per-source irq handles if the
> > > platform (data) has multiple irq sources.
> > > 
> > > Also, as suggested by Guennadi Liakhovetski,
> > > add and use defines the index or irqs in platform data.
> > > 
> > > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > > Cc: Magnus Damm <magnus.damm@gmail.com>
> > > Signed-off-by: Simon Horman <horms@verge.net.au>
> > > 
> > > ---
> > > 
> > > v5
> > > * As suggested by Guennadi Liakhovetski:
> > >   - Allow only SH_MOBILE_SDHI_IRQ_SDCARD and
> > >     SH_MOBILE_SDHI_IRQ_SDIO to be specified in platform data
> > 
> > This means, CARD_DETECT is optional in a multi-irq configuration, agree.
> 
> [snip]
> 
> > > +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
> > > +	if (irq >= 0) {
> > > +		multi_irq = true;
> > > +		ret = request_irq(irq, tmio_mmc_sdcard_irq, 0,
> > > +				  dev_name(&pdev->dev), host);
> > > +		if (ret)
> > > +			goto eirq_sdcard;
> > >  	}
> > 
> > This means SDCARD is optional in both single- and multi-irq 
> > configurations.
> > 
> > > +
> > > +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
> > > +	if (irq < 0)
> > > +		goto eirq_card_detect;
> > 
> > This means, CARD_DETECT is compulsory in all configurations.
> > 
> > One of us must be speaking klingonish today.
> 
> Sorry, I messed that up a bit.
> 
> I will make CARD_DETECT optional in multi-irq configurations.
> Likewise SDIO will be optional in multi-irq configurations.
> 
> But SDCARD will always be required.

Oops, that scheme won't work because in single-irq configuration
the only IRQ will be #0, but SDCARD != 0.

So I have it in mind to allow the following combinations.

i) SDIO, SDCARD, CARD_DETECT
ii) SDIO, SDCARD
iii) SDCARD, CARD_DETECT
iv) SDCARD only (sets multi_irq = true locally, but only one irq!)
v) CARD_DETECT only (uses tmio_mmc_irq(), traditional single-irq setup)

Do you think iii and iv are necessary?

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

* Re: [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers
  2011-08-17 12:07       ` Simon Horman
@ 2011-08-17 12:22         ` Guennadi Liakhovetski
  2011-08-17 12:57           ` Simon Horman
  0 siblings, 1 reply; 23+ messages in thread
From: Guennadi Liakhovetski @ 2011-08-17 12:22 UTC (permalink / raw)
  To: Simon Horman; +Cc: linux-mmc, linux-sh, Chris Ball, Magnus Damm, Paul Mundt

On Wed, 17 Aug 2011, Simon Horman wrote:

> On Wed, Aug 17, 2011 at 08:41:47PM +0900, Simon Horman wrote:
> > On Wed, Aug 17, 2011 at 01:24:58PM +0200, Guennadi Liakhovetski wrote:
> > > On Wed, 17 Aug 2011, Simon Horman wrote:
> > > 
> > > > Make use of per-source irq handles if the
> > > > platform (data) has multiple irq sources.
> > > > 
> > > > Also, as suggested by Guennadi Liakhovetski,
> > > > add and use defines the index or irqs in platform data.
> > > > 
> > > > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > > > Cc: Magnus Damm <magnus.damm@gmail.com>
> > > > Signed-off-by: Simon Horman <horms@verge.net.au>
> > > > 
> > > > ---
> > > > 
> > > > v5
> > > > * As suggested by Guennadi Liakhovetski:
> > > >   - Allow only SH_MOBILE_SDHI_IRQ_SDCARD and
> > > >     SH_MOBILE_SDHI_IRQ_SDIO to be specified in platform data
> > > 
> > > This means, CARD_DETECT is optional in a multi-irq configuration, agree.
> > 
> > [snip]
> > 
> > > > +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
> > > > +	if (irq >= 0) {
> > > > +		multi_irq = true;
> > > > +		ret = request_irq(irq, tmio_mmc_sdcard_irq, 0,
> > > > +				  dev_name(&pdev->dev), host);
> > > > +		if (ret)
> > > > +			goto eirq_sdcard;
> > > >  	}
> > > 
> > > This means SDCARD is optional in both single- and multi-irq 
> > > configurations.
> > > 
> > > > +
> > > > +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
> > > > +	if (irq < 0)
> > > > +		goto eirq_card_detect;
> > > 
> > > This means, CARD_DETECT is compulsory in all configurations.
> > > 
> > > One of us must be speaking klingonish today.
> > 
> > Sorry, I messed that up a bit.
> > 
> > I will make CARD_DETECT optional in multi-irq configurations.
> > Likewise SDIO will be optional in multi-irq configurations.
> > 
> > But SDCARD will always be required.
> 
> Oops, that scheme won't work because in single-irq configuration
> the only IRQ will be #0, but SDCARD != 0.
> 
> So I have it in mind to allow the following combinations.
> 
> i) SDIO, SDCARD, CARD_DETECT
> ii) SDIO, SDCARD
> iii) SDCARD, CARD_DETECT
> iv) SDCARD only (sets multi_irq = true locally, but only one irq!)
> v) CARD_DETECT only (uses tmio_mmc_irq(), traditional single-irq setup)

I would make it simple:

(1) 1 IRQ: only resource #0 (CARD_DETECT, use tmio_mmc_irq())
(2) 2 or 3 IRQs: compulsory SDCARD and any further IRQs: use respective 
    specialised ISRs.

> Do you think iii and iv are necessary?

Accordingly: iii - yes, iv - no.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

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

* Re: [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers
  2011-08-17 12:22         ` Guennadi Liakhovetski
@ 2011-08-17 12:57           ` Simon Horman
  2011-08-17 22:02             ` [PATCH 3/4 v7] " Guennadi Liakhovetski
  0 siblings, 1 reply; 23+ messages in thread
From: Simon Horman @ 2011-08-17 12:57 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: linux-mmc, linux-sh, Chris Ball, Magnus Damm, Paul Mundt

On Wed, Aug 17, 2011 at 02:22:56PM +0200, Guennadi Liakhovetski wrote:
> On Wed, 17 Aug 2011, Simon Horman wrote:
> 
> > On Wed, Aug 17, 2011 at 08:41:47PM +0900, Simon Horman wrote:
> > > On Wed, Aug 17, 2011 at 01:24:58PM +0200, Guennadi Liakhovetski wrote:
> > > > On Wed, 17 Aug 2011, Simon Horman wrote:
> > > > 
> > > > > Make use of per-source irq handles if the
> > > > > platform (data) has multiple irq sources.
> > > > > 
> > > > > Also, as suggested by Guennadi Liakhovetski,
> > > > > add and use defines the index or irqs in platform data.
> > > > > 
> > > > > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > > > > Cc: Magnus Damm <magnus.damm@gmail.com>
> > > > > Signed-off-by: Simon Horman <horms@verge.net.au>
> > > > > 
> > > > > ---
> > > > > 
> > > > > v5
> > > > > * As suggested by Guennadi Liakhovetski:
> > > > >   - Allow only SH_MOBILE_SDHI_IRQ_SDCARD and
> > > > >     SH_MOBILE_SDHI_IRQ_SDIO to be specified in platform data
> > > > 
> > > > This means, CARD_DETECT is optional in a multi-irq configuration, agree.
> > > 
> > > [snip]
> > > 
> > > > > +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
> > > > > +	if (irq >= 0) {
> > > > > +		multi_irq = true;
> > > > > +		ret = request_irq(irq, tmio_mmc_sdcard_irq, 0,
> > > > > +				  dev_name(&pdev->dev), host);
> > > > > +		if (ret)
> > > > > +			goto eirq_sdcard;
> > > > >  	}
> > > > 
> > > > This means SDCARD is optional in both single- and multi-irq 
> > > > configurations.
> > > > 
> > > > > +
> > > > > +	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
> > > > > +	if (irq < 0)
> > > > > +		goto eirq_card_detect;
> > > > 
> > > > This means, CARD_DETECT is compulsory in all configurations.
> > > > 
> > > > One of us must be speaking klingonish today.
> > > 
> > > Sorry, I messed that up a bit.
> > > 
> > > I will make CARD_DETECT optional in multi-irq configurations.
> > > Likewise SDIO will be optional in multi-irq configurations.
> > > 
> > > But SDCARD will always be required.
> > 
> > Oops, that scheme won't work because in single-irq configuration
> > the only IRQ will be #0, but SDCARD != 0.
> > 
> > So I have it in mind to allow the following combinations.
> > 
> > i) SDIO, SDCARD, CARD_DETECT
> > ii) SDIO, SDCARD
> > iii) SDCARD, CARD_DETECT
> > iv) SDCARD only (sets multi_irq = true locally, but only one irq!)
> > v) CARD_DETECT only (uses tmio_mmc_irq(), traditional single-irq setup)
> 
> I would make it simple:
> 
> (1) 1 IRQ: only resource #0 (CARD_DETECT, use tmio_mmc_irq())
> (2) 2 or 3 IRQs: compulsory SDCARD and any further IRQs: use respective 
>     specialised ISRs.
> 
> > Do you think iii and iv are necessary?
> 
> Accordingly: iii - yes, iv - no.

Ok, how about this?


>From 6514563c5637e65a397d0bf88cda95168a69dea1 Mon Sep 17 00:00:00 2001
From: Simon Horman <horms@verge.net.au>
Date: Tue, 16 Aug 2011 10:42:35 +0900
Subject: [PATCH] mmc: sdhi: Make use of per-source irq handlers

Make use of per-source irq handles if the
platform (data) has multiple irq sources.

Also, as suggested by Guennadi Liakhovetski,
add and use defines the index or irqs in platform data.

Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Magnus Damm <magnus.damm@gmail.com>
Signed-off-by: Simon Horman <horms@verge.net.au>

---

v6
* As discussed with Guennadi Liakhovetski:
  - The SDCARD/SDIO change in v5 was not implemented as described.
    And the logic wasn't fully described in any case. This
    version (hope to!) implement the following logic:
    1) 1 IRQ: only resource #0 (CARD_DETECT, use tmio_mmc_irq())
    2) 2 or 3 IRQs: compulsory SDCARD and any further IRQs: use respective
       specialised ISRs.

v5
* As suggested by Guennadi Liakhovetski:
  - Allow only SH_MOBILE_SDHI_IRQ_SDCARD and
    SH_MOBILE_SDHI_IRQ_SDIO to be specified in platform data

v4
* As suggested by Guennadi Liakhovetski:
  - Correct inverted values of SH_MOBILE_SDHI_IRQ_SDCARD and
    SH_MOBILE_SDHI_IRQ_CARD_DETECT

v3
* Update for changes to "mmc: tmio: Provide separate interrupt handlers"
* As suggested by Guennadi Liakhovetski:
  - Merge in patch "mmc: sdhi: Add defines for platform irq indexes"
  - Use an enum instead of defines for irq indexes

v2
* Update for changes to "mmc: tmio: Provide separate interrupt handlers"
* Make use of defines provided by
  "mmc: sdhi: Add defines for platform irq indexes"
* As suggested by Guennadi Liakhovetski:
  - Don't use a loop to initialise irq handlers, the unrolled version
    is easier on the eyes (and exactly the same number of lines of code!)
---
 drivers/mmc/host/sh_mobile_sdhi.c  |   65 +++++++++++++++++++++++------------
 include/linux/mmc/sh_mobile_sdhi.h |    7 ++++
 2 files changed, 50 insertions(+), 22 deletions(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 774f643..2fa7bbc 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -96,7 +96,8 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
 	struct sh_mobile_sdhi_info *p = pdev->dev.platform_data;
 	struct tmio_mmc_host *host;
 	char clk_name[8];
-	int i, irq, ret;
+	int irq, ret, irq_count = 0;
+	irqreturn_t (*f)(int irq, void *devid);
 
 	priv = kzalloc(sizeof(struct sh_mobile_sdhi), GFP_KERNEL);
 	if (priv == NULL) {
@@ -153,27 +154,39 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto eprobe;
 
-	for (i = 0; i < 3; i++) {
-		irq = platform_get_irq(pdev, i);
-		if (irq < 0) {
-			if (i) {
-				continue;
-			} else {
-				ret = irq;
-				goto eirq;
-			}
-		}
-		ret = request_irq(irq, tmio_mmc_irq, 0,
+	/* Allow a single IRQ resource #0 (CARD_DETECT) which will
+	 * use tmio_mmc_irq() or;
+	 * Allow 2 or 3 IRQ resources in which case SDCARD is required
+	 * and specialised ISRs are used.
+	 */
+	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
+	if (irq >= 0) {
+		irq_count++;
+		ret = request_irq(irq, tmio_mmc_sdcard_irq, 0,
 				  dev_name(&pdev->dev), host);
-		if (ret) {
-			while (i--) {
-				irq = platform_get_irq(pdev, i);
-				if (irq >= 0)
-					free_irq(irq, host);
-			}
-			goto eirq;
-		}
+		if (ret)
+			goto eirq_sdcard;
 	}
+
+	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
+	if (irq_count && irq >= 0) {
+		irq_count++;
+		ret = request_irq(irq, tmio_mmc_sdio_irq, 0,
+				  dev_name(&pdev->dev), host);
+		if (ret)
+			goto eirq_sdio;
+	} else if (!irq_count)
+		goto eirq_sdio;
+
+	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
+	if (irq >= 0) {
+		f = irq_count ? tmio_mmc_card_detect_irq : tmio_mmc_irq;
+		ret = request_irq(irq, f, 0, dev_name(&pdev->dev), host);
+		if (ret)
+			goto eirq_card_detect;
+	} else if (irq_count < 2)
+		goto eirq_card_detect;
+
 	dev_info(&pdev->dev, "%s base at 0x%08lx clock rate %u MHz\n",
 		 mmc_hostname(host->mmc), (unsigned long)
 		 (platform_get_resource(pdev,IORESOURCE_MEM, 0)->start),
@@ -181,7 +194,15 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
 
 	return ret;
 
-eirq:
+eirq_card_detect:
+	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
+	if (irq >= 0)
+		free_irq(irq, host);
+eirq_sdio:
+	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
+	if (irq >= 0)
+		free_irq(irq, host);
+eirq_sdcard:
 	tmio_mmc_host_remove(host);
 eprobe:
 	clk_disable(priv->clk);
@@ -203,7 +224,7 @@ static int sh_mobile_sdhi_remove(struct platform_device *pdev)
 
 	tmio_mmc_host_remove(host);
 
-	for (i = 0; i < 3; i++) {
+	for (i = 0; i < SH_MOBILE_SDHI_IRQ_MAX; i++) {
 		irq = platform_get_irq(pdev, i);
 		if (irq >= 0)
 			free_irq(irq, host);
diff --git a/include/linux/mmc/sh_mobile_sdhi.h b/include/linux/mmc/sh_mobile_sdhi.h
index bd50b36..80d3629 100644
--- a/include/linux/mmc/sh_mobile_sdhi.h
+++ b/include/linux/mmc/sh_mobile_sdhi.h
@@ -3,6 +3,13 @@
 
 #include <linux/types.h>
 
+enum {
+	SH_MOBILE_SDHI_IRQ_CARD_DETECT = 0,
+	SH_MOBILE_SDHI_IRQ_SDCARD,
+	SH_MOBILE_SDHI_IRQ_SDIO,
+	SH_MOBILE_SDHI_IRQ_MAX
+};
+
 struct platform_device;
 struct tmio_mmc_data;
 
-- 
1.7.5.4


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

* [PATCH 3/4 v7] mmc: sdhi: Make use of per-source irq handlers
  2011-08-17 12:57           ` Simon Horman
@ 2011-08-17 22:02             ` Guennadi Liakhovetski
  0 siblings, 0 replies; 23+ messages in thread
From: Guennadi Liakhovetski @ 2011-08-17 22:02 UTC (permalink / raw)
  To: Simon Horman; +Cc: linux-mmc, linux-sh, Chris Ball, Magnus Damm, Paul Mundt

From: Simon Horman <horms@verge.net.au>

Make use of per-source irq handles if the
platform (data) has multiple irq sources.

Also, as suggested by Guennadi Liakhovetski,
add and use defines the index or irqs in platform data.

Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Magnus Damm <magnus.damm@gmail.com>
Signed-off-by: Simon Horman <horms@verge.net.au>
[g.liakhovetski@gmx.de: fixed IRQ requesting logic]
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---

No, your last version was wrong too. Please, give this a go and tell me if 
it works as expected.

Thanks
Guennadi

 drivers/mmc/host/sh_mobile_sdhi.c  |   67 ++++++++++++++++++++++++------------
 include/linux/mmc/sh_mobile_sdhi.h |    7 ++++
 2 files changed, 52 insertions(+), 22 deletions(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 774f643..2e3ec8b 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -96,7 +96,9 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
 	struct sh_mobile_sdhi_info *p = pdev->dev.platform_data;
 	struct tmio_mmc_host *host;
 	char clk_name[8];
-	int i, irq, ret;
+	int irq, ret;
+	irqreturn_t (*f)(int irq, void *devid);
+	bool multi_irq = false;
 
 	priv = kzalloc(sizeof(struct sh_mobile_sdhi), GFP_KERNEL);
 	if (priv == NULL) {
@@ -153,27 +155,40 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto eprobe;
 
-	for (i = 0; i < 3; i++) {
-		irq = platform_get_irq(pdev, i);
-		if (irq < 0) {
-			if (i) {
-				continue;
-			} else {
-				ret = irq;
-				goto eirq;
-			}
-		}
-		ret = request_irq(irq, tmio_mmc_irq, 0,
+	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
+	if (irq >= 0) {
+		multi_irq = true;
+		ret = request_irq(irq, tmio_mmc_sdio_irq, 0,
 				  dev_name(&pdev->dev), host);
-		if (ret) {
-			while (i--) {
-				irq = platform_get_irq(pdev, i);
-				if (irq >= 0)
-					free_irq(irq, host);
-			}
-			goto eirq;
-		}
+		if (ret)
+			goto eirq_sdio;
 	}
+
+	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
+	if (irq >= 0) {
+		multi_irq = true;
+		ret = request_irq(irq, tmio_mmc_sdcard_irq, 0,
+				  dev_name(&pdev->dev), host);
+		if (ret)
+			goto eirq_sdcard;
+	} else if (multi_irq) {
+		ret = irq;
+		goto eirq_sdcard;
+	}
+
+	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
+	if (irq >= 0) {
+		f = multi_irq ? tmio_mmc_card_detect_irq : tmio_mmc_irq;
+		ret = request_irq(irq, f, 0, dev_name(&pdev->dev), host);
+		if (ret)
+			goto eirq_card_detect;
+	} else if (!multi_irq) {
+		ret = irq;
+		goto eirq_card_detect;
+	} else {
+		ret = 0;
+	}
+
 	dev_info(&pdev->dev, "%s base at 0x%08lx clock rate %u MHz\n",
 		 mmc_hostname(host->mmc), (unsigned long)
 		 (platform_get_resource(pdev,IORESOURCE_MEM, 0)->start),
@@ -181,7 +196,15 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
 
 	return ret;
 
-eirq:
+eirq_card_detect:
+	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
+	if (irq >= 0)
+		free_irq(irq, host);
+eirq_sdcard:
+	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
+	if (irq >= 0)
+		free_irq(irq, host);
+eirq_sdio:
 	tmio_mmc_host_remove(host);
 eprobe:
 	clk_disable(priv->clk);
@@ -203,7 +226,7 @@ static int sh_mobile_sdhi_remove(struct platform_device *pdev)
 
 	tmio_mmc_host_remove(host);
 
-	for (i = 0; i < 3; i++) {
+	for (i = 0; i < SH_MOBILE_SDHI_IRQ_MAX; i++) {
 		irq = platform_get_irq(pdev, i);
 		if (irq >= 0)
 			free_irq(irq, host);
diff --git a/include/linux/mmc/sh_mobile_sdhi.h b/include/linux/mmc/sh_mobile_sdhi.h
index bd50b36..80d3629 100644
--- a/include/linux/mmc/sh_mobile_sdhi.h
+++ b/include/linux/mmc/sh_mobile_sdhi.h
@@ -3,6 +3,13 @@
 
 #include <linux/types.h>
 
+enum {
+	SH_MOBILE_SDHI_IRQ_CARD_DETECT = 0,
+	SH_MOBILE_SDHI_IRQ_SDCARD,
+	SH_MOBILE_SDHI_IRQ_SDIO,
+	SH_MOBILE_SDHI_IRQ_MAX
+};
+
 struct platform_device;
 struct tmio_mmc_data;
 
-- 
1.7.2.5


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

* [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers
  2011-08-19  1:10 [PATCH 0/4 v6] mmc: tmio, sdhi: provide multiple irq handlers Simon Horman
@ 2011-08-19  1:10 ` Simon Horman
  0 siblings, 0 replies; 23+ messages in thread
From: Simon Horman @ 2011-08-19  1:10 UTC (permalink / raw)
  To: linux-mmc, linux-sh
  Cc: Chris Ball, Paul Mundt, Guennadi Liakhovetski, Magnus Damm,
	Simon Horman

Make use of per-source irq handles if the
platform (data) has multiple irq sources.

Also, as suggested by Guennadi Liakhovetski,
add and use defines the index or irqs in platform data.

Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Magnus Damm <magnus.damm@gmail.com>
Signed-off-by: Simon Horman <horms@verge.net.au>

---

v6
* As discussed with Guennadi Liakhovetski:
  - The SDCARD/SDIO change in v5 was not implemented as described.
    And the logic wasn't fully described in any case. This
    version (hope to!) implement the following logic:
    1) 1 IRQ: only resource #0 (CARD_DETECT, use tmio_mmc_irq())
    2) 2 or 3 IRQs: compulsory SDCARD and any further IRQs: use respective
       specialised ISRs.

v5
* As suggested by Guennadi Liakhovetski:
  - Allow only SH_MOBILE_SDHI_IRQ_SDCARD and
    SH_MOBILE_SDHI_IRQ_SDIO to be specified in platform data

v4
* As suggested by Guennadi Liakhovetski:
  - Correct inverted values of SH_MOBILE_SDHI_IRQ_SDCARD and
    SH_MOBILE_SDHI_IRQ_CARD_DETECT

v3
* Update for changes to "mmc: tmio: Provide separate interrupt handlers"
* As suggested by Guennadi Liakhovetski:
  - Merge in patch "mmc: sdhi: Add defines for platform irq indexes"
  - Use an enum instead of defines for irq indexes

v2
* Update for changes to "mmc: tmio: Provide separate interrupt handlers"
* Make use of defines provided by
  "mmc: sdhi: Add defines for platform irq indexes"
* As suggested by Guennadi Liakhovetski:
  - Don't use a loop to initialise irq handlers, the unrolled version
    is easier on the eyes (and exactly the same number of lines of code!)
---
 drivers/mmc/host/sh_mobile_sdhi.c  |   65 +++++++++++++++++++++++------------
 include/linux/mmc/sh_mobile_sdhi.h |    7 ++++
 2 files changed, 50 insertions(+), 22 deletions(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 774f643..2fa7bbc 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -96,7 +96,8 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
 	struct sh_mobile_sdhi_info *p = pdev->dev.platform_data;
 	struct tmio_mmc_host *host;
 	char clk_name[8];
-	int i, irq, ret;
+	int irq, ret, irq_count = 0;
+	irqreturn_t (*f)(int irq, void *devid);
 
 	priv = kzalloc(sizeof(struct sh_mobile_sdhi), GFP_KERNEL);
 	if (priv == NULL) {
@@ -153,27 +154,39 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto eprobe;
 
-	for (i = 0; i < 3; i++) {
-		irq = platform_get_irq(pdev, i);
-		if (irq < 0) {
-			if (i) {
-				continue;
-			} else {
-				ret = irq;
-				goto eirq;
-			}
-		}
-		ret = request_irq(irq, tmio_mmc_irq, 0,
+	/* Allow a single IRQ resource #0 (CARD_DETECT) which will
+	 * use tmio_mmc_irq() or;
+	 * Allow 2 or 3 IRQ resources in which case SDCARD is required
+	 * and specialised ISRs are used.
+	 */
+	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
+	if (irq >= 0) {
+		irq_count++;
+		ret = request_irq(irq, tmio_mmc_sdcard_irq, 0,
 				  dev_name(&pdev->dev), host);
-		if (ret) {
-			while (i--) {
-				irq = platform_get_irq(pdev, i);
-				if (irq >= 0)
-					free_irq(irq, host);
-			}
-			goto eirq;
-		}
+		if (ret)
+			goto eirq_sdcard;
 	}
+
+	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
+	if (irq_count && irq >= 0) {
+		irq_count++;
+		ret = request_irq(irq, tmio_mmc_sdio_irq, 0,
+				  dev_name(&pdev->dev), host);
+		if (ret)
+			goto eirq_sdio;
+	} else if (!irq_count)
+		goto eirq_sdio;
+
+	ret = irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
+	if (irq >= 0) {
+		f = irq_count ? tmio_mmc_card_detect_irq : tmio_mmc_irq;
+		ret = request_irq(irq, f, 0, dev_name(&pdev->dev), host);
+		if (ret)
+			goto eirq_card_detect;
+	} else if (irq_count < 2)
+		goto eirq_card_detect;
+
 	dev_info(&pdev->dev, "%s base at 0x%08lx clock rate %u MHz\n",
 		 mmc_hostname(host->mmc), (unsigned long)
 		 (platform_get_resource(pdev,IORESOURCE_MEM, 0)->start),
@@ -181,7 +194,15 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
 
 	return ret;
 
-eirq:
+eirq_card_detect:
+	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
+	if (irq >= 0)
+		free_irq(irq, host);
+eirq_sdio:
+	irq = platform_get_irq(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
+	if (irq >= 0)
+		free_irq(irq, host);
+eirq_sdcard:
 	tmio_mmc_host_remove(host);
 eprobe:
 	clk_disable(priv->clk);
@@ -203,7 +224,7 @@ static int sh_mobile_sdhi_remove(struct platform_device *pdev)
 
 	tmio_mmc_host_remove(host);
 
-	for (i = 0; i < 3; i++) {
+	for (i = 0; i < SH_MOBILE_SDHI_IRQ_MAX; i++) {
 		irq = platform_get_irq(pdev, i);
 		if (irq >= 0)
 			free_irq(irq, host);
diff --git a/include/linux/mmc/sh_mobile_sdhi.h b/include/linux/mmc/sh_mobile_sdhi.h
index bd50b36..80d3629 100644
--- a/include/linux/mmc/sh_mobile_sdhi.h
+++ b/include/linux/mmc/sh_mobile_sdhi.h
@@ -3,6 +3,13 @@
 
 #include <linux/types.h>
 
+enum {
+	SH_MOBILE_SDHI_IRQ_CARD_DETECT = 0,
+	SH_MOBILE_SDHI_IRQ_SDCARD,
+	SH_MOBILE_SDHI_IRQ_SDIO,
+	SH_MOBILE_SDHI_IRQ_MAX
+};
+
 struct platform_device;
 struct tmio_mmc_data;
 
-- 
1.7.5.4


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

end of thread, other threads:[~2011-08-19  1:10 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-08-17 10:59 [PATCH 0/4 v5] mmc: tmio, sdhi: provide multiple irq handlers Simon Horman
2011-08-17 10:59 ` [PATCH 1/4] mmc: tmio: Cache interrupt masks Simon Horman
2011-08-17 10:59 ` [PATCH 2/4] mmc: tmio: Provide separate interrupt handlers Simon Horman
2011-08-17 10:59 ` [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers Simon Horman
2011-08-17 11:24   ` Guennadi Liakhovetski
2011-08-17 11:41     ` Simon Horman
2011-08-17 11:54       ` Guennadi Liakhovetski
2011-08-17 12:07       ` Simon Horman
2011-08-17 12:22         ` Guennadi Liakhovetski
2011-08-17 12:57           ` Simon Horman
2011-08-17 22:02             ` [PATCH 3/4 v7] " Guennadi Liakhovetski
2011-08-17 10:59 ` [PATCH 4/4] ARM: shmobile: ag5evm, ap4: Make use of irq index enum Simon Horman
  -- strict thread matches above, loose matches on Subject: below --
2011-08-19  1:10 [PATCH 0/4 v6] mmc: tmio, sdhi: provide multiple irq handlers Simon Horman
2011-08-19  1:10 ` [PATCH 3/4] mmc: sdhi: Make use of per-source " Simon Horman
2011-08-17  0:50 [PATCH 0/4 v4] mmc: tmio, sdhi: provide multiple " Simon Horman
2011-08-17  0:50 ` [PATCH 3/4] mmc: sdhi: Make use of per-source " Simon Horman
2011-08-17  8:20   ` Guennadi Liakhovetski
2011-08-17  9:49     ` Simon Horman
2011-08-17 10:06       ` Guennadi Liakhovetski
2011-08-17 10:46         ` Simon Horman
2011-08-16 10:11 [PATCH 0/4 v3] mmc: tmio, sdhi: provide multiple " Simon Horman
2011-08-16 10:11 ` [PATCH 3/4] mmc: sdhi: Make use of per-source " Simon Horman
2011-08-16 11:11   ` Guennadi Liakhovetski
2011-08-16 11:51     ` Simon Horman
2011-08-16 12:40       ` Guennadi Liakhovetski
2011-08-16 13:45         ` Simon Horman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox