Re: [PATCH 06/11] mtd: rawnand: denali: use more precise timeout for NAND_OP_WAITRDT_INSTR

From: Masahiro Yamada
Date: Sun Feb 10 2019 - 20:27:34 EST

Hi Miquel,

On Sat, Feb 9, 2019 at 7:05 AM Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote:
> Hi Masahiro,
> Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx> wrote on Fri, 8 Feb
> 2019 17:08:50 +0900:
> > Currently, wait_for_completion_timeout() is always passed in the
> > hard-coded msec_to_jiffies(1000). There is no specific reason for
> > 1000 msec, but I just chose it long enough.
> >
> > With the exec_op() conversion, NAND_OP_WAITRDY_INSTR provides more
> > precise timeout value, depending on the preceding command. Let's use
> > it to bail out earlier in error case.
> I'm not sure using 10ms instead of 1000ms is relevant in the below
> cases, 10ms is rather short for an IRQ, if your system is under load
> you might end up with a timeout, not because the right IRQ did not
> fire, but because the handler was not executed yet (it happened to me
> in the marvell_nand.c driver recently).

Good point.
Since Linux is not RT-OS, there is no defined worst-case time
until the handler is invoked.

I will add the following to denali_wait_for_irq().

/* Prolong the IRQ wait time in case the system is under heavy load. */
timeout_ms += 100;

> Also, would you mind using a define instead of hardcoding '1000'?

I do not think this is worth doing.

> >
> > I am still keeping the hard-coded values for other higher level hooks
> > such as page_read, page_write, etc. We know the value of tR, tPROG, but
> > we have unknowledge about the data transfer speed of the DMA engine.
> >
> > Signed-off-by: Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx>
> Thanks,
> MiquÃl

Best Regards
Masahiro Yamada