[PATCH v4 00/20] mtd: spi-nor: Quad Enable and (un)lock methods

[PATCH v4 00/20] mtd: spi-nor: Quad Enable and (un)lock methods

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

Tested on s25fl116k and w25q128jv-q.

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.

v4:
- Use dev_dbg insted of dev_err for low level info
- replace "&nor->bouncebuf[0]" with "nor->bouncebuf" and "&sr_cr[0]" with
  "sr_cr". Update across all patches.

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.

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.

Tudor Ambarus (20):
  mtd: spi-nor: Use dev_dbg insted of dev_err for low level info
  mtd: spi-nor: Print debug info inside Reg Ops methods
  mtd: spi-nor: Check for errors after each Register Operation
  mtd: spi-nor: Rename label as it is no longer generic
  mtd: spi-nor: Void return type for spi_nor_clear_sr/fsr()
  mtd: spi-nor: Move the WE and wait calls inside Write SR methods
  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 debug 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 | 952 +++++++++++++++++++++++++-----------------
 include/linux/mtd/spi-nor.h   |  12 +-
 2 files changed, 583 insertions(+), 381 deletions(-)

-- 
2.9.5


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

[PATCH v4 01/20] mtd: spi-nor: Use dev_dbg insted of dev_err for low level info

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

What most users care about is "my dev is not working properly".
All low level information should be discovered when activating
the debug traces.

Keep error messages for just three cases:
- when the JEDEC ID is not recognized
- when the resume() call fails
- when the spi_nor_check() fails.

Suggested-by: Boris Brezillon <boris.brezillon@collabora.com>
Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 52 +++++++++++++++++++++----------------------
 1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index f89620005198..eca6bce7c336 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -451,7 +451,7 @@ static int spi_nor_read_sr(struct spi_nor *nor, u8 *sr)
 	}
 
 	if (ret)
-		dev_err(nor->dev, "error %d reading SR\n", ret);
+		dev_dbg(nor->dev, "error %d reading SR\n", ret);
 
 	return ret;
 }
@@ -482,7 +482,7 @@ static int spi_nor_read_fsr(struct spi_nor *nor, u8 *fsr)
 	}
 
 	if (ret)
-		dev_err(nor->dev, "error %d reading FSR\n", ret);
+		dev_dbg(nor->dev, "error %d reading FSR\n", ret);
 
 	return ret;
 }
@@ -513,7 +513,7 @@ static int spi_nor_read_cr(struct spi_nor *nor, u8 *cr)
 	}
 
 	if (ret)
-		dev_err(nor->dev, "error %d reading CR\n", ret);
+		dev_dbg(nor->dev, "error %d reading CR\n", ret);
 
 	return ret;
 }
@@ -647,7 +647,7 @@ static int s3an_sr_ready(struct spi_nor *nor)
 
 	ret = spi_nor_xread_sr(nor, nor->bouncebuf);
 	if (ret) {
-		dev_err(nor->dev, "error %d reading XRDSR\n", ret);
+		dev_dbg(nor->dev, "error %d reading XRDSR\n", ret);
 		return ret;
 	}
 
@@ -679,9 +679,9 @@ static int spi_nor_sr_ready(struct spi_nor *nor)
 	if (nor->flags & SNOR_F_USE_CLSR &&
 	    nor->bouncebuf[0] & (SR_E_ERR | SR_P_ERR)) {
 		if (nor->bouncebuf[0] & SR_E_ERR)
-			dev_err(nor->dev, "Erase Error occurred\n");
+			dev_dbg(nor->dev, "Erase Error occurred\n");
 		else
-			dev_err(nor->dev, "Programming Error occurred\n");
+			dev_dbg(nor->dev, "Programming Error occurred\n");
 
 		spi_nor_clear_sr(nor);
 		return -EIO;
@@ -714,12 +714,12 @@ static int spi_nor_fsr_ready(struct spi_nor *nor)
 
 	if (nor->bouncebuf[0] & (FSR_E_ERR | FSR_P_ERR)) {
 		if (nor->bouncebuf[0] & FSR_E_ERR)
-			dev_err(nor->dev, "Erase operation failed.\n");
+			dev_dbg(nor->dev, "Erase operation failed.\n");
 		else
-			dev_err(nor->dev, "Program operation failed.\n");
+			dev_dbg(nor->dev, "Program operation failed.\n");
 
 		if (nor->bouncebuf[0] & FSR_PT_ERR)
-			dev_err(nor->dev,
+			dev_dbg(nor->dev,
 			"Attempted to modify a protected sector.\n");
 
 		spi_nor_clear_fsr(nor);
@@ -770,7 +770,7 @@ static int spi_nor_wait_till_ready_with_timeout(struct spi_nor *nor,
 		cond_resched();
 	}
 
-	dev_err(nor->dev, "flash operation timed out\n");
+	dev_dbg(nor->dev, "flash operation timed out\n");
 
 	return -ETIMEDOUT;
 }
@@ -807,7 +807,7 @@ static int spi_nor_write_sr_cr(struct spi_nor *nor, const u8 *sr_cr)
 	}
 
 	if (ret) {
-		dev_err(nor->dev,
+		dev_dbg(nor->dev,
 			"error while writing configuration register\n");
 		return -EINVAL;
 	}
@@ -1771,7 +1771,7 @@ static int macronix_quad_enable(struct spi_nor *nor)
 		return ret;
 
 	if (!(nor->bouncebuf[0] & SR_QUAD_EN_MX)) {
-		dev_err(nor->dev, "Macronix Quad bit not set\n");
+		dev_dbg(nor->dev, "Macronix Quad bit not set\n");
 		return -EINVAL;
 	}
 
@@ -1819,7 +1819,7 @@ static int spansion_quad_enable(struct spi_nor *nor)
 		return ret;
 
 	if (!(nor->bouncebuf[0] & CR_QUAD_EN_SPAN)) {
-		dev_err(nor->dev, "Spansion Quad bit not set\n");
+		dev_dbg(nor->dev, "Spansion Quad bit not set\n");
 		return -EINVAL;
 	}
 
@@ -1897,7 +1897,7 @@ static int spansion_read_cr_quad_enable(struct spi_nor *nor)
 		return ret;
 
 	if (!(sr_cr[1] & CR_QUAD_EN_SPAN)) {
-		dev_err(nor->dev, "Spansion Quad bit not set\n");
+		dev_dbg(nor->dev, "Spansion Quad bit not set\n");
 		return -EINVAL;
 	}
 
@@ -1935,7 +1935,7 @@ static int sr2_bit7_quad_enable(struct spi_nor *nor)
 
 	ret = spi_nor_write_sr2(nor, sr2);
 	if (ret) {
-		dev_err(nor->dev, "error while writing status register 2\n");
+		dev_dbg(nor->dev, "error while writing status register 2\n");
 		return ret;
 	}
 
@@ -1949,7 +1949,7 @@ static int sr2_bit7_quad_enable(struct spi_nor *nor)
 		return ret;
 
 	if (!(*sr2 & SR2_QUAD_EN_BIT7)) {
-		dev_err(nor->dev, "SR2 Quad bit not set\n");
+		dev_dbg(nor->dev, "SR2 Quad bit not set\n");
 		return -EINVAL;
 	}
 
@@ -1978,7 +1978,7 @@ static int spi_nor_clear_sr_bp(struct spi_nor *nor)
 
 	ret = spi_nor_write_sr(nor, nor->bouncebuf[0] & ~mask);
 	if (ret) {
-		dev_err(nor->dev, "write to status register failed\n");
+		dev_dbg(nor->dev, "write to status register failed\n");
 		return ret;
 	}
 
@@ -2525,7 +2525,7 @@ static const struct flash_info *spi_nor_read_id(struct spi_nor *nor)
 						    SPI_NOR_MAX_ID_LEN);
 	}
 	if (tmp) {
-		dev_err(nor->dev, "error %d reading JEDEC ID\n", tmp);
+		dev_dbg(nor->dev, "error %d reading JEDEC ID\n", tmp);
 		return ERR_PTR(tmp);
 	}
 
@@ -2740,7 +2740,7 @@ static int s3an_nor_setup(struct spi_nor *nor,
 
 	ret = spi_nor_xread_sr(nor, nor->bouncebuf);
 	if (ret) {
-		dev_err(nor->dev, "error %d reading XRDSR\n", ret);
+		dev_dbg(nor->dev, "error %d reading XRDSR\n", ret);
 		return ret;
 	}
 
@@ -4102,7 +4102,7 @@ static int spi_nor_parse_sfdp(struct spi_nor *nor,
 		err = spi_nor_read_sfdp(nor, sizeof(header),
 					psize, param_headers);
 		if (err < 0) {
-			dev_err(dev, "failed to read SFDP parameter headers\n");
+			dev_dbg(dev, "failed to read SFDP parameter headers\n");
 			goto exit;
 		}
 	}
@@ -4349,7 +4349,7 @@ static int spi_nor_default_setup(struct spi_nor *nor,
 	/* Select the (Fast) Read command. */
 	err = spi_nor_select_read(nor, shared_mask);
 	if (err) {
-		dev_err(nor->dev,
+		dev_dbg(nor->dev,
 			"can't select read settings supported by both the SPI controller and memory.\n");
 		return err;
 	}
@@ -4357,7 +4357,7 @@ static int spi_nor_default_setup(struct spi_nor *nor,
 	/* Select the Page Program command. */
 	err = spi_nor_select_pp(nor, shared_mask);
 	if (err) {
-		dev_err(nor->dev,
+		dev_dbg(nor->dev,
 			"can't select write settings supported by both the SPI controller and memory.\n");
 		return err;
 	}
@@ -4365,7 +4365,7 @@ static int spi_nor_default_setup(struct spi_nor *nor,
 	/* Select the Sector Erase command. */
 	err = spi_nor_select_erase(nor);
 	if (err) {
-		dev_err(nor->dev,
+		dev_dbg(nor->dev,
 			"can't select erase settings supported by both the SPI controller and memory.\n");
 		return err;
 	}
@@ -4686,7 +4686,7 @@ static int spi_nor_init(struct spi_nor *nor)
 
 		err = nor->clear_sr_bp(nor);
 		if (err) {
-			dev_err(nor->dev,
+			dev_dbg(nor->dev,
 				"fail to clear block protection bits\n");
 			return err;
 		}
@@ -4694,7 +4694,7 @@ static int spi_nor_init(struct spi_nor *nor)
 
 	err = spi_nor_quad_enable(nor);
 	if (err) {
-		dev_err(nor->dev, "quad mode not supported\n");
+		dev_dbg(nor->dev, "quad mode not supported\n");
 		return err;
 	}
 
@@ -4762,7 +4762,7 @@ static int spi_nor_set_addr_width(struct spi_nor *nor)
 	}
 
 	if (nor->addr_width > SPI_NOR_MAX_ADDR_WIDTH) {
-		dev_err(nor->dev, "address width is too large: %u\n",
+		dev_dbg(nor->dev, "address width is too large: %u\n",
 			nor->addr_width);
 		return -EINVAL;
 	}
-- 
2.9.5


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

Re: [PATCH v4 01/20] mtd: spi-nor: Use dev_dbg insted of dev_err for low level info

[Vignesh Raghavendra]

Hi,

On 02/11/19 4:53 PM, Tudor.Ambarus@microchip.com wrote:
> From: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
> What most users care about is "my dev is not working properly".
> All low level information should be discovered when activating
> the debug traces.
> 
> Keep error messages for just three cases:
> - when the JEDEC ID is not recognized
> - when the resume() call fails
> - when the spi_nor_check() fails.
> 
> Suggested-by: Boris Brezillon <boris.brezillon@collabora.com>
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 52 +++++++++++++++++++++----------------------
>  1 file changed, 26 insertions(+), 26 deletions(-)
> 
[...]
>  
> @@ -679,9 +679,9 @@ static int spi_nor_sr_ready(struct spi_nor *nor)
>  	if (nor->flags & SNOR_F_USE_CLSR &&
>  	    nor->bouncebuf[0] & (SR_E_ERR | SR_P_ERR)) {
>  		if (nor->bouncebuf[0] & SR_E_ERR)
> -			dev_err(nor->dev, "Erase Error occurred\n");
> +			dev_dbg(nor->dev, "Erase Error occurred\n");
>  		else
> -			dev_err(nor->dev, "Programming Error occurred\n");
> +			dev_dbg(nor->dev, "Programming Error occurred\n");
>  
>  		spi_nor_clear_sr(nor);
>  		return -EIO;
> @@ -714,12 +714,12 @@ static int spi_nor_fsr_ready(struct spi_nor *nor)
>  
>  	if (nor->bouncebuf[0] & (FSR_E_ERR | FSR_P_ERR)) {
>  		if (nor->bouncebuf[0] & FSR_E_ERR)
> -			dev_err(nor->dev, "Erase operation failed.\n");
> +			dev_dbg(nor->dev, "Erase operation failed.\n");
>  		else
> -			dev_err(nor->dev, "Program operation failed.\n");
> +			dev_dbg(nor->dev, "Program operation failed.\n");
>  
>  		if (nor->bouncebuf[0] & FSR_PT_ERR)
> -			dev_err(nor->dev,
> +			dev_dbg(nor->dev,
>  			"Attempted to modify a protected sector.\n");
> 

Since, we are specifically parsing FSR bits to know the reason for
failure, I think we should use dev_err()s here.
I specifically like the last one which informs the user that
program/erase operation failed as sector was write protected.

Rest looks fine to me:

Reviewed-by: Vignesh Raghavendra <vigneshr@ti.com>

Regards
Vignesh

>  		spi_nor_clear_fsr(nor);
> @@ -770,7 +770,7 @@ static int spi_nor_wait_till_ready_with_timeout(struct spi_nor *nor,
>  		cond_resched();
>  	}
>  
> -	dev_err(nor->dev, "flash operation timed out\n");
> +	dev_dbg(nor->dev, "flash operation timed out\n");
>  
>  	return -ETIMEDOUT;
>  }
> @@ -807,7 +807,7 @@ static int spi_nor_write_sr_cr(struct spi_nor *nor, const u8 *sr_cr)
>  	}
>  
>  	if (ret) {
> -		dev_err(nor->dev,
> +		dev_dbg(nor->dev,
>  			"error while writing configuration register\n");
>  		return -EINVAL;
>  	}
> @@ -1771,7 +1771,7 @@ static int macronix_quad_enable(struct spi_nor *nor)
>  		return ret;
>  
>  	if (!(nor->bouncebuf[0] & SR_QUAD_EN_MX)) {
> -		dev_err(nor->dev, "Macronix Quad bit not set\n");
> +		dev_dbg(nor->dev, "Macronix Quad bit not set\n");
>  		return -EINVAL;
>  	}
>  
> @@ -1819,7 +1819,7 @@ static int spansion_quad_enable(struct spi_nor *nor)
>  		return ret;
>  
>  	if (!(nor->bouncebuf[0] & CR_QUAD_EN_SPAN)) {
> -		dev_err(nor->dev, "Spansion Quad bit not set\n");
> +		dev_dbg(nor->dev, "Spansion Quad bit not set\n");
>  		return -EINVAL;
>  	}
>  
> @@ -1897,7 +1897,7 @@ static int spansion_read_cr_quad_enable(struct spi_nor *nor)
>  		return ret;
>  
>  	if (!(sr_cr[1] & CR_QUAD_EN_SPAN)) {
> -		dev_err(nor->dev, "Spansion Quad bit not set\n");
> +		dev_dbg(nor->dev, "Spansion Quad bit not set\n");
>  		return -EINVAL;
>  	}
>  
> @@ -1935,7 +1935,7 @@ static int sr2_bit7_quad_enable(struct spi_nor *nor)
>  
>  	ret = spi_nor_write_sr2(nor, sr2);
>  	if (ret) {
> -		dev_err(nor->dev, "error while writing status register 2\n");
> +		dev_dbg(nor->dev, "error while writing status register 2\n");
>  		return ret;
>  	}
>  
> @@ -1949,7 +1949,7 @@ static int sr2_bit7_quad_enable(struct spi_nor *nor)
>  		return ret;
>  
>  	if (!(*sr2 & SR2_QUAD_EN_BIT7)) {
> -		dev_err(nor->dev, "SR2 Quad bit not set\n");
> +		dev_dbg(nor->dev, "SR2 Quad bit not set\n");
>  		return -EINVAL;
>  	}
>  
> @@ -1978,7 +1978,7 @@ static int spi_nor_clear_sr_bp(struct spi_nor *nor)
>  
>  	ret = spi_nor_write_sr(nor, nor->bouncebuf[0] & ~mask);
>  	if (ret) {
> -		dev_err(nor->dev, "write to status register failed\n");
> +		dev_dbg(nor->dev, "write to status register failed\n");
>  		return ret;
>  	}
>  
> @@ -2525,7 +2525,7 @@ static const struct flash_info *spi_nor_read_id(struct spi_nor *nor)
>  						    SPI_NOR_MAX_ID_LEN);
>  	}
>  	if (tmp) {
> -		dev_err(nor->dev, "error %d reading JEDEC ID\n", tmp);
> +		dev_dbg(nor->dev, "error %d reading JEDEC ID\n", tmp);
>  		return ERR_PTR(tmp);
>  	}
>  
> @@ -2740,7 +2740,7 @@ static int s3an_nor_setup(struct spi_nor *nor,
>  
>  	ret = spi_nor_xread_sr(nor, nor->bouncebuf);
>  	if (ret) {
> -		dev_err(nor->dev, "error %d reading XRDSR\n", ret);
> +		dev_dbg(nor->dev, "error %d reading XRDSR\n", ret);
>  		return ret;
>  	}
>  
> @@ -4102,7 +4102,7 @@ static int spi_nor_parse_sfdp(struct spi_nor *nor,
>  		err = spi_nor_read_sfdp(nor, sizeof(header),
>  					psize, param_headers);
>  		if (err < 0) {
> -			dev_err(dev, "failed to read SFDP parameter headers\n");
> +			dev_dbg(dev, "failed to read SFDP parameter headers\n");
>  			goto exit;
>  		}
>  	}
> @@ -4349,7 +4349,7 @@ static int spi_nor_default_setup(struct spi_nor *nor,
>  	/* Select the (Fast) Read command. */
>  	err = spi_nor_select_read(nor, shared_mask);
>  	if (err) {
> -		dev_err(nor->dev,
> +		dev_dbg(nor->dev,
>  			"can't select read settings supported by both the SPI controller and memory.\n");
>  		return err;
>  	}
> @@ -4357,7 +4357,7 @@ static int spi_nor_default_setup(struct spi_nor *nor,
>  	/* Select the Page Program command. */
>  	err = spi_nor_select_pp(nor, shared_mask);
>  	if (err) {
> -		dev_err(nor->dev,
> +		dev_dbg(nor->dev,
>  			"can't select write settings supported by both the SPI controller and memory.\n");
>  		return err;
>  	}
> @@ -4365,7 +4365,7 @@ static int spi_nor_default_setup(struct spi_nor *nor,
>  	/* Select the Sector Erase command. */
>  	err = spi_nor_select_erase(nor);
>  	if (err) {
> -		dev_err(nor->dev,
> +		dev_dbg(nor->dev,
>  			"can't select erase settings supported by both the SPI controller and memory.\n");
>  		return err;
>  	}
> @@ -4686,7 +4686,7 @@ static int spi_nor_init(struct spi_nor *nor)
>  
>  		err = nor->clear_sr_bp(nor);
>  		if (err) {
> -			dev_err(nor->dev,
> +			dev_dbg(nor->dev,
>  				"fail to clear block protection bits\n");
>  			return err;
>  		}
> @@ -4694,7 +4694,7 @@ static int spi_nor_init(struct spi_nor *nor)
>  
>  	err = spi_nor_quad_enable(nor);
>  	if (err) {
> -		dev_err(nor->dev, "quad mode not supported\n");
> +		dev_dbg(nor->dev, "quad mode not supported\n");
>  		return err;
>  	}
>  
> @@ -4762,7 +4762,7 @@ static int spi_nor_set_addr_width(struct spi_nor *nor)
>  	}
>  
>  	if (nor->addr_width > SPI_NOR_MAX_ADDR_WIDTH) {
> -		dev_err(nor->dev, "address width is too large: %u\n",
> +		dev_dbg(nor->dev, "address width is too large: %u\n",
>  			nor->addr_width);
>  		return -EINVAL;
>  	}
> 

-- 
Regards
Vignesh

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

Re: [PATCH v4 01/20] mtd: spi-nor: Use dev_dbg insted of dev_err for low level info

[Tudor.Ambarus]

On 11/05/2019 02:12 PM, Vignesh Raghavendra wrote:
> External E-Mail
> 
> 
> Hi,
> 
> On 02/11/19 4:53 PM, Tudor.Ambarus@microchip.com wrote:
>> From: Tudor Ambarus <tudor.ambarus@microchip.com>
>>
>> What most users care about is "my dev is not working properly".
>> All low level information should be discovered when activating
>> the debug traces.
>>
>> Keep error messages for just three cases:
>> - when the JEDEC ID is not recognized
>> - when the resume() call fails
>> - when the spi_nor_check() fails.
>>
>> Suggested-by: Boris Brezillon <boris.brezillon@collabora.com>
>> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>> ---
>>  drivers/mtd/spi-nor/spi-nor.c | 52 +++++++++++++++++++++----------------------
>>  1 file changed, 26 insertions(+), 26 deletions(-)
>>
> [...]
>>  
>> @@ -679,9 +679,9 @@ static int spi_nor_sr_ready(struct spi_nor *nor)
>>  	if (nor->flags & SNOR_F_USE_CLSR &&
>>  	    nor->bouncebuf[0] & (SR_E_ERR | SR_P_ERR)) {
>>  		if (nor->bouncebuf[0] & SR_E_ERR)
>> -			dev_err(nor->dev, "Erase Error occurred\n");
>> +			dev_dbg(nor->dev, "Erase Error occurred\n");
>>  		else
>> -			dev_err(nor->dev, "Programming Error occurred\n");
>> +			dev_dbg(nor->dev, "Programming Error occurred\n");
>>  
>>  		spi_nor_clear_sr(nor);
>>  		return -EIO;
>> @@ -714,12 +714,12 @@ static int spi_nor_fsr_ready(struct spi_nor *nor)
>>  
>>  	if (nor->bouncebuf[0] & (FSR_E_ERR | FSR_P_ERR)) {
>>  		if (nor->bouncebuf[0] & FSR_E_ERR)
>> -			dev_err(nor->dev, "Erase operation failed.\n");
>> +			dev_dbg(nor->dev, "Erase operation failed.\n");
>>  		else
>> -			dev_err(nor->dev, "Program operation failed.\n");
>> +			dev_dbg(nor->dev, "Program operation failed.\n");
>>  
>>  		if (nor->bouncebuf[0] & FSR_PT_ERR)
>> -			dev_err(nor->dev,
>> +			dev_dbg(nor->dev,
>>  			"Attempted to modify a protected sector.\n");
>>
> 
> Since, we are specifically parsing FSR bits to know the reason for
> failure, I think we should use dev_err()s here.
> I specifically like the last one which informs the user that
> program/erase operation failed as sector was write protected.
> 

Yep, will use dev_err to report erase/program errors in both spi_nor_sr_ready()
and spi_nor_fsr_ready(), together with the attempt of modifying a protected sector.

Thanks!

> Rest looks fine to me:
> 
> Reviewed-by: Vignesh Raghavendra <vigneshr@ti.com>
> 
> Regards
> Vignesh
> 
>>  		spi_nor_clear_fsr(nor);
>> @@ -770,7 +770,7 @@ static int spi_nor_wait_till_ready_with_timeout(struct spi_nor *nor,
>>  		cond_resched();
>>  	}
>>  
>> -	dev_err(nor->dev, "flash operation timed out\n");
>> +	dev_dbg(nor->dev, "flash operation timed out\n");
>>  
>>  	return -ETIMEDOUT;
>>  }
>> @@ -807,7 +807,7 @@ static int spi_nor_write_sr_cr(struct spi_nor *nor, const u8 *sr_cr)
>>  	}
>>  
>>  	if (ret) {
>> -		dev_err(nor->dev,
>> +		dev_dbg(nor->dev,
>>  			"error while writing configuration register\n");
>>  		return -EINVAL;
>>  	}
>> @@ -1771,7 +1771,7 @@ static int macronix_quad_enable(struct spi_nor *nor)
>>  		return ret;
>>  
>>  	if (!(nor->bouncebuf[0] & SR_QUAD_EN_MX)) {
>> -		dev_err(nor->dev, "Macronix Quad bit not set\n");
>> +		dev_dbg(nor->dev, "Macronix Quad bit not set\n");
>>  		return -EINVAL;
>>  	}
>>  
>> @@ -1819,7 +1819,7 @@ static int spansion_quad_enable(struct spi_nor *nor)
>>  		return ret;
>>  
>>  	if (!(nor->bouncebuf[0] & CR_QUAD_EN_SPAN)) {
>> -		dev_err(nor->dev, "Spansion Quad bit not set\n");
>> +		dev_dbg(nor->dev, "Spansion Quad bit not set\n");
>>  		return -EINVAL;
>>  	}
>>  
>> @@ -1897,7 +1897,7 @@ static int spansion_read_cr_quad_enable(struct spi_nor *nor)
>>  		return ret;
>>  
>>  	if (!(sr_cr[1] & CR_QUAD_EN_SPAN)) {
>> -		dev_err(nor->dev, "Spansion Quad bit not set\n");
>> +		dev_dbg(nor->dev, "Spansion Quad bit not set\n");
>>  		return -EINVAL;
>>  	}
>>  
>> @@ -1935,7 +1935,7 @@ static int sr2_bit7_quad_enable(struct spi_nor *nor)
>>  
>>  	ret = spi_nor_write_sr2(nor, sr2);
>>  	if (ret) {
>> -		dev_err(nor->dev, "error while writing status register 2\n");
>> +		dev_dbg(nor->dev, "error while writing status register 2\n");
>>  		return ret;
>>  	}
>>  
>> @@ -1949,7 +1949,7 @@ static int sr2_bit7_quad_enable(struct spi_nor *nor)
>>  		return ret;
>>  
>>  	if (!(*sr2 & SR2_QUAD_EN_BIT7)) {
>> -		dev_err(nor->dev, "SR2 Quad bit not set\n");
>> +		dev_dbg(nor->dev, "SR2 Quad bit not set\n");
>>  		return -EINVAL;
>>  	}
>>  
>> @@ -1978,7 +1978,7 @@ static int spi_nor_clear_sr_bp(struct spi_nor *nor)
>>  
>>  	ret = spi_nor_write_sr(nor, nor->bouncebuf[0] & ~mask);
>>  	if (ret) {
>> -		dev_err(nor->dev, "write to status register failed\n");
>> +		dev_dbg(nor->dev, "write to status register failed\n");
>>  		return ret;
>>  	}
>>  
>> @@ -2525,7 +2525,7 @@ static const struct flash_info *spi_nor_read_id(struct spi_nor *nor)
>>  						    SPI_NOR_MAX_ID_LEN);
>>  	}
>>  	if (tmp) {
>> -		dev_err(nor->dev, "error %d reading JEDEC ID\n", tmp);
>> +		dev_dbg(nor->dev, "error %d reading JEDEC ID\n", tmp);
>>  		return ERR_PTR(tmp);
>>  	}
>>  
>> @@ -2740,7 +2740,7 @@ static int s3an_nor_setup(struct spi_nor *nor,
>>  
>>  	ret = spi_nor_xread_sr(nor, nor->bouncebuf);
>>  	if (ret) {
>> -		dev_err(nor->dev, "error %d reading XRDSR\n", ret);
>> +		dev_dbg(nor->dev, "error %d reading XRDSR\n", ret);
>>  		return ret;
>>  	}
>>  
>> @@ -4102,7 +4102,7 @@ static int spi_nor_parse_sfdp(struct spi_nor *nor,
>>  		err = spi_nor_read_sfdp(nor, sizeof(header),
>>  					psize, param_headers);
>>  		if (err < 0) {
>> -			dev_err(dev, "failed to read SFDP parameter headers\n");
>> +			dev_dbg(dev, "failed to read SFDP parameter headers\n");
>>  			goto exit;
>>  		}
>>  	}
>> @@ -4349,7 +4349,7 @@ static int spi_nor_default_setup(struct spi_nor *nor,
>>  	/* Select the (Fast) Read command. */
>>  	err = spi_nor_select_read(nor, shared_mask);
>>  	if (err) {
>> -		dev_err(nor->dev,
>> +		dev_dbg(nor->dev,
>>  			"can't select read settings supported by both the SPI controller and memory.\n");
>>  		return err;
>>  	}
>> @@ -4357,7 +4357,7 @@ static int spi_nor_default_setup(struct spi_nor *nor,
>>  	/* Select the Page Program command. */
>>  	err = spi_nor_select_pp(nor, shared_mask);
>>  	if (err) {
>> -		dev_err(nor->dev,
>> +		dev_dbg(nor->dev,
>>  			"can't select write settings supported by both the SPI controller and memory.\n");
>>  		return err;
>>  	}
>> @@ -4365,7 +4365,7 @@ static int spi_nor_default_setup(struct spi_nor *nor,
>>  	/* Select the Sector Erase command. */
>>  	err = spi_nor_select_erase(nor);
>>  	if (err) {
>> -		dev_err(nor->dev,
>> +		dev_dbg(nor->dev,
>>  			"can't select erase settings supported by both the SPI controller and memory.\n");
>>  		return err;
>>  	}
>> @@ -4686,7 +4686,7 @@ static int spi_nor_init(struct spi_nor *nor)
>>  
>>  		err = nor->clear_sr_bp(nor);
>>  		if (err) {
>> -			dev_err(nor->dev,
>> +			dev_dbg(nor->dev,
>>  				"fail to clear block protection bits\n");
>>  			return err;
>>  		}
>> @@ -4694,7 +4694,7 @@ static int spi_nor_init(struct spi_nor *nor)
>>  
>>  	err = spi_nor_quad_enable(nor);
>>  	if (err) {
>> -		dev_err(nor->dev, "quad mode not supported\n");
>> +		dev_dbg(nor->dev, "quad mode not supported\n");
>>  		return err;
>>  	}
>>  
>> @@ -4762,7 +4762,7 @@ static int spi_nor_set_addr_width(struct spi_nor *nor)
>>  	}
>>  
>>  	if (nor->addr_width > SPI_NOR_MAX_ADDR_WIDTH) {
>> -		dev_err(nor->dev, "address width is too large: %u\n",
>> +		dev_dbg(nor->dev, "address width is too large: %u\n",
>>  			nor->addr_width);
>>  		return -EINVAL;
>>  	}
>>
> 
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v4 02/20] mtd: spi-nor: Print debug info inside Reg Ops methods

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

Spare the callers of printing debug messages by themselves.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 169 +++++++++++++++++++++++++++++++-----------
 1 file changed, 127 insertions(+), 42 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index eca6bce7c336..0cb3122e74ad 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -394,6 +394,8 @@ static ssize_t spi_nor_write_data(struct spi_nor *nor, loff_t to, size_t len,
  */
 static int spi_nor_write_enable(struct spi_nor *nor)
 {
+	int ret;
+
 	if (nor->spimem) {
 		struct spi_mem_op op =
 			SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WREN, 1),
@@ -401,10 +403,16 @@ static int spi_nor_write_enable(struct spi_nor *nor)
 				   SPI_MEM_OP_NO_DUMMY,
 				   SPI_MEM_OP_NO_DATA);
 
-		return spi_mem_exec_op(nor->spimem, &op);
+		ret = spi_mem_exec_op(nor->spimem, &op);
+	} else {
+		ret = nor->controller_ops->write_reg(nor, SPINOR_OP_WREN,
+						     NULL, 0);
 	}
 
-	return nor->controller_ops->write_reg(nor, SPINOR_OP_WREN, NULL, 0);
+	if (ret)
+		dev_dbg(nor->dev, "error %d on Write Enable\n", ret);
+
+	return ret;
 }
 
 /*
@@ -412,6 +420,8 @@ static int spi_nor_write_enable(struct spi_nor *nor)
  */
 static int spi_nor_write_disable(struct spi_nor *nor)
 {
+	int ret;
+
 	if (nor->spimem) {
 		struct spi_mem_op op =
 			SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRDI, 1),
@@ -419,10 +429,16 @@ static int spi_nor_write_disable(struct spi_nor *nor)
 				   SPI_MEM_OP_NO_DUMMY,
 				   SPI_MEM_OP_NO_DATA);
 
-		return spi_mem_exec_op(nor->spimem, &op);
+		ret = spi_mem_exec_op(nor->spimem, &op);
+	} else {
+		ret = nor->controller_ops->write_reg(nor, SPINOR_OP_WRDI,
+						     NULL, 0);
 	}
 
-	return nor->controller_ops->write_reg(nor, SPINOR_OP_WRDI, NULL, 0);
+	if (ret)
+		dev_dbg(nor->dev, "error %d on Write Disable\n", ret);
+
+	return ret;
 }
 
 /**
@@ -524,6 +540,8 @@ static int spi_nor_read_cr(struct spi_nor *nor, u8 *cr)
  */
 static int spi_nor_write_sr(struct spi_nor *nor, u8 val)
 {
+	int ret;
+
 	nor->bouncebuf[0] = val;
 	if (nor->spimem) {
 		struct spi_mem_op op =
@@ -532,15 +550,23 @@ static int spi_nor_write_sr(struct spi_nor *nor, u8 val)
 				   SPI_MEM_OP_NO_DUMMY,
 				   SPI_MEM_OP_DATA_OUT(1, nor->bouncebuf, 1));
 
-		return spi_mem_exec_op(nor->spimem, &op);
+		ret = spi_mem_exec_op(nor->spimem, &op);
+	} else {
+		ret = nor->controller_ops->write_reg(nor, SPINOR_OP_WRSR,
+						     nor->bouncebuf, 1);
 	}
 
-	return nor->controller_ops->write_reg(nor, SPINOR_OP_WRSR,
-					      nor->bouncebuf, 1);
+	if (ret)
+		dev_dbg(nor->dev, "error %d writing SR\n", ret);
+
+	return ret;
+
 }
 
 static int macronix_set_4byte(struct spi_nor *nor, bool enable)
 {
+	int ret;
+
 	if (nor->spimem) {
 		struct spi_mem_op op =
 			SPI_MEM_OP(SPI_MEM_OP_CMD(enable ?
@@ -551,12 +577,18 @@ static int macronix_set_4byte(struct spi_nor *nor, bool enable)
 				  SPI_MEM_OP_NO_DUMMY,
 				  SPI_MEM_OP_NO_DATA);
 
-		return spi_mem_exec_op(nor->spimem, &op);
+		ret = spi_mem_exec_op(nor->spimem, &op);
+	} else {
+		ret = nor->controller_ops->write_reg(nor,
+						     enable ? SPINOR_OP_EN4B :
+							      SPINOR_OP_EX4B,
+						     NULL, 0);
 	}
 
-	return nor->controller_ops->write_reg(nor, enable ? SPINOR_OP_EN4B :
-							    SPINOR_OP_EX4B,
-					      NULL, 0);
+	if (ret)
+		dev_dbg(nor->dev, "error %d setting 4-byte mode\n", ret);
+
+	return ret;
 }
 
 static int st_micron_set_4byte(struct spi_nor *nor, bool enable)
@@ -572,6 +604,8 @@ static int st_micron_set_4byte(struct spi_nor *nor, bool enable)
 
 static int spansion_set_4byte(struct spi_nor *nor, bool enable)
 {
+	int ret;
+
 	nor->bouncebuf[0] = enable << 7;
 
 	if (nor->spimem) {
@@ -581,15 +615,22 @@ static int spansion_set_4byte(struct spi_nor *nor, bool enable)
 				   SPI_MEM_OP_NO_DUMMY,
 				   SPI_MEM_OP_DATA_OUT(1, nor->bouncebuf, 1));
 
-		return spi_mem_exec_op(nor->spimem, &op);
+		ret = spi_mem_exec_op(nor->spimem, &op);
+	} else {
+		ret = nor->controller_ops->write_reg(nor, SPINOR_OP_BRWR,
+						     nor->bouncebuf, 1);
 	}
 
-	return nor->controller_ops->write_reg(nor, SPINOR_OP_BRWR,
-					      nor->bouncebuf, 1);
+	if (ret)
+		dev_dbg(nor->dev, "error %d setting 4-byte mode\n", ret);
+
+	return ret;
 }
 
 static int spi_nor_write_ear(struct spi_nor *nor, u8 ear)
 {
+	int ret;
+
 	nor->bouncebuf[0] = ear;
 
 	if (nor->spimem) {
@@ -599,11 +640,16 @@ static int spi_nor_write_ear(struct spi_nor *nor, u8 ear)
 				   SPI_MEM_OP_NO_DUMMY,
 				   SPI_MEM_OP_DATA_OUT(1, nor->bouncebuf, 1));
 
-		return spi_mem_exec_op(nor->spimem, &op);
+		ret = spi_mem_exec_op(nor->spimem, &op);
+	} else {
+		ret = nor->controller_ops->write_reg(nor, SPINOR_OP_WREAR,
+						     nor->bouncebuf, 1);
 	}
 
-	return nor->controller_ops->write_reg(nor, SPINOR_OP_WREAR,
-					      nor->bouncebuf, 1);
+	if (ret)
+		dev_dbg(nor->dev, "error %d writing EAR\n", ret);
+
+	return ret;
 }
 
 static int winbond_set_4byte(struct spi_nor *nor, bool enable)
@@ -628,6 +674,8 @@ static int winbond_set_4byte(struct spi_nor *nor, bool enable)
 
 static int spi_nor_xread_sr(struct spi_nor *nor, u8 *sr)
 {
+	int ret;
+
 	if (nor->spimem) {
 		struct spi_mem_op op =
 			SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_XRDSR, 1),
@@ -635,10 +683,16 @@ static int spi_nor_xread_sr(struct spi_nor *nor, u8 *sr)
 				   SPI_MEM_OP_NO_DUMMY,
 				   SPI_MEM_OP_DATA_IN(1, sr, 1));
 
-		return spi_mem_exec_op(nor->spimem, &op);
+		ret = spi_mem_exec_op(nor->spimem, &op);
+	} else {
+		ret = nor->controller_ops->read_reg(nor, SPINOR_OP_XRDSR,
+						    sr, 1);
 	}
 
-	return nor->controller_ops->read_reg(nor, SPINOR_OP_XRDSR, sr, 1);
+	if (ret)
+		dev_dbg(nor->dev, "error %d reading XRDSR\n", ret);
+
+	return ret;
 }
 
 static int s3an_sr_ready(struct spi_nor *nor)
@@ -646,16 +700,16 @@ static int s3an_sr_ready(struct spi_nor *nor)
 	int ret;
 
 	ret = spi_nor_xread_sr(nor, nor->bouncebuf);
-	if (ret) {
-		dev_dbg(nor->dev, "error %d reading XRDSR\n", ret);
+	if (ret)
 		return ret;
-	}
 
 	return !!(nor->bouncebuf[0] & XSR_RDY);
 }
 
 static int spi_nor_clear_sr(struct spi_nor *nor)
 {
+	int ret;
+
 	if (nor->spimem) {
 		struct spi_mem_op op =
 			SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_CLSR, 1),
@@ -663,10 +717,16 @@ static int spi_nor_clear_sr(struct spi_nor *nor)
 				   SPI_MEM_OP_NO_DUMMY,
 				   SPI_MEM_OP_NO_DATA);
 
-		return spi_mem_exec_op(nor->spimem, &op);
+		ret = spi_mem_exec_op(nor->spimem, &op);
+	} else {
+		ret = nor->controller_ops->write_reg(nor, SPINOR_OP_CLSR,
+						     NULL, 0);
 	}
 
-	return nor->controller_ops->write_reg(nor, SPINOR_OP_CLSR, NULL, 0);
+	if (ret)
+		dev_dbg(nor->dev, "error %d clearing SR\n", ret);
+
+	return ret;
 }
 
 static int spi_nor_sr_ready(struct spi_nor *nor)
@@ -692,6 +752,8 @@ static int spi_nor_sr_ready(struct spi_nor *nor)
 
 static int spi_nor_clear_fsr(struct spi_nor *nor)
 {
+	int ret;
+
 	if (nor->spimem) {
 		struct spi_mem_op op =
 			SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_CLFSR, 1),
@@ -699,10 +761,16 @@ static int spi_nor_clear_fsr(struct spi_nor *nor)
 				   SPI_MEM_OP_NO_DUMMY,
 				   SPI_MEM_OP_NO_DATA);
 
-		return spi_mem_exec_op(nor->spimem, &op);
+		ret = spi_mem_exec_op(nor->spimem, &op);
+	} else {
+		ret = nor->controller_ops->write_reg(nor, SPINOR_OP_CLFSR,
+						     NULL, 0);
 	}
 
-	return nor->controller_ops->write_reg(nor, SPINOR_OP_CLFSR, NULL, 0);
+	if (ret)
+		dev_dbg(nor->dev, "error %d clearing FSR\n", ret);
+
+	return ret;
 }
 
 static int spi_nor_fsr_ready(struct spi_nor *nor)
@@ -839,6 +907,8 @@ static int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 status_new,
 
 static int spi_nor_write_sr2(struct spi_nor *nor, const u8 *sr2)
 {
+	int ret;
+
 	if (nor->spimem) {
 		struct spi_mem_op op =
 			SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRSR2, 1),
@@ -846,14 +916,22 @@ static int spi_nor_write_sr2(struct spi_nor *nor, const u8 *sr2)
 				   SPI_MEM_OP_NO_DUMMY,
 				   SPI_MEM_OP_DATA_OUT(1, sr2, 1));
 
-		return spi_mem_exec_op(nor->spimem, &op);
+		ret = spi_mem_exec_op(nor->spimem, &op);
+	} else {
+		ret = nor->controller_ops->write_reg(nor, SPINOR_OP_WRSR2,
+						     sr2, 1);
 	}
 
-	return nor->controller_ops->write_reg(nor, SPINOR_OP_WRSR2, sr2, 1);
+	if (ret)
+		dev_dbg(nor->dev, "error %d writing SR2\n", ret);
+
+	return ret;
 }
 
 static int spi_nor_read_sr2(struct spi_nor *nor, u8 *sr2)
 {
+	int ret;
+
 	if (nor->spimem) {
 		struct spi_mem_op op =
 			SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RDSR2, 1),
@@ -861,10 +939,16 @@ static int spi_nor_read_sr2(struct spi_nor *nor, u8 *sr2)
 				   SPI_MEM_OP_NO_DUMMY,
 				   SPI_MEM_OP_DATA_IN(1, sr2, 1));
 
-		return spi_mem_exec_op(nor->spimem, &op);
+		ret = spi_mem_exec_op(nor->spimem, &op);
+	} else {
+		ret = nor->controller_ops->read_reg(nor, SPINOR_OP_RDSR2,
+						    sr2, 1);
 	}
 
-	return nor->controller_ops->read_reg(nor, SPINOR_OP_RDSR2, sr2, 1);
+	if (ret)
+		dev_dbg(nor->dev, "error %d reading SR2\n", ret);
+
+	return ret;
 }
 
 /*
@@ -874,6 +958,8 @@ static int spi_nor_read_sr2(struct spi_nor *nor, u8 *sr2)
  */
 static int spi_nor_erase_chip(struct spi_nor *nor)
 {
+	int ret;
+
 	dev_dbg(nor->dev, " %lldKiB\n", (long long)(nor->mtd.size >> 10));
 
 	if (nor->spimem) {
@@ -883,11 +969,16 @@ static int spi_nor_erase_chip(struct spi_nor *nor)
 				   SPI_MEM_OP_NO_DUMMY,
 				   SPI_MEM_OP_NO_DATA);
 
-		return spi_mem_exec_op(nor->spimem, &op);
+		ret = spi_mem_exec_op(nor->spimem, &op);
+	} else {
+		ret = nor->controller_ops->write_reg(nor, SPINOR_OP_CHIP_ERASE,
+						     NULL, 0);
 	}
 
-	return nor->controller_ops->write_reg(nor, SPINOR_OP_CHIP_ERASE,
-					      NULL, 0);
+	if (ret)
+		dev_dbg(nor->dev, "error %d erasing chip\n", ret);
+
+	return ret;
 }
 
 static struct spi_nor *mtd_to_spi_nor(struct mtd_info *mtd)
@@ -1934,10 +2025,8 @@ static int sr2_bit7_quad_enable(struct spi_nor *nor)
 	spi_nor_write_enable(nor);
 
 	ret = spi_nor_write_sr2(nor, sr2);
-	if (ret) {
-		dev_dbg(nor->dev, "error while writing status register 2\n");
+	if (ret)
 		return ret;
-	}
 
 	ret = spi_nor_wait_till_ready(nor);
 	if (ret)
@@ -1977,10 +2066,8 @@ static int spi_nor_clear_sr_bp(struct spi_nor *nor)
 	spi_nor_write_enable(nor);
 
 	ret = spi_nor_write_sr(nor, nor->bouncebuf[0] & ~mask);
-	if (ret) {
-		dev_dbg(nor->dev, "write to status register failed\n");
+	if (ret)
 		return ret;
-	}
 
 	return spi_nor_wait_till_ready(nor);
 }
@@ -2739,10 +2826,8 @@ static int s3an_nor_setup(struct spi_nor *nor,
 	int ret;
 
 	ret = spi_nor_xread_sr(nor, nor->bouncebuf);
-	if (ret) {
-		dev_dbg(nor->dev, "error %d reading XRDSR\n", ret);
+	if (ret)
 		return ret;
-	}
 
 	nor->erase_opcode = SPINOR_OP_XSE;
 	nor->program_opcode = SPINOR_OP_XPP;
-- 
2.9.5


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

Re: [PATCH v4 02/20] mtd: spi-nor: Print debug info inside Reg Ops methods

[Vignesh Raghavendra]

On 02/11/19 4:53 PM, Tudor.Ambarus@microchip.com wrote:
> From: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
> Spare the callers of printing debug messages by themselves.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>

Reviewed-by: Vignesh Raghavendra <vigneshr@ti.com>

Regards
Vignesh

> ---
>  drivers/mtd/spi-nor/spi-nor.c | 169 +++++++++++++++++++++++++++++++-----------
>  1 file changed, 127 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index eca6bce7c336..0cb3122e74ad 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -394,6 +394,8 @@ static ssize_t spi_nor_write_data(struct spi_nor *nor, loff_t to, size_t len,
>   */
>  static int spi_nor_write_enable(struct spi_nor *nor)
>  {
> +	int ret;
> +
>  	if (nor->spimem) {
>  		struct spi_mem_op op =
>  			SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WREN, 1),
> @@ -401,10 +403,16 @@ static int spi_nor_write_enable(struct spi_nor *nor)
>  				   SPI_MEM_OP_NO_DUMMY,
>  				   SPI_MEM_OP_NO_DATA);
>  
> -		return spi_mem_exec_op(nor->spimem, &op);
> +		ret = spi_mem_exec_op(nor->spimem, &op);
> +	} else {
> +		ret = nor->controller_ops->write_reg(nor, SPINOR_OP_WREN,
> +						     NULL, 0);
>  	}
>  
> -	return nor->controller_ops->write_reg(nor, SPINOR_OP_WREN, NULL, 0);
> +	if (ret)
> +		dev_dbg(nor->dev, "error %d on Write Enable\n", ret);
> +
> +	return ret;
>  }
>  
>  /*
> @@ -412,6 +420,8 @@ static int spi_nor_write_enable(struct spi_nor *nor)
>   */
>  static int spi_nor_write_disable(struct spi_nor *nor)
>  {
> +	int ret;
> +
>  	if (nor->spimem) {
>  		struct spi_mem_op op =
>  			SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRDI, 1),
> @@ -419,10 +429,16 @@ static int spi_nor_write_disable(struct spi_nor *nor)
>  				   SPI_MEM_OP_NO_DUMMY,
>  				   SPI_MEM_OP_NO_DATA);
>  
> -		return spi_mem_exec_op(nor->spimem, &op);
> +		ret = spi_mem_exec_op(nor->spimem, &op);
> +	} else {
> +		ret = nor->controller_ops->write_reg(nor, SPINOR_OP_WRDI,
> +						     NULL, 0);
>  	}
>  
> -	return nor->controller_ops->write_reg(nor, SPINOR_OP_WRDI, NULL, 0);
> +	if (ret)
> +		dev_dbg(nor->dev, "error %d on Write Disable\n", ret);
> +
> +	return ret;
>  }
>  
>  /**
> @@ -524,6 +540,8 @@ static int spi_nor_read_cr(struct spi_nor *nor, u8 *cr)
>   */
>  static int spi_nor_write_sr(struct spi_nor *nor, u8 val)
>  {
> +	int ret;
> +
>  	nor->bouncebuf[0] = val;
>  	if (nor->spimem) {
>  		struct spi_mem_op op =
> @@ -532,15 +550,23 @@ static int spi_nor_write_sr(struct spi_nor *nor, u8 val)
>  				   SPI_MEM_OP_NO_DUMMY,
>  				   SPI_MEM_OP_DATA_OUT(1, nor->bouncebuf, 1));
>  
> -		return spi_mem_exec_op(nor->spimem, &op);
> +		ret = spi_mem_exec_op(nor->spimem, &op);
> +	} else {
> +		ret = nor->controller_ops->write_reg(nor, SPINOR_OP_WRSR,
> +						     nor->bouncebuf, 1);
>  	}
>  
> -	return nor->controller_ops->write_reg(nor, SPINOR_OP_WRSR,
> -					      nor->bouncebuf, 1);
> +	if (ret)
> +		dev_dbg(nor->dev, "error %d writing SR\n", ret);
> +
> +	return ret;
> +
>  }
>  
>  static int macronix_set_4byte(struct spi_nor *nor, bool enable)
>  {
> +	int ret;
> +
>  	if (nor->spimem) {
>  		struct spi_mem_op op =
>  			SPI_MEM_OP(SPI_MEM_OP_CMD(enable ?
> @@ -551,12 +577,18 @@ static int macronix_set_4byte(struct spi_nor *nor, bool enable)
>  				  SPI_MEM_OP_NO_DUMMY,
>  				  SPI_MEM_OP_NO_DATA);
>  
> -		return spi_mem_exec_op(nor->spimem, &op);
> +		ret = spi_mem_exec_op(nor->spimem, &op);
> +	} else {
> +		ret = nor->controller_ops->write_reg(nor,
> +						     enable ? SPINOR_OP_EN4B :
> +							      SPINOR_OP_EX4B,
> +						     NULL, 0);
>  	}
>  
> -	return nor->controller_ops->write_reg(nor, enable ? SPINOR_OP_EN4B :
> -							    SPINOR_OP_EX4B,
> -					      NULL, 0);
> +	if (ret)
> +		dev_dbg(nor->dev, "error %d setting 4-byte mode\n", ret);
> +
> +	return ret;
>  }
>  
>  static int st_micron_set_4byte(struct spi_nor *nor, bool enable)
> @@ -572,6 +604,8 @@ static int st_micron_set_4byte(struct spi_nor *nor, bool enable)
>  
>  static int spansion_set_4byte(struct spi_nor *nor, bool enable)
>  {
> +	int ret;
> +
>  	nor->bouncebuf[0] = enable << 7;
>  
>  	if (nor->spimem) {
> @@ -581,15 +615,22 @@ static int spansion_set_4byte(struct spi_nor *nor, bool enable)
>  				   SPI_MEM_OP_NO_DUMMY,
>  				   SPI_MEM_OP_DATA_OUT(1, nor->bouncebuf, 1));
>  
> -		return spi_mem_exec_op(nor->spimem, &op);
> +		ret = spi_mem_exec_op(nor->spimem, &op);
> +	} else {
> +		ret = nor->controller_ops->write_reg(nor, SPINOR_OP_BRWR,
> +						     nor->bouncebuf, 1);
>  	}
>  
> -	return nor->controller_ops->write_reg(nor, SPINOR_OP_BRWR,
> -					      nor->bouncebuf, 1);
> +	if (ret)
> +		dev_dbg(nor->dev, "error %d setting 4-byte mode\n", ret);
> +
> +	return ret;
>  }
>  
>  static int spi_nor_write_ear(struct spi_nor *nor, u8 ear)
>  {
> +	int ret;
> +
>  	nor->bouncebuf[0] = ear;
>  
>  	if (nor->spimem) {
> @@ -599,11 +640,16 @@ static int spi_nor_write_ear(struct spi_nor *nor, u8 ear)
>  				   SPI_MEM_OP_NO_DUMMY,
>  				   SPI_MEM_OP_DATA_OUT(1, nor->bouncebuf, 1));
>  
> -		return spi_mem_exec_op(nor->spimem, &op);
> +		ret = spi_mem_exec_op(nor->spimem, &op);
> +	} else {
> +		ret = nor->controller_ops->write_reg(nor, SPINOR_OP_WREAR,
> +						     nor->bouncebuf, 1);
>  	}
>  
> -	return nor->controller_ops->write_reg(nor, SPINOR_OP_WREAR,
> -					      nor->bouncebuf, 1);
> +	if (ret)
> +		dev_dbg(nor->dev, "error %d writing EAR\n", ret);
> +
> +	return ret;
>  }
>  
>  static int winbond_set_4byte(struct spi_nor *nor, bool enable)
> @@ -628,6 +674,8 @@ static int winbond_set_4byte(struct spi_nor *nor, bool enable)
>  
>  static int spi_nor_xread_sr(struct spi_nor *nor, u8 *sr)
>  {
> +	int ret;
> +
>  	if (nor->spimem) {
>  		struct spi_mem_op op =
>  			SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_XRDSR, 1),
> @@ -635,10 +683,16 @@ static int spi_nor_xread_sr(struct spi_nor *nor, u8 *sr)
>  				   SPI_MEM_OP_NO_DUMMY,
>  				   SPI_MEM_OP_DATA_IN(1, sr, 1));
>  
> -		return spi_mem_exec_op(nor->spimem, &op);
> +		ret = spi_mem_exec_op(nor->spimem, &op);
> +	} else {
> +		ret = nor->controller_ops->read_reg(nor, SPINOR_OP_XRDSR,
> +						    sr, 1);
>  	}
>  
> -	return nor->controller_ops->read_reg(nor, SPINOR_OP_XRDSR, sr, 1);
> +	if (ret)
> +		dev_dbg(nor->dev, "error %d reading XRDSR\n", ret);
> +
> +	return ret;
>  }
>  
>  static int s3an_sr_ready(struct spi_nor *nor)
> @@ -646,16 +700,16 @@ static int s3an_sr_ready(struct spi_nor *nor)
>  	int ret;
>  
>  	ret = spi_nor_xread_sr(nor, nor->bouncebuf);
> -	if (ret) {
> -		dev_dbg(nor->dev, "error %d reading XRDSR\n", ret);
> +	if (ret)
>  		return ret;
> -	}
>  
>  	return !!(nor->bouncebuf[0] & XSR_RDY);
>  }
>  
>  static int spi_nor_clear_sr(struct spi_nor *nor)
>  {
> +	int ret;
> +
>  	if (nor->spimem) {
>  		struct spi_mem_op op =
>  			SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_CLSR, 1),
> @@ -663,10 +717,16 @@ static int spi_nor_clear_sr(struct spi_nor *nor)
>  				   SPI_MEM_OP_NO_DUMMY,
>  				   SPI_MEM_OP_NO_DATA);
>  
> -		return spi_mem_exec_op(nor->spimem, &op);
> +		ret = spi_mem_exec_op(nor->spimem, &op);
> +	} else {
> +		ret = nor->controller_ops->write_reg(nor, SPINOR_OP_CLSR,
> +						     NULL, 0);
>  	}
>  
> -	return nor->controller_ops->write_reg(nor, SPINOR_OP_CLSR, NULL, 0);
> +	if (ret)
> +		dev_dbg(nor->dev, "error %d clearing SR\n", ret);
> +
> +	return ret;
>  }
>  
>  static int spi_nor_sr_ready(struct spi_nor *nor)
> @@ -692,6 +752,8 @@ static int spi_nor_sr_ready(struct spi_nor *nor)
>  
>  static int spi_nor_clear_fsr(struct spi_nor *nor)
>  {
> +	int ret;
> +
>  	if (nor->spimem) {
>  		struct spi_mem_op op =
>  			SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_CLFSR, 1),
> @@ -699,10 +761,16 @@ static int spi_nor_clear_fsr(struct spi_nor *nor)
>  				   SPI_MEM_OP_NO_DUMMY,
>  				   SPI_MEM_OP_NO_DATA);
>  
> -		return spi_mem_exec_op(nor->spimem, &op);
> +		ret = spi_mem_exec_op(nor->spimem, &op);
> +	} else {
> +		ret = nor->controller_ops->write_reg(nor, SPINOR_OP_CLFSR,
> +						     NULL, 0);
>  	}
>  
> -	return nor->controller_ops->write_reg(nor, SPINOR_OP_CLFSR, NULL, 0);
> +	if (ret)
> +		dev_dbg(nor->dev, "error %d clearing FSR\n", ret);
> +
> +	return ret;
>  }
>  
>  static int spi_nor_fsr_ready(struct spi_nor *nor)
> @@ -839,6 +907,8 @@ static int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 status_new,
>  
>  static int spi_nor_write_sr2(struct spi_nor *nor, const u8 *sr2)
>  {
> +	int ret;
> +
>  	if (nor->spimem) {
>  		struct spi_mem_op op =
>  			SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRSR2, 1),
> @@ -846,14 +916,22 @@ static int spi_nor_write_sr2(struct spi_nor *nor, const u8 *sr2)
>  				   SPI_MEM_OP_NO_DUMMY,
>  				   SPI_MEM_OP_DATA_OUT(1, sr2, 1));
>  
> -		return spi_mem_exec_op(nor->spimem, &op);
> +		ret = spi_mem_exec_op(nor->spimem, &op);
> +	} else {
> +		ret = nor->controller_ops->write_reg(nor, SPINOR_OP_WRSR2,
> +						     sr2, 1);
>  	}
>  
> -	return nor->controller_ops->write_reg(nor, SPINOR_OP_WRSR2, sr2, 1);
> +	if (ret)
> +		dev_dbg(nor->dev, "error %d writing SR2\n", ret);
> +
> +	return ret;
>  }
>  
>  static int spi_nor_read_sr2(struct spi_nor *nor, u8 *sr2)
>  {
> +	int ret;
> +
>  	if (nor->spimem) {
>  		struct spi_mem_op op =
>  			SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RDSR2, 1),
> @@ -861,10 +939,16 @@ static int spi_nor_read_sr2(struct spi_nor *nor, u8 *sr2)
>  				   SPI_MEM_OP_NO_DUMMY,
>  				   SPI_MEM_OP_DATA_IN(1, sr2, 1));
>  
> -		return spi_mem_exec_op(nor->spimem, &op);
> +		ret = spi_mem_exec_op(nor->spimem, &op);
> +	} else {
> +		ret = nor->controller_ops->read_reg(nor, SPINOR_OP_RDSR2,
> +						    sr2, 1);
>  	}
>  
> -	return nor->controller_ops->read_reg(nor, SPINOR_OP_RDSR2, sr2, 1);
> +	if (ret)
> +		dev_dbg(nor->dev, "error %d reading SR2\n", ret);
> +
> +	return ret;
>  }
>  
>  /*
> @@ -874,6 +958,8 @@ static int spi_nor_read_sr2(struct spi_nor *nor, u8 *sr2)
>   */
>  static int spi_nor_erase_chip(struct spi_nor *nor)
>  {
> +	int ret;
> +
>  	dev_dbg(nor->dev, " %lldKiB\n", (long long)(nor->mtd.size >> 10));
>  
>  	if (nor->spimem) {
> @@ -883,11 +969,16 @@ static int spi_nor_erase_chip(struct spi_nor *nor)
>  				   SPI_MEM_OP_NO_DUMMY,
>  				   SPI_MEM_OP_NO_DATA);
>  
> -		return spi_mem_exec_op(nor->spimem, &op);
> +		ret = spi_mem_exec_op(nor->spimem, &op);
> +	} else {
> +		ret = nor->controller_ops->write_reg(nor, SPINOR_OP_CHIP_ERASE,
> +						     NULL, 0);
>  	}
>  
> -	return nor->controller_ops->write_reg(nor, SPINOR_OP_CHIP_ERASE,
> -					      NULL, 0);
> +	if (ret)
> +		dev_dbg(nor->dev, "error %d erasing chip\n", ret);
> +
> +	return ret;
>  }
>  
>  static struct spi_nor *mtd_to_spi_nor(struct mtd_info *mtd)
> @@ -1934,10 +2025,8 @@ static int sr2_bit7_quad_enable(struct spi_nor *nor)
>  	spi_nor_write_enable(nor);
>  
>  	ret = spi_nor_write_sr2(nor, sr2);
> -	if (ret) {
> -		dev_dbg(nor->dev, "error while writing status register 2\n");
> +	if (ret)
>  		return ret;
> -	}
>  
>  	ret = spi_nor_wait_till_ready(nor);
>  	if (ret)
> @@ -1977,10 +2066,8 @@ static int spi_nor_clear_sr_bp(struct spi_nor *nor)
>  	spi_nor_write_enable(nor);
>  
>  	ret = spi_nor_write_sr(nor, nor->bouncebuf[0] & ~mask);
> -	if (ret) {
> -		dev_dbg(nor->dev, "write to status register failed\n");
> +	if (ret)
>  		return ret;
> -	}
>  
>  	return spi_nor_wait_till_ready(nor);
>  }
> @@ -2739,10 +2826,8 @@ static int s3an_nor_setup(struct spi_nor *nor,
>  	int ret;
>  
>  	ret = spi_nor_xread_sr(nor, nor->bouncebuf);
> -	if (ret) {
> -		dev_dbg(nor->dev, "error %d reading XRDSR\n", ret);
> +	if (ret)
>  		return ret;
> -	}
>  
>  	nor->erase_opcode = SPINOR_OP_XSE;
>  	nor->program_opcode = SPINOR_OP_XPP;
> 



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

[PATCH v4 03/20] mtd: spi-nor: Check for errors after each Register Operation

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

Check for the return vales of each Register Operation.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 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 0cb3122e74ad..5debb0f7ca13 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)
@@ -859,7 +867,9 @@ static int spi_nor_write_sr_cr(struct spi_nor *nor, const 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 =
@@ -889,7 +899,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;
@@ -1397,7 +1410,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)
@@ -1452,7 +1467,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)
@@ -1479,7 +1496,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)
@@ -1500,7 +1519,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);
@@ -1849,9 +1868,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)
@@ -2022,7 +2045,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)
@@ -2063,7 +2088,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)
@@ -2680,7 +2707,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;
 
@@ -2718,14 +2747,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);
@@ -2735,8 +2769,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;
@@ -2787,7 +2823,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


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

Re: [PATCH v4 03/20] mtd: spi-nor: Check for errors after each Register Operation

[Vignesh Raghavendra]

On 02/11/19 4:53 PM, Tudor.Ambarus@microchip.com wrote:
> From: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
> Check for the return vales of each Register Operation.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---


Reviewed-by: Vignesh Raghavendra <vigneshr@ti.com>

Regards
Vignesh

>  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 0cb3122e74ad..5debb0f7ca13 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)
> @@ -859,7 +867,9 @@ static int spi_nor_write_sr_cr(struct spi_nor *nor, const 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 =
> @@ -889,7 +899,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;
> @@ -1397,7 +1410,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)
> @@ -1452,7 +1467,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)
> @@ -1479,7 +1496,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)
> @@ -1500,7 +1519,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);
> @@ -1849,9 +1868,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)
> @@ -2022,7 +2045,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)
> @@ -2063,7 +2088,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)
> @@ -2680,7 +2707,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;
>  
> @@ -2718,14 +2747,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);
> @@ -2735,8 +2769,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;
> @@ -2787,7 +2823,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;
> 

-- 
Regards
Vignesh

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

[PATCH v4 04/20] mtd: spi-nor: Rename label as it is no longer generic

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

Rename 'sst_write_err' label to 'out' as it is no longer generic for
all the errors in the sst_write() method, and may introduce confusion.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 5debb0f7ca13..01842cd92126 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -2709,7 +2709,7 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
 
 	ret = spi_nor_write_enable(nor);
 	if (ret)
-		goto sst_write_err;
+		goto out;
 
 	nor->sst_write_second = false;
 
@@ -2720,11 +2720,11 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
 		/* write one byte. */
 		ret = spi_nor_write_data(nor, to, 1, buf);
 		if (ret < 0)
-			goto sst_write_err;
+			goto out;
 		WARN(ret != 1, "While writing 1 byte written %i bytes\n", ret);
 		ret = spi_nor_wait_till_ready(nor);
 		if (ret)
-			goto sst_write_err;
+			goto out;
 
 		to++;
 		actual++;
@@ -2737,11 +2737,11 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
 		/* write two bytes. */
 		ret = spi_nor_write_data(nor, to, 2, buf + actual);
 		if (ret < 0)
-			goto sst_write_err;
+			goto out;
 		WARN(ret != 2, "While writing 2 bytes written %i bytes\n", ret);
 		ret = spi_nor_wait_till_ready(nor);
 		if (ret)
-			goto sst_write_err;
+			goto out;
 		to += 2;
 		nor->sst_write_second = true;
 	}
@@ -2749,32 +2749,32 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
 
 	ret = spi_nor_write_disable(nor);
 	if (ret)
-		goto sst_write_err;
+		goto out;
 
 	ret = spi_nor_wait_till_ready(nor);
 	if (ret)
-		goto sst_write_err;
+		goto out;
 
 	/* Write out trailing byte if it exists. */
 	if (actual != len) {
 		ret = spi_nor_write_enable(nor);
 		if (ret)
-			goto sst_write_err;
+			goto out;
 
 		nor->program_opcode = SPINOR_OP_BP;
 		ret = spi_nor_write_data(nor, to, 1, buf + actual);
 		if (ret < 0)
-			goto sst_write_err;
+			goto out;
 		WARN(ret != 1, "While writing 1 byte written %i bytes\n", ret);
 		ret = spi_nor_wait_till_ready(nor);
 		if (ret)
-			goto sst_write_err;
+			goto out;
 
 		actual += 1;
 
 		ret = spi_nor_write_disable(nor);
 	}
-sst_write_err:
+out:
 	*retlen += actual;
 	spi_nor_unlock_and_unprep(nor, SPI_NOR_OPS_WRITE);
 	return ret;
-- 
2.9.5


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

[PATCH v4 05/20] mtd: spi-nor: Void return type for spi_nor_clear_sr/fsr()

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

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 <tudor.ambarus@microchip.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 01842cd92126..db1bb2b536ee 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -714,7 +714,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)
 {
 	int ret;
 
@@ -733,8 +733,6 @@ static int spi_nor_clear_sr(struct spi_nor *nor)
 
 	if (ret)
 		dev_dbg(nor->dev, "error %d clearing SR\n", ret);
-
-	return ret;
 }
 
 static int spi_nor_sr_ready(struct spi_nor *nor)
@@ -758,7 +756,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)
 {
 	int ret;
 
@@ -777,8 +775,6 @@ static int spi_nor_clear_fsr(struct spi_nor *nor)
 
 	if (ret)
 		dev_dbg(nor->dev, "error %d clearing FSR\n", ret);
-
-	return ret;
 }
 
 static int spi_nor_fsr_ready(struct spi_nor *nor)
-- 
2.9.5


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

[PATCH v4 06/20] mtd: spi-nor: Move the WE and wait calls inside Write SR methods

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

Avoid duplicating code by moving the calls to spi_nor_write_enable() and
spi_nor_wait_till_ready() inside the Write Status Register methods.

Move spi_nor_write_sr() to avoid forward declaration of
spi_nor_wait_till_ready().

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 108 +++++++++++++++++-------------------------
 1 file changed, 44 insertions(+), 64 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index db1bb2b536ee..ce32b84f050a 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -534,35 +534,6 @@ static int spi_nor_read_cr(struct spi_nor *nor, u8 *cr)
 	return ret;
 }
 
-/*
- * Write status register 1 byte
- * Returns negative if error occurred.
- */
-static int spi_nor_write_sr(struct spi_nor *nor, u8 val)
-{
-	int ret;
-
-	nor->bouncebuf[0] = val;
-	if (nor->spimem) {
-		struct spi_mem_op op =
-			SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRSR, 1),
-				   SPI_MEM_OP_NO_ADDR,
-				   SPI_MEM_OP_NO_DUMMY,
-				   SPI_MEM_OP_DATA_OUT(1, nor->bouncebuf, 1));
-
-		ret = spi_mem_exec_op(nor->spimem, &op);
-	} else {
-		ret = nor->controller_ops->write_reg(nor, SPINOR_OP_WRSR,
-						     nor->bouncebuf, 1);
-	}
-
-	if (ret)
-		dev_dbg(nor->dev, "error %d writing SR\n", ret);
-
-	return ret;
-
-}
-
 static int macronix_set_4byte(struct spi_nor *nor, bool enable)
 {
 	int ret;
@@ -854,6 +825,41 @@ static int spi_nor_wait_till_ready(struct spi_nor *nor)
 }
 
 /*
+ * Write status register 1 byte
+ * Returns negative if error occurred.
+ */
+static int spi_nor_write_sr(struct spi_nor *nor, u8 val)
+{
+	int ret;
+
+	nor->bouncebuf[0] = val;
+
+	ret = spi_nor_write_enable(nor);
+	if (ret)
+		return ret;
+
+	if (nor->spimem) {
+		struct spi_mem_op op =
+			SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRSR, 1),
+				   SPI_MEM_OP_NO_ADDR,
+				   SPI_MEM_OP_NO_DUMMY,
+				   SPI_MEM_OP_DATA_OUT(1, nor->bouncebuf, 1));
+
+		ret = spi_mem_exec_op(nor->spimem, &op);
+	} else {
+		ret = nor->controller_ops->write_reg(nor, SPINOR_OP_WRSR,
+						     nor->bouncebuf, 1);
+	}
+
+	if (ret) {
+		dev_dbg(nor->dev, "error %d writing SR\n", ret);
+		return ret;
+	}
+
+	return spi_nor_wait_till_ready(nor);
+}
+
+/*
  * Write status Register and configuration register with 2 bytes
  * The first byte will be written to the status register, while the
  * second byte will be written to the configuration register.
@@ -895,18 +901,10 @@ static int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 status_new,
 {
 	int ret;
 
-	ret = spi_nor_write_enable(nor);
-	if (ret)
-		return ret;
-
 	ret = spi_nor_write_sr(nor, status_new);
 	if (ret)
 		return ret;
 
-	ret = spi_nor_wait_till_ready(nor);
-	if (ret)
-		return ret;
-
 	ret = spi_nor_read_sr(nor, nor->bouncebuf);
 	if (ret)
 		return ret;
@@ -918,6 +916,10 @@ static int spi_nor_write_sr2(struct spi_nor *nor, const u8 *sr2)
 {
 	int ret;
 
+	ret = spi_nor_write_enable(nor);
+	if (ret)
+		return ret;
+
 	if (nor->spimem) {
 		struct spi_mem_op op =
 			SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRSR2, 1),
@@ -931,10 +933,12 @@ static int spi_nor_write_sr2(struct spi_nor *nor, const u8 *sr2)
 						     sr2, 1);
 	}
 
-	if (ret)
+	if (ret) {
 		dev_dbg(nor->dev, "error %d writing SR2\n", ret);
+		return ret;
+	}
 
-	return ret;
+	return spi_nor_wait_till_ready(nor);
 }
 
 static int spi_nor_read_sr2(struct spi_nor *nor, u8 *sr2)
@@ -1864,18 +1868,10 @@ static int macronix_quad_enable(struct spi_nor *nor)
 	if (nor->bouncebuf[0] & SR_QUAD_EN_MX)
 		return 0;
 
-	ret = spi_nor_write_enable(nor);
-	if (ret)
-		return ret;
-
 	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)
-		return ret;
-
 	ret = spi_nor_read_sr(nor, nor->bouncebuf);
 	if (ret)
 		return ret;
@@ -2041,18 +2037,10 @@ static int sr2_bit7_quad_enable(struct spi_nor *nor)
 	/* Update the Quad Enable bit. */
 	*sr2 |= SR2_QUAD_EN_BIT7;
 
-	ret = spi_nor_write_enable(nor);
-	if (ret)
-		return ret;
-
 	ret = spi_nor_write_sr2(nor, sr2);
 	if (ret)
 		return ret;
 
-	ret = spi_nor_wait_till_ready(nor);
-	if (ret)
-		return ret;
-
 	/* Read back and check it. */
 	ret = spi_nor_read_sr2(nor, sr2);
 	if (ret)
@@ -2084,15 +2072,7 @@ static int spi_nor_clear_sr_bp(struct spi_nor *nor)
 	if (ret)
 		return ret;
 
-	ret = spi_nor_write_enable(nor);
-	if (ret)
-		return ret;
-
-	ret = spi_nor_write_sr(nor, nor->bouncebuf[0] & ~mask);
-	if (ret)
-		return ret;
-
-	return spi_nor_wait_till_ready(nor);
+	return spi_nor_write_sr(nor, nor->bouncebuf[0] & ~mask);
 }
 
 /**
-- 
2.9.5


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

[PATCH v4 07/20] mtd: spi-nor: Merge spi_nor_write_sr() and spi_nor_write_sr_cr()

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

Merge
static int spi_nor_write_sr(struct spi_nor *nor, u8 val)
static int spi_nor_write_sr_cr(struct spi_nor *nor, const u8 *sr_cr)
into
static int spi_nor_write_sr(struct spi_nor *nor, const u8 *sr, size_t len)

The Status Register can be written with one or two bytes. Merge
the two functions to avoid code duplication.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 74 ++++++++++++++-----------------------------
 1 file changed, 23 insertions(+), 51 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index ce32b84f050a..857675a4e329 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -824,16 +824,18 @@ static int spi_nor_wait_till_ready(struct spi_nor *nor)
 						    DEFAULT_READY_WAIT_JIFFIES);
 }
 
-/*
- * Write status register 1 byte
- * Returns negative if error occurred.
+/**
+ * spi_nor_write_sr() - Write the Status Register.
+ * @nor:	pointer to 'struct spi_nor'.
+ * @sr:		pointer to DMA-able buffer to write to the Status Register.
+ * @len:	number of bytes to write to the Status Register.
+ *
+ * Return: 0 on success, -errno otherwise.
  */
-static int spi_nor_write_sr(struct spi_nor *nor, u8 val)
+static int spi_nor_write_sr(struct spi_nor *nor, const u8 *sr, size_t len)
 {
 	int ret;
 
-	nor->bouncebuf[0] = val;
-
 	ret = spi_nor_write_enable(nor);
 	if (ret)
 		return ret;
@@ -843,12 +845,12 @@ static int spi_nor_write_sr(struct spi_nor *nor, u8 val)
 			SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRSR, 1),
 				   SPI_MEM_OP_NO_ADDR,
 				   SPI_MEM_OP_NO_DUMMY,
-				   SPI_MEM_OP_DATA_OUT(1, nor->bouncebuf, 1));
+				   SPI_MEM_OP_DATA_OUT(len, sr, 1));
 
 		ret = spi_mem_exec_op(nor->spimem, &op);
 	} else {
 		ret = nor->controller_ops->write_reg(nor, SPINOR_OP_WRSR,
-						     nor->bouncebuf, 1);
+						     sr, len);
 	}
 
 	if (ret) {
@@ -859,49 +861,15 @@ static int spi_nor_write_sr(struct spi_nor *nor, u8 val)
 	return spi_nor_wait_till_ready(nor);
 }
 
-/*
- * Write status Register and configuration register with 2 bytes
- * The first byte will be written to the status register, while the
- * second byte will be written to the configuration register.
- * Return negative if error occurred.
- */
-static int spi_nor_write_sr_cr(struct spi_nor *nor, const u8 *sr_cr)
-{
-	int ret;
-
-	ret = spi_nor_write_enable(nor);
-	if (ret)
-		return ret;
-
-	if (nor->spimem) {
-		struct spi_mem_op op =
-			SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRSR, 1),
-				   SPI_MEM_OP_NO_ADDR,
-				   SPI_MEM_OP_NO_DUMMY,
-				   SPI_MEM_OP_DATA_OUT(2, sr_cr, 1));
-
-		ret = spi_mem_exec_op(nor->spimem, &op);
-	} else {
-		ret = nor->controller_ops->write_reg(nor, SPINOR_OP_WRSR,
-						     sr_cr, 2);
-	}
-
-	if (ret) {
-		dev_dbg(nor->dev,
-			"error while writing configuration register\n");
-		return -EINVAL;
-	}
-
-	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,
 				      u8 mask)
 {
 	int ret;
 
-	ret = spi_nor_write_sr(nor, status_new);
+	nor->bouncebuf[0] = status_new;
+
+	ret = spi_nor_write_sr(nor, nor->bouncebuf, 1);
 	if (ret)
 		return ret;
 
@@ -1868,7 +1836,9 @@ static int macronix_quad_enable(struct spi_nor *nor)
 	if (nor->bouncebuf[0] & SR_QUAD_EN_MX)
 		return 0;
 
-	ret = spi_nor_write_sr(nor, nor->bouncebuf[0] | SR_QUAD_EN_MX);
+	nor->bouncebuf[0] |= SR_QUAD_EN_MX;
+
+	ret = spi_nor_write_sr(nor, nor->bouncebuf, 1);
 	if (ret)
 		return ret;
 
@@ -1915,7 +1885,7 @@ static int spansion_quad_enable(struct spi_nor *nor)
 
 	sr_cr[0] = 0;
 	sr_cr[1] = CR_QUAD_EN_SPAN;
-	ret = spi_nor_write_sr_cr(nor, sr_cr);
+	ret = spi_nor_write_sr(nor, sr_cr, 2);
 	if (ret)
 		return ret;
 
@@ -1957,7 +1927,7 @@ static int spansion_no_read_cr_quad_enable(struct spi_nor *nor)
 
 	sr_cr[1] = CR_QUAD_EN_SPAN;
 
-	return spi_nor_write_sr_cr(nor, sr_cr);
+	return spi_nor_write_sr(nor, sr_cr, 2);
 }
 
 /**
@@ -1993,7 +1963,7 @@ static int spansion_read_cr_quad_enable(struct spi_nor *nor)
 	if (ret)
 		return ret;
 
-	ret = spi_nor_write_sr_cr(nor, sr_cr);
+	ret = spi_nor_write_sr(nor, sr_cr, 2);
 	if (ret)
 		return ret;
 
@@ -2072,7 +2042,9 @@ static int spi_nor_clear_sr_bp(struct spi_nor *nor)
 	if (ret)
 		return ret;
 
-	return spi_nor_write_sr(nor, nor->bouncebuf[0] & ~mask);
+	nor->bouncebuf[0] &= ~mask;
+
+	return spi_nor_write_sr(nor, nor->bouncebuf, 1);
 }
 
 /**
@@ -2110,7 +2082,7 @@ static int spi_nor_spansion_clear_sr_bp(struct spi_nor *nor)
 
 		sr_cr[0] &= ~mask;
 
-		return spi_nor_write_sr_cr(nor, sr_cr);
+		return spi_nor_write_sr(nor, sr_cr, 2);
 	}
 
 	/*
-- 
2.9.5


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

[PATCH v4 08/20] mtd: spi-nor: Describe all the Reg Ops

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

Document all the Register Operations.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 138 ++++++++++++++++++++++++++++++++++++++----
 1 file changed, 127 insertions(+), 11 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 857675a4e329..99a9a6aba41d 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -388,9 +388,11 @@ static ssize_t spi_nor_write_data(struct spi_nor *nor, loff_t to, size_t len,
 	return nor->controller_ops->write(nor, to, len, buf);
 }
 
-/*
- * Set write enable latch with Write Enable command.
- * Returns negative if error occurred.
+/**
+ * spi_nor_write_enable() - Set write enable latch with Write Enable command.
+ * @nor:	pointer to 'struct spi_nor'.
+ *
+ * Return: 0 on success, -errno otherwise.
  */
 static int spi_nor_write_enable(struct spi_nor *nor)
 {
@@ -415,8 +417,11 @@ static int spi_nor_write_enable(struct spi_nor *nor)
 	return ret;
 }
 
-/*
- * Send write disable instruction to the chip.
+/**
+ * spi_nor_write_disable() - Send Write Disable instruction to the chip.
+ * @nor:	pointer to 'struct spi_nor'.
+ *
+ * Return: 0 on success, -errno otherwise.
  */
 static int spi_nor_write_disable(struct spi_nor *nor)
 {
@@ -534,6 +539,14 @@ static int spi_nor_read_cr(struct spi_nor *nor, u8 *cr)
 	return ret;
 }
 
+/**
+ * macronix_set_4byte() - Set 4-byte address mode for Macronix flashes.
+ * @nor:	pointer to 'struct spi_nor'.
+ * @enable:	true to enter the 4-byte address mode, false to exit the 4-byte
+ *		address mode.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
 static int macronix_set_4byte(struct spi_nor *nor, bool enable)
 {
 	int ret;
@@ -562,6 +575,14 @@ static int macronix_set_4byte(struct spi_nor *nor, bool enable)
 	return ret;
 }
 
+/**
+ * st_micron_set_4byte() - Set 4-byte address mode for ST and Micron flashes.
+ * @nor:	pointer to 'struct spi_nor'.
+ * @enable:	true to enter the 4-byte address mode, false to exit the 4-byte
+ *		address mode.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
 static int st_micron_set_4byte(struct spi_nor *nor, bool enable)
 {
 	int ret;
@@ -577,6 +598,14 @@ static int st_micron_set_4byte(struct spi_nor *nor, bool enable)
 	return spi_nor_write_disable(nor);
 }
 
+/**
+ * spansion_set_4byte() - Set 4-byte address mode for Spansion flashes.
+ * @nor:	pointer to 'struct spi_nor'.
+ * @enable:	true to enter the 4-byte address mode, false to exit the 4-byte
+ *		address mode.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
 static int spansion_set_4byte(struct spi_nor *nor, bool enable)
 {
 	int ret;
@@ -602,6 +631,13 @@ static int spansion_set_4byte(struct spi_nor *nor, bool enable)
 	return ret;
 }
 
+/**
+ * spi_nor_write_ear() - Write Extended Address Register.
+ * @nor:	pointer to 'struct spi_nor'.
+ * @ear:	value to write to the Extended Address Register.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
 static int spi_nor_write_ear(struct spi_nor *nor, u8 ear)
 {
 	int ret;
@@ -627,6 +663,14 @@ static int spi_nor_write_ear(struct spi_nor *nor, u8 ear)
 	return ret;
 }
 
+/**
+ * winbond_set_4byte() - Set 4-byte address mode for Winbond flashes.
+ * @nor:	pointer to 'struct spi_nor'.
+ * @enable:	true to enter the 4-byte address mode, false to exit the 4-byte
+ *		address mode.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
 static int winbond_set_4byte(struct spi_nor *nor, bool enable)
 {
 	int ret;
@@ -651,6 +695,14 @@ static int winbond_set_4byte(struct spi_nor *nor, bool enable)
 	return spi_nor_write_disable(nor);
 }
 
+/**
+ * spi_nor_xread_sr() - Read the Status Register on S3AN flashes.
+ * @nor:	pointer to 'struct spi_nor'.
+ * @sr:		pointer to a DMA-able buffer where the value of the
+ *              Status Register will be written.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
 static int spi_nor_xread_sr(struct spi_nor *nor, u8 *sr)
 {
 	int ret;
@@ -674,6 +726,13 @@ static int spi_nor_xread_sr(struct spi_nor *nor, u8 *sr)
 	return ret;
 }
 
+/**
+ * s3an_sr_ready() - Query the Status Register of the S3AN flash to see if the
+ * flash is ready for new commands.
+ * @nor:	pointer to 'struct spi_nor'.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
 static int s3an_sr_ready(struct spi_nor *nor)
 {
 	int ret;
@@ -685,6 +744,10 @@ static int s3an_sr_ready(struct spi_nor *nor)
 	return !!(nor->bouncebuf[0] & XSR_RDY);
 }
 
+/**
+ * spi_nor_clear_sr() - Clear the Status Register.
+ * @nor:	pointer to 'struct spi_nor'.
+ */
 static void spi_nor_clear_sr(struct spi_nor *nor)
 {
 	int ret;
@@ -706,6 +769,13 @@ static void spi_nor_clear_sr(struct spi_nor *nor)
 		dev_dbg(nor->dev, "error %d clearing SR\n", ret);
 }
 
+/**
+ * spi_nor_sr_ready() - Query the Status Register to see if the flash is ready
+ * for new commands.
+ * @nor:	pointer to 'struct spi_nor'.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
 static int spi_nor_sr_ready(struct spi_nor *nor)
 {
 	int ret = spi_nor_read_sr(nor, nor->bouncebuf);
@@ -727,6 +797,10 @@ static int spi_nor_sr_ready(struct spi_nor *nor)
 	return !(nor->bouncebuf[0] & SR_WIP);
 }
 
+/**
+ * spi_nor_clear_fsr() - Clear the Flag Status Register.
+ * @nor:	pointer to 'struct spi_nor'.
+ */
 static void spi_nor_clear_fsr(struct spi_nor *nor)
 {
 	int ret;
@@ -748,6 +822,13 @@ static void spi_nor_clear_fsr(struct spi_nor *nor)
 		dev_dbg(nor->dev, "error %d clearing FSR\n", ret);
 }
 
+/**
+ * spi_nor_fsr_ready() - Query the Flag Status Register to see if the flash is
+ * ready for new commands.
+ * @nor:	pointer to 'struct spi_nor'.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
 static int spi_nor_fsr_ready(struct spi_nor *nor)
 {
 	int ret = spi_nor_read_fsr(nor, nor->bouncebuf);
@@ -772,6 +853,12 @@ static int spi_nor_fsr_ready(struct spi_nor *nor)
 	return nor->bouncebuf[0] & FSR_READY;
 }
 
+/**
+ * spi_nor_ready() - Query the flash to see if it is ready for new commands.
+ * @nor:	pointer to 'struct spi_nor'.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
 static int spi_nor_ready(struct spi_nor *nor)
 {
 	int sr, fsr;
@@ -788,9 +875,13 @@ static int spi_nor_ready(struct spi_nor *nor)
 	return sr && fsr;
 }
 
-/*
- * Service routine to read status register until ready, or timeout occurs.
- * Returns non-zero if error.
+/**
+ * spi_nor_wait_till_ready_with_timeout() - Service routine to read the
+ * Status Register until ready, or timeout occurs.
+ * @nor:		pointer to "struct spi_nor".
+ * @timeout_jiffies:	jiffies to wait until timeout.
+ *
+ * Return: 0 on success, -errno otherwise.
  */
 static int spi_nor_wait_till_ready_with_timeout(struct spi_nor *nor,
 						unsigned long timeout_jiffies)
@@ -818,6 +909,13 @@ static int spi_nor_wait_till_ready_with_timeout(struct spi_nor *nor,
 	return -ETIMEDOUT;
 }
 
+/**
+ * spi_nor_wait_till_ready() - Wait for a predefined amount of time for the
+ * flash to be ready, or timeout occurs.
+ * @nor:	pointer to "struct spi_nor".
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
 static int spi_nor_wait_till_ready(struct spi_nor *nor)
 {
 	return spi_nor_wait_till_ready_with_timeout(nor,
@@ -880,6 +978,14 @@ static int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 status_new,
 	return ((nor->bouncebuf[0] & mask) != (status_new & mask)) ? -EIO : 0;
 }
 
+/**
+ * spi_nor_write_sr2() - Write the Status Register 2 using the
+ * SPINOR_OP_WRSR2 (3eh) command.
+ * @nor:	pointer to 'struct spi_nor'.
+ * @sr2:	pointer to DMA-able buffer to write to the Status Register 2.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
 static int spi_nor_write_sr2(struct spi_nor *nor, const u8 *sr2)
 {
 	int ret;
@@ -909,6 +1015,15 @@ static int spi_nor_write_sr2(struct spi_nor *nor, const u8 *sr2)
 	return spi_nor_wait_till_ready(nor);
 }
 
+/**
+ * spi_nor_read_sr2() - Read the Status Register 2 using the
+ * SPINOR_OP_RDSR2 (3fh) command.
+ * @nor:	pointer to 'struct spi_nor'.
+ * @sr2:	pointer to DMA-able buffer where the value of the
+ *		Status Register 2 will be written.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
 static int spi_nor_read_sr2(struct spi_nor *nor, u8 *sr2)
 {
 	int ret;
@@ -932,10 +1047,11 @@ static int spi_nor_read_sr2(struct spi_nor *nor, u8 *sr2)
 	return ret;
 }
 
-/*
- * Erase the whole flash memory
+/**
+ * spi_nor_erase_chip() - Erase the entire flash memory.
+ * @nor:	pointer to 'struct spi_nor'.
  *
- * Returns 0 if successful, non-zero otherwise.
+ * Return: 0 on success, -errno otherwise.
  */
 static int spi_nor_erase_chip(struct spi_nor *nor)
 {
-- 
2.9.5


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

Re: [PATCH v4 08/20] mtd: spi-nor: Describe all the Reg Ops

[Vignesh Raghavendra]

On 02/11/19 4:53 PM, Tudor.Ambarus@microchip.com wrote:
> From: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
> Document all the Register Operations.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>

Reviewed-by: Vignesh Raghavendra <vigneshr@ti.com>

Regards
Vignesh

> ---
>  drivers/mtd/spi-nor/spi-nor.c | 138 ++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 127 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 857675a4e329..99a9a6aba41d 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -388,9 +388,11 @@ static ssize_t spi_nor_write_data(struct spi_nor *nor, loff_t to, size_t len,
>  	return nor->controller_ops->write(nor, to, len, buf);
>  }
>  
> -/*
> - * Set write enable latch with Write Enable command.
> - * Returns negative if error occurred.
> +/**
> + * spi_nor_write_enable() - Set write enable latch with Write Enable command.
> + * @nor:	pointer to 'struct spi_nor'.
> + *
> + * Return: 0 on success, -errno otherwise.
>   */
>  static int spi_nor_write_enable(struct spi_nor *nor)
>  {
> @@ -415,8 +417,11 @@ static int spi_nor_write_enable(struct spi_nor *nor)
>  	return ret;
>  }
>  
> -/*
> - * Send write disable instruction to the chip.
> +/**
> + * spi_nor_write_disable() - Send Write Disable instruction to the chip.
> + * @nor:	pointer to 'struct spi_nor'.
> + *
> + * Return: 0 on success, -errno otherwise.
>   */
>  static int spi_nor_write_disable(struct spi_nor *nor)
>  {
> @@ -534,6 +539,14 @@ static int spi_nor_read_cr(struct spi_nor *nor, u8 *cr)
>  	return ret;
>  }
>  
> +/**
> + * macronix_set_4byte() - Set 4-byte address mode for Macronix flashes.
> + * @nor:	pointer to 'struct spi_nor'.
> + * @enable:	true to enter the 4-byte address mode, false to exit the 4-byte
> + *		address mode.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
>  static int macronix_set_4byte(struct spi_nor *nor, bool enable)
>  {
>  	int ret;
> @@ -562,6 +575,14 @@ static int macronix_set_4byte(struct spi_nor *nor, bool enable)
>  	return ret;
>  }
>  
> +/**
> + * st_micron_set_4byte() - Set 4-byte address mode for ST and Micron flashes.
> + * @nor:	pointer to 'struct spi_nor'.
> + * @enable:	true to enter the 4-byte address mode, false to exit the 4-byte
> + *		address mode.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
>  static int st_micron_set_4byte(struct spi_nor *nor, bool enable)
>  {
>  	int ret;
> @@ -577,6 +598,14 @@ static int st_micron_set_4byte(struct spi_nor *nor, bool enable)
>  	return spi_nor_write_disable(nor);
>  }
>  
> +/**
> + * spansion_set_4byte() - Set 4-byte address mode for Spansion flashes.
> + * @nor:	pointer to 'struct spi_nor'.
> + * @enable:	true to enter the 4-byte address mode, false to exit the 4-byte
> + *		address mode.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
>  static int spansion_set_4byte(struct spi_nor *nor, bool enable)
>  {
>  	int ret;
> @@ -602,6 +631,13 @@ static int spansion_set_4byte(struct spi_nor *nor, bool enable)
>  	return ret;
>  }
>  
> +/**
> + * spi_nor_write_ear() - Write Extended Address Register.
> + * @nor:	pointer to 'struct spi_nor'.
> + * @ear:	value to write to the Extended Address Register.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
>  static int spi_nor_write_ear(struct spi_nor *nor, u8 ear)
>  {
>  	int ret;
> @@ -627,6 +663,14 @@ static int spi_nor_write_ear(struct spi_nor *nor, u8 ear)
>  	return ret;
>  }
>  
> +/**
> + * winbond_set_4byte() - Set 4-byte address mode for Winbond flashes.
> + * @nor:	pointer to 'struct spi_nor'.
> + * @enable:	true to enter the 4-byte address mode, false to exit the 4-byte
> + *		address mode.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
>  static int winbond_set_4byte(struct spi_nor *nor, bool enable)
>  {
>  	int ret;
> @@ -651,6 +695,14 @@ static int winbond_set_4byte(struct spi_nor *nor, bool enable)
>  	return spi_nor_write_disable(nor);
>  }
>  
> +/**
> + * spi_nor_xread_sr() - Read the Status Register on S3AN flashes.
> + * @nor:	pointer to 'struct spi_nor'.
> + * @sr:		pointer to a DMA-able buffer where the value of the
> + *              Status Register will be written.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
>  static int spi_nor_xread_sr(struct spi_nor *nor, u8 *sr)
>  {
>  	int ret;
> @@ -674,6 +726,13 @@ static int spi_nor_xread_sr(struct spi_nor *nor, u8 *sr)
>  	return ret;
>  }
>  
> +/**
> + * s3an_sr_ready() - Query the Status Register of the S3AN flash to see if the
> + * flash is ready for new commands.
> + * @nor:	pointer to 'struct spi_nor'.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
>  static int s3an_sr_ready(struct spi_nor *nor)
>  {
>  	int ret;
> @@ -685,6 +744,10 @@ static int s3an_sr_ready(struct spi_nor *nor)
>  	return !!(nor->bouncebuf[0] & XSR_RDY);
>  }
>  
> +/**
> + * spi_nor_clear_sr() - Clear the Status Register.
> + * @nor:	pointer to 'struct spi_nor'.
> + */
>  static void spi_nor_clear_sr(struct spi_nor *nor)
>  {
>  	int ret;
> @@ -706,6 +769,13 @@ static void spi_nor_clear_sr(struct spi_nor *nor)
>  		dev_dbg(nor->dev, "error %d clearing SR\n", ret);
>  }
>  
> +/**
> + * spi_nor_sr_ready() - Query the Status Register to see if the flash is ready
> + * for new commands.
> + * @nor:	pointer to 'struct spi_nor'.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
>  static int spi_nor_sr_ready(struct spi_nor *nor)
>  {
>  	int ret = spi_nor_read_sr(nor, nor->bouncebuf);
> @@ -727,6 +797,10 @@ static int spi_nor_sr_ready(struct spi_nor *nor)
>  	return !(nor->bouncebuf[0] & SR_WIP);
>  }
>  
> +/**
> + * spi_nor_clear_fsr() - Clear the Flag Status Register.
> + * @nor:	pointer to 'struct spi_nor'.
> + */
>  static void spi_nor_clear_fsr(struct spi_nor *nor)
>  {
>  	int ret;
> @@ -748,6 +822,13 @@ static void spi_nor_clear_fsr(struct spi_nor *nor)
>  		dev_dbg(nor->dev, "error %d clearing FSR\n", ret);
>  }
>  
> +/**
> + * spi_nor_fsr_ready() - Query the Flag Status Register to see if the flash is
> + * ready for new commands.
> + * @nor:	pointer to 'struct spi_nor'.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
>  static int spi_nor_fsr_ready(struct spi_nor *nor)
>  {
>  	int ret = spi_nor_read_fsr(nor, nor->bouncebuf);
> @@ -772,6 +853,12 @@ static int spi_nor_fsr_ready(struct spi_nor *nor)
>  	return nor->bouncebuf[0] & FSR_READY;
>  }
>  
> +/**
> + * spi_nor_ready() - Query the flash to see if it is ready for new commands.
> + * @nor:	pointer to 'struct spi_nor'.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
>  static int spi_nor_ready(struct spi_nor *nor)
>  {
>  	int sr, fsr;
> @@ -788,9 +875,13 @@ static int spi_nor_ready(struct spi_nor *nor)
>  	return sr && fsr;
>  }
>  
> -/*
> - * Service routine to read status register until ready, or timeout occurs.
> - * Returns non-zero if error.
> +/**
> + * spi_nor_wait_till_ready_with_timeout() - Service routine to read the
> + * Status Register until ready, or timeout occurs.
> + * @nor:		pointer to "struct spi_nor".
> + * @timeout_jiffies:	jiffies to wait until timeout.
> + *
> + * Return: 0 on success, -errno otherwise.
>   */
>  static int spi_nor_wait_till_ready_with_timeout(struct spi_nor *nor,
>  						unsigned long timeout_jiffies)
> @@ -818,6 +909,13 @@ static int spi_nor_wait_till_ready_with_timeout(struct spi_nor *nor,
>  	return -ETIMEDOUT;
>  }
>  
> +/**
> + * spi_nor_wait_till_ready() - Wait for a predefined amount of time for the
> + * flash to be ready, or timeout occurs.
> + * @nor:	pointer to "struct spi_nor".
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
>  static int spi_nor_wait_till_ready(struct spi_nor *nor)
>  {
>  	return spi_nor_wait_till_ready_with_timeout(nor,
> @@ -880,6 +978,14 @@ static int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 status_new,
>  	return ((nor->bouncebuf[0] & mask) != (status_new & mask)) ? -EIO : 0;
>  }
>  
> +/**
> + * spi_nor_write_sr2() - Write the Status Register 2 using the
> + * SPINOR_OP_WRSR2 (3eh) command.
> + * @nor:	pointer to 'struct spi_nor'.
> + * @sr2:	pointer to DMA-able buffer to write to the Status Register 2.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
>  static int spi_nor_write_sr2(struct spi_nor *nor, const u8 *sr2)
>  {
>  	int ret;
> @@ -909,6 +1015,15 @@ static int spi_nor_write_sr2(struct spi_nor *nor, const u8 *sr2)
>  	return spi_nor_wait_till_ready(nor);
>  }
>  
> +/**
> + * spi_nor_read_sr2() - Read the Status Register 2 using the
> + * SPINOR_OP_RDSR2 (3fh) command.
> + * @nor:	pointer to 'struct spi_nor'.
> + * @sr2:	pointer to DMA-able buffer where the value of the
> + *		Status Register 2 will be written.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
>  static int spi_nor_read_sr2(struct spi_nor *nor, u8 *sr2)
>  {
>  	int ret;
> @@ -932,10 +1047,11 @@ static int spi_nor_read_sr2(struct spi_nor *nor, u8 *sr2)
>  	return ret;
>  }
>  
> -/*
> - * Erase the whole flash memory
> +/**
> + * spi_nor_erase_chip() - Erase the entire flash memory.
> + * @nor:	pointer to 'struct spi_nor'.
>   *
> - * Returns 0 if successful, non-zero otherwise.
> + * Return: 0 on success, -errno otherwise.
>   */
>  static int spi_nor_erase_chip(struct spi_nor *nor)
>  {
> 

-- 
Regards
Vignesh

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

[PATCH v4 09/20] mtd: spi-nor: Drop spansion_quad_enable()

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

Drop the default spansion_quad_enable() method and replace it with
spansion_read_cr_quad_enable().

The function was buggy, it didn't care about the previous values
of the Status and Configuration Registers. spansion_read_cr_quad_enable()
is a Read-Modify-Write-Check function that keeps track of what were
the previous values of the Status and Configuration Registers.

In terms of instruction types sent to the flash, the only difference
between the spansion_quad_enable() and spansion_read_cr_quad_enable()
is that the later calls spi_nor_read_sr(). We can safely assume that all
flashes support spi_nor_read_sr(), because all flashes call it in
spi_nor_sr_ready(). The transition from spansion_quad_enable() to
spansion_read_cr_quad_enable() will not affect anybody, drop the buggy
code.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 58 ++++---------------------------------------
 1 file changed, 5 insertions(+), 53 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 99a9a6aba41d..f5193733a0f6 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1971,54 +1971,6 @@ static int macronix_quad_enable(struct spi_nor *nor)
 }
 
 /**
- * spansion_quad_enable() - set QE bit in Configuraiton Register.
- * @nor:	pointer to a 'struct spi_nor'
- *
- * Set the Quad Enable (QE) bit in the Configuration Register.
- * This function is kept for legacy purpose because it has been used for a
- * long time without anybody complaining but it should be considered as
- * deprecated and maybe buggy.
- * First, this function doesn't care about the previous values of the Status
- * and Configuration Registers when it sets the QE bit (bit 1) in the
- * Configuration Register: all other bits are cleared, which may have unwanted
- * side effects like removing some block protections.
- * Secondly, it uses the Read Configuration Register (35h) instruction though
- * some very old and few memories don't support this instruction. If a pull-up
- * resistor is present on the MISO/IO1 line, we might still be able to pass the
- * "read back" test because the QSPI memory doesn't recognize the command,
- * so leaves the MISO/IO1 line state unchanged, hence spi_nor_read_cr() returns
- * 0xFF.
- *
- * bit 1 of the Configuration Register is the QE bit for Spansion like QSPI
- * memories.
- *
- * Return: 0 on success, -errno otherwise.
- */
-static int spansion_quad_enable(struct spi_nor *nor)
-{
-	u8 *sr_cr = nor->bouncebuf;
-	int ret;
-
-	sr_cr[0] = 0;
-	sr_cr[1] = CR_QUAD_EN_SPAN;
-	ret = spi_nor_write_sr(nor, sr_cr, 2);
-	if (ret)
-		return ret;
-
-	/* read back and check it */
-	ret = spi_nor_read_cr(nor, nor->bouncebuf);
-	if (ret)
-		return ret;
-
-	if (!(nor->bouncebuf[0] & CR_QUAD_EN_SPAN)) {
-		dev_dbg(nor->dev, "Spansion Quad bit not set\n");
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
-/**
  * spansion_no_read_cr_quad_enable() - set QE bit in Configuration Register.
  * @nor:	pointer to a 'struct spi_nor'
  *
@@ -2170,9 +2122,9 @@ static int spi_nor_clear_sr_bp(struct spi_nor *nor)
  *
  * Read-modify-write function that clears the Block Protection bits from the
  * Status Register without affecting other bits. The function is tightly
- * coupled with the spansion_quad_enable() function. Both assume that the Write
- * Register with 16 bits, together with the Read Configuration Register (35h)
- * instructions are supported.
+ * coupled with the spansion_read_cr_quad_enable() function. Both assume that
+ * the Write Register with 16 bits, together with the Read Configuration
+ * Register (35h) instructions are supported.
  *
  * Return: 0 on success, -errno otherwise.
  */
@@ -4654,7 +4606,7 @@ static void spi_nor_info_init_params(struct spi_nor *nor)
 	u8 i, erase_mask;
 
 	/* Initialize legacy flash parameters and settings. */
-	params->quad_enable = spansion_quad_enable;
+	params->quad_enable = spansion_read_cr_quad_enable;
 	params->set_4byte = spansion_set_4byte;
 	params->setup = spi_nor_default_setup;
 
@@ -4869,7 +4821,7 @@ static int spi_nor_init(struct spi_nor *nor)
 	int err;
 
 	if (nor->clear_sr_bp) {
-		if (nor->params.quad_enable == spansion_quad_enable)
+		if (nor->params.quad_enable == spansion_read_cr_quad_enable)
 			nor->clear_sr_bp = spi_nor_spansion_clear_sr_bp;
 
 		err = nor->clear_sr_bp(nor);
-- 
2.9.5


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

Re: [PATCH v4 09/20] mtd: spi-nor: Drop spansion_quad_enable()

[Vignesh Raghavendra]

On 02/11/19 4:53 PM, Tudor.Ambarus@microchip.com wrote:
> From: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
> Drop the default spansion_quad_enable() method and replace it with
> spansion_read_cr_quad_enable().
> 
> The function was buggy, it didn't care about the previous values
> of the Status and Configuration Registers. spansion_read_cr_quad_enable()
> is a Read-Modify-Write-Check function that keeps track of what were
> the previous values of the Status and Configuration Registers.
> 
> In terms of instruction types sent to the flash, the only difference
> between the spansion_quad_enable() and spansion_read_cr_quad_enable()
> is that the later calls spi_nor_read_sr(). We can safely assume that all
> flashes support spi_nor_read_sr(), because all flashes call it in
> spi_nor_sr_ready(). The transition from spansion_quad_enable() to
> spansion_read_cr_quad_enable() will not affect anybody, drop the buggy
> code.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>

Reviewed-by: Vignesh Raghavendra <vigneshr@ti.com>

Regards
Vignesh

> ---
>  drivers/mtd/spi-nor/spi-nor.c | 58 ++++---------------------------------------
>  1 file changed, 5 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 99a9a6aba41d..f5193733a0f6 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -1971,54 +1971,6 @@ static int macronix_quad_enable(struct spi_nor *nor)
>  }
>  
>  /**
> - * spansion_quad_enable() - set QE bit in Configuraiton Register.
> - * @nor:	pointer to a 'struct spi_nor'
> - *
> - * Set the Quad Enable (QE) bit in the Configuration Register.
> - * This function is kept for legacy purpose because it has been used for a
> - * long time without anybody complaining but it should be considered as
> - * deprecated and maybe buggy.
> - * First, this function doesn't care about the previous values of the Status
> - * and Configuration Registers when it sets the QE bit (bit 1) in the
> - * Configuration Register: all other bits are cleared, which may have unwanted
> - * side effects like removing some block protections.
> - * Secondly, it uses the Read Configuration Register (35h) instruction though
> - * some very old and few memories don't support this instruction. If a pull-up
> - * resistor is present on the MISO/IO1 line, we might still be able to pass the
> - * "read back" test because the QSPI memory doesn't recognize the command,
> - * so leaves the MISO/IO1 line state unchanged, hence spi_nor_read_cr() returns
> - * 0xFF.
> - *
> - * bit 1 of the Configuration Register is the QE bit for Spansion like QSPI
> - * memories.
> - *
> - * Return: 0 on success, -errno otherwise.
> - */
> -static int spansion_quad_enable(struct spi_nor *nor)
> -{
> -	u8 *sr_cr = nor->bouncebuf;
> -	int ret;
> -
> -	sr_cr[0] = 0;
> -	sr_cr[1] = CR_QUAD_EN_SPAN;
> -	ret = spi_nor_write_sr(nor, sr_cr, 2);
> -	if (ret)
> -		return ret;
> -
> -	/* read back and check it */
> -	ret = spi_nor_read_cr(nor, nor->bouncebuf);
> -	if (ret)
> -		return ret;
> -
> -	if (!(nor->bouncebuf[0] & CR_QUAD_EN_SPAN)) {
> -		dev_dbg(nor->dev, "Spansion Quad bit not set\n");
> -		return -EINVAL;
> -	}
> -
> -	return 0;
> -}
> -
> -/**
>   * spansion_no_read_cr_quad_enable() - set QE bit in Configuration Register.
>   * @nor:	pointer to a 'struct spi_nor'
>   *
> @@ -2170,9 +2122,9 @@ static int spi_nor_clear_sr_bp(struct spi_nor *nor)
>   *
>   * Read-modify-write function that clears the Block Protection bits from the
>   * Status Register without affecting other bits. The function is tightly
> - * coupled with the spansion_quad_enable() function. Both assume that the Write
> - * Register with 16 bits, together with the Read Configuration Register (35h)
> - * instructions are supported.
> + * coupled with the spansion_read_cr_quad_enable() function. Both assume that
> + * the Write Register with 16 bits, together with the Read Configuration
> + * Register (35h) instructions are supported.
>   *
>   * Return: 0 on success, -errno otherwise.
>   */
> @@ -4654,7 +4606,7 @@ static void spi_nor_info_init_params(struct spi_nor *nor)
>  	u8 i, erase_mask;
>  
>  	/* Initialize legacy flash parameters and settings. */
> -	params->quad_enable = spansion_quad_enable;
> +	params->quad_enable = spansion_read_cr_quad_enable;
>  	params->set_4byte = spansion_set_4byte;
>  	params->setup = spi_nor_default_setup;
>  
> @@ -4869,7 +4821,7 @@ static int spi_nor_init(struct spi_nor *nor)
>  	int err;
>  
>  	if (nor->clear_sr_bp) {
> -		if (nor->params.quad_enable == spansion_quad_enable)
> +		if (nor->params.quad_enable == spansion_read_cr_quad_enable)
>  			nor->clear_sr_bp = spi_nor_spansion_clear_sr_bp;
>  
>  		err = nor->clear_sr_bp(nor);
> 

-- 
Regards
Vignesh

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

[PATCH v4 10/20] mtd: spi-nor: Fix errno on Quad Enable methods

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

When the Read-Modify-Write-Read-Back Quad Enable methods failed on
the Read-Back, they returned -EINVAL. Since this is an I/O error,
return -EIO.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 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 f5193733a0f6..14146619bf19 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1964,7 +1964,7 @@ static int macronix_quad_enable(struct spi_nor *nor)
 
 	if (!(nor->bouncebuf[0] & SR_QUAD_EN_MX)) {
 		dev_dbg(nor->dev, "Macronix Quad bit not set\n");
-		return -EINVAL;
+		return -EIO;
 	}
 
 	return 0;
@@ -2042,7 +2042,7 @@ static int spansion_read_cr_quad_enable(struct spi_nor *nor)
 
 	if (!(sr_cr[1] & CR_QUAD_EN_SPAN)) {
 		dev_dbg(nor->dev, "Spansion Quad bit not set\n");
-		return -EINVAL;
+		return -EIO;
 	}
 
 	return 0;
@@ -2086,7 +2086,7 @@ static int sr2_bit7_quad_enable(struct spi_nor *nor)
 
 	if (!(*sr2 & SR2_QUAD_EN_BIT7)) {
 		dev_dbg(nor->dev, "SR2 Quad bit not set\n");
-		return -EINVAL;
+		return -EIO;
 	}
 
 	return 0;
-- 
2.9.5


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

Re: [PATCH v4 10/20] mtd: spi-nor: Fix errno on Quad Enable methods

[Vignesh Raghavendra]

On 02/11/19 4:53 PM, Tudor.Ambarus@microchip.com wrote:
> From: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
> When the Read-Modify-Write-Read-Back Quad Enable methods failed on
> the Read-Back, they returned -EINVAL. Since this is an I/O error,
> return -EIO.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>

Reviewed-by: Vignesh Raghavendra <vigneshr@ti.com>

> ---
>  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 f5193733a0f6..14146619bf19 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -1964,7 +1964,7 @@ static int macronix_quad_enable(struct spi_nor *nor)
>  
>  	if (!(nor->bouncebuf[0] & SR_QUAD_EN_MX)) {
>  		dev_dbg(nor->dev, "Macronix Quad bit not set\n");
> -		return -EINVAL;
> +		return -EIO;
>  	}
>  
>  	return 0;
> @@ -2042,7 +2042,7 @@ static int spansion_read_cr_quad_enable(struct spi_nor *nor)
>  
>  	if (!(sr_cr[1] & CR_QUAD_EN_SPAN)) {
>  		dev_dbg(nor->dev, "Spansion Quad bit not set\n");
> -		return -EINVAL;
> +		return -EIO;
>  	}
>  
>  	return 0;
> @@ -2086,7 +2086,7 @@ static int sr2_bit7_quad_enable(struct spi_nor *nor)
>  
>  	if (!(*sr2 & SR2_QUAD_EN_BIT7)) {
>  		dev_dbg(nor->dev, "SR2 Quad bit not set\n");
> -		return -EINVAL;
> +		return -EIO;
>  	}
>  
>  	return 0;
> 

-- 
Regards
Vignesh

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

[PATCH v4 11/20] mtd: spi-nor: Check all the bits written, not just the BP ones

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

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 <tudor.ambarus@microchip.com>
---
 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 14146619bf19..8f5e9833081b 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


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

Re: [PATCH v4 11/20] mtd: spi-nor: Check all the bits written, not just the BP ones

[Vignesh Raghavendra]

On 02/11/19 4:53 PM, Tudor.Ambarus@microchip.com wrote:
> From: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
> 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 <tudor.ambarus@microchip.com>
> ---

Reviewed-by: Vignesh Raghavendra <vigneshr@ti.com>

Regards
Vignesh

>  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 14146619bf19..8f5e9833081b 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);
>  }
>  
>  /*
> 

-- 
Regards
Vignesh

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

[PATCH v4 12/20] mtd: spi-nor: Print debug message when the read back test fails

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

Demystify where the EIO error occurs.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 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 8f5e9833081b..725dab241271 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_dbg(nor->dev, "SR: read back test failed\n");
+		return -EIO;
+	}
+
+	return 0;
 }
 
 /**
-- 
2.9.5


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

Re: [PATCH v4 12/20] mtd: spi-nor: Print debug message when the read back test fails

[Vignesh Raghavendra]

On 02/11/19 4:53 PM, Tudor.Ambarus@microchip.com wrote:
> From: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
> Demystify where the EIO error occurs.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---

I think this is a small enough change that can be squashed into previous
patch itself

>  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 8f5e9833081b..725dab241271 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_dbg(nor->dev, "SR: read back test failed\n");
> +		return -EIO;
> +	}
> +
> +	return 0;
>  }
>  
>  /**
> 

-- 
Regards
Vignesh

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

Re: [PATCH v4 12/20] mtd: spi-nor: Print debug message when the read back test fails

[Tudor.Ambarus]

On 11/05/2019 02:37 PM, Vignesh Raghavendra wrote:
> On 02/11/19 4:53 PM, Tudor.Ambarus@microchip.com wrote:
>> From: Tudor Ambarus <tudor.ambarus@microchip.com>
>>
>> Demystify where the EIO error occurs.
>>
>> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>> ---
> I think this is a small enough change that can be squashed into previous
> patch itself
> 

I made separate patches because this is a separate logical change. The previous
patch extends the check on all bits of the Status Register, while this one
prints a debug message in case of EIO. Thus I tried to have a single logical
change contained in a single patch. I'm clearly no expert in this (Boris asked
me in v3 to split patches because I did too many things in one patch :) ), so I
would keep this as is, but if you still feel that it should be squashed, then
I'll do it. Please let me know.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4 12/20] mtd: spi-nor: Print debug message when the read back test fails

[Vignesh Raghavendra]

On 06/11/19 12:54 PM, Tudor.Ambarus@microchip.com wrote:
> 
> 
> On 11/05/2019 02:37 PM, Vignesh Raghavendra wrote:
>> On 02/11/19 4:53 PM, Tudor.Ambarus@microchip.com wrote:
>>> From: Tudor Ambarus <tudor.ambarus@microchip.com>
>>>
>>> Demystify where the EIO error occurs.
>>>
>>> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>>> ---
>> I think this is a small enough change that can be squashed into previous
>> patch itself
>>
> 
> I made separate patches because this is a separate logical change. The previous
> patch extends the check on all bits of the Status Register, while this one
> prints a debug message in case of EIO. Thus I tried to have a single logical
> change contained in a single patch. I'm clearly no expert in this (Boris asked
> me in v3 to split patches because I did too many things in one patch :) ), so I
> would keep this as is, but if you still feel that it should be squashed, then
> I'll do it. Please let me know.
> 

I am fine either way. I don't have a strong preference...

-- 
Regards
Vignesh

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

Re: [PATCH v4 12/20] mtd: spi-nor: Print debug message when the read back test fails

[Vignesh Raghavendra]

On 06/11/19 1:09 PM, Vignesh Raghavendra wrote:
> 
> 
> On 06/11/19 12:54 PM, Tudor.Ambarus@microchip.com wrote:
>>
>>
>> On 11/05/2019 02:37 PM, Vignesh Raghavendra wrote:
>>> On 02/11/19 4:53 PM, Tudor.Ambarus@microchip.com wrote:
>>>> From: Tudor Ambarus <tudor.ambarus@microchip.com>
>>>>
>>>> Demystify where the EIO error occurs.
>>>>
>>>> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>>>> ---
>>> I think this is a small enough change that can be squashed into previous
>>> patch itself
>>>
>>
>> I made separate patches because this is a separate logical change. The previous
>> patch extends the check on all bits of the Status Register, while this one
>> prints a debug message in case of EIO. Thus I tried to have a single logical
>> change contained in a single patch. I'm clearly no expert in this (Boris asked
>> me in v3 to split patches because I did too many things in one patch :) ), so I
>> would keep this as is, but if you still feel that it should be squashed, then
>> I'll do it. Please let me know.
>>
> 
> I am fine either way. I don't have a strong preference...
> 

If you want to keep these separate:

Reviewed-by: Vignesh Raghavendra <vigneshr@ti.com>

Regards
Vignesh


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

[PATCH v4 13/20] mtd: spi-nor: Fix clearing of QE bit on lock()/unlock()

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

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 <boris.brezillon@collabora.com>
Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 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 725dab241271..f96bc80c0ed1 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, 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_dbg(nor->dev, "SR: read back test failed\n");
+	if (nor->bouncebuf[0] != sr1) {
+		dev_dbg(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_dbg(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


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

Re: [PATCH v4 13/20] mtd: spi-nor: Fix clearing of QE bit on lock()/unlock()

[Vignesh Raghavendra]

On 02-Nov-19 4:53 PM, Tudor.Ambarus@microchip.com wrote:
> From: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
> 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.
>

But I see that 1 byte SR1 write still happens as part of
spi_nor_clear_sr_bp() until patch 20/20. So is this only a partial fix?
Should this patch be rearranged to appear along with 20/20?


> Suggested-by: Boris Brezillon <boris.brezillon@collabora.com>
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
>  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 725dab241271..f96bc80c0ed1 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);
>  }
>  
[...]
> +/**
>   * 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;

Since SNOR_F_HAS_16BIT_SR is set by default in
spi_nor_info_init_params(), no need to set the flag here again

>  		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;
> +

Same here...

>  		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),
> +
>  };
>  
>  /**
> 

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

Re: [PATCH v4 13/20] mtd: spi-nor: Fix clearing of QE bit on lock()/unlock()

[Tudor.Ambarus]

On 11/05/2019 07:07 PM, Vignesh Raghavendra wrote:
> On 02-Nov-19 4:53 PM, Tudor.Ambarus@microchip.com wrote:
>> From: Tudor Ambarus <tudor.ambarus@microchip.com>
>>
>> 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.
>>
> But I see that 1 byte SR1 write still happens as part of
> spi_nor_clear_sr_bp() until patch 20/20. So is this only a partial fix?

Fixing spi_nor_clear_sr_bp() would mean to add dead code that will be removed
anyway with patch 20/20. This patch fixes the clearing of the QE bit, while in
20/20 the QE bit is already zero when the one byte SR1 write is used, so the
quad mode is not affected. 20/20 fixes indirectly the clearing of all the bits
from SR2 but QE bit, because it's already zero. I would say it's not a partial
fix, but a different bug.

There are different angles to look at this, I chose the modifying of the quad
mode angle. Given the two arguments from above (avoid adding dead code and
changing of quad mode approach), I would prefer to keep things as they are. But
I get your approach too, so if you still want yours, I can do it. Please let me
know.

> Should this patch be rearranged to appear along with 20/20?

Not necessarily (different bugs) but I can bring 20/20 immediately after this
one if you want.

> 
> 
>> Suggested-by: Boris Brezillon <boris.brezillon@collabora.com>
>> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>> ---
>>  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 725dab241271..f96bc80c0ed1 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);
>>  }
>>  
> [...]
>> +/**
>>   * 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;
> Since SNOR_F_HAS_16BIT_SR is set by default in
> spi_nor_info_init_params(), no need to set the flag here again
> 

I did this on purpose. I set SNOR_F_HAS_16BIT_SR here based on SFDP standard, I
want to indicate where the standard requires the 16 bit SR write .
spi_nor_info_init_params() initializes data based on info, but that data can be
overwritten (even with the same data) when parsing SFDP.

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

Re: [PATCH v4 13/20] mtd: spi-nor: Fix clearing of QE bit on lock()/unlock()

[Vignesh Raghavendra]

Hi,

On 06/11/19 2:03 PM, Tudor.Ambarus@microchip.com wrote:
> 
> 
> On 11/05/2019 07:07 PM, Vignesh Raghavendra wrote:
>> On 02-Nov-19 4:53 PM, Tudor.Ambarus@microchip.com wrote:
>>> From: Tudor Ambarus <tudor.ambarus@microchip.com>
>>>
>>> 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.
>>>
>> But I see that 1 byte SR1 write still happens as part of
>> spi_nor_clear_sr_bp() until patch 20/20. So is this only a partial fix?
> 
> Fixing spi_nor_clear_sr_bp() would mean to add dead code that will be removed
> anyway with patch 20/20. This patch fixes the clearing of the QE bit, while in
> 20/20 the QE bit is already zero when the one byte SR1 write is used, so the
> quad mode is not affected. 20/20 fixes indirectly the clearing of all the bits
> from SR2 but QE bit, because it's already zero. I would say it's not a partial
> fix, but a different bug.
> 

I was not suggesting on fixing up spi_nor_clear_sr_bp(), but pointing
out that single byte writes SR1 can happen until patch 20/20.

But now I see that these patches are fixing related but different bugs.

> There are different angles to look at this, I chose the modifying of the quad
> mode angle. Given the two arguments from above (avoid adding dead code and
> changing of quad mode approach), I would prefer to keep things as they are. 

Ok, sounds fine, no need to change...

> But I get your approach too, so if you still want yours, I can do it. Please let me
> know.
> 
>> Should this patch be rearranged to appear along with 20/20?
> 
> Not necessarily (different bugs) but I can bring 20/20 immediately after this
> one if you want.
> >>
>>
>>> Suggested-by: Boris Brezillon <boris.brezillon@collabora.com>
>>> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>>> ---
>>>  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 725dab241271..f96bc80c0ed1 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);
>>>  }
>>>  
>> [...]
>>> +/**
>>>   * 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;
>> Since SNOR_F_HAS_16BIT_SR is set by default in
>> spi_nor_info_init_params(), no need to set the flag here again
>>
> 
> I did this on purpose. I set SNOR_F_HAS_16BIT_SR here based on SFDP standard, I
> want to indicate where the standard requires the 16 bit SR write .
> spi_nor_info_init_params() initializes data based on info, but that data can be
> overwritten (even with the same data) when parsing SFDP.
> 

Alright.

Reviewed-by: Vignesh Raghavendra <vigneshr@ti.com>

Regards
Vignesh

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

[PATCH v4 14/20] mtd: spi-nor: Extend the QE Read Back test to the entire SR byte

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

Extend the Quad Enable Read Back test to the entire Status
Register byte. Make sure that other bits were not changed.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 29 +++++++++++------------------
 1 file changed, 11 insertions(+), 18 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index f96bc80c0ed1..08fd2c97897d 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -2047,20 +2047,7 @@ static int macronix_quad_enable(struct spi_nor *nor)
 
 	nor->bouncebuf[0] |= SR_QUAD_EN_MX;
 
-	ret = spi_nor_write_sr(nor, nor->bouncebuf, 1);
-	if (ret)
-		return ret;
-
-	ret = spi_nor_read_sr(nor, nor->bouncebuf);
-	if (ret)
-		return ret;
-
-	if (!(nor->bouncebuf[0] & SR_QUAD_EN_MX)) {
-		dev_dbg(nor->dev, "Macronix Quad bit not set\n");
-		return -EIO;
-	}
-
-	return 0;
+	return spi_nor_write_sr1_and_check(nor, nor->bouncebuf[0]);
 }
 
 /**
@@ -2108,6 +2095,7 @@ static int spansion_read_cr_quad_enable(struct spi_nor *nor)
 {
 	u8 *sr_cr = nor->bouncebuf;
 	int ret;
+	u8 sr_written;
 
 	/* Check current Quad Enable bit value. */
 	ret = spi_nor_read_cr(nor, &sr_cr[1]);
@@ -2128,13 +2116,15 @@ static int spansion_read_cr_quad_enable(struct spi_nor *nor)
 	if (ret)
 		return ret;
 
+	sr_written = sr_cr[1];
+
 	/* Read back and check it. */
 	ret = spi_nor_read_cr(nor, &sr_cr[1]);
 	if (ret)
 		return ret;
 
-	if (!(sr_cr[1] & CR_QUAD_EN_SPAN)) {
-		dev_dbg(nor->dev, "Spansion Quad bit not set\n");
+	if (sr_cr[1] != sr_written) {
+		dev_dbg(nor->dev, "CR: Read back test failed\n");
 		return -EIO;
 	}
 
@@ -2157,6 +2147,7 @@ static int sr2_bit7_quad_enable(struct spi_nor *nor)
 {
 	u8 *sr2 = nor->bouncebuf;
 	int ret;
+	u8 sr2_written;
 
 	/* Check current Quad Enable bit value. */
 	ret = spi_nor_read_sr2(nor, sr2);
@@ -2172,13 +2163,15 @@ static int sr2_bit7_quad_enable(struct spi_nor *nor)
 	if (ret)
 		return ret;
 
+	sr2_written = *sr2;
+
 	/* Read back and check it. */
 	ret = spi_nor_read_sr2(nor, sr2);
 	if (ret)
 		return ret;
 
-	if (!(*sr2 & SR2_QUAD_EN_BIT7)) {
-		dev_dbg(nor->dev, "SR2 Quad bit not set\n");
+	if (*sr2 != sr2_written) {
+		dev_dbg(nor->dev, "SR2: Read back test failed\n");
 		return -EIO;
 	}
 
-- 
2.9.5


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

[PATCH v4 15/20] mtd: spi-nor: Extend the QE Read Back test to both SR1 and SR2

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

In case of 16-bit Write Status Register, check that both SR1 and
SR2 were written correctly.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 08fd2c97897d..8f11c00e8ae5 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -2067,6 +2067,7 @@ static int spansion_no_read_cr_quad_enable(struct spi_nor *nor)
 {
 	u8 *sr_cr = nor->bouncebuf;
 	int ret;
+	u8 sr_written;
 
 	/* Keep the current value of the Status Register. */
 	ret = spi_nor_read_sr(nor, sr_cr);
@@ -2075,7 +2076,22 @@ static int spansion_no_read_cr_quad_enable(struct spi_nor *nor)
 
 	sr_cr[1] = CR_QUAD_EN_SPAN;
 
-	return spi_nor_write_sr(nor, sr_cr, 2);
+	ret = spi_nor_write_sr(nor, sr_cr, 2);
+	if (ret)
+		return ret;
+
+	sr_written = sr_cr[0];
+
+	ret = spi_nor_read_sr(nor, sr_cr);
+	if (ret)
+		return ret;
+
+	if (sr_cr[0] != sr_written) {
+		dev_err(nor->dev, "SR: Read back test failed\n");
+		return -EIO;
+	}
+
+	return 0;
 }
 
 /**
@@ -2116,6 +2132,17 @@ static int spansion_read_cr_quad_enable(struct spi_nor *nor)
 	if (ret)
 		return ret;
 
+	sr_written = sr_cr[0];
+
+	ret = spi_nor_read_sr(nor, sr_cr);
+	if (ret)
+		return ret;
+
+	if (sr_written != sr_cr[0]) {
+		dev_err(nor->dev, "SR: Read back test failed\n");
+		return -EIO;
+	}
+
 	sr_written = sr_cr[1];
 
 	/* Read back and check it. */
-- 
2.9.5


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

Re: [PATCH v4 15/20] mtd: spi-nor: Extend the QE Read Back test to both SR1 and SR2

[Vignesh Raghavendra]

On 02/11/19 4:53 PM, Tudor.Ambarus@microchip.com wrote:
> From: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
> In case of 16-bit Write Status Register, check that both SR1 and
> SR2 were written correctly.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---

This can be merged with previous patch as changes are quite similar

Regards
Vignesh

>  drivers/mtd/spi-nor/spi-nor.c | 29 ++++++++++++++++++++++++++++-
>  1 file changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 08fd2c97897d..8f11c00e8ae5 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -2067,6 +2067,7 @@ static int spansion_no_read_cr_quad_enable(struct spi_nor *nor)
>  {
>  	u8 *sr_cr = nor->bouncebuf;
>  	int ret;
> +	u8 sr_written;
>  
>  	/* Keep the current value of the Status Register. */
>  	ret = spi_nor_read_sr(nor, sr_cr);
> @@ -2075,7 +2076,22 @@ static int spansion_no_read_cr_quad_enable(struct spi_nor *nor)
>  
>  	sr_cr[1] = CR_QUAD_EN_SPAN;
>  
> -	return spi_nor_write_sr(nor, sr_cr, 2);
> +	ret = spi_nor_write_sr(nor, sr_cr, 2);
> +	if (ret)
> +		return ret;
> +
> +	sr_written = sr_cr[0];
> +
> +	ret = spi_nor_read_sr(nor, sr_cr);
> +	if (ret)
> +		return ret;
> +
> +	if (sr_cr[0] != sr_written) {
> +		dev_err(nor->dev, "SR: Read back test failed\n");
> +		return -EIO;
> +	}
> +
> +	return 0;
>  }
>  
>  /**
> @@ -2116,6 +2132,17 @@ static int spansion_read_cr_quad_enable(struct spi_nor *nor)
>  	if (ret)
>  		return ret;
>  
> +	sr_written = sr_cr[0];
> +
> +	ret = spi_nor_read_sr(nor, sr_cr);
> +	if (ret)
> +		return ret;
> +
> +	if (sr_written != sr_cr[0]) {
> +		dev_err(nor->dev, "SR: Read back test failed\n");
> +		return -EIO;
> +	}
> +
>  	sr_written = sr_cr[1];
>  
>  	/* Read back and check it. */
> 

-- 
Regards
Vignesh

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

Re: [PATCH v4 15/20] mtd: spi-nor: Extend the QE Read Back test to both SR1 and SR2

[Tudor.Ambarus]

On 11/05/2019 06:06 PM, Vignesh Raghavendra wrote:
>> From: Tudor Ambarus <tudor.ambarus@microchip.com>
>>
>> In case of 16-bit Write Status Register, check that both SR1 and
>> SR2 were written correctly.
>>
>> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>> ---
> This can be merged with previous patch as changes are quite similar

Ok, I'll rename the squashed patch to "Extend the SR read back test".

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

[PATCH v4 16/20] mtd: spi-nor: Rename CR_QUAD_EN_SPAN to SR2_QUAD_EN_BIT1

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

JEDEC Basic Flash Parameter Table, 15th DWORD, bits 22:20,
refers to this bit as "bit 1 of the status register 2".
Rename the macro accordingly.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 10 +++++-----
 include/linux/mtd/spi-nor.h   |  4 +---
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 8f11c00e8ae5..e367a4862ec1 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1026,7 +1026,7 @@ static int spi_nor_write_16bit_sr_and_check(struct spi_nor *nor, u8 sr1)
 		 * 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;
+		sr_cr[1] = SR2_QUAD_EN_BIT1;
 	} else {
 		sr_cr[1] = 0;
 	}
@@ -2074,7 +2074,7 @@ static int spansion_no_read_cr_quad_enable(struct spi_nor *nor)
 	if (ret)
 		return ret;
 
-	sr_cr[1] = CR_QUAD_EN_SPAN;
+	sr_cr[1] = SR2_QUAD_EN_BIT1;
 
 	ret = spi_nor_write_sr(nor, sr_cr, 2);
 	if (ret)
@@ -2118,10 +2118,10 @@ static int spansion_read_cr_quad_enable(struct spi_nor *nor)
 	if (ret)
 		return ret;
 
-	if (sr_cr[1] & CR_QUAD_EN_SPAN)
+	if (sr_cr[1] & SR2_QUAD_EN_BIT1)
 		return 0;
 
-	sr_cr[1] |= CR_QUAD_EN_SPAN;
+	sr_cr[1] |= SR2_QUAD_EN_BIT1;
 
 	/* Keep the current value of the Status Register. */
 	ret = spi_nor_read_sr(nor, sr_cr);
@@ -2256,7 +2256,7 @@ static int spi_nor_spansion_clear_sr_bp(struct spi_nor *nor)
 	 * When the configuration register Quad Enable bit is one, only the
 	 * Write Status (01h) command with two data bytes may be used.
 	 */
-	if (sr_cr[1] & CR_QUAD_EN_SPAN) {
+	if (sr_cr[1] & SR2_QUAD_EN_BIT1) {
 		ret = spi_nor_read_sr(nor, sr_cr);
 		if (ret)
 			return ret;
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index d6ec55cc6d97..f626e0e52909 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -144,10 +144,8 @@
 #define FSR_P_ERR		BIT(4)	/* Program operation status */
 #define FSR_PT_ERR		BIT(1)	/* Protection error bit */
 
-/* Configuration Register bits. */
-#define CR_QUAD_EN_SPAN		BIT(1)	/* Spansion Quad I/O */
-
 /* Status Register 2 bits. */
+#define SR2_QUAD_EN_BIT1	BIT(1)
 #define SR2_QUAD_EN_BIT7	BIT(7)
 
 /* Supported SPI protocols */
-- 
2.9.5


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

Re: [PATCH v4 16/20] mtd: spi-nor: Rename CR_QUAD_EN_SPAN to SR2_QUAD_EN_BIT1

[Vignesh Raghavendra]

On 02/11/19 4:53 PM, Tudor.Ambarus@microchip.com wrote:
> From: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
> JEDEC Basic Flash Parameter Table, 15th DWORD, bits 22:20,
> refers to this bit as "bit 1 of the status register 2".
> Rename the macro accordingly.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>

Reviewed-by: Vignesh Raghavendra <vigneshr@ti.com>

Regards
Vignesh

> ---
>  drivers/mtd/spi-nor/spi-nor.c | 10 +++++-----
>  include/linux/mtd/spi-nor.h   |  4 +---
>  2 files changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 8f11c00e8ae5..e367a4862ec1 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -1026,7 +1026,7 @@ static int spi_nor_write_16bit_sr_and_check(struct spi_nor *nor, u8 sr1)
>  		 * 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;
> +		sr_cr[1] = SR2_QUAD_EN_BIT1;
>  	} else {
>  		sr_cr[1] = 0;
>  	}
> @@ -2074,7 +2074,7 @@ static int spansion_no_read_cr_quad_enable(struct spi_nor *nor)
>  	if (ret)
>  		return ret;
>  
> -	sr_cr[1] = CR_QUAD_EN_SPAN;
> +	sr_cr[1] = SR2_QUAD_EN_BIT1;
>  
>  	ret = spi_nor_write_sr(nor, sr_cr, 2);
>  	if (ret)
> @@ -2118,10 +2118,10 @@ static int spansion_read_cr_quad_enable(struct spi_nor *nor)
>  	if (ret)
>  		return ret;
>  
> -	if (sr_cr[1] & CR_QUAD_EN_SPAN)
> +	if (sr_cr[1] & SR2_QUAD_EN_BIT1)
>  		return 0;
>  
> -	sr_cr[1] |= CR_QUAD_EN_SPAN;
> +	sr_cr[1] |= SR2_QUAD_EN_BIT1;
>  
>  	/* Keep the current value of the Status Register. */
>  	ret = spi_nor_read_sr(nor, sr_cr);
> @@ -2256,7 +2256,7 @@ static int spi_nor_spansion_clear_sr_bp(struct spi_nor *nor)
>  	 * When the configuration register Quad Enable bit is one, only the
>  	 * Write Status (01h) command with two data bytes may be used.
>  	 */
> -	if (sr_cr[1] & CR_QUAD_EN_SPAN) {
> +	if (sr_cr[1] & SR2_QUAD_EN_BIT1) {
>  		ret = spi_nor_read_sr(nor, sr_cr);
>  		if (ret)
>  			return ret;
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index d6ec55cc6d97..f626e0e52909 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -144,10 +144,8 @@
>  #define FSR_P_ERR		BIT(4)	/* Program operation status */
>  #define FSR_PT_ERR		BIT(1)	/* Protection error bit */
>  
> -/* Configuration Register bits. */
> -#define CR_QUAD_EN_SPAN		BIT(1)	/* Spansion Quad I/O */
> -
>  /* Status Register 2 bits. */
> +#define SR2_QUAD_EN_BIT1	BIT(1)
>  #define SR2_QUAD_EN_BIT7	BIT(7)
>  
>  /* Supported SPI protocols */
> 

-- 
Regards
Vignesh

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

[PATCH v4 17/20] mtd: spi-nor: Merge spansion Quad Enable methods

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

Merge
    spansion_no_read_cr_quad_enable()
    spansion_read_cr_quad_enable()
into
    spi_nor_sr2_bit1_quad_enable().

Reduce code duplication by introducing spi_nor_write_16bit_cr_and_check().
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.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 165 +++++++++++++++++-------------------------
 1 file changed, 68 insertions(+), 97 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index e367a4862ec1..8bc29cc073a0 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1055,6 +1055,59 @@ static int spi_nor_write_16bit_sr_and_check(struct spi_nor *nor, u8 sr1)
 }
 
 /**
+ * spi_nor_write_16bit_cr_and_check() - Write the Status Register 1 and the
+ * Configuration Register in one shot. Ensure that the byte written in the
+ * Configuration Register match the received value, and that the 16-bit Write
+ * did not affect what was already in the Status Register 1.
+ * @nor:	pointer to a 'struct spi_nor'.
+ * @cr:		byte value to be written to the Configuration Register.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_write_16bit_cr_and_check(struct spi_nor *nor, u8 cr)
+{
+	int ret;
+	u8 *sr_cr = nor->bouncebuf;
+	u8 sr_written;
+
+	/* Keep the current value of the Status Register 1. */
+	ret = spi_nor_read_sr(nor, sr_cr);
+	if (ret)
+		return ret;
+
+	sr_cr[1] = cr;
+
+	ret = spi_nor_write_sr(nor, sr_cr, 2);
+	if (ret)
+		return ret;
+
+	sr_written = sr_cr[0];
+
+	ret = spi_nor_read_sr(nor, sr_cr);
+	if (ret)
+		return ret;
+
+	if (sr_written != sr_cr[0]) {
+		dev_dbg(nor->dev, "SR: Read back test failed\n");
+		return -EIO;
+	}
+
+	if (nor->flags & SNOR_F_NO_READ_CR)
+		return 0;
+
+	ret = spi_nor_read_cr(nor, &sr_cr[1]);
+	if (ret)
+		return ret;
+
+	if (cr != sr_cr[1]) {
+		dev_dbg(nor->dev, "CR: read back test failed\n");
+		return -EIO;
+	}
+
+	return 0;
+}
+
+/**
  * 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.
@@ -2051,111 +2104,29 @@ static int macronix_quad_enable(struct spi_nor *nor)
 }
 
 /**
- * spansion_no_read_cr_quad_enable() - set QE bit in Configuration Register.
- * @nor:	pointer to a 'struct spi_nor'
+ * spi_nor_sr2_bit1_quad_enable() - set the Quad Enable BIT(1) in the Status
+ * Register 2.
+ * @nor:       pointer to a 'struct spi_nor'.
  *
- * Set the Quad Enable (QE) bit in the Configuration Register.
- * This function should be used with QSPI memories not supporting the Read
- * Configuration Register (35h) instruction.
- *
- * bit 1 of the Configuration Register is the QE bit for Spansion like QSPI
- * memories.
+ * Bit 1 of the Status Register 2 is the QE bit for Spansion like QSPI memories.
  *
  * Return: 0 on success, -errno otherwise.
  */
-static int spansion_no_read_cr_quad_enable(struct spi_nor *nor)
+static int spi_nor_sr2_bit1_quad_enable(struct spi_nor *nor)
 {
-	u8 *sr_cr = nor->bouncebuf;
 	int ret;
-	u8 sr_written;
-
-	/* Keep the current value of the Status Register. */
-	ret = spi_nor_read_sr(nor, sr_cr);
-	if (ret)
-		return ret;
-
-	sr_cr[1] = SR2_QUAD_EN_BIT1;
-
-	ret = spi_nor_write_sr(nor, sr_cr, 2);
-	if (ret)
-		return ret;
 
-	sr_written = sr_cr[0];
-
-	ret = spi_nor_read_sr(nor, sr_cr);
-	if (ret)
-		return ret;
-
-	if (sr_cr[0] != sr_written) {
-		dev_err(nor->dev, "SR: Read back test failed\n");
-		return -EIO;
-	}
-
-	return 0;
-}
-
-/**
- * spansion_read_cr_quad_enable() - set QE bit in Configuration Register.
- * @nor:	pointer to a 'struct spi_nor'
- *
- * Set the Quad Enable (QE) bit in the Configuration Register.
- * This function should be used with QSPI memories supporting the Read
- * Configuration Register (35h) instruction.
- *
- * bit 1 of the Configuration Register is the QE bit for Spansion like QSPI
- * memories.
- *
- * Return: 0 on success, -errno otherwise.
- */
-static int spansion_read_cr_quad_enable(struct spi_nor *nor)
-{
-	u8 *sr_cr = nor->bouncebuf;
-	int ret;
-	u8 sr_written;
+	if (nor->flags & SNOR_F_NO_READ_CR)
+		return spi_nor_write_16bit_cr_and_check(nor, SR2_QUAD_EN_BIT1);
 
-	/* Check current Quad Enable bit value. */
-	ret = spi_nor_read_cr(nor, &sr_cr[1]);
+	ret = spi_nor_read_cr(nor, nor->bouncebuf);
 	if (ret)
 		return ret;
 
-	if (sr_cr[1] & SR2_QUAD_EN_BIT1)
+	if (nor->bouncebuf[0] & SR2_QUAD_EN_BIT1)
 		return 0;
 
-	sr_cr[1] |= SR2_QUAD_EN_BIT1;
-
-	/* Keep the current value of the Status Register. */
-	ret = spi_nor_read_sr(nor, sr_cr);
-	if (ret)
-		return ret;
-
-	ret = spi_nor_write_sr(nor, sr_cr, 2);
-	if (ret)
-		return ret;
-
-	sr_written = sr_cr[0];
-
-	ret = spi_nor_read_sr(nor, sr_cr);
-	if (ret)
-		return ret;
-
-	if (sr_written != sr_cr[0]) {
-		dev_err(nor->dev, "SR: Read back test failed\n");
-		return -EIO;
-	}
-
-	sr_written = sr_cr[1];
-
-	/* Read back and check it. */
-	ret = spi_nor_read_cr(nor, &sr_cr[1]);
-	if (ret)
-		return ret;
-
-	if (sr_cr[1] != sr_written) {
-		dev_dbg(nor->dev, "CR: Read back test failed\n");
-		return -EIO;
-	}
-
-	return 0;
+	return spi_nor_write_16bit_cr_and_check(nor, nor->bouncebuf[0]);
 }
 
 /**
@@ -2235,7 +2206,7 @@ static int spi_nor_clear_sr_bp(struct spi_nor *nor)
  *
  * Read-modify-write function that clears the Block Protection bits from the
  * Status Register without affecting other bits. The function is tightly
- * coupled with the spansion_read_cr_quad_enable() function. Both assume that
+ * coupled with the spi_nor_sr2_bit1_quad_enable() function. Both assume that
  * the Write Register with 16 bits, together with the Read Configuration
  * Register (35h) instructions are supported.
  *
@@ -3753,7 +3724,7 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
 		 * supported.
 		 */
 		nor->flags |= SNOR_F_HAS_16BIT_SR | SNOR_F_NO_READ_CR;
-		params->quad_enable = spansion_no_read_cr_quad_enable;
+		params->quad_enable = spi_nor_sr2_bit1_quad_enable;
 		break;
 
 	case BFPT_DWORD15_QER_SR1_BIT6:
@@ -3775,7 +3746,7 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
 		 */
 		nor->flags |= SNOR_F_HAS_16BIT_SR;
 
-		params->quad_enable = spansion_read_cr_quad_enable;
+		params->quad_enable = spi_nor_sr2_bit1_quad_enable;
 		break;
 
 	default:
@@ -4738,7 +4709,7 @@ static void spi_nor_info_init_params(struct spi_nor *nor)
 	u8 i, erase_mask;
 
 	/* Initialize legacy flash parameters and settings. */
-	params->quad_enable = spansion_read_cr_quad_enable;
+	params->quad_enable = spi_nor_sr2_bit1_quad_enable;
 	params->set_4byte = spansion_set_4byte;
 	params->setup = spi_nor_default_setup;
 	/* Default to 16-bit Write Status (01h) Command */
@@ -4955,7 +4926,7 @@ static int spi_nor_init(struct spi_nor *nor)
 	int err;
 
 	if (nor->clear_sr_bp) {
-		if (nor->params.quad_enable == spansion_read_cr_quad_enable)
+		if (nor->params.quad_enable == spi_nor_sr2_bit1_quad_enable)
 			nor->clear_sr_bp = spi_nor_spansion_clear_sr_bp;
 
 		err = nor->clear_sr_bp(nor);
-- 
2.9.5


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

Re: [PATCH v4 17/20] mtd: spi-nor: Merge spansion Quad Enable methods

[Vignesh Raghavendra]

On 02/11/19 4:53 PM, Tudor.Ambarus@microchip.com wrote:
> From: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
> Merge
>     spansion_no_read_cr_quad_enable()
>     spansion_read_cr_quad_enable()
> into
>     spi_nor_sr2_bit1_quad_enable().
> 
> Reduce code duplication by introducing spi_nor_write_16bit_cr_and_check().
> 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.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>

Reviewed-by: Vignesh Raghavendra <vigneshr@ti.com>

Regards
Vignesh

> ---
>  drivers/mtd/spi-nor/spi-nor.c | 165 +++++++++++++++++-------------------------
>  1 file changed, 68 insertions(+), 97 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index e367a4862ec1..8bc29cc073a0 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -1055,6 +1055,59 @@ static int spi_nor_write_16bit_sr_and_check(struct spi_nor *nor, u8 sr1)
>  }
>  
>  /**
> + * spi_nor_write_16bit_cr_and_check() - Write the Status Register 1 and the
> + * Configuration Register in one shot. Ensure that the byte written in the
> + * Configuration Register match the received value, and that the 16-bit Write
> + * did not affect what was already in the Status Register 1.
> + * @nor:	pointer to a 'struct spi_nor'.
> + * @cr:		byte value to be written to the Configuration Register.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +static int spi_nor_write_16bit_cr_and_check(struct spi_nor *nor, u8 cr)
> +{
> +	int ret;
> +	u8 *sr_cr = nor->bouncebuf;
> +	u8 sr_written;
> +
> +	/* Keep the current value of the Status Register 1. */
> +	ret = spi_nor_read_sr(nor, sr_cr);
> +	if (ret)
> +		return ret;
> +
> +	sr_cr[1] = cr;
> +
> +	ret = spi_nor_write_sr(nor, sr_cr, 2);
> +	if (ret)
> +		return ret;
> +
> +	sr_written = sr_cr[0];
> +
> +	ret = spi_nor_read_sr(nor, sr_cr);
> +	if (ret)
> +		return ret;
> +
> +	if (sr_written != sr_cr[0]) {
> +		dev_dbg(nor->dev, "SR: Read back test failed\n");
> +		return -EIO;
> +	}
> +
> +	if (nor->flags & SNOR_F_NO_READ_CR)
> +		return 0;
> +
> +	ret = spi_nor_read_cr(nor, &sr_cr[1]);
> +	if (ret)
> +		return ret;
> +
> +	if (cr != sr_cr[1]) {
> +		dev_dbg(nor->dev, "CR: read back test failed\n");
> +		return -EIO;
> +	}
> +
> +	return 0;
> +}
> +
> +/**
>   * 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.
> @@ -2051,111 +2104,29 @@ static int macronix_quad_enable(struct spi_nor *nor)
>  }
>  
>  /**
> - * spansion_no_read_cr_quad_enable() - set QE bit in Configuration Register.
> - * @nor:	pointer to a 'struct spi_nor'
> + * spi_nor_sr2_bit1_quad_enable() - set the Quad Enable BIT(1) in the Status
> + * Register 2.
> + * @nor:       pointer to a 'struct spi_nor'.
>   *
> - * Set the Quad Enable (QE) bit in the Configuration Register.
> - * This function should be used with QSPI memories not supporting the Read
> - * Configuration Register (35h) instruction.
> - *
> - * bit 1 of the Configuration Register is the QE bit for Spansion like QSPI
> - * memories.
> + * Bit 1 of the Status Register 2 is the QE bit for Spansion like QSPI memories.
>   *
>   * Return: 0 on success, -errno otherwise.
>   */
> -static int spansion_no_read_cr_quad_enable(struct spi_nor *nor)
> +static int spi_nor_sr2_bit1_quad_enable(struct spi_nor *nor)
>  {
> -	u8 *sr_cr = nor->bouncebuf;
>  	int ret;
> -	u8 sr_written;
> -
> -	/* Keep the current value of the Status Register. */
> -	ret = spi_nor_read_sr(nor, sr_cr);
> -	if (ret)
> -		return ret;
> -
> -	sr_cr[1] = SR2_QUAD_EN_BIT1;
> -
> -	ret = spi_nor_write_sr(nor, sr_cr, 2);
> -	if (ret)
> -		return ret;
>  
> -	sr_written = sr_cr[0];
> -
> -	ret = spi_nor_read_sr(nor, sr_cr);
> -	if (ret)
> -		return ret;
> -
> -	if (sr_cr[0] != sr_written) {
> -		dev_err(nor->dev, "SR: Read back test failed\n");
> -		return -EIO;
> -	}
> -
> -	return 0;
> -}
> -
> -/**
> - * spansion_read_cr_quad_enable() - set QE bit in Configuration Register.
> - * @nor:	pointer to a 'struct spi_nor'
> - *
> - * Set the Quad Enable (QE) bit in the Configuration Register.
> - * This function should be used with QSPI memories supporting the Read
> - * Configuration Register (35h) instruction.
> - *
> - * bit 1 of the Configuration Register is the QE bit for Spansion like QSPI
> - * memories.
> - *
> - * Return: 0 on success, -errno otherwise.
> - */
> -static int spansion_read_cr_quad_enable(struct spi_nor *nor)
> -{
> -	u8 *sr_cr = nor->bouncebuf;
> -	int ret;
> -	u8 sr_written;
> +	if (nor->flags & SNOR_F_NO_READ_CR)
> +		return spi_nor_write_16bit_cr_and_check(nor, SR2_QUAD_EN_BIT1);
>  
> -	/* Check current Quad Enable bit value. */
> -	ret = spi_nor_read_cr(nor, &sr_cr[1]);
> +	ret = spi_nor_read_cr(nor, nor->bouncebuf);
>  	if (ret)
>  		return ret;
>  
> -	if (sr_cr[1] & SR2_QUAD_EN_BIT1)
> +	if (nor->bouncebuf[0] & SR2_QUAD_EN_BIT1)
>  		return 0;
>  
> -	sr_cr[1] |= SR2_QUAD_EN_BIT1;
> -
> -	/* Keep the current value of the Status Register. */
> -	ret = spi_nor_read_sr(nor, sr_cr);
> -	if (ret)
> -		return ret;
> -
> -	ret = spi_nor_write_sr(nor, sr_cr, 2);
> -	if (ret)
> -		return ret;
> -
> -	sr_written = sr_cr[0];
> -
> -	ret = spi_nor_read_sr(nor, sr_cr);
> -	if (ret)
> -		return ret;
> -
> -	if (sr_written != sr_cr[0]) {
> -		dev_err(nor->dev, "SR: Read back test failed\n");
> -		return -EIO;
> -	}
> -
> -	sr_written = sr_cr[1];
> -
> -	/* Read back and check it. */
> -	ret = spi_nor_read_cr(nor, &sr_cr[1]);
> -	if (ret)
> -		return ret;
> -
> -	if (sr_cr[1] != sr_written) {
> -		dev_dbg(nor->dev, "CR: Read back test failed\n");
> -		return -EIO;
> -	}
> -
> -	return 0;
> +	return spi_nor_write_16bit_cr_and_check(nor, nor->bouncebuf[0]);
>  }
>  
>  /**
> @@ -2235,7 +2206,7 @@ static int spi_nor_clear_sr_bp(struct spi_nor *nor)
>   *
>   * Read-modify-write function that clears the Block Protection bits from the
>   * Status Register without affecting other bits. The function is tightly
> - * coupled with the spansion_read_cr_quad_enable() function. Both assume that
> + * coupled with the spi_nor_sr2_bit1_quad_enable() function. Both assume that
>   * the Write Register with 16 bits, together with the Read Configuration
>   * Register (35h) instructions are supported.
>   *
> @@ -3753,7 +3724,7 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
>  		 * supported.
>  		 */
>  		nor->flags |= SNOR_F_HAS_16BIT_SR | SNOR_F_NO_READ_CR;
> -		params->quad_enable = spansion_no_read_cr_quad_enable;
> +		params->quad_enable = spi_nor_sr2_bit1_quad_enable;
>  		break;
>  
>  	case BFPT_DWORD15_QER_SR1_BIT6:
> @@ -3775,7 +3746,7 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
>  		 */
>  		nor->flags |= SNOR_F_HAS_16BIT_SR;
>  
> -		params->quad_enable = spansion_read_cr_quad_enable;
> +		params->quad_enable = spi_nor_sr2_bit1_quad_enable;
>  		break;
>  
>  	default:
> @@ -4738,7 +4709,7 @@ static void spi_nor_info_init_params(struct spi_nor *nor)
>  	u8 i, erase_mask;
>  
>  	/* Initialize legacy flash parameters and settings. */
> -	params->quad_enable = spansion_read_cr_quad_enable;
> +	params->quad_enable = spi_nor_sr2_bit1_quad_enable;
>  	params->set_4byte = spansion_set_4byte;
>  	params->setup = spi_nor_default_setup;
>  	/* Default to 16-bit Write Status (01h) Command */
> @@ -4955,7 +4926,7 @@ static int spi_nor_init(struct spi_nor *nor)
>  	int err;
>  
>  	if (nor->clear_sr_bp) {
> -		if (nor->params.quad_enable == spansion_read_cr_quad_enable)
> +		if (nor->params.quad_enable == spi_nor_sr2_bit1_quad_enable)
>  			nor->clear_sr_bp = spi_nor_spansion_clear_sr_bp;
>  
>  		err = nor->clear_sr_bp(nor);
> 

-- 
Regards
Vignesh

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

[PATCH v4 18/20] mtd: spi-nor: Rename macronix_quad_enable to spi_nor_sr1_bit6_quad_enable

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

Rename method to a generic name: spi_nor_sr1_bit6_quad_enable().

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 19 +++++++++----------
 include/linux/mtd/spi-nor.h   |  2 +-
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 8bc29cc073a0..85e5a56fb2d7 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -2078,16 +2078,15 @@ static int spi_nor_is_locked(struct mtd_info *mtd, loff_t ofs, uint64_t len)
 }
 
 /**
- * macronix_quad_enable() - set QE bit in Status Register.
+ * spi_nor_sr1_bit6_quad_enable() - Set the Quad Enable BIT(6) in the Status
+ * Register 1.
  * @nor:	pointer to a 'struct spi_nor'
  *
- * Set the Quad Enable (QE) bit in the Status Register.
- *
- * bit 6 of the Status Register is the QE bit for Macronix like QSPI memories.
+ * Bit 6 of the Status Register 1 is the QE bit for Macronix like QSPI memories.
  *
  * Return: 0 on success, -errno otherwise.
  */
-static int macronix_quad_enable(struct spi_nor *nor)
+static int spi_nor_sr1_bit6_quad_enable(struct spi_nor *nor)
 {
 	int ret;
 
@@ -2095,10 +2094,10 @@ static int macronix_quad_enable(struct spi_nor *nor)
 	if (ret)
 		return ret;
 
-	if (nor->bouncebuf[0] & SR_QUAD_EN_MX)
+	if (nor->bouncebuf[0] & SR1_QUAD_EN_BIT6)
 		return 0;
 
-	nor->bouncebuf[0] |= SR_QUAD_EN_MX;
+	nor->bouncebuf[0] |= SR1_QUAD_EN_BIT6;
 
 	return spi_nor_write_sr1_and_check(nor, nor->bouncebuf[0]);
 }
@@ -2349,7 +2348,7 @@ static void gd25q256_default_init(struct spi_nor *nor)
 	 * indicate the quad_enable method for this case, we need
 	 * to set it in the default_init fixup hook.
 	 */
-	nor->params.quad_enable = macronix_quad_enable;
+	nor->params.quad_enable = spi_nor_sr1_bit6_quad_enable;
 }
 
 static struct spi_nor_fixups gd25q256_fixups = {
@@ -3729,7 +3728,7 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
 
 	case BFPT_DWORD15_QER_SR1_BIT6:
 		nor->flags &= ~SNOR_F_HAS_16BIT_SR;
-		params->quad_enable = macronix_quad_enable;
+		params->quad_enable = spi_nor_sr1_bit6_quad_enable;
 		break;
 
 	case BFPT_DWORD15_QER_SR2_BIT7:
@@ -4627,7 +4626,7 @@ static int spi_nor_setup(struct spi_nor *nor,
 
 static void macronix_set_default_init(struct spi_nor *nor)
 {
-	nor->params.quad_enable = macronix_quad_enable;
+	nor->params.quad_enable = spi_nor_sr1_bit6_quad_enable;
 	nor->params.set_4byte = macronix_set_4byte;
 }
 
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index f626e0e52909..6d703df97f13 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -133,7 +133,7 @@
 #define SR_E_ERR		BIT(5)
 #define SR_P_ERR		BIT(6)
 
-#define SR_QUAD_EN_MX		BIT(6)	/* Macronix Quad I/O */
+#define SR1_QUAD_EN_BIT6	BIT(6)
 
 /* Enhanced Volatile Configuration Register bits */
 #define EVCR_QUAD_EN_MICRON	BIT(7)	/* Micron Quad I/O */
-- 
2.9.5


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

Re: [PATCH v4 18/20] mtd: spi-nor: Rename macronix_quad_enable to spi_nor_sr1_bit6_quad_enable

[Vignesh Raghavendra]

On 02/11/19 4:53 PM, Tudor.Ambarus@microchip.com wrote:
> From: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
> Rename method to a generic name: spi_nor_sr1_bit6_quad_enable().
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>

Reviewed-by: Vignesh Raghavendra <vigneshr@ti.com>

Regards
Vignesh

> ---
>  drivers/mtd/spi-nor/spi-nor.c | 19 +++++++++----------
>  include/linux/mtd/spi-nor.h   |  2 +-
>  2 files changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 8bc29cc073a0..85e5a56fb2d7 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -2078,16 +2078,15 @@ static int spi_nor_is_locked(struct mtd_info *mtd, loff_t ofs, uint64_t len)
>  }
>  
>  /**
> - * macronix_quad_enable() - set QE bit in Status Register.
> + * spi_nor_sr1_bit6_quad_enable() - Set the Quad Enable BIT(6) in the Status
> + * Register 1.
>   * @nor:	pointer to a 'struct spi_nor'
>   *
> - * Set the Quad Enable (QE) bit in the Status Register.
> - *
> - * bit 6 of the Status Register is the QE bit for Macronix like QSPI memories.
> + * Bit 6 of the Status Register 1 is the QE bit for Macronix like QSPI memories.
>   *
>   * Return: 0 on success, -errno otherwise.
>   */
> -static int macronix_quad_enable(struct spi_nor *nor)
> +static int spi_nor_sr1_bit6_quad_enable(struct spi_nor *nor)
>  {
>  	int ret;
>  
> @@ -2095,10 +2094,10 @@ static int macronix_quad_enable(struct spi_nor *nor)
>  	if (ret)
>  		return ret;
>  
> -	if (nor->bouncebuf[0] & SR_QUAD_EN_MX)
> +	if (nor->bouncebuf[0] & SR1_QUAD_EN_BIT6)
>  		return 0;
>  
> -	nor->bouncebuf[0] |= SR_QUAD_EN_MX;
> +	nor->bouncebuf[0] |= SR1_QUAD_EN_BIT6;
>  
>  	return spi_nor_write_sr1_and_check(nor, nor->bouncebuf[0]);
>  }
> @@ -2349,7 +2348,7 @@ static void gd25q256_default_init(struct spi_nor *nor)
>  	 * indicate the quad_enable method for this case, we need
>  	 * to set it in the default_init fixup hook.
>  	 */
> -	nor->params.quad_enable = macronix_quad_enable;
> +	nor->params.quad_enable = spi_nor_sr1_bit6_quad_enable;
>  }
>  
>  static struct spi_nor_fixups gd25q256_fixups = {
> @@ -3729,7 +3728,7 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
>  
>  	case BFPT_DWORD15_QER_SR1_BIT6:
>  		nor->flags &= ~SNOR_F_HAS_16BIT_SR;
> -		params->quad_enable = macronix_quad_enable;
> +		params->quad_enable = spi_nor_sr1_bit6_quad_enable;
>  		break;
>  
>  	case BFPT_DWORD15_QER_SR2_BIT7:
> @@ -4627,7 +4626,7 @@ static int spi_nor_setup(struct spi_nor *nor,
>  
>  static void macronix_set_default_init(struct spi_nor *nor)
>  {
> -	nor->params.quad_enable = macronix_quad_enable;
> +	nor->params.quad_enable = spi_nor_sr1_bit6_quad_enable;
>  	nor->params.set_4byte = macronix_set_4byte;
>  }
>  
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index f626e0e52909..6d703df97f13 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -133,7 +133,7 @@
>  #define SR_E_ERR		BIT(5)
>  #define SR_P_ERR		BIT(6)
>  
> -#define SR_QUAD_EN_MX		BIT(6)	/* Macronix Quad I/O */
> +#define SR1_QUAD_EN_BIT6	BIT(6)
>  
>  /* Enhanced Volatile Configuration Register bits */
>  #define EVCR_QUAD_EN_MICRON	BIT(7)	/* Micron Quad I/O */
> 

-- 
Regards
Vignesh

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

[PATCH v4 19/20] mtd: spi-nor: Prepend "spi_nor_" to "sr2_bit7_quad_enable"

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

All SPI NOR generic methods should be prepended by "spi_nor_".

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 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 85e5a56fb2d7..09b1af2cf0d4 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


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

[PATCH v4 20/20] mtd: spi-nor: Rework the disabling of block write protection

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

spi_nor_unlock() unlocks blocks of memory or the entire flash memory
array, if requested. clear_sr_bp() unlocks the entire flash memory
array at boot time. This calls for some unification, clear_sr_bp() is
just an optimization for the case when the unlock request covers the
entire flash size.

Get rid of clear_sr_bp() and introduce spi_nor_unlock_all(), which is
just a call to spi_nor_unlock() for the entire flash memory array.
This fixes a bug that was present in spi_nor_spansion_clear_sr_bp().
When the QE bit was zero, we used the Write Status (01h) command with
one data byte, which might cleared the Status Register 2. We now always
use the Write Status (01h) command with two data bytes when
SNOR_F_HAS_16BIT_SR is set, to avoid clearing the Status Register 2.

The SNOR_F_NO_READ_CR case is treated as well. 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().

Get rid of the MFR handling and implement specific manufacturer
default_init() fixup hooks.

Note that this changes a bit the logic for the SNOR_MFR_ATMEL,
SNOR_MFR_INTEL and SNOR_MFR_SST cases. Before this patch, the Atmel,
Intel and SST chips did not set the locking ops, but unlocked the entire
flash at boot time, while now they are setting the locking ops to
stm_locking_ops. This should work, since the the disable of the block
protection at the boot time used the same Status Register bits to unlock
the flash, as in the stm_locking_ops case.

Suggested-by: Boris Brezillon <boris.brezillon@collabora.com>
Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 140 +++++++++++++++---------------------------
 include/linux/mtd/spi-nor.h   |   3 -
 2 files changed, 50 insertions(+), 93 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 09b1af2cf0d4..05bf4003c9e7 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -2175,74 +2175,6 @@ static int spi_nor_sr2_bit7_quad_enable(struct spi_nor *nor)
 	return 0;
 }
 
-/**
- * spi_nor_clear_sr_bp() - clear the Status Register Block Protection bits.
- * @nor:        pointer to a 'struct spi_nor'
- *
- * Read-modify-write function that clears the Block Protection bits from the
- * Status Register without affecting other bits.
- *
- * Return: 0 on success, -errno otherwise.
- */
-static int spi_nor_clear_sr_bp(struct spi_nor *nor)
-{
-	int ret;
-	u8 mask = SR_BP2 | SR_BP1 | SR_BP0;
-
-	ret = spi_nor_read_sr(nor, nor->bouncebuf);
-	if (ret)
-		return ret;
-
-	nor->bouncebuf[0] &= ~mask;
-
-	return spi_nor_write_sr(nor, nor->bouncebuf, 1);
-}
-
-/**
- * spi_nor_spansion_clear_sr_bp() - clear the Status Register Block Protection
- * bits on spansion flashes.
- * @nor:        pointer to a 'struct spi_nor'
- *
- * Read-modify-write function that clears the Block Protection bits from the
- * Status Register without affecting other bits. The function is tightly
- * coupled with the spi_nor_sr2_bit1_quad_enable() function. Both assume that
- * the Write Register with 16 bits, together with the Read Configuration
- * Register (35h) instructions are supported.
- *
- * Return: 0 on success, -errno otherwise.
- */
-static int spi_nor_spansion_clear_sr_bp(struct spi_nor *nor)
-{
-	int ret;
-	u8 mask = SR_BP2 | SR_BP1 | SR_BP0;
-	u8 *sr_cr =  nor->bouncebuf;
-
-	/* Check current Quad Enable bit value. */
-	ret = spi_nor_read_cr(nor, &sr_cr[1]);
-	if (ret)
-		return ret;
-
-	/*
-	 * When the configuration register Quad Enable bit is one, only the
-	 * Write Status (01h) command with two data bytes may be used.
-	 */
-	if (sr_cr[1] & SR2_QUAD_EN_BIT1) {
-		ret = spi_nor_read_sr(nor, sr_cr);
-		if (ret)
-			return ret;
-
-		sr_cr[0] &= ~mask;
-
-		return spi_nor_write_sr(nor, sr_cr, 2);
-	}
-
-	/*
-	 * If the Quad Enable bit is zero, use the Write Status (01h) command
-	 * with one data byte.
-	 */
-	return spi_nor_clear_sr_bp(nor);
-}
-
 /* Used when the "_ext_id" is two bytes at most */
 #define INFO(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags)	\
 		.id = {							\
@@ -4624,12 +4556,27 @@ static int spi_nor_setup(struct spi_nor *nor,
 	return nor->params.setup(nor, hwcaps);
 }
 
+static void atmel_set_default_init(struct spi_nor *nor)
+{
+	nor->flags |= SNOR_F_HAS_LOCK;
+}
+
+static void intel_set_default_init(struct spi_nor *nor)
+{
+	nor->flags |= SNOR_F_HAS_LOCK;
+}
+
 static void macronix_set_default_init(struct spi_nor *nor)
 {
 	nor->params.quad_enable = spi_nor_sr1_bit6_quad_enable;
 	nor->params.set_4byte = macronix_set_4byte;
 }
 
+static void sst_set_default_init(struct spi_nor *nor)
+{
+	nor->flags |= SNOR_F_HAS_LOCK;
+}
+
 static void st_micron_set_default_init(struct spi_nor *nor)
 {
 	nor->flags |= SNOR_F_HAS_LOCK;
@@ -4651,6 +4598,14 @@ static void spi_nor_manufacturer_init_params(struct spi_nor *nor)
 {
 	/* Init flash parameters based on MFR */
 	switch (JEDEC_MFR(nor->info)) {
+	case SNOR_MFR_ATMEL:
+		atmel_set_default_init(nor);
+		break;
+
+	case SNOR_MFR_INTEL:
+		intel_set_default_init(nor);
+		break;
+
 	case SNOR_MFR_MACRONIX:
 		macronix_set_default_init(nor);
 		break;
@@ -4660,6 +4615,10 @@ static void spi_nor_manufacturer_init_params(struct spi_nor *nor)
 		st_micron_set_default_init(nor);
 		break;
 
+	case SNOR_MFR_SST:
+		sst_set_default_init(nor);
+		break;
+
 	case SNOR_MFR_WINBOND:
 		winbond_set_default_init(nor);
 		break;
@@ -4920,21 +4879,26 @@ static int spi_nor_quad_enable(struct spi_nor *nor)
 	return nor->params.quad_enable(nor);
 }
 
-static int spi_nor_init(struct spi_nor *nor)
+/**
+ * spi_nor_unlock_all() - Unlocks the entire flash memory array.
+ * @nor:	pointer to a 'struct spi_nor'.
+ *
+ * Some SPI NOR flashes are write protected by default after a power-on reset
+ * cycle, in order to avoid inadvertent writes during power-up. Backward
+ * compatibility imposes to unlock the entire flash memory array at power-up
+ * by default.
+ */
+static int spi_nor_unlock_all(struct spi_nor *nor)
 {
-	int err;
+	if (nor->flags & SNOR_F_HAS_LOCK)
+		return spi_nor_unlock(&nor->mtd, 0, nor->params.size);
 
-	if (nor->clear_sr_bp) {
-		if (nor->params.quad_enable == spi_nor_sr2_bit1_quad_enable)
-			nor->clear_sr_bp = spi_nor_spansion_clear_sr_bp;
+	return 0;
+}
 
-		err = nor->clear_sr_bp(nor);
-		if (err) {
-			dev_dbg(nor->dev,
-				"fail to clear block protection bits\n");
-			return err;
-		}
-	}
+static int spi_nor_init(struct spi_nor *nor)
+{
+	int err;
 
 	err = spi_nor_quad_enable(nor);
 	if (err) {
@@ -4942,6 +4906,12 @@ static int spi_nor_init(struct spi_nor *nor)
 		return err;
 	}
 
+	err = spi_nor_unlock_all(nor);
+	if (err) {
+		dev_dbg(nor->dev, "Failed to unlock the entire flash memory array\n");
+		return err;
+	}
+
 	if (nor->addr_width == 4 && !(nor->flags & SNOR_F_4B_OPCODES)) {
 		/*
 		 * If the RESET# pin isn't hooked up properly, or the system
@@ -5124,16 +5094,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
 	if (info->flags & SPI_NOR_HAS_LOCK)
 		nor->flags |= SNOR_F_HAS_LOCK;
 
-	/*
-	 * Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up
-	 * with the software protection bits set.
-	 */
-	if (JEDEC_MFR(nor->info) == SNOR_MFR_ATMEL ||
-	    JEDEC_MFR(nor->info) == SNOR_MFR_INTEL ||
-	    JEDEC_MFR(nor->info) == SNOR_MFR_SST ||
-	    nor->info->flags & SPI_NOR_HAS_LOCK)
-		nor->clear_sr_bp = spi_nor_clear_sr_bp;
-
 	/* Init flash parameters based on flash_info struct and SFDP */
 	spi_nor_init_params(nor);
 
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 6d703df97f13..9eae35c60bce 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -579,8 +579,6 @@ struct flash_info;
  * @write_proto:	the SPI protocol for write operations
  * @reg_proto		the SPI protocol for read_reg/write_reg/erase operations
  * @controller_ops:	SPI NOR controller driver specific operations.
- * @clear_sr_bp:	[FLASH-SPECIFIC] clears the Block Protection Bits from
- *			the SPI NOR Status Register.
  * @params:		[FLASH-SPECIFIC] SPI-NOR flash parameters and settings.
  *                      The structure includes legacy flash parameters and
  *                      settings that can be overwritten by the spi_nor_fixups
@@ -609,7 +607,6 @@ struct spi_nor {
 
 	const struct spi_nor_controller_ops *controller_ops;
 
-	int (*clear_sr_bp)(struct spi_nor *nor);
 	struct spi_nor_flash_parameter params;
 
 	void *priv;
-- 
2.9.5


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

Re: [PATCH v4 00/20] mtd: spi-nor: Quad Enable and (un)lock methods

[Tudor.Ambarus]

On 11/02/2019 01:23 PM, Tudor Ambarus - M18064 wrote:
> From: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
> Tested on s25fl116k and w25q128jv-q.
> 
> 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.
> 
> v4:
> - Use dev_dbg insted of dev_err for low level info
> - replace "&nor->bouncebuf[0]" with "nor->bouncebuf" and "&sr_cr[0]" with
>   "sr_cr". Update across all patches.
> 
> 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.
> 
> 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.
> 
> Tudor Ambarus (20):
>   mtd: spi-nor: Use dev_dbg insted of dev_err for low level info
>   mtd: spi-nor: Print debug info inside Reg Ops methods
>   mtd: spi-nor: Check for errors after each Register Operation
>   mtd: spi-nor: Rename label as it is no longer generic
>   mtd: spi-nor: Void return type for spi_nor_clear_sr/fsr()
>   mtd: spi-nor: Move the WE and wait calls inside Write SR methods
>   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 debug 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 | 952 +++++++++++++++++++++++++-----------------
>  include/linux/mtd/spi-nor.h   |  12 +-
>  2 files changed, 583 insertions(+), 381 deletions(-)
> 

Updated 1/20 to use dev_err() when the SR/FSR report program or erase fails, or
attempts of modifying a protected sector. Patches 1-12 applied to spi-nor/next.
Thanks.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/