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

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

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.