Web lists-archives.com

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




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