All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] mtd: nand: Remove unused features
@ 2017-05-15 22:17 Boris Brezillon
  2017-05-15 22:17 ` [PATCH 1/3] mtd: nand: Drop unused cached programming support Boris Brezillon
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Boris Brezillon @ 2017-05-15 22:17 UTC (permalink / raw)
  To: Boris Brezillon, Richard Weinberger, linux-mtd
  Cc: David Woodhouse, Brian Norris, Marek Vasut, Cyrille Pitchen

Hello,

This 3 patches are removing features that have either been introduced
and never used or that are no longer used.

Regards,

Boris

Boris Brezillon (3):
  mtd: nand: Drop unused cached programming support
  mtd: nand: Remove support for block locking/unlocking
  mtd: nand: Drop the ->errstat() hook

 drivers/mtd/nand/nand_base.c | 216 ++-----------------------------------------
 include/linux/mtd/nand.h     |  15 ---
 2 files changed, 7 insertions(+), 224 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/3] mtd: nand: Drop unused cached programming support
  2017-05-15 22:17 [PATCH 0/3] mtd: nand: Remove unused features Boris Brezillon
@ 2017-05-15 22:17 ` Boris Brezillon
  2017-05-15 22:17 ` [PATCH 2/3] mtd: nand: Remove support for block locking/unlocking Boris Brezillon
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Boris Brezillon @ 2017-05-15 22:17 UTC (permalink / raw)
  To: Boris Brezillon, Richard Weinberger, linux-mtd
  Cc: David Woodhouse, Brian Norris, Marek Vasut, Cyrille Pitchen

Cached programming is always skipped, so drop the associated code until
we decide to really support it.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/mtd/nand/nand_base.c | 38 ++++++++++++--------------------------
 1 file changed, 12 insertions(+), 26 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 54a1dfa327ff..6b98c032ef50 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2720,7 +2720,7 @@ static int nand_write_page_syndrome(struct mtd_info *mtd,
  */
 static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 		uint32_t offset, int data_len, const uint8_t *buf,
-		int oob_required, int page, int cached, int raw)
+		int oob_required, int page, int raw)
 {
 	int status, subpage;
 
@@ -2746,31 +2746,19 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 	if (status < 0)
 		return status;
 
+	if (nand_standard_page_accessors(&chip->ecc))
+		chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
+	status = chip->waitfunc(mtd, chip);
 	/*
-	 * Cached progamming disabled for now. Not sure if it's worth the
-	 * trouble. The speed gain is not very impressive. (2.3->2.6Mib/s).
+	 * See if operation failed and additional status checks are
+	 * available.
 	 */
-	cached = 0;
-
-	if (!cached || !NAND_HAS_CACHEPROG(chip)) {
-
-		if (nand_standard_page_accessors(&chip->ecc))
-			chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
-		status = chip->waitfunc(mtd, chip);
-		/*
-		 * See if operation failed and additional status checks are
-		 * available.
-		 */
-		if ((status & NAND_STATUS_FAIL) && (chip->errstat))
-			status = chip->errstat(mtd, chip, FL_WRITING, status,
-					       page);
+	if ((status & NAND_STATUS_FAIL) && (chip->errstat))
+		status = chip->errstat(mtd, chip, FL_WRITING, status,
+				       page);
 
-		if (status & NAND_STATUS_FAIL)
-			return -EIO;
-	} else {
-		chip->cmdfunc(mtd, NAND_CMD_CACHEDPROG, -1, -1);
-		status = chip->waitfunc(mtd, chip);
-	}
+	if (status & NAND_STATUS_FAIL)
+		return -EIO;
 
 	return 0;
 }
@@ -2877,7 +2865,6 @@ static int nand_do_write_ops(struct mtd_info *mtd, loff_t to,
 
 	while (1) {
 		int bytes = mtd->writesize;
-		int cached = writelen > bytes && page != blockmask;
 		uint8_t *wbuf = buf;
 		int use_bufpoi;
 		int part_pagewr = (column || writelen < mtd->writesize);
@@ -2895,7 +2882,6 @@ static int nand_do_write_ops(struct mtd_info *mtd, loff_t to,
 		if (use_bufpoi) {
 			pr_debug("%s: using write bounce buffer for buf@%p\n",
 					 __func__, buf);
-			cached = 0;
 			if (part_pagewr)
 				bytes = min_t(int, bytes - column, writelen);
 			chip->pagebuf = -1;
@@ -2914,7 +2900,7 @@ static int nand_do_write_ops(struct mtd_info *mtd, loff_t to,
 		}
 
 		ret = nand_write_page(mtd, chip, column, bytes, wbuf,
-				      oob_required, page, cached,
+				      oob_required, page,
 				      (ops->mode == MTD_OPS_RAW));
 		if (ret)
 			break;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/3] mtd: nand: Remove support for block locking/unlocking
  2017-05-15 22:17 [PATCH 0/3] mtd: nand: Remove unused features Boris Brezillon
  2017-05-15 22:17 ` [PATCH 1/3] mtd: nand: Drop unused cached programming support Boris Brezillon
@ 2017-05-15 22:17 ` Boris Brezillon
  2017-08-04  8:32   ` Boris Brezillon
  2017-05-15 22:17 ` [PATCH 3/3] mtd: nand: Drop the ->errstat() hook Boris Brezillon
  2017-05-29 18:52 ` [PATCH 0/3] mtd: nand: Remove unused features Boris Brezillon
  3 siblings, 1 reply; 8+ messages in thread
From: Boris Brezillon @ 2017-05-15 22:17 UTC (permalink / raw)
  To: Boris Brezillon, Richard Weinberger, linux-mtd
  Cc: David Woodhouse, Brian Norris, Marek Vasut, Cyrille Pitchen

Commit 7d70f334ad2b ("mtd: nand: add lock/unlock routines") introduced
support for the Micron LOCK/UNLOCK commands but no one ever used the
nand_lock/unlock() functions.

Remove support for these vendor-specific operations from the core. If
one ever wants to add them back they should be put in nand_micron.c and
mtd->_lock/_unlock should be directly assigned from there instead of
exporting the functions.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/mtd/nand/nand_base.c | 172 -------------------------------------------
 include/linux/mtd/nand.h     |  10 ---
 2 files changed, 182 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 6b98c032ef50..6eba5ba51c90 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -1215,178 +1215,6 @@ int nand_reset(struct nand_chip *chip, int chipnr)
 }
 
 /**
- * __nand_unlock - [REPLACEABLE] unlocks specified locked blocks
- * @mtd: mtd info
- * @ofs: offset to start unlock from
- * @len: length to unlock
- * @invert: when = 0, unlock the range of blocks within the lower and
- *                    upper boundary address
- *          when = 1, unlock the range of blocks outside the boundaries
- *                    of the lower and upper boundary address
- *
- * Returs unlock status.
- */
-static int __nand_unlock(struct mtd_info *mtd, loff_t ofs,
-					uint64_t len, int invert)
-{
-	int ret = 0;
-	int status, page;
-	struct nand_chip *chip = mtd_to_nand(mtd);
-
-	/* Submit address of first page to unlock */
-	page = ofs >> chip->page_shift;
-	chip->cmdfunc(mtd, NAND_CMD_UNLOCK1, -1, page & chip->pagemask);
-
-	/* Submit address of last page to unlock */
-	page = (ofs + len) >> chip->page_shift;
-	chip->cmdfunc(mtd, NAND_CMD_UNLOCK2, -1,
-				(page | invert) & chip->pagemask);
-
-	/* Call wait ready function */
-	status = chip->waitfunc(mtd, chip);
-	/* See if device thinks it succeeded */
-	if (status & NAND_STATUS_FAIL) {
-		pr_debug("%s: error status = 0x%08x\n",
-					__func__, status);
-		ret = -EIO;
-	}
-
-	return ret;
-}
-
-/**
- * nand_unlock - [REPLACEABLE] unlocks specified locked blocks
- * @mtd: mtd info
- * @ofs: offset to start unlock from
- * @len: length to unlock
- *
- * Returns unlock status.
- */
-int nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
-{
-	int ret = 0;
-	int chipnr;
-	struct nand_chip *chip = mtd_to_nand(mtd);
-
-	pr_debug("%s: start = 0x%012llx, len = %llu\n",
-			__func__, (unsigned long long)ofs, len);
-
-	if (check_offs_len(mtd, ofs, len))
-		return -EINVAL;
-
-	/* Align to last block address if size addresses end of the device */
-	if (ofs + len == mtd->size)
-		len -= mtd->erasesize;
-
-	nand_get_device(mtd, FL_UNLOCKING);
-
-	/* Shift to get chip number */
-	chipnr = ofs >> chip->chip_shift;
-
-	/*
-	 * Reset the chip.
-	 * If we want to check the WP through READ STATUS and check the bit 7
-	 * we must reset the chip
-	 * some operation can also clear the bit 7 of status register
-	 * eg. erase/program a locked block
-	 */
-	nand_reset(chip, chipnr);
-
-	chip->select_chip(mtd, chipnr);
-
-	/* Check, if it is write protected */
-	if (nand_check_wp(mtd)) {
-		pr_debug("%s: device is write protected!\n",
-					__func__);
-		ret = -EIO;
-		goto out;
-	}
-
-	ret = __nand_unlock(mtd, ofs, len, 0);
-
-out:
-	chip->select_chip(mtd, -1);
-	nand_release_device(mtd);
-
-	return ret;
-}
-EXPORT_SYMBOL(nand_unlock);
-
-/**
- * nand_lock - [REPLACEABLE] locks all blocks present in the device
- * @mtd: mtd info
- * @ofs: offset to start unlock from
- * @len: length to unlock
- *
- * This feature is not supported in many NAND parts. 'Micron' NAND parts do
- * have this feature, but it allows only to lock all blocks, not for specified
- * range for block. Implementing 'lock' feature by making use of 'unlock', for
- * now.
- *
- * Returns lock status.
- */
-int nand_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
-{
-	int ret = 0;
-	int chipnr, status, page;
-	struct nand_chip *chip = mtd_to_nand(mtd);
-
-	pr_debug("%s: start = 0x%012llx, len = %llu\n",
-			__func__, (unsigned long long)ofs, len);
-
-	if (check_offs_len(mtd, ofs, len))
-		return -EINVAL;
-
-	nand_get_device(mtd, FL_LOCKING);
-
-	/* Shift to get chip number */
-	chipnr = ofs >> chip->chip_shift;
-
-	/*
-	 * Reset the chip.
-	 * If we want to check the WP through READ STATUS and check the bit 7
-	 * we must reset the chip
-	 * some operation can also clear the bit 7 of status register
-	 * eg. erase/program a locked block
-	 */
-	nand_reset(chip, chipnr);
-
-	chip->select_chip(mtd, chipnr);
-
-	/* Check, if it is write protected */
-	if (nand_check_wp(mtd)) {
-		pr_debug("%s: device is write protected!\n",
-					__func__);
-		status = MTD_ERASE_FAILED;
-		ret = -EIO;
-		goto out;
-	}
-
-	/* Submit address of first page to lock */
-	page = ofs >> chip->page_shift;
-	chip->cmdfunc(mtd, NAND_CMD_LOCK, -1, page & chip->pagemask);
-
-	/* Call wait ready function */
-	status = chip->waitfunc(mtd, chip);
-	/* See if device thinks it succeeded */
-	if (status & NAND_STATUS_FAIL) {
-		pr_debug("%s: error status = 0x%08x\n",
-					__func__, status);
-		ret = -EIO;
-		goto out;
-	}
-
-	ret = __nand_unlock(mtd, ofs, len, 0x1);
-
-out:
-	chip->select_chip(mtd, -1);
-	nand_release_device(mtd);
-
-	return ret;
-}
-EXPORT_SYMBOL(nand_lock);
-
-/**
  * nand_check_erased_buf - check if a buffer contains (almost) only 0xff data
  * @buf: buffer to test
  * @len: buffer length
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index f01991649118..9ca3ad20faea 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -44,12 +44,6 @@ void nand_release(struct mtd_info *mtd);
 /* Internal helper for board drivers which need to override command function */
 void nand_wait_ready(struct mtd_info *mtd);
 
-/* locks all blocks present in the device */
-int nand_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len);
-
-/* unlocks specified locked blocks */
-int nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len);
-
 /* The maximum number of NAND chips in an array */
 #define NAND_MAX_CHIPS		8
 
@@ -89,10 +83,6 @@ int nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len);
 #define NAND_CMD_SET_FEATURES	0xef
 #define NAND_CMD_RESET		0xff
 
-#define NAND_CMD_LOCK		0x2a
-#define NAND_CMD_UNLOCK1	0x23
-#define NAND_CMD_UNLOCK2	0x24
-
 /* Extended commands for large page devices */
 #define NAND_CMD_READSTART	0x30
 #define NAND_CMD_RNDOUTSTART	0xE0
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/3] mtd: nand: Drop the ->errstat() hook
  2017-05-15 22:17 [PATCH 0/3] mtd: nand: Remove unused features Boris Brezillon
  2017-05-15 22:17 ` [PATCH 1/3] mtd: nand: Drop unused cached programming support Boris Brezillon
  2017-05-15 22:17 ` [PATCH 2/3] mtd: nand: Remove support for block locking/unlocking Boris Brezillon
@ 2017-05-15 22:17 ` Boris Brezillon
  2017-05-17  8:49   ` Boris Brezillon
  2017-05-29 18:52 ` [PATCH 0/3] mtd: nand: Remove unused features Boris Brezillon
  3 siblings, 1 reply; 8+ messages in thread
From: Boris Brezillon @ 2017-05-15 22:17 UTC (permalink / raw)
  To: Boris Brezillon, Richard Weinberger, linux-mtd
  Cc: David Woodhouse, Brian Norris, Marek Vasut, Cyrille Pitchen

TODO: add a real commit message

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/mtd/nand/nand_base.c | 16 ----------------
 include/linux/mtd/nand.h     |  5 -----
 2 files changed, 21 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 6eba5ba51c90..8dafd2a54e11 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2577,14 +2577,6 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 	if (nand_standard_page_accessors(&chip->ecc))
 		chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
 	status = chip->waitfunc(mtd, chip);
-	/*
-	 * See if operation failed and additional status checks are
-	 * available.
-	 */
-	if ((status & NAND_STATUS_FAIL) && (chip->errstat))
-		status = chip->errstat(mtd, chip, FL_WRITING, status,
-				       page);
-
 	if (status & NAND_STATUS_FAIL)
 		return -EIO;
 
@@ -3044,14 +3036,6 @@ int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr,
 
 		status = chip->erase(mtd, page & chip->pagemask);
 
-		/*
-		 * See if operation failed and additional status checks are
-		 * available
-		 */
-		if ((status & NAND_STATUS_FAIL) && (chip->errstat))
-			status = chip->errstat(mtd, chip, FL_ERASING,
-					       status, page);
-
 		/* See if block erase succeeded */
 		if (status & NAND_STATUS_FAIL) {
 			pr_debug("%s: failed erase, page 0x%08x\n",
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 9ca3ad20faea..2dcdd07e7810 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -819,9 +819,6 @@ struct nand_manufacturer_ops {
  *			structure which is shared among multiple independent
  *			devices.
  * @priv:		[OPTIONAL] pointer to private chip data
- * @errstat:		[OPTIONAL] hardware specific function to perform
- *			additional error status checks (determine if errors are
- *			correctable).
  * @manufacturer:	[INTERN] Contains manufacturer information
  */
 
@@ -845,8 +842,6 @@ struct nand_chip {
 	int(*waitfunc)(struct mtd_info *mtd, struct nand_chip *this);
 	int (*erase)(struct mtd_info *mtd, int page);
 	int (*scan_bbt)(struct mtd_info *mtd);
-	int (*errstat)(struct mtd_info *mtd, struct nand_chip *this, int state,
-			int status, int page);
 	int (*onfi_set_features)(struct mtd_info *mtd, struct nand_chip *chip,
 			int feature_addr, uint8_t *subfeature_para);
 	int (*onfi_get_features)(struct mtd_info *mtd, struct nand_chip *chip,
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] mtd: nand: Drop the ->errstat() hook
  2017-05-15 22:17 ` [PATCH 3/3] mtd: nand: Drop the ->errstat() hook Boris Brezillon
@ 2017-05-17  8:49   ` Boris Brezillon
  0 siblings, 0 replies; 8+ messages in thread
From: Boris Brezillon @ 2017-05-17  8:49 UTC (permalink / raw)
  To: Boris Brezillon, Richard Weinberger, linux-mtd
  Cc: David Woodhouse, Brian Norris, Marek Vasut, Cyrille Pitchen

On Tue, 16 May 2017 00:17:43 +0200
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:

> TODO: add a real commit message

Duh. Forgot to add a real commit message here :-).

> 
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
>  drivers/mtd/nand/nand_base.c | 16 ----------------
>  include/linux/mtd/nand.h     |  5 -----
>  2 files changed, 21 deletions(-)
> 
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 6eba5ba51c90..8dafd2a54e11 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -2577,14 +2577,6 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
>  	if (nand_standard_page_accessors(&chip->ecc))
>  		chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
>  	status = chip->waitfunc(mtd, chip);
> -	/*
> -	 * See if operation failed and additional status checks are
> -	 * available.
> -	 */
> -	if ((status & NAND_STATUS_FAIL) && (chip->errstat))
> -		status = chip->errstat(mtd, chip, FL_WRITING, status,
> -				       page);
> -
>  	if (status & NAND_STATUS_FAIL)
>  		return -EIO;
>  
> @@ -3044,14 +3036,6 @@ int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr,
>  
>  		status = chip->erase(mtd, page & chip->pagemask);
>  
> -		/*
> -		 * See if operation failed and additional status checks are
> -		 * available
> -		 */
> -		if ((status & NAND_STATUS_FAIL) && (chip->errstat))
> -			status = chip->errstat(mtd, chip, FL_ERASING,
> -					       status, page);
> -
>  		/* See if block erase succeeded */
>  		if (status & NAND_STATUS_FAIL) {
>  			pr_debug("%s: failed erase, page 0x%08x\n",
> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> index 9ca3ad20faea..2dcdd07e7810 100644
> --- a/include/linux/mtd/nand.h
> +++ b/include/linux/mtd/nand.h
> @@ -819,9 +819,6 @@ struct nand_manufacturer_ops {
>   *			structure which is shared among multiple independent
>   *			devices.
>   * @priv:		[OPTIONAL] pointer to private chip data
> - * @errstat:		[OPTIONAL] hardware specific function to perform
> - *			additional error status checks (determine if errors are
> - *			correctable).
>   * @manufacturer:	[INTERN] Contains manufacturer information
>   */
>  
> @@ -845,8 +842,6 @@ struct nand_chip {
>  	int(*waitfunc)(struct mtd_info *mtd, struct nand_chip *this);
>  	int (*erase)(struct mtd_info *mtd, int page);
>  	int (*scan_bbt)(struct mtd_info *mtd);
> -	int (*errstat)(struct mtd_info *mtd, struct nand_chip *this, int state,
> -			int status, int page);
>  	int (*onfi_set_features)(struct mtd_info *mtd, struct nand_chip *chip,
>  			int feature_addr, uint8_t *subfeature_para);
>  	int (*onfi_get_features)(struct mtd_info *mtd, struct nand_chip *chip,

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/3] mtd: nand: Remove unused features
  2017-05-15 22:17 [PATCH 0/3] mtd: nand: Remove unused features Boris Brezillon
                   ` (2 preceding siblings ...)
  2017-05-15 22:17 ` [PATCH 3/3] mtd: nand: Drop the ->errstat() hook Boris Brezillon
@ 2017-05-29 18:52 ` Boris Brezillon
  3 siblings, 0 replies; 8+ messages in thread
From: Boris Brezillon @ 2017-05-29 18:52 UTC (permalink / raw)
  To: Boris Brezillon, Richard Weinberger, linux-mtd
  Cc: Marek Vasut, Cyrille Pitchen, Brian Norris, David Woodhouse

On Tue, 16 May 2017 00:17:40 +0200
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:

> Hello,
> 
> This 3 patches are removing features that have either been introduced
> and never used or that are no longer used.

Applied to nand/next after adding a real commit message in patch 1.

> 
> Regards,
> 
> Boris
> 
> Boris Brezillon (3):
>   mtd: nand: Drop unused cached programming support
>   mtd: nand: Remove support for block locking/unlocking
>   mtd: nand: Drop the ->errstat() hook
> 
>  drivers/mtd/nand/nand_base.c | 216 ++-----------------------------------------
>  include/linux/mtd/nand.h     |  15 ---
>  2 files changed, 7 insertions(+), 224 deletions(-)
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/3] mtd: nand: Remove support for block locking/unlocking
  2017-05-15 22:17 ` [PATCH 2/3] mtd: nand: Remove support for block locking/unlocking Boris Brezillon
@ 2017-08-04  8:32   ` Boris Brezillon
       [not found]     ` <CAHCN7xKtR=kjLLFYkQkhjKGsuU+ttRE9FA_p7aWGDWVWxf2S8Q@mail.gmail.com>
  0 siblings, 1 reply; 8+ messages in thread
From: Boris Brezillon @ 2017-08-04  8:32 UTC (permalink / raw)
  To: Boris Brezillon, Richard Weinberger, linux-mtd
  Cc: Marek Vasut, Cyrille Pitchen, Brian Norris, David Woodhouse

On Tue, 16 May 2017 00:17:42 +0200
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:

> Commit 7d70f334ad2b ("mtd: nand: add lock/unlock routines") introduced
> support for the Micron LOCK/UNLOCK commands but no one ever used the
> nand_lock/unlock() functions.
> 
> Remove support for these vendor-specific operations from the core. If
> one ever wants to add them back they should be put in nand_micron.c and
> mtd->_lock/_unlock should be directly assigned from there instead of
> exporting the functions.

Applied.

> 
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
>  drivers/mtd/nand/nand_base.c | 172 -------------------------------------------
>  include/linux/mtd/nand.h     |  10 ---
>  2 files changed, 182 deletions(-)
> 
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 6b98c032ef50..6eba5ba51c90 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -1215,178 +1215,6 @@ int nand_reset(struct nand_chip *chip, int chipnr)
>  }
>  
>  /**
> - * __nand_unlock - [REPLACEABLE] unlocks specified locked blocks
> - * @mtd: mtd info
> - * @ofs: offset to start unlock from
> - * @len: length to unlock
> - * @invert: when = 0, unlock the range of blocks within the lower and
> - *                    upper boundary address
> - *          when = 1, unlock the range of blocks outside the boundaries
> - *                    of the lower and upper boundary address
> - *
> - * Returs unlock status.
> - */
> -static int __nand_unlock(struct mtd_info *mtd, loff_t ofs,
> -					uint64_t len, int invert)
> -{
> -	int ret = 0;
> -	int status, page;
> -	struct nand_chip *chip = mtd_to_nand(mtd);
> -
> -	/* Submit address of first page to unlock */
> -	page = ofs >> chip->page_shift;
> -	chip->cmdfunc(mtd, NAND_CMD_UNLOCK1, -1, page & chip->pagemask);
> -
> -	/* Submit address of last page to unlock */
> -	page = (ofs + len) >> chip->page_shift;
> -	chip->cmdfunc(mtd, NAND_CMD_UNLOCK2, -1,
> -				(page | invert) & chip->pagemask);
> -
> -	/* Call wait ready function */
> -	status = chip->waitfunc(mtd, chip);
> -	/* See if device thinks it succeeded */
> -	if (status & NAND_STATUS_FAIL) {
> -		pr_debug("%s: error status = 0x%08x\n",
> -					__func__, status);
> -		ret = -EIO;
> -	}
> -
> -	return ret;
> -}
> -
> -/**
> - * nand_unlock - [REPLACEABLE] unlocks specified locked blocks
> - * @mtd: mtd info
> - * @ofs: offset to start unlock from
> - * @len: length to unlock
> - *
> - * Returns unlock status.
> - */
> -int nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
> -{
> -	int ret = 0;
> -	int chipnr;
> -	struct nand_chip *chip = mtd_to_nand(mtd);
> -
> -	pr_debug("%s: start = 0x%012llx, len = %llu\n",
> -			__func__, (unsigned long long)ofs, len);
> -
> -	if (check_offs_len(mtd, ofs, len))
> -		return -EINVAL;
> -
> -	/* Align to last block address if size addresses end of the device */
> -	if (ofs + len == mtd->size)
> -		len -= mtd->erasesize;
> -
> -	nand_get_device(mtd, FL_UNLOCKING);
> -
> -	/* Shift to get chip number */
> -	chipnr = ofs >> chip->chip_shift;
> -
> -	/*
> -	 * Reset the chip.
> -	 * If we want to check the WP through READ STATUS and check the bit 7
> -	 * we must reset the chip
> -	 * some operation can also clear the bit 7 of status register
> -	 * eg. erase/program a locked block
> -	 */
> -	nand_reset(chip, chipnr);
> -
> -	chip->select_chip(mtd, chipnr);
> -
> -	/* Check, if it is write protected */
> -	if (nand_check_wp(mtd)) {
> -		pr_debug("%s: device is write protected!\n",
> -					__func__);
> -		ret = -EIO;
> -		goto out;
> -	}
> -
> -	ret = __nand_unlock(mtd, ofs, len, 0);
> -
> -out:
> -	chip->select_chip(mtd, -1);
> -	nand_release_device(mtd);
> -
> -	return ret;
> -}
> -EXPORT_SYMBOL(nand_unlock);
> -
> -/**
> - * nand_lock - [REPLACEABLE] locks all blocks present in the device
> - * @mtd: mtd info
> - * @ofs: offset to start unlock from
> - * @len: length to unlock
> - *
> - * This feature is not supported in many NAND parts. 'Micron' NAND parts do
> - * have this feature, but it allows only to lock all blocks, not for specified
> - * range for block. Implementing 'lock' feature by making use of 'unlock', for
> - * now.
> - *
> - * Returns lock status.
> - */
> -int nand_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
> -{
> -	int ret = 0;
> -	int chipnr, status, page;
> -	struct nand_chip *chip = mtd_to_nand(mtd);
> -
> -	pr_debug("%s: start = 0x%012llx, len = %llu\n",
> -			__func__, (unsigned long long)ofs, len);
> -
> -	if (check_offs_len(mtd, ofs, len))
> -		return -EINVAL;
> -
> -	nand_get_device(mtd, FL_LOCKING);
> -
> -	/* Shift to get chip number */
> -	chipnr = ofs >> chip->chip_shift;
> -
> -	/*
> -	 * Reset the chip.
> -	 * If we want to check the WP through READ STATUS and check the bit 7
> -	 * we must reset the chip
> -	 * some operation can also clear the bit 7 of status register
> -	 * eg. erase/program a locked block
> -	 */
> -	nand_reset(chip, chipnr);
> -
> -	chip->select_chip(mtd, chipnr);
> -
> -	/* Check, if it is write protected */
> -	if (nand_check_wp(mtd)) {
> -		pr_debug("%s: device is write protected!\n",
> -					__func__);
> -		status = MTD_ERASE_FAILED;
> -		ret = -EIO;
> -		goto out;
> -	}
> -
> -	/* Submit address of first page to lock */
> -	page = ofs >> chip->page_shift;
> -	chip->cmdfunc(mtd, NAND_CMD_LOCK, -1, page & chip->pagemask);
> -
> -	/* Call wait ready function */
> -	status = chip->waitfunc(mtd, chip);
> -	/* See if device thinks it succeeded */
> -	if (status & NAND_STATUS_FAIL) {
> -		pr_debug("%s: error status = 0x%08x\n",
> -					__func__, status);
> -		ret = -EIO;
> -		goto out;
> -	}
> -
> -	ret = __nand_unlock(mtd, ofs, len, 0x1);
> -
> -out:
> -	chip->select_chip(mtd, -1);
> -	nand_release_device(mtd);
> -
> -	return ret;
> -}
> -EXPORT_SYMBOL(nand_lock);
> -
> -/**
>   * nand_check_erased_buf - check if a buffer contains (almost) only 0xff data
>   * @buf: buffer to test
>   * @len: buffer length
> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> index f01991649118..9ca3ad20faea 100644
> --- a/include/linux/mtd/nand.h
> +++ b/include/linux/mtd/nand.h
> @@ -44,12 +44,6 @@ void nand_release(struct mtd_info *mtd);
>  /* Internal helper for board drivers which need to override command function */
>  void nand_wait_ready(struct mtd_info *mtd);
>  
> -/* locks all blocks present in the device */
> -int nand_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len);
> -
> -/* unlocks specified locked blocks */
> -int nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len);
> -
>  /* The maximum number of NAND chips in an array */
>  #define NAND_MAX_CHIPS		8
>  
> @@ -89,10 +83,6 @@ int nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len);
>  #define NAND_CMD_SET_FEATURES	0xef
>  #define NAND_CMD_RESET		0xff
>  
> -#define NAND_CMD_LOCK		0x2a
> -#define NAND_CMD_UNLOCK1	0x23
> -#define NAND_CMD_UNLOCK2	0x24
> -
>  /* Extended commands for large page devices */
>  #define NAND_CMD_READSTART	0x30
>  #define NAND_CMD_RNDOUTSTART	0xE0

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/3] mtd: nand: Remove support for block locking/unlocking
       [not found]     ` <CAHCN7xKtR=kjLLFYkQkhjKGsuU+ttRE9FA_p7aWGDWVWxf2S8Q@mail.gmail.com>
@ 2017-12-04  9:29       ` Boris Brezillon
  0 siblings, 0 replies; 8+ messages in thread
From: Boris Brezillon @ 2017-12-04  9:29 UTC (permalink / raw)
  To: Adam Ford
  Cc: Richard Weinberger, linux-mtd, Marek Vasut, Cyrille Pitchen,
	Brian Norris, David Woodhouse

Hi Adam,

On Sun, 3 Dec 2017 08:57:17 -0600
Adam Ford <aford173@gmail.com> wrote:

> On Fri, Aug 4, 2017 at 3:32 AM, Boris Brezillon <
> boris.brezillon@free-electrons.com> wrote:
> 
> > On Tue, 16 May 2017 00:17:42 +0200
> > Boris Brezillon <boris.brezillon@free-electrons.com> wrote:
> >  
> > > Commit 7d70f334ad2b ("mtd: nand: add lock/unlock routines") introduced
> > > support for the Micron LOCK/UNLOCK commands but no one ever used the
> > > nand_lock/unlock() functions.
> > >
> > > Remove support for these vendor-specific operations from the core. If
> > > one ever wants to add them back they should be put in nand_micron.c and
> > > mtd->_lock/_unlock should be directly assigned from there instead of
> > > exporting the functions.  
> >
> >  
> I am running into some issues with a board using Micron Flash that cannot
> boot from a UBIFS partition and it appears to be related the NAND being
> locked.  I was going to investigate how to unlock the NAND and I came
> across this.  I am attempting to do what you suggested by moving these
> functions to the nand_micron.c file, but these functions are dependant on
> several static functions inside nand_base.c
> namely: check_offs_len, nand_get_device, nand_check_wp,
> and nand_release_device.  I assume that we don't want to export all those
> functions, but it seems redundant to copy these functions to nand_micron.c.

I don't see a problem in exposing those symbols so that NAND vendor
drivers can use them (note that you don't need to use EXPORT_SYMBOL()
in this case, because nand_xxx.c files are all linked in a single
object)?

This being said, I think the check_offs_len() can be dropped since it's
already checked in mtd_[un]lock().

> 
> Do you have any thoughts or suggestions on how to go about using these
> functions without replicating code?

I have no objection to adding this feature back in the Micron driver,
though I'd like to wait for the ->exec_op() series [1] to be merged,
because I'm not sure all NAND controller drivers handle the UNLOCK/LOCK
operations properly (custom ->cmdfunc() implementations tend to only
support basic operations). The idea is to check whether the controller
supports the LOCK/UNLOCK operations and in this case assign the
mtd->_lock/_unlock() hooks to micron's implementation.

Regards,

Boris

[1]http://patchwork.ozlabs.org/project/linux-mtd/list/?series=16013&state=%2A&archive=both

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-12-04  9:30 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-15 22:17 [PATCH 0/3] mtd: nand: Remove unused features Boris Brezillon
2017-05-15 22:17 ` [PATCH 1/3] mtd: nand: Drop unused cached programming support Boris Brezillon
2017-05-15 22:17 ` [PATCH 2/3] mtd: nand: Remove support for block locking/unlocking Boris Brezillon
2017-08-04  8:32   ` Boris Brezillon
     [not found]     ` <CAHCN7xKtR=kjLLFYkQkhjKGsuU+ttRE9FA_p7aWGDWVWxf2S8Q@mail.gmail.com>
2017-12-04  9:29       ` Boris Brezillon
2017-05-15 22:17 ` [PATCH 3/3] mtd: nand: Drop the ->errstat() hook Boris Brezillon
2017-05-17  8:49   ` Boris Brezillon
2017-05-29 18:52 ` [PATCH 0/3] mtd: nand: Remove unused features Boris Brezillon

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.