linux-mtd.lists.infradead.org archive mirror
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).