Re: [PATCH v2] mtd: rawnand: marvell: check for RDY bits after enabling the IRQ

[Miquel Raynal <miquel.raynal@xxxxxxxxxxx>]

Hi Chris,

Chris Packham <Chris.Packham@xxxxxxxxxxxxxxxxxxx> wrote on Tue, 2 Oct
2018 20:53:14 +0000:

> On 02/10/18 21:23, Daniel Mack wrote:
> > Hi Miquel,
> > 
> > On 2/10/2018 9:25 AM, Miquel Raynal wrote:  
> >> Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote on Tue, 2 Oct 2018
> >> 08:46:01 +0200:  
> >>> Boris Brezillon <boris.brezillon@xxxxxxxxxxx> wrote on Tue, 2 Oct 2018
> >>> 00:13:28 +0200:  
> >   
> >>>> Also, it looks like complete() is not called until the RDDREQ, WRDREQ
> >>>> and WRCMDREQ are cleared in the interrupt handler [1], which is weird.
> >>>> Miquel, do you happen to remember why you had to do that?  
> >>>
> >>> The RDDREQ, WRDREQ and WRCMDREQ events might potentially happen while
> >>> the interrupts are enabled while we only wait for R/B signalling. This
> >>> check is to avoid calling complete() on these situations.  
> >>
> >> Actually Boris is right on the fact that while the intention is good,
> >> the writing of [1] is not accurate. Daniel, could you please test if
> >> the following diff changes something with your setup, without your
> >> patch?
> >>
> >>
> >> diff --git a/drivers/mtd/nand/raw/marvell_nand.c b/drivers/mtd/nand/raw/marvell_nand.c
> >> index bc2ef5209783..c7573ccdbacd 100644
> >> --- a/drivers/mtd/nand/raw/marvell_nand.c
> >> +++ b/drivers/mtd/nand/raw/marvell_nand.c
> >> @@ -686,7 +686,7 @@ static irqreturn_t marvell_nfc_isr(int irq, void *dev_id)
> >>    
> >>           marvell_nfc_disable_int(nfc, st & NDCR_ALL_INT);
> >>    
> >> -       if (!(st & (NDSR_RDDREQ | NDSR_WRDREQ | NDSR_WRCMDREQ)))
> >> +       if (st & (NDSR_RDY(0) | NDSR_RDY(1)))
> >>                   complete(&nfc->complete);
> >>    
> >>           return IRQ_HANDLED;  
> > 
> > Yes! That seems to work nicely as a replacement for my patch.
> > 
> > Chris, how is that going on your board?  
> 
> Looks good to me. I've just re-run stress_{1,2,3} on the custom board 
> and DB-88F6820-AMC.

Great! There is still something weird about the complete() though.

Daniel, do you plan to send the above change or do you want me to do
it? I would like to integrate the fix in nand/next before sending the
PR.


Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/