On Tue, Aug 16, 2011 at 01:13:06PM +0200, Guennadi Liakhovetski wrote:
> On Tue, 16 Aug 2011, Simon Horman wrote:
> 
> > On Tue, Aug 16, 2011 at 09:41:52AM +0200, Guennadi Liakhovetski wrote:
> > > On Tue, 16 Aug 2011, Simon Horman wrote:
> 
> [snip]
> 
> > > > +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);
> > > > +       __tmio_mmc_card_detect_irq(host, ireg, status);
> > > 
> > > Same here - I would return, if a card hot-plug event occurred.
> > 
> > Will do.
> > 
> > > > +       __tmio_mmc_sdcard_irq(host, ireg, status);
> > > 
> > > Ditto
> > > 
> > > > +
> > > > +       tmio_mmc_sdio_irq(irq, devid);
> > > 
> > > Any specific reason, why you now process SDIO IRQs last?
> > 
> > I believe this is in keeping with the ordering implied by original code.
> 
> I believe it's not. The original version did SDIO first, then hotplug, 
> then normal IO.

My reading of the original code is that SDIO was the lowest priority
although its code appeared near the top of tmio_mmc_irq().

irqreturn_t tmio_mmc_irq(int irq, void *devid)
{
        ...

        status = sd_ctrl_read32(host, CTL_STATUS);
        irq_mask = sd_ctrl_read32(host, CTL_IRQ_MASK);
        ireg = status & TMIO_MASK_IRQ & ~irq_mask;

        sdio_ireg = 0;
        if (!ireg pdata->flags & TMIO_MMC_SDIO_IRQ) {
                /* Handle SDIO Interrupt */
                ...
                goto out;
        }

        /* Handle Card detect Interrupts */

        /* Handle other Interrupts */

        ...
}

> I'm not necessarily saying, that the original code was 
> correct or better, I'm just saying, it was different. As for which one we 
> should prefer... I think, I'd check for hotplug first: if the card is 
> removed, no reason to try to communicate with it. And we have to first 
> report a new card, before reporting any IRQs from it. But then - IO or 
> SDIO as second? Well, since SDIO IRQs are asynchronous, it shouldn't be a 
> big problem for them to wait a bit, whereas normal IO IRQs are card's 
> response to host's actions, so, the originator might want to know the 
> result before processing any asynchronous events. So, I actually like your 
> ordering better... But I'll give it a short spin with SDIO, unless you do 
> it yourself.

I intend to test my code with SDIO, however I don't have access to hardware
at this exact moment. So if you could do so, that would be great.

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to