[PATCH v7 2/3] mtd: spi-nor: spansion: Add support for volatile QE bit

[Takahiro Kuwano]

On 1/29/2022 1:38 AM, Pratyush Yadav wrote:
> On 28/01/22 10:12AM, Tudor.Ambarus at microchip.com wrote:
>> On 7/19/21 11:03, tkuw584924 at gmail.com wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> From: Takahiro Kuwano <Takahiro.Kuwano at infineon.com>
>>>
>>> Some of Spansion/Cypress chips support volatile version of configuration
>>> registers and it is recommended to update volatile registers in the field
>>> application due to a risk of the non-volatile registers corruption by
>>> power interrupt.
>>>
>>> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano at infineon.com>
>>> ---
>>> Changes in v7:
>>>   - Add missing macro definitions in v6
>>>
>>> Changes in v6:
>>>   - Remove multi die package support
>>>
>>> Changes in v5:
>>>   - No change
>>>
>>> Changes in v4:
>>>   - No change
>>>
>>> Changes in v3:
>>>   - Add multi-die package parts support
>>>
>>>  drivers/mtd/spi-nor/spansion.c | 59 ++++++++++++++++++++++++++++++++++
>>>  1 file changed, 59 insertions(+)
>>>
>>> diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
>>> index 5e7e62704e11..ca50e77b4220 100644
>>> --- a/drivers/mtd/spi-nor/spansion.c
>>> +++ b/drivers/mtd/spi-nor/spansion.c
>>> @@ -10,6 +10,8 @@
>>>
>>>  #define SPINOR_OP_RD_ANY_REG                   0x65    /* Read any register */
>>>  #define SPINOR_OP_WR_ANY_REG                   0x71    /* Write any register */
>>> +#define SPINOR_REG_CYPRESS_CFR1V               0x00800002
>>> +#define SPINOR_REG_CYPRESS_CFR1V_QUAD_EN       BIT(1)  /* Quad Enable */
> 
> I have the Cypress S28 flash and this bit is marked as "reserved" there. 
> Maybe we should have a flash family specific name? Dunno. But I guess it 
> does not matter so much since it will be very localised anyway.
> 
>>>  #define SPINOR_REG_CYPRESS_CFR2V               0x00800003
>>>  #define SPINOR_REG_CYPRESS_CFR2V_MEMLAT_11_24  0xb
>>>  #define SPINOR_REG_CYPRESS_CFR3V               0x00800004
>>> @@ -161,6 +163,63 @@ static int spansion_write_any_reg(struct spi_nor *nor, u32 reg_addr,
>>>                                             reg_val);
>>>  }
>>>
>>> +/**
>>> + * spansion_quad_enable_volatile() - enable Quad I/O mode in volatile register.
>>> + * @nor:       pointer to a 'struct spi_nor'
>>> + * @reg_dummy: number of dummy cycles for register read
>>> + *
>>> + * It is recommended to update volatile registers in the field application due
>>> + * to a risk of the non-volatile registers corruption by power interrupt. This
>>> + * function sets Quad Enable bit in CFR1 volatile. If users set the Quad Enable
>>> + * bit in the CFR1 non-volatile in advance (typically by a Flash programmer
>>> + * before mounting Flash on PCB), the Quad Enable bit in the CFR1 volatile is
>>> + * also set during Flash power-up. This function supports multi-die package
>>
>> Why do you duplicate the setting of QE bit in CFR1V if QE bit is already set in
>> CFR1NV? Does the Volatile Register has a higher priority than the Non-Volatile one?
>> What happens when QE is zero in CFR1NV and QE is one in CFR1V?
> 
> I have the Cypress S28 flash family, and on there I see that the 
> volatile bit takes precedence over non-volatile bit*. At powerup data in 
> NV bits is transferred to V bits to set their default state.
> 
> What I am more worried about is if quad mode is enabled in a 
> non-volatile register (which should NEVER be done by Linux), will we be 
> able to properly detect and initialize the flash? We certainly can't do 
> it for 8D-8D-8D mode flashes.
> 
If quad mode is enabled, 1S-1S-4S/1S-4S-4S/1S-4D-4D ops are supported.
The 1S-1S-1S ops like read ID and registers used for initializationare still available.

Thanks,
Takahiro