From patchwork Wed Aug 17 12:57:31 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Horman X-Patchwork-Id: 1073852 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter2.kernel.org (8.14.4/8.14.4) with ESMTP id p7HCvdUq010455 for ; Wed, 17 Aug 2011 12:57:39 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752890Ab1HQM5i (ORCPT ); Wed, 17 Aug 2011 08:57:38 -0400 Received: from kirsty.vergenet.net ([202.4.237.240]:57492 "EHLO kirsty.vergenet.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752126Ab1HQM5h (ORCPT ); Wed, 17 Aug 2011 08:57:37 -0400 Received: from joe.akashicho.tokyo.vergenet.net (joe.akashicho.tokyo.vergenet.net [IPv6:2001:470:f44f:403:1ec1:deff:fe98:754d]) by kirsty.vergenet.net (Postfix) with ESMTP id AF3AD24062; Wed, 17 Aug 2011 22:57:33 +1000 (EST) Received: by joe.akashicho.tokyo.vergenet.net (Postfix, from userid 7100) id B7CC028A034; Wed, 17 Aug 2011 21:57:31 +0900 (JST) Date: Wed, 17 Aug 2011 21:57:31 +0900 From: Simon Horman To: Guennadi Liakhovetski Cc: linux-mmc@vger.kernel.org, linux-sh@vger.kernel.org, Chris Ball , Magnus Damm , Paul Mundt Subject: Re: [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers Message-ID: <20110817125728.GD13086@verge.net.au> References: <1313578756-10413-1-git-send-email-horms@verge.net.au> <1313578756-10413-4-git-send-email-horms@verge.net.au> <20110817114147.GA13086@verge.net.au> <20110817120748.GB13086@verge.net.au> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Organisation: Horms Solutions Ltd. User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-sh-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-sh@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter2.kernel.org [140.211.167.43]); Wed, 17 Aug 2011 12:57:39 +0000 (UTC) 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 > > > > > Cc: Magnus Damm > > > > > Signed-off-by: Simon Horman > > > > > > > > > > --- > > > > > > > > > > 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 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 Cc: Magnus Damm Signed-off-by: Simon Horman --- 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 +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;