Message ID | 20200519123032.32547-1-miquel.raynal@bootlin.com |
---|---|
State | Changes Requested |
Delegated to: | Miquel Raynal |
Headers | show |
Series | [v2] mtd: rawnand: micron: Adapt the PAGE READ flow to constraint controllers | expand |
On Tue, 19 May 2020 14:30:32 +0200 Miquel Raynal <miquel.raynal@bootlin.com> wrote: > There are controllers not able to just read data cycles on the > bus. There are controllers not able to do a change column. > > If we want to support both, we need to check which operation is > supported first. This is the exact same mechanism that is in use for > parameter page reads (ONFI/JEDEC) as the same problem occurs. > > Speed testing does not show any throughput penalty so we do not > optimize more than that. However it is likely that, in the future, a > more robust and exhaustive test will run at boot time to avoid > re-checking what is supported and what is not at every call. > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com> > --- > > Changes in v2: > * Added a comment before each check which is a condensate of the > commit message. I don't see those comments :P. > * Collected Boris' R-by tag. > > drivers/mtd/nand/raw/nand_micron.c | 44 ++++++++++++++++++++++++------ > 1 file changed, 35 insertions(+), 9 deletions(-) > > diff --git a/drivers/mtd/nand/raw/nand_micron.c b/drivers/mtd/nand/raw/nand_micron.c > index b2b047b245f4..bbd0ffbae19a 100644 > --- a/drivers/mtd/nand/raw/nand_micron.c > +++ b/drivers/mtd/nand/raw/nand_micron.c > @@ -192,6 +192,7 @@ static int micron_nand_on_die_ecc_status_4(struct nand_chip *chip, u8 status, > struct micron_nand *micron = nand_get_manufacturer_data(chip); > struct mtd_info *mtd = nand_to_mtd(chip); > unsigned int step, max_bitflips = 0; > + bool use_datain = false; > int ret; > > if (!(status & NAND_ECC_STATUS_WRITE_RECOMMENDED)) { > @@ -211,8 +212,18 @@ static int micron_nand_on_die_ecc_status_4(struct nand_chip *chip, u8 status, > * in non-raw mode, even if the user did not request those bytes. > */ > if (!oob_required) { > - ret = nand_read_data_op(chip, chip->oob_poi, mtd->oobsize, > - false, false); > + if (!nand_has_exec_op(chip) || > + !nand_read_data_op(chip, chip->oob_poi, mtd->oobsize, false, > + true)) > + use_datain = true; > + > + if (use_datain) > + ret = nand_read_data_op(chip, chip->oob_poi, > + mtd->oobsize, false, false); > + else > + ret = nand_change_read_column_op(chip, mtd->writesize, > + chip->oob_poi, > + mtd->oobsize, false); > if (ret) > return ret; > } > @@ -285,6 +296,7 @@ micron_nand_read_page_on_die_ecc(struct nand_chip *chip, uint8_t *buf, > int oob_required, int page) > { > struct mtd_info *mtd = nand_to_mtd(chip); > + bool use_datain = false; > u8 status; > int ret, max_bitflips = 0; > > @@ -300,14 +312,28 @@ micron_nand_read_page_on_die_ecc(struct nand_chip *chip, uint8_t *buf, > if (ret) > goto out; > > - ret = nand_exit_status_op(chip); > - if (ret) > - goto out; > + if (!nand_has_exec_op(chip) || > + !nand_read_data_op(chip, buf, mtd->writesize, false, true)) > + use_datain = true; > > - ret = nand_read_data_op(chip, buf, mtd->writesize, false, false); > - if (!ret && oob_required) > - ret = nand_read_data_op(chip, chip->oob_poi, mtd->oobsize, > - false, false); > + if (use_datain) { > + ret = nand_exit_status_op(chip); > + if (ret) > + goto out; > + > + ret = nand_read_data_op(chip, buf, mtd->writesize, false, > + false); > + if (!ret && oob_required) > + ret = nand_read_data_op(chip, chip->oob_poi, > + mtd->oobsize, false, false); > + } else { > + ret = nand_change_read_column_op(chip, 0, buf, mtd->writesize, > + false); > + if (!ret && oob_required) > + ret = nand_change_read_column_op(chip, mtd->writesize, > + chip->oob_poi, > + mtd->oobsize, false); > + } > > if (chip->ecc.strength == 4) > max_bitflips = micron_nand_on_die_ecc_status_4(chip, status,
Boris Brezillon <boris.brezillon@collabora.com> wrote on Tue, 19 May 2020 15:01:57 +0200: > On Tue, 19 May 2020 14:30:32 +0200 > Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > There are controllers not able to just read data cycles on the > > bus. There are controllers not able to do a change column. > > > > If we want to support both, we need to check which operation is > > supported first. This is the exact same mechanism that is in use for > > parameter page reads (ONFI/JEDEC) as the same problem occurs. > > > > Speed testing does not show any throughput penalty so we do not > > optimize more than that. However it is likely that, in the future, a > > more robust and exhaustive test will run at boot time to avoid > > re-checking what is supported and what is not at every call. > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com> > > --- > > > > Changes in v2: > > * Added a comment before each check which is a condensate of the > > commit message. > > I don't see those comments :P. Do you think I forgot a "git add ."?... Let me resend this one.
diff --git a/drivers/mtd/nand/raw/nand_micron.c b/drivers/mtd/nand/raw/nand_micron.c index b2b047b245f4..bbd0ffbae19a 100644 --- a/drivers/mtd/nand/raw/nand_micron.c +++ b/drivers/mtd/nand/raw/nand_micron.c @@ -192,6 +192,7 @@ static int micron_nand_on_die_ecc_status_4(struct nand_chip *chip, u8 status, struct micron_nand *micron = nand_get_manufacturer_data(chip); struct mtd_info *mtd = nand_to_mtd(chip); unsigned int step, max_bitflips = 0; + bool use_datain = false; int ret; if (!(status & NAND_ECC_STATUS_WRITE_RECOMMENDED)) { @@ -211,8 +212,18 @@ static int micron_nand_on_die_ecc_status_4(struct nand_chip *chip, u8 status, * in non-raw mode, even if the user did not request those bytes. */ if (!oob_required) { - ret = nand_read_data_op(chip, chip->oob_poi, mtd->oobsize, - false, false); + if (!nand_has_exec_op(chip) || + !nand_read_data_op(chip, chip->oob_poi, mtd->oobsize, false, + true)) + use_datain = true; + + if (use_datain) + ret = nand_read_data_op(chip, chip->oob_poi, + mtd->oobsize, false, false); + else + ret = nand_change_read_column_op(chip, mtd->writesize, + chip->oob_poi, + mtd->oobsize, false); if (ret) return ret; } @@ -285,6 +296,7 @@ micron_nand_read_page_on_die_ecc(struct nand_chip *chip, uint8_t *buf, int oob_required, int page) { struct mtd_info *mtd = nand_to_mtd(chip); + bool use_datain = false; u8 status; int ret, max_bitflips = 0; @@ -300,14 +312,28 @@ micron_nand_read_page_on_die_ecc(struct nand_chip *chip, uint8_t *buf, if (ret) goto out; - ret = nand_exit_status_op(chip); - if (ret) - goto out; + if (!nand_has_exec_op(chip) || + !nand_read_data_op(chip, buf, mtd->writesize, false, true)) + use_datain = true; - ret = nand_read_data_op(chip, buf, mtd->writesize, false, false); - if (!ret && oob_required) - ret = nand_read_data_op(chip, chip->oob_poi, mtd->oobsize, - false, false); + if (use_datain) { + ret = nand_exit_status_op(chip); + if (ret) + goto out; + + ret = nand_read_data_op(chip, buf, mtd->writesize, false, + false); + if (!ret && oob_required) + ret = nand_read_data_op(chip, chip->oob_poi, + mtd->oobsize, false, false); + } else { + ret = nand_change_read_column_op(chip, 0, buf, mtd->writesize, + false); + if (!ret && oob_required) + ret = nand_change_read_column_op(chip, mtd->writesize, + chip->oob_poi, + mtd->oobsize, false); + } if (chip->ecc.strength == 4) max_bitflips = micron_nand_on_die_ecc_status_4(chip, status,