On Thu, 15 Jul 2010 13:25:52 -0700 Andrew Morton <[email protected]> wrote:
> On Tue, 13 Jul 2010 10:16:39 +0900 > Yusuke Goda <[email protected]> wrote: > > > Hi Andrew > > > > Thank you for your comment. > > > > >> #define ack_mmc_irqs(host, i) \ > > >> do { \ > > >> - u32 mask;\ > > >> - mask = sd_ctrl_read32((host), CTL_STATUS); \ > > >> - mask &= ~((i) & TMIO_MASK_IRQ); \ > > >> - sd_ctrl_write32((host), CTL_STATUS, mask); \ > > >> + sd_ctrl_write32((host), CTL_STATUS, ~(i)); \ > > >> } while (0) > > > > > > Can we have a better changelog please? > > > > > > What was wrong with the old code? > > > > > > How does the patch fix it? > > > > > > What are the user-visible runtime effects of the bug? > > > > > > (It looks like that was a pretty gross bug - how did it pass testing??) > > Example > > - CMD53(Single block read / Received data size : 64Byte) > > > > 1) Send CMD53 > > 2) Receive "CMD53 response" > > 3) Call tmio_mmc_cmd_irq(host, status); > > -- original code ---------------------------------------------------- > > #define ack_mmc_irqs(host, i) \ > > do { \ > > u32 mask;\ > > mask = sd_ctrl_read32((host), CTL_STATUS); \ > > < case 1 > > > mask &= ~((i) & TMIO_MASK_IRQ); \ > > < case 2 > > > sd_ctrl_write32((host), CTL_STATUS, mask); \ > > } while (0) > > --------------------------------------------------------------------- > > > > TMIO_STAT_RXRDY status will be cleared by "sd_ctrl_write32((host), > > CTL_STATUS, mask);" > > if TMIO_STAT_RXRDY becomes effective between "< case 1 >" and "< case 2 >". > > > > This causes the phenomenon that a TMIO_STAT_RXRDY interrupt does not occur. > > When received data are small, it rarely occurs. > > > > OK.. > > But with both this patch and "tmio_mmc-revise-limit-on-data-size.patch" > the changelogs fail to describe the impact of the bug upon our users. > So when I sit here trying to work out whether the patches should be > applied to 2.6.35 and whether they should be backported into -stable, I > don't have enough information. > > What are your thoughts on this? Goda, do you have any more ideas on updating the changelog for this patch? It looks to me like this patch is a candidate for stable (whereas the "tmio_mmc-revise-limit-on-data-size.patch" is not, sorry about replying to that one first, I'm reading my mail backwards) because, without this patch, it's possible to miss interrupts because the ack_mmc_irqs() macro clears bit in the CTL_STATUS register that it should not do? Is that correct? If that is the case then would this be a more appropriate changelog, "tmio_mmc: Don't clear unhandled pending interrupts Previously, it was possible for ack_mmc_irqs() to clear pending interrupt bits in the CTL_STATUS register, even though the interrupt handler had not been called. This was because of a race that existed when doing a read-modify-write sequence on CTL_STATUS. After the read step in this sequence, if an interrupt occurred (causing one of the bits in CTL_STATUS to be set) the write step would inadvertently clear it. This patch eliminates this race by only writing to CTL_STATUS and clearing the interrupts that were passed as an argument to ack_mmc_irqs()." -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
