Re: [PATCH] mtd: rawnand: marvell: prevent timeouts on a loaded machine

[Miquel Raynal <miquel.raynal@xxxxxxxxxxx>]

Hi Boris,

Boris Brezillon <boris.brezillon@xxxxxxxxxxx> wrote on Sat, 15 Dec 2018
13:41:37 +0100:

> On Tue, 11 Dec 2018 18:38:28 +0100
> Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote:
> 
> > marvell_nfc_wait_op() waits for completion during 'timeout_ms'
> > milliseconds before throwing an error. While the logic is fine, the
> > value of 'timeout_ms' is given by the core and actually correspond to
> > the maximum time the NAND chip will take to complete the
> > operation. Assuming there is no overhead in the propagation of the
> > interrupt signal to the the NAND controller (through the Ready/Busy
> > line), this delay does not take into account the latency of the
> > operating system. For instance, for a page write, the delay given by
> > the core is rounded up to 1ms. Hence, when the machine is over loaded,
> > there is chances that this timeout will be reached.
> > 
> > There are two ways to solve this issue that are not incompatible:
> > 1/ Enlarge the timeout value (if so, how much?).
> > 2/ Check after the waiting method if we did not miss any interrupt
> > because of the OS latency (an interrupt is still pending). In this
> > case, we assume the operation exited successfully.
> > 
> > We choose the second approach that is a must in all cases, with the
> > possibility to also modify the timeout value to be, e.g. at least 1
> > second in all cases.
> > 
> > Fixes: 02f26ecf8c77 ("mtd: nand: add reworked Marvell NAND controller driver")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxx>
> > ---
> >  drivers/mtd/nand/raw/marvell_nand.c | 17 ++++++++++++++---
> >  1 file changed, 14 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/raw/marvell_nand.c b/drivers/mtd/nand/raw/marvell_nand.c
> > index e6c3739cea73..bc0eef4ade4f 100644
> > --- a/drivers/mtd/nand/raw/marvell_nand.c
> > +++ b/drivers/mtd/nand/raw/marvell_nand.c
> > @@ -514,9 +514,14 @@ static void marvell_nfc_enable_int(struct marvell_nfc *nfc, u32 int_mask)
> >  	writel_relaxed(reg & ~int_mask, nfc->regs + NDCR);
> >  }
> >  
> > -static void marvell_nfc_clear_int(struct marvell_nfc *nfc, u32 int_mask)
> > +static int marvell_nfc_clear_int(struct marvell_nfc *nfc, u32 int_mask)  
> 
> 	  ^ u32 ?
> 
> With this fixed:
> 
> Reviewed-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxx>

Applied with your change to nand/next.

Thanks,
Miquèl

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