Hi Miquel On 5/26/21 5:42 PM, Miquel Raynal wrote: > Hello, > > <patrice.chotard@xxxxxxxxxxx> wrote on Wed, 26 May 2021 17:30:16 +0200: > >> From: Christophe Kerello <christophe.kerello@xxxxxxxxxxx> > > Would you mind to use "add SPI-NAND MTD resume handler" as title? (with > upper case letters) Ok > >> After power up, all SPI NAND's blocks are locked. Only read operations >> are allowed, write and erase operations are forbidden. >> The SPI NAND framework unlocks all the blocks during its initialization. >> >> During a standby low power, the memory is powered down, losing its >> configuration. >> During the resume, the QSPI driver state is restored but the SPI NAND >> framework does not reconfigured the memory. >> >> This patch adds spi nand mtd PM handlers for resume ops. > > ditto ^^^^^^^^^^^^ Ok > >> SPI NAND resume op re-initializes SPI NAND flash to its probed state. >> >> Signed-off-by: Christophe Kerello <christophe.kerello@xxxxxxxxxxx> >> Signed-off-by: Patrice Chotard <patrice.chotard@xxxxxxxxxxx> >> --- >> drivers/mtd/nand/spi/core.c | 56 +++++++++++++++++++++++++++++++++++++ >> 1 file changed, 56 insertions(+) >> >> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c >> index 17f63f95f4a2..6abaf874eb3f 100644 >> --- a/drivers/mtd/nand/spi/core.c >> +++ b/drivers/mtd/nand/spi/core.c >> @@ -1074,6 +1074,61 @@ static int spinand_detect(struct spinand_device *spinand) >> return 0; >> } >> >> +static void spinand_mtd_resume(struct mtd_info *mtd) >> +{ >> + struct spinand_device *spinand = mtd_to_spinand(mtd); >> + struct nand_device *nand = mtd_to_nanddev(mtd); >> + struct device *dev = &spinand->spimem->spi->dev; >> + int ret, i; >> + >> + ret = spinand_reset_op(spinand); >> + if (ret) >> + return; >> + >> + ret = spinand_init_quad_enable(spinand); >> + if (ret) { >> + dev_err(dev, >> + "Failed to initialize the quad part (err = %d)\n", > > quad part? what about "Failed to resume the quad state" or something > alike? Agree, i will update this > >> + ret); >> + return; >> + } >> + >> + ret = spinand_upd_cfg(spinand, CFG_OTP_ENABLE, 0); >> + if (ret) { >> + dev_err(dev, >> + "Failed to updtae the OTP (err = %d)\n", > > update ok > >> + ret); >> + return; >> + } >> + >> + ret = spinand_manufacturer_init(spinand); >> + if (ret) { >> + dev_err(dev, >> + "Failed to initialize the SPI NAND chip (err = %d)\n", >> + ret); >> + return; >> + } >> + >> + /* After power up, all blocks are locked, so unlock them here. */ >> + for (i = 0; i < nand->memorg.ntargets; i++) { >> + ret = spinand_select_target(spinand, i); >> + if (ret) { >> + dev_err(dev, >> + "Failed to select the target (err = %d)\n", >> + ret); >> + return; >> + } >> + >> + ret = spinand_lock_block(spinand, BL_ALL_UNLOCKED); >> + if (ret) { >> + dev_err(dev, >> + "Failed to unlock block (err = %d)\n", >> + ret); >> + return; >> + } >> + } > > I bet this would deserve a helper as this is the exact same peace of > code that is being run in spinnand_init()? I will add a new function spinand_block_unlock(struct spinand_device *spinand) > > At the very least I think that spinand_ecc_enable(spinand, false); > should be called. I will add it. > > Ideally, a resume operation should be provided by ECC engines, but that > can be added later. > >> +} >> + >> static int spinand_init(struct spinand_device *spinand) >> { >> struct device *dev = &spinand->spimem->spi->dev; >> @@ -1167,6 +1222,7 @@ static int spinand_init(struct spinand_device *spinand) >> mtd->_block_isreserved = spinand_mtd_block_isreserved; >> mtd->_erase = spinand_mtd_erase; >> mtd->_max_bad_blocks = nanddev_mtd_max_bad_blocks; >> + mtd->_resume = spinand_mtd_resume; >> >> if (nand->ecc.engine) { >> ret = mtd_ooblayout_count_freebytes(mtd); > > Thanks, > Miquèl > Thanks Patrice