On 1/6/2023 6:47 PM, Tudor Ambarus wrote:
> Hey, Takahiro,
>
> On 06.01.2023 05:06, tkuw584924@xxxxxxxxx wrote:
>> From: Takahiro Kuwano <Takahiro.Kuwano@xxxxxxxxxxxx>
>>
>> CFR5V[6] is reserved bit and must always be 1.
>
> have you seen some strange behavior?
No, just to follow the datasheet.
>>
>> Fixes: c3266af101f2 ("mtd: spi-nor: spansion: add support for Cypress Semper flash")
>> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@xxxxxxxxxxxx>
>> Cc: stable@xxxxxxxxxxxxxxx
>> ---
>> drivers/mtd/spi-nor/spansion.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
>> index b621cdfd506f..4e094a432d29 100644
>> --- a/drivers/mtd/spi-nor/spansion.c
>> +++ b/drivers/mtd/spi-nor/spansion.c
>> @@ -21,8 +21,8 @@
>> #define SPINOR_REG_CYPRESS_CFR3V 0x00800004
>> #define SPINOR_REG_CYPRESS_CFR3V_PGSZ BIT(4) /* Page size. */
>> #define SPINOR_REG_CYPRESS_CFR5V 0x00800006
>> -#define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_EN 0x3
>> -#define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_DS 0
>> +#define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_EN 0x43
>> +#define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_DS 0x40
>
> No, this looks bad. Instead of overwriting CFR5V with whatever value, we
> should instead first read it and then update only the bit that we're
> interested in. If it happens to write CFR5V before octal enable/disable,
> you'll overwrite the previous set values.
>
I understand read-modify-write is the right thing in general. Actually
CFR5V[7] and CFR5V[5:2] are reserved and must be 0 so I preferred a simple
fix for this particular case.
Thanks,
Takahiro
