mtd: mxc_nand: Fix crash after MTD resync

PATCH New Marek Vasut 2 participants 5 messages Jul 2, 2013

Subsystems (3)

Arm i.MX and Layerscape Memory technology devices (MTD) NAND flash

lore

Share & tools

lore

Marek Vasut

author

2013-07-02 15:11 UTC #0 lore raw

PATCH 1/1 mtd: mxc_nand: Fix crash after MTD resync

The driver triggered a BUG() in nand_base.c:3214/nand_scan_tail() because the ecc.strength was incorrectly set in case of NAND_ECC_HW instead of NAND_ECC_HW_SYNDROME ECC mode.

Signed-off-by Marek Vasut <[email protected]>

Cc Beno?t Th?baudeau <[email protected]>

Cc Fabio Estevam <[email protected]>

Cc Scott Wood <[email protected]>

---
 drivers/mtd/nand/mxc_nand.c |    7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

drivers/mtd/nand/mxc_nand.c +2 -5

index ac435f2..08c7b8b 100644
@@ -1238,15 +1238,12 @@ int board_nand_init(struct nand_chip *this)
 		this->ecc.write_oob = mxc_nand_write_oob_syndrome;
 		this->ecc.bytes = 9;
 		this->ecc.prepad = 7;
-	} else {
-		this->ecc.mode = NAND_ECC_HW;
-	}
-
-	if (this->ecc.mode == NAND_ECC_HW) {
 		if (is_mxc_nfc_1())
 			this->ecc.strength = 1;
 		else
 			this->ecc.strength = 4;
+	} else {
+		this->ecc.mode = NAND_ECC_HW;
 	}
 
 	host->pagesize_2k = 0;

Signature · 1 line

1.7.10.4

Scott Wood

2013-07-02 19:13 UTC #1 lore raw

PATCH 1/1 mtd: mxc_nand: Fix crash after MTD resync

Quoted 22 lines · Marek Vasut

The driver triggered a BUG() in nand_base.c:3214/nand_scan_tail() because the ecc.strength was incorrectly set in case of NAND_ECC_HW instead of NAND_ECC_HW_SYNDROME ECC mode.
Signed-off-by Marek Vasut <[email protected]>
Cc Beno?t Th?baudeau <[email protected]>
Cc Fabio Estevam <[email protected]>
Cc Scott Wood <[email protected]>
---
 drivers/mtd/nand/mxc_nand.c |    7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)
drivers/mtd/nand/mxc_nand.c +0 -2
index ac435f2..08c7b8b 100644
@@ -1238,15 +1238,12 @@ int board_nand_init(struct nand_chip *this)
 		this->ecc.write_oob = mxc_nand_write_oob_syndrome;
 		this->ecc.bytes = 9;
 		this->ecc.prepad = 7;
-	} else {
-		this->ecc.mode = NAND_ECC_HW;

drivers/mtd/nand/mxc_nand.c int board_nand_init(struct nand_chip *this) · line 1247

Quoted 10 lines

drivers/mtd/nand/mxc_nand.c +2 -3

-	}
-
-	if (this->ecc.mode == NAND_ECC_HW) {
 		if (is_mxc_nfc_1())
 			this->ecc.strength = 1;
 		else
 			this->ecc.strength = 4;
+	} else {
+		this->ecc.mode = NAND_ECC_HW;
 	}

Where does ecc.strength now get set for the non-SYNDROME case?

-Scott

Marek Vasut

author

2013-07-02 20:57 UTC #2 lore raw

PATCH 1/1 mtd: mxc_nand: Fix crash after MTD resync

Dear Scott Wood,

Quoted 36 lines · Marek Vasut

Quoted 35 lines
The driver triggered a BUG() in nand_base.c:3214/nand_scan_tail() because the ecc.strength was incorrectly set in case of NAND_ECC_HW instead of NAND_ECC_HW_SYNDROME ECC mode.
Signed-off-by Marek Vasut <[email protected]>
Cc Beno?t Th?baudeau <[email protected]>
Cc Fabio Estevam <[email protected]>
Cc Scott Wood <[email protected]>
---
 drivers/mtd/nand/mxc_nand.c |    7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)
drivers/mtd/nand/mxc_nand.c +0 -5
index ac435f2..08c7b8b 100644
@@ -1238,15 +1238,12 @@ int board_nand_init(struct nand_chip *this)
 
 		this->ecc.write_oob = mxc_nand_write_oob_syndrome;
 		this->ecc.bytes = 9;
 		this->ecc.prepad = 7;
 
-	} else {
-		this->ecc.mode = NAND_ECC_HW;
-	}
-
-	if (this->ecc.mode == NAND_ECC_HW) {
 
 		if (is_mxc_nfc_1())
 
 			this->ecc.strength = 1;
 
 		else

Quoted 8 lines

drivers/mtd/nand/mxc_nand.c int board_nand_init(struct nand_chip *this)
Quoted 6 lines
drivers/mtd/nand/mxc_nand.c +2 -0
 			this->ecc.strength = 4;
 
+	} else {
+		this->ecc.mode = NAND_ECC_HW;
 
 	}
Where does ecc.strength now get set for the non-SYNDROME case?

Gone ;-) The strength only needs to be set for the SYNDROME case according to the code in nand_base.c .

Best regards,
Marek Vasut

Scott Wood

2013-07-02 21:02 UTC #3 lore raw

PATCH 1/1 mtd: mxc_nand: Fix crash after MTD resync

Quoted 46 lines · Marek Vasut

Dear Scott Wood,
Quoted 3 lines · Marek Vasut
Quoted 2 lines
The driver triggered a BUG() in nand_base.c:3214/nand_scan_tail() because the ecc.strength was incorrectly set in case of
NAND_ECC_HW
Quoted 8 lines
Quoted 8 lines
instead of NAND_ECC_HW_SYNDROME ECC mode.
Signed-off-by Marek Vasut <[email protected]>
Cc Beno?t Th?baudeau <[email protected]>
Cc Fabio Estevam <[email protected]>
Cc Scott Wood <[email protected]>
---
Quoted 4 lines
Review
Quoted 4 lines
 drivers/mtd/nand/mxc_nand.c |    7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)
(quoted diff) +0 -0
b/drivers/mtd/nand/mxc_nand.c
Quoted 4 lines
drivers/mtd/nand/mxc_nand.c int board_nand_init(struct nand_chip
Quoted 4 lines
index ac435f2..08c7b8b 100644
drivers/mtd/nand/mxc_nand.c +0 -0
@@ -1238,15 +1238,12 @@ int board_nand_init(struct nand_chip  
*this)
Quoted 22 lines
Quoted 22 lines
drivers/mtd/nand/mxc_nand.c +2 -5
 		this->ecc.write_oob = mxc_nand_write_oob_syndrome;
 		this->ecc.bytes = 9;
 		this->ecc.prepad = 7;
 
-	} else {
-		this->ecc.mode = NAND_ECC_HW;
-	}
-
-	if (this->ecc.mode == NAND_ECC_HW) {
 
 		if (is_mxc_nfc_1())
 
 			this->ecc.strength = 1;
 
 		else
 
 			this->ecc.strength = 4;
 
+	} else {
+		this->ecc.mode = NAND_ECC_HW;

Quoted 7 lines

Quoted 3 lines
Quoted 1 line
}
Where does ecc.strength now get set for the non-SYNDROME case?
Gone ;-) The strength only needs to be set for the SYNDROME case according to
the code in nand_base.c .

Read it again. Note the lack of a "break;" before "case NAND_ECC_HW_SYNDROME:".

-Scott

Marek Vasut

author

2013-07-02 21:11 UTC #4 lore raw

PATCH 1/1 mtd: mxc_nand: Fix crash after MTD resync

Dear Scott Wood,

Quoted 56 lines · Marek Vasut

Quoted 55 lines
Dear Scott Wood,
Quoted 3 lines · Marek Vasut
Quoted 2 lines
The driver triggered a BUG() in nand_base.c:3214/nand_scan_tail() because the ecc.strength was incorrectly set in case of
NAND_ECC_HW
Quoted 8 lines
Quoted 8 lines
instead of NAND_ECC_HW_SYNDROME ECC mode.
Signed-off-by Marek Vasut <[email protected]>
Cc Beno?t Th?baudeau <[email protected]>
Cc Fabio Estevam <[email protected]>
Cc Scott Wood <[email protected]>
---
Quoted 4 lines
Review
Quoted 4 lines
 drivers/mtd/nand/mxc_nand.c |    7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)
(quoted diff) +0 -0
b/drivers/mtd/nand/mxc_nand.c
Quoted 4 lines
drivers/mtd/nand/mxc_nand.c int board_nand_init(struct nand_chip
Quoted 4 lines
index ac435f2..08c7b8b 100644
drivers/mtd/nand/mxc_nand.c +0 -0
@@ -1238,15 +1238,12 @@ int board_nand_init(struct nand_chip
*this)
Quoted 24 lines
Quoted 12 lines
drivers/mtd/nand/mxc_nand.c +0 -5
 		this->ecc.write_oob = mxc_nand_write_oob_syndrome;
 		this->ecc.bytes = 9;
 		this->ecc.prepad = 7;
 
-	} else {
-		this->ecc.mode = NAND_ECC_HW;
-	}
-
-	if (this->ecc.mode == NAND_ECC_HW) {
 
 		if (is_mxc_nfc_1())
 
drivers/mtd/nand/mxc_nand.c int board_nand_init(struct nand_chip
Quoted 10 lines
drivers/mtd/nand/mxc_nand.c +2 -0
 			this->ecc.strength = 1;
 
 		else
 
 			this->ecc.strength = 4;
 
+	} else {
+		this->ecc.mode = NAND_ECC_HW;
 
 	}
Where does ecc.strength now get set for the non-SYNDROME case?

Quoted 6 lines

Quoted 3 lines
Gone ;-) The strength only needs to be set for the SYNDROME case according to
the code in nand_base.c .
Read it again. Note the lack of a "break;" before "case NAND_ECC_HW_SYNDROME:".

Blargh. V2-time.

Best regards,
Marek Vasut