diff mbox series

driver/mtd/spi-nor: Regression lock/unlock fail

Message ID 20190919014916.1303-1-shreyas.joshi@biamp.com
State Changes Requested
Delegated to: Ambarus Tudor
Headers show
Series driver/mtd/spi-nor: Regression lock/unlock fail | expand

Commit Message

Shreyas Joshi Sept. 19, 2019, 1:49 a.m. UTC 
From: shreyas <shreyasjoshi15@gmail.com>

      The n25q128 micron chips cannot be treated similar to STM SPI NOR
      when it comes to flash lock/unlock. The JDEC ID read here is just 1 byte
      and it is not sufficient to distinguish between different chips. Thus,
      memory type is used here in addition to JDEC ID to distinguish further.
      Once the unique manufacture id is detected, it will invoke the required
      lock/unlock function required for n25 micron chipsets.

Signed-off-by: shreyas <shreyasjoshi15@gmail.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 20 +++++++++++---------
 include/linux/mtd/spi-nor.h   |  4 ++--
 2 files changed, 13 insertions(+), 11 deletions(-)

Comments

Tudor Ambarus Sept. 19, 2019, 5:05 a.m. UTC | #1 
On 09/19/2019 04:49 AM, Shreyas Joshi wrote:
> @@ -1356,6 +1356,7 @@ static int n25q_lock(struct spi_nor *nor, loff_t ofs, uint64_t len)

n25q_lock is not yet defined in spi-nor.c.

You should do your patches on top of
https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git, spi-nor/next branch.

Cheers,
ta
Tudor Ambarus April 20, 2020, 7:29 a.m. UTC | #2 
Hi, Shreyas,

This is not a regression because locking was never enabled for n25q128a11.
Please apply the following patch on spi-nor/next, it should solve your locking 
problems:

https://patchwork.ozlabs.org/project/linux-mtd/patch/1587103677-244754-1-git-send-email-chenxiang66@hisilicon.com/

On Monday, April 20, 2020 4:25:55 AM EEST Shreyas Joshi wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe I tried but then I gave up since the latest code has no
> room for porting those changes. Instead, I have raised an issue in bugzilla
> here  -
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=206867#
> 
> Am I missing anything while raising the jira? Is it wrong way to raise issue
> or is that Ok? I didn't see any action so I was worried if I haven't raised

I don't know, I have never used bugzilla.kernel.org. A bug report to linux-mtd 
mailing list is good enough for me.

Cheers,
ta
Tudor Ambarus April 21, 2020, 4:42 a.m. UTC | #3 
On Tuesday, April 21, 2020 2:42:29 AM EEST Shreyas Joshi wrote:
>  Thanks, it works.

Great! Can I have your Tested-by on that patch?
https://patchwork.ozlabs.org/project/linux-mtd/patch/1587103677-244754-1-git-send-email-chenxiang66@hisilicon.com/

> I wish, I could have assigned this bug to you but I am unable to edit this.
> However, I have added your email now.

No worries, I'm sure that someone will update the status of the bug.

Cheers,
ta
diff mbox series

Patch

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index f3b2df2c52b4..6dfdd95a5961 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1356,6 +1356,7 @@  static int n25q_lock(struct spi_nor *nor, loff_t ofs, uint64_t len)
 	int status_old, status_new;
 	uint32_t offset = ofs;
 	int ret = 0;
+
 	status_old = read_sr(nor);
 	if (status_old < 0)
 		return status_old;
@@ -1373,7 +1374,8 @@  static int n25q_lock(struct spi_nor *nor, loff_t ofs, uint64_t len)
 		status_new = (status_old & ~(SR_BP2|SR_BP0)) | SR_BP1;
 	else
 		status_new = (status_old & ~(SR_BP2|SR_BP1)) | SR_BP0;
-	if ((status_new & (SR_BP2|SR_BP1|SR_BP0)) > (status_old & (SR_BP2|SR_BP1|SR_BP0))) {
+	if ((status_new & (SR_BP2|SR_BP1|SR_BP0)) >
+			(status_old & (SR_BP2|SR_BP1|SR_BP0))) {
 		write_enable(nor);
 		if (write_sr(nor, status_new) < 0)
 			return -EINVAL;
@@ -1392,6 +1394,7 @@  static int n25q_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len)
 	int status_old, status_new;
 	uint32_t offset = ofs;
 	int ret = 0;
+
 	status_old = read_sr(nor);
 	if (status_old < 0)
 		return status_old;
@@ -4266,14 +4269,13 @@  int spi_nor_scan(struct spi_nor *nor, const char *name,
 	if (JEDEC_MFR(info) == SNOR_MFR_ST ||
 	    JEDEC_MFR(info) == SNOR_MFR_MICRON ||
 	    info->flags & SPI_NOR_HAS_LOCK) {
-	if (JEDEC_MT(info) == SNOR_MT_MICRON) {
-		nor->flash_lock = n25q_lock;
-		nor->flash_unlock = n25q_unlock;
-	}
-	else {
-		nor->flash_lock = stm_lock;
-		nor->flash_unlock = stm_unlock;
-	}
+		if (JEDEC_MT(info) == SNOR_MT_MICRON) {
+			nor->flash_lock = n25q_lock;
+			nor->flash_unlock = n25q_unlock;
+		} else {
+			nor->flash_lock = stm_lock;
+			nor->flash_unlock = stm_unlock;
+		}
 	nor->flash_is_locked = stm_is_locked;
 	}
 	if (nor->flash_lock && nor->flash_unlock && nor->flash_is_locked) {
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index aaa3c1065b3b..3939cd77d778 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -27,8 +27,8 @@ 
 #define SNOR_MFR_WINBOND	0xef /* Also used by some Spansion */
 
 /* Manufacturer Memory Type
-* The second byte returned from the flash after sending opcode SPINOR_OP_RDID.
-*/
+ * The second byte returned from the flash after sending opcode SPINOR_OP_RDID.
+ */
 #define SNOR_MT_MICRON	0xba
 #define SNOR_MT_ST      0x20