[PATCH] mtd: spi-nor: Keep CFR5V[6] as 1 in Octal DTR enable

[Dhruva Gole]

Hi Takahiro,

On 06/01/23 09:04, tkuw584924 at gmail.com wrote:
> From: Takahiro Kuwano <Takahiro.Kuwano at infineon.com>
> 
> CFR5V[6] is reserved bit and must always be 1.
> 
> Fixes: ea9a22f7e79c ("mtd: spi-nor-core: Add support for Cypress Semper flash")
> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano at infineon.com>
> ---
>   include/linux/mtd/spi-nor.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index 30f15452aa68..181eb6710d7e 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -194,7 +194,7 @@
>   #define SPINOR_REG_CYPRESS_CFR3V_PGSZ		BIT(4) /* Page size. */
>   #define SPINOR_REG_CYPRESS_CFR3V_UNISECT	BIT(3) /* Uniform sector mode */
>   #define SPINOR_REG_CYPRESS_CFR5V		0x00800006
> -#define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_EN	0x3
> +#define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_EN	0x43

I second Tudor's review on the linux-mtd mailing list here as well,

(<493d9a10-aaf3-70f6-36c3-9a2cf39f0759 at linaro.org>)

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.

>   #define SPINOR_OP_CYPRESS_RD_FAST		0xee
>   
>   /* Supported SPI protocols */

-- 
Thanks and Regards,
Dhruva Gole