On Fri, Aug 19, 2011 at 1:59 PM, Simon Horman [off-list ref] wrote:

On Fri, Aug 19, 2011 at 01:27:41PM +0900, Magnus Damm wrote:

quoted

On Fri, Aug 19, 2011 at 12:30 PM, Simon Horman [off-list ref] wrote:

quoted

On Fri, Aug 19, 2011 at 12:09:50PM +0900, Magnus Damm wrote:

quoted

On Fri, Aug 19, 2011 at 10:10 AM, Simon Horman [off-list ref] wrote:

quoted

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 <redacted>
Cc: Magnus Damm <magnus.damm@gmail.com>
Signed-off-by: Simon Horman <horms@verge.net.au>

---

quoted

+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);

Is there any particular reason for returning early in this interrupt
handler? By returning early I mean the "if ... return IRQ_HANDLED"
cases above.

I realize the old ISR code in the driver does just this, so if the
goal is to stay compatible then I guess we should keep this behavior.

quoted

From my point of view it usually makes more sense to try to handle all

events that may be associated with the IRQ.

My original post had the behaviour that you suggest but Guennadi
indicted that he would be much more comfortable with keeping the original
behaviour as it is know to work on a wide range of hardware.

I see, thanks for your patience...

So I may remember this wrong, but for the 3 different interrupt
sources I believe that the SDIO IRQ code was added by Arnd for one of
the Renesas SDHI platforms. Back then Ian disliked supporting more
than a single interrupt source, so for that reason the SDIO IRQ code
was added on top of the common interrupt handler. We had no
documentation either, so it was added in a rather random way. I recall
the SDIO IRQ being handled before the other interrupt types in the
common handler not last, but that's not very important. Anyway, with
the fact that SDIO IRQ support was added for SDHI platforms in mind
then we can assume that other platforms won't need it. Not sure if
this fact will improve our situation or not.

As for the two remaining interrupts, I believe they share hardware
registers somehow. I guess I'm OK keeping the original behavior
somehow, but I still believe it's incorrect. It may not matter very
much though since it's rather unlikely that hotplug insertion or eject
coincides with the data IRQs.

Hi Magnus,

to be honest I'm unsure if the current behaviour is correct or not.
But it does appear to work on a wide range of hardware. And as
tmio_mmc_irq() is an implementation of the single-source ISR intended
for legacy purposes it does seem reasonable to maintain the existing
behaviour.

Sure, I agree.

If there is a need to change the behaviour of tmio_mmc_irq() in some way
then I think that is best dealt with in a separate patch (series)
subsequent to the current series that is intended to introduce broken-out
handlers.

Sounds good, I agree that this should be handled separately if needed.

Thanks,

/ magnus