diff mbox series

[v4,06/11] mtd: spi-nor: Parse BFPT to determine the 4-Byte Address Mode methods

Message ID 20230322064033.2370483-7-tudor.ambarus@linaro.org
State Superseded
Delegated to: Ambarus Tudor
Headers show
Series mtd: spi-nor: Address mode discovery (BFPT method & current address mode) | expand

Commit Message

Tudor Ambarus March 22, 2023, 6:40 a.m. UTC
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.

Some regressions may be introduced by this patch, because the
params->set_4byte_addr_mode method that was set either in
spi_nor_init_default_params() or later overwritten in default_init() hooks,
may now be overwritten with a different value based on the BFPT data. If
that's the case, the fix is to introduce a post_bfpt fixup hook where one
should fix the wrong BFPT info.

Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
 drivers/mtd/spi-nor/sfdp.c    | 11 +++++++++++
 drivers/mtd/spi-nor/sfdp.h    | 28 ++++++++++++++++++++++++++++
 drivers/mtd/spi-nor/winbond.c | 11 ++++++++++-
 3 files changed, 49 insertions(+), 1 deletion(-)

Comments

Tudor Ambarus March 29, 2023, 5:22 p.m. UTC | #1
On 3/22/23 06:40, Tudor Ambarus wrote:
> 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.>

I forgot to update the commit message, will respin.

> Some regressions may be introduced by this patch, because the
> params->set_4byte_addr_mode method that was set either in
> spi_nor_init_default_params() or later overwritten in default_init() hooks,
> may now be overwritten with a different value based on the BFPT data. If
> that's the case, the fix is to introduce a post_bfpt fixup hook where one
> should fix the wrong BFPT info.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
> ---
>  drivers/mtd/spi-nor/sfdp.c    | 11 +++++++++++
>  drivers/mtd/spi-nor/sfdp.h    | 28 ++++++++++++++++++++++++++++
>  drivers/mtd/spi-nor/winbond.c | 11 ++++++++++-
>  3 files changed, 49 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> index 298ab5e53a8c..63b2810cf75e 100644
> --- a/drivers/mtd/spi-nor/sfdp.c
> +++ b/drivers/mtd/spi-nor/sfdp.c
> @@ -438,6 +438,7 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
>  	size_t len;
>  	int i, cmd, err;
>  	u32 addr, val;
> +	u32 dword;
>  	u16 half;
>  	u8 erase_mask;
>  
> @@ -607,6 +608,16 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
>  		break;
>  	}
>  
> +	dword = bfpt.dwords[SFDP_DWORD(16)] & BFPT_DWORD16_4B_ADDR_MODE_MASK;
> +	if (sfdp_bits_set(dword, BFPT_DWORD16_4B_ADDR_MODE_BRWR))
> +		params->set_4byte_addr_mode = spi_nor_set_4byte_addr_mode_brwr;
> +	else if (sfdp_bits_set(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 (sfdp_bits_set(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");
> +
>  	/* Soft Reset support. */
>  	if (bfpt.dwords[SFDP_DWORD(16)] & BFPT_DWORD16_SWRST_EN_RST)
>  		nor->flags |= SNOR_F_SOFT_RESET;
> diff --git a/drivers/mtd/spi-nor/sfdp.h b/drivers/mtd/spi-nor/sfdp.h
> index 500659b35655..a4b5fe795f18 100644
> --- a/drivers/mtd/spi-nor/sfdp.h
> +++ b/drivers/mtd/spi-nor/sfdp.h
> @@ -16,6 +16,8 @@
>  /* SFDP DWORDS are indexed from 1 but C arrays are indexed from 0. */
>  #define SFDP_DWORD(i)		((i) - 1)
>  
> +#define sfdp_bits_set(dword, mask)		(((dword) & (mask)) == (mask))

s/sfdp_bits_set/sfdp_mask_check> +
>  /* Basic Flash Parameter Table */
>  
>  /* JESD216 rev D defines a Basic Flash Parameter Table of 20 DWORDs. */
> @@ -89,6 +91,32 @@ struct sfdp_bfpt {
>  #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_SWRST_EN_RST		BIT(12)
>  
>  #define BFPT_DWORD18_CMD_EXT_MASK		GENMASK(30, 29)
> diff --git a/drivers/mtd/spi-nor/winbond.c b/drivers/mtd/spi-nor/winbond.c
> index 9cea241c204b..a1b387accc07 100644
> --- a/drivers/mtd/spi-nor/winbond.c
> +++ b/drivers/mtd/spi-nor/winbond.c
> @@ -218,13 +218,22 @@ static const struct spi_nor_otp_ops winbond_nor_otp_ops = {
>  
>  static void winbond_nor_default_init(struct spi_nor *nor)
>  {
> -	nor->params->set_4byte_addr_mode = winbond_nor_set_4byte_addr_mode;
>  }

I should have got ridden of default_init entirely, it's now an empty
function.
>  
>  static void winbond_nor_late_init(struct spi_nor *nor)
>  {
>  	if (nor->params->otp.org->n_regions)
>  		nor->params->otp.ops = &winbond_nor_otp_ops;
> +
> +	/*
> +	 * Winbond seems to require that the Extended Address Register to be set
> +	 * to zero when exiting the 4-Byte Address Mode, at least for W25Q256FV.
> +	 * This requirement is not described in the JESD216 SFDP standard, thus
> +	 * it is Winbond specific. Since we do not know if other Winbond flashes
> +	 * have the same requirement, play safe and overwrite the method parsed
> +	 * from BFPT, if any.
> +	 */
> +	nor->params->set_4byte_addr_mode = winbond_nor_set_4byte_addr_mode;
>  }
>  
>  static const struct spi_nor_fixups winbond_nor_fixups = {
diff mbox series

Patch

diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index 298ab5e53a8c..63b2810cf75e 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -438,6 +438,7 @@  static int spi_nor_parse_bfpt(struct spi_nor *nor,
 	size_t len;
 	int i, cmd, err;
 	u32 addr, val;
+	u32 dword;
 	u16 half;
 	u8 erase_mask;
 
@@ -607,6 +608,16 @@  static int spi_nor_parse_bfpt(struct spi_nor *nor,
 		break;
 	}
 
+	dword = bfpt.dwords[SFDP_DWORD(16)] & BFPT_DWORD16_4B_ADDR_MODE_MASK;
+	if (sfdp_bits_set(dword, BFPT_DWORD16_4B_ADDR_MODE_BRWR))
+		params->set_4byte_addr_mode = spi_nor_set_4byte_addr_mode_brwr;
+	else if (sfdp_bits_set(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 (sfdp_bits_set(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");
+
 	/* Soft Reset support. */
 	if (bfpt.dwords[SFDP_DWORD(16)] & BFPT_DWORD16_SWRST_EN_RST)
 		nor->flags |= SNOR_F_SOFT_RESET;
diff --git a/drivers/mtd/spi-nor/sfdp.h b/drivers/mtd/spi-nor/sfdp.h
index 500659b35655..a4b5fe795f18 100644
--- a/drivers/mtd/spi-nor/sfdp.h
+++ b/drivers/mtd/spi-nor/sfdp.h
@@ -16,6 +16,8 @@ 
 /* SFDP DWORDS are indexed from 1 but C arrays are indexed from 0. */
 #define SFDP_DWORD(i)		((i) - 1)
 
+#define sfdp_bits_set(dword, mask)		(((dword) & (mask)) == (mask))
+
 /* Basic Flash Parameter Table */
 
 /* JESD216 rev D defines a Basic Flash Parameter Table of 20 DWORDs. */
@@ -89,6 +91,32 @@  struct sfdp_bfpt {
 #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_SWRST_EN_RST		BIT(12)
 
 #define BFPT_DWORD18_CMD_EXT_MASK		GENMASK(30, 29)
diff --git a/drivers/mtd/spi-nor/winbond.c b/drivers/mtd/spi-nor/winbond.c
index 9cea241c204b..a1b387accc07 100644
--- a/drivers/mtd/spi-nor/winbond.c
+++ b/drivers/mtd/spi-nor/winbond.c
@@ -218,13 +218,22 @@  static const struct spi_nor_otp_ops winbond_nor_otp_ops = {
 
 static void winbond_nor_default_init(struct spi_nor *nor)
 {
-	nor->params->set_4byte_addr_mode = winbond_nor_set_4byte_addr_mode;
 }
 
 static void winbond_nor_late_init(struct spi_nor *nor)
 {
 	if (nor->params->otp.org->n_regions)
 		nor->params->otp.ops = &winbond_nor_otp_ops;
+
+	/*
+	 * Winbond seems to require that the Extended Address Register to be set
+	 * to zero when exiting the 4-Byte Address Mode, at least for W25Q256FV.
+	 * This requirement is not described in the JESD216 SFDP standard, thus
+	 * it is Winbond specific. Since we do not know if other Winbond flashes
+	 * have the same requirement, play safe and overwrite the method parsed
+	 * from BFPT, if any.
+	 */
+	nor->params->set_4byte_addr_mode = winbond_nor_set_4byte_addr_mode;
 }
 
 static const struct spi_nor_fixups winbond_nor_fixups = {