[PATCH 2/4] mtd: spi-nor: Set the 4-Byte Address Mode method based on SFDP data

[Tudor Ambarus]

On 08.08.2024 09:00, tkuw584924 at gmail.com wrote:
> From: Takahiro Kuwano <Takahiro.Kuwano at infineon.com>
> 
> This patch partially ports the Linux commit:
> 4e53ab0c292d ("mtd: spi-nor: Set the 4-Byte Address Mode method based on
>                SFDP data")
> 
> BFPT[DWORD(16)] defines the methods to enter and exit the 4-Byte Address
> Mode. Parse BFPT to determine the method. Will rename the methods with
> generic names in a further patch, to keep things trackable in this one.

aren't the names already generic?
> 
> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano at infineon.com>
> ---
>  drivers/mtd/spi/spi-nor-core.c | 40 +++++++++++++++++++++++++++++++++-
>  1 file changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> index d523c045f4..54fd2869a2 100644
> --- a/drivers/mtd/spi/spi-nor-core.c
> +++ b/drivers/mtd/spi/spi-nor-core.c
> @@ -96,6 +96,7 @@ struct sfdp_header {
>   */
>  #define BFPT_DWORD(i)		((i) - 1)
>  #define BFPT_DWORD_MAX		20
> +#define BFPT_MASK_CHECK(dword, mask)		(((dword) & (mask)) == (mask))

follow linux, what was wrong with SFDP_MASK_CHECK,  why did you rename it?
>  
>  /* The first version of JESB216 defined only 9 DWORDs. */
>  #define BFPT_DWORD_MAX_JESD216			9
> @@ -161,6 +162,32 @@ struct sfdp_header {
>  #define BFPT_DWORD15_QER_SR2_BIT1_NO_RD		(0x4UL << 20)
>  #define BFPT_DWORD15_QER_SR2_BIT1		(0x5UL << 20) /* Spansion */
>  
> +#define BFPT_DWORD16_EN4B_MASK			GENMASK(31, 24)
> +#define BFPT_DWORD16_EN4B_ALWAYS_4B		BIT(30)
> +#define BFPT_DWORD16_EN4B_4B_OPCODES		BIT(29)
> +#define BFPT_DWORD16_EN4B_16BIT_NV_CR		BIT(28)
> +#define BFPT_DWORD16_EN4B_BRWR			BIT(27)
> +#define BFPT_DWORD16_EN4B_WREAR			BIT(26)
> +#define BFPT_DWORD16_EN4B_WREN_EN4B		BIT(25)
> +#define BFPT_DWORD16_EN4B_EN4B			BIT(24)
> +#define BFPT_DWORD16_EX4B_MASK			GENMASK(18, 14)
> +#define BFPT_DWORD16_EX4B_16BIT_NV_CR		BIT(18)
> +#define BFPT_DWORD16_EX4B_BRWR			BIT(17)
> +#define BFPT_DWORD16_EX4B_WREAR			BIT(16)
> +#define BFPT_DWORD16_EX4B_WREN_EX4B		BIT(15)
> +#define BFPT_DWORD16_EX4B_EX4B			BIT(14)
> +#define BFPT_DWORD16_4B_ADDR_MODE_MASK			\
> +	(BFPT_DWORD16_EN4B_MASK | BFPT_DWORD16_EX4B_MASK)
> +#define BFPT_DWORD16_4B_ADDR_MODE_16BIT_NV_CR		\
> +	(BFPT_DWORD16_EN4B_16BIT_NV_CR | BFPT_DWORD16_EX4B_16BIT_NV_CR)
> +#define BFPT_DWORD16_4B_ADDR_MODE_BRWR			\
> +	(BFPT_DWORD16_EN4B_BRWR | BFPT_DWORD16_EX4B_BRWR)
> +#define BFPT_DWORD16_4B_ADDR_MODE_WREAR			\
> +	(BFPT_DWORD16_EN4B_WREAR | BFPT_DWORD16_EX4B_WREAR)
> +#define BFPT_DWORD16_4B_ADDR_MODE_WREN_EN4B_EX4B	\
> +	(BFPT_DWORD16_EN4B_WREN_EN4B | BFPT_DWORD16_EX4B_WREN_EX4B)
> +#define BFPT_DWORD16_4B_ADDR_MODE_EN4B_EX4B		\
> +	(BFPT_DWORD16_EN4B_EN4B | BFPT_DWORD16_EX4B_EX4B)
>  #define BFPT_DWORD16_SOFT_RST			BIT(12)
>  #define BFPT_DWORD16_EX4B_PWRCYC		BIT(21)
>  
> @@ -2372,7 +2399,7 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
>  	struct sfdp_bfpt bfpt;
>  	size_t len;
>  	int i, cmd, err;
> -	u32 addr;
> +	u32 addr, dword;
>  	u16 half;
>  
>  	/* JESD216 Basic Flash Parameter Table length is at least 9 DWORDs. */
> @@ -2510,6 +2537,17 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
>  		break;
>  	}
>  
> +	/* 4-byte address mode entry */
> +	dword = bfpt.dwords[BFPT_DWORD(16)] & BFPT_DWORD16_4B_ADDR_MODE_MASK;
> +	if (BFPT_MASK_CHECK(dword, BFPT_DWORD16_4B_ADDR_MODE_BRWR))
> +		params->set_4byte_addr_mode = spi_nor_set_4byte_addr_mode_brwr;
> +	else if (BFPT_MASK_CHECK(dword, BFPT_DWORD16_4B_ADDR_MODE_WREN_EN4B_EX4B))
> +		params->set_4byte_addr_mode = spi_nor_set_4byte_addr_mode_wren_en4b_ex4b;
> +	else if (BFPT_MASK_CHECK(dword, BFPT_DWORD16_4B_ADDR_MODE_EN4B_EX4B))
> +		params->set_4byte_addr_mode = spi_nor_set_4byte_addr_mode_en4b_ex4b;
> +	else
> +		dev_dbg(nor->dev, "BFPT: 4-Byte Address Mode method is not recognized or not implemented\n");
> +

you need to specify in the commit message whether SFDP parsing can
change the set_4byte_addr_mode set based on MFR and explain what happens.

>  	/* Soft Reset support. */
>  	if (bfpt.dwords[BFPT_DWORD(16)] & BFPT_DWORD16_SOFT_RST)
>  		nor->flags |= SNOR_F_SOFT_RESET;