Messages in this thread | | | Date | Wed, 15 Feb 2023 14:09:50 +0100 | From | Miquel Raynal <> | Subject | Re: [PATCH] mtd: nand: mxic-ecc: Fix mxic_ecc_data_xfer_wait_for_completion() when irq is used |
| |
Hi Christophe,
christophe.jaillet@wanadoo.fr wrote on Wed, 15 Feb 2023 12:08:45 +0100:
> wait_for_completion_timeout() and readl_poll_timeout() don't handle their > return value the same way. > > wait_for_completion_timeout() returns 0 on time out (and >0 in all other > cases) > readl_poll_timeout() returns 0 on success and -ETIMEDOUT upon a timeout.
That's a shame, but yeah, excellent catch!
> In order for the error handling path to work in both cases, the logic > against wait_for_completion_timeout() needs to be inverted. > > Fixes: 48e6633a9fa2 ("mtd: nand: mxic-ecc: Add Macronix external ECC engine support") > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > --- > Compile tested only. > > This is really spurious. > If I'm right, this means that it never worked! > > Can any one with the hardware test?
The design I used for development and testing had no interrupt line available for that IIRC, so I only tested the polling case ('else' side) and completely overlooked that difference. I might have mentioned it in the cover letter, if I didn't, it's an oversight.
> --- > drivers/mtd/nand/ecc-mxic.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/mtd/nand/ecc-mxic.c b/drivers/mtd/nand/ecc-mxic.c > index 8afdca731b87..6b487ffe2f2d 100644 > --- a/drivers/mtd/nand/ecc-mxic.c > +++ b/drivers/mtd/nand/ecc-mxic.c > @@ -429,6 +429,7 @@ static int mxic_ecc_data_xfer_wait_for_completion(struct mxic_ecc_engine *mxic) > mxic_ecc_enable_int(mxic); > ret = wait_for_completion_timeout(&mxic->complete, > msecs_to_jiffies(1000)); > + ret = ret ? 0 : -ETIMEDOUT; > mxic_ecc_disable_int(mxic); > } else { > ret = readl_poll_timeout(mxic->regs + INTRPT_STS, val,
Thanks, Miquèl
| |