| Message ID | 20210527161252.16620-4-patrice.chotard@foss.st.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3,1/3] mtd: spinand: Add spinand_block_unlock() helper | expand |
Hi Patrice, + Pratyush <patrice.chotard@foss.st.com> wrote on Thu, 27 May 2021 18:12:52 +0200: > From: Patrice Chotard <patrice.chotard@foss.st.com> > > 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. > SPI NAND resume op re-initializes SPI NAND flash to its probed state. > > Signed-off-by: Christophe Kerello <christophe.kerello@foss.st.com> > Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com> > --- > Changes in v3: > - Add spinand_read_cfg() call to repopulate cache > > Changes in v2: > - Add helper spinand_block_unlock(). > - Add spinand_ecc_enable() call. > - Remove some dev_err(). > - Fix commit's title and message. > > drivers/mtd/nand/spi/core.c | 33 +++++++++++++++++++++++++++++++++ > 1 file changed, 33 insertions(+) > > diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c > index 1f699ad84f1b..e3fcbcf381c3 100644 > --- a/drivers/mtd/nand/spi/core.c > +++ b/drivers/mtd/nand/spi/core.c > @@ -1099,6 +1099,38 @@ static int spinand_block_unlock(struct spinand_device *spinand) > return ret; > } > > +static void spinand_mtd_resume(struct mtd_info *mtd) > +{ > + struct spinand_device *spinand = mtd_to_spinand(mtd); > + int ret; > + > + ret = spinand_reset_op(spinand); > + if (ret) > + return; > + > + ret = spinand_read_cfg(spinand); > + if (ret) > + return; > + > + ret = spinand_init_quad_enable(spinand); > + if (ret) > + return; > + > + ret = spinand_upd_cfg(spinand, CFG_OTP_ENABLE, 0); > + if (ret) > + return; > + > + ret = spinand_manufacturer_init(spinand); > + if (ret) > + return; > + > + ret = spinand_block_unlock(spinand); > + if (ret) > + return; > + > + spinand_ecc_enable(spinand, false); > +} Sorry for not being clear, but I think what Pratyush meant was that you could create a helper doing all the common initializations between spinand_init() and spinand_resume() and call it from these places to avoid code duplication. His comment somehow outclassed mine as I only focused on the unlock part (which I think is clearer anyway, please keep it like that). > + > static int spinand_init(struct spinand_device *spinand) > { > struct device *dev = &spinand->spimem->spi->dev; > @@ -1186,6 +1218,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
Hi Miquel On 5/27/21 6:23 PM, Miquel Raynal wrote: > Hi Patrice, > > + Pratyush > > <patrice.chotard@foss.st.com> wrote on Thu, 27 May 2021 18:12:52 +0200: > >> From: Patrice Chotard <patrice.chotard@foss.st.com> >> >> 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. >> SPI NAND resume op re-initializes SPI NAND flash to its probed state. >> >> Signed-off-by: Christophe Kerello <christophe.kerello@foss.st.com> >> Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com> >> --- >> Changes in v3: >> - Add spinand_read_cfg() call to repopulate cache >> >> Changes in v2: >> - Add helper spinand_block_unlock(). >> - Add spinand_ecc_enable() call. >> - Remove some dev_err(). >> - Fix commit's title and message. >> >> drivers/mtd/nand/spi/core.c | 33 +++++++++++++++++++++++++++++++++ >> 1 file changed, 33 insertions(+) >> >> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c >> index 1f699ad84f1b..e3fcbcf381c3 100644 >> --- a/drivers/mtd/nand/spi/core.c >> +++ b/drivers/mtd/nand/spi/core.c >> @@ -1099,6 +1099,38 @@ static int spinand_block_unlock(struct spinand_device *spinand) >> return ret; >> } >> >> +static void spinand_mtd_resume(struct mtd_info *mtd) >> +{ >> + struct spinand_device *spinand = mtd_to_spinand(mtd); >> + int ret; >> + >> + ret = spinand_reset_op(spinand); >> + if (ret) >> + return; >> + >> + ret = spinand_read_cfg(spinand); >> + if (ret) >> + return; >> + >> + ret = spinand_init_quad_enable(spinand); >> + if (ret) >> + return; >> + >> + ret = spinand_upd_cfg(spinand, CFG_OTP_ENABLE, 0); >> + if (ret) >> + return; >> + >> + ret = spinand_manufacturer_init(spinand); >> + if (ret) >> + return; >> + >> + ret = spinand_block_unlock(spinand); >> + if (ret) >> + return; >> + >> + spinand_ecc_enable(spinand, false); >> +} > > Sorry for not being clear, but I think what Pratyush meant was that > you could create a helper doing all the common initializations between > spinand_init() and spinand_resume() and call it from these places to > avoid code duplication. His comment somehow outclassed mine as I only > focused on the unlock part (which I think is clearer anyway, please keep > it like that). > My bad too, i read too quickly Pratyush's asnwer. I am preparing a v4. Thanks Patrice >> + >> static int spinand_init(struct spinand_device *spinand) >> { >> struct device *dev = &spinand->spimem->spi->dev; >> @@ -1186,6 +1218,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 >
On 27/05/21 06:12PM, patrice.chotard@foss.st.com wrote: > From: Patrice Chotard <patrice.chotard@foss.st.com> > > 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. > SPI NAND resume op re-initializes SPI NAND flash to its probed state. > > Signed-off-by: Christophe Kerello <christophe.kerello@foss.st.com> > Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com> > --- > Changes in v3: > - Add spinand_read_cfg() call to repopulate cache > > Changes in v2: > - Add helper spinand_block_unlock(). > - Add spinand_ecc_enable() call. > - Remove some dev_err(). > - Fix commit's title and message. > > drivers/mtd/nand/spi/core.c | 33 +++++++++++++++++++++++++++++++++ > 1 file changed, 33 insertions(+) > > diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c > index 1f699ad84f1b..e3fcbcf381c3 100644 > --- a/drivers/mtd/nand/spi/core.c > +++ b/drivers/mtd/nand/spi/core.c > @@ -1099,6 +1099,38 @@ static int spinand_block_unlock(struct spinand_device *spinand) > return ret; > } > > +static void spinand_mtd_resume(struct mtd_info *mtd) > +{ > + struct spinand_device *spinand = mtd_to_spinand(mtd); > + int ret; > + > + ret = spinand_reset_op(spinand); > + if (ret) > + return; > + > + ret = spinand_read_cfg(spinand); > + if (ret) > + return; > + > + ret = spinand_init_quad_enable(spinand); > + if (ret) > + return; > + > + ret = spinand_upd_cfg(spinand, CFG_OTP_ENABLE, 0); > + if (ret) > + return; > + > + ret = spinand_manufacturer_init(spinand); > + if (ret) > + return; > + > + ret = spinand_block_unlock(spinand); > + if (ret) > + return; > + > + spinand_ecc_enable(spinand, false); > +} > + I don't think you quite get what me and Miquel are suggesting. The helper should call all these functions like read_cfg() quad_enable(), etc. So it should look something like: int spinand_init_flash() { ret = spinand_read_cfg(spinand); if (ret) return; ret = spinand_init_quad_enable(spinand); if (ret) return; ret = spinand_upd_cfg(spinand, CFG_OTP_ENABLE, 0); if (ret) return; ret = spinand_manufacturer_init(spinand); if (ret) return; ret = spinand_block_unlock(spinand); if (ret) return; spinand_ecc_enable(spinand, false); } Then spinand_mtd_resume should look something like: int spinand_mtd_resume() { ret = spinand_reset_op(spinand); if (ret) return; return spinand_init_flash(); } And spinand_init() should look something like: int spinand_init() { ... spinand->oobbuf = ... spinand_init_flash(); spinand_create_dirmaps(); ... } > static int spinand_init(struct spinand_device *spinand) > { > struct device *dev = &spinand->spimem->spi->dev; > @@ -1186,6 +1218,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); > -- > 2.17.1 -- Regards, Pratyush Yadav Texas Instruments Inc.
Hi Pratyush On 5/28/21 2:05 PM, Pratyush Yadav wrote: > On 27/05/21 06:12PM, patrice.chotard@foss.st.com wrote: >> From: Patrice Chotard <patrice.chotard@foss.st.com> >> >> 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. >> SPI NAND resume op re-initializes SPI NAND flash to its probed state. >> >> Signed-off-by: Christophe Kerello <christophe.kerello@foss.st.com> >> Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com> >> --- >> Changes in v3: >> - Add spinand_read_cfg() call to repopulate cache >> >> Changes in v2: >> - Add helper spinand_block_unlock(). >> - Add spinand_ecc_enable() call. >> - Remove some dev_err(). >> - Fix commit's title and message. >> >> drivers/mtd/nand/spi/core.c | 33 +++++++++++++++++++++++++++++++++ >> 1 file changed, 33 insertions(+) >> >> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c >> index 1f699ad84f1b..e3fcbcf381c3 100644 >> --- a/drivers/mtd/nand/spi/core.c >> +++ b/drivers/mtd/nand/spi/core.c >> @@ -1099,6 +1099,38 @@ static int spinand_block_unlock(struct spinand_device *spinand) >> return ret; >> } >> >> +static void spinand_mtd_resume(struct mtd_info *mtd) >> +{ >> + struct spinand_device *spinand = mtd_to_spinand(mtd); >> + int ret; >> + >> + ret = spinand_reset_op(spinand); >> + if (ret) >> + return; >> + >> + ret = spinand_read_cfg(spinand); >> + if (ret) >> + return; >> + >> + ret = spinand_init_quad_enable(spinand); >> + if (ret) >> + return; >> + >> + ret = spinand_upd_cfg(spinand, CFG_OTP_ENABLE, 0); >> + if (ret) >> + return; >> + >> + ret = spinand_manufacturer_init(spinand); >> + if (ret) >> + return; >> + >> + ret = spinand_block_unlock(spinand); >> + if (ret) >> + return; >> + >> + spinand_ecc_enable(spinand, false); >> +} >> + > > I don't think you quite get what me and Miquel are suggesting. > > The helper should call all these functions like read_cfg() > quad_enable(), etc. So it should look something like: Yes, this series was sent too quickly on my side, and i misunderstood what you suggested, sorry for that. > > int spinand_init_flash() > { > ret = spinand_read_cfg(spinand); > if (ret) > return; > The new helper spinand_read_cfg() must not be called in spinand_init_flash() but directly in spinand_resume(). This because spinand_read_cfg() only performs a REG_CFG read without doing the memory allocation of spinand->cfg_cache. In spinand_init(), spinand_init_cfg_cache() must be called as previously as it does spinand->cfg_cache memory allocation and call the new helper spinand_read_cfg(). Then after, spinand_init_flash() can be called. > ret = spinand_init_quad_enable(spinand); > if (ret) > return; > > ret = spinand_upd_cfg(spinand, CFG_OTP_ENABLE, 0); > if (ret) > return; > > ret = spinand_manufacturer_init(spinand); > if (ret) > return; > > ret = spinand_block_unlock(spinand); > if (ret) > return; > > spinand_ecc_enable(spinand, false); > } > > Then spinand_mtd_resume should look something like: > > int spinand_mtd_resume() > { > ret = spinand_reset_op(spinand); > if (ret) > return; > > return spinand_init_flash(); > } > > And spinand_init() should look something like: > > int spinand_init() > { > ... > spinand->oobbuf = ... > > spinand_init_flash(); > > spinand_create_dirmaps(); > > ... As explained just above, spinand_init() will look like : int spinand_init() { ... spinand->oobbuf = ... spinand_init_cfg_cache(); => perform cfg cache memory allocation and read the REG_CFG spinand_init_flash(); spinand_create_dirmaps(); > } > > >> static int spinand_init(struct spinand_device *spinand) >> { >> struct device *dev = &spinand->spimem->spi->dev; >> @@ -1186,6 +1218,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); >> -- >> 2.17.1 > Thanks Patrice
On 28/05/21 02:51PM, Patrice CHOTARD wrote: > Hi Pratyush > > On 5/28/21 2:05 PM, Pratyush Yadav wrote: > > On 27/05/21 06:12PM, patrice.chotard@foss.st.com wrote: > >> From: Patrice Chotard <patrice.chotard@foss.st.com> > >> > >> 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. > >> SPI NAND resume op re-initializes SPI NAND flash to its probed state. > >> > >> Signed-off-by: Christophe Kerello <christophe.kerello@foss.st.com> > >> Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com> > >> --- > >> Changes in v3: > >> - Add spinand_read_cfg() call to repopulate cache > >> > >> Changes in v2: > >> - Add helper spinand_block_unlock(). > >> - Add spinand_ecc_enable() call. > >> - Remove some dev_err(). > >> - Fix commit's title and message. > >> > >> drivers/mtd/nand/spi/core.c | 33 +++++++++++++++++++++++++++++++++ > >> 1 file changed, 33 insertions(+) > >> > >> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c > >> index 1f699ad84f1b..e3fcbcf381c3 100644 > >> --- a/drivers/mtd/nand/spi/core.c > >> +++ b/drivers/mtd/nand/spi/core.c > >> @@ -1099,6 +1099,38 @@ static int spinand_block_unlock(struct spinand_device *spinand) > >> return ret; > >> } > >> > >> +static void spinand_mtd_resume(struct mtd_info *mtd) > >> +{ > >> + struct spinand_device *spinand = mtd_to_spinand(mtd); > >> + int ret; > >> + > >> + ret = spinand_reset_op(spinand); > >> + if (ret) > >> + return; > >> + > >> + ret = spinand_read_cfg(spinand); > >> + if (ret) > >> + return; > >> + > >> + ret = spinand_init_quad_enable(spinand); > >> + if (ret) > >> + return; > >> + > >> + ret = spinand_upd_cfg(spinand, CFG_OTP_ENABLE, 0); > >> + if (ret) > >> + return; > >> + > >> + ret = spinand_manufacturer_init(spinand); > >> + if (ret) > >> + return; > >> + > >> + ret = spinand_block_unlock(spinand); > >> + if (ret) > >> + return; > >> + > >> + spinand_ecc_enable(spinand, false); > >> +} > >> + > > > > I don't think you quite get what me and Miquel are suggesting. > > > > The helper should call all these functions like read_cfg() > > quad_enable(), etc. So it should look something like: > > Yes, this series was sent too quickly on my side, and i misunderstood > what you suggested, sorry for that. Ah, I thought this was the v4. I see Miquel clarified already in reply to this patch. > > > > > int spinand_init_flash() > > { > > ret = spinand_read_cfg(spinand); > > if (ret) > > return; > > > > The new helper spinand_read_cfg() must not be called in spinand_init_flash() > but directly in spinand_resume(). > This because spinand_read_cfg() only performs a REG_CFG read without doing the > memory allocation of spinand->cfg_cache. > > In spinand_init(), spinand_init_cfg_cache() must be called as previously as it does spinand->cfg_cache > memory allocation and call the new helper spinand_read_cfg(). > Then after, spinand_init_flash() can be called. I think it would be simpler for spinand_init_cfg_cache() to just allocate the spinand->cfg_cache array and then for spinand_init_flash() to call spinand_read_cfg(). > > > ret = spinand_init_quad_enable(spinand); > > if (ret) > > return; > > > > ret = spinand_upd_cfg(spinand, CFG_OTP_ENABLE, 0); > > if (ret) > > return; > > > > ret = spinand_manufacturer_init(spinand); > > if (ret) > > return; > > > > ret = spinand_block_unlock(spinand); > > if (ret) > > return; > > > > spinand_ecc_enable(spinand, false); > > } > > > > Then spinand_mtd_resume should look something like: > > > > int spinand_mtd_resume() > > { > > ret = spinand_reset_op(spinand); > > if (ret) > > return; > > > > return spinand_init_flash(); > > } > > > > And spinand_init() should look something like: > > > > int spinand_init() > > { > > ... > > spinand->oobbuf = ... > > > > spinand_init_flash(); > > > > spinand_create_dirmaps(); > > > > ... > > As explained just above, spinand_init() will look like : > > int spinand_init() > { > ... > spinand->oobbuf = ... > > spinand_init_cfg_cache(); => perform cfg cache memory allocation and read the REG_CFG > > spinand_init_flash(); > > spinand_create_dirmaps(); > > > } > > > > > >> static int spinand_init(struct spinand_device *spinand) > >> { > >> struct device *dev = &spinand->spimem->spi->dev; > >> @@ -1186,6 +1218,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); > >> -- > >> 2.17.1 > > > > Thanks > Patrice -- Regards, Pratyush Yadav Texas Instruments Inc.
On 5/28/21 3:17 PM, Pratyush Yadav wrote: > On 28/05/21 02:51PM, Patrice CHOTARD wrote: >> Hi Pratyush >> >> On 5/28/21 2:05 PM, Pratyush Yadav wrote: >>> On 27/05/21 06:12PM, patrice.chotard@foss.st.com wrote: >>>> From: Patrice Chotard <patrice.chotard@foss.st.com> >>>> >>>> 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. >>>> SPI NAND resume op re-initializes SPI NAND flash to its probed state. >>>> >>>> Signed-off-by: Christophe Kerello <christophe.kerello@foss.st.com> >>>> Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com> >>>> --- >>>> Changes in v3: >>>> - Add spinand_read_cfg() call to repopulate cache >>>> >>>> Changes in v2: >>>> - Add helper spinand_block_unlock(). >>>> - Add spinand_ecc_enable() call. >>>> - Remove some dev_err(). >>>> - Fix commit's title and message. >>>> >>>> drivers/mtd/nand/spi/core.c | 33 +++++++++++++++++++++++++++++++++ >>>> 1 file changed, 33 insertions(+) >>>> >>>> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c >>>> index 1f699ad84f1b..e3fcbcf381c3 100644 >>>> --- a/drivers/mtd/nand/spi/core.c >>>> +++ b/drivers/mtd/nand/spi/core.c >>>> @@ -1099,6 +1099,38 @@ static int spinand_block_unlock(struct spinand_device *spinand) >>>> return ret; >>>> } >>>> >>>> +static void spinand_mtd_resume(struct mtd_info *mtd) >>>> +{ >>>> + struct spinand_device *spinand = mtd_to_spinand(mtd); >>>> + int ret; >>>> + >>>> + ret = spinand_reset_op(spinand); >>>> + if (ret) >>>> + return; >>>> + >>>> + ret = spinand_read_cfg(spinand); >>>> + if (ret) >>>> + return; >>>> + >>>> + ret = spinand_init_quad_enable(spinand); >>>> + if (ret) >>>> + return; >>>> + >>>> + ret = spinand_upd_cfg(spinand, CFG_OTP_ENABLE, 0); >>>> + if (ret) >>>> + return; >>>> + >>>> + ret = spinand_manufacturer_init(spinand); >>>> + if (ret) >>>> + return; >>>> + >>>> + ret = spinand_block_unlock(spinand); >>>> + if (ret) >>>> + return; >>>> + >>>> + spinand_ecc_enable(spinand, false); >>>> +} >>>> + >>> >>> I don't think you quite get what me and Miquel are suggesting. >>> >>> The helper should call all these functions like read_cfg() >>> quad_enable(), etc. So it should look something like: >> >> Yes, this series was sent too quickly on my side, and i misunderstood >> what you suggested, sorry for that. > > Ah, I thought this was the v4. I see Miquel clarified already in reply > to this patch. > >> >>> >>> int spinand_init_flash() >>> { >>> ret = spinand_read_cfg(spinand); >>> if (ret) >>> return; >>> >> >> The new helper spinand_read_cfg() must not be called in spinand_init_flash() >> but directly in spinand_resume(). >> This because spinand_read_cfg() only performs a REG_CFG read without doing the >> memory allocation of spinand->cfg_cache. >> >> In spinand_init(), spinand_init_cfg_cache() must be called as previously as it does spinand->cfg_cache >> memory allocation and call the new helper spinand_read_cfg(). >> Then after, spinand_init_flash() can be called. > > I think it would be simpler for spinand_init_cfg_cache() to just > allocate the spinand->cfg_cache array and then for spinand_init_flash() > to call spinand_read_cfg(). Argh, i just send the v5 ;-) .... i will wait the full v5 review and will follow your proposal regarding spinand_init_cfg_cache(). Patrice > >> >>> ret = spinand_init_quad_enable(spinand); >>> if (ret) >>> return; >>> >>> ret = spinand_upd_cfg(spinand, CFG_OTP_ENABLE, 0); >>> if (ret) >>> return; >>> >>> ret = spinand_manufacturer_init(spinand); >>> if (ret) >>> return; >>> >>> ret = spinand_block_unlock(spinand); >>> if (ret) >>> return; >>> >>> spinand_ecc_enable(spinand, false); >>> } >>> >>> Then spinand_mtd_resume should look something like: >>> >>> int spinand_mtd_resume() >>> { >>> ret = spinand_reset_op(spinand); >>> if (ret) >>> return; >>> >>> return spinand_init_flash(); >>> } >>> >>> And spinand_init() should look something like: >>> >>> int spinand_init() >>> { >>> ... >>> spinand->oobbuf = ... >>> >>> spinand_init_flash(); >>> >>> spinand_create_dirmaps(); >>> >>> ... >> >> As explained just above, spinand_init() will look like : >> >> int spinand_init() >> { >> ... >> spinand->oobbuf = ... >> >> spinand_init_cfg_cache(); => perform cfg cache memory allocation and read the REG_CFG >> >> spinand_init_flash(); >> >> spinand_create_dirmaps(); >> >>> } >>> >>> >>>> static int spinand_init(struct spinand_device *spinand) >>>> { >>>> struct device *dev = &spinand->spimem->spi->dev; >>>> @@ -1186,6 +1218,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); >>>> -- >>>> 2.17.1 >>> >> >> Thanks >> Patrice >
diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c index 1f699ad84f1b..e3fcbcf381c3 100644 --- a/drivers/mtd/nand/spi/core.c +++ b/drivers/mtd/nand/spi/core.c @@ -1099,6 +1099,38 @@ static int spinand_block_unlock(struct spinand_device *spinand) return ret; } +static void spinand_mtd_resume(struct mtd_info *mtd) +{ + struct spinand_device *spinand = mtd_to_spinand(mtd); + int ret; + + ret = spinand_reset_op(spinand); + if (ret) + return; + + ret = spinand_read_cfg(spinand); + if (ret) + return; + + ret = spinand_init_quad_enable(spinand); + if (ret) + return; + + ret = spinand_upd_cfg(spinand, CFG_OTP_ENABLE, 0); + if (ret) + return; + + ret = spinand_manufacturer_init(spinand); + if (ret) + return; + + ret = spinand_block_unlock(spinand); + if (ret) + return; + + spinand_ecc_enable(spinand, false); +} + static int spinand_init(struct spinand_device *spinand) { struct device *dev = &spinand->spimem->spi->dev; @@ -1186,6 +1218,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);