Hi Pratyush,
On 2/2/2021 3:56 AM, Pratyush Yadav wrote:
> Hi Takahiro,
>
> On 28/01/21 01:36PM, tkuw584...@gmail.com wrote:
>> From: Takahiro Kuwano <takahiro.kuw...@infineon.com>
>>
>> Some of Spansion/Cypress chips have overlaid 4KB sectors at top and/or
>> bottom, depending on the device configuration, while U-Boot supports
>> uniform sector layout only. This patch adds an erase hook that emulates
>> uniform sector layout.
>>
>> Signed-off-by: Takahiro Kuwano <takahiro.kuw...@infineon.com>
>> ---
>> drivers/mtd/spi/spi-nor-core.c | 48 ++++++++++++++++++++++++++++++++++
>> 1 file changed, 48 insertions(+)
>>
>> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
>> index 1c0ba5abf9..70da0081b6 100644
>> --- a/drivers/mtd/spi/spi-nor-core.c
>> +++ b/drivers/mtd/spi/spi-nor-core.c
>> @@ -788,6 +788,54 @@ erase_err:
>> return ret;
>> }
>>
>> +#ifdef CONFIG_SPI_FLASH_SPANSION
>> +/*
>> + * Erase for Spansion/Cypress Flash devices that has overlaid 4KB sectors at
>> + * the top and/or bottom.
>> + */
>> +static int spansion_overlaid_erase(struct mtd_info *mtd,
>> + struct erase_info *instr)
>> +{
>> + struct spi_nor *nor = mtd_to_spi_nor(mtd);
>> + struct erase_info instr_4k;
>> + u8 opcode;
>> + u32 erasesize;
>> + int ret;
>> +
>> + /* Perform default erase operation (non-overlaid portion is erased) */
>> + ret = spi_nor_erase(mtd, instr);
>> + if (ret)
>> + return ret;
>> +
>> + /* Backup default erase opcode and size */
>> + opcode = nor->erase_opcode;
>> + erasesize = mtd->erasesize;
>> +
>> + /*
>> + * Erase 4KB sectors. Use the possible max length of 4KB sector region.
>> + * The Flash just ignores the command if the address is not configured
>> + * as 4KB sector and reports ready status immediately.
>> + */
>> + instr_4k.len = SZ_128K;
>> + nor->erase_opcode = SPINOR_OP_BE_4K_4B;
>> + mtd->erasesize = SZ_4K;
>> + if (instr->addr == 0) {
>> + instr_4k.addr = 0;
>> + ret = spi_nor_erase(mtd, &instr_4k);
>> + }
>> + if (!ret && instr->addr + instr->len == mtd->size) {
>> + instr_4k.addr = mtd->size - instr_4k.len;
>> + ret = spi_nor_erase(mtd, &instr_4k);
>> + }
>
> This feels like a hack to me. Does the flash datasheet explicitly say
> that erasing the overlaid area with the "normal" erase opcode is a
> no-op?
>
Sorry, I have noticed the datasheet I mentioned in the cover letter is a
summary version. Here is the link to he full version, but you need
registration to access.
https://community.cypress.com/t5/Semper-Flash-Access-Program/Datasheet-Semper-Flash-with-Quad-SPI/ta-p/260789?attachment-id=19522
So, let me quote the description about erasing overlaid area:
"If a sector erase transaction is applied to a 256 KB sector that is
overlaid by 4 KB sectors, the overlaid 4 KB sectors are not affected
by the erase. Only the visible (non-overlaid) portion of the 128 KB
or 192 KB sector is erased."
And about 4 KB sector erase:
"This transaction is ignored when the device is configured for uniform
sector only (CFR3V[3] = 1). If the Erase 4K B sector transaction is issued
to a non-4 KB sector address, the device will abort the operation and will
not set the ERSERR status fail bit."
> I don't see a big reason to run this hack. You are already in a
> flash-specific erase hook. Why not just directly issue the correct erase
> commands to the sectors? That is, why not issue 4k erase to overlaid
> sectors and normal erase to the rest? Why do you need to emulate uniform
> erase?
>
Thanks for pointing this out. I should probably hook nor->erase() instead
of mtd->_erase(), then issue 4k or normal erase depending on the address.
I will introduce that in v5.
>> +
>> + /* Restore erase opcode and size */
>> + nor->erase_opcode = opcode;
>> + mtd->erasesize = erasesize;
>> +
>> + return ret;
>> +}
>> +#endif
>> +
>> #if defined(CONFIG_SPI_FLASH_STMICRO) || defined(CONFIG_SPI_FLASH_SST)
>> /* Write status register and ensure bits in mask match written values */
>> static int write_sr_and_check(struct spi_nor *nor, u8 status_new, u8 mask)
>> --
>> 2.25.1
>>
>
Best Regards,
Takahiro
