| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <58b39090-d26f-271f-4832-8fb41c624a9c@microchip.com>
Date: Tue, 22 Nov 2022 10:28:22 +0000
From: <Tudor.Ambarus@...rochip.com>
To: <yaliang.wang@...driver.com>, <pratyush@...nel.org>,
<michael@...le.cc>, <miquel.raynal@...tlin.com>, <richard@....at>,
<vigneshr@...com>, <Takahiro.Kuwano@...ineon.com>,
<tkuw584924@...il.com>
CC: <linux-mtd@...ts.infradead.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mtd: spi-nor: spansion: add clear sr fixup for s25fl-l
family
+ Takahiro
Hi, Takahiro,
Would you please review/test this spansion patch?
Thanks,
ta
On 10/18/22 12:57, yaliang.wang@...driver.com wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> From: Yaliang Wang <Yaliang.Wang@...driver.com>
>
> Spansion S25FL-L family flashes s25fl064l/s25fl128l/s25fl256l can't
> automatically recover from programming/erase errors, the Status Register
> error bits inflecting the errors will not change until a Clear Status
> Register command be executed.
>
> Same thing also happens on other Spansion flash families, they've
> properly handled it, USE_CLSR manufacturer flag was introduced for this
> purpose, but S25FL-L cannot simply reuse their work, because S25FL-L has
> the different error bit settings. S25FL-L defines programming/erase
> error bits in Status Register 2, whereas the other families define the
> same error bits in Status Register 1, causing S25FL-L needs a different
> method to handle this problem.
>
> Cc: stable@...r.kernel.org
> Fixes: 0074a8f3b303 ("mtd: spi-nor: Add support for s25fl128l and s25fl256l")
> Fixes: d8b494a32889 ("mtd: spi-nor: Add support for Spansion S25FL064L")
> Signed-off-by: Yaliang Wang <Yaliang.Wang@...driver.com>
> ---
> drivers/mtd/spi-nor/spansion.c | 119 ++++++++++++++++++++++++++-------
> 1 file changed, 93 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
> index 0150049007be..8f353ddda5f5 100644
> --- a/drivers/mtd/spi-nor/spansion.c
> +++ b/drivers/mtd/spi-nor/spansion.c
> @@ -14,6 +14,7 @@
> #define SPINOR_OP_CLSR 0x30 /* Clear status register 1 */
> #define SPINOR_OP_RD_ANY_REG 0x65 /* Read any register */
> #define SPINOR_OP_WR_ANY_REG 0x71 /* Write any register */
> +#define SPINOR_REG_CYPRESS_SR2V 0x00800001
> #define SPINOR_REG_CYPRESS_CFR1V 0x00800002
> #define SPINOR_REG_CYPRESS_CFR1V_QUAD_EN BIT(1) /* Quad Enable */
> #define SPINOR_REG_CYPRESS_CFR2V 0x00800003
> @@ -25,6 +26,10 @@
> #define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_DS 0
> #define SPINOR_OP_CYPRESS_RD_FAST 0xee
>
> +/* s25fl-l family specific */
> +#define S25FL_L_SR2V_P_ERR BIT(5) /* Programming Error Occurred */
> +#define S25FL_L_SR2V_E_ERR BIT(6) /* Erase Error Occurred */
> +
> /* Cypress SPI NOR flash operations. */
> #define CYPRESS_NOR_WR_ANY_REG_OP(naddr, addr, ndata, buf) \
> SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_ANY_REG, 0), \
> @@ -44,6 +49,29 @@
> SPI_MEM_OP_NO_DUMMY, \
> SPI_MEM_OP_NO_DATA)
>
> +/**
> + * spansion_nor_clear_sr() - Clear the Status Register.
> + * @nor: pointer to 'struct spi_nor'.
> + */
> +static void spansion_nor_clear_sr(struct spi_nor *nor)
> +{
> + int ret;
> +
> + if (nor->spimem) {
> + struct spi_mem_op op = SPANSION_CLSR_OP;
> +
> + spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
> +
> + ret = spi_mem_exec_op(nor->spimem, &op);
> + } else {
> + ret = spi_nor_controller_ops_write_reg(nor, SPINOR_OP_CLSR,
> + NULL, 0);
> + }
> +
> + if (ret)
> + dev_dbg(nor->dev, "error %d clearing SR\n", ret);
> +}
> +
> static int cypress_nor_octal_dtr_en(struct spi_nor *nor)
> {
> struct spi_mem_op op;
> @@ -342,6 +370,65 @@ static const struct spi_nor_fixups s25fs_s_nor_fixups = {
> .post_bfpt = s25fs_s_nor_post_bfpt_fixups,
> };
>
> +/**
> + * s25fl_l_sr_ready_and_clear() - S25FL_L family flashes need to query
> + * Status Register 1 to check if the flash is ready and clear it if
> + * there are Programming/Erase errors in Status Register 2.
> + * @nor: pointer to 'struct spi_nor'.
> + *
> + * Return: 1 if ready, 0 if not ready, -errno on errors.
> + */
> +static int s25fl_l_sr_ready_and_clear(struct spi_nor *nor)
> +{
> + int ret;
> + u8 addr_mode_nbytes = nor->params->addr_mode_nbytes;
> + struct spi_mem_op op =
> + CYPRESS_NOR_RD_ANY_REG_OP(addr_mode_nbytes,
> + SPINOR_REG_CYPRESS_SR2V,
> + &nor->bouncebuf[1]);
> +
> + /* Read Status Register 1 */
> + ret = spi_nor_read_sr(nor, nor->bouncebuf);
> + if (ret)
> + return ret;
> +
> + /* RDSR2 command isn't available in QPI mode, use RDAR instead */
> + ret = spi_nor_read_any_reg(nor, &op, nor->reg_proto);
> + if (ret)
> + return ret;
> +
> + if (nor->bouncebuf[1] & (S25FL_L_SR2V_P_ERR | S25FL_L_SR2V_E_ERR)) {
> + if (nor->bouncebuf[1] & S25FL_L_SR2V_E_ERR)
> + dev_err(nor->dev, "Erase Error occurred\n");
> + else
> + dev_err(nor->dev, "Programming Error occurred\n");
> +
> + spansion_nor_clear_sr(nor);
> +
> + /*
> + * WEL bit remains set to one when an erase or page program
> + * error occurs. Issue a Write Disable command to protect
> + * against inadvertent writes that can possibly corrupt the
> + * contents of the memory.
> + */
> + ret = spi_nor_write_disable(nor);
> + if (ret)
> + return ret;
> +
> + return -EIO;
> + }
> +
> + return !(nor->bouncebuf[0] & SR_WIP);
> +}
> +
> +static void s25fl_l_late_init(struct spi_nor *nor)
> +{
> + nor->params->ready = s25fl_l_sr_ready_and_clear;
> +}
> +
> +static const struct spi_nor_fixups s25fl_l_fixups = {
> + .late_init = s25fl_l_late_init,
> +};
> static const struct flash_info spansion_nor_parts[] = {
> /* Spansion/Cypress -- single (large) sector size only, at least
> * for the chips listed here (without boot sectors).
> @@ -428,13 +515,16 @@ static const struct flash_info spansion_nor_parts[] = {
> NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ) },
> { "s25fl064l", INFO(0x016017, 0, 64 * 1024, 128)
> NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> - FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
> + FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
> + .fixups = &s25fl_l_fixups },
> { "s25fl128l", INFO(0x016018, 0, 64 * 1024, 256)
> NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> - FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
> + FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
> + .fixups = &s25fl_l_fixups },
> { "s25fl256l", INFO(0x016019, 0, 64 * 1024, 512)
> NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> - FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
> + FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
> + .fixups = &s25fl_l_fixups },
> { "s25hl512t", INFO6(0x342a1a, 0x0f0390, 256 * 1024, 256)
> PARSE_SFDP
> MFR_FLAGS(USE_CLSR)
> @@ -460,29 +550,6 @@ static const struct flash_info spansion_nor_parts[] = {
> },
> };
>
> -/**
> - * spansion_nor_clear_sr() - Clear the Status Register.
> - * @nor: pointer to 'struct spi_nor'.
> - */
> -static void spansion_nor_clear_sr(struct spi_nor *nor)
> -{
> - int ret;
> -
> - if (nor->spimem) {
> - struct spi_mem_op op = SPANSION_CLSR_OP;
> -
> - spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
> -
> - ret = spi_mem_exec_op(nor->spimem, &op);
> - } else {
> - ret = spi_nor_controller_ops_write_reg(nor, SPINOR_OP_CLSR,
> - NULL, 0);
> - }
> -
> - if (ret)
> - dev_dbg(nor->dev, "error %d clearing SR\n", ret);
> -}
> -
> /**
> * spansion_nor_sr_ready_and_clear() - Query the Status Register to see if the
> * flash is ready for new commands and clear it if there are any errors.
> --
> 2.34.1
>
--
Cheers,
ta
Powered by blists - more mailing lists