| Message ID | 1313578756-10413-4-git-send-email-horms@verge.net.au (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
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/ -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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/ -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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? -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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/ -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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;
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(-)