On 16 August 2015 at 19:18, Robert Jarzmik <robert.jarz...@free.fr> wrote: > Ezequiel Garcia <ezequ...@vanguardiasur.com.ar> writes: > >> On 12 Aug 06:22 PM, Robert Jarzmik wrote: >> >> This fix looks correct. Thanks! >> >> Couple questions: >> >> 1. In which platform are you seeing this bug? > zylonite with a pxa310 (ie. internal stacked NAND). > >> 2. Is this a regression? (i.e. should we queue it for -stable?) > No, it's been there for ages I think. > >> Also, one might question why we can't just write NDSR right after it's read, >> before we wake the IRQ thread or start DMA. It appears this is >> a requirement of BCH, as per the comment in drain_fifo. > For irq thread that won't make any difference, the irq handler will finish > first > and clear the bits anyway. For DMA it's better. > > And more generaly speaking, I like it better, to clear it once read. > >> It would be nice to put a comment explaining why we clear NDSR only >> before the check to WRCMDREQ. Maybe even copy-pasting something >> from the commit log? > If we move it up to something like that : > status = nand_readl(info, NDSR); > nand_writel(info, NDSR, status); > Then the comment is overkill I think. >
Well, the comment in drain_fifo says that doing this would break reads with BCH enabled: /* * According to the datasheet, when reading from NDDB * with BCH enabled, after each 32 bytes reads, we * have to make sure that the NDSR.RDDREQ bit is set. * * Drain the FIFO 8 32 bits reads at a time, and skip * the polling on the last read. */ In other words, it seems that we must wake the IRQ thread handler _before_ we clear RDDREQ, not after. So unless I'm completely off, the current patch is right, and a comment would be helpful. >> I'd like to say "Yay, let's pick it" but I'd like to make sure this is >> tested on all platforms first (unless you've tested it already). > I tested on zylonite (where bug occurs) and cm-x300 (where bug never occurs). > OK, I'll see about testing your four patches on some Armada 370/XP. -- Ezequiel GarcĂa, VanguardiaSur www.vanguardiasur.com.ar -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/