Re: [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers

From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
To: Simon Horman <horms@verge.net.au>
Cc: linux-mmc@vger.kernel.org, linux-sh@vger.kernel.org,
	Chris Ball <cjb@laptop.org>, Magnus Damm <magnus.damm@gmail.com>,
	Paul Mundt <lethal@linux-sh.org>
Subject: Re: [PATCH 3/4] mmc: sdhi: Make use of per-source irq handlers
Date: Wed, 17 Aug 2011 12:22:56 +0000	[thread overview]
Message-ID: <Pine.LNX.4.64.1108171416010.18317@axis700.grange> (raw)
In-Reply-To: <20110817120748.GB13086@verge.net.au>

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/