[PATCH v4 5/9] mtd: spi-nor-core: Add the ->ready() hook

[Takahiro Kuwano <tkuw584924@gmail.com>]

Hi Pratyush,

Thank you for the feedback. I will address this in v5.

On 1/30/2021 3:49 AM, Pratyush Yadav wrote:
> Hi,
> 
> On 28/01/21 01:36PM, tkuw584924 at gmail.com wrote:
>> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
>>
>> For dual/quad die package devices from Spansion/Cypress, the device's
>> status needs to be checked by reading status registers in all dies, by
>> using Read Any Register command. To support this, a Flash specific hook
>> that can overwrite the legacy status check is needed.
>>
>> The spansion_sr_ready() reads status register 1 by Read Any Register
>> commnad. This function is called from Flash specific hook with die address
>> and dummy cycles.
>>
>> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
>> ---
>>  drivers/mtd/spi/spi-nor-core.c | 32 ++++++++++++++++++++++++++++++++
>>  include/linux/mtd/spi-nor.h    |  4 +++-
>>  2 files changed, 35 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
>> index 624e730524..1c0ba5abf9 100644
>> --- a/drivers/mtd/spi/spi-nor-core.c
>> +++ b/drivers/mtd/spi/spi-nor-core.c
>> @@ -522,6 +522,35 @@ static int set_4byte(struct spi_nor *nor, const struct flash_info *info,
>>  	}
>>  }
>>  
>> +#ifdef CONFIG_SPI_FLASH_SPANSION
>> +/*
>> + * Read status register 1 by using Read Any Register command to support multi
>> + * die package parts.
>> + */
>> +static int spansion_sr_ready(struct spi_nor *nor, u32 addr_base, u8 dummy)
>> +{
>> +	u32 reg_addr = addr_base + SPINOR_REG_ADDR_STR1V;
>> +	u8 sr;
>> +	int ret;
>> +
>> +	ret = spansion_read_any_reg(nor, reg_addr, dummy, &sr);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	if (sr & (SR_E_ERR | SR_P_ERR)) {
>> +		if (sr & SR_E_ERR)
>> +			dev_dbg(nor->dev, "Erase Error occurred\n");
>> +		else
>> +			dev_dbg(nor->dev, "Programming Error occurred\n");
>> +
>> +		nor->write_reg(nor, SPINOR_OP_CLSR, NULL, 0);
>> +		return -EIO;
>> +	}
>> +
>> +	return !(sr & SR_WIP);
>> +}
>> +#endif
>> +
> 
> Do not add this function in this patch. Just add the hook here. Add it 
> in the patch that actually uses it.
> 
>>  static int spi_nor_sr_ready(struct spi_nor *nor)
>>  {
>>  	int sr = read_sr(nor);
>> @@ -570,6 +599,9 @@ static int spi_nor_ready(struct spi_nor *nor)
>>  {
>>  	int sr, fsr;
>>  
>> +	if (nor->ready)
>> +		return nor->ready(nor);
>> +
> 
> Move the code below in a separate function and call it something like 
> spi_nor_default_ready(). Then call that function from here.
> 
>>  	sr = spi_nor_sr_ready(nor);
>>  	if (sr < 0)
>>  		return sr;
>> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
>> index e31073eb24..25234177de 100644
>> --- a/include/linux/mtd/spi-nor.h
>> +++ b/include/linux/mtd/spi-nor.h
>> @@ -434,8 +434,9 @@ struct flash_info;
>>   * @flash_lock:		[FLASH-SPECIFIC] lock a region of the SPI NOR
>>   * @flash_unlock:	[FLASH-SPECIFIC] unlock a region of the SPI NOR
>>   * @flash_is_locked:	[FLASH-SPECIFIC] check if a region of the SPI NOR is
>> - * @quad_enable:	[FLASH-SPECIFIC] enables SPI NOR quad mode
>>   *			completely locked
>> + * @quad_enable:	[FLASH-SPECIFIC] enables SPI NOR quad mode
>> + * @ready:		[FLASH-SPECIFIC] check if the flash is ready
>>   * @priv:		the private data
>>   */
>>  struct spi_nor {
>> @@ -481,6 +482,7 @@ struct spi_nor {
>>  	int (*flash_unlock)(struct spi_nor *nor, loff_t ofs, uint64_t len);
>>  	int (*flash_is_locked)(struct spi_nor *nor, loff_t ofs, uint64_t len);
>>  	int (*quad_enable)(struct spi_nor *nor);
>> +	int (*ready)(struct spi_nor *nor);
>>  
>>  	void *priv;
>>  /* Compatibility for spi_flash, remove once sf layer is merged with mtd */
>> -- 
>> 2.25.1
>>
> 

Best Regards,
Takahiro