[PATCH 1/3] mtd: spi-nor: Rename SR_TB to indicate the bit used

[PATCH 1/3] mtd: spi-nor: Rename SR_TB to indicate the bit used

[Jungseung Lee]

Each vendor uses different bits for SR_TB of flash.
To avoid ambiguity, rename SR_TB to indicate the bit used.

Signed-off-by: Jungseung Lee <js07.lee@samsung.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 10 +++++-----
 include/linux/mtd/spi-nor.h   |  3 ++-
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 8d4d721fd7f4..7dea5734f085 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1771,7 +1771,7 @@ static void stm_get_locked_range(struct spi_nor *nor, u8 sr, loff_t *ofs,
 	} else {
 		pow = ((sr & mask) ^ mask) >> shift;
 		*len = mtd->size >> pow;
-		if (nor->flags & SNOR_F_HAS_SR_TB && sr & SR_TB)
+		if (nor->flags & SNOR_F_HAS_SR_TB && sr & SR_TB_BIT5)
 			*ofs = 0;
 		else
 			*ofs = mtd->size - *len;
@@ -1903,13 +1903,13 @@ static int stm_lock(struct spi_nor *nor, loff_t ofs, uint64_t len)
 	if (!(val & mask))
 		return -EINVAL;
 
-	status_new = (status_old & ~mask & ~SR_TB) | val;
+	status_new = (status_old & ~mask & ~SR_TB_BIT5) | val;
 
 	/* Disallow further writes if WP pin is asserted */
 	status_new |= SR_SRWD;
 
 	if (!use_top)
-		status_new |= SR_TB;
+		status_new |= SR_TB_BIT5;
 
 	/* Don't bother if they're the same */
 	if (status_new == status_old)
@@ -1987,14 +1987,14 @@ static int stm_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len)
 			return -EINVAL;
 	}
 
-	status_new = (status_old & ~mask & ~SR_TB) | val;
+	status_new = (status_old & ~mask & ~SR_TB_BIT5) | val;
 
 	/* Don't protect status register if we're fully unlocked */
 	if (lock_len == 0)
 		status_new &= ~SR_SRWD;
 
 	if (!use_top)
-		status_new |= SR_TB;
+		status_new |= SR_TB_BIT5;
 
 	/* Don't bother if they're the same */
 	if (status_new == status_old)
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 5a4623fc586b..5124c306f60b 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -128,7 +128,8 @@
 #define SR_BP0			BIT(2)	/* Block protect 0 */
 #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_TB_BIT5		BIT(5)	/* Top/Bottom protect */
+#define SR_TB_BIT6		BIT(6)	/* Top/Bottom protect */
 #define SR_SRWD			BIT(7)	/* SR write protect */
 /* Spansion/Cypress specific status bits */
 #define SR_E_ERR		BIT(5)
-- 
2.17.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 2/3] mtd: spi-nor: Support TB selection using SR bit 6

[Jungseung Lee]

There are some flashes to use bit 6 of status register for Top/Bottom (TB).
Use top/bottom bit variable instead of fixed value and support this case.

Signed-off-by: Jungseung Lee <js07.lee@samsung.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 34 +++++++++++++++++++++++++++-------
 include/linux/mtd/spi-nor.h   |  1 +
 2 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 7dea5734f085..227b56b0a5b0 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,11 @@ 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 SPI_NOR_TB_SR_BIT6	BIT(16)	/*
+					 * Top/Bottom (TB) is bit 6 of
+					 * status register. Must be used with
+					 * SPI_NOR_HAS_TB.
+					 */
 
 	/* Part specific fixup hooks. */
 	const struct spi_nor_fixups *fixups;
@@ -1761,9 +1766,13 @@ static void stm_get_locked_range(struct spi_nor *nor, u8 sr, loff_t *ofs,
 {
 	struct mtd_info *mtd = &nor->mtd;
 	u8 mask = SR_BP2 | SR_BP1 | SR_BP0;
+	u8 tb_mask = SR_TB_BIT5;
 	int shift = ffs(mask) - 1;
 	int pow;
 
+	if (nor->flags & SNOR_F_HAS_SR_TB_BIT6)
+		tb_mask = SR_TB_BIT6;
+
 	if (!(sr & mask)) {
 		/* No protection */
 		*ofs = 0;
@@ -1771,7 +1780,7 @@ static void stm_get_locked_range(struct spi_nor *nor, u8 sr, loff_t *ofs,
 	} else {
 		pow = ((sr & mask) ^ mask) >> shift;
 		*len = mtd->size >> pow;
-		if (nor->flags & SNOR_F_HAS_SR_TB && sr & SR_TB_BIT5)
+		if (nor->flags & SNOR_F_HAS_SR_TB && sr & tb_mask)
 			*ofs = 0;
 		else
 			*ofs = mtd->size - *len;
@@ -1850,6 +1859,7 @@ static int stm_lock(struct spi_nor *nor, loff_t ofs, uint64_t len)
 	struct mtd_info *mtd = &nor->mtd;
 	int ret, status_old, status_new;
 	u8 mask = SR_BP2 | SR_BP1 | SR_BP0;
+	u8 tb_mask = SR_TB_BIT5;
 	u8 shift = ffs(mask) - 1, pow, val;
 	loff_t lock_len;
 	bool can_be_top = true, can_be_bottom = nor->flags & SNOR_F_HAS_SR_TB;
@@ -1886,6 +1896,9 @@ static int stm_lock(struct spi_nor *nor, loff_t ofs, uint64_t len)
 	else
 		lock_len = ofs + len;
 
+	if (nor->flags & SNOR_F_HAS_SR_TB_BIT6)
+		tb_mask = SR_TB_BIT6;
+
 	/*
 	 * Need smallest pow such that:
 	 *
@@ -1903,13 +1916,13 @@ static int stm_lock(struct spi_nor *nor, loff_t ofs, uint64_t len)
 	if (!(val & mask))
 		return -EINVAL;
 
-	status_new = (status_old & ~mask & ~SR_TB_BIT5) | val;
+	status_new = (status_old & ~mask & ~tb_mask) | val;
 
 	/* Disallow further writes if WP pin is asserted */
 	status_new |= SR_SRWD;
 
 	if (!use_top)
-		status_new |= SR_TB_BIT5;
+		status_new |= tb_mask;
 
 	/* Don't bother if they're the same */
 	if (status_new == status_old)
@@ -1932,6 +1945,7 @@ static int stm_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len)
 	struct mtd_info *mtd = &nor->mtd;
 	int ret, status_old, status_new;
 	u8 mask = SR_BP2 | SR_BP1 | SR_BP0;
+	u8 tb_mask = SR_TB_BIT5;
 	u8 shift = ffs(mask) - 1, pow, val;
 	loff_t lock_len;
 	bool can_be_top = true, can_be_bottom = nor->flags & SNOR_F_HAS_SR_TB;
@@ -1968,6 +1982,8 @@ static int stm_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len)
 	else
 		lock_len = ofs;
 
+	if (nor->flags & SNOR_F_HAS_SR_TB_BIT6)
+		tb_mask = SR_TB_BIT6;
 	/*
 	 * Need largest pow such that:
 	 *
@@ -1987,14 +2003,14 @@ static int stm_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len)
 			return -EINVAL;
 	}
 
-	status_new = (status_old & ~mask & ~SR_TB_BIT5) | val;
+	status_new = (status_old & ~mask & ~tb_mask) | val;
 
 	/* Don't protect status register if we're fully unlocked */
 	if (lock_len == 0)
 		status_new &= ~SR_SRWD;
 
 	if (!use_top)
-		status_new |= SR_TB_BIT5;
+		status_new |= tb_mask;
 
 	/* Don't bother if they're the same */
 	if (status_new == status_old)
@@ -5142,8 +5158,12 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
 
 	if (info->flags & USE_FSR)
 		nor->flags |= SNOR_F_USE_FSR;
-	if (info->flags & SPI_NOR_HAS_TB)
+	if (info->flags & SPI_NOR_HAS_TB) {
 		nor->flags |= SNOR_F_HAS_SR_TB;
+		if (info->flags & SPI_NOR_TB_SR_BIT6)
+			nor->flags |= SNOR_F_HAS_SR_TB_BIT6;
+	}
+
 	if (info->flags & NO_CHIP_ERASE)
 		nor->flags |= SNOR_F_NO_OP_CHIP_ERASE;
 	if (info->flags & USE_CLSR)
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 5124c306f60b..7e32adce72f7 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -245,6 +245,7 @@ enum spi_nor_option_flags {
 	SNOR_F_HAS_LOCK		= BIT(8),
 	SNOR_F_HAS_16BIT_SR	= BIT(9),
 	SNOR_F_NO_READ_CR	= BIT(10),
+	SNOR_F_HAS_SR_TB_BIT6	= BIT(11),
 
 };
 
-- 
2.17.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/3] mtd: spi-nor: Support TB selection using SR bit 6

[Tudor.Ambarus]

Hi, Jungseung,

It's great to see this happen :).

On 12/2/19 8:35 AM, Jungseung Lee wrote:
> 
> There are some flashes to use bit 6 of status register for Top/Bottom (TB).

What flashes are using the 6th bit of the SR as TB? Is something that we can
generalize per manufacturer? I'm thinking of using a SNOR_F instead.

Cheers,
ta
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/3] mtd: spi-nor: Support TB selection using SR bit 6

[Jungseung Lee]

Hi, Tudor
2019-12-10 (Tue), 17:08 +0000, Tudor.Ambarus@microchip.com:
> Hi, Jungseung,
> 
> It's great to see this happen :).
> 
> On 12/2/19 8:35 AM, Jungseung Lee wrote:
> > 
> > There are some flashes to use bit 6 of status register for
> > Top/Bottom (TB).
> 
> What flashes are using the 6th bit of the SR as TB? Is something that
> we can
> generalize per manufacturer? I'm thinking of using a SNOR_F instead.
> 
Thanks for your comment.

Actually, I failed to find some generalized way to know which bit is
used for TB.

I was able to find some pattern that it was affected by capacity.

Winbond : Use the 6th bit from 32MB capacity
W25Q20EW, W25Q50BW, W25Q128V - TB(5)
W25Q256JV, W25M512JV - TB(6)

GigaDevice : Use the 6th bit from 32MB capacity
GD25Q16C, GD25Q32C, GD25LQ32D, GD25Q64C, GD25Q128 - TB(5)
GD25Q256 - TB(6)

Micron/STM : Keep to use 5th bit
M25PX64, N25Q128A, N25Q512A, MT25QL512ABB, MT25QL02GCBB - TB(5)

Spansion : Use the 6th bit from 16MB capacity
S25FL116K, S25FL132K, S25FL165K - TB(5)
S25FL128L, S25FL256L - TB(6)

Some of manufacturer use 6th bit for some flashes, that is probably
some cases to need additional BP bit (BP3).

Anyway it was hard to find anything that could be normalized. That's
why I add SPI_NOR_TB_SR_BIT6 that could be set on each flash info.

> Cheers,
> ta
Best Regards,
Jungseung Lee


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/3] mtd: spi-nor: Support TB selection using SR bit 6

[Tudor.Ambarus]

On 12/11/19 8:47 AM, Jungseung Lee wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi, Tudor
> 2019-12-10 (Tue), 17:08 +0000, Tudor.Ambarus@microchip.com:
>> Hi, Jungseung,
>>
>> It's great to see this happen :).
>>
>> On 12/2/19 8:35 AM, Jungseung Lee wrote:
>>>
>>> There are some flashes to use bit 6 of status register for
>>> Top/Bottom (TB).
>>
>> What flashes are using the 6th bit of the SR as TB? Is something that
>> we can
>> generalize per manufacturer? I'm thinking of using a SNOR_F instead.
>>
> Thanks for your comment.
> 
> Actually, I failed to find some generalized way to know which bit is
> used for TB.
> 
> I was able to find some pattern that it was affected by capacity.
> 
> Winbond : Use the 6th bit from 32MB capacity
> W25Q20EW, W25Q50BW, W25Q128V - TB(5)
> W25Q256JV, W25M512JV - TB(6)
> 
> GigaDevice : Use the 6th bit from 32MB capacity
> GD25Q16C, GD25Q32C, GD25LQ32D, GD25Q64C, GD25Q128 - TB(5)
> GD25Q256 - TB(6)
> 
> Micron/STM : Keep to use 5th bit
> M25PX64, N25Q128A, N25Q512A, MT25QL512ABB, MT25QL02GCBB - TB(5)

They keep TB at BIT(5) and define BP3 at BIT(6).

> 
> Spansion : Use the 6th bit from 16MB capacity
> S25FL116K, S25FL132K, S25FL165K - TB(5)
> S25FL128L, S25FL256L - TB(6)

While spansion defines BP3 at BIT(5) and TB at BIT(6). I hoped that at least we
could made a correlation between TB and BP3, i.e. assume that if BP3 is defined
then TB will be at BIT(6), but we can't do this. What a mess.
> 
> Some of manufacturer use 6th bit for some flashes, that is probably
> some cases to need additional BP bit (BP3).
> 
> Anyway it was hard to find anything that could be normalized. That's
> why I add SPI_NOR_TB_SR_BIT6 that could be set on each flash info.
> 

Yeah, makes sense. The explanations from above should fit in the commit message,
but I think I can amend it when applying.

Thanks,
ta
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/3] mtd: spi-nor: Support TB selection using SR bit 6

[Jungseung Lee]

2019-12-11 (Tue), 07:28 +0000, Tudor.Ambarus@microchip.com:
> 
> On 12/11/19 8:47 AM, Jungseung Lee wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you
> > know the content is safe
> > 
> > Hi, Tudor
> > 2019-12-10 (Tue), 17:08 +0000, Tudor.Ambarus@microchip.com:
> > > Hi, Jungseung,
> > > 
> > > It's great to see this happen :).
> > > 
> > > On 12/2/19 8:35 AM, Jungseung Lee wrote:
> > > > 
> > > > There are some flashes to use bit 6 of status register for
> > > > Top/Bottom (TB).
> > > 
> > > What flashes are using the 6th bit of the SR as TB? Is something
> > > that
> > > we can
> > > generalize per manufacturer? I'm thinking of using a SNOR_F
> > > instead.
> > > 
> > 
> > Thanks for your comment.
> > 
> > Actually, I failed to find some generalized way to know which bit
> > is
> > used for TB.
> > 
> > I was able to find some pattern that it was affected by capacity.
> > 
> > Winbond : Use the 6th bit from 32MB capacity
> > W25Q20EW, W25Q50BW, W25Q128V - TB(5)
> > W25Q256JV, W25M512JV - TB(6)
> > 
> > GigaDevice : Use the 6th bit from 32MB capacity
> > GD25Q16C, GD25Q32C, GD25LQ32D, GD25Q64C, GD25Q128 - TB(5)
> > GD25Q256 - TB(6)
> > 
> > Micron/STM : Keep to use 5th bit
> > M25PX64, N25Q128A, N25Q512A, MT25QL512ABB, MT25QL02GCBB - TB(5)
> 
> They keep TB at BIT(5) and define BP3 at BIT(6).
> 
> > 
> > Spansion : Use the 6th bit from 16MB capacity
> > S25FL116K, S25FL132K, S25FL165K - TB(5)
> > S25FL128L, S25FL256L - TB(6)
> 
> While spansion defines BP3 at BIT(5) and TB at BIT(6). I hoped that
> at least we
> could made a correlation between TB and BP3, i.e. assume that if BP3
> is defined
> then TB will be at BIT(6), but we can't do this. What a mess.
Yeah, Really.

> > 
> > Some of manufacturer use 6th bit for some flashes, that is probably
> > some cases to need additional BP bit (BP3).
> > 
> > Anyway it was hard to find anything that could be normalized.
> > That's
> > why I add SPI_NOR_TB_SR_BIT6 that could be set on each flash info.
> > 
> 
> Yeah, makes sense. The explanations from above should fit in the
> commit message,
> but I think I can amend it when applying.
I really appreicate if you can do it.

> 
> Thanks,
> ta
Best Regards,
Jungseung Lee


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 3/3] mtd: spi-nor: Fix wrong TB selection of GD25Q256

[Jungseung Lee]

For GD25Q256, wrong SR bit for top/bottom selection
are being used. Fix it to use appropriate bit.

Signed-off-by: Jungseung Lee <js07.lee@samsung.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 227b56b0a5b0..7cd29491ceae 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -2395,7 +2395,8 @@ static const struct flash_info spi_nor_ids[] = {
 	{
 		"gd25q256", INFO(0xc84019, 0, 64 * 1024, 512,
 			SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
-			SPI_NOR_4B_OPCODES | SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB)
+			SPI_NOR_4B_OPCODES | SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB |
+			SPI_NOR_TB_SR_BIT6)
 			.fixups = &gd25q256_fixups,
 	},
 
-- 
2.17.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 1/3] mtd: spi-nor: Rename SR_TB to indicate the bit used

[Tudor.Ambarus]

On 12/2/19 8:35 AM, Jungseung Lee wrote:
> Each vendor uses different bits for SR_TB of flash.
> To avoid ambiguity, rename SR_TB to indicate the bit used.
> 
> Signed-off-by: Jungseung Lee <js07.lee@samsung.com>
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 10 +++++-----
>  include/linux/mtd/spi-nor.h   |  3 ++-
>  2 files changed, 7 insertions(+), 6 deletions(-)

All applied, thanks.
ta
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/