All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/4] mtd: rawnand: Add support Macronix Block Portection & Deep Power Down mode
@ 2020-03-03  7:21 ` Mason Yang
  0 siblings, 0 replies; 46+ messages in thread
From: Mason Yang @ 2020-03-03  7:21 UTC (permalink / raw)
  To: miquel.raynal, richard, vigneshr
  Cc: frieder.schrempf, tglx, stefan, juliensu, allison, linux-kernel,
	bbrezillon, rfontana, linux-mtd, yuehaibing, s.hauer, Mason Yang

Hi,

Changelog

v3:
patch nand_lock_area/nand_unlock_area.
fixed kbuidtest robot warnings and reviewer's comments.

v2:
Patch nand_lock() & nand_unlock() for MTD->_lock/_unlock default call-back
function replacement. 
Patch nand_suspend() & nand_resume() with manufacturer specific operation.

v1:
Patch manufacturer post_init for MTD->_lock/_unlock & MTD->_suspend/_resume
replacement.

thanks for your time & review.
Mason


Mason Yang (4):
  mtd: rawnand: Add support manufacturer specific lock/unlock operation
  mtd: rawnand: Add support Macronix Block Protection function
  mtd: rawnand: Add support manufacturer specific suspend/resume
    operation
  mtd: rawnand: Add support Macronix deep power down mode

 drivers/mtd/nand/raw/nand_base.c     |  47 +++++++++--
 drivers/mtd/nand/raw/nand_macronix.c | 146 +++++++++++++++++++++++++++++++++++
 include/linux/mtd/rawnand.h          |   9 +++
 3 files changed, 197 insertions(+), 5 deletions(-)

-- 
1.9.1


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

* [PATCH v3 0/4] mtd: rawnand: Add support Macronix Block Portection & Deep Power Down mode
@ 2020-03-03  7:21 ` Mason Yang
  0 siblings, 0 replies; 46+ messages in thread
From: Mason Yang @ 2020-03-03  7:21 UTC (permalink / raw)
  To: miquel.raynal, richard, vigneshr
  Cc: bbrezillon, juliensu, s.hauer, yuehaibing, linux-kernel, stefan,
	rfontana, linux-mtd, frieder.schrempf, tglx, Mason Yang, allison

Hi,

Changelog

v3:
patch nand_lock_area/nand_unlock_area.
fixed kbuidtest robot warnings and reviewer's comments.

v2:
Patch nand_lock() & nand_unlock() for MTD->_lock/_unlock default call-back
function replacement. 
Patch nand_suspend() & nand_resume() with manufacturer specific operation.

v1:
Patch manufacturer post_init for MTD->_lock/_unlock & MTD->_suspend/_resume
replacement.

thanks for your time & review.
Mason


Mason Yang (4):
  mtd: rawnand: Add support manufacturer specific lock/unlock operation
  mtd: rawnand: Add support Macronix Block Protection function
  mtd: rawnand: Add support manufacturer specific suspend/resume
    operation
  mtd: rawnand: Add support Macronix deep power down mode

 drivers/mtd/nand/raw/nand_base.c     |  47 +++++++++--
 drivers/mtd/nand/raw/nand_macronix.c | 146 +++++++++++++++++++++++++++++++++++
 include/linux/mtd/rawnand.h          |   9 +++
 3 files changed, 197 insertions(+), 5 deletions(-)

-- 
1.9.1


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

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

* [PATCH v3 1/4] mtd: rawnand: Add support manufacturer specific lock/unlock operation
  2020-03-03  7:21 ` Mason Yang
@ 2020-03-03  7:21   ` Mason Yang
  -1 siblings, 0 replies; 46+ messages in thread
From: Mason Yang @ 2020-03-03  7:21 UTC (permalink / raw)
  To: miquel.raynal, richard, vigneshr
  Cc: frieder.schrempf, tglx, stefan, juliensu, allison, linux-kernel,
	bbrezillon, rfontana, linux-mtd, yuehaibing, s.hauer, Mason Yang

Add nand_lock() & nand_unlock() for manufacturer specific lock & unlock
operation while the device supports Block Portection function.

Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
Reported-by: kbuild test robot <lkp@intel.com>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/nand_base.c | 36 ++++++++++++++++++++++++++++++++++--
 include/linux/mtd/rawnand.h      |  5 +++++
 2 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index f64e3b6..769be81 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -4360,6 +4360,38 @@ static void nand_shutdown(struct mtd_info *mtd)
 	nand_suspend(mtd);
 }
 
+/**
+ * nand_lock - [MTD Interface] Lock the NAND flash
+ * @mtd: MTD device structure
+ * @ofs: offset byte address
+ * @len: number of bytes to lock (must be a multiple of block/page size)
+ */
+static int nand_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
+{
+	struct nand_chip *chip = mtd_to_nand(mtd);
+
+	if (!chip->lock_area)
+		return -ENOTSUPP;
+
+	return chip->lock_area(chip, ofs, len);
+}
+
+/**
+ * nand_unlock - [MTD Interface] Unlock the NAND flash
+ * @mtd: MTD device structure
+ * @ofs: offset byte address
+ * @len: number of bytes to unlock (must be a multiple of block/page size)
+ */
+static int nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
+{
+	struct nand_chip *chip = mtd_to_nand(mtd);
+
+	if (!chip->unlock_area)
+		return -ENOTSUPP;
+
+	return chip->unlock_area(chip, ofs, len);
+}
+
 /* Set default functions */
 static void nand_set_defaults(struct nand_chip *chip)
 {
@@ -5786,8 +5818,8 @@ static int nand_scan_tail(struct nand_chip *chip)
 	mtd->_read_oob = nand_read_oob;
 	mtd->_write_oob = nand_write_oob;
 	mtd->_sync = nand_sync;
-	mtd->_lock = NULL;
-	mtd->_unlock = NULL;
+	mtd->_lock = nand_lock;
+	mtd->_unlock = nand_unlock;
 	mtd->_suspend = nand_suspend;
 	mtd->_resume = nand_resume;
 	mtd->_reboot = nand_shutdown;
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 4ab9bcc..bc2fa3c 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -1077,6 +1077,8 @@ struct nand_legacy {
  * @manufacturer:	[INTERN] Contains manufacturer information
  * @manufacturer.desc:	[INTERN] Contains manufacturer's description
  * @manufacturer.priv:	[INTERN] Contains manufacturer private information
+ * @lock_area:		[REPLACEABLE] specific NAND chip lock operation
+ * @unlock_area:	[REPLACEABLE] specific NAND chip unlock operation
  */
 
 struct nand_chip {
@@ -1136,6 +1138,9 @@ struct nand_chip {
 		const struct nand_manufacturer *desc;
 		void *priv;
 	} manufacturer;
+
+	int (*lock_area)(struct nand_chip *chip, loff_t ofs, uint64_t len);
+	int (*unlock_area)(struct nand_chip *chip, loff_t ofs, uint64_t len);
 };
 
 extern const struct mtd_ooblayout_ops nand_ooblayout_sp_ops;
-- 
1.9.1


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

* [PATCH v3 1/4] mtd: rawnand: Add support manufacturer specific lock/unlock operation
@ 2020-03-03  7:21   ` Mason Yang
  0 siblings, 0 replies; 46+ messages in thread
From: Mason Yang @ 2020-03-03  7:21 UTC (permalink / raw)
  To: miquel.raynal, richard, vigneshr
  Cc: bbrezillon, juliensu, s.hauer, yuehaibing, linux-kernel, stefan,
	rfontana, linux-mtd, frieder.schrempf, tglx, Mason Yang, allison

Add nand_lock() & nand_unlock() for manufacturer specific lock & unlock
operation while the device supports Block Portection function.

Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
Reported-by: kbuild test robot <lkp@intel.com>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/nand_base.c | 36 ++++++++++++++++++++++++++++++++++--
 include/linux/mtd/rawnand.h      |  5 +++++
 2 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index f64e3b6..769be81 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -4360,6 +4360,38 @@ static void nand_shutdown(struct mtd_info *mtd)
 	nand_suspend(mtd);
 }
 
+/**
+ * nand_lock - [MTD Interface] Lock the NAND flash
+ * @mtd: MTD device structure
+ * @ofs: offset byte address
+ * @len: number of bytes to lock (must be a multiple of block/page size)
+ */
+static int nand_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
+{
+	struct nand_chip *chip = mtd_to_nand(mtd);
+
+	if (!chip->lock_area)
+		return -ENOTSUPP;
+
+	return chip->lock_area(chip, ofs, len);
+}
+
+/**
+ * nand_unlock - [MTD Interface] Unlock the NAND flash
+ * @mtd: MTD device structure
+ * @ofs: offset byte address
+ * @len: number of bytes to unlock (must be a multiple of block/page size)
+ */
+static int nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
+{
+	struct nand_chip *chip = mtd_to_nand(mtd);
+
+	if (!chip->unlock_area)
+		return -ENOTSUPP;
+
+	return chip->unlock_area(chip, ofs, len);
+}
+
 /* Set default functions */
 static void nand_set_defaults(struct nand_chip *chip)
 {
@@ -5786,8 +5818,8 @@ static int nand_scan_tail(struct nand_chip *chip)
 	mtd->_read_oob = nand_read_oob;
 	mtd->_write_oob = nand_write_oob;
 	mtd->_sync = nand_sync;
-	mtd->_lock = NULL;
-	mtd->_unlock = NULL;
+	mtd->_lock = nand_lock;
+	mtd->_unlock = nand_unlock;
 	mtd->_suspend = nand_suspend;
 	mtd->_resume = nand_resume;
 	mtd->_reboot = nand_shutdown;
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 4ab9bcc..bc2fa3c 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -1077,6 +1077,8 @@ struct nand_legacy {
  * @manufacturer:	[INTERN] Contains manufacturer information
  * @manufacturer.desc:	[INTERN] Contains manufacturer's description
  * @manufacturer.priv:	[INTERN] Contains manufacturer private information
+ * @lock_area:		[REPLACEABLE] specific NAND chip lock operation
+ * @unlock_area:	[REPLACEABLE] specific NAND chip unlock operation
  */
 
 struct nand_chip {
@@ -1136,6 +1138,9 @@ struct nand_chip {
 		const struct nand_manufacturer *desc;
 		void *priv;
 	} manufacturer;
+
+	int (*lock_area)(struct nand_chip *chip, loff_t ofs, uint64_t len);
+	int (*unlock_area)(struct nand_chip *chip, loff_t ofs, uint64_t len);
 };
 
 extern const struct mtd_ooblayout_ops nand_ooblayout_sp_ops;
-- 
1.9.1


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

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

* [PATCH v3 2/4] mtd: rawnand: Add support Macronix Block Protection function
  2020-03-03  7:21 ` Mason Yang
@ 2020-03-03  7:21   ` Mason Yang
  -1 siblings, 0 replies; 46+ messages in thread
From: Mason Yang @ 2020-03-03  7:21 UTC (permalink / raw)
  To: miquel.raynal, richard, vigneshr
  Cc: frieder.schrempf, tglx, stefan, juliensu, allison, linux-kernel,
	bbrezillon, rfontana, linux-mtd, yuehaibing, s.hauer, Mason Yang

Macronix AC/AD series support using SET_FEATURES to change
Block Portection and Unprotection. By GET_FEATURES operation
to detect if block protection support.

Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
---
 drivers/mtd/nand/raw/nand_macronix.c | 72 ++++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/drivers/mtd/nand/raw/nand_macronix.c b/drivers/mtd/nand/raw/nand_macronix.c
index 3ff7ce0..a4cd12c 100644
--- a/drivers/mtd/nand/raw/nand_macronix.c
+++ b/drivers/mtd/nand/raw/nand_macronix.c
@@ -11,6 +11,10 @@
 #define MACRONIX_READ_RETRY_BIT BIT(0)
 #define MACRONIX_NUM_READ_RETRY_MODES 6
 
+#define ONFI_FEATURE_ADDR_MXIC_PROTECTION 0xA0
+#define MXIC_BLOCK_PROTECTION_ALL_LOCK 0x38
+#define MXIC_BLOCK_PROTECTION_ALL_UNLOCK 0x0
+
 struct nand_onfi_vendor_macronix {
 	u8 reserved;
 	u8 reliability_func;
@@ -91,6 +95,73 @@ static void macronix_nand_fix_broken_get_timings(struct nand_chip *chip)
 		     ONFI_FEATURE_ADDR_TIMING_MODE, 1);
 }
 
+/*
+ * Macronix NAND supports Block Protection by Protectoin(PT) pin;
+ * active high at power-on which protects the entire chip even the #WP is
+ * disabled. Lock/unlock protection area can be partition according to
+ * protection bits, i.e. upper 1/2 locked, upper 1/4 locked and so on.
+ */
+static int mxic_nand_lock(struct nand_chip *chip, loff_t ofs, uint64_t len)
+{
+	u8 feature[ONFI_SUBFEATURE_PARAM_LEN];
+	int ret;
+
+	feature[0] = MXIC_BLOCK_PROTECTION_ALL_LOCK;
+	nand_select_target(chip, 0);
+	ret = nand_set_features(chip, ONFI_FEATURE_ADDR_MXIC_PROTECTION,
+				feature);
+	nand_deselect_target(chip);
+	if (ret)
+		pr_err("%s all blocks failed\n", __func__);
+
+	return ret;
+}
+
+static int mxic_nand_unlock(struct nand_chip *chip, loff_t ofs, uint64_t len)
+{
+	u8 feature[ONFI_SUBFEATURE_PARAM_LEN];
+	int ret;
+
+	feature[0] = MXIC_BLOCK_PROTECTION_ALL_UNLOCK;
+	nand_select_target(chip, 0);
+	ret = nand_set_features(chip, ONFI_FEATURE_ADDR_MXIC_PROTECTION,
+				feature);
+	nand_deselect_target(chip);
+	if (ret)
+		pr_err("%s all blocks failed\n", __func__);
+
+	return ret;
+}
+
+static void macronix_nand_block_protection_support(struct nand_chip *chip)
+{
+	u8 feature[ONFI_SUBFEATURE_PARAM_LEN];
+	int ret;
+
+	bitmap_set(chip->parameters.get_feature_list,
+		   ONFI_FEATURE_ADDR_MXIC_PROTECTION, 1);
+
+	feature[0] = MXIC_BLOCK_PROTECTION_ALL_UNLOCK;
+	nand_select_target(chip, 0);
+	ret = nand_get_features(chip, ONFI_FEATURE_ADDR_MXIC_PROTECTION,
+				feature);
+	nand_deselect_target(chip);
+	if (ret || feature[0] != MXIC_BLOCK_PROTECTION_ALL_LOCK) {
+		if (ret)
+			pr_err("Block protection check failed\n");
+
+		bitmap_clear(chip->parameters.get_feature_list,
+			     ONFI_FEATURE_ADDR_MXIC_PROTECTION, 1);
+		return;
+	}
+
+	bitmap_set(chip->parameters.set_feature_list,
+		   ONFI_FEATURE_ADDR_MXIC_PROTECTION, 1);
+
+	chip->lock_area = mxic_nand_lock;
+	chip->unlock_area = mxic_nand_unlock;
+}
+
 static int macronix_nand_init(struct nand_chip *chip)
 {
 	if (nand_is_slc(chip))
@@ -98,6 +169,7 @@ static int macronix_nand_init(struct nand_chip *chip)
 
 	macronix_nand_fix_broken_get_timings(chip);
 	macronix_nand_onfi_init(chip);
+	macronix_nand_block_protection_support(chip);
 
 	return 0;
 }
-- 
1.9.1


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

* [PATCH v3 2/4] mtd: rawnand: Add support Macronix Block Protection function
@ 2020-03-03  7:21   ` Mason Yang
  0 siblings, 0 replies; 46+ messages in thread
From: Mason Yang @ 2020-03-03  7:21 UTC (permalink / raw)
  To: miquel.raynal, richard, vigneshr
  Cc: bbrezillon, juliensu, s.hauer, yuehaibing, linux-kernel, stefan,
	rfontana, linux-mtd, frieder.schrempf, tglx, Mason Yang, allison

Macronix AC/AD series support using SET_FEATURES to change
Block Portection and Unprotection. By GET_FEATURES operation
to detect if block protection support.

Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
---
 drivers/mtd/nand/raw/nand_macronix.c | 72 ++++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/drivers/mtd/nand/raw/nand_macronix.c b/drivers/mtd/nand/raw/nand_macronix.c
index 3ff7ce0..a4cd12c 100644
--- a/drivers/mtd/nand/raw/nand_macronix.c
+++ b/drivers/mtd/nand/raw/nand_macronix.c
@@ -11,6 +11,10 @@
 #define MACRONIX_READ_RETRY_BIT BIT(0)
 #define MACRONIX_NUM_READ_RETRY_MODES 6
 
+#define ONFI_FEATURE_ADDR_MXIC_PROTECTION 0xA0
+#define MXIC_BLOCK_PROTECTION_ALL_LOCK 0x38
+#define MXIC_BLOCK_PROTECTION_ALL_UNLOCK 0x0
+
 struct nand_onfi_vendor_macronix {
 	u8 reserved;
 	u8 reliability_func;
@@ -91,6 +95,73 @@ static void macronix_nand_fix_broken_get_timings(struct nand_chip *chip)
 		     ONFI_FEATURE_ADDR_TIMING_MODE, 1);
 }
 
+/*
+ * Macronix NAND supports Block Protection by Protectoin(PT) pin;
+ * active high at power-on which protects the entire chip even the #WP is
+ * disabled. Lock/unlock protection area can be partition according to
+ * protection bits, i.e. upper 1/2 locked, upper 1/4 locked and so on.
+ */
+static int mxic_nand_lock(struct nand_chip *chip, loff_t ofs, uint64_t len)
+{
+	u8 feature[ONFI_SUBFEATURE_PARAM_LEN];
+	int ret;
+
+	feature[0] = MXIC_BLOCK_PROTECTION_ALL_LOCK;
+	nand_select_target(chip, 0);
+	ret = nand_set_features(chip, ONFI_FEATURE_ADDR_MXIC_PROTECTION,
+				feature);
+	nand_deselect_target(chip);
+	if (ret)
+		pr_err("%s all blocks failed\n", __func__);
+
+	return ret;
+}
+
+static int mxic_nand_unlock(struct nand_chip *chip, loff_t ofs, uint64_t len)
+{
+	u8 feature[ONFI_SUBFEATURE_PARAM_LEN];
+	int ret;
+
+	feature[0] = MXIC_BLOCK_PROTECTION_ALL_UNLOCK;
+	nand_select_target(chip, 0);
+	ret = nand_set_features(chip, ONFI_FEATURE_ADDR_MXIC_PROTECTION,
+				feature);
+	nand_deselect_target(chip);
+	if (ret)
+		pr_err("%s all blocks failed\n", __func__);
+
+	return ret;
+}
+
+static void macronix_nand_block_protection_support(struct nand_chip *chip)
+{
+	u8 feature[ONFI_SUBFEATURE_PARAM_LEN];
+	int ret;
+
+	bitmap_set(chip->parameters.get_feature_list,
+		   ONFI_FEATURE_ADDR_MXIC_PROTECTION, 1);
+
+	feature[0] = MXIC_BLOCK_PROTECTION_ALL_UNLOCK;
+	nand_select_target(chip, 0);
+	ret = nand_get_features(chip, ONFI_FEATURE_ADDR_MXIC_PROTECTION,
+				feature);
+	nand_deselect_target(chip);
+	if (ret || feature[0] != MXIC_BLOCK_PROTECTION_ALL_LOCK) {
+		if (ret)
+			pr_err("Block protection check failed\n");
+
+		bitmap_clear(chip->parameters.get_feature_list,
+			     ONFI_FEATURE_ADDR_MXIC_PROTECTION, 1);
+		return;
+	}
+
+	bitmap_set(chip->parameters.set_feature_list,
+		   ONFI_FEATURE_ADDR_MXIC_PROTECTION, 1);
+
+	chip->lock_area = mxic_nand_lock;
+	chip->unlock_area = mxic_nand_unlock;
+}
+
 static int macronix_nand_init(struct nand_chip *chip)
 {
 	if (nand_is_slc(chip))
@@ -98,6 +169,7 @@ static int macronix_nand_init(struct nand_chip *chip)
 
 	macronix_nand_fix_broken_get_timings(chip);
 	macronix_nand_onfi_init(chip);
+	macronix_nand_block_protection_support(chip);
 
 	return 0;
 }
-- 
1.9.1


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

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

* [PATCH v3 3/4] mtd: rawnand: Add support manufacturer specific suspend/resume operation
  2020-03-03  7:21 ` Mason Yang
@ 2020-03-03  7:21   ` Mason Yang
  -1 siblings, 0 replies; 46+ messages in thread
From: Mason Yang @ 2020-03-03  7:21 UTC (permalink / raw)
  To: miquel.raynal, richard, vigneshr
  Cc: frieder.schrempf, tglx, stefan, juliensu, allison, linux-kernel,
	bbrezillon, rfontana, linux-mtd, yuehaibing, s.hauer, Mason Yang

Patch nand_suspend() & nand_resume() for manufacturer specific
suspend/resume operation.

Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
Reported-by: kbuild test robot <lkp@intel.com>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/nand_base.c | 11 ++++++++---
 include/linux/mtd/rawnand.h      |  4 ++++
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 769be81..b44e460 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -4327,7 +4327,9 @@ static int nand_suspend(struct mtd_info *mtd)
 	struct nand_chip *chip = mtd_to_nand(mtd);
 
 	mutex_lock(&chip->lock);
-	chip->suspended = 1;
+	if (chip->_suspend)
+		if (!chip->_suspend(chip))
+			chip->suspended = 1;
 	mutex_unlock(&chip->lock);
 
 	return 0;
@@ -4342,11 +4344,14 @@ static void nand_resume(struct mtd_info *mtd)
 	struct nand_chip *chip = mtd_to_nand(mtd);
 
 	mutex_lock(&chip->lock);
-	if (chip->suspended)
+	if (chip->suspended) {
+		if (chip->_resume)
+			chip->_resume(chip);
 		chip->suspended = 0;
-	else
+	} else {
 		pr_err("%s called for a chip which is not in suspended state\n",
 			__func__);
+	}
 	mutex_unlock(&chip->lock);
 }
 
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index bc2fa3c..c0055ed 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -1064,6 +1064,8 @@ struct nand_legacy {
  * @lock:		lock protecting the suspended field. Also used to
  *			serialize accesses to the NAND device.
  * @suspended:		set to 1 when the device is suspended, 0 when it's not.
+ * @_suspend:		[REPLACEABLE] specific NAND device suspend operation
+ * @_resume:		[REPLACEABLE] specific NAND device resume operation
  * @bbt:		[INTERN] bad block table pointer
  * @bbt_td:		[REPLACEABLE] bad block table descriptor for flash
  *			lookup.
@@ -1119,6 +1121,8 @@ struct nand_chip {
 
 	struct mutex lock;
 	unsigned int suspended : 1;
+	int (*_suspend)(struct nand_chip *chip);
+	void (*_resume)(struct nand_chip *chip);
 
 	uint8_t *oob_poi;
 	struct nand_controller *controller;
-- 
1.9.1


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

* [PATCH v3 3/4] mtd: rawnand: Add support manufacturer specific suspend/resume operation
@ 2020-03-03  7:21   ` Mason Yang
  0 siblings, 0 replies; 46+ messages in thread
From: Mason Yang @ 2020-03-03  7:21 UTC (permalink / raw)
  To: miquel.raynal, richard, vigneshr
  Cc: bbrezillon, juliensu, s.hauer, yuehaibing, linux-kernel, stefan,
	rfontana, linux-mtd, frieder.schrempf, tglx, Mason Yang, allison

Patch nand_suspend() & nand_resume() for manufacturer specific
suspend/resume operation.

Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
Reported-by: kbuild test robot <lkp@intel.com>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/nand_base.c | 11 ++++++++---
 include/linux/mtd/rawnand.h      |  4 ++++
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 769be81..b44e460 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -4327,7 +4327,9 @@ static int nand_suspend(struct mtd_info *mtd)
 	struct nand_chip *chip = mtd_to_nand(mtd);
 
 	mutex_lock(&chip->lock);
-	chip->suspended = 1;
+	if (chip->_suspend)
+		if (!chip->_suspend(chip))
+			chip->suspended = 1;
 	mutex_unlock(&chip->lock);
 
 	return 0;
@@ -4342,11 +4344,14 @@ static void nand_resume(struct mtd_info *mtd)
 	struct nand_chip *chip = mtd_to_nand(mtd);
 
 	mutex_lock(&chip->lock);
-	if (chip->suspended)
+	if (chip->suspended) {
+		if (chip->_resume)
+			chip->_resume(chip);
 		chip->suspended = 0;
-	else
+	} else {
 		pr_err("%s called for a chip which is not in suspended state\n",
 			__func__);
+	}
 	mutex_unlock(&chip->lock);
 }
 
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index bc2fa3c..c0055ed 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -1064,6 +1064,8 @@ struct nand_legacy {
  * @lock:		lock protecting the suspended field. Also used to
  *			serialize accesses to the NAND device.
  * @suspended:		set to 1 when the device is suspended, 0 when it's not.
+ * @_suspend:		[REPLACEABLE] specific NAND device suspend operation
+ * @_resume:		[REPLACEABLE] specific NAND device resume operation
  * @bbt:		[INTERN] bad block table pointer
  * @bbt_td:		[REPLACEABLE] bad block table descriptor for flash
  *			lookup.
@@ -1119,6 +1121,8 @@ struct nand_chip {
 
 	struct mutex lock;
 	unsigned int suspended : 1;
+	int (*_suspend)(struct nand_chip *chip);
+	void (*_resume)(struct nand_chip *chip);
 
 	uint8_t *oob_poi;
 	struct nand_controller *controller;
-- 
1.9.1


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

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

* [PATCH v3 4/4] mtd: rawnand: Add support Macronix deep power down mode
  2020-03-03  7:21 ` Mason Yang
@ 2020-03-03  7:21   ` Mason Yang
  -1 siblings, 0 replies; 46+ messages in thread
From: Mason Yang @ 2020-03-03  7:21 UTC (permalink / raw)
  To: miquel.raynal, richard, vigneshr
  Cc: frieder.schrempf, tglx, stefan, juliensu, allison, linux-kernel,
	bbrezillon, rfontana, linux-mtd, yuehaibing, s.hauer, Mason Yang

Macronix AD series support deep power down mode for a minimum
power consumption state.

Patch nand_suspend() & nand_resume() by Macronix specific deep
power down mode command and exit it.

Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
---
 drivers/mtd/nand/raw/nand_macronix.c | 74 ++++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)

diff --git a/drivers/mtd/nand/raw/nand_macronix.c b/drivers/mtd/nand/raw/nand_macronix.c
index a4cd12c..ca46ec1 100644
--- a/drivers/mtd/nand/raw/nand_macronix.c
+++ b/drivers/mtd/nand/raw/nand_macronix.c
@@ -6,6 +6,7 @@
  * Author: Boris Brezillon <boris.brezillon@free-electrons.com>
  */
 
+#include "linux/delay.h"
 #include "internals.h"
 
 #define MACRONIX_READ_RETRY_BIT BIT(0)
@@ -15,6 +16,8 @@
 #define MXIC_BLOCK_PROTECTION_ALL_LOCK 0x38
 #define MXIC_BLOCK_PROTECTION_ALL_UNLOCK 0x0
 
+#define MXIC_CMD_POWER_DOWN 0xB9
+
 struct nand_onfi_vendor_macronix {
 	u8 reserved;
 	u8 reliability_func;
@@ -162,6 +165,76 @@ static void macronix_nand_block_protection_support(struct nand_chip *chip)
 	chip->unlock_area = mxic_nand_unlock;
 }
 
+static int nand_power_down_op(struct nand_chip *chip)
+{
+	int ret;
+
+	if (nand_has_exec_op(chip)) {
+		struct nand_op_instr instrs[] = {
+			NAND_OP_CMD(MXIC_CMD_POWER_DOWN, 0),
+		};
+
+		struct nand_operation op = NAND_OPERATION(chip->cur_cs, instrs);
+
+		ret = nand_exec_op(chip, &op);
+		if (ret)
+			return ret;
+
+	} else {
+		chip->legacy.cmdfunc(chip, MXIC_CMD_POWER_DOWN, -1, -1);
+	}
+
+	return 0;
+}
+
+static int mxic_nand_suspend(struct nand_chip *chip)
+{
+	int ret;
+
+	nand_select_target(chip, 0);
+	ret = nand_power_down_op(chip);
+	if (ret < 0)
+		pr_err("Suspending MXIC NAND chip failed (%d)\n", ret);
+	nand_deselect_target(chip);
+
+	return ret;
+}
+
+static void mxic_nand_resume(struct nand_chip *chip)
+{
+	/*
+	 * Toggle #CS pin to resume NAND device and don't care
+	 * of the others CLE, #WE, #RE pins status.
+	 * A NAND controller ensure it is able to assert/de-assert #CS
+	 * by sending any byte over the NAND bus.
+	 * i.e.,
+	 * NAND power down command or reset command w/o R/B# status checking.
+	 */
+	nand_select_target(chip, 0);
+	nand_power_down_op(chip);
+	/* The minimum of a recovery time tRDP is 35 us */
+	usleep_range(35, 100);
+	nand_deselect_target(chip);
+}
+
+static void macronix_nand_deep_power_down_support(struct nand_chip *chip)
+{
+	int i;
+	static const char * const deep_power_down_dev[] = {
+		"MX30UF1G28AD",
+		"MX30UF2G28AD",
+		"MX30UF4G28AD",
+	};
+
+	i = match_string(deep_power_down_dev, ARRAY_SIZE(deep_power_down_dev),
+			 chip->parameters.model);
+	if (i < 0)
+		return;
+
+	chip->_suspend = mxic_nand_suspend;
+	chip->_resume = mxic_nand_resume;
+}
+
 static int macronix_nand_init(struct nand_chip *chip)
 {
 	if (nand_is_slc(chip))
@@ -170,6 +243,7 @@ static int macronix_nand_init(struct nand_chip *chip)
 	macronix_nand_fix_broken_get_timings(chip);
 	macronix_nand_onfi_init(chip);
 	macronix_nand_block_protection_support(chip);
+	macronix_nand_deep_power_down_support(chip);
 
 	return 0;
 }
-- 
1.9.1


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

* [PATCH v3 4/4] mtd: rawnand: Add support Macronix deep power down mode
@ 2020-03-03  7:21   ` Mason Yang
  0 siblings, 0 replies; 46+ messages in thread
From: Mason Yang @ 2020-03-03  7:21 UTC (permalink / raw)
  To: miquel.raynal, richard, vigneshr
  Cc: bbrezillon, juliensu, s.hauer, yuehaibing, linux-kernel, stefan,
	rfontana, linux-mtd, frieder.schrempf, tglx, Mason Yang, allison

Macronix AD series support deep power down mode for a minimum
power consumption state.

Patch nand_suspend() & nand_resume() by Macronix specific deep
power down mode command and exit it.

Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
---
 drivers/mtd/nand/raw/nand_macronix.c | 74 ++++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)

diff --git a/drivers/mtd/nand/raw/nand_macronix.c b/drivers/mtd/nand/raw/nand_macronix.c
index a4cd12c..ca46ec1 100644
--- a/drivers/mtd/nand/raw/nand_macronix.c
+++ b/drivers/mtd/nand/raw/nand_macronix.c
@@ -6,6 +6,7 @@
  * Author: Boris Brezillon <boris.brezillon@free-electrons.com>
  */
 
+#include "linux/delay.h"
 #include "internals.h"
 
 #define MACRONIX_READ_RETRY_BIT BIT(0)
@@ -15,6 +16,8 @@
 #define MXIC_BLOCK_PROTECTION_ALL_LOCK 0x38
 #define MXIC_BLOCK_PROTECTION_ALL_UNLOCK 0x0
 
+#define MXIC_CMD_POWER_DOWN 0xB9
+
 struct nand_onfi_vendor_macronix {
 	u8 reserved;
 	u8 reliability_func;
@@ -162,6 +165,76 @@ static void macronix_nand_block_protection_support(struct nand_chip *chip)
 	chip->unlock_area = mxic_nand_unlock;
 }
 
+static int nand_power_down_op(struct nand_chip *chip)
+{
+	int ret;
+
+	if (nand_has_exec_op(chip)) {
+		struct nand_op_instr instrs[] = {
+			NAND_OP_CMD(MXIC_CMD_POWER_DOWN, 0),
+		};
+
+		struct nand_operation op = NAND_OPERATION(chip->cur_cs, instrs);
+
+		ret = nand_exec_op(chip, &op);
+		if (ret)
+			return ret;
+
+	} else {
+		chip->legacy.cmdfunc(chip, MXIC_CMD_POWER_DOWN, -1, -1);
+	}
+
+	return 0;
+}
+
+static int mxic_nand_suspend(struct nand_chip *chip)
+{
+	int ret;
+
+	nand_select_target(chip, 0);
+	ret = nand_power_down_op(chip);
+	if (ret < 0)
+		pr_err("Suspending MXIC NAND chip failed (%d)\n", ret);
+	nand_deselect_target(chip);
+
+	return ret;
+}
+
+static void mxic_nand_resume(struct nand_chip *chip)
+{
+	/*
+	 * Toggle #CS pin to resume NAND device and don't care
+	 * of the others CLE, #WE, #RE pins status.
+	 * A NAND controller ensure it is able to assert/de-assert #CS
+	 * by sending any byte over the NAND bus.
+	 * i.e.,
+	 * NAND power down command or reset command w/o R/B# status checking.
+	 */
+	nand_select_target(chip, 0);
+	nand_power_down_op(chip);
+	/* The minimum of a recovery time tRDP is 35 us */
+	usleep_range(35, 100);
+	nand_deselect_target(chip);
+}
+
+static void macronix_nand_deep_power_down_support(struct nand_chip *chip)
+{
+	int i;
+	static const char * const deep_power_down_dev[] = {
+		"MX30UF1G28AD",
+		"MX30UF2G28AD",
+		"MX30UF4G28AD",
+	};
+
+	i = match_string(deep_power_down_dev, ARRAY_SIZE(deep_power_down_dev),
+			 chip->parameters.model);
+	if (i < 0)
+		return;
+
+	chip->_suspend = mxic_nand_suspend;
+	chip->_resume = mxic_nand_resume;
+}
+
 static int macronix_nand_init(struct nand_chip *chip)
 {
 	if (nand_is_slc(chip))
@@ -170,6 +243,7 @@ static int macronix_nand_init(struct nand_chip *chip)
 	macronix_nand_fix_broken_get_timings(chip);
 	macronix_nand_onfi_init(chip);
 	macronix_nand_block_protection_support(chip);
+	macronix_nand_deep_power_down_support(chip);
 
 	return 0;
 }
-- 
1.9.1


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

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

* Re: [PATCH v3 0/4] mtd: rawnand: Add support Macronix Block Portection & Deep Power Down mode
  2020-03-03  7:21 ` Mason Yang
@ 2020-03-09 13:14   ` Miquel Raynal
  -1 siblings, 0 replies; 46+ messages in thread
From: Miquel Raynal @ 2020-03-09 13:14 UTC (permalink / raw)
  To: Mason Yang
  Cc: richard, vigneshr, frieder.schrempf, tglx, stefan, juliensu,
	allison, linux-kernel, bbrezillon, rfontana, linux-mtd,
	yuehaibing, s.hauer

Hi Mason,

Mason Yang <masonccyang@mxic.com.tw> wrote on Tue,  3 Mar 2020 15:21:20
+0800:

> Hi,
> 
> Changelog
> 
> v3:
> patch nand_lock_area/nand_unlock_area.
> fixed kbuidtest robot warnings and reviewer's comments.

I know it is painful for the contributor but I really need more details
in the changelog. This is something I care about because I can speed-up
my reviews when I know what I already acked or not. "fixing reviewer's
comments" is way too vague, I have absolutely no idea of what I told
you last time :) So please, for the next iterations, be more verbose in
these changelogs! (that's fine for this one, I'll check myself).

Cheers,
Miquèl

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

* Re: [PATCH v3 0/4] mtd: rawnand: Add support Macronix Block Portection & Deep Power Down mode
@ 2020-03-09 13:14   ` Miquel Raynal
  0 siblings, 0 replies; 46+ messages in thread
From: Miquel Raynal @ 2020-03-09 13:14 UTC (permalink / raw)
  To: Mason Yang
  Cc: vigneshr, bbrezillon, juliensu, richard, s.hauer, yuehaibing,
	linux-kernel, stefan, rfontana, linux-mtd, frieder.schrempf,
	tglx, allison

Hi Mason,

Mason Yang <masonccyang@mxic.com.tw> wrote on Tue,  3 Mar 2020 15:21:20
+0800:

> Hi,
> 
> Changelog
> 
> v3:
> patch nand_lock_area/nand_unlock_area.
> fixed kbuidtest robot warnings and reviewer's comments.

I know it is painful for the contributor but I really need more details
in the changelog. This is something I care about because I can speed-up
my reviews when I know what I already acked or not. "fixing reviewer's
comments" is way too vague, I have absolutely no idea of what I told
you last time :) So please, for the next iterations, be more verbose in
these changelogs! (that's fine for this one, I'll check myself).

Cheers,
Miquèl

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

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

* Re: [PATCH v3 0/4] mtd: rawnand: Add support Macronix Block Portection & Deep Power Down mode
  2020-03-09 13:14   ` Miquel Raynal
@ 2020-03-10  2:30     ` masonccyang
  -1 siblings, 0 replies; 46+ messages in thread
From: masonccyang @ 2020-03-10  2:30 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: allison, bbrezillon, frieder.schrempf, juliensu, linux-kernel,
	linux-mtd, rfontana, richard, s.hauer, stefan, tglx, vigneshr,
	yuehaibing


Hi Miquel,

> 
> Mason Yang <masonccyang@mxic.com.tw> wrote on Tue,  3 Mar 2020 15:21:20
> +0800:
> 
> > Hi,
> > 
> > Changelog
> > 
> > v3:
> > patch nand_lock_area/nand_unlock_area.
> > fixed kbuidtest robot warnings and reviewer's comments.
> 
> I know it is painful for the contributor but I really need more details
> in the changelog. This is something I care about because I can speed-up

okay, more changelog as

1. Patched the Kdoc for both lock_area/unlock_area and _suspend/_resume
2. Created a helper to read default protected value (after device power 
on)
        for protection function detection.
3. patched the prefix for Macronix deep power down command, 0xB9
4. Patched the description of mxic_nand_resume() and add a small sleeping 
delay.
5. Created a helper for deep power down device part number detection.


> my reviews when I know what I already acked or not. "fixing reviewer's
> comments" is way too vague, I have absolutely no idea of what I told
> you last time :) So please, for the next iterations, be more verbose in
> these changelogs! (that's fine for this one, I'll check myself).
> 
> Cheers,
> Miquèl

thanks for your time and review.
Mason


CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information 
and/or personal data, which is protected by applicable laws. Please be 
reminded that duplication, disclosure, distribution, or use of this e-mail 
(and/or its attachments) or any part thereof is prohibited. If you receive 
this e-mail in error, please notify us immediately and delete this mail as 
well as its attachment(s) from your system. In addition, please be 
informed that collection, processing, and/or use of personal data is 
prohibited unless expressly permitted by personal data protection laws. 
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================


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

* Re: [PATCH v3 0/4] mtd: rawnand: Add support Macronix Block Portection & Deep Power Down mode
@ 2020-03-10  2:30     ` masonccyang
  0 siblings, 0 replies; 46+ messages in thread
From: masonccyang @ 2020-03-10  2:30 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: vigneshr, bbrezillon, juliensu, richard, s.hauer, yuehaibing,
	linux-kernel, frieder.schrempf, rfontana, linux-mtd, stefan,
	tglx, allison


Hi Miquel,

> 
> Mason Yang <masonccyang@mxic.com.tw> wrote on Tue,  3 Mar 2020 15:21:20
> +0800:
> 
> > Hi,
> > 
> > Changelog
> > 
> > v3:
> > patch nand_lock_area/nand_unlock_area.
> > fixed kbuidtest robot warnings and reviewer's comments.
> 
> I know it is painful for the contributor but I really need more details
> in the changelog. This is something I care about because I can speed-up

okay, more changelog as

1. Patched the Kdoc for both lock_area/unlock_area and _suspend/_resume
2. Created a helper to read default protected value (after device power 
on)
        for protection function detection.
3. patched the prefix for Macronix deep power down command, 0xB9
4. Patched the description of mxic_nand_resume() and add a small sleeping 
delay.
5. Created a helper for deep power down device part number detection.


> my reviews when I know what I already acked or not. "fixing reviewer's
> comments" is way too vague, I have absolutely no idea of what I told
> you last time :) So please, for the next iterations, be more verbose in
> these changelogs! (that's fine for this one, I'll check myself).
> 
> Cheers,
> Miquèl

thanks for your time and review.
Mason


CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information 
and/or personal data, which is protected by applicable laws. Please be 
reminded that duplication, disclosure, distribution, or use of this e-mail 
(and/or its attachments) or any part thereof is prohibited. If you receive 
this e-mail in error, please notify us immediately and delete this mail as 
well as its attachment(s) from your system. In addition, please be 
informed that collection, processing, and/or use of personal data is 
prohibited unless expressly permitted by personal data protection laws. 
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================


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

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

* Re: [PATCH v3 0/4] mtd: rawnand: Add support Macronix Block Portection & Deep Power Down mode
  2020-03-10  2:30     ` masonccyang
@ 2020-03-10  7:44       ` Miquel Raynal
  -1 siblings, 0 replies; 46+ messages in thread
From: Miquel Raynal @ 2020-03-10  7:44 UTC (permalink / raw)
  To: masonccyang
  Cc: allison, bbrezillon, frieder.schrempf, juliensu, linux-kernel,
	linux-mtd, rfontana, richard, s.hauer, stefan, tglx, vigneshr,
	yuehaibing

Hi Mason,

masonccyang@mxic.com.tw wrote on Tue, 10 Mar 2020 10:30:09 +0800:

> Hi Miquel,
> 
> > 
> > Mason Yang <masonccyang@mxic.com.tw> wrote on Tue,  3 Mar 2020 15:21:20
> > +0800:
> >   
> > > Hi,
> > > 
> > > Changelog
> > > 
> > > v3:
> > > patch nand_lock_area/nand_unlock_area.
> > > fixed kbuidtest robot warnings and reviewer's comments.  
> > 
> > I know it is painful for the contributor but I really need more details
> > in the changelog. This is something I care about because I can speed-up  
> 
> okay, more changelog as
> 
> 1. Patched the Kdoc for both lock_area/unlock_area and _suspend/_resume
> 2. Created a helper to read default protected value (after device power 
> on)
>         for protection function detection.
> 3. patched the prefix for Macronix deep power down command, 0xB9
> 4. Patched the description of mxic_nand_resume() and add a small sleeping 
> delay.
> 5. Created a helper for deep power down device part number detection.
> 

Way more descriptive! Thanks a lot.


Cheers,
Miquèl

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

* Re: [PATCH v3 0/4] mtd: rawnand: Add support Macronix Block Portection & Deep Power Down mode
@ 2020-03-10  7:44       ` Miquel Raynal
  0 siblings, 0 replies; 46+ messages in thread
From: Miquel Raynal @ 2020-03-10  7:44 UTC (permalink / raw)
  To: masonccyang
  Cc: vigneshr, bbrezillon, juliensu, richard, s.hauer, yuehaibing,
	linux-kernel, frieder.schrempf, rfontana, linux-mtd, stefan,
	tglx, allison

Hi Mason,

masonccyang@mxic.com.tw wrote on Tue, 10 Mar 2020 10:30:09 +0800:

> Hi Miquel,
> 
> > 
> > Mason Yang <masonccyang@mxic.com.tw> wrote on Tue,  3 Mar 2020 15:21:20
> > +0800:
> >   
> > > Hi,
> > > 
> > > Changelog
> > > 
> > > v3:
> > > patch nand_lock_area/nand_unlock_area.
> > > fixed kbuidtest robot warnings and reviewer's comments.  
> > 
> > I know it is painful for the contributor but I really need more details
> > in the changelog. This is something I care about because I can speed-up  
> 
> okay, more changelog as
> 
> 1. Patched the Kdoc for both lock_area/unlock_area and _suspend/_resume
> 2. Created a helper to read default protected value (after device power 
> on)
>         for protection function detection.
> 3. patched the prefix for Macronix deep power down command, 0xB9
> 4. Patched the description of mxic_nand_resume() and add a small sleeping 
> delay.
> 5. Created a helper for deep power down device part number detection.
> 

Way more descriptive! Thanks a lot.


Cheers,
Miquèl

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

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

* Re: [PATCH v3 3/4] mtd: rawnand: Add support manufacturer specific suspend/resume operation
  2020-03-03  7:21   ` Mason Yang
@ 2020-03-10 18:30     ` Miquel Raynal
  -1 siblings, 0 replies; 46+ messages in thread
From: Miquel Raynal @ 2020-03-10 18:30 UTC (permalink / raw)
  To: Mason Yang, miquel.raynal, richard, vigneshr
  Cc: bbrezillon, juliensu, s.hauer, yuehaibing, linux-kernel, stefan,
	rfontana, linux-mtd, frieder.schrempf, tglx, allison

On Tue, 2020-03-03 at 07:21:23 UTC, Mason Yang wrote:
> Patch nand_suspend() & nand_resume() for manufacturer specific
> suspend/resume operation.
> 
> Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> Reported-by: kbuild test robot <lkp@intel.com>
> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.

Miquel

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

* Re: [PATCH v3 3/4] mtd: rawnand: Add support manufacturer specific suspend/resume operation
@ 2020-03-10 18:30     ` Miquel Raynal
  0 siblings, 0 replies; 46+ messages in thread
From: Miquel Raynal @ 2020-03-10 18:30 UTC (permalink / raw)
  To: Mason Yang, miquel.raynal, richard, vigneshr
  Cc: bbrezillon, juliensu, s.hauer, yuehaibing, linux-kernel, stefan,
	rfontana, linux-mtd, frieder.schrempf, tglx, allison

On Tue, 2020-03-03 at 07:21:23 UTC, Mason Yang wrote:
> Patch nand_suspend() & nand_resume() for manufacturer specific
> suspend/resume operation.
> 
> Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> Reported-by: kbuild test robot <lkp@intel.com>
> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.

Miquel

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

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

* Re: [PATCH v3 1/4] mtd: rawnand: Add support manufacturer specific lock/unlock operation
  2020-03-03  7:21   ` Mason Yang
@ 2020-03-10 18:30     ` Miquel Raynal
  -1 siblings, 0 replies; 46+ messages in thread
From: Miquel Raynal @ 2020-03-10 18:30 UTC (permalink / raw)
  To: Mason Yang, miquel.raynal, richard, vigneshr
  Cc: bbrezillon, juliensu, s.hauer, yuehaibing, linux-kernel, stefan,
	rfontana, linux-mtd, frieder.schrempf, tglx, allison

On Tue, 2020-03-03 at 07:21:21 UTC, Mason Yang wrote:
> Add nand_lock() & nand_unlock() for manufacturer specific lock & unlock
> operation while the device supports Block Portection function.
> 
> Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> Reported-by: kbuild test robot <lkp@intel.com>
> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.

Miquel

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

* Re: [PATCH v3 1/4] mtd: rawnand: Add support manufacturer specific lock/unlock operation
@ 2020-03-10 18:30     ` Miquel Raynal
  0 siblings, 0 replies; 46+ messages in thread
From: Miquel Raynal @ 2020-03-10 18:30 UTC (permalink / raw)
  To: Mason Yang, miquel.raynal, richard, vigneshr
  Cc: bbrezillon, juliensu, s.hauer, yuehaibing, linux-kernel, stefan,
	rfontana, linux-mtd, frieder.schrempf, tglx, allison

On Tue, 2020-03-03 at 07:21:21 UTC, Mason Yang wrote:
> Add nand_lock() & nand_unlock() for manufacturer specific lock & unlock
> operation while the device supports Block Portection function.
> 
> Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> Reported-by: kbuild test robot <lkp@intel.com>
> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.

Miquel

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

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

* Re: [PATCH v3 1/4] mtd: rawnand: Add support manufacturer specific lock/unlock operation
  2020-03-03  7:21   ` Mason Yang
@ 2020-03-10 19:27     ` Boris Brezillon
  -1 siblings, 0 replies; 46+ messages in thread
From: Boris Brezillon @ 2020-03-10 19:27 UTC (permalink / raw)
  To: Mason Yang
  Cc: miquel.raynal, richard, vigneshr, frieder.schrempf, tglx, stefan,
	juliensu, allison, linux-kernel, bbrezillon, rfontana, linux-mtd,
	yuehaibing, s.hauer

On Tue,  3 Mar 2020 15:21:21 +0800
Mason Yang <masonccyang@mxic.com.tw> wrote:

> Add nand_lock() & nand_unlock() for manufacturer specific lock & unlock
> operation while the device supports Block Portection function.
> 
> Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> Reported-by: kbuild test robot <lkp@intel.com>

Reported-by on something that's not a fix doesn't make sense. I know
the 0day bot asked you to add this tag, but that should only be done if
you submit a separate patch, ideally with a Fixes tag. If the offending
patch has not been merged yet, you should just fix the commit and ignore
the Reported-by tag (you can mention who reported the problem in the
changelog though).

> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/nand_base.c | 36 ++++++++++++++++++++++++++++++++++--
>  include/linux/mtd/rawnand.h      |  5 +++++
>  2 files changed, 39 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index f64e3b6..769be81 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -4360,6 +4360,38 @@ static void nand_shutdown(struct mtd_info *mtd)
>  	nand_suspend(mtd);
>  }
>  
> +/**
> + * nand_lock - [MTD Interface] Lock the NAND flash
> + * @mtd: MTD device structure
> + * @ofs: offset byte address
> + * @len: number of bytes to lock (must be a multiple of block/page size)
> + */
> +static int nand_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
> +{
> +	struct nand_chip *chip = mtd_to_nand(mtd);
> +
> +	if (!chip->lock_area)
> +		return -ENOTSUPP;
> +
> +	return chip->lock_area(chip, ofs, len);
> +}
> +
> +/**
> + * nand_unlock - [MTD Interface] Unlock the NAND flash
> + * @mtd: MTD device structure
> + * @ofs: offset byte address
> + * @len: number of bytes to unlock (must be a multiple of block/page size)
> + */
> +static int nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
> +{
> +	struct nand_chip *chip = mtd_to_nand(mtd);
> +
> +	if (!chip->unlock_area)
> +		return -ENOTSUPP;
> +
> +	return chip->unlock_area(chip, ofs, len);
> +}
> +
>  /* Set default functions */
>  static void nand_set_defaults(struct nand_chip *chip)
>  {
> @@ -5786,8 +5818,8 @@ static int nand_scan_tail(struct nand_chip *chip)
>  	mtd->_read_oob = nand_read_oob;
>  	mtd->_write_oob = nand_write_oob;
>  	mtd->_sync = nand_sync;
> -	mtd->_lock = NULL;
> -	mtd->_unlock = NULL;
> +	mtd->_lock = nand_lock;
> +	mtd->_unlock = nand_unlock;
>  	mtd->_suspend = nand_suspend;
>  	mtd->_resume = nand_resume;
>  	mtd->_reboot = nand_shutdown;
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index 4ab9bcc..bc2fa3c 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -1077,6 +1077,8 @@ struct nand_legacy {
>   * @manufacturer:	[INTERN] Contains manufacturer information
>   * @manufacturer.desc:	[INTERN] Contains manufacturer's description
>   * @manufacturer.priv:	[INTERN] Contains manufacturer private information
> + * @lock_area:		[REPLACEABLE] specific NAND chip lock operation
> + * @unlock_area:	[REPLACEABLE] specific NAND chip unlock operation
>   */
>  
>  struct nand_chip {
> @@ -1136,6 +1138,9 @@ struct nand_chip {
>  		const struct nand_manufacturer *desc;
>  		void *priv;
>  	} manufacturer;
> +
> +	int (*lock_area)(struct nand_chip *chip, loff_t ofs, uint64_t len);
> +	int (*unlock_area)(struct nand_chip *chip, loff_t ofs, uint64_t len);
>  };
>  
>  extern const struct mtd_ooblayout_ops nand_ooblayout_sp_ops;


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

* Re: [PATCH v3 1/4] mtd: rawnand: Add support manufacturer specific lock/unlock operation
@ 2020-03-10 19:27     ` Boris Brezillon
  0 siblings, 0 replies; 46+ messages in thread
From: Boris Brezillon @ 2020-03-10 19:27 UTC (permalink / raw)
  To: Mason Yang
  Cc: vigneshr, bbrezillon, juliensu, richard, s.hauer, yuehaibing,
	linux-kernel, stefan, rfontana, linux-mtd, frieder.schrempf,
	miquel.raynal, tglx, allison

On Tue,  3 Mar 2020 15:21:21 +0800
Mason Yang <masonccyang@mxic.com.tw> wrote:

> Add nand_lock() & nand_unlock() for manufacturer specific lock & unlock
> operation while the device supports Block Portection function.
> 
> Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> Reported-by: kbuild test robot <lkp@intel.com>

Reported-by on something that's not a fix doesn't make sense. I know
the 0day bot asked you to add this tag, but that should only be done if
you submit a separate patch, ideally with a Fixes tag. If the offending
patch has not been merged yet, you should just fix the commit and ignore
the Reported-by tag (you can mention who reported the problem in the
changelog though).

> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/nand_base.c | 36 ++++++++++++++++++++++++++++++++++--
>  include/linux/mtd/rawnand.h      |  5 +++++
>  2 files changed, 39 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index f64e3b6..769be81 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -4360,6 +4360,38 @@ static void nand_shutdown(struct mtd_info *mtd)
>  	nand_suspend(mtd);
>  }
>  
> +/**
> + * nand_lock - [MTD Interface] Lock the NAND flash
> + * @mtd: MTD device structure
> + * @ofs: offset byte address
> + * @len: number of bytes to lock (must be a multiple of block/page size)
> + */
> +static int nand_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
> +{
> +	struct nand_chip *chip = mtd_to_nand(mtd);
> +
> +	if (!chip->lock_area)
> +		return -ENOTSUPP;
> +
> +	return chip->lock_area(chip, ofs, len);
> +}
> +
> +/**
> + * nand_unlock - [MTD Interface] Unlock the NAND flash
> + * @mtd: MTD device structure
> + * @ofs: offset byte address
> + * @len: number of bytes to unlock (must be a multiple of block/page size)
> + */
> +static int nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
> +{
> +	struct nand_chip *chip = mtd_to_nand(mtd);
> +
> +	if (!chip->unlock_area)
> +		return -ENOTSUPP;
> +
> +	return chip->unlock_area(chip, ofs, len);
> +}
> +
>  /* Set default functions */
>  static void nand_set_defaults(struct nand_chip *chip)
>  {
> @@ -5786,8 +5818,8 @@ static int nand_scan_tail(struct nand_chip *chip)
>  	mtd->_read_oob = nand_read_oob;
>  	mtd->_write_oob = nand_write_oob;
>  	mtd->_sync = nand_sync;
> -	mtd->_lock = NULL;
> -	mtd->_unlock = NULL;
> +	mtd->_lock = nand_lock;
> +	mtd->_unlock = nand_unlock;
>  	mtd->_suspend = nand_suspend;
>  	mtd->_resume = nand_resume;
>  	mtd->_reboot = nand_shutdown;
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index 4ab9bcc..bc2fa3c 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -1077,6 +1077,8 @@ struct nand_legacy {
>   * @manufacturer:	[INTERN] Contains manufacturer information
>   * @manufacturer.desc:	[INTERN] Contains manufacturer's description
>   * @manufacturer.priv:	[INTERN] Contains manufacturer private information
> + * @lock_area:		[REPLACEABLE] specific NAND chip lock operation
> + * @unlock_area:	[REPLACEABLE] specific NAND chip unlock operation
>   */
>  
>  struct nand_chip {
> @@ -1136,6 +1138,9 @@ struct nand_chip {
>  		const struct nand_manufacturer *desc;
>  		void *priv;
>  	} manufacturer;
> +
> +	int (*lock_area)(struct nand_chip *chip, loff_t ofs, uint64_t len);
> +	int (*unlock_area)(struct nand_chip *chip, loff_t ofs, uint64_t len);
>  };
>  
>  extern const struct mtd_ooblayout_ops nand_ooblayout_sp_ops;


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

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

* Re: [PATCH v3 3/4] mtd: rawnand: Add support manufacturer specific suspend/resume operation
  2020-03-03  7:21   ` Mason Yang
@ 2020-03-10 19:33     ` Boris Brezillon
  -1 siblings, 0 replies; 46+ messages in thread
From: Boris Brezillon @ 2020-03-10 19:33 UTC (permalink / raw)
  To: Mason Yang
  Cc: miquel.raynal, richard, vigneshr, frieder.schrempf, tglx, stefan,
	juliensu, allison, linux-kernel, bbrezillon, rfontana, linux-mtd,
	yuehaibing, s.hauer

On Tue,  3 Mar 2020 15:21:23 +0800
Mason Yang <masonccyang@mxic.com.tw> wrote:

> Patch nand_suspend() & nand_resume() for manufacturer specific
> suspend/resume operation.
> 
> Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> Reported-by: kbuild test robot <lkp@intel.com>
> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/nand_base.c | 11 ++++++++---
>  include/linux/mtd/rawnand.h      |  4 ++++
>  2 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 769be81..b44e460 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -4327,7 +4327,9 @@ static int nand_suspend(struct mtd_info *mtd)
>  	struct nand_chip *chip = mtd_to_nand(mtd);
>  
>  	mutex_lock(&chip->lock);
> -	chip->suspended = 1;
> +	if (chip->_suspend)
> +		if (!chip->_suspend(chip))
> +			chip->suspended = 1;
>  	mutex_unlock(&chip->lock);
>  
>  	return 0;
> @@ -4342,11 +4344,14 @@ static void nand_resume(struct mtd_info *mtd)
>  	struct nand_chip *chip = mtd_to_nand(mtd);
>  
>  	mutex_lock(&chip->lock);
> -	if (chip->suspended)
> +	if (chip->suspended) {
> +		if (chip->_resume)
> +			chip->_resume(chip);
>  		chip->suspended = 0;
> -	else
> +	} else {
>  		pr_err("%s called for a chip which is not in suspended state\n",
>  			__func__);
> +	}
>  	mutex_unlock(&chip->lock);
>  }
>  
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index bc2fa3c..c0055ed 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -1064,6 +1064,8 @@ struct nand_legacy {
>   * @lock:		lock protecting the suspended field. Also used to
>   *			serialize accesses to the NAND device.
>   * @suspended:		set to 1 when the device is suspended, 0 when it's not.
> + * @_suspend:		[REPLACEABLE] specific NAND device suspend operation
> + * @_resume:		[REPLACEABLE] specific NAND device resume operation
>   * @bbt:		[INTERN] bad block table pointer
>   * @bbt_td:		[REPLACEABLE] bad block table descriptor for flash
>   *			lookup.
> @@ -1119,6 +1121,8 @@ struct nand_chip {
>  
>  	struct mutex lock;
>  	unsigned int suspended : 1;
> +	int (*_suspend)(struct nand_chip *chip);
> +	void (*_resume)(struct nand_chip *chip);

I thought we agreed on not prefixing new hooks with _ ?

>  
>  	uint8_t *oob_poi;
>  	struct nand_controller *controller;


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

* Re: [PATCH v3 3/4] mtd: rawnand: Add support manufacturer specific suspend/resume operation
@ 2020-03-10 19:33     ` Boris Brezillon
  0 siblings, 0 replies; 46+ messages in thread
From: Boris Brezillon @ 2020-03-10 19:33 UTC (permalink / raw)
  To: Mason Yang
  Cc: vigneshr, bbrezillon, juliensu, richard, s.hauer, yuehaibing,
	linux-kernel, stefan, rfontana, linux-mtd, frieder.schrempf,
	miquel.raynal, tglx, allison

On Tue,  3 Mar 2020 15:21:23 +0800
Mason Yang <masonccyang@mxic.com.tw> wrote:

> Patch nand_suspend() & nand_resume() for manufacturer specific
> suspend/resume operation.
> 
> Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> Reported-by: kbuild test robot <lkp@intel.com>
> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/nand_base.c | 11 ++++++++---
>  include/linux/mtd/rawnand.h      |  4 ++++
>  2 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 769be81..b44e460 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -4327,7 +4327,9 @@ static int nand_suspend(struct mtd_info *mtd)
>  	struct nand_chip *chip = mtd_to_nand(mtd);
>  
>  	mutex_lock(&chip->lock);
> -	chip->suspended = 1;
> +	if (chip->_suspend)
> +		if (!chip->_suspend(chip))
> +			chip->suspended = 1;
>  	mutex_unlock(&chip->lock);
>  
>  	return 0;
> @@ -4342,11 +4344,14 @@ static void nand_resume(struct mtd_info *mtd)
>  	struct nand_chip *chip = mtd_to_nand(mtd);
>  
>  	mutex_lock(&chip->lock);
> -	if (chip->suspended)
> +	if (chip->suspended) {
> +		if (chip->_resume)
> +			chip->_resume(chip);
>  		chip->suspended = 0;
> -	else
> +	} else {
>  		pr_err("%s called for a chip which is not in suspended state\n",
>  			__func__);
> +	}
>  	mutex_unlock(&chip->lock);
>  }
>  
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index bc2fa3c..c0055ed 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -1064,6 +1064,8 @@ struct nand_legacy {
>   * @lock:		lock protecting the suspended field. Also used to
>   *			serialize accesses to the NAND device.
>   * @suspended:		set to 1 when the device is suspended, 0 when it's not.
> + * @_suspend:		[REPLACEABLE] specific NAND device suspend operation
> + * @_resume:		[REPLACEABLE] specific NAND device resume operation
>   * @bbt:		[INTERN] bad block table pointer
>   * @bbt_td:		[REPLACEABLE] bad block table descriptor for flash
>   *			lookup.
> @@ -1119,6 +1121,8 @@ struct nand_chip {
>  
>  	struct mutex lock;
>  	unsigned int suspended : 1;
> +	int (*_suspend)(struct nand_chip *chip);
> +	void (*_resume)(struct nand_chip *chip);

I thought we agreed on not prefixing new hooks with _ ?

>  
>  	uint8_t *oob_poi;
>  	struct nand_controller *controller;


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

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

* Re: [PATCH v3 3/4] mtd: rawnand: Add support manufacturer specific suspend/resume operation
  2020-03-03  7:21   ` Mason Yang
@ 2020-03-10 19:39     ` Boris Brezillon
  -1 siblings, 0 replies; 46+ messages in thread
From: Boris Brezillon @ 2020-03-10 19:39 UTC (permalink / raw)
  To: Mason Yang
  Cc: miquel.raynal, richard, vigneshr, frieder.schrempf, tglx, stefan,
	juliensu, allison, linux-kernel, bbrezillon, rfontana, linux-mtd,
	yuehaibing, s.hauer

On Tue,  3 Mar 2020 15:21:23 +0800
Mason Yang <masonccyang@mxic.com.tw> wrote:

> Patch nand_suspend() & nand_resume() for manufacturer specific
> suspend/resume operation.
> 
> Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> Reported-by: kbuild test robot <lkp@intel.com>
> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/nand_base.c | 11 ++++++++---
>  include/linux/mtd/rawnand.h      |  4 ++++
>  2 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 769be81..b44e460 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -4327,7 +4327,9 @@ static int nand_suspend(struct mtd_info *mtd)
>  	struct nand_chip *chip = mtd_to_nand(mtd);
>  
>  	mutex_lock(&chip->lock);
> -	chip->suspended = 1;
> +	if (chip->_suspend)
> +		if (!chip->_suspend(chip))
> +			chip->suspended = 1;
>  	mutex_unlock(&chip->lock);
>  
>  	return 0;
> @@ -4342,11 +4344,14 @@ static void nand_resume(struct mtd_info *mtd)
>  	struct nand_chip *chip = mtd_to_nand(mtd);
>  
>  	mutex_lock(&chip->lock);
> -	if (chip->suspended)
> +	if (chip->suspended) {
> +		if (chip->_resume)
> +			chip->_resume(chip);
>  		chip->suspended = 0;
> -	else
> +	} else {
>  		pr_err("%s called for a chip which is not in suspended state\n",
>  			__func__);
> +	}
>  	mutex_unlock(&chip->lock);
>  }
>  
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index bc2fa3c..c0055ed 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -1064,6 +1064,8 @@ struct nand_legacy {
>   * @lock:		lock protecting the suspended field. Also used to
>   *			serialize accesses to the NAND device.
>   * @suspended:		set to 1 when the device is suspended, 0 when it's not.
> + * @_suspend:		[REPLACEABLE] specific NAND device suspend operation
> + * @_resume:		[REPLACEABLE] specific NAND device resume operation

Given you added 4 more methods in this series, I think now would be a
good time to introduce a nand_chip_ops struct grouping all ops together.

>   * @bbt:		[INTERN] bad block table pointer
>   * @bbt_td:		[REPLACEABLE] bad block table descriptor for flash
>   *			lookup.
> @@ -1119,6 +1121,8 @@ struct nand_chip {
>  
>  	struct mutex lock;
>  	unsigned int suspended : 1;
> +	int (*_suspend)(struct nand_chip *chip);
> +	void (*_resume)(struct nand_chip *chip);
>  
>  	uint8_t *oob_poi;
>  	struct nand_controller *controller;


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

* Re: [PATCH v3 3/4] mtd: rawnand: Add support manufacturer specific suspend/resume operation
@ 2020-03-10 19:39     ` Boris Brezillon
  0 siblings, 0 replies; 46+ messages in thread
From: Boris Brezillon @ 2020-03-10 19:39 UTC (permalink / raw)
  To: Mason Yang
  Cc: vigneshr, bbrezillon, juliensu, richard, s.hauer, yuehaibing,
	linux-kernel, stefan, rfontana, linux-mtd, frieder.schrempf,
	miquel.raynal, tglx, allison

On Tue,  3 Mar 2020 15:21:23 +0800
Mason Yang <masonccyang@mxic.com.tw> wrote:

> Patch nand_suspend() & nand_resume() for manufacturer specific
> suspend/resume operation.
> 
> Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> Reported-by: kbuild test robot <lkp@intel.com>
> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/nand_base.c | 11 ++++++++---
>  include/linux/mtd/rawnand.h      |  4 ++++
>  2 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 769be81..b44e460 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -4327,7 +4327,9 @@ static int nand_suspend(struct mtd_info *mtd)
>  	struct nand_chip *chip = mtd_to_nand(mtd);
>  
>  	mutex_lock(&chip->lock);
> -	chip->suspended = 1;
> +	if (chip->_suspend)
> +		if (!chip->_suspend(chip))
> +			chip->suspended = 1;
>  	mutex_unlock(&chip->lock);
>  
>  	return 0;
> @@ -4342,11 +4344,14 @@ static void nand_resume(struct mtd_info *mtd)
>  	struct nand_chip *chip = mtd_to_nand(mtd);
>  
>  	mutex_lock(&chip->lock);
> -	if (chip->suspended)
> +	if (chip->suspended) {
> +		if (chip->_resume)
> +			chip->_resume(chip);
>  		chip->suspended = 0;
> -	else
> +	} else {
>  		pr_err("%s called for a chip which is not in suspended state\n",
>  			__func__);
> +	}
>  	mutex_unlock(&chip->lock);
>  }
>  
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index bc2fa3c..c0055ed 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -1064,6 +1064,8 @@ struct nand_legacy {
>   * @lock:		lock protecting the suspended field. Also used to
>   *			serialize accesses to the NAND device.
>   * @suspended:		set to 1 when the device is suspended, 0 when it's not.
> + * @_suspend:		[REPLACEABLE] specific NAND device suspend operation
> + * @_resume:		[REPLACEABLE] specific NAND device resume operation

Given you added 4 more methods in this series, I think now would be a
good time to introduce a nand_chip_ops struct grouping all ops together.

>   * @bbt:		[INTERN] bad block table pointer
>   * @bbt_td:		[REPLACEABLE] bad block table descriptor for flash
>   *			lookup.
> @@ -1119,6 +1121,8 @@ struct nand_chip {
>  
>  	struct mutex lock;
>  	unsigned int suspended : 1;
> +	int (*_suspend)(struct nand_chip *chip);
> +	void (*_resume)(struct nand_chip *chip);
>  
>  	uint8_t *oob_poi;
>  	struct nand_controller *controller;


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

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

* Re: [PATCH v3 3/4] mtd: rawnand: Add support manufacturer specific suspend/resume operation
  2020-03-10 19:39     ` Boris Brezillon
@ 2020-03-10 19:41       ` Boris Brezillon
  -1 siblings, 0 replies; 46+ messages in thread
From: Boris Brezillon @ 2020-03-10 19:41 UTC (permalink / raw)
  To: Mason Yang
  Cc: miquel.raynal, richard, vigneshr, frieder.schrempf, tglx, stefan,
	juliensu, allison, linux-kernel, bbrezillon, rfontana, linux-mtd,
	yuehaibing, s.hauer

On Tue, 10 Mar 2020 20:39:30 +0100
Boris Brezillon <boris.brezillon@collabora.com> wrote:

> On Tue,  3 Mar 2020 15:21:23 +0800
> Mason Yang <masonccyang@mxic.com.tw> wrote:
> 
> > Patch nand_suspend() & nand_resume() for manufacturer specific
> > suspend/resume operation.
> > 
> > Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> > Reported-by: kbuild test robot <lkp@intel.com>
> > Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  drivers/mtd/nand/raw/nand_base.c | 11 ++++++++---
> >  include/linux/mtd/rawnand.h      |  4 ++++
> >  2 files changed, 12 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> > index 769be81..b44e460 100644
> > --- a/drivers/mtd/nand/raw/nand_base.c
> > +++ b/drivers/mtd/nand/raw/nand_base.c
> > @@ -4327,7 +4327,9 @@ static int nand_suspend(struct mtd_info *mtd)
> >  	struct nand_chip *chip = mtd_to_nand(mtd);
> >  
> >  	mutex_lock(&chip->lock);
> > -	chip->suspended = 1;
> > +	if (chip->_suspend)
> > +		if (!chip->_suspend(chip))
> > +			chip->suspended = 1;

Shouldn't you propagate the error to the caller if chip->_suspend()
fails?

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

* Re: [PATCH v3 3/4] mtd: rawnand: Add support manufacturer specific suspend/resume operation
@ 2020-03-10 19:41       ` Boris Brezillon
  0 siblings, 0 replies; 46+ messages in thread
From: Boris Brezillon @ 2020-03-10 19:41 UTC (permalink / raw)
  To: Mason Yang
  Cc: vigneshr, bbrezillon, juliensu, richard, s.hauer, yuehaibing,
	linux-kernel, stefan, rfontana, linux-mtd, frieder.schrempf,
	miquel.raynal, tglx, allison

On Tue, 10 Mar 2020 20:39:30 +0100
Boris Brezillon <boris.brezillon@collabora.com> wrote:

> On Tue,  3 Mar 2020 15:21:23 +0800
> Mason Yang <masonccyang@mxic.com.tw> wrote:
> 
> > Patch nand_suspend() & nand_resume() for manufacturer specific
> > suspend/resume operation.
> > 
> > Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> > Reported-by: kbuild test robot <lkp@intel.com>
> > Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  drivers/mtd/nand/raw/nand_base.c | 11 ++++++++---
> >  include/linux/mtd/rawnand.h      |  4 ++++
> >  2 files changed, 12 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> > index 769be81..b44e460 100644
> > --- a/drivers/mtd/nand/raw/nand_base.c
> > +++ b/drivers/mtd/nand/raw/nand_base.c
> > @@ -4327,7 +4327,9 @@ static int nand_suspend(struct mtd_info *mtd)
> >  	struct nand_chip *chip = mtd_to_nand(mtd);
> >  
> >  	mutex_lock(&chip->lock);
> > -	chip->suspended = 1;
> > +	if (chip->_suspend)
> > +		if (!chip->_suspend(chip))
> > +			chip->suspended = 1;

Shouldn't you propagate the error to the caller if chip->_suspend()
fails?

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

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

* Re: [PATCH v3 1/4] mtd: rawnand: Add support manufacturer specific lock/unlock operation
  2020-03-10 19:27     ` Boris Brezillon
@ 2020-03-11  2:40       ` masonccyang
  -1 siblings, 0 replies; 46+ messages in thread
From: masonccyang @ 2020-03-11  2:40 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: allison, bbrezillon, frieder.schrempf, juliensu, linux-kernel,
	linux-mtd, miquel.raynal, rfontana, richard, s.hauer, stefan,
	tglx, vigneshr, yuehaibing


Hi Boris,

> > Add nand_lock() & nand_unlock() for manufacturer specific lock & 
unlock
> > operation while the device supports Block Portection function.
> > 
> > Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> > Reported-by: kbuild test robot <lkp@intel.com>
> 
> Reported-by on something that's not a fix doesn't make sense. I know
> the 0day bot asked you to add this tag, but that should only be done if
> you submit a separate patch, ideally with a Fixes tag. If the offending
> patch has not been merged yet, you should just fix the commit and ignore
> the Reported-by tag (you can mention who reported the problem in the
> changelog though).
> 

okay, understand it.
Thanks a lot for your explanation.

Mason

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information 
and/or personal data, which is protected by applicable laws. Please be 
reminded that duplication, disclosure, distribution, or use of this e-mail 
(and/or its attachments) or any part thereof is prohibited. If you receive 
this e-mail in error, please notify us immediately and delete this mail as 
well as its attachment(s) from your system. In addition, please be 
informed that collection, processing, and/or use of personal data is 
prohibited unless expressly permitted by personal data protection laws. 
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================


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

* Re: [PATCH v3 1/4] mtd: rawnand: Add support manufacturer specific lock/unlock operation
@ 2020-03-11  2:40       ` masonccyang
  0 siblings, 0 replies; 46+ messages in thread
From: masonccyang @ 2020-03-11  2:40 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: vigneshr, bbrezillon, juliensu, richard, s.hauer, yuehaibing,
	linux-kernel, frieder.schrempf, rfontana, linux-mtd, stefan,
	miquel.raynal, tglx, allison


Hi Boris,

> > Add nand_lock() & nand_unlock() for manufacturer specific lock & 
unlock
> > operation while the device supports Block Portection function.
> > 
> > Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> > Reported-by: kbuild test robot <lkp@intel.com>
> 
> Reported-by on something that's not a fix doesn't make sense. I know
> the 0day bot asked you to add this tag, but that should only be done if
> you submit a separate patch, ideally with a Fixes tag. If the offending
> patch has not been merged yet, you should just fix the commit and ignore
> the Reported-by tag (you can mention who reported the problem in the
> changelog though).
> 

okay, understand it.
Thanks a lot for your explanation.

Mason

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information 
and/or personal data, which is protected by applicable laws. Please be 
reminded that duplication, disclosure, distribution, or use of this e-mail 
(and/or its attachments) or any part thereof is prohibited. If you receive 
this e-mail in error, please notify us immediately and delete this mail as 
well as its attachment(s) from your system. In addition, please be 
informed that collection, processing, and/or use of personal data is 
prohibited unless expressly permitted by personal data protection laws. 
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================


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

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

* Re: [PATCH v3 3/4] mtd: rawnand: Add support manufacturer specific suspend/resume operation
  2020-03-10 19:33     ` Boris Brezillon
@ 2020-03-11  5:40       ` masonccyang
  -1 siblings, 0 replies; 46+ messages in thread
From: masonccyang @ 2020-03-11  5:40 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: allison, bbrezillon, frieder.schrempf, juliensu, linux-kernel,
	linux-mtd, miquel.raynal, rfontana, richard, s.hauer, stefan,
	tglx, vigneshr, yuehaibing


Hi Boris,

> > diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> > index bc2fa3c..c0055ed 100644
> > --- a/include/linux/mtd/rawnand.h
> > +++ b/include/linux/mtd/rawnand.h
> > @@ -1064,6 +1064,8 @@ struct nand_legacy {
> >   * @lock:      lock protecting the suspended field. Also used to
> >   *         serialize accesses to the NAND device.
> >   * @suspended:      set to 1 when the device is suspended, 0 when 
it's not.
> > + * @_suspend:      [REPLACEABLE] specific NAND device suspend 
operation
> > + * @_resume:      [REPLACEABLE] specific NAND device resume operation
> >   * @bbt:      [INTERN] bad block table pointer
> >   * @bbt_td:      [REPLACEABLE] bad block table descriptor for flash
> >   *         lookup.
> > @@ -1119,6 +1121,8 @@ struct nand_chip {
> > 
> >     struct mutex lock;
> >     unsigned int suspended : 1;
> > +   int (*_suspend)(struct nand_chip *chip);
> > +   void (*_resume)(struct nand_chip *chip);
> 
> I thought we agreed on not prefixing new hooks with _ ?

For [PATCH v2] series, you mentioned to drop the _ prefix 
of _lock/_unlock only and we finally patched to lock_area/unlock_area.

thanks for your time & review.
Mason

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information 
and/or personal data, which is protected by applicable laws. Please be 
reminded that duplication, disclosure, distribution, or use of this e-mail 
(and/or its attachments) or any part thereof is prohibited. If you receive 
this e-mail in error, please notify us immediately and delete this mail as 
well as its attachment(s) from your system. In addition, please be 
informed that collection, processing, and/or use of personal data is 
prohibited unless expressly permitted by personal data protection laws. 
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================


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

* Re: [PATCH v3 3/4] mtd: rawnand: Add support manufacturer specific suspend/resume operation
@ 2020-03-11  5:40       ` masonccyang
  0 siblings, 0 replies; 46+ messages in thread
From: masonccyang @ 2020-03-11  5:40 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: vigneshr, bbrezillon, juliensu, richard, s.hauer, yuehaibing,
	linux-kernel, frieder.schrempf, rfontana, linux-mtd, stefan,
	miquel.raynal, tglx, allison


Hi Boris,

> > diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> > index bc2fa3c..c0055ed 100644
> > --- a/include/linux/mtd/rawnand.h
> > +++ b/include/linux/mtd/rawnand.h
> > @@ -1064,6 +1064,8 @@ struct nand_legacy {
> >   * @lock:      lock protecting the suspended field. Also used to
> >   *         serialize accesses to the NAND device.
> >   * @suspended:      set to 1 when the device is suspended, 0 when 
it's not.
> > + * @_suspend:      [REPLACEABLE] specific NAND device suspend 
operation
> > + * @_resume:      [REPLACEABLE] specific NAND device resume operation
> >   * @bbt:      [INTERN] bad block table pointer
> >   * @bbt_td:      [REPLACEABLE] bad block table descriptor for flash
> >   *         lookup.
> > @@ -1119,6 +1121,8 @@ struct nand_chip {
> > 
> >     struct mutex lock;
> >     unsigned int suspended : 1;
> > +   int (*_suspend)(struct nand_chip *chip);
> > +   void (*_resume)(struct nand_chip *chip);
> 
> I thought we agreed on not prefixing new hooks with _ ?

For [PATCH v2] series, you mentioned to drop the _ prefix 
of _lock/_unlock only and we finally patched to lock_area/unlock_area.

thanks for your time & review.
Mason

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information 
and/or personal data, which is protected by applicable laws. Please be 
reminded that duplication, disclosure, distribution, or use of this e-mail 
(and/or its attachments) or any part thereof is prohibited. If you receive 
this e-mail in error, please notify us immediately and delete this mail as 
well as its attachment(s) from your system. In addition, please be 
informed that collection, processing, and/or use of personal data is 
prohibited unless expressly permitted by personal data protection laws. 
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================


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

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

* Re: [PATCH v3 3/4] mtd: rawnand: Add support manufacturer specific suspend/resume operation
  2020-03-10 19:41       ` Boris Brezillon
@ 2020-03-11  6:13         ` masonccyang
  -1 siblings, 0 replies; 46+ messages in thread
From: masonccyang @ 2020-03-11  6:13 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: allison, bbrezillon, frieder.schrempf, juliensu, linux-kernel,
	linux-mtd, miquel.raynal, rfontana, richard, s.hauer, stefan,
	tglx, vigneshr, yuehaibing


Hi Boris,

> > > ---
> > >  drivers/mtd/nand/raw/nand_base.c | 11 ++++++++---
> > >  include/linux/mtd/rawnand.h      |  4 ++++
> > >  2 files changed, 12 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/mtd/nand/raw/nand_base.c 
b/drivers/mtd/nand/raw/nand_base.c
> > > index 769be81..b44e460 100644
> > > --- a/drivers/mtd/nand/raw/nand_base.c
> > > +++ b/drivers/mtd/nand/raw/nand_base.c
> > > @@ -4327,7 +4327,9 @@ static int nand_suspend(struct mtd_info *mtd)
> > >     struct nand_chip *chip = mtd_to_nand(mtd);
> > > 
> > >     mutex_lock(&chip->lock);
> > > -   chip->suspended = 1;
> > > +   if (chip->_suspend)
> > > +      if (!chip->_suspend(chip))
> > > +         chip->suspended = 1;
> 
> Shouldn't you propagate the error to the caller if chip->_suspend()
> fails?

Currently, chip->suspend() just do sending command to nand chip and
I think caller could check chip->suspend = 1 or 0 to know the status
of nand chip.

thanks & best regards,
Mason

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information 
and/or personal data, which is protected by applicable laws. Please be 
reminded that duplication, disclosure, distribution, or use of this e-mail 
(and/or its attachments) or any part thereof is prohibited. If you receive 
this e-mail in error, please notify us immediately and delete this mail as 
well as its attachment(s) from your system. In addition, please be 
informed that collection, processing, and/or use of personal data is 
prohibited unless expressly permitted by personal data protection laws. 
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================


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

* Re: [PATCH v3 3/4] mtd: rawnand: Add support manufacturer specific suspend/resume operation
@ 2020-03-11  6:13         ` masonccyang
  0 siblings, 0 replies; 46+ messages in thread
From: masonccyang @ 2020-03-11  6:13 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: vigneshr, bbrezillon, juliensu, richard, s.hauer, yuehaibing,
	linux-kernel, frieder.schrempf, rfontana, linux-mtd, stefan,
	miquel.raynal, tglx, allison


Hi Boris,

> > > ---
> > >  drivers/mtd/nand/raw/nand_base.c | 11 ++++++++---
> > >  include/linux/mtd/rawnand.h      |  4 ++++
> > >  2 files changed, 12 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/mtd/nand/raw/nand_base.c 
b/drivers/mtd/nand/raw/nand_base.c
> > > index 769be81..b44e460 100644
> > > --- a/drivers/mtd/nand/raw/nand_base.c
> > > +++ b/drivers/mtd/nand/raw/nand_base.c
> > > @@ -4327,7 +4327,9 @@ static int nand_suspend(struct mtd_info *mtd)
> > >     struct nand_chip *chip = mtd_to_nand(mtd);
> > > 
> > >     mutex_lock(&chip->lock);
> > > -   chip->suspended = 1;
> > > +   if (chip->_suspend)
> > > +      if (!chip->_suspend(chip))
> > > +         chip->suspended = 1;
> 
> Shouldn't you propagate the error to the caller if chip->_suspend()
> fails?

Currently, chip->suspend() just do sending command to nand chip and
I think caller could check chip->suspend = 1 or 0 to know the status
of nand chip.

thanks & best regards,
Mason

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information 
and/or personal data, which is protected by applicable laws. Please be 
reminded that duplication, disclosure, distribution, or use of this e-mail 
(and/or its attachments) or any part thereof is prohibited. If you receive 
this e-mail in error, please notify us immediately and delete this mail as 
well as its attachment(s) from your system. In addition, please be 
informed that collection, processing, and/or use of personal data is 
prohibited unless expressly permitted by personal data protection laws. 
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================


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

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

* Re: [PATCH v3 1/4] mtd: rawnand: Add support manufacturer specific lock/unlock operation
  2020-03-11  2:40       ` masonccyang
@ 2020-03-11  7:25         ` Miquel Raynal
  -1 siblings, 0 replies; 46+ messages in thread
From: Miquel Raynal @ 2020-03-11  7:25 UTC (permalink / raw)
  To: masonccyang
  Cc: Boris Brezillon, allison, bbrezillon, frieder.schrempf, juliensu,
	linux-kernel, linux-mtd, rfontana, richard, s.hauer, stefan,
	tglx, vigneshr, yuehaibing

Hi Mason, Boris,

masonccyang@mxic.com.tw wrote on Wed, 11 Mar 2020 10:40:04 +0800:

> Hi Boris,
> 
> > > Add nand_lock() & nand_unlock() for manufacturer specific lock &   
> unlock
> > > operation while the device supports Block Portection function.
> > > 
> > > Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> > > Reported-by: kbuild test robot <lkp@intel.com>  
> > 
> > Reported-by on something that's not a fix doesn't make sense. I know
> > the 0day bot asked you to add this tag, but that should only be done if
> > you submit a separate patch, ideally with a Fixes tag. If the offending
> > patch has not been merged yet, you should just fix the commit and ignore
> > the Reported-by tag (you can mention who reported the problem in the
> > changelog though).

Yesterday when applying all the NAND patches my personal IP address got
flagged as spam by mistake (~500 mails in ~10s) so I could not answer
manually as I wished.

Indeed, this Reported-by tag was not needed and I dropped it manually
when applying. This tag is meant to show an error that was existing
*before* your series and that you are fixing with your series. This is
not something you should add when a robot tells you something is wrong
in your series.

Also, I rewrote several paragraphs and I prefixed two of them with "mtd:
rawnand: macronix:".

Thanks,
Miquèl

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

* Re: [PATCH v3 1/4] mtd: rawnand: Add support manufacturer specific lock/unlock operation
@ 2020-03-11  7:25         ` Miquel Raynal
  0 siblings, 0 replies; 46+ messages in thread
From: Miquel Raynal @ 2020-03-11  7:25 UTC (permalink / raw)
  To: masonccyang
  Cc: vigneshr, bbrezillon, juliensu, richard, s.hauer, yuehaibing,
	linux-kernel, frieder.schrempf, rfontana, Boris Brezillon,
	linux-mtd, stefan, tglx, allison

Hi Mason, Boris,

masonccyang@mxic.com.tw wrote on Wed, 11 Mar 2020 10:40:04 +0800:

> Hi Boris,
> 
> > > Add nand_lock() & nand_unlock() for manufacturer specific lock &   
> unlock
> > > operation while the device supports Block Portection function.
> > > 
> > > Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> > > Reported-by: kbuild test robot <lkp@intel.com>  
> > 
> > Reported-by on something that's not a fix doesn't make sense. I know
> > the 0day bot asked you to add this tag, but that should only be done if
> > you submit a separate patch, ideally with a Fixes tag. If the offending
> > patch has not been merged yet, you should just fix the commit and ignore
> > the Reported-by tag (you can mention who reported the problem in the
> > changelog though).

Yesterday when applying all the NAND patches my personal IP address got
flagged as spam by mistake (~500 mails in ~10s) so I could not answer
manually as I wished.

Indeed, this Reported-by tag was not needed and I dropped it manually
when applying. This tag is meant to show an error that was existing
*before* your series and that you are fixing with your series. This is
not something you should add when a robot tells you something is wrong
in your series.

Also, I rewrote several paragraphs and I prefixed two of them with "mtd:
rawnand: macronix:".

Thanks,
Miquèl

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

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

* Re: [PATCH v3 3/4] mtd: rawnand: Add support manufacturer specific suspend/resume operation
  2020-03-11  5:40       ` masonccyang
@ 2020-03-11  7:43         ` Miquel Raynal
  -1 siblings, 0 replies; 46+ messages in thread
From: Miquel Raynal @ 2020-03-11  7:43 UTC (permalink / raw)
  To: masonccyang
  Cc: Boris Brezillon, allison, bbrezillon, frieder.schrempf, juliensu,
	linux-kernel, linux-mtd, rfontana, richard, s.hauer, stefan,
	tglx, vigneshr, yuehaibing

Hi Mason,

masonccyang@mxic.com.tw wrote on Wed, 11 Mar 2020 13:40:52 +0800:

> Hi Boris,
> 
> > > diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> > > index bc2fa3c..c0055ed 100644
> > > --- a/include/linux/mtd/rawnand.h
> > > +++ b/include/linux/mtd/rawnand.h
> > > @@ -1064,6 +1064,8 @@ struct nand_legacy {
> > >   * @lock:      lock protecting the suspended field. Also used to
> > >   *         serialize accesses to the NAND device.
> > >   * @suspended:      set to 1 when the device is suspended, 0 when   
> it's not.
> > > + * @_suspend:      [REPLACEABLE] specific NAND device suspend   
> operation
> > > + * @_resume:      [REPLACEABLE] specific NAND device resume operation
> > >   * @bbt:      [INTERN] bad block table pointer
> > >   * @bbt_td:      [REPLACEABLE] bad block table descriptor for flash
> > >   *         lookup.
> > > @@ -1119,6 +1121,8 @@ struct nand_chip {
> > > 
> > >     struct mutex lock;
> > >     unsigned int suspended : 1;
> > > +   int (*_suspend)(struct nand_chip *chip);
> > > +   void (*_resume)(struct nand_chip *chip);  
> > 
> > I thought we agreed on not prefixing new hooks with _ ?  
> 
> For [PATCH v2] series, you mentioned to drop the _ prefix 
> of _lock/_unlock only and we finally patched to lock_area/unlock_area.
> 

I missed this _, this is not something we want to add.

Also, when applying your patches I had several issues because they
where not base on the last -rc1.

Finally, I think I forgot a line when patching manually so it produces
a warning now.

I am dropping patch 3 and 4, I keep patch 1 and 2 which seem fine.

Please send a rebased and edited v4 for these, don't forget to drop the
kbuildtest robot tag and please also follow these slightly edited
commit logs:

2/4

    mtd: rawnand: Add support for manufacturer specific suspend/resume operation
    
    Patch nand_suspend() & nand_resume() to let manufacturers overwrite
    suspend/resume operations.

3/4

    mtd: rawnand: macronix: Add support for deep power down mode
    
    Macronix AD series support deep power down mode for a minimum
    power consumption state.
    
    Overlaod nand_suspend() & nand_resume() in Macronix specific code to
    support deep power down mode.

Thanks,
Miquèl

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

* Re: [PATCH v3 3/4] mtd: rawnand: Add support manufacturer specific suspend/resume operation
@ 2020-03-11  7:43         ` Miquel Raynal
  0 siblings, 0 replies; 46+ messages in thread
From: Miquel Raynal @ 2020-03-11  7:43 UTC (permalink / raw)
  To: masonccyang
  Cc: vigneshr, bbrezillon, juliensu, richard, s.hauer, yuehaibing,
	linux-kernel, frieder.schrempf, rfontana, Boris Brezillon,
	linux-mtd, stefan, tglx, allison

Hi Mason,

masonccyang@mxic.com.tw wrote on Wed, 11 Mar 2020 13:40:52 +0800:

> Hi Boris,
> 
> > > diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> > > index bc2fa3c..c0055ed 100644
> > > --- a/include/linux/mtd/rawnand.h
> > > +++ b/include/linux/mtd/rawnand.h
> > > @@ -1064,6 +1064,8 @@ struct nand_legacy {
> > >   * @lock:      lock protecting the suspended field. Also used to
> > >   *         serialize accesses to the NAND device.
> > >   * @suspended:      set to 1 when the device is suspended, 0 when   
> it's not.
> > > + * @_suspend:      [REPLACEABLE] specific NAND device suspend   
> operation
> > > + * @_resume:      [REPLACEABLE] specific NAND device resume operation
> > >   * @bbt:      [INTERN] bad block table pointer
> > >   * @bbt_td:      [REPLACEABLE] bad block table descriptor for flash
> > >   *         lookup.
> > > @@ -1119,6 +1121,8 @@ struct nand_chip {
> > > 
> > >     struct mutex lock;
> > >     unsigned int suspended : 1;
> > > +   int (*_suspend)(struct nand_chip *chip);
> > > +   void (*_resume)(struct nand_chip *chip);  
> > 
> > I thought we agreed on not prefixing new hooks with _ ?  
> 
> For [PATCH v2] series, you mentioned to drop the _ prefix 
> of _lock/_unlock only and we finally patched to lock_area/unlock_area.
> 

I missed this _, this is not something we want to add.

Also, when applying your patches I had several issues because they
where not base on the last -rc1.

Finally, I think I forgot a line when patching manually so it produces
a warning now.

I am dropping patch 3 and 4, I keep patch 1 and 2 which seem fine.

Please send a rebased and edited v4 for these, don't forget to drop the
kbuildtest robot tag and please also follow these slightly edited
commit logs:

2/4

    mtd: rawnand: Add support for manufacturer specific suspend/resume operation
    
    Patch nand_suspend() & nand_resume() to let manufacturers overwrite
    suspend/resume operations.

3/4

    mtd: rawnand: macronix: Add support for deep power down mode
    
    Macronix AD series support deep power down mode for a minimum
    power consumption state.
    
    Overlaod nand_suspend() & nand_resume() in Macronix specific code to
    support deep power down mode.

Thanks,
Miquèl

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

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

* Re: [PATCH v3 3/4] mtd: rawnand: Add support manufacturer specific suspend/resume operation
  2020-03-11  7:43         ` Miquel Raynal
@ 2020-03-11  7:56           ` Boris Brezillon
  -1 siblings, 0 replies; 46+ messages in thread
From: Boris Brezillon @ 2020-03-11  7:56 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: masonccyang, allison, bbrezillon, frieder.schrempf, juliensu,
	linux-kernel, linux-mtd, rfontana, richard, s.hauer, stefan,
	tglx, vigneshr, yuehaibing

On Wed, 11 Mar 2020 08:43:04 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Hi Mason,
> 
> masonccyang@mxic.com.tw wrote on Wed, 11 Mar 2020 13:40:52 +0800:
> 
> > Hi Boris,
> >   
> > > > diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> > > > index bc2fa3c..c0055ed 100644
> > > > --- a/include/linux/mtd/rawnand.h
> > > > +++ b/include/linux/mtd/rawnand.h
> > > > @@ -1064,6 +1064,8 @@ struct nand_legacy {
> > > >   * @lock:      lock protecting the suspended field. Also used to
> > > >   *         serialize accesses to the NAND device.
> > > >   * @suspended:      set to 1 when the device is suspended, 0 when     
> > it's not.  
> > > > + * @_suspend:      [REPLACEABLE] specific NAND device suspend     
> > operation  
> > > > + * @_resume:      [REPLACEABLE] specific NAND device resume operation
> > > >   * @bbt:      [INTERN] bad block table pointer
> > > >   * @bbt_td:      [REPLACEABLE] bad block table descriptor for flash
> > > >   *         lookup.
> > > > @@ -1119,6 +1121,8 @@ struct nand_chip {
> > > > 
> > > >     struct mutex lock;
> > > >     unsigned int suspended : 1;
> > > > +   int (*_suspend)(struct nand_chip *chip);
> > > > +   void (*_resume)(struct nand_chip *chip);    
> > > 
> > > I thought we agreed on not prefixing new hooks with _ ?    
> > 
> > For [PATCH v2] series, you mentioned to drop the _ prefix 
> > of _lock/_unlock only and we finally patched to lock_area/unlock_area.
> >   
> 
> I missed this _, this is not something we want to add.
> 
> Also, when applying your patches I had several issues because they
> where not base on the last -rc1.
> 
> Finally, I think I forgot a line when patching manually so it produces
> a warning now.
> 
> I am dropping patch 3 and 4, I keep patch 1 and 2 which seem fine.
> 
> Please send a rebased and edited v4 for these, don't forget to drop the
> kbuildtest robot tag and please also follow these slightly edited
> commit logs:
> 
> 2/4
> 
>     mtd: rawnand: Add support for manufacturer specific suspend/resume operation
>     
>     Patch nand_suspend() & nand_resume() to let manufacturers overwrite
>     suspend/resume operations.
> 
> 3/4
> 
>     mtd: rawnand: macronix: Add support for deep power down mode
>     
>     Macronix AD series support deep power down mode for a minimum
>     power consumption state.
>     
>     Overlaod nand_suspend() & nand_resume() in Macronix specific code to
>     support deep power down mode.

And don't forget to propagate the ->suspend() error code to the upper
layer.

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

* Re: [PATCH v3 3/4] mtd: rawnand: Add support manufacturer specific suspend/resume operation
@ 2020-03-11  7:56           ` Boris Brezillon
  0 siblings, 0 replies; 46+ messages in thread
From: Boris Brezillon @ 2020-03-11  7:56 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: vigneshr, bbrezillon, juliensu, richard, s.hauer, yuehaibing,
	linux-kernel, frieder.schrempf, rfontana, linux-mtd, stefan,
	tglx, masonccyang, allison

On Wed, 11 Mar 2020 08:43:04 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Hi Mason,
> 
> masonccyang@mxic.com.tw wrote on Wed, 11 Mar 2020 13:40:52 +0800:
> 
> > Hi Boris,
> >   
> > > > diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> > > > index bc2fa3c..c0055ed 100644
> > > > --- a/include/linux/mtd/rawnand.h
> > > > +++ b/include/linux/mtd/rawnand.h
> > > > @@ -1064,6 +1064,8 @@ struct nand_legacy {
> > > >   * @lock:      lock protecting the suspended field. Also used to
> > > >   *         serialize accesses to the NAND device.
> > > >   * @suspended:      set to 1 when the device is suspended, 0 when     
> > it's not.  
> > > > + * @_suspend:      [REPLACEABLE] specific NAND device suspend     
> > operation  
> > > > + * @_resume:      [REPLACEABLE] specific NAND device resume operation
> > > >   * @bbt:      [INTERN] bad block table pointer
> > > >   * @bbt_td:      [REPLACEABLE] bad block table descriptor for flash
> > > >   *         lookup.
> > > > @@ -1119,6 +1121,8 @@ struct nand_chip {
> > > > 
> > > >     struct mutex lock;
> > > >     unsigned int suspended : 1;
> > > > +   int (*_suspend)(struct nand_chip *chip);
> > > > +   void (*_resume)(struct nand_chip *chip);    
> > > 
> > > I thought we agreed on not prefixing new hooks with _ ?    
> > 
> > For [PATCH v2] series, you mentioned to drop the _ prefix 
> > of _lock/_unlock only and we finally patched to lock_area/unlock_area.
> >   
> 
> I missed this _, this is not something we want to add.
> 
> Also, when applying your patches I had several issues because they
> where not base on the last -rc1.
> 
> Finally, I think I forgot a line when patching manually so it produces
> a warning now.
> 
> I am dropping patch 3 and 4, I keep patch 1 and 2 which seem fine.
> 
> Please send a rebased and edited v4 for these, don't forget to drop the
> kbuildtest robot tag and please also follow these slightly edited
> commit logs:
> 
> 2/4
> 
>     mtd: rawnand: Add support for manufacturer specific suspend/resume operation
>     
>     Patch nand_suspend() & nand_resume() to let manufacturers overwrite
>     suspend/resume operations.
> 
> 3/4
> 
>     mtd: rawnand: macronix: Add support for deep power down mode
>     
>     Macronix AD series support deep power down mode for a minimum
>     power consumption state.
>     
>     Overlaod nand_suspend() & nand_resume() in Macronix specific code to
>     support deep power down mode.

And don't forget to propagate the ->suspend() error code to the upper
layer.

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

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

* Re: [PATCH v3 3/4] mtd: rawnand: Add support manufacturer specific suspend/resume operation
  2020-03-11  6:13         ` masonccyang
@ 2020-03-11  8:01           ` Boris Brezillon
  -1 siblings, 0 replies; 46+ messages in thread
From: Boris Brezillon @ 2020-03-11  8:01 UTC (permalink / raw)
  To: masonccyang
  Cc: allison, bbrezillon, frieder.schrempf, juliensu, linux-kernel,
	linux-mtd, miquel.raynal, rfontana, richard, s.hauer, stefan,
	tglx, vigneshr, yuehaibing

On Wed, 11 Mar 2020 14:13:57 +0800
masonccyang@mxic.com.tw wrote:

> Hi Boris,
> 
> > > > ---
> > > >  drivers/mtd/nand/raw/nand_base.c | 11 ++++++++---
> > > >  include/linux/mtd/rawnand.h      |  4 ++++
> > > >  2 files changed, 12 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/mtd/nand/raw/nand_base.c   
> b/drivers/mtd/nand/raw/nand_base.c
> > > > index 769be81..b44e460 100644
> > > > --- a/drivers/mtd/nand/raw/nand_base.c
> > > > +++ b/drivers/mtd/nand/raw/nand_base.c
> > > > @@ -4327,7 +4327,9 @@ static int nand_suspend(struct mtd_info *mtd)
> > > >     struct nand_chip *chip = mtd_to_nand(mtd);
> > > > 
> > > >     mutex_lock(&chip->lock);
> > > > -   chip->suspended = 1;
> > > > +   if (chip->_suspend)
> > > > +      if (!chip->_suspend(chip))
> > > > +         chip->suspended = 1;  
> > 
> > Shouldn't you propagate the error to the caller if chip->_suspend()
> > fails?  
> 
> Currently, chip->suspend() just do sending command to nand chip and
> I think caller could check chip->suspend = 1 or 0 to know the status
> of nand chip.

No, it can't. The caller (AKA the MTD layer) has no idea about this
chip->suspend field, actually it doesn't even know about the nand_chip
struct. The mtd->_suspend() hook is here to abstract HW details, so
it's the raw NAND framework responsibility to propagate the error code
returned by chip->suspend().

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

* Re: [PATCH v3 3/4] mtd: rawnand: Add support manufacturer specific suspend/resume operation
@ 2020-03-11  8:01           ` Boris Brezillon
  0 siblings, 0 replies; 46+ messages in thread
From: Boris Brezillon @ 2020-03-11  8:01 UTC (permalink / raw)
  To: masonccyang
  Cc: vigneshr, bbrezillon, juliensu, richard, s.hauer, yuehaibing,
	linux-kernel, frieder.schrempf, rfontana, linux-mtd, stefan,
	miquel.raynal, tglx, allison

On Wed, 11 Mar 2020 14:13:57 +0800
masonccyang@mxic.com.tw wrote:

> Hi Boris,
> 
> > > > ---
> > > >  drivers/mtd/nand/raw/nand_base.c | 11 ++++++++---
> > > >  include/linux/mtd/rawnand.h      |  4 ++++
> > > >  2 files changed, 12 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/mtd/nand/raw/nand_base.c   
> b/drivers/mtd/nand/raw/nand_base.c
> > > > index 769be81..b44e460 100644
> > > > --- a/drivers/mtd/nand/raw/nand_base.c
> > > > +++ b/drivers/mtd/nand/raw/nand_base.c
> > > > @@ -4327,7 +4327,9 @@ static int nand_suspend(struct mtd_info *mtd)
> > > >     struct nand_chip *chip = mtd_to_nand(mtd);
> > > > 
> > > >     mutex_lock(&chip->lock);
> > > > -   chip->suspended = 1;
> > > > +   if (chip->_suspend)
> > > > +      if (!chip->_suspend(chip))
> > > > +         chip->suspended = 1;  
> > 
> > Shouldn't you propagate the error to the caller if chip->_suspend()
> > fails?  
> 
> Currently, chip->suspend() just do sending command to nand chip and
> I think caller could check chip->suspend = 1 or 0 to know the status
> of nand chip.

No, it can't. The caller (AKA the MTD layer) has no idea about this
chip->suspend field, actually it doesn't even know about the nand_chip
struct. The mtd->_suspend() hook is here to abstract HW details, so
it's the raw NAND framework responsibility to propagate the error code
returned by chip->suspend().

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

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

* Re: [PATCH v3 3/4] mtd: rawnand: Add support manufacturer specific suspend/resume operation
  2020-03-11  7:43         ` Miquel Raynal
@ 2020-03-12  1:45           ` masonccyang
  -1 siblings, 0 replies; 46+ messages in thread
From: masonccyang @ 2020-03-12  1:45 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: allison, bbrezillon, Boris Brezillon, frieder.schrempf, juliensu,
	linux-kernel, linux-mtd, rfontana, richard, s.hauer, stefan,
	tglx, vigneshr, yuehaibing


Hi Miquel,
 
> > > > diff --git a/include/linux/mtd/rawnand.h 
b/include/linux/mtd/rawnand.h
> > > > index bc2fa3c..c0055ed 100644
> > > > --- a/include/linux/mtd/rawnand.h
> > > > +++ b/include/linux/mtd/rawnand.h
> > > > @@ -1064,6 +1064,8 @@ struct nand_legacy {
> > > >   * @lock:      lock protecting the suspended field. Also used to
> > > >   *         serialize accesses to the NAND device.
> > > >   * @suspended:      set to 1 when the device is suspended, 0 when 
 
> > it's not.
> > > > + * @_suspend:      [REPLACEABLE] specific NAND device suspend 
> > operation
> > > > + * @_resume:      [REPLACEABLE] specific NAND device resume 
operation
> > > >   * @bbt:      [INTERN] bad block table pointer
> > > >   * @bbt_td:      [REPLACEABLE] bad block table descriptor for 
flash
> > > >   *         lookup.
> > > > @@ -1119,6 +1121,8 @@ struct nand_chip {
> > > > 
> > > >     struct mutex lock;
> > > >     unsigned int suspended : 1;
> > > > +   int (*_suspend)(struct nand_chip *chip);
> > > > +   void (*_resume)(struct nand_chip *chip); 
> > > 
> > > I thought we agreed on not prefixing new hooks with _ ? 
> > 
> > For [PATCH v2] series, you mentioned to drop the _ prefix 
> > of _lock/_unlock only and we finally patched to lock_area/unlock_area.
> > 
> 
> I missed this _, this is not something we want to add.
> 
> Also, when applying your patches I had several issues because they
> where not base on the last -rc1.
> 
> Finally, I think I forgot a line when patching manually so it produces
> a warning now.
> 
> I am dropping patch 3 and 4, I keep patch 1 and 2 which seem fine.
> 
> Please send a rebased and edited v4 for these, don't forget to drop the
> kbuildtest robot tag and please also follow these slightly edited
> commit logs:
> 
> 2/4
> 
>     mtd: rawnand: Add support for manufacturer specific suspend/resume 
operation
> 
>     Patch nand_suspend() & nand_resume() to let manufacturers overwrite
>     suspend/resume operations.
> 
> 3/4
> 
>     mtd: rawnand: macronix: Add support for deep power down mode
> 
>     Macronix AD series support deep power down mode for a minimum
>     power consumption state.
> 
>     Overlaod nand_suspend() & nand_resume() in Macronix specific code to
>     support deep power down mode.

okay, will resend [PATCH v4 xx/2] for suspend/resume operation with these 
commit logs.

> 
> Thanks,
> Miquèl

thanks for your review & comments.
Mason


CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information 
and/or personal data, which is protected by applicable laws. Please be 
reminded that duplication, disclosure, distribution, or use of this e-mail 
(and/or its attachments) or any part thereof is prohibited. If you receive 
this e-mail in error, please notify us immediately and delete this mail as 
well as its attachment(s) from your system. In addition, please be 
informed that collection, processing, and/or use of personal data is 
prohibited unless expressly permitted by personal data protection laws. 
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================


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

* Re: [PATCH v3 3/4] mtd: rawnand: Add support manufacturer specific suspend/resume operation
@ 2020-03-12  1:45           ` masonccyang
  0 siblings, 0 replies; 46+ messages in thread
From: masonccyang @ 2020-03-12  1:45 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: vigneshr, bbrezillon, juliensu, richard, s.hauer, yuehaibing,
	linux-kernel, frieder.schrempf, rfontana, Boris Brezillon,
	linux-mtd, stefan, tglx, allison


Hi Miquel,
 
> > > > diff --git a/include/linux/mtd/rawnand.h 
b/include/linux/mtd/rawnand.h
> > > > index bc2fa3c..c0055ed 100644
> > > > --- a/include/linux/mtd/rawnand.h
> > > > +++ b/include/linux/mtd/rawnand.h
> > > > @@ -1064,6 +1064,8 @@ struct nand_legacy {
> > > >   * @lock:      lock protecting the suspended field. Also used to
> > > >   *         serialize accesses to the NAND device.
> > > >   * @suspended:      set to 1 when the device is suspended, 0 when 
 
> > it's not.
> > > > + * @_suspend:      [REPLACEABLE] specific NAND device suspend 
> > operation
> > > > + * @_resume:      [REPLACEABLE] specific NAND device resume 
operation
> > > >   * @bbt:      [INTERN] bad block table pointer
> > > >   * @bbt_td:      [REPLACEABLE] bad block table descriptor for 
flash
> > > >   *         lookup.
> > > > @@ -1119,6 +1121,8 @@ struct nand_chip {
> > > > 
> > > >     struct mutex lock;
> > > >     unsigned int suspended : 1;
> > > > +   int (*_suspend)(struct nand_chip *chip);
> > > > +   void (*_resume)(struct nand_chip *chip); 
> > > 
> > > I thought we agreed on not prefixing new hooks with _ ? 
> > 
> > For [PATCH v2] series, you mentioned to drop the _ prefix 
> > of _lock/_unlock only and we finally patched to lock_area/unlock_area.
> > 
> 
> I missed this _, this is not something we want to add.
> 
> Also, when applying your patches I had several issues because they
> where not base on the last -rc1.
> 
> Finally, I think I forgot a line when patching manually so it produces
> a warning now.
> 
> I am dropping patch 3 and 4, I keep patch 1 and 2 which seem fine.
> 
> Please send a rebased and edited v4 for these, don't forget to drop the
> kbuildtest robot tag and please also follow these slightly edited
> commit logs:
> 
> 2/4
> 
>     mtd: rawnand: Add support for manufacturer specific suspend/resume 
operation
> 
>     Patch nand_suspend() & nand_resume() to let manufacturers overwrite
>     suspend/resume operations.
> 
> 3/4
> 
>     mtd: rawnand: macronix: Add support for deep power down mode
> 
>     Macronix AD series support deep power down mode for a minimum
>     power consumption state.
> 
>     Overlaod nand_suspend() & nand_resume() in Macronix specific code to
>     support deep power down mode.

okay, will resend [PATCH v4 xx/2] for suspend/resume operation with these 
commit logs.

> 
> Thanks,
> Miquèl

thanks for your review & comments.
Mason


CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information 
and/or personal data, which is protected by applicable laws. Please be 
reminded that duplication, disclosure, distribution, or use of this e-mail 
(and/or its attachments) or any part thereof is prohibited. If you receive 
this e-mail in error, please notify us immediately and delete this mail as 
well as its attachment(s) from your system. In addition, please be 
informed that collection, processing, and/or use of personal data is 
prohibited unless expressly permitted by personal data protection laws. 
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================


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

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

* Re: [PATCH v3 3/4] mtd: rawnand: Add support manufacturer specific suspend/resume operation
  2020-03-11  8:01           ` Boris Brezillon
@ 2020-03-12  1:48             ` masonccyang
  -1 siblings, 0 replies; 46+ messages in thread
From: masonccyang @ 2020-03-12  1:48 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: allison, bbrezillon, frieder.schrempf, juliensu, linux-kernel,
	linux-mtd, miquel.raynal, rfontana, richard, s.hauer, stefan,
	tglx, vigneshr, yuehaibing


Hi Boris,

> > > > > ---
> > > > >  drivers/mtd/nand/raw/nand_base.c | 11 ++++++++---
> > > > >  include/linux/mtd/rawnand.h      |  4 ++++
> > > > >  2 files changed, 12 insertions(+), 3 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/mtd/nand/raw/nand_base.c 
> > b/drivers/mtd/nand/raw/nand_base.c
> > > > > index 769be81..b44e460 100644
> > > > > --- a/drivers/mtd/nand/raw/nand_base.c
> > > > > +++ b/drivers/mtd/nand/raw/nand_base.c
> > > > > @@ -4327,7 +4327,9 @@ static int nand_suspend(struct mtd_info 
*mtd)
> > > > >     struct nand_chip *chip = mtd_to_nand(mtd);
> > > > > 
> > > > >     mutex_lock(&chip->lock);
> > > > > -   chip->suspended = 1;
> > > > > +   if (chip->_suspend)
> > > > > +      if (!chip->_suspend(chip))
> > > > > +         chip->suspended = 1; 
> > > 
> > > Shouldn't you propagate the error to the caller if chip->_suspend()
> > > fails? 
> > 
> > Currently, chip->suspend() just do sending command to nand chip and
> > I think caller could check chip->suspend = 1 or 0 to know the status
> > of nand chip.
> 
> No, it can't. The caller (AKA the MTD layer) has no idea about this
> chip->suspend field, actually it doesn't even know about the nand_chip
> struct. The mtd->_suspend() hook is here to abstract HW details, so
> it's the raw NAND framework responsibility to propagate the error code
> returned by chip->suspend().

Got it, thanks!

best regards,
Mason

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information 
and/or personal data, which is protected by applicable laws. Please be 
reminded that duplication, disclosure, distribution, or use of this e-mail 
(and/or its attachments) or any part thereof is prohibited. If you receive 
this e-mail in error, please notify us immediately and delete this mail as 
well as its attachment(s) from your system. In addition, please be 
informed that collection, processing, and/or use of personal data is 
prohibited unless expressly permitted by personal data protection laws. 
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================


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

* Re: [PATCH v3 3/4] mtd: rawnand: Add support manufacturer specific suspend/resume operation
@ 2020-03-12  1:48             ` masonccyang
  0 siblings, 0 replies; 46+ messages in thread
From: masonccyang @ 2020-03-12  1:48 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: vigneshr, bbrezillon, juliensu, richard, s.hauer, yuehaibing,
	linux-kernel, frieder.schrempf, rfontana, linux-mtd, stefan,
	miquel.raynal, tglx, allison


Hi Boris,

> > > > > ---
> > > > >  drivers/mtd/nand/raw/nand_base.c | 11 ++++++++---
> > > > >  include/linux/mtd/rawnand.h      |  4 ++++
> > > > >  2 files changed, 12 insertions(+), 3 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/mtd/nand/raw/nand_base.c 
> > b/drivers/mtd/nand/raw/nand_base.c
> > > > > index 769be81..b44e460 100644
> > > > > --- a/drivers/mtd/nand/raw/nand_base.c
> > > > > +++ b/drivers/mtd/nand/raw/nand_base.c
> > > > > @@ -4327,7 +4327,9 @@ static int nand_suspend(struct mtd_info 
*mtd)
> > > > >     struct nand_chip *chip = mtd_to_nand(mtd);
> > > > > 
> > > > >     mutex_lock(&chip->lock);
> > > > > -   chip->suspended = 1;
> > > > > +   if (chip->_suspend)
> > > > > +      if (!chip->_suspend(chip))
> > > > > +         chip->suspended = 1; 
> > > 
> > > Shouldn't you propagate the error to the caller if chip->_suspend()
> > > fails? 
> > 
> > Currently, chip->suspend() just do sending command to nand chip and
> > I think caller could check chip->suspend = 1 or 0 to know the status
> > of nand chip.
> 
> No, it can't. The caller (AKA the MTD layer) has no idea about this
> chip->suspend field, actually it doesn't even know about the nand_chip
> struct. The mtd->_suspend() hook is here to abstract HW details, so
> it's the raw NAND framework responsibility to propagate the error code
> returned by chip->suspend().

Got it, thanks!

best regards,
Mason

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information 
and/or personal data, which is protected by applicable laws. Please be 
reminded that duplication, disclosure, distribution, or use of this e-mail 
(and/or its attachments) or any part thereof is prohibited. If you receive 
this e-mail in error, please notify us immediately and delete this mail as 
well as its attachment(s) from your system. In addition, please be 
informed that collection, processing, and/or use of personal data is 
prohibited unless expressly permitted by personal data protection laws. 
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================


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

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

end of thread, other threads:[~2020-03-12  1:49 UTC | newest]

Thread overview: 46+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-03  7:21 [PATCH v3 0/4] mtd: rawnand: Add support Macronix Block Portection & Deep Power Down mode Mason Yang
2020-03-03  7:21 ` Mason Yang
2020-03-03  7:21 ` [PATCH v3 1/4] mtd: rawnand: Add support manufacturer specific lock/unlock operation Mason Yang
2020-03-03  7:21   ` Mason Yang
2020-03-10 18:30   ` Miquel Raynal
2020-03-10 18:30     ` Miquel Raynal
2020-03-10 19:27   ` Boris Brezillon
2020-03-10 19:27     ` Boris Brezillon
2020-03-11  2:40     ` masonccyang
2020-03-11  2:40       ` masonccyang
2020-03-11  7:25       ` Miquel Raynal
2020-03-11  7:25         ` Miquel Raynal
2020-03-03  7:21 ` [PATCH v3 2/4] mtd: rawnand: Add support Macronix Block Protection function Mason Yang
2020-03-03  7:21   ` Mason Yang
2020-03-03  7:21 ` [PATCH v3 3/4] mtd: rawnand: Add support manufacturer specific suspend/resume operation Mason Yang
2020-03-03  7:21   ` Mason Yang
2020-03-10 18:30   ` Miquel Raynal
2020-03-10 18:30     ` Miquel Raynal
2020-03-10 19:33   ` Boris Brezillon
2020-03-10 19:33     ` Boris Brezillon
2020-03-11  5:40     ` masonccyang
2020-03-11  5:40       ` masonccyang
2020-03-11  7:43       ` Miquel Raynal
2020-03-11  7:43         ` Miquel Raynal
2020-03-11  7:56         ` Boris Brezillon
2020-03-11  7:56           ` Boris Brezillon
2020-03-12  1:45         ` masonccyang
2020-03-12  1:45           ` masonccyang
2020-03-10 19:39   ` Boris Brezillon
2020-03-10 19:39     ` Boris Brezillon
2020-03-10 19:41     ` Boris Brezillon
2020-03-10 19:41       ` Boris Brezillon
2020-03-11  6:13       ` masonccyang
2020-03-11  6:13         ` masonccyang
2020-03-11  8:01         ` Boris Brezillon
2020-03-11  8:01           ` Boris Brezillon
2020-03-12  1:48           ` masonccyang
2020-03-12  1:48             ` masonccyang
2020-03-03  7:21 ` [PATCH v3 4/4] mtd: rawnand: Add support Macronix deep power down mode Mason Yang
2020-03-03  7:21   ` Mason Yang
2020-03-09 13:14 ` [PATCH v3 0/4] mtd: rawnand: Add support Macronix Block Portection & Deep Power Down mode Miquel Raynal
2020-03-09 13:14   ` Miquel Raynal
2020-03-10  2:30   ` masonccyang
2020-03-10  2:30     ` masonccyang
2020-03-10  7:44     ` Miquel Raynal
2020-03-10  7:44       ` Miquel Raynal

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.