Re: [PATCH v13 2/4] mtd: spi-nor: spansion: Add support for volatile QE bit

Takahiro Kuwano
2022-04-22T09:05:38Z

On 4/21/2022 8:48 PM, Tudor.Ambarus@microchip.com wrote:
> On 4/21/22 14:36, Tudor.Ambarus@microchip.com wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> On 4/21/22 13:56, Tudor.Ambarus@microchip.com wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> On 4/21/22 13:47, Takahiro Kuwano wrote:
>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>
>>>> On 4/21/2022 7:41 PM, Tudor.Ambarus@microchip.com wrote:
>>>>> On 4/21/22 12:40, tkuw584924@gmail.com wrote:
>>>> [...]
>>>>>> +/**
>>>>>> + * cypress_nor_quad_enable_volatile() - enable Quad I/O mode in volatile
>>>>>> + * register.
>>>>>> + * @nor: pointer to a 'struct spi_nor'
>>>>>> + *
>>>>>> + * 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.
>>>>>> + *
>>>>>> + * Return: 0 on success, -errno otherwise.
>>>>>> + */
>>>>>> +static int cypress_nor_quad_enable_volatile(struct spi_nor *nor)
>>>>>> +{
>>>>>> + struct spi_mem_op op;
>>>>>> + u8 cfr1v_written;
>>>>>> + int ret;
>>>>>> +
>>>>>> + op = (struct spi_mem_op)
>>>>>> + CYPRESS_NOR_RD_ANY_REG_OP(3, SPINOR_REG_CYPRESS_CFR1V,
>>>>> nor->addr_width is 3, isn't it? can we use nor->addr_width instead of 3, please?
>>>>>
>>>> No, at the time this method is called, nor->addr_width is set to 4 by
>>>> spi_nor_set_addr_width().
>>>
>>> I see. Allow me some time to re-read this.
>>
>> Does this help you?
>> https://github.com/ambarus/linux-0day/commit/05f20ab7ee349628f0e2d22a4d3852038a6c8c70
>
Yes, I have confirmed it's working on my hardware setup. Thanks a lot.

Some typos in commit description.
> commit 05f20ab7ee349628f0e2d22a4d3852038a6c8c70
> Author: Tudor Ambarus <tudor.ambarus@microchip.com>
> Date: Thu Apr 21 14:15:42 2022 +0300
>
> mtd: spi-nor: core: Couple the number of address bytes with the address mode
>
> Some of Infineon 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.
> Such a volatile configuration register is used to enable the Quad mode.
> The register write sequence requires the number of bytes of address in
> order to be programmed. As it was before, the nor->addr_width was set to 4
> before calling the volatile Quad enable method. This was incorrect as the
> address mode was still at default (3-byte address), which resulted in
> incorrect register configuration.
> Move the setting of the number of bytes of adress after the the Quad enable
s/adress/address

> method to allow reads or writes to registers that reguire the number of
s/reguire/require

> address bytes to work with the default address mode. Now the number of
> address bytes and the adress mode are tightly coupled, which is a natural
s/adress/address

Thanks again,
Takahiro