From: "Michael Walle" <michael@walle.cc>
To: "Marek Vasut" <marex@denx.de>, <u-boot@lists.denx.de>
Cc: "Hai Pham" <hai.pham.ud@renesas.com>,
"Heinrich Schuchardt" <xypron.glpk@gmx.de>,
"Jagan Teki" <jagan@amarulasolutions.com>,
"Johann Neuhauser" <jneuhauser@dh-electronics.com>,
"Simon Glass" <sjg@chromium.org>,
"Takahiro Kuwano" <Takahiro.Kuwano@infineon.com>,
"Venkatesh Yadav Abbarapu" <venkatesh.abbarapu@amd.com>,
"Vignesh R" <vigneshr@ti.com>, <linux-mtd@lists.infradead.org>
Subject: Re: [PATCH] mtd: spi-nor: Clear Winbond SR3 WPS bit on boot
Date: Tue, 05 Mar 2024 09:55:30 +0100 [thread overview]
Message-ID: <CZLOVK212G8L.2ISEL1IIWU1YE@walle.cc> (raw)
In-Reply-To: <20240304161607.82759-1-marex@denx.de>
[-- Attachment #1.1: Type: text/plain, Size: 4831 bytes --]
[+ linux-mtd ]
Hi Marek,
On Mon Mar 4, 2024 at 5:16 PM CET, Marek Vasut wrote:
> Some Winbond SPI NORs have special SR3 register which is
> used among other things to control whether non-standard
> "Individual Block/Sector Write Protection" (WPS bit)
> locking scheme is activated. This non-standard locking
> scheme is not supported by either U-Boot or Linux SPI
> NOR stack so make sure it is disabled, otherwise the
> SPI NOR may appear locked for no obvious reason.
I don't think it is a good idea to just disable the WPS bit.
Usually, that bit is non-volatile and the default is not set. Thus,
there is likely someone, probably the manufacturer of the board,
who intentionally set this bit. Now, with this patch it will get
disabled *unconditionally*, forever.
-michael
> W25Q16DW, but the W25Q16DW does not implement the SR3 WPS bit.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Hai Pham <hai.pham.ud@renesas.com>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Cc: Jagan Teki <jagan@amarulasolutions.com>
> Cc: Johann Neuhauser <jneuhauser@dh-electronics.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
> Cc: Venkatesh Yadav Abbarapu <venkatesh.abbarapu@amd.com>
> Cc: Vignesh R <vigneshr@ti.com>
> ---
> drivers/mtd/spi/spi-nor-core.c | 47 ++++++++++++++++++++++++++++++++++
> include/linux/mtd/spi-nor.h | 5 ++++
> 2 files changed, 52 insertions(+)
>
> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> index 9a1801ba93d..68dee57258d 100644
> --- a/drivers/mtd/spi/spi-nor-core.c
> +++ b/drivers/mtd/spi/spi-nor-core.c
> @@ -544,6 +544,24 @@ static int read_cr(struct spi_nor *nor)
> }
> #endif
>
> +/**
> + * read_sr3() - Read status register 3 unique to newer Winbond flashes
> + * @nor: pointer to a 'struct spi_nor'
> + */
> +static int read_sr3(struct spi_nor *nor)
> +{
> + int ret;
> + u8 val;
> +
> + ret = nor->read_reg(nor, SPINOR_OP_RDSR3, &val, 1);
> + if (ret < 0) {
> + dev_dbg(nor->dev, "error %d reading SR3\n", ret);
> + return ret;
> + }
> +
> + return val;
> +}
> +
> /*
> * Write status register 1 byte
> * Returns negative if error occurred.
> @@ -554,6 +572,17 @@ static int write_sr(struct spi_nor *nor, u8 val)
> return nor->write_reg(nor, SPINOR_OP_WRSR, nor->cmd_buf, 1);
> }
>
> +/**
> + * write_sr3() - Write status register 3 unique to newer Winbond flashes
> + * @nor: pointer to a 'struct spi_nor'
> + * @val: value to be written into SR3
> + */
> +static int write_sr3(struct spi_nor *nor, u8 val)
> +{
> + nor->cmd_buf[0] = val;
> + return nor->write_reg(nor, SPINOR_OP_WRSR3, nor->cmd_buf, 1);
> +}
> +
> /*
> * Set write enable latch with Write Enable command.
> * Returns negative if error occurred.
> @@ -3890,6 +3919,24 @@ static int spi_nor_init(struct spi_nor *nor)
> write_enable(nor);
> write_sr(nor, 0);
> spi_nor_wait_till_ready(nor);
> +
> + /*
> + * Some Winbond SPI NORs have special SR3 register which is
> + * used among other things to control whether non-standard
> + * "Individual Block/Sector Write Protection" (WPS bit)
> + * locking scheme is activated. This non-standard locking
> + * scheme is not supported by either U-Boot or Linux SPI
> + * NOR stack so make sure it is disabled, otherwise the
> + * SPI NOR may appear locked for no obvious reason.
> + */
> + if (JEDEC_MFR(nor->info) == SNOR_MFR_WINBOND) {
> + err = read_sr3(nor);
> + if (err > 0 && err & SR3_WPS) {
> + write_enable(nor);
> + write_sr3(nor, err & ~SR3_WPS);
> + write_disable(nor);
> + }
> + }
> }
>
> if (nor->quad_enable) {
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index 2861b73edbc..ceb32e3906f 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -45,6 +45,8 @@
> #define SPINOR_OP_WRSR 0x01 /* Write status register 1 byte */
> #define SPINOR_OP_RDSR2 0x3f /* Read status register 2 */
> #define SPINOR_OP_WRSR2 0x3e /* Write status register 2 */
> +#define SPINOR_OP_RDSR3 0x15 /* Read status register 3 */
> +#define SPINOR_OP_WRSR3 0x11 /* Write status register 3 */
> #define SPINOR_OP_READ 0x03 /* Read data bytes (low frequency) */
> #define SPINOR_OP_READ_FAST 0x0b /* Read data bytes (high frequency) */
> #define SPINOR_OP_READ_1_1_2 0x3b /* Read data bytes (Dual Output SPI) */
> @@ -185,6 +187,9 @@
> /* Status Register 2 bits. */
> #define SR2_QUAD_EN_BIT7 BIT(7)
>
> +/* Status Register 3 bits. */
> +#define SR3_WPS BIT(2)
> +
> /* For Cypress flash. */
> #define SPINOR_OP_RD_ANY_REG 0x65 /* Read any register */
> #define SPINOR_OP_WR_ANY_REG 0x71 /* Write any register */
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 252 bytes --]
[-- Attachment #2: Type: text/plain, Size: 144 bytes --]
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
next parent reply other threads:[~2024-03-05 8:56 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20240304161607.82759-1-marex@denx.de>
2024-03-05 8:55 ` Michael Walle [this message]
2024-03-05 12:31 ` [PATCH] mtd: spi-nor: Clear Winbond SR3 WPS bit on boot Marek Vasut
2024-03-05 12:50 ` Michael Walle
2024-03-05 15:37 ` Marek Vasut
2024-03-05 15:53 ` Michael Walle
2024-03-05 16:28 ` Marek Vasut
2024-03-05 16:55 ` Michael Walle
2024-03-05 18:54 ` Marek Vasut
2024-03-05 21:41 ` Michael Walle
2024-03-06 2:56 ` Marek Vasut
2024-03-06 12:55 ` Michael Walle
2024-05-15 14:55 ` Marek Vasut
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=CZLOVK212G8L.2ISEL1IIWU1YE@walle.cc \
--to=michael@walle.cc \
--cc=Takahiro.Kuwano@infineon.com \
--cc=hai.pham.ud@renesas.com \
--cc=jagan@amarulasolutions.com \
--cc=jneuhauser@dh-electronics.com \
--cc=linux-mtd@lists.infradead.org \
--cc=marex@denx.de \
--cc=sjg@chromium.org \
--cc=u-boot@lists.denx.de \
--cc=venkatesh.abbarapu@amd.com \
--cc=vigneshr@ti.com \
--cc=xypron.glpk@gmx.de \
/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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).