[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20190825144921.66139a65@collabora.com>
Date: Sun, 25 Aug 2019 14:49:21 +0200
From: Boris Brezillon <boris.brezillon@...labora.com>
To: <Tudor.Ambarus@...rochip.com>
Cc: <marek.vasut@...il.com>, <vigneshr@...com>,
<miquel.raynal@...tlin.com>, <richard@....at>,
<linux-mtd@...ts.infradead.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 1/2] mtd: spi-nor: add Global Block Unlock support
On Sat, 24 Aug 2019 12:27:12 +0000
<Tudor.Ambarus@...rochip.com> wrote:
> From: Tudor Ambarus <tudor.ambarus@...rochip.com>
>
> To avoid inadvertent writes during power-up, some flashes are
> write-protected by default after a power-on reset cycle.
> A Global Block-Protection Unlock command offers a single
> command cycle that unlocks the entire memory array. This is
> identical with what other nor flashes are doing by clearing
> the block protection bits from the status register: disable
> the write protection after a power-on reset cycle.
>
> We can't determine this purely by manufacturer type and it's not
> autodetectable by anything like SFDP, so make a new flag for it:
> UNLOCK_GLOBAL_BLOCK.
>
> Note that the Global Block Unlock command has different names
> depending on the manufacturer, but always the same command value:
> 0x98. Macronix's MX25U12835F names it Gang Block Unlock,
> Winbound's W25Q128FV names it Global Block Unlock and
> Microchip's SST26VF064B names it Global Block Protection Unlock.
>
> Signed-off-by: Tudor Ambarus <tudor.ambarus@...rochip.com>
> ---
> v2: the check for UNLOCK_GLOBAL_BLOCK should be done the first
> thing in spi_nor_disable_block_protection(). We use it for a faster
> throughput, a single command cycle that unlocks the entire memory
> array. Fix it.
>
> drivers/mtd/spi-nor/spi-nor.c | 46 ++++++++++++++++++++++++++++++++++++++++++-
> include/linux/mtd/spi-nor.h | 1 +
> 2 files changed, 46 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 1896d36a7d11..c0ba6fe62461 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -196,7 +196,7 @@ struct flash_info {
> u16 page_size;
> u16 addr_width;
>
> - u16 flags;
> + u32 flags;
> #define SECT_4K BIT(0) /* SPINOR_OP_BE_4K works uniformly */
> #define SPI_NOR_NO_ERASE BIT(1) /* No erase command needed */
> #define SST_WRITE BIT(2) /* use SST byte programming */
> @@ -233,6 +233,7 @@ struct flash_info {
> #define SPI_NOR_SKIP_SFDP BIT(13) /* Skip parsing of SFDP tables */
> #define USE_CLSR BIT(14) /* use CLSR command */
> #define SPI_NOR_OCTAL_READ BIT(15) /* Flash supports Octal Read */
> +#define UNLOCK_GLOBAL_BLOCK BIT(16) /* Unlock global block protection */
Let's add the corresponding SNOR_F_ flag so that manufacturer/sfdp init
can set the flag directly.
>
> /* Part specific fixup hooks. */
> const struct spi_nor_fixups *fixups;
> @@ -2031,6 +2032,41 @@ static int spi_nor_spansion_clear_sr_bp(struct spi_nor *nor)
> return spi_nor_clear_sr_bp(nor);
> }
>
> +/**
> + * spi_nor_unlock_global_block_protection() - Unlock the Global Block Protection
> + * @nor: pointer to a 'struct spi_nor'
> + *
> + * The Global Block-Protection Unlock command offers a single command cycle
> + * that unlocks the entire memory array.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +static int spi_nor_unlock_global_block_protection(struct spi_nor *nor)
> +{
> + int ret;
> +
> + write_enable(nor);
> +
> + if (nor->spimem) {
> + struct spi_mem_op op =
> + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_ULBPR, 1),
> + SPI_MEM_OP_NO_ADDR,
> + SPI_MEM_OP_NO_DUMMY,
> + SPI_MEM_OP_NO_DATA);
> +
> + ret = spi_mem_exec_op(nor->spimem, &op);
> + } else {
> + ret = nor->write_reg(nor, SPINOR_OP_ULBPR, NULL, 0);
> + }
> +
> + if (ret < 0) {
> + dev_err(nor->dev, "error %d on ULBPR\n", ret);
> + return ret;
> + }
> +
> + return spi_nor_wait_till_ready(nor);
> +}
> +
> /* Used when the "_ext_id" is two bytes at most */
> #define INFO(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags) \
> .id = { \
> @@ -4697,6 +4733,14 @@ static int spi_nor_quad_enable(struct spi_nor *nor)
> */
> static int spi_nor_disable_block_protection(struct spi_nor *nor)
> {
> + /*
> + * If the flash supports the Global Block-Protection Unlock command,
> + * use it for faster throughput: a single command cycle that unlocks
> + * the entire memory array.
> + */
> + if (nor->info->flags & UNLOCK_GLOBAL_BLOCK)
> + return spi_nor_unlock_global_block_protection(nor);
Hm, I'd rather put that as a new hook in spi_nor_locking_ops, this way
we're ready to support any vendor-specific variant (and I like the idea
of having all locking-related functions placed in a separate _ops
struct).
> +
> if (!nor->params.disable_block_protection)
> return 0;
>
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index 4752d08e9a3e..31b99a7743fc 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -66,6 +66,7 @@
> #define SPINOR_OP_CLFSR 0x50 /* Clear flag status register */
> #define SPINOR_OP_RDEAR 0xc8 /* Read Extended Address Register */
> #define SPINOR_OP_WREAR 0xc5 /* Write Extended Address Register */
> +#define SPINOR_OP_ULBPR 0x98 /* Global Block Unlock Protection */
>
> /* 4-byte address opcodes - used on Spansion and some Macronix flashes. */
> #define SPINOR_OP_READ_4B 0x13 /* Read data bytes (low frequency) */
Powered by blists - more mailing lists