[PATCH v4 6/9] mtd: spi-nor-core: Add overlaid sector erase feature

[Pratyush Yadav <p.yadav@ti.com>]

Hi Takahiro,

On 28/01/21 01:36PM, tkuw584924 at gmail.com wrote:
> From: Takahiro Kuwano <Takahiro.Kuwano@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.Kuwano@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?

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?

> +
> +	/* 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
> 

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.