From: Tudor Ambarus <[email protected]>
Patches 1 - 20 are just clean up patches for the Flash Register
Operations.
Patches 21 - 32 deal with the Quad Enable and the (un)lock methods.
Fixed the clearing of QE bit on (un)lock() operations. Reworked the
Quad Enable methods and the disabling of the block write protection
at power-up.
Tested on s25fl116k and w25q128jv-q.
The patch set can be tested using mtd-utils:
1/ do a read-erase-write-read-back test immediately after boot, to check
the spi_nor_unlock_all() method. The focus is on the erase/write
methods, we want to see if the flash is unlocked at power-up.
mtd_debug read /dev/mtd-yours offset size read-file
hexdump read-file
mtd_debug erase /dev/mtd-yours offset size
dd if=/dev/urandom of=write-file bs=please-choose count=please-choose
mtd_debug write /dev/mtd-yours offset write-file-size write-file
mtd_debug read /dev/mtd-yours offset write-file-size read-file
sha1sum read-file write-file
2/ lock flash then try to erase/write it, to see if the lock works
flash_lock /dev/mtd-yours offset block-count
Do the read-erase-write-read-back test from 1/. The contents of
flash should not change in the erase and write steps.
3/ unlock flash and do the read-erase-write-read-back from 1/. The value of the
QEE should not change and you should be able to erase and write the flash.
Test 1/ should be successful.
v3: split patches, update retlen handling in sst_write.
v2:
- Introduce spi_nor_write_16bit_cr_and_check() as per Vignesh's suggestion. The
Configuration Register contains bits that can be updated in future: FREEZE,
CMP. Provide a generic method that allows updating all bits of the
Configuration Register.
- Fix SNOR_F_NO_READ_CR case in
"mtd: spi-nor: Rework the disabling of block write protection". When the flash
doesn't support the CR Read command, we make an assumption about the value of
the QE bit. In spi_nor_init(), call spi_nor_quad_enable() first, then
spi_nor_unlock_all(), so that at the spi_nor_unlock_all() time we can be sure
the QE bit has value one, because of the previous call to spi_nor_quad_enable().
- Fix if statement in spi_nor_write_sr_and_check():
if (nor->flags & SNOR_F_HAS_16BIT_SR)
- Fix documentation warnings.
- New patch: "mtd: spi-nor: Check all the bits written, not just the BP ones".
- Drop Global Unlock patches, will send them in a different patch set.
Tudor Ambarus (32):
mtd: spi-nor: Prepend spi_nor_ to all Reg Ops methods
mtd: spi-nor: Drop duplicated new line
mtd: spi-nor: Group all Reg Ops to avoid forward declarations
mtd: spi-nor: Stop compare with negative in Reg Ops methods
mtd: spi-nor: Drop explicit cast to int to already int value
mtd: spi-nor: Use dev_err() instead of pr_err()
mtd: spi-nor: Don't overwrite errno from Reg Ops
mtd: spi-nor: Pointer parameter for SR in spi_nor_read_sr()
mtd: spi-nor: Pointer parameter for FSR in spi_nor_read_fsr()
mtd: spi-nor: Pointer parameter for CR in spi_nor_read_cr()
mtd: spi-nor: Drop redundant error reports in Reg Ops callers
mtd: spi-nor: Void return type for spi_nor_clear_sr/fsr()
mtd: spi-nor: Print error messages inside Reg Ops methods
mtd: spi-nor: Fix retlen handling in sst_write()
mtd: spi-nor: Check for errors after each Register Operation
mtd: spi-nor: Rename label as it is no longer generic
mtd: spi-nor: Move the WE and wait calls inside Write SR methods
mtd: spi-nor: Constify data to write to the Status Register
mtd: spi-nor: Merge spi_nor_write_sr() and spi_nor_write_sr_cr()
mtd: spi-nor: Describe all the Reg Ops
mtd: spi-nor: Drop spansion_quad_enable()
mtd: spi-nor: Fix errno on Quad Enable methods
mtd: spi-nor: Check all the bits written, not just the BP ones
mtd: spi-nor: Print error message when the read back test fails
mtd: spi-nor: Fix clearing of QE bit on lock()/unlock()
mtd: spi-nor: Extend the QE Read Back test to the entire SR byte
mtd: spi-nor: Extend the QE Read Back test to both SR1 and SR2
mtd: spi-nor: Rename CR_QUAD_EN_SPAN to SR2_QUAD_EN_BIT1
mtd: spi-nor: Merge spansion Quad Enable methods
mtd: spi-nor: Rename macronix_quad_enable to
spi_nor_sr1_bit6_quad_enable
mtd: spi-nor: Prepend "spi_nor_" to "sr2_bit7_quad_enable"
mtd: spi-nor: Rework the disabling of block write protection
drivers/mtd/spi-nor/spi-nor.c | 1645 +++++++++++++++++++++++------------------
include/linux/mtd/spi-nor.h | 12 +-
2 files changed, 924 insertions(+), 733 deletions(-)
--
2.9.5
From: Tudor Ambarus <[email protected]>
spi_nor_clear_sr() and spi_nor_clear_fsr() are called just in case
of errors. The callers didn't check their return value, make them
of type void.
Signed-off-by: Tudor Ambarus <[email protected]>
---
drivers/mtd/spi-nor/spi-nor.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 4a1ecf452a39..e5ed9012cd50 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -654,7 +654,7 @@ static int s3an_sr_ready(struct spi_nor *nor)
return !!(nor->bouncebuf[0] & XSR_RDY);
}
-static int spi_nor_clear_sr(struct spi_nor *nor)
+static void spi_nor_clear_sr(struct spi_nor *nor)
{
if (nor->spimem) {
struct spi_mem_op op =
@@ -690,7 +690,7 @@ static int spi_nor_sr_ready(struct spi_nor *nor)
return !(nor->bouncebuf[0] & SR_WIP);
}
-static int spi_nor_clear_fsr(struct spi_nor *nor)
+static void spi_nor_clear_fsr(struct spi_nor *nor)
{
if (nor->spimem) {
struct spi_mem_op op =
--
2.9.5
From: Tudor Ambarus <[email protected]>
Check for the return vales of each Register Operation.
Signed-off-by: Tudor Ambarus <[email protected]>
---
drivers/mtd/spi-nor/spi-nor.c | 81 ++++++++++++++++++++++++++++++++-----------
1 file changed, 60 insertions(+), 21 deletions(-)
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 889fd77dbe96..21f01fdcfa16 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -595,11 +595,15 @@ static int st_micron_set_4byte(struct spi_nor *nor, bool enable)
{
int ret;
- spi_nor_write_enable(nor);
+ ret == spi_nor_write_enable(nor);
+ if (ret)
+ return ret;
+
ret = macronix_set_4byte(nor, enable);
- spi_nor_write_disable(nor);
+ if (ret)
+ return ret;
- return ret;
+ return spi_nor_write_disable(nor);
}
static int spansion_set_4byte(struct spi_nor *nor, bool enable)
@@ -665,11 +669,15 @@ static int winbond_set_4byte(struct spi_nor *nor, bool enable)
* Register to be set to 1, so all 3-byte-address reads come from the
* second 16M. We must clear the register to enable normal behavior.
*/
- spi_nor_write_enable(nor);
+ ret = spi_nor_write_enable(nor);
+ if (ret)
+ return ret;
+
ret = spi_nor_write_ear(nor, 0);
- spi_nor_write_disable(nor);
+ if (ret)
+ return ret;
- return ret;
+ return spi_nor_write_disable(nor);
}
static int spi_nor_xread_sr(struct spi_nor *nor, u8 *sr)
@@ -855,7 +863,9 @@ static int spi_nor_write_sr_cr(struct spi_nor *nor, u8 *sr_cr)
{
int ret;
- spi_nor_write_enable(nor);
+ ret = spi_nor_write_enable(nor);
+ if (ret)
+ return ret;
if (nor->spimem) {
struct spi_mem_op op =
@@ -885,7 +895,10 @@ static int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 status_new,
{
int ret;
- spi_nor_write_enable(nor);
+ ret = spi_nor_write_enable(nor);
+ if (ret)
+ return ret;
+
ret = spi_nor_write_sr(nor, status_new);
if (ret)
return ret;
@@ -1393,7 +1406,9 @@ static int spi_nor_erase_multi_sectors(struct spi_nor *nor, u64 addr, u32 len)
list_for_each_entry_safe(cmd, next, &erase_list, list) {
nor->erase_opcode = cmd->opcode;
while (cmd->count) {
- spi_nor_write_enable(nor);
+ ret = spi_nor_write_enable(nor);
+ if (ret)
+ goto destroy_erase_cmd_list;
ret = spi_nor_erase_sector(nor, addr);
if (ret)
@@ -1448,7 +1463,9 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)
if (len == mtd->size && !(nor->flags & SNOR_F_NO_OP_CHIP_ERASE)) {
unsigned long timeout;
- spi_nor_write_enable(nor);
+ ret = spi_nor_write_enable(nor);
+ if (ret)
+ goto erase_err;
ret = spi_nor_erase_chip(nor);
if (ret)
@@ -1475,7 +1492,9 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)
/* "sector"-at-a-time erase */
} else if (spi_nor_has_uniform_erase(nor)) {
while (len) {
- spi_nor_write_enable(nor);
+ ret = spi_nor_write_enable(nor);
+ if (ret)
+ goto erase_err;
ret = spi_nor_erase_sector(nor, addr);
if (ret)
@@ -1496,7 +1515,7 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)
goto erase_err;
}
- spi_nor_write_disable(nor);
+ ret = spi_nor_write_disable(nor);
erase_err:
spi_nor_unlock_and_unprep(nor, SPI_NOR_OPS_ERASE);
@@ -1845,9 +1864,13 @@ static int macronix_quad_enable(struct spi_nor *nor)
if (nor->bouncebuf[0] & SR_QUAD_EN_MX)
return 0;
- spi_nor_write_enable(nor);
+ ret = spi_nor_write_enable(nor);
+ if (ret)
+ return ret;
- spi_nor_write_sr(nor, nor->bouncebuf[0] | SR_QUAD_EN_MX);
+ ret = spi_nor_write_sr(nor, nor->bouncebuf[0] | SR_QUAD_EN_MX);
+ if (ret)
+ return ret;
ret = spi_nor_wait_till_ready(nor);
if (ret)
@@ -2018,7 +2041,9 @@ static int sr2_bit7_quad_enable(struct spi_nor *nor)
/* Update the Quad Enable bit. */
*sr2 |= SR2_QUAD_EN_BIT7;
- spi_nor_write_enable(nor);
+ ret = spi_nor_write_enable(nor);
+ if (ret)
+ return ret;
ret = spi_nor_write_sr2(nor, sr2);
if (ret)
@@ -2059,7 +2084,9 @@ static int spi_nor_clear_sr_bp(struct spi_nor *nor)
if (ret)
return ret;
- spi_nor_write_enable(nor);
+ ret = spi_nor_write_enable(nor);
+ if (ret)
+ return ret;
ret = spi_nor_write_sr(nor, nor->bouncebuf[0] & ~mask);
if (ret)
@@ -2676,7 +2703,9 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
if (ret)
return ret;
- spi_nor_write_enable(nor);
+ ret = spi_nor_write_enable(nor);
+ if (ret)
+ goto sst_write_err;
nor->sst_write_second = false;
@@ -2714,14 +2743,19 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
}
nor->sst_write_second = false;
- spi_nor_write_disable(nor);
+ ret = spi_nor_write_disable(nor);
+ if (ret)
+ goto sst_write_err;
+
ret = spi_nor_wait_till_ready(nor);
if (ret)
goto sst_write_err;
/* Write out trailing byte if it exists. */
if (actual != len) {
- spi_nor_write_enable(nor);
+ ret = spi_nor_write_enable(nor);
+ if (ret)
+ goto sst_write_err;
nor->program_opcode = SPINOR_OP_BP;
ret = spi_nor_write_data(nor, to, 1, buf + actual);
@@ -2731,8 +2765,10 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
ret = spi_nor_wait_till_ready(nor);
if (ret)
goto sst_write_err;
- spi_nor_write_disable(nor);
+
actual += 1;
+
+ ret = spi_nor_write_disable(nor);
}
sst_write_err:
*retlen += actual;
@@ -2783,7 +2819,10 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
addr = spi_nor_convert_addr(nor, addr);
- spi_nor_write_enable(nor);
+ ret = spi_nor_write_enable(nor);
+ if (ret)
+ goto write_err;
+
ret = spi_nor_write_data(nor, addr, page_remain, buf + i);
if (ret < 0)
goto write_err;
--
2.9.5
From: Tudor Ambarus <[email protected]>
Check that all the bits written in the write_sr_and_check() method
match the status_new received value. Failing to write the other bits
is dangerous too, extend the check.
Signed-off-by: Tudor Ambarus <[email protected]>
---
drivers/mtd/spi-nor/spi-nor.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 21a974b88c3b..5b30fc73fdba 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -960,8 +960,7 @@ static int spi_nor_write_sr(struct spi_nor *nor, const u8 *sr, size_t len)
}
/* Write status register and ensure bits in mask match written values */
-static int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 status_new,
- u8 mask)
+static int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 status_new)
{
int ret;
@@ -975,7 +974,7 @@ static int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 status_new,
if (ret)
return ret;
- return ((nor->bouncebuf[0] & mask) != (status_new & mask)) ? -EIO : 0;
+ return (nor->bouncebuf[0] != status_new) ? -EIO : 0;
}
/**
@@ -1774,7 +1773,7 @@ static int stm_lock(struct spi_nor *nor, loff_t ofs, uint64_t len)
if ((status_new & mask) < (status_old & mask))
return -EINVAL;
- return spi_nor_write_sr_and_check(nor, status_new, mask);
+ return spi_nor_write_sr_and_check(nor, status_new);
}
/*
@@ -1859,7 +1858,7 @@ static int stm_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len)
if ((status_new & mask) > (status_old & mask))
return -EINVAL;
- return spi_nor_write_sr_and_check(nor, status_new, mask);
+ return spi_nor_write_sr_and_check(nor, status_new);
}
/*
--
2.9.5
From: Tudor Ambarus <[email protected]>
Make sure that when doing a lock() or an unlock() operation we don't clear
the QE bit from Status Register 2.
JESD216 revB or later offers information about the *default* Status
Register commands to use (see BFPT DWORDS[15], bits 22:20). In this
standard, Status Register 1 refers to the first data byte transferred on a
Read Status (05h) or Write Status (01h) command. Status register 2 refers
to the byte read using instruction 35h. Status register 2 is the second
byte transferred in a Write Status (01h) command.
Industry naming and definitions of these Status Registers may differ.
The definitions are described in JESD216B, BFPT DWORDS[15], bits 22:20.
There are cases in which writing only one byte to the Status Register 1
has the side-effect of clearing Status Register 2 and implicitly the Quad
Enable bit. This side-effect is hit just by the
BFPT_DWORD15_QER_SR2_BIT1_BUGGY and BFPT_DWORD15_QER_SR2_BIT1 cases.
Suggested-by: Boris Brezillon <[email protected]>
Signed-off-by: Tudor Ambarus <[email protected]>
---
drivers/mtd/spi-nor/spi-nor.c | 120 ++++++++++++++++++++++++++++++++++++++++--
include/linux/mtd/spi-nor.h | 3 ++
2 files changed, 118 insertions(+), 5 deletions(-)
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 30ce83426266..0dcc4f12e4de 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -959,12 +959,19 @@ static int spi_nor_write_sr(struct spi_nor *nor, const u8 *sr, size_t len)
return spi_nor_wait_till_ready(nor);
}
-/* Write status register and ensure bits in mask match written values */
-static int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 status_new)
+/**
+ * spi_nor_write_sr1_and_check() - Write one byte to the Status Register 1 and
+ * ensure that the byte written match the received value.
+ * @nor: pointer to a 'struct spi_nor'.
+ * @sr1: byte value to be written to the Status Register.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_write_sr1_and_check(struct spi_nor *nor, u8 sr1)
{
int ret;
- nor->bouncebuf[0] = status_new;
+ nor->bouncebuf[0] = sr1;
ret = spi_nor_write_sr(nor, &nor->bouncebuf[0], 1);
if (ret)
@@ -974,8 +981,73 @@ static int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 status_new)
if (ret)
return ret;
- if (nor->bouncebuf[0] != status_new) {
- dev_err(nor->dev, "SR: read back test failed\n");
+ if (nor->bouncebuf[0] != sr1) {
+ dev_err(nor->dev, "SR1: read back test failed\n");
+ return -EIO;
+ }
+
+ return 0;
+}
+
+/**
+ * spi_nor_write_16bit_sr_and_check() - Write the Status Register 1 and the
+ * Status Register 2 in one shot. Ensure that the byte written in the Status
+ * Register 1 match the received value, and that the 16-bit Write did not
+ * affect what was already in the Status Register 2.
+ * @nor: pointer to a 'struct spi_nor'.
+ * @sr1: byte value to be written to the Status Register 1.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_write_16bit_sr_and_check(struct spi_nor *nor, u8 sr1)
+{
+ int ret;
+ u8 *sr_cr = nor->bouncebuf;
+ u8 cr_written;
+
+ /* Make sure we don't overwrite the contents of Status Register 2. */
+ if (!(nor->flags & SNOR_F_NO_READ_CR)) {
+ ret = spi_nor_read_cr(nor, &sr_cr[1]);
+ if (ret)
+ return ret;
+ } else if (nor->params.quad_enable) {
+ /*
+ * If the Status Register 2 Read command (35h) is not
+ * supported, we should at least be sure we don't
+ * change the value of the SR2 Quad Enable bit.
+ *
+ * We can safely assume that when the Quad Enable method is
+ * set, the value of the QE bit is one, as a consequence of the
+ * nor->params.quad_enable() call.
+ *
+ * We can safely assume that the Quad Enable bit is present in
+ * the Status Register 2 at BIT(1). According to the JESD216
+ * revB standard, BFPT DWORDS[15], bits 22:20, the 16-bit
+ * Write Status (01h) command is available just for the cases
+ * in which the QE bit is described in SR2 at BIT(1).
+ */
+ sr_cr[1] = CR_QUAD_EN_SPAN;
+ } else {
+ sr_cr[1] = 0;
+ }
+
+ sr_cr[0] = sr1;
+
+ ret = spi_nor_write_sr(nor, sr_cr, 2);
+ if (ret)
+ return ret;
+
+ if (nor->flags & SNOR_F_NO_READ_CR)
+ return 0;
+
+ cr_written = sr_cr[1];
+
+ ret = spi_nor_read_cr(nor, &sr_cr[1]);
+ if (ret)
+ return ret;
+
+ if (cr_written != sr_cr[1]) {
+ dev_err(nor->dev, "CR: read back test failed\n");
return -EIO;
}
@@ -983,6 +1055,23 @@ static int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 status_new)
}
/**
+ * spi_nor_write_sr_and_check() - Write the Status Register 1 and ensure that
+ * the byte written match the received value without affecting other bits in the
+ * Status Register 1 and 2.
+ * @nor: pointer to a 'struct spi_nor'.
+ * @sr1: byte value to be written to the Status Register.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 sr1)
+{
+ if (nor->flags & SNOR_F_HAS_16BIT_SR)
+ return spi_nor_write_16bit_sr_and_check(nor, sr1);
+
+ return spi_nor_write_sr1_and_check(nor, sr1);
+}
+
+/**
* spi_nor_write_sr2() - Write the Status Register 2 using the
* SPINOR_OP_WRSR2 (3eh) command.
* @nor: pointer to 'struct spi_nor'.
@@ -3634,19 +3723,38 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
break;
case BFPT_DWORD15_QER_SR2_BIT1_BUGGY:
+ /*
+ * Writing only one byte to the Status Register has the
+ * side-effect of clearing Status Register 2.
+ */
case BFPT_DWORD15_QER_SR2_BIT1_NO_RD:
+ /*
+ * Read Configuration Register (35h) instruction is not
+ * supported.
+ */
+ nor->flags |= SNOR_F_HAS_16BIT_SR | SNOR_F_NO_READ_CR;
params->quad_enable = spansion_no_read_cr_quad_enable;
break;
case BFPT_DWORD15_QER_SR1_BIT6:
+ nor->flags &= ~SNOR_F_HAS_16BIT_SR;
params->quad_enable = macronix_quad_enable;
break;
case BFPT_DWORD15_QER_SR2_BIT7:
+ nor->flags &= ~SNOR_F_HAS_16BIT_SR;
params->quad_enable = sr2_bit7_quad_enable;
break;
case BFPT_DWORD15_QER_SR2_BIT1:
+ /*
+ * JESD216 rev B or later does not specify if writing only one
+ * byte to the Status Register clears or not the Status
+ * Register 2, so let's be cautious and keep the default
+ * assumption of a 16-bit Write Status (01h) command.
+ */
+ nor->flags |= SNOR_F_HAS_16BIT_SR;
+
params->quad_enable = spansion_read_cr_quad_enable;
break;
@@ -4613,6 +4721,8 @@ static void spi_nor_info_init_params(struct spi_nor *nor)
params->quad_enable = spansion_read_cr_quad_enable;
params->set_4byte = spansion_set_4byte;
params->setup = spi_nor_default_setup;
+ /* Default to 16-bit Write Status (01h) Command */
+ nor->flags |= SNOR_F_HAS_16BIT_SR;
/* Set SPI NOR sizes. */
params->size = (u64)info->sector_size * info->n_sectors;
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index d1d736d3c8ab..d6ec55cc6d97 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -243,6 +243,9 @@ enum spi_nor_option_flags {
SNOR_F_4B_OPCODES = BIT(6),
SNOR_F_HAS_4BAIT = BIT(7),
SNOR_F_HAS_LOCK = BIT(8),
+ SNOR_F_HAS_16BIT_SR = BIT(9),
+ SNOR_F_NO_READ_CR = BIT(10),
+
};
/**
--
2.9.5
From: Tudor Ambarus <[email protected]>
Demystify where the EIO error occurs.
Signed-off-by: Tudor Ambarus <[email protected]>
---
drivers/mtd/spi-nor/spi-nor.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 5b30fc73fdba..30ce83426266 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -974,7 +974,12 @@ static int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 status_new)
if (ret)
return ret;
- return (nor->bouncebuf[0] != status_new) ? -EIO : 0;
+ if (nor->bouncebuf[0] != status_new) {
+ dev_err(nor->dev, "SR: read back test failed\n");
+ return -EIO;
+ }
+
+ return 0;
}
/**
--
2.9.5
From: Tudor Ambarus <[email protected]>
All SPI NOR generic methods should be prepended by "spi_nor_".
Signed-off-by: Tudor Ambarus <[email protected]>
---
drivers/mtd/spi-nor/spi-nor.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 47d159959461..87660b101c98 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -2129,7 +2129,7 @@ static int spi_nor_sr2_bit1_quad_enable(struct spi_nor *nor)
}
/**
- * sr2_bit7_quad_enable() - set QE bit in Status Register 2.
+ * spi_nor_sr2_bit7_quad_enable() - set QE bit in Status Register 2.
* @nor: pointer to a 'struct spi_nor'
*
* Set the Quad Enable (QE) bit in the Status Register 2.
@@ -2140,7 +2140,7 @@ static int spi_nor_sr2_bit1_quad_enable(struct spi_nor *nor)
*
* Return: 0 on success, -errno otherwise.
*/
-static int sr2_bit7_quad_enable(struct spi_nor *nor)
+static int spi_nor_sr2_bit7_quad_enable(struct spi_nor *nor)
{
u8 *sr2 = nor->bouncebuf;
int ret;
@@ -3733,7 +3733,7 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
case BFPT_DWORD15_QER_SR2_BIT7:
nor->flags &= ~SNOR_F_HAS_16BIT_SR;
- params->quad_enable = sr2_bit7_quad_enable;
+ params->quad_enable = spi_nor_sr2_bit7_quad_enable;
break;
case BFPT_DWORD15_QER_SR2_BIT1:
--
2.9.5
From: Tudor Ambarus <[email protected]>
Print identifying information about struct device.
Signed-off-by: Tudor Ambarus <[email protected]>
---
drivers/mtd/spi-nor/spi-nor.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index e801f390728c..c794eff69fe9 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -448,7 +448,7 @@ static int spi_nor_read_sr(struct spi_nor *nor)
}
if (ret) {
- pr_err("error %d reading SR\n", ret);
+ dev_err(nor->dev, "error %d reading SR\n", ret);
return ret;
}
@@ -478,7 +478,7 @@ static int spi_nor_read_fsr(struct spi_nor *nor)
}
if (ret) {
- pr_err("error %d reading FSR\n", ret);
+ dev_err(nor->dev, "error %d reading FSR\n", ret);
return ret;
}
--
2.9.5
On 10/29/2019 01:17 PM, Tudor Ambarus - M18064 wrote:
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -595,11 +595,15 @@ static int st_micron_set_4byte(struct spi_nor *nor, bool enable)
> {
> int ret;
>
> - spi_nor_write_enable(nor);
> + ret == spi_nor_write_enable(nor);
there's a typo here, it should have been initialization. Will respin if other
comments will arise.
On Tue, 29 Oct 2019 11:16:57 +0000
<[email protected]> wrote:
> From: Tudor Ambarus <[email protected]>
>
> Print identifying information about struct device.
>
> Signed-off-by: Tudor Ambarus <[email protected]>
> ---
> drivers/mtd/spi-nor/spi-nor.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index e801f390728c..c794eff69fe9 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -448,7 +448,7 @@ static int spi_nor_read_sr(struct spi_nor *nor)
> }
>
> if (ret) {
> - pr_err("error %d reading SR\n", ret);
> + dev_err(nor->dev, "error %d reading SR\n", ret);
nor->dev is not exactly what we should use since it points to the SPI
NOR controller device in the !SPI_MEM case, and those controllers can
be attached several SPI NOR devs. Ideally we should use mtd->dev, but
that requires splitting the MTD initialization and registration steps
so mtd->dev can have a valid name before registration time.
Anyway, I guess this change is good enough for now, just mentioned the
problem in case anyone is interested working on it.
Reviewed-by: Boris Brezillon <[email protected]>
> return ret;
> }
>
> @@ -478,7 +478,7 @@ static int spi_nor_read_fsr(struct spi_nor *nor)
> }
>
> if (ret) {
> - pr_err("error %d reading FSR\n", ret);
> + dev_err(nor->dev, "error %d reading FSR\n", ret);
> return ret;
> }
>
On Tue, 29 Oct 2019 11:17:07 +0000
<[email protected]> wrote:
> From: Tudor Ambarus <[email protected]>
>
> spi_nor_clear_sr() and spi_nor_clear_fsr() are called just in case
> of errors. The callers didn't check their return value, make them
> of type void.
>
> Signed-off-by: Tudor Ambarus <[email protected]>
Reviewed-by: Boris Brezillon <[email protected]>
> ---
> drivers/mtd/spi-nor/spi-nor.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 4a1ecf452a39..e5ed9012cd50 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -654,7 +654,7 @@ static int s3an_sr_ready(struct spi_nor *nor)
> return !!(nor->bouncebuf[0] & XSR_RDY);
> }
>
> -static int spi_nor_clear_sr(struct spi_nor *nor)
> +static void spi_nor_clear_sr(struct spi_nor *nor)
> {
> if (nor->spimem) {
> struct spi_mem_op op =
> @@ -690,7 +690,7 @@ static int spi_nor_sr_ready(struct spi_nor *nor)
> return !(nor->bouncebuf[0] & SR_WIP);
> }
>
> -static int spi_nor_clear_fsr(struct spi_nor *nor)
> +static void spi_nor_clear_fsr(struct spi_nor *nor)
> {
> if (nor->spimem) {
> struct spi_mem_op op =
On 10/29/2019 01:16 PM, Tudor Ambarus - M18064 wrote:
> From: Tudor Ambarus <[email protected]>
>
> Print identifying information about struct device.
>
> Signed-off-by: Tudor Ambarus <[email protected]>
> ---
> drivers/mtd/spi-nor/spi-nor.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Changed subject to "mtd: spi-nor: Print device info in case of error" and
applied to spi-nor/next. Thanks.