From: Tudor Ambarus <tudor.ambarus@linaro.org> To: michael@walle.cc, pratyush@kernel.org Cc: miquel.raynal@bootlin.com, richard@nod.at, Takahiro.Kuwano@infineon.com, bacem.daassi@infineon.com, linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v4 06/11] mtd: spi-nor: Parse BFPT to determine the 4-Byte Address Mode methods Date: Wed, 29 Mar 2023 18:22:35 +0100 [thread overview] Message-ID: <86d79a51-d7d9-897e-9a4a-590987270e05@linaro.org> (raw) In-Reply-To: <20230322064033.2370483-7-tudor.ambarus@linaro.org> 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 = {
WARNING: multiple messages have this Message-ID (diff)
From: Tudor Ambarus <tudor.ambarus@linaro.org> To: michael@walle.cc, pratyush@kernel.org Cc: miquel.raynal@bootlin.com, richard@nod.at, Takahiro.Kuwano@infineon.com, bacem.daassi@infineon.com, linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v4 06/11] mtd: spi-nor: Parse BFPT to determine the 4-Byte Address Mode methods Date: Wed, 29 Mar 2023 18:22:35 +0100 [thread overview] Message-ID: <86d79a51-d7d9-897e-9a4a-590987270e05@linaro.org> (raw) In-Reply-To: <20230322064033.2370483-7-tudor.ambarus@linaro.org> 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 = { ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
next prev parent reply other threads:[~2023-03-29 17:22 UTC|newest] Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-03-22 6:40 [PATCH v4 00/11] mtd: spi-nor: Address mode discovery (BFPT method & current address mode) Tudor Ambarus 2023-03-22 6:40 ` Tudor Ambarus 2023-03-22 6:40 ` [PATCH v4 01/11] mtd: spi-nor: core: Move generic method to core - micron_st_nor_set_4byte_addr_mode Tudor Ambarus 2023-03-22 6:40 ` Tudor Ambarus 2023-03-22 6:40 ` [PATCH v4 02/11] mtd: spi-nor: core: Update name and description of micron_st_nor_set_4byte_addr_mode Tudor Ambarus 2023-03-22 6:40 ` Tudor Ambarus 2023-03-22 6:40 ` [PATCH v4 03/11] mtd: spi-nor: core: Update name and description of spansion_set_4byte_addr_mode Tudor Ambarus 2023-03-22 6:40 ` Tudor Ambarus 2023-03-22 6:40 ` [PATCH v4 04/11] mtd: spi-nor: core: Update name and description of spi_nor_set_4byte_addr_mode Tudor Ambarus 2023-03-22 6:40 ` Tudor Ambarus 2023-03-22 6:40 ` [PATCH v4 05/11] mtd: spi-nor: core: Make spi_nor_set_4byte_addr_mode_brwr public Tudor Ambarus 2023-03-22 6:40 ` Tudor Ambarus 2023-03-22 6:40 ` [PATCH v4 06/11] mtd: spi-nor: Parse BFPT to determine the 4-Byte Address Mode methods Tudor Ambarus 2023-03-22 6:40 ` Tudor Ambarus 2023-03-29 17:22 ` Tudor Ambarus [this message] 2023-03-29 17:22 ` Tudor Ambarus 2023-03-22 6:40 ` [PATCH v4 07/11] mtd: spi-nor: Favor the BFPT-parsed set_4byte_addr_mode method Tudor Ambarus 2023-03-22 6:40 ` Tudor Ambarus 2023-03-29 17:24 ` Tudor Ambarus 2023-03-29 17:24 ` Tudor Ambarus 2023-03-22 6:40 ` [PATCH v4 08/11] mtd: spi-nor: Stop exporting spi_nor_restore() Tudor Ambarus 2023-03-22 6:40 ` Tudor Ambarus 2023-03-22 6:40 ` [PATCH v4 09/11] mtd: spi-nor: core: Update flash's current address mode when changing address mode Tudor Ambarus 2023-03-22 6:40 ` Tudor Ambarus 2023-03-22 6:40 ` [PATCH v4 10/11] mtd: spi-nor: core: Introduce spi_nor_set_4byte_addr_mode() Tudor Ambarus 2023-03-22 6:40 ` Tudor Ambarus 2023-03-22 6:40 ` [PATCH v4 11/11] mtd: spi-nor: spansion: Determine current address mode Tudor Ambarus 2023-03-22 6:40 ` Tudor Ambarus 2023-03-29 17:28 ` Tudor Ambarus 2023-03-29 17:28 ` Tudor Ambarus
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=86d79a51-d7d9-897e-9a4a-590987270e05@linaro.org \ --to=tudor.ambarus@linaro.org \ --cc=Takahiro.Kuwano@infineon.com \ --cc=bacem.daassi@infineon.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mtd@lists.infradead.org \ --cc=michael@walle.cc \ --cc=miquel.raynal@bootlin.com \ --cc=pratyush@kernel.org \ --cc=richard@nod.at \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.