[PATCH] mtd: spi-nor: spansion: Use nor->addr_nbytes in octal DTR mode in RD_ANY_REG_OP

[Takahiro Kuwano]

Hi Pratyush,

On 10/16/2024 8:59 PM, Pratyush Yadav wrote:
> On Wed, Oct 16 2024, tkuw584924 at gmail.com wrote:
> 
>> From: Takahiro Kuwano <Takahiro.Kuwano at infineon.com>
>>
>> In octal DTR mode, RD_ANY_REG_OP needs to use 4-byte address regardless
>> of flash's internal address mode. Use nor->addr_nbytes which is set to 4
>> during setup.
> 
> If the flash is in Octal DTR mode, shouldn't addr_mode_nbytes also be 4?
> IIUC addr_mode_nbytes is supposed to track the flash's internal address
> mode. If the flash goes into Octal DTR mode then its internal address
> mode switches to 4, and we should update params->addr_mode_nbytes as
> well. We do that in spi_nor_set_4byte_addr_mode() for example.
> 
The flash's internal address mode is represented by CFR2[7] (ADRBYT) and
addr_mode_nbytes tracks the state of this bit. If the flash goes into Octal
DTR mode, this configuration bit is unchanged and referred after exit from
Octal DTR mode. We need to remember addr_mode_nbytes for MTD suspend/resume
event that exits/enter Octal DTR mode.

> I suppose the best place to do it for Octal DTR would be
> spi_nor_set_octal_dtr().
> 
> Side note: honestly, this whole thing with params->addr_nbytes,
> params->addr_mode_nbytes, and nor->addr_nbytes is quite confusing. I
> hope to find some time to clean it up some day.
> 
Agree. This is because flash's behavior itself is confusing:(

>>
>> Fixes: eff9604390d6 ("mtd: spi-nor: spansion: add octal DTR support in RD_ANY_REG_OP")
>> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano at infineon.com>
>> ---
>>  drivers/mtd/spi-nor/spansion.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
>> index d6c92595f6bc..5a88a6096ca8 100644
>> --- a/drivers/mtd/spi-nor/spansion.c
>> +++ b/drivers/mtd/spi-nor/spansion.c
>> @@ -106,6 +106,7 @@ static int cypress_nor_sr_ready_and_clear_reg(struct spi_nor *nor, u64 addr)
>>  	int ret;
>>  
>>  	if (nor->reg_proto == SNOR_PROTO_8_8_8_DTR) {
>> +		op.addr.nbytes = nor->addr_nbytes;
>>  		op.dummy.nbytes = params->rdsr_dummy;
>>  		op.data.nbytes = 2;
>>  	}
> 

Thanks,
Takahiro