All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/8] Misc timing changes
@ 2020-04-21 16:46 Miquel Raynal
  2020-04-21 16:46 ` [PATCH 1/8] mtd: rawnand: timings: Add mode information to the timings structure Miquel Raynal
                   ` (7 more replies)
  0 siblings, 8 replies; 20+ messages in thread
From: Miquel Raynal @ 2020-04-21 16:46 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd
  Cc: Michal Simek, Boris Brezillon, Naga Sureshkumar Relli,
	Thomas Petazzoni, Miquel Raynal

Hello,

While working on the early ->exec_op() initializations, I figured the
logic might need to be updated a little bit in order to support
"problematic" IPs like Arasan which do not leave a lot of latitude to
the core. While doing these changes in the logic, I also decided to
cleanup this portion a bit, which ended with the writing of a few
fixes.

Cheers,
Miquèl

Miquel Raynal (8):
  mtd: rawnand: timings: Add mode information to the timings structure
  mtd: rawnand: timings: Fix default tR_max and tCCS_min timings
  mtd: rawnand: onfi: Fix redundancy detection check
  mtd: rawnand: onfi: Use an intermediate variable to decomplefixy
    conditions
  mtd: rawnand: onfi: Avoid doing a copy of the parameter page
  mtd: rawnand: onfi: Simplify the NAND operations during detection
  mtd: rawnand: jedec: Use an intermediate variable to decomplefixy
    conditions
  mtd: rawnand: jedec: Simplify the NAND operations during detection

 drivers/mtd/nand/raw/nand_jedec.c   | 21 ++++++++---------
 drivers/mtd/nand/raw/nand_onfi.c    | 36 ++++++++++++-----------------
 drivers/mtd/nand/raw/nand_timings.c | 11 ++++++---
 include/linux/mtd/rawnand.h         | 10 +++++---
 4 files changed, 39 insertions(+), 39 deletions(-)

-- 
2.20.1


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

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

* [PATCH 1/8] mtd: rawnand: timings: Add mode information to the timings structure
  2020-04-21 16:46 [PATCH 0/8] Misc timing changes Miquel Raynal
@ 2020-04-21 16:46 ` Miquel Raynal
  2020-04-22  6:36   ` Boris Brezillon
  2020-04-21 16:46 ` [PATCH 2/8] mtd: rawnand: timings: Fix default tR_max and tCCS_min timings Miquel Raynal
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: Miquel Raynal @ 2020-04-21 16:46 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd
  Cc: Michal Simek, Boris Brezillon, Naga Sureshkumar Relli,
	Thomas Petazzoni, Miquel Raynal

Convert the timings union into a structure containing the mode and the
actual values. The values are still a union in prevision of the
addition of the NVDDR modes.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/nand_timings.c |  6 ++++++
 include/linux/mtd/rawnand.h         | 10 +++++++---
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_timings.c b/drivers/mtd/nand/raw/nand_timings.c
index f64b06a71dfa..0061cbaf931d 100644
--- a/drivers/mtd/nand/raw/nand_timings.c
+++ b/drivers/mtd/nand/raw/nand_timings.c
@@ -16,6 +16,7 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
 	/* Mode 0 */
 	{
 		.type = NAND_SDR_IFACE,
+		.timings.mode = 0,
 		.timings.sdr = {
 			.tCCS_min = 500000,
 			.tR_max = 200000000,
@@ -58,6 +59,7 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
 	/* Mode 1 */
 	{
 		.type = NAND_SDR_IFACE,
+		.timings.mode = 1,
 		.timings.sdr = {
 			.tCCS_min = 500000,
 			.tR_max = 200000000,
@@ -100,6 +102,7 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
 	/* Mode 2 */
 	{
 		.type = NAND_SDR_IFACE,
+		.timings.mode = 2,
 		.timings.sdr = {
 			.tCCS_min = 500000,
 			.tR_max = 200000000,
@@ -142,6 +145,7 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
 	/* Mode 3 */
 	{
 		.type = NAND_SDR_IFACE,
+		.timings.mode = 3,
 		.timings.sdr = {
 			.tCCS_min = 500000,
 			.tR_max = 200000000,
@@ -184,6 +188,7 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
 	/* Mode 4 */
 	{
 		.type = NAND_SDR_IFACE,
+		.timings.mode = 4,
 		.timings.sdr = {
 			.tCCS_min = 500000,
 			.tR_max = 200000000,
@@ -226,6 +231,7 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
 	/* Mode 5 */
 	{
 		.type = NAND_SDR_IFACE,
+		.timings.mode = 5,
 		.timings.sdr = {
 			.tCCS_min = 500000,
 			.tR_max = 200000000,
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 1e76196f9829..d91f914d2e9e 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -491,13 +491,17 @@ enum nand_data_interface_type {
 /**
  * struct nand_data_interface - NAND interface timing
  * @type:	 type of the timing
- * @timings:	 The timing, type according to @type
+ * @timings:	 The timing
+ * @timings.mode: Timing mode as referred in the specification
  * @timings.sdr: Use it when @type is %NAND_SDR_IFACE.
  */
 struct nand_data_interface {
 	enum nand_data_interface_type type;
-	union {
-		struct nand_sdr_timings sdr;
+	struct nand_timings {
+		unsigned int mode;
+		union {
+			struct nand_sdr_timings sdr;
+		};
 	} timings;
 };
 
-- 
2.20.1


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

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

* [PATCH 2/8] mtd: rawnand: timings: Fix default tR_max and tCCS_min timings
  2020-04-21 16:46 [PATCH 0/8] Misc timing changes Miquel Raynal
  2020-04-21 16:46 ` [PATCH 1/8] mtd: rawnand: timings: Add mode information to the timings structure Miquel Raynal
@ 2020-04-21 16:46 ` Miquel Raynal
  2020-04-22  6:47   ` Boris Brezillon
  2020-04-21 16:46 ` [PATCH 3/8] mtd: rawnand: onfi: Fix redundancy detection check Miquel Raynal
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: Miquel Raynal @ 2020-04-21 16:46 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd
  Cc: Michal Simek, Boris Brezillon, Naga Sureshkumar Relli,
	Thomas Petazzoni, Miquel Raynal

These values are hardcoded, there was no need to try to convert them
in picoseconds, better write the right values in picoseconds directly.

Fixes: 6a943386ee36 mtd: rawnand: add default values for dynamic timings
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/nand_timings.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_timings.c b/drivers/mtd/nand/raw/nand_timings.c
index 0061cbaf931d..36d21be3dfe5 100644
--- a/drivers/mtd/nand/raw/nand_timings.c
+++ b/drivers/mtd/nand/raw/nand_timings.c
@@ -320,10 +320,9 @@ int onfi_fill_data_interface(struct nand_chip *chip,
 		/* microseconds -> picoseconds */
 		timings->tPROG_max = 1000000ULL * ONFI_DYN_TIMING_MAX;
 		timings->tBERS_max = 1000000ULL * ONFI_DYN_TIMING_MAX;
-		timings->tR_max = 1000000ULL * 200000000ULL;
 
-		/* nanoseconds -> picoseconds */
-		timings->tCCS_min = 1000UL * 500000;
+		timings->tR_max = 200000000;
+		timings->tCCS_min = 500000;
 	}
 
 	return 0;
-- 
2.20.1


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

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

* [PATCH 3/8] mtd: rawnand: onfi: Fix redundancy detection check
  2020-04-21 16:46 [PATCH 0/8] Misc timing changes Miquel Raynal
  2020-04-21 16:46 ` [PATCH 1/8] mtd: rawnand: timings: Add mode information to the timings structure Miquel Raynal
  2020-04-21 16:46 ` [PATCH 2/8] mtd: rawnand: timings: Fix default tR_max and tCCS_min timings Miquel Raynal
@ 2020-04-21 16:46 ` Miquel Raynal
  2020-04-22  6:49   ` Boris Brezillon
  2020-04-21 16:46 ` [PATCH 4/8] mtd: rawnand: onfi: Use an intermediate variable to decomplefixy conditions Miquel Raynal
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: Miquel Raynal @ 2020-04-21 16:46 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd
  Cc: Michal Simek, Boris Brezillon, Naga Sureshkumar Relli,
	Thomas Petazzoni, Miquel Raynal

During ONFI detection, the CRC derived from the parameter page and the
CRC supposed to be at the end of the parameter page are compared. If
they do not match, the second then the third copies of the page are
tried.

The current implementation compares the newly derived CRC with the CRC
contained in the first page only. So if this particular CRC area has
been corrupted, then the detection will fail for a wrong reason.

Fix this issue by checking the derived CRC against the right one.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/nand_onfi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/nand_onfi.c b/drivers/mtd/nand/raw/nand_onfi.c
index 0b879bd0a68c..8fe8d7bdd203 100644
--- a/drivers/mtd/nand/raw/nand_onfi.c
+++ b/drivers/mtd/nand/raw/nand_onfi.c
@@ -173,7 +173,7 @@ int nand_onfi_detect(struct nand_chip *chip)
 		}
 
 		if (onfi_crc16(ONFI_CRC_BASE, (u8 *)&p[i], 254) ==
-				le16_to_cpu(p->crc)) {
+		    le16_to_cpu(p[i].crc)) {
 			if (i)
 				memcpy(p, &p[i], sizeof(*p));
 			break;
-- 
2.20.1


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

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

* [PATCH 4/8] mtd: rawnand: onfi: Use an intermediate variable to decomplefixy conditions
  2020-04-21 16:46 [PATCH 0/8] Misc timing changes Miquel Raynal
                   ` (2 preceding siblings ...)
  2020-04-21 16:46 ` [PATCH 3/8] mtd: rawnand: onfi: Fix redundancy detection check Miquel Raynal
@ 2020-04-21 16:46 ` Miquel Raynal
  2020-04-22  6:51   ` Boris Brezillon
  2020-04-21 16:46 ` [PATCH 5/8] mtd: rawnand: onfi: Avoid doing a copy of the parameter page Miquel Raynal
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: Miquel Raynal @ 2020-04-21 16:46 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd
  Cc: Michal Simek, Boris Brezillon, Naga Sureshkumar Relli,
	Thomas Petazzoni, Miquel Raynal

Before reworking a little bit the ONFI detection code, let's
decomplefixy the if statements.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/nand_onfi.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_onfi.c b/drivers/mtd/nand/raw/nand_onfi.c
index 8fe8d7bdd203..7d9a3130443a 100644
--- a/drivers/mtd/nand/raw/nand_onfi.c
+++ b/drivers/mtd/nand/raw/nand_onfi.c
@@ -146,6 +146,7 @@ int nand_onfi_detect(struct nand_chip *chip)
 	int onfi_version = 0;
 	char id[4];
 	int i, ret, val;
+	u16 crc;
 
 	memorg = nanddev_get_memorg(&chip->base);
 
@@ -172,8 +173,8 @@ int nand_onfi_detect(struct nand_chip *chip)
 			goto free_onfi_param_page;
 		}
 
-		if (onfi_crc16(ONFI_CRC_BASE, (u8 *)&p[i], 254) ==
-		    le16_to_cpu(p[i].crc)) {
+		crc = onfi_crc16(ONFI_CRC_BASE, (u8 *)&p[i], 254);
+		if (crc == le16_to_cpu(p[i].crc)) {
 			if (i)
 				memcpy(p, &p[i], sizeof(*p));
 			break;
@@ -187,8 +188,8 @@ int nand_onfi_detect(struct nand_chip *chip)
 		nand_bit_wise_majority(srcbufs, ARRAY_SIZE(srcbufs), p,
 				       sizeof(*p));
 
-		if (onfi_crc16(ONFI_CRC_BASE, (u8 *)p, 254) !=
-				le16_to_cpu(p->crc)) {
+		crc = onfi_crc16(ONFI_CRC_BASE, (u8 *)p, 254);
+		if (crc != le16_to_cpu(p->crc)) {
 			pr_err("ONFI parameter recovery failed, aborting\n");
 			goto free_onfi_param_page;
 		}
-- 
2.20.1


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

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

* [PATCH 5/8] mtd: rawnand: onfi: Avoid doing a copy of the parameter page
  2020-04-21 16:46 [PATCH 0/8] Misc timing changes Miquel Raynal
                   ` (3 preceding siblings ...)
  2020-04-21 16:46 ` [PATCH 4/8] mtd: rawnand: onfi: Use an intermediate variable to decomplefixy conditions Miquel Raynal
@ 2020-04-21 16:46 ` Miquel Raynal
  2020-04-22  6:54   ` Boris Brezillon
  2020-04-21 16:46 ` [PATCH 6/8] mtd: rawnand: onfi: Simplify the NAND operations during detection Miquel Raynal
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: Miquel Raynal @ 2020-04-21 16:46 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd
  Cc: Michal Simek, Boris Brezillon, Naga Sureshkumar Relli,
	Thomas Petazzoni, Miquel Raynal

There is not need for copying the parameter page, playing with
pointers works the same.

There is not functional change.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/nand_onfi.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_onfi.c b/drivers/mtd/nand/raw/nand_onfi.c
index 7d9a3130443a..d6124180b47b 100644
--- a/drivers/mtd/nand/raw/nand_onfi.c
+++ b/drivers/mtd/nand/raw/nand_onfi.c
@@ -141,7 +141,7 @@ int nand_onfi_detect(struct nand_chip *chip)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	struct nand_memory_organization *memorg;
-	struct nand_onfi_params *p;
+	struct nand_onfi_params *p = NULL, *pbuf;
 	struct onfi_params *onfi;
 	int onfi_version = 0;
 	char id[4];
@@ -156,8 +156,8 @@ int nand_onfi_detect(struct nand_chip *chip)
 		return 0;
 
 	/* ONFI chip: allocate a buffer to hold its parameter page */
-	p = kzalloc((sizeof(*p) * 3), GFP_KERNEL);
-	if (!p)
+	pbuf = kzalloc((sizeof(*pbuf) * 3), GFP_KERNEL);
+	if (!pbuf)
 		return -ENOMEM;
 
 	ret = nand_read_param_page_op(chip, 0, NULL, 0);
@@ -167,32 +167,31 @@ int nand_onfi_detect(struct nand_chip *chip)
 	}
 
 	for (i = 0; i < 3; i++) {
-		ret = nand_read_data_op(chip, &p[i], sizeof(*p), true);
+		ret = nand_read_data_op(chip, &pbuf[i], sizeof(*pbuf), true);
 		if (ret) {
 			ret = 0;
 			goto free_onfi_param_page;
 		}
 
-		crc = onfi_crc16(ONFI_CRC_BASE, (u8 *)&p[i], 254);
-		if (crc == le16_to_cpu(p[i].crc)) {
-			if (i)
-				memcpy(p, &p[i], sizeof(*p));
+		crc = onfi_crc16(ONFI_CRC_BASE, (u8 *)&pbuf[i], 254);
+		if (crc == le16_to_cpu(pbuf[i].crc)) {
+			p = &pbuf[i];
 			break;
 		}
 	}
 
 	if (i == 3) {
-		const void *srcbufs[3] = {p, p + 1, p + 2};
-
+		const void *srcbufs[3] = {pbuf, pbuf + 1, pbuf + 2};
 		pr_warn("Could not find a valid ONFI parameter page, trying bit-wise majority to recover it\n");
-		nand_bit_wise_majority(srcbufs, ARRAY_SIZE(srcbufs), p,
-				       sizeof(*p));
+		nand_bit_wise_majority(srcbufs, ARRAY_SIZE(srcbufs), pbuf,
+				       sizeof(*pbuf));
 
-		crc = onfi_crc16(ONFI_CRC_BASE, (u8 *)p, 254);
-		if (crc != le16_to_cpu(p->crc)) {
+		crc = onfi_crc16(ONFI_CRC_BASE, (u8 *)pbuf, 254);
+		if (crc != le16_to_cpu(pbuf->crc)) {
 			pr_err("ONFI parameter recovery failed, aborting\n");
 			goto free_onfi_param_page;
 		}
+		p = pbuf;
 	}
 
 	if (chip->manufacturer.desc && chip->manufacturer.desc->ops &&
@@ -300,7 +299,7 @@ int nand_onfi_detect(struct nand_chip *chip)
 	chip->parameters.onfi = onfi;
 
 	/* Identification done, free the full ONFI parameter page and exit */
-	kfree(p);
+	kfree(pbuf);
 
 	return 1;
 
-- 
2.20.1


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

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

* [PATCH 6/8] mtd: rawnand: onfi: Simplify the NAND operations during detection
  2020-04-21 16:46 [PATCH 0/8] Misc timing changes Miquel Raynal
                   ` (4 preceding siblings ...)
  2020-04-21 16:46 ` [PATCH 5/8] mtd: rawnand: onfi: Avoid doing a copy of the parameter page Miquel Raynal
@ 2020-04-21 16:46 ` Miquel Raynal
  2020-04-22  7:00   ` Boris Brezillon
  2020-04-21 16:46 ` [PATCH 7/8] mtd: rawnand: jedec: Use an intermediate variable to decomplefixy conditions Miquel Raynal
  2020-04-21 16:46 ` [PATCH 8/8] mtd: rawnand: jedec: Simplify the NAND operations during detection Miquel Raynal
  7 siblings, 1 reply; 20+ messages in thread
From: Miquel Raynal @ 2020-04-21 16:46 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd
  Cc: Michal Simek, Boris Brezillon, Naga Sureshkumar Relli,
	Thomas Petazzoni, Miquel Raynal

There is no need for separate parameter page reads, the delay penalty
is negligible so let's do read the three copies in one go.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/nand_onfi.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_onfi.c b/drivers/mtd/nand/raw/nand_onfi.c
index d6124180b47b..b76772666b82 100644
--- a/drivers/mtd/nand/raw/nand_onfi.c
+++ b/drivers/mtd/nand/raw/nand_onfi.c
@@ -160,19 +160,13 @@ int nand_onfi_detect(struct nand_chip *chip)
 	if (!pbuf)
 		return -ENOMEM;
 
-	ret = nand_read_param_page_op(chip, 0, NULL, 0);
+	ret = nand_read_param_page_op(chip, 0, pbuf, 3 * sizeof(*pbuf));
 	if (ret) {
 		ret = 0;
 		goto free_onfi_param_page;
 	}
 
 	for (i = 0; i < 3; i++) {
-		ret = nand_read_data_op(chip, &pbuf[i], sizeof(*pbuf), true);
-		if (ret) {
-			ret = 0;
-			goto free_onfi_param_page;
-		}
-
 		crc = onfi_crc16(ONFI_CRC_BASE, (u8 *)&pbuf[i], 254);
 		if (crc == le16_to_cpu(pbuf[i].crc)) {
 			p = &pbuf[i];
-- 
2.20.1


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

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

* [PATCH 7/8] mtd: rawnand: jedec: Use an intermediate variable to decomplefixy conditions
  2020-04-21 16:46 [PATCH 0/8] Misc timing changes Miquel Raynal
                   ` (5 preceding siblings ...)
  2020-04-21 16:46 ` [PATCH 6/8] mtd: rawnand: onfi: Simplify the NAND operations during detection Miquel Raynal
@ 2020-04-21 16:46 ` Miquel Raynal
  2020-04-22  7:02   ` Boris Brezillon
  2020-04-22  8:49   ` Sergei Shtylyov
  2020-04-21 16:46 ` [PATCH 8/8] mtd: rawnand: jedec: Simplify the NAND operations during detection Miquel Raynal
  7 siblings, 2 replies; 20+ messages in thread
From: Miquel Raynal @ 2020-04-21 16:46 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd
  Cc: Michal Simek, Boris Brezillon, Naga Sureshkumar Relli,
	Thomas Petazzoni, Miquel Raynal

Before reworking a little bit the JEDEC detection code, let's
decomplefixy an if statement.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/nand_jedec.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_jedec.c b/drivers/mtd/nand/raw/nand_jedec.c
index 9b540e76f84f..4cc1ea512887 100644
--- a/drivers/mtd/nand/raw/nand_jedec.c
+++ b/drivers/mtd/nand/raw/nand_jedec.c
@@ -28,6 +28,7 @@ int nand_jedec_detect(struct nand_chip *chip)
 	int jedec_version = 0;
 	char id[5];
 	int i, val, ret;
+	u16 crc;
 
 	memorg = nanddev_get_memorg(&chip->base);
 
@@ -54,8 +55,8 @@ int nand_jedec_detect(struct nand_chip *chip)
 			goto free_jedec_param_page;
 		}
 
-		if (onfi_crc16(ONFI_CRC_BASE, (uint8_t *)p, 510) ==
-				le16_to_cpu(p->crc))
+		crc = onfi_crc16(ONFI_CRC_BASE, (u8 *)p, 510);
+		if (crc == le16_to_cpu(p->crc))
 			break;
 	}
 
-- 
2.20.1


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

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

* [PATCH 8/8] mtd: rawnand: jedec: Simplify the NAND operations during detection
  2020-04-21 16:46 [PATCH 0/8] Misc timing changes Miquel Raynal
                   ` (6 preceding siblings ...)
  2020-04-21 16:46 ` [PATCH 7/8] mtd: rawnand: jedec: Use an intermediate variable to decomplefixy conditions Miquel Raynal
@ 2020-04-21 16:46 ` Miquel Raynal
  2020-04-22  7:05   ` Boris Brezillon
  7 siblings, 1 reply; 20+ messages in thread
From: Miquel Raynal @ 2020-04-21 16:46 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd
  Cc: Michal Simek, Boris Brezillon, Naga Sureshkumar Relli,
	Thomas Petazzoni, Miquel Raynal

There is no need for separate parameter page reads, the delay penalty
is negligible so let's do read the three copies in one go.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/nand_jedec.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_jedec.c b/drivers/mtd/nand/raw/nand_jedec.c
index 4cc1ea512887..befe8c29b505 100644
--- a/drivers/mtd/nand/raw/nand_jedec.c
+++ b/drivers/mtd/nand/raw/nand_jedec.c
@@ -23,7 +23,7 @@ int nand_jedec_detect(struct nand_chip *chip)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	struct nand_memory_organization *memorg;
-	struct nand_jedec_params *p;
+	struct nand_jedec_params *p = NULL, *pbuf;
 	struct jedec_ecc_info *ecc;
 	int jedec_version = 0;
 	char id[5];
@@ -38,25 +38,19 @@ int nand_jedec_detect(struct nand_chip *chip)
 		return 0;
 
 	/* JEDEC chip: allocate a buffer to hold its parameter page */
-	p = kzalloc(sizeof(*p), GFP_KERNEL);
-	if (!p)
+	pbuf = kzalloc(sizeof(*pbuf) * 3, GFP_KERNEL);
+	if (!pbuf)
 		return -ENOMEM;
 
-	ret = nand_read_param_page_op(chip, 0x40, NULL, 0);
+	ret = nand_read_param_page_op(chip, 0x40, pbuf, 3 * sizeof(*pbuf));
 	if (ret) {
 		ret = 0;
 		goto free_jedec_param_page;
 	}
 
 	for (i = 0; i < 3; i++) {
-		ret = nand_read_data_op(chip, p, sizeof(*p), true);
-		if (ret) {
-			ret = 0;
-			goto free_jedec_param_page;
-		}
-
-		crc = onfi_crc16(ONFI_CRC_BASE, (u8 *)p, 510);
-		if (crc == le16_to_cpu(p->crc))
+		crc = onfi_crc16(ONFI_CRC_BASE, (u8 *)&pbuf[i], 510);
+		if (crc == le16_to_cpu(pbuf[i].crc))
 			break;
 	}
 
@@ -65,6 +59,8 @@ int nand_jedec_detect(struct nand_chip *chip)
 		goto free_jedec_param_page;
 	}
 
+	p = pbuf;
+
 	/* Check version */
 	val = le16_to_cpu(p->revision);
 	if (val & (1 << 2))
-- 
2.20.1


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

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

* Re: [PATCH 1/8] mtd: rawnand: timings: Add mode information to the timings structure
  2020-04-21 16:46 ` [PATCH 1/8] mtd: rawnand: timings: Add mode information to the timings structure Miquel Raynal
@ 2020-04-22  6:36   ` Boris Brezillon
  0 siblings, 0 replies; 20+ messages in thread
From: Boris Brezillon @ 2020-04-22  6:36 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Michal Simek, Vignesh Raghavendra, Tudor Ambarus,
	Richard Weinberger, linux-mtd, Thomas Petazzoni,
	Naga Sureshkumar Relli

On Tue, 21 Apr 2020 18:46:30 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Convert the timings union into a structure containing the mode and the
> actual values. The values are still a union in prevision of the
> addition of the NVDDR modes.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>

Two nits below.

> ---
>  drivers/mtd/nand/raw/nand_timings.c |  6 ++++++
>  include/linux/mtd/rawnand.h         | 10 +++++++---
>  2 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_timings.c b/drivers/mtd/nand/raw/nand_timings.c
> index f64b06a71dfa..0061cbaf931d 100644
> --- a/drivers/mtd/nand/raw/nand_timings.c
> +++ b/drivers/mtd/nand/raw/nand_timings.c
> @@ -16,6 +16,7 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
>  	/* Mode 0 */
>  	{
>  		.type = NAND_SDR_IFACE,
> +		.timings.mode = 0,
>  		.timings.sdr = {
>  			.tCCS_min = 500000,
>  			.tR_max = 200000000,
> @@ -58,6 +59,7 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
>  	/* Mode 1 */
>  	{
>  		.type = NAND_SDR_IFACE,
> +		.timings.mode = 1,
>  		.timings.sdr = {
>  			.tCCS_min = 500000,
>  			.tR_max = 200000000,
> @@ -100,6 +102,7 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
>  	/* Mode 2 */
>  	{
>  		.type = NAND_SDR_IFACE,
> +		.timings.mode = 2,
>  		.timings.sdr = {
>  			.tCCS_min = 500000,
>  			.tR_max = 200000000,
> @@ -142,6 +145,7 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
>  	/* Mode 3 */
>  	{
>  		.type = NAND_SDR_IFACE,
> +		.timings.mode = 3,
>  		.timings.sdr = {
>  			.tCCS_min = 500000,
>  			.tR_max = 200000000,
> @@ -184,6 +188,7 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
>  	/* Mode 4 */
>  	{
>  		.type = NAND_SDR_IFACE,
> +		.timings.mode = 4,
>  		.timings.sdr = {
>  			.tCCS_min = 500000,
>  			.tR_max = 200000000,
> @@ -226,6 +231,7 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
>  	/* Mode 5 */
>  	{
>  		.type = NAND_SDR_IFACE,
> +		.timings.mode = 5,
>  		.timings.sdr = {
>  			.tCCS_min = 500000,
>  			.tR_max = 200000000,
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index 1e76196f9829..d91f914d2e9e 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -491,13 +491,17 @@ enum nand_data_interface_type {
>  /**
>   * struct nand_data_interface - NAND interface timing
>   * @type:	 type of the timing
> - * @timings:	 The timing, type according to @type
> + * @timings:	 The timing

			     ^timing information

> + * @timings.mode: Timing mode as referred in the specification

				  ^ as defined?

>   * @timings.sdr: Use it when @type is %NAND_SDR_IFACE.
>   */
>  struct nand_data_interface {
>  	enum nand_data_interface_type type;
> -	union {
> -		struct nand_sdr_timings sdr;
> +	struct nand_timings {
> +		unsigned int mode;
> +		union {
> +			struct nand_sdr_timings sdr;
> +		};
>  	} timings;
>  };
>  


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

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

* Re: [PATCH 2/8] mtd: rawnand: timings: Fix default tR_max and tCCS_min timings
  2020-04-21 16:46 ` [PATCH 2/8] mtd: rawnand: timings: Fix default tR_max and tCCS_min timings Miquel Raynal
@ 2020-04-22  6:47   ` Boris Brezillon
  0 siblings, 0 replies; 20+ messages in thread
From: Boris Brezillon @ 2020-04-22  6:47 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Michal Simek, Vignesh Raghavendra, Tudor Ambarus,
	Richard Weinberger, linux-mtd, Thomas Petazzoni,
	Naga Sureshkumar Relli

On Tue, 21 Apr 2020 18:46:31 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> These values are hardcoded, there was no need to try to convert them
> in picoseconds, better write the right values in picoseconds directly.

Hm I had a hard time understanding what was wrong and whether this fix
was correct or not. Maybe you should just say that tR and tCCS are
currently wrongly expressed in femto seconds and this is restoring it
to pico seconds.

Other than that, this diff looks good.

Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>

> 
> Fixes: 6a943386ee36 mtd: rawnand: add default values for dynamic timings
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/nand_timings.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_timings.c b/drivers/mtd/nand/raw/nand_timings.c
> index 0061cbaf931d..36d21be3dfe5 100644
> --- a/drivers/mtd/nand/raw/nand_timings.c
> +++ b/drivers/mtd/nand/raw/nand_timings.c
> @@ -320,10 +320,9 @@ int onfi_fill_data_interface(struct nand_chip *chip,
>  		/* microseconds -> picoseconds */
>  		timings->tPROG_max = 1000000ULL * ONFI_DYN_TIMING_MAX;
>  		timings->tBERS_max = 1000000ULL * ONFI_DYN_TIMING_MAX;
> -		timings->tR_max = 1000000ULL * 200000000ULL;
>  
> -		/* nanoseconds -> picoseconds */
> -		timings->tCCS_min = 1000UL * 500000;
> +		timings->tR_max = 200000000;
> +		timings->tCCS_min = 500000;
>  	}
>  
>  	return 0;


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

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

* Re: [PATCH 3/8] mtd: rawnand: onfi: Fix redundancy detection check
  2020-04-21 16:46 ` [PATCH 3/8] mtd: rawnand: onfi: Fix redundancy detection check Miquel Raynal
@ 2020-04-22  6:49   ` Boris Brezillon
  0 siblings, 0 replies; 20+ messages in thread
From: Boris Brezillon @ 2020-04-22  6:49 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Michal Simek, Vignesh Raghavendra, Tudor Ambarus,
	Richard Weinberger, linux-mtd, Thomas Petazzoni,
	Naga Sureshkumar Relli

On Tue, 21 Apr 2020 18:46:32 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> During ONFI detection, the CRC derived from the parameter page and the
> CRC supposed to be at the end of the parameter page are compared. If
> they do not match, the second then the third copies of the page are
> tried.
> 
> The current implementation compares the newly derived CRC with the CRC
> contained in the first page only. So if this particular CRC area has
> been corrupted, then the detection will fail for a wrong reason.
> 
> Fix this issue by checking the derived CRC against the right one.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>

> ---
>  drivers/mtd/nand/raw/nand_onfi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_onfi.c b/drivers/mtd/nand/raw/nand_onfi.c
> index 0b879bd0a68c..8fe8d7bdd203 100644
> --- a/drivers/mtd/nand/raw/nand_onfi.c
> +++ b/drivers/mtd/nand/raw/nand_onfi.c
> @@ -173,7 +173,7 @@ int nand_onfi_detect(struct nand_chip *chip)
>  		}
>  
>  		if (onfi_crc16(ONFI_CRC_BASE, (u8 *)&p[i], 254) ==
> -				le16_to_cpu(p->crc)) {
> +		    le16_to_cpu(p[i].crc)) {
>  			if (i)
>  				memcpy(p, &p[i], sizeof(*p));
>  			break;


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

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

* Re: [PATCH 4/8] mtd: rawnand: onfi: Use an intermediate variable to decomplefixy conditions
  2020-04-21 16:46 ` [PATCH 4/8] mtd: rawnand: onfi: Use an intermediate variable to decomplefixy conditions Miquel Raynal
@ 2020-04-22  6:51   ` Boris Brezillon
  0 siblings, 0 replies; 20+ messages in thread
From: Boris Brezillon @ 2020-04-22  6:51 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Michal Simek, Vignesh Raghavendra, Tudor Ambarus,
	Richard Weinberger, linux-mtd, Thomas Petazzoni,
	Naga Sureshkumar Relli

On Tue, 21 Apr 2020 18:46:33 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Before reworking a little bit the ONFI detection code, let's
> decomplefixy the if statements.

It's more about coding style/formatting than complexity, but the change
improves readability indeed, so

Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>

> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/nand_onfi.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_onfi.c b/drivers/mtd/nand/raw/nand_onfi.c
> index 8fe8d7bdd203..7d9a3130443a 100644
> --- a/drivers/mtd/nand/raw/nand_onfi.c
> +++ b/drivers/mtd/nand/raw/nand_onfi.c
> @@ -146,6 +146,7 @@ int nand_onfi_detect(struct nand_chip *chip)
>  	int onfi_version = 0;
>  	char id[4];
>  	int i, ret, val;
> +	u16 crc;
>  
>  	memorg = nanddev_get_memorg(&chip->base);
>  
> @@ -172,8 +173,8 @@ int nand_onfi_detect(struct nand_chip *chip)
>  			goto free_onfi_param_page;
>  		}
>  
> -		if (onfi_crc16(ONFI_CRC_BASE, (u8 *)&p[i], 254) ==
> -		    le16_to_cpu(p[i].crc)) {
> +		crc = onfi_crc16(ONFI_CRC_BASE, (u8 *)&p[i], 254);
> +		if (crc == le16_to_cpu(p[i].crc)) {
>  			if (i)
>  				memcpy(p, &p[i], sizeof(*p));
>  			break;
> @@ -187,8 +188,8 @@ int nand_onfi_detect(struct nand_chip *chip)
>  		nand_bit_wise_majority(srcbufs, ARRAY_SIZE(srcbufs), p,
>  				       sizeof(*p));
>  
> -		if (onfi_crc16(ONFI_CRC_BASE, (u8 *)p, 254) !=
> -				le16_to_cpu(p->crc)) {
> +		crc = onfi_crc16(ONFI_CRC_BASE, (u8 *)p, 254);
> +		if (crc != le16_to_cpu(p->crc)) {
>  			pr_err("ONFI parameter recovery failed, aborting\n");
>  			goto free_onfi_param_page;
>  		}


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

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

* Re: [PATCH 5/8] mtd: rawnand: onfi: Avoid doing a copy of the parameter page
  2020-04-21 16:46 ` [PATCH 5/8] mtd: rawnand: onfi: Avoid doing a copy of the parameter page Miquel Raynal
@ 2020-04-22  6:54   ` Boris Brezillon
  0 siblings, 0 replies; 20+ messages in thread
From: Boris Brezillon @ 2020-04-22  6:54 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Michal Simek, Vignesh Raghavendra, Tudor Ambarus,
	Richard Weinberger, linux-mtd, Thomas Petazzoni,
	Naga Sureshkumar Relli

On Tue, 21 Apr 2020 18:46:34 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> There is not need for copying the parameter page, playing with

	    ^no

> pointers works the same.

	   ^does the trick?

> 
> There is not functional change.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>

> ---
>  drivers/mtd/nand/raw/nand_onfi.c | 29 ++++++++++++++---------------
>  1 file changed, 14 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_onfi.c b/drivers/mtd/nand/raw/nand_onfi.c
> index 7d9a3130443a..d6124180b47b 100644
> --- a/drivers/mtd/nand/raw/nand_onfi.c
> +++ b/drivers/mtd/nand/raw/nand_onfi.c
> @@ -141,7 +141,7 @@ int nand_onfi_detect(struct nand_chip *chip)
>  {
>  	struct mtd_info *mtd = nand_to_mtd(chip);
>  	struct nand_memory_organization *memorg;
> -	struct nand_onfi_params *p;
> +	struct nand_onfi_params *p = NULL, *pbuf;
>  	struct onfi_params *onfi;
>  	int onfi_version = 0;
>  	char id[4];
> @@ -156,8 +156,8 @@ int nand_onfi_detect(struct nand_chip *chip)
>  		return 0;
>  
>  	/* ONFI chip: allocate a buffer to hold its parameter page */
> -	p = kzalloc((sizeof(*p) * 3), GFP_KERNEL);
> -	if (!p)
> +	pbuf = kzalloc((sizeof(*pbuf) * 3), GFP_KERNEL);
> +	if (!pbuf)
>  		return -ENOMEM;
>  
>  	ret = nand_read_param_page_op(chip, 0, NULL, 0);
> @@ -167,32 +167,31 @@ int nand_onfi_detect(struct nand_chip *chip)
>  	}
>  
>  	for (i = 0; i < 3; i++) {
> -		ret = nand_read_data_op(chip, &p[i], sizeof(*p), true);
> +		ret = nand_read_data_op(chip, &pbuf[i], sizeof(*pbuf), true);
>  		if (ret) {
>  			ret = 0;
>  			goto free_onfi_param_page;
>  		}
>  
> -		crc = onfi_crc16(ONFI_CRC_BASE, (u8 *)&p[i], 254);
> -		if (crc == le16_to_cpu(p[i].crc)) {
> -			if (i)
> -				memcpy(p, &p[i], sizeof(*p));
> +		crc = onfi_crc16(ONFI_CRC_BASE, (u8 *)&pbuf[i], 254);
> +		if (crc == le16_to_cpu(pbuf[i].crc)) {
> +			p = &pbuf[i];
>  			break;
>  		}
>  	}
>  
>  	if (i == 3) {
> -		const void *srcbufs[3] = {p, p + 1, p + 2};
> -
> +		const void *srcbufs[3] = {pbuf, pbuf + 1, pbuf + 2};
>  		pr_warn("Could not find a valid ONFI parameter page, trying bit-wise majority to recover it\n");
> -		nand_bit_wise_majority(srcbufs, ARRAY_SIZE(srcbufs), p,
> -				       sizeof(*p));
> +		nand_bit_wise_majority(srcbufs, ARRAY_SIZE(srcbufs), pbuf,
> +				       sizeof(*pbuf));
>  
> -		crc = onfi_crc16(ONFI_CRC_BASE, (u8 *)p, 254);
> -		if (crc != le16_to_cpu(p->crc)) {
> +		crc = onfi_crc16(ONFI_CRC_BASE, (u8 *)pbuf, 254);
> +		if (crc != le16_to_cpu(pbuf->crc)) {
>  			pr_err("ONFI parameter recovery failed, aborting\n");
>  			goto free_onfi_param_page;
>  		}
> +		p = pbuf;
>  	}
>  
>  	if (chip->manufacturer.desc && chip->manufacturer.desc->ops &&
> @@ -300,7 +299,7 @@ int nand_onfi_detect(struct nand_chip *chip)
>  	chip->parameters.onfi = onfi;
>  
>  	/* Identification done, free the full ONFI parameter page and exit */
> -	kfree(p);
> +	kfree(pbuf);
>  
>  	return 1;
>  


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

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

* Re: [PATCH 6/8] mtd: rawnand: onfi: Simplify the NAND operations during detection
  2020-04-21 16:46 ` [PATCH 6/8] mtd: rawnand: onfi: Simplify the NAND operations during detection Miquel Raynal
@ 2020-04-22  7:00   ` Boris Brezillon
  2020-04-22  7:25     ` Miquel Raynal
  0 siblings, 1 reply; 20+ messages in thread
From: Boris Brezillon @ 2020-04-22  7:00 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Michal Simek, Vignesh Raghavendra, Tudor Ambarus,
	Richard Weinberger, linux-mtd, Thomas Petazzoni,
	Naga Sureshkumar Relli

On Tue, 21 Apr 2020 18:46:35 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> There is no need for separate parameter page reads, the delay penalty
> is negligible so let's do read the three copies in one go.

		    ^let's read

In theory that's correct, but I fear this was done because some
controllers couldn't read 768 bytes in one go. Could we do that only if
the controller implements exec_op() and exec_op(check_only) returns true?

> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/nand_onfi.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_onfi.c b/drivers/mtd/nand/raw/nand_onfi.c
> index d6124180b47b..b76772666b82 100644
> --- a/drivers/mtd/nand/raw/nand_onfi.c
> +++ b/drivers/mtd/nand/raw/nand_onfi.c
> @@ -160,19 +160,13 @@ int nand_onfi_detect(struct nand_chip *chip)
>  	if (!pbuf)
>  		return -ENOMEM;
>  
> -	ret = nand_read_param_page_op(chip, 0, NULL, 0);
> +	ret = nand_read_param_page_op(chip, 0, pbuf, 3 * sizeof(*pbuf));
>  	if (ret) {
>  		ret = 0;
>  		goto free_onfi_param_page;
>  	}
>  
>  	for (i = 0; i < 3; i++) {
> -		ret = nand_read_data_op(chip, &pbuf[i], sizeof(*pbuf), true);
> -		if (ret) {
> -			ret = 0;
> -			goto free_onfi_param_page;
> -		}
> -
>  		crc = onfi_crc16(ONFI_CRC_BASE, (u8 *)&pbuf[i], 254);
>  		if (crc == le16_to_cpu(pbuf[i].crc)) {
>  			p = &pbuf[i];


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

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

* Re: [PATCH 7/8] mtd: rawnand: jedec: Use an intermediate variable to decomplefixy conditions
  2020-04-21 16:46 ` [PATCH 7/8] mtd: rawnand: jedec: Use an intermediate variable to decomplefixy conditions Miquel Raynal
@ 2020-04-22  7:02   ` Boris Brezillon
  2020-04-22  8:49   ` Sergei Shtylyov
  1 sibling, 0 replies; 20+ messages in thread
From: Boris Brezillon @ 2020-04-22  7:02 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Michal Simek, Vignesh Raghavendra, Tudor Ambarus,
	Richard Weinberger, linux-mtd, Thomas Petazzoni,
	Naga Sureshkumar Relli

On Tue, 21 Apr 2020 18:46:36 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Before reworking a little bit the JEDEC detection code, let's
> decomplefixy an if statement.

Same comment as earlier, it's not about complexity but readability.

Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>

> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/nand_jedec.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_jedec.c b/drivers/mtd/nand/raw/nand_jedec.c
> index 9b540e76f84f..4cc1ea512887 100644
> --- a/drivers/mtd/nand/raw/nand_jedec.c
> +++ b/drivers/mtd/nand/raw/nand_jedec.c
> @@ -28,6 +28,7 @@ int nand_jedec_detect(struct nand_chip *chip)
>  	int jedec_version = 0;
>  	char id[5];
>  	int i, val, ret;
> +	u16 crc;
>  
>  	memorg = nanddev_get_memorg(&chip->base);
>  
> @@ -54,8 +55,8 @@ int nand_jedec_detect(struct nand_chip *chip)
>  			goto free_jedec_param_page;
>  		}
>  
> -		if (onfi_crc16(ONFI_CRC_BASE, (uint8_t *)p, 510) ==
> -				le16_to_cpu(p->crc))
> +		crc = onfi_crc16(ONFI_CRC_BASE, (u8 *)p, 510);
> +		if (crc == le16_to_cpu(p->crc))
>  			break;
>  	}
>  


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

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

* Re: [PATCH 8/8] mtd: rawnand: jedec: Simplify the NAND operations during detection
  2020-04-21 16:46 ` [PATCH 8/8] mtd: rawnand: jedec: Simplify the NAND operations during detection Miquel Raynal
@ 2020-04-22  7:05   ` Boris Brezillon
  0 siblings, 0 replies; 20+ messages in thread
From: Boris Brezillon @ 2020-04-22  7:05 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Michal Simek, Vignesh Raghavendra, Tudor Ambarus,
	Richard Weinberger, linux-mtd, Thomas Petazzoni,
	Naga Sureshkumar Relli

On Tue, 21 Apr 2020 18:46:37 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> There is no need for separate parameter page reads, the delay penalty
> is negligible so let's do read the three copies in one go.

Same comment as for the ONFI param page read. Let's make sure we don't
break existing users by using exec_op()'s check capability.

> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/nand_jedec.c | 20 ++++++++------------
>  1 file changed, 8 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_jedec.c b/drivers/mtd/nand/raw/nand_jedec.c
> index 4cc1ea512887..befe8c29b505 100644
> --- a/drivers/mtd/nand/raw/nand_jedec.c
> +++ b/drivers/mtd/nand/raw/nand_jedec.c
> @@ -23,7 +23,7 @@ int nand_jedec_detect(struct nand_chip *chip)
>  {
>  	struct mtd_info *mtd = nand_to_mtd(chip);
>  	struct nand_memory_organization *memorg;
> -	struct nand_jedec_params *p;
> +	struct nand_jedec_params *p = NULL, *pbuf;
>  	struct jedec_ecc_info *ecc;
>  	int jedec_version = 0;
>  	char id[5];
> @@ -38,25 +38,19 @@ int nand_jedec_detect(struct nand_chip *chip)
>  		return 0;
>  
>  	/* JEDEC chip: allocate a buffer to hold its parameter page */
> -	p = kzalloc(sizeof(*p), GFP_KERNEL);
> -	if (!p)
> +	pbuf = kzalloc(sizeof(*pbuf) * 3, GFP_KERNEL);
> +	if (!pbuf)
>  		return -ENOMEM;
>  
> -	ret = nand_read_param_page_op(chip, 0x40, NULL, 0);
> +	ret = nand_read_param_page_op(chip, 0x40, pbuf, 3 * sizeof(*pbuf));
>  	if (ret) {
>  		ret = 0;
>  		goto free_jedec_param_page;
>  	}
>  
>  	for (i = 0; i < 3; i++) {
> -		ret = nand_read_data_op(chip, p, sizeof(*p), true);
> -		if (ret) {
> -			ret = 0;
> -			goto free_jedec_param_page;
> -		}
> -
> -		crc = onfi_crc16(ONFI_CRC_BASE, (u8 *)p, 510);
> -		if (crc == le16_to_cpu(p->crc))
> +		crc = onfi_crc16(ONFI_CRC_BASE, (u8 *)&pbuf[i], 510);
> +		if (crc == le16_to_cpu(pbuf[i].crc))
>  			break;
>  	}
>  
> @@ -65,6 +59,8 @@ int nand_jedec_detect(struct nand_chip *chip)
>  		goto free_jedec_param_page;
>  	}
>  
> +	p = pbuf;
> +
>  	/* Check version */
>  	val = le16_to_cpu(p->revision);
>  	if (val & (1 << 2))


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

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

* Re: [PATCH 6/8] mtd: rawnand: onfi: Simplify the NAND operations during detection
  2020-04-22  7:00   ` Boris Brezillon
@ 2020-04-22  7:25     ` Miquel Raynal
  0 siblings, 0 replies; 20+ messages in thread
From: Miquel Raynal @ 2020-04-22  7:25 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Michal Simek, Vignesh Raghavendra, Tudor Ambarus,
	Richard Weinberger, linux-mtd, Thomas Petazzoni,
	Naga Sureshkumar Relli

Hi Boris,

Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed, 22 Apr
2020 09:00:52 +0200:

> On Tue, 21 Apr 2020 18:46:35 +0200
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > There is no need for separate parameter page reads, the delay penalty
> > is negligible so let's do read the three copies in one go.  
> 
> 		    ^let's read
> 
> In theory that's correct, but I fear this was done because some
> controllers couldn't read 768 bytes in one go. Could we do that only if
> the controller implements exec_op() and exec_op(check_only) returns true?

Thanks for reviewing all the patches. You are right that it might break
drivers so I'll find a more appropriate way to do it (same for JEDEC).

> 
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  drivers/mtd/nand/raw/nand_onfi.c | 8 +-------
> >  1 file changed, 1 insertion(+), 7 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/raw/nand_onfi.c b/drivers/mtd/nand/raw/nand_onfi.c
> > index d6124180b47b..b76772666b82 100644
> > --- a/drivers/mtd/nand/raw/nand_onfi.c
> > +++ b/drivers/mtd/nand/raw/nand_onfi.c
> > @@ -160,19 +160,13 @@ int nand_onfi_detect(struct nand_chip *chip)
> >  	if (!pbuf)
> >  		return -ENOMEM;
> >  
> > -	ret = nand_read_param_page_op(chip, 0, NULL, 0);
> > +	ret = nand_read_param_page_op(chip, 0, pbuf, 3 * sizeof(*pbuf));
> >  	if (ret) {
> >  		ret = 0;
> >  		goto free_onfi_param_page;
> >  	}
> >  
> >  	for (i = 0; i < 3; i++) {
> > -		ret = nand_read_data_op(chip, &pbuf[i], sizeof(*pbuf), true);
> > -		if (ret) {
> > -			ret = 0;
> > -			goto free_onfi_param_page;
> > -		}
> > -
> >  		crc = onfi_crc16(ONFI_CRC_BASE, (u8 *)&pbuf[i], 254);
> >  		if (crc == le16_to_cpu(pbuf[i].crc)) {
> >  			p = &pbuf[i];  
> 

Thanks,
Miquèl

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

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

* Re: [PATCH 7/8] mtd: rawnand: jedec: Use an intermediate variable to decomplefixy conditions
  2020-04-21 16:46 ` [PATCH 7/8] mtd: rawnand: jedec: Use an intermediate variable to decomplefixy conditions Miquel Raynal
  2020-04-22  7:02   ` Boris Brezillon
@ 2020-04-22  8:49   ` Sergei Shtylyov
  2020-04-22  9:33     ` Miquel Raynal
  1 sibling, 1 reply; 20+ messages in thread
From: Sergei Shtylyov @ 2020-04-22  8:49 UTC (permalink / raw)
  To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	Tudor Ambarus, linux-mtd
  Cc: Michal Simek, Boris Brezillon, Naga Sureshkumar Relli, Thomas Petazzoni

Hello!

On 21.04.2020 19:46, Miquel Raynal wrote:

> Before reworking a little bit the JEDEC detection code, let's
> decomplefixy an if statement.

    Decomplexify, maybe? :-)

> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
[...]

MBR, Sergei

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

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

* Re: [PATCH 7/8] mtd: rawnand: jedec: Use an intermediate variable to decomplefixy conditions
  2020-04-22  8:49   ` Sergei Shtylyov
@ 2020-04-22  9:33     ` Miquel Raynal
  0 siblings, 0 replies; 20+ messages in thread
From: Miquel Raynal @ 2020-04-22  9:33 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Michal Simek, Vignesh Raghavendra, Tudor Ambarus,
	Richard Weinberger, Boris Brezillon, linux-mtd, Thomas Petazzoni,
	Naga Sureshkumar Relli

Hi Sergei,

Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote on Wed, 22
Apr 2020 11:49:14 +0300:

> Hello!
> 
> On 21.04.2020 19:46, Miquel Raynal wrote:
> 
> > Before reworking a little bit the JEDEC detection code, let's
> > decomplefixy an if statement.  
> 
>     Decomplexify, maybe? :-)

Oops :)

> 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>  
> [...]
> 
> MBR, Sergei

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

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

end of thread, other threads:[~2020-04-22  9:34 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-21 16:46 [PATCH 0/8] Misc timing changes Miquel Raynal
2020-04-21 16:46 ` [PATCH 1/8] mtd: rawnand: timings: Add mode information to the timings structure Miquel Raynal
2020-04-22  6:36   ` Boris Brezillon
2020-04-21 16:46 ` [PATCH 2/8] mtd: rawnand: timings: Fix default tR_max and tCCS_min timings Miquel Raynal
2020-04-22  6:47   ` Boris Brezillon
2020-04-21 16:46 ` [PATCH 3/8] mtd: rawnand: onfi: Fix redundancy detection check Miquel Raynal
2020-04-22  6:49   ` Boris Brezillon
2020-04-21 16:46 ` [PATCH 4/8] mtd: rawnand: onfi: Use an intermediate variable to decomplefixy conditions Miquel Raynal
2020-04-22  6:51   ` Boris Brezillon
2020-04-21 16:46 ` [PATCH 5/8] mtd: rawnand: onfi: Avoid doing a copy of the parameter page Miquel Raynal
2020-04-22  6:54   ` Boris Brezillon
2020-04-21 16:46 ` [PATCH 6/8] mtd: rawnand: onfi: Simplify the NAND operations during detection Miquel Raynal
2020-04-22  7:00   ` Boris Brezillon
2020-04-22  7:25     ` Miquel Raynal
2020-04-21 16:46 ` [PATCH 7/8] mtd: rawnand: jedec: Use an intermediate variable to decomplefixy conditions Miquel Raynal
2020-04-22  7:02   ` Boris Brezillon
2020-04-22  8:49   ` Sergei Shtylyov
2020-04-22  9:33     ` Miquel Raynal
2020-04-21 16:46 ` [PATCH 8/8] mtd: rawnand: jedec: Simplify the NAND operations during detection Miquel Raynal
2020-04-22  7:05   ` Boris Brezillon

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