[1/2,v2] mtd: spi-nor: add 4bit block protection support

Message ID 20181212102515.7296-1-js07.lee@samsung.com
State Changes Requested
Delegated to: Ambarus Tudor
Headers show
Series
  • [1/2,v2] mtd: spi-nor: add 4bit block protection support
Related show

Commit Message

Jungseung Lee Dec. 12, 2018, 10:25 a.m.
Currently, we are supporting block protection only for
flash chips with 3 block protection bits in the SR register.
This patch enables block protection support for some flash with
4 block protection bits (bp0-3).

Because this feature is not universal to all flash that support
lock/unlock, control it via a new flag.

Signed-off-by: Jungseung Lee <js07.lee@samsung.com>
---
ChangeLog v1->v2:
- Rebase on latest MTD development branch
---
 drivers/mtd/spi-nor/spi-nor.c | 61 ++++++++++++++++++++++++++++++-----
 include/linux/mtd/spi-nor.h   |  4 +++
 2 files changed, 57 insertions(+), 8 deletions(-)

Comments

Ambarus Tudor Feb. 26, 2019, 10:18 a.m. | #1
+Uwe, he showed interest in this.

Hi, Jungseung,

On 12/12/2018 12:25 PM, Jungseung Lee wrote:
> Currently, we are supporting block protection only for
> flash chips with 3 block protection bits in the SR register.
> This patch enables block protection support for some flash with
> 4 block protection bits (bp0-3).
> 
> Because this feature is not universal to all flash that support
> lock/unlock, control it via a new flag.
> 
> Signed-off-by: Jungseung Lee <js07.lee@samsung.com>
> ---
> ChangeLog v1->v2:
> - Rebase on latest MTD development branch
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 61 ++++++++++++++++++++++++++++++-----
>  include/linux/mtd/spi-nor.h   |  4 +++
>  2 files changed, 57 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 6e13bbd1aaa5..c33b72eeae12 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -96,6 +96,7 @@ enum spi_nor_pp_command_index {
>  struct spi_nor_flash_parameter {
>  	u64				size;
>  	u32				page_size;
> +	u16				n_sectors;
>  
>  	struct spi_nor_hwcaps		hwcaps;
>  	struct spi_nor_read_command	reads[SNOR_CMD_READ_MAX];
> @@ -278,6 +279,7 @@ struct flash_info {
>  #define NO_CHIP_ERASE		BIT(12) /* Chip does not support chip erase */
>  #define SPI_NOR_SKIP_SFDP	BIT(13)	/* Skip parsing of SFDP tables */
>  #define USE_CLSR		BIT(14)	/* use CLSR command */
> +#define SPI_NOR_HAS_BP3		BIT(15)	/* use 4 bits field for block protect */
>  
>  	/* Part specific fixup hooks. */
>  	const struct spi_nor_fixups *fixups;
> @@ -1087,18 +1089,36 @@ static void stm_get_locked_range(struct spi_nor *nor, u8 sr, loff_t *ofs,
>  	int shift = ffs(mask) - 1;
>  	int pow;
>  
> +	if (nor->flags & SNOR_F_HAS_SR_BP3)
> +		mask |= SR_BP3;

I asked myself if the shift variable is still conclusive after updating the
mask. I assume we can add a macro for it, to skip this kind of questions.
> +
>  	if (!(sr & mask)) {
>  		/* No protection */
>  		*ofs = 0;
>  		*len = 0;
> +		return;
> +	}
> +
> +	if (nor->flags & SNOR_F_HAS_SR_BP3) {
> +		u8 temp = sr & mask;
> +
> +		if (temp & SR_BP3)
> +			temp = (temp & ~SR_BP3) | BIT(5);

N25Q512A defines SR_BP3 at BIT(6) indeed, but W25M512JV defines it directly at
BIT(5) and moves the TB definition at BIT(6). We will need to know how the large
majority of manufacturers are implementing BP3, and based on that to decide how
we will implement the "generic" BP3 support. I would look at spi_nor_ids[],
check which manufacturers have flashes that set SPI_NOR_HAS_LOCK and I would
look at those manufacturers for newer flashes that might have BP3 support. Would
you please make this study and tell us your findings?

> +
> +		pow = ilog2(nor->n_sectors) + 1 - (temp >> shift);
> +		if (pow > 0)
> +			*len = mtd->size >> pow;
> +		else
> +			*len = mtd->size;

having negative values for pow is not so intuitive. Use Brian's way of computing
pow, and it will result in nicer code: pow = ((sr & mask) ^ mask) >> shift;

Cheers,
ta

Patch

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 6e13bbd1aaa5..c33b72eeae12 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -96,6 +96,7 @@  enum spi_nor_pp_command_index {
 struct spi_nor_flash_parameter {
 	u64				size;
 	u32				page_size;
+	u16				n_sectors;
 
 	struct spi_nor_hwcaps		hwcaps;
 	struct spi_nor_read_command	reads[SNOR_CMD_READ_MAX];
@@ -278,6 +279,7 @@  struct flash_info {
 #define NO_CHIP_ERASE		BIT(12) /* Chip does not support chip erase */
 #define SPI_NOR_SKIP_SFDP	BIT(13)	/* Skip parsing of SFDP tables */
 #define USE_CLSR		BIT(14)	/* use CLSR command */
+#define SPI_NOR_HAS_BP3		BIT(15)	/* use 4 bits field for block protect */
 
 	/* Part specific fixup hooks. */
 	const struct spi_nor_fixups *fixups;
@@ -1087,18 +1089,36 @@  static void stm_get_locked_range(struct spi_nor *nor, u8 sr, loff_t *ofs,
 	int shift = ffs(mask) - 1;
 	int pow;
 
+	if (nor->flags & SNOR_F_HAS_SR_BP3)
+		mask |= SR_BP3;
+
 	if (!(sr & mask)) {
 		/* No protection */
 		*ofs = 0;
 		*len = 0;
+		return;
+	}
+
+	if (nor->flags & SNOR_F_HAS_SR_BP3) {
+		u8 temp = sr & mask;
+
+		if (temp & SR_BP3)
+			temp = (temp & ~SR_BP3) | BIT(5);
+
+		pow = ilog2(nor->n_sectors) + 1 - (temp >> shift);
+		if (pow > 0)
+			*len = mtd->size >> pow;
+		else
+			*len = mtd->size;
 	} else {
 		pow = ((sr & mask) ^ mask) >> shift;
 		*len = mtd->size >> pow;
-		if (nor->flags & SNOR_F_HAS_SR_TB && sr & SR_TB)
-			*ofs = 0;
-		else
-			*ofs = mtd->size - *len;
 	}
+
+	if (nor->flags & SNOR_F_HAS_SR_TB && sr & SR_TB)
+		*ofs = 0;
+	else
+		*ofs = mtd->size - *len;
 }
 
 /*
@@ -1178,6 +1198,9 @@  static int stm_lock(struct spi_nor *nor, loff_t ofs, uint64_t len)
 	bool can_be_top = true, can_be_bottom = nor->flags & SNOR_F_HAS_SR_TB;
 	bool use_top;
 
+	if (nor->flags & SNOR_F_HAS_SR_BP3)
+		mask |= SR_BP3;
+
 	status_old = read_sr(nor);
 	if (status_old < 0)
 		return status_old;
@@ -1217,7 +1240,16 @@  static int stm_lock(struct spi_nor *nor, loff_t ofs, uint64_t len)
 	 *   pow = ceil(log2(size / len)) = log2(size) - floor(log2(len))
 	 */
 	pow = ilog2(mtd->size) - ilog2(lock_len);
-	val = mask - (pow << shift);
+
+	if (nor->flags & SNOR_F_HAS_SR_BP3) {
+		val = ilog2(nor->n_sectors) + 1 - pow;
+		val = val << shift;
+		if (val & BIT(5))
+			val = (val & ~BIT(5)) | SR_BP3;
+	} else {
+		val = mask - (pow << shift);
+	}
+
 	if (val & ~mask)
 		return -EINVAL;
 	/* Don't "lock" with no region! */
@@ -1258,6 +1290,9 @@  static int stm_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len)
 	bool can_be_top = true, can_be_bottom = nor->flags & SNOR_F_HAS_SR_TB;
 	bool use_top;
 
+	if (nor->flags & SNOR_F_HAS_SR_BP3)
+		mask |= SR_BP3;
+
 	status_old = read_sr(nor);
 	if (status_old < 0)
 		return status_old;
@@ -1299,13 +1334,19 @@  static int stm_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len)
 	pow = ilog2(mtd->size) - order_base_2(lock_len);
 	if (lock_len == 0) {
 		val = 0; /* fully unlocked */
+	} else if (nor->flags & SNOR_F_HAS_SR_BP3) {
+		val = ilog2(nor->n_sectors) + 1 - pow;
+		val = val << shift;
+		if (val & BIT(5))
+			val = (val & ~BIT(5)) | SR_BP3;
 	} else {
 		val = mask - (pow << shift);
-		/* Some power-of-two sizes are not supported */
-		if (val & ~mask)
-			return -EINVAL;
 	}
 
+	/* Some power-of-two sizes are not supported */
+	if (val & ~mask)
+		return -EINVAL;
+
 	status_new = (status_old & ~mask & ~SR_TB) | val;
 
 	/* Don't protect status register if we're fully unlocked */
@@ -3563,6 +3604,7 @@  static int spi_nor_init_params(struct spi_nor *nor,
 	/* Set SPI NOR sizes. */
 	params->size = (u64)info->sector_size * info->n_sectors;
 	params->page_size = info->page_size;
+	params->n_sectors = info->n_sectors;
 
 	/* (Fast) Read settings. */
 	params->hwcaps.mask |= SNOR_HWCAPS_READ;
@@ -4065,12 +4107,15 @@  int spi_nor_scan(struct spi_nor *nor, const char *name,
 		nor->flags |= SNOR_F_NO_OP_CHIP_ERASE;
 	if (info->flags & USE_CLSR)
 		nor->flags |= SNOR_F_USE_CLSR;
+	if (info->flags & SPI_NOR_HAS_BP3)
+		nor->flags |= SNOR_F_HAS_SR_BP3;
 
 	if (info->flags & SPI_NOR_NO_ERASE)
 		mtd->flags |= MTD_NO_ERASE;
 
 	mtd->dev.parent = dev;
 	nor->page_size = params.page_size;
+	nor->n_sectors = params.n_sectors;
 	mtd->writebufsize = nor->page_size;
 
 	if (np) {
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index fa2d89e38e40..b00ae7c6ecab 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -119,6 +119,7 @@ 
 #define SR_BP1			BIT(3)	/* Block protect 1 */
 #define SR_BP2			BIT(4)	/* Block protect 2 */
 #define SR_TB			BIT(5)	/* Top/Bottom protect */
+#define SR_BP3			BIT(6)	/* Block protect 3 */
 #define SR_SRWD			BIT(7)	/* SR write protect */
 /* Spansion/Cypress specific status bits */
 #define SR_E_ERR		BIT(5)
@@ -235,6 +236,7 @@  enum spi_nor_option_flags {
 	SNOR_F_BROKEN_RESET	= BIT(6),
 	SNOR_F_4B_OPCODES	= BIT(7),
 	SNOR_F_HAS_4BAIT	= BIT(8),
+	SNOR_F_HAS_SR_BP3	= BIT(9),
 };
 
 /**
@@ -338,6 +340,7 @@  struct flash_info;
  * @dev:		point to a spi device, or a spi nor controller device.
  * @info:		spi-nor part JDEC MFR id and other info
  * @page_size:		the page size of the SPI NOR
+ * @n_sectors:		number of sector
  * @addr_width:		number of address bytes
  * @erase_opcode:	the opcode for erasing a sector
  * @read_opcode:	the read opcode
@@ -374,6 +377,7 @@  struct spi_nor {
 	struct device		*dev;
 	const struct flash_info	*info;
 	u32			page_size;
+	u16			n_sectors;
 	u8			addr_width;
 	u8			erase_opcode;
 	u8			read_opcode;