From: Vignesh Raghavendra <vigneshr@ti.com>
To: Michael Walle <michael@walle.cc>, <linux-mtd@lists.infradead.org>,
<linux-kernel@vger.kernel.org>
Cc: Richard Weinberger <richard@nod.at>,
Tudor Ambarus <tudor.ambarus@microchip.com>,
Boris Brezillon <boris.brezillon@collabora.com>,
Miquel Raynal <miquel.raynal@bootlin.com>
Subject: Re: [PATCH v3] mtd: spi-nor: keep lock bits if they are non-volatile
Date: Wed, 30 Sep 2020 16:05:09 +0530 [thread overview]
Message-ID: <523c3645-e37d-5d86-ba91-5c1be9e3881e@ti.com> (raw)
In-Reply-To: <20200327155939.13153-1-michael@walle.cc>
On 3/27/20 9:29 PM, Michael Walle wrote:
> Traditionally, linux unlocks the whole flash because there are legacy
> devices which has the write protections bits set by default at startup.
> If you actually want to use the flash protection bits, eg. because there
> is a read-only part for a bootloader, this automatic unlocking is
> harmful. If there is no hardware write protection in place (usually
> called WP#), a startup of the kernel just discards this protection.
>
[...]
> Further, the commit 3e0930f109e76 ("mtd: spi-nor: Rework the disabling of
> block write protection") expanded the unlock_all() feature to ANY flash
> which supports locking.
>
Appreciate the detail commit log.
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
> changes since v2:
> - add Kconfig option to be able to retain legacy behaviour
> - rebased the patch due to the spi-nor rewrite
> - dropped the Fixes: tag, it doens't make sense after the spi-nor rewrite
> - mention commit 3e0930f109e76 which further modified the unlock
> behaviour.
>
> changes since v1:
> - completely rewrote patch, the first version used a device tree flag
>
> drivers/mtd/spi-nor/Kconfig | 35 +++++++++++++++++++++++++++++
> drivers/mtd/spi-nor/atmel.c | 24 +++++++++++++-------
> drivers/mtd/spi-nor/core.c | 44 ++++++++++++++++++++++++++++---------
> drivers/mtd/spi-nor/core.h | 6 +++++
> drivers/mtd/spi-nor/esmt.c | 6 ++---
> drivers/mtd/spi-nor/intel.c | 6 ++---
> drivers/mtd/spi-nor/sst.c | 21 +++++++++---------
> include/linux/mtd/spi-nor.h | 6 +++++
> 8 files changed, 114 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
> index 6e816eafb312..647de17c81e2 100644
> --- a/drivers/mtd/spi-nor/Kconfig
> +++ b/drivers/mtd/spi-nor/Kconfig
> @@ -24,6 +24,41 @@ config MTD_SPI_NOR_USE_4K_SECTORS
> Please note that some tools/drivers/filesystems may not work with
> 4096 B erase size (e.g. UBIFS requires 15 KiB as a minimum).
>
> +choice
> + prompt "Write protection at boot"
> + default MTD_SPI_NOR_WP_DISABLE
These choice control how BP0-X bits are manipulated on boot. Hence, to
be consistent should use Block Protection (BP) terminology throughout.
This would also be inline with most flash datasheets which also use term BP
> +
> +config MTD_SPI_NOR_WP_DISABLE
> + bool "Disable WP on any flashes (legacy behaviour)"
> + help
> + This option disables the write protection on any SPI flashes at
> + boot-up.
> +
> + Don't use this if you intent to use the write protection of your
> + SPI flash. This is only to keep backwards compatibility.
> +
> +config MTD_SPI_NOR_WP_DISABLE_ON_VOLATILE
> + bool "Disable WP on flashes w/ volatile protection bits"
> + help
> + Some SPI flashes have volatile block protection bits, ie. after a
> + power-up or a reset the flash is write protected by default.
> +
> + This option disables the write protection for these kind of flashes
> + while keeping it enabled for any other SPI flashes which have
> + non-volatile block protection bits.
> +
> + If you are unsure, select this option.
> +
> +config MTD_SPI_NOR_WP_KEEP
> + bool "Keep write protection as is"
> + help
> + If you select this option the write protection of any SPI flashes
> + will not be changed. If your flash is write protected or will be
> + automatically write protected after power-up you have to manually
> + unlock it before you are able to write to it.
> +
> +endchoice
> +
> source "drivers/mtd/spi-nor/controllers/Kconfig"
>
> endif # MTD_SPI_NOR
[...]
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index 6f2f6b27173f..9a33c023717f 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -26,6 +26,7 @@ enum spi_nor_option_flags {
> SNOR_F_HAS_SR_TB_BIT6 = BIT(11),
> SNOR_F_HAS_4BIT_BP = BIT(12),
> SNOR_F_HAS_SR_BP3_BIT6 = BIT(13),
> + SNOR_F_NEED_UNPROTECT = BIT(14),
> };
>
> struct spi_nor_read_command {
> @@ -311,6 +312,11 @@ struct flash_info {
> * BP3 is bit 6 of status register.
> * Must be used with SPI_NOR_4BIT_BP.
> */
> +#define SPI_NOR_UNPROTECT BIT(19) /*
> + * Flash is write-protected after
> + * power-up and needs a global
> + * unprotect.
> + */
>
It would be better to name the flag to indicate BP bits are volatile or
powers up locked instead of SPI_NOR_UNPROTECT. This makes it easier to
understand what this flag means wrt flash HW feature. Maybe:
SPI_NOR_LOCKED_ON_POWER_UP or SPI_NOR_BP_IS_VOLATILE
Reset looks fine to me
[...]
Regards
Vignesh
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
next prev parent reply other threads:[~2020-09-30 10:36 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-27 15:59 [PATCH v3] mtd: spi-nor: keep lock bits if they are non-volatile Michael Walle
2020-09-30 10:35 ` Vignesh Raghavendra [this message]
2020-09-30 22:51 ` Michael Walle
2020-10-01 10:40 ` Vignesh Raghavendra
2020-09-30 14:00 ` Tudor.Ambarus
2020-09-30 22:38 ` Michael Walle
2020-10-01 7:07 ` Tudor.Ambarus
2020-10-01 7:38 ` Michael Walle
2020-10-01 11:46 ` Tudor.Ambarus
2020-10-01 12:26 ` Michael Walle
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=523c3645-e37d-5d86-ba91-5c1be9e3881e@ti.com \
--to=vigneshr@ti.com \
--cc=boris.brezillon@collabora.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mtd@lists.infradead.org \
--cc=michael@walle.cc \
--cc=miquel.raynal@bootlin.com \
--cc=richard@nod.at \
--cc=tudor.ambarus@microchip.com \
/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 NNTP newsgroup(s).