Messages in this thread Patch in this message | | | Date | Thu, 3 May 2018 09:05:07 +0200 | From | Miquel Raynal <> | Subject | Re: [PATCH] mtd: rawnand: marvell: pass ms delay to wait_op |
| |
Hi Chris,
On Thu, 3 May 2018 05:28:32 +0000, Chris Packham <Chris.Packham@alliedtelesis.co.nz> wrote:
> On 03/05/18 14:21, Chris Packham wrote: > > marvell_nfc_wait_op() expects the delay to be expressed in milliseconds > > but nand_sdr_timings uses picoseconds. Use PSEC_TO_MSEC when passing > > tPROG_max to marvell_nfc_wait_op(). > > > > Fixes: 02f26ecf8c772 ("mtd: nand: add reworked Marvell NAND controller driver") > > Cc: stable@vger.kernel.org > > Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> > > --- > > drivers/mtd/nand/raw/marvell_nand.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/mtd/nand/raw/marvell_nand.c b/drivers/mtd/nand/raw/marvell_nand.c > > index 1d779a35ac8e..e4b964fd40d8 100644 > > --- a/drivers/mtd/nand/raw/marvell_nand.c > > +++ b/drivers/mtd/nand/raw/marvell_nand.c > > @@ -1074,7 +1074,7 @@ static int marvell_nfc_hw_ecc_hmg_do_write_page(struct nand_chip *chip, > > return ret; > > > > ret = marvell_nfc_wait_op(chip, > > - chip->data_interface.timings.sdr.tPROG_max); > > + PSEC_TO_MSEC(chip->data_interface.timings.sdr.tPROG_max)); > > return ret; > > } > > > > @@ -1494,7 +1494,7 @@ static int marvell_nfc_hw_ecc_bch_write_page(struct mtd_info *mtd, > > } > > > > ret = marvell_nfc_wait_op(chip, > > - chip->data_interface.timings.sdr.tPROG_max); > > + PSEC_TO_MSEC(chip->data_interface.timings.sdr.tPROG_max)); > > > > marvell_nfc_disable_hw_ecc(chip); > > > > Actually I'm not so sure about this patch. While passing the pico-second > value for tPROG_max is clearly wrong and leads to seemingly indefinite > hangs on some systems. Converting the times to micro-seconds leaves us > with delays that are far too short.
It is not micro but milli-seconds here.
> > The old pxa3xx driver had hard coded 200ms delays. These delays now work > out to 1ms which seems every bit as wrong as 600000000ms.
200ms is extremely long, I guess typical values are ~200-500us, so this 1ms timeout does not scare me :)
I'll let Boris conclude.
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Thanks.
-- Miquel Raynal, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com
| |