[PATCH 2/3] mtd: spi-nor: Implement die erase command logic
Cyrille Pitchen
cyrille.pitchen at atmel.com
Wed Nov 30 09:20:26 PST 2016
Le 30/11/2016 à 17:44, Krzeminski, Marcin (Nokia - PL/Wroclaw) a écrit :
>
>
>> -----Original Message-----
>> From: Cyrille Pitchen [mailto:cyrille.pitchen at atmel.com]
>> Sent: Tuesday, November 29, 2016 5:47 PM
>> To: Krzeminski, Marcin (Nokia - PL/Wroclaw)
>> <marcin.krzeminski at nokia.com>; linux-mtd at lists.infradead.org
>> Cc: rfsw-patches at mlist.nokia.com; dwmw2 at infradead.org;
>> computersforpeace at gmail.com; marek.vasut at gmail.com
>> Subject: Re: [PATCH 2/3] mtd: spi-nor: Implement die erase command logic
>>
>> Hi Marcin,
>>
>> I know you have already answered some of my questions of IRC but I ask
>> them again here on the mailing list so everybody can benefit from your
>> answers.
>>
>> Le 24/10/2016 à 15:03, marcin.krzeminski at nokia.com a écrit :
>>> From: Marcin Krzeminski <marcin.krzeminski at nokia.com>
>>>
>>> This commit implements die erase logic.
>>> Sector at a time procedure is moved to function, then erase algorithm
>>> will try to use die erase cmd if size and address cover one or more
>>> dies.
>>>
>>
>> I'm reading your v2 series but indeed I try to understand the purpose of the
>> series simply because I don't know what does die erase do as compared to
>> chip erase of sector erase.
>> Is you series a bug fix or an optmization, maybe both?
>
> In current spi-nor framework when user want to erase whole flash
> (eg. mtd_debug erase /dev/mtdX 0 0x8000000) framework issue chip erase cmd.
> N25Q00 does not support chip erase, instead there was introduced
> die erase command that can erase whole die (N25Q00 is a mulit-die NOR chip).
> So for this case this patch is a bug fix.
> Since I have die erase command implemented it was tempting to use die erase
> also in case where erase area cover one or more dies to speed up erase process,
> especially when user has enabled 4KiB sector. That is a small optimization.
>
> I will rewrite commit message and add those informations.
>
> Thanks,
> Marcin
>
OK, then I think your patch should be split into two different patches.
1 - bug fix:
The first patch introducing an explicit flag such as SPI_NOR_NO_CHIP_ERASE,
which would be set in the relevant entries of the spi_nor_ids[] (see already
existing flags such as SPI_NOR_NO_FR, SPI_NOR_NO_ERASE, ...).
In spi_nor_scan():
if (info->flags & USE_FSR)
nor->flags |= SNOR_F_USE_FSR;
if (info->flags & SPI_NOR_HAS_TB)
nor->flags |= SNOR_F_HAS_SR_TB;
+ if (info->flags & SPI_NOR_NO_CHIP_ERASE)
+ nor->flags |= SNOR_F_NO_CHIP_ERASE;
Then in spi_nor_erase():
/* whole-chip erase? */
- if (len == mtd->size) {
+ if (len == mtd->size && !(nor->flags & SNOR_F_NO_CHIP_ERASE)) {
unsigned long timeout;
So you fall into the sector erase case, no optimization yet.
2 - optimization of spi_nor_erase()
In the "sector"-at-a-time erase branch, try to optimize when possible using
die erase instead of sector erase.
Each commit message describing the purpose of the patch.
I would like to also handle the cases where the memory supports:
- both chip and die erases: if so, no reason not to use chip erase on such
multi-die memory.
- neither chip nor die erases.
Maybe I'm wrong but the assumption (multi-die => chip erase not supported)
might be false.
Best regards,
Cyrille
>>
>> Best regards,
>>
>> Cyrille
>>
>>> Signed-off-by: Marcin Krzeminski <marcin.krzeminski at nokia.com>
>>> ---
>>> drivers/mtd/spi-nor/spi-nor.c | 90
>>> +++++++++++++++++++++++++++++++++++++------
>>> 1 file changed, 78 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/mtd/spi-nor/spi-nor.c
>>> b/drivers/mtd/spi-nor/spi-nor.c index 3afe207..17bbec0 100644
>>> --- a/drivers/mtd/spi-nor/spi-nor.c
>>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>>> @@ -360,6 +360,29 @@ static int spi_nor_erase_sector(struct spi_nor
>> *nor, u32 addr)
>>> return nor->write_reg(nor, nor->erase_opcode, buf, nor-
>>> addr_width);
>>> }
>>>
>>> +static inline int spi_nor_sector_at_time_erase(struct mtd_info *mtd,
>>> +u32 addr, u32 len) {
>>> + struct spi_nor *nor = mtd_to_spi_nor(mtd);
>>> + int ret = 0;
>>> +
>>> + while (len) {
>>> + write_enable(nor);
>>> +
>>> + ret = spi_nor_erase_sector(nor, addr);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + addr += mtd->erasesize;
>>> + len -= mtd->erasesize;
>>> +
>>> + ret = spi_nor_wait_till_ready(nor);
>>> + if (ret)
>>> + return ret;
>>> + }
>>> +
>>> + return ret;
>>> +}
>>> +
>>> /*
>>> * Erase an address range on the nor chip. The address range may extend
>>> * one or more erase sectors. Return an error is there is a problem erasing.
>>> @@ -367,9 +390,11 @@ static int spi_nor_erase_sector(struct spi_nor
>>> *nor, u32 addr) static int spi_nor_erase(struct mtd_info *mtd, struct
>>> erase_info *instr) {
>>> struct spi_nor *nor = mtd_to_spi_nor(mtd);
>>> - u32 addr, len;
>>> + u32 addr, len, die_no, die_size;
>>> uint32_t rem;
>>> int ret;
>>> + unsigned long timeout;
>>> + u8 die_erase = nor->die_cnt && SNOR_F_DIE_ERASE;
>>>
>>> dev_dbg(nor->dev, "at 0x%llx, len %lld\n", (long long)instr->addr,
>>> (long long)instr->len);
>>> @@ -386,7 +411,7 @@ static int spi_nor_erase(struct mtd_info *mtd,
>> struct erase_info *instr)
>>> return ret;
>>>
>>> /* whole-chip erase? */
>>> - if (len == mtd->size) {
>>> + if (len == mtd->size && !die_erase) {
>>> unsigned long timeout;
>>>
>>> write_enable(nor);
>>> @@ -416,17 +441,58 @@ static int spi_nor_erase(struct mtd_info *mtd,
>>> struct erase_info *instr)
>>>
>>> /* "sector"-at-a-time erase */
>>> } else {
>>> - while (len) {
>>> - write_enable(nor);
>>> -
>>> - ret = spi_nor_erase_sector(nor, addr);
>>> - if (ret)
>>> + if (die_erase) {
>>> + die_size = div_u64_rem(mtd->size, nor->die_cnt,
>> &rem);
>>> + if (rem) {
>>> + ret = -EINVAL;
>>> goto erase_err;
>>> -
>>> - addr += mtd->erasesize;
>>> - len -= mtd->erasesize;
>>> -
>>> - ret = spi_nor_wait_till_ready(nor);
>>> + }
>>> + while (len) {
>>> + die_no = div_u64_rem(addr, die_size,
>> &rem);
>>> +
>>> + /* Check if address is aligned to die begin*/
>>> + if (!rem) {
>>> + /* die erase? */
>>> + if (len >= die_size) {
>>> + ret = spi_nor_die_erase(nor,
>> addr);
>>> + if (ret)
>>> + goto erase_err;
>>> +
>>> + len -= die_size;
>>> + addr += die_size;
>>> +
>>> + timeout =
>> max(CHIP_ERASE_2MB_READY_WAIT_JIFFIES,
>>> +
>> CHIP_ERASE_2MB_READY_WAIT_JIFFIES *
>>> + (unsigned
>> long)(die_size / SZ_2M));
>>> + ret =
>> spi_nor_wait_till_ready_with_timeout(nor, timeout);
>>> + if (ret)
>>> + goto erase_err;
>>> + } else {
>>> + ret =
>> spi_nor_sector_at_time_erase(mtd, addr, len);
>>> + if (ret)
>>> + goto erase_err;
>>> + len = 0;
>>> + }
>>> + } else {
>>> + /* check if end address cover at least
>> one die */
>>> + if (div64_ul(addr + len, die_size) >
>> ++die_no) {
>>> + /* align to next die */
>>> + rem = die_size - rem;
>>> + ret =
>> spi_nor_sector_at_time_erase(mtd, addr, rem);
>>> + if (ret)
>>> + goto erase_err;
>>> + len -= rem;
>>> + addr += rem;
>>> + } else {
>>> + ret =
>> spi_nor_sector_at_time_erase(mtd, addr, len);
>>> + if (ret)
>>> + goto erase_err;
>>> + len = 0;
>>> + }
>>> + }
>>> + }
>>> + } else {
>>> + ret = spi_nor_sector_at_time_erase(mtd, addr, len);
>>> if (ret)
>>> goto erase_err;
>>> }
>>>
>
>
More information about the linux-mtd
mailing list