Skip to contentSkip to conversation

mtd/nand:Fix wrong address read in is_blank()

PATCH New Prabhakar Kushwaha 4 participants 5 messages Dec 28, 2011 – Jan 4, 2012

Subsystems (2)
Memory technology devices (MTD) NAND flash
lore
Share & tools
lore

Prabhakar Kushwaha

author
#0 lore raw

PATCH 1/2 mtd/nand:Fix wrong address read in is_blank()

IFC NAND Machine calculates ECC on 512byte sector. Same is taken care in fsl_ifc_run_command() while ECC status verification. Here buffer number is calculated assuming 512byte sector and same is passed to is_blank. However in is_blank() buffer address is calculated using mdt->writesize which is wrong. It should be calculated on basis of ecc sector size.

Also, in fsl_ifc_run_command() bufferpage is calculated on the basis of ecc sector size instead of hard coded value.

Signed-off-by Poonam Aggrwal <[email protected]>
Signed-off-by Prabhakar Kushwaha <[email protected]>

---
Based upon git://git.denx.de/u-boot.git (branch master)

Top commit:
commit bfcc40bb09b05c90cc3b1496abb270eb8aa72134
Merge: cba9a89 1895420
Author: Wolfgang Denk <[email protected]>
Date: Fri Dec 23 20:53:58 2011 +0100

Merge branch 'next' of ../next

* 'next' of ../next:
mkenvimage: Add version info switch (-V)
mkenvimage: Fix getopt() error handling
mkenvimage: Fix some typos
phy: add Micrel KS8721BL phy definition
net: introduce per device index
mvgbe: remove setting of ethaddr within the driver
x86: Add support for specifying an initrd with the zboot command
x86: Refactor the zboot innards so they can be reused with a vboot image
x86: Add infrastructure to extract an e820 table from the coreboot tables
x86: Add support for booting Linux using the 32 bit boot protocol
x86: Clean up the x86 zimage code in preparation to extend it
x86: Import code from coreboot's libpayload to parse the coreboot table
x86: Initial commit for running as a coreboot payload
CHECKPATCH: ./board/esd/hh405/logo_320_240_8bpp.c
CHECKPATCH: ./board/esd/hh405/logo_1024_768_8bpp.c
CHECKPATCH: ./board/esd/hh405/logo_320_240_4bpp.c
CHECKPATCH: ./board/esd/hh405/logo_640_480_24bpp.c
CHECKPATCH: ./board/esd/apc405/logo_640_480_24bpp.c
CHECKPATCH: ./board/esd/voh405/logo_320_240_4bpp.c
CHECKPATCH: ./board/esd/voh405/logo_640_480_24bpp.c
CHECKPATCH: ./board/esd/hh405/fpgadata.c
CHECKPATCH: ./board/esd/pci405/fpgadata.c
CHECKPATCH: ./board/esd/tasreg/fpgadata.c

 drivers/mtd/nand/fsl_ifc_nand.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)
drivers/mtd/nand/fsl_ifc_nand.c +4 -2
index b3f3c3c..3a28ab8 100644
@@ -196,7 +196,9 @@ static int is_blank(struct mtd_info *mtd, struct fsl_ifc_ctrl *ctrl,
 {
 	struct nand_chip *chip = mtd->priv;
 	struct fsl_ifc_mtd *priv = chip->priv;
-	u8 __iomem *addr = priv->vbase + bufnum * (mtd->writesize * 2);
+	int bufperpage = mtd->writesize / chip->ecc.size;
+	u8 __iomem *addr = priv->vbase + bufnum / bufperpage
+					* (mtd->writesize * 2);
 	u32 __iomem *main = (u32 *)addr;
 	u8 __iomem *oob = addr + mtd->writesize;
 	int i;

@@ -279,7 +281,7 @@ static int fsl_ifc_run_command(struct mtd_info *mtd)
 		printf("%s: Write Protect Error\n", __func__);
 
 	if (ctrl->eccread) {
-		int bufperpage = mtd->writesize / 512;
+		int bufperpage = mtd->writesize / chip->ecc.size;
 		int bufnum = (ctrl->page & priv->bufnum_mask) * bufperpage;
 		int bufnum_end = bufnum + bufperpage - 1;
Signature · 1 line
1.7.5.4
#1 lore raw

PATCH 1/2 mtd/nand:Fix wrong address read in is_blank()

Quoted 11 lines

IFC NAND Machine calculates ECC on 512byte sector. Same is taken care in fsl_ifc_run_command() while ECC status verification. Here buffer number is calculated assuming 512byte sector and same is passed to is_blank. However in is_blank() buffer address is calculated using mdt->writesize which is wrong. It should be calculated on basis of ecc sector size.

Also, in fsl_ifc_run_command() bufferpage is calculated on the basis of ecc sector size instead of hard coded value.

Signed-off-by Poonam Aggrwal <[email protected]>
Signed-off-by Prabhakar Kushwaha <[email protected]>

You just sent two patches based on the same commit, but those are clearly dependent. This is weird. So next time:

1) Submit this with git-send-email as a series
2) iomem looks linuxish, so instead of specifying u-boot commit, specify Linux commit/tree etc. from where this is pulled so when someone synces next time, he'll know where he is in linux.
3) Cc Scott Wood

M

Quoted 62 lines

---
Based upon git://git.denx.de/u-boot.git (branch master)

Top commit:
commit bfcc40bb09b05c90cc3b1496abb270eb8aa72134
Merge: cba9a89 1895420
Author: Wolfgang Denk <[email protected]>
Date: Fri Dec 23 20:53:58 2011 +0100

Merge branch 'next' of ../next

* 'next' of ../next:
mkenvimage: Add version info switch (-V)
mkenvimage: Fix getopt() error handling
mkenvimage: Fix some typos
phy: add Micrel KS8721BL phy definition
net: introduce per device index
mvgbe: remove setting of ethaddr within the driver
x86: Add support for specifying an initrd with the zboot
command x86: Refactor the zboot innards so they can be reused with a vboot image x86: Add infrastructure to extract an e820 table from the coreboot tables x86: Add support for booting Linux using the 32 bit boot protocol
x86: Clean up the x86 zimage code in preparation to extend it x86: Import code from coreboot's libpayload to parse the coreboot table x86: Initial commit for running as a coreboot payload CHECKPATCH:
./board/esd/hh405/logo_320_240_8bpp.c

                CHECKPATCH: ./board/esd/hh405/logo_1024_768_8bpp.c
                CHECKPATCH: ./board/esd/hh405/logo_320_240_4bpp.c
                CHECKPATCH: ./board/esd/hh405/logo_640_480_24bpp.c
                CHECKPATCH: ./board/esd/apc405/logo_640_480_24bpp.c
                CHECKPATCH: ./board/esd/voh405/logo_320_240_4bpp.c
                CHECKPATCH: ./board/esd/voh405/logo_640_480_24bpp.c
                CHECKPATCH: ./board/esd/hh405/fpgadata.c
                CHECKPATCH: ./board/esd/pci405/fpgadata.c
                CHECKPATCH: ./board/esd/tasreg/fpgadata.c
 drivers/mtd/nand/fsl_ifc_nand.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)
(quoted diff) +0 -0

b/drivers/mtd/nand/fsl_ifc_nand.c index b3f3c3c..3a28ab8 100644

drivers/mtd/nand/fsl_ifc_nand.c +4 -2
@@ -196,7 +196,9 @@ static int is_blank(struct mtd_info *mtd, struct
 fsl_ifc_ctrl *ctrl, {
 	struct nand_chip *chip = mtd->priv;
 	struct fsl_ifc_mtd *priv = chip->priv;
-	u8 __iomem *addr = priv->vbase + bufnum * (mtd->writesize * 2);
+	int bufperpage = mtd->writesize / chip->ecc.size;
+	u8 __iomem *addr = priv->vbase + bufnum / bufperpage
+					* (mtd->writesize * 2);
 	u32 __iomem *main = (u32 *)addr;
 	u8 __iomem *oob = addr + mtd->writesize;
 	int i;

@@ -279,7 +281,7 @@ static int fsl_ifc_run_command(struct mtd_info *mtd)
 		printf("%s: Write Protect Error\n", __func__);
 
 	if (ctrl->eccread) {
-		int bufperpage = mtd->writesize / 512;
+		int bufperpage = mtd->writesize / chip->ecc.size;
 		int bufnum = (ctrl->page & priv->bufnum_mask) * bufperpage;
 		int bufnum_end = bufnum + bufperpage - 1;
#2 lore raw

PATCH 1/2 mtd/nand:Fix wrong address read in is_blank()

Quoted 12 lines
Quoted 4 lines

IFC NAND Machine calculates ECC on 512byte sector. Same is taken care in
fsl_ifc_run_command() while ECC status verification. Here buffer number is calculated assuming 512byte sector and same is passed to

is_blank.

Quoted 2 lines

However in is_blank() buffer address is calculated using mdt->writesize which is wrong. It should be calculated on basis of ecc

sector size.

Quoted 4 lines

Also, in fsl_ifc_run_command() bufferpage is calculated on the basis of ecc sector size instead of hard coded value.

Quoted 7 lines
Quoted 2 lines
Signed-off-by Poonam Aggrwal <[email protected]>
Signed-off-by Prabhakar Kushwaha <[email protected]>

You just sent two patches based on the same commit, but those are clearly dependent. This is weird. So next time:

1) Submit this with git-send-email as a series

I will take care this point in future

Quoted 3 lines

2) iomem looks linuxish, so instead of specifying u-boot commit, specify Linux commit/tree etc. from where this is pulled so when someone synces next time, he'll know where he is in linux.

This patch is independent of Linux tree. I will float similar patch to Linux tree.

Quoted 1 line

3) Cc Scott Wood

I will take care this point in future

--Prabhakar

Quoted 69 lines
Quoted 11 lines

---
Based upon git://git.denx.de/u-boot.git (branch master)

Top commit:
commit bfcc40bb09b05c90cc3b1496abb270eb8aa72134
Merge: cba9a89 1895420
Author: Wolfgang Denk <[email protected]>
Date: Fri Dec 23 20:53:58 2011 +0100

Merge branch 'next' of ../next

Quoted 8 lines

* 'next' of ../next:
mkenvimage: Add version info switch (-V)
mkenvimage: Fix getopt() error handling
mkenvimage: Fix some typos
phy: add Micrel KS8721BL phy definition
net: introduce per device index
mvgbe: remove setting of ethaddr within the driver
x86: Add support for specifying an initrd with the

zboot

Quoted 1 line

command x86: Refactor the zboot innards so they can be reused with a

vboot

Quoted 1 line

image x86: Add infrastructure to extract an e820 table from the

coreboot

Quoted 1 line

tables x86: Add support for booting Linux using the 32 bit boot

protocol

Quoted 1 line

x86: Clean up the x86 zimage code in preparation to extend it x86:

Import

Quoted 1 line

code from coreboot's libpayload to parse the coreboot table x86:

Initial

drivers/mtd/nand/fsl_ifc_nand.c static int fsl_ifc_run_command(struct mtd_info

Quoted 31 lines

commit for running as a coreboot payload CHECKPATCH:
./board/esd/hh405/logo_320_240_8bpp.c

                CHECKPATCH: ./board/esd/hh405/logo_1024_768_8bpp.c
                CHECKPATCH: ./board/esd/hh405/logo_320_240_4bpp.c
                CHECKPATCH: ./board/esd/hh405/logo_640_480_24bpp.c
                CHECKPATCH: ./board/esd/apc405/logo_640_480_24bpp.c
                CHECKPATCH: ./board/esd/voh405/logo_320_240_4bpp.c
                CHECKPATCH: ./board/esd/voh405/logo_640_480_24bpp.c
                CHECKPATCH: ./board/esd/hh405/fpgadata.c
                CHECKPATCH: ./board/esd/pci405/fpgadata.c
                CHECKPATCH: ./board/esd/tasreg/fpgadata.c
 drivers/mtd/nand/fsl_ifc_nand.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)
(quoted diff) +0 -0

b/drivers/mtd/nand/fsl_ifc_nand.c index b3f3c3c..3a28ab8 100644

drivers/mtd/nand/fsl_ifc_nand.c +3 -1
@@ -196,7 +196,9 @@ static int is_blank(struct mtd_info *mtd, struct
 fsl_ifc_ctrl *ctrl, {
 	struct nand_chip *chip = mtd->priv;
 	struct fsl_ifc_mtd *priv = chip->priv;
-	u8 __iomem *addr = priv->vbase + bufnum * (mtd->writesize * 2);
+	int bufperpage = mtd->writesize / chip->ecc.size;
+	u8 __iomem *addr = priv->vbase + bufnum / bufperpage
+					* (mtd->writesize * 2);
 	u32 __iomem *main = (u32 *)addr;
 	u8 __iomem *oob = addr + mtd->writesize;
 	int i;

@@ -279,7 +281,7 @@ static int fsl_ifc_run_command(struct mtd_info

*mtd)

Quoted 7 lines
drivers/mtd/nand/fsl_ifc_nand.c +1 -1
 		printf("%s: Write Protect Error\n", __func__);
 
 	if (ctrl->eccread) {
-		int bufperpage = mtd->writesize / 512;
+		int bufperpage = mtd->writesize / chip->ecc.size;
 		int bufnum = (ctrl->page & priv->bufnum_mask) * bufperpage;
 		int bufnum_end = bufnum + bufperpage - 1;
#3 lore raw

PATCH 1/2 mtd/nand:Fix wrong address read in is_blank()

Quoted 29 lines
Quoted 15 lines
Quoted 4 lines

IFC NAND Machine calculates ECC on 512byte sector. Same is taken care in
fsl_ifc_run_command() while ECC status verification. Here buffer number is calculated assuming 512byte sector and same is passed to

is_blank.

Quoted 2 lines

However in is_blank() buffer address is calculated using mdt->writesize which is wrong. It should be calculated on basis of ecc

sector size.

Quoted 3 lines

Also, in fsl_ifc_run_command() bufferpage is calculated on the basis of ecc sector size instead of hard coded value.

Quoted 7 lines
Quoted 2 lines
Signed-off-by Poonam Aggrwal <[email protected]>
Signed-off-by Prabhakar Kushwaha <[email protected]>

You just sent two patches based on the same commit, but those are clearly dependent. This is weird. So next time:

1) Submit this with git-send-email as a series

I will take care this point in future

Quoted 3 lines

2) iomem looks linuxish, so instead of specifying u-boot commit, specify Linux commit/tree etc. from where this is pulled so when someone synces next time, he'll know where he is in linux.

Quoted 8 lines

This patch is independent of Linux tree. I will float similar patch to Linux tree.

Quoted 1 line

3) Cc Scott Wood

I will take care this point in future

--Prabhakar

Awesome, good going, thanks!

M

#4 lore raw

PATCH 1/2 mtd/nand:Fix wrong address read in is_blank()

In reply to Prabhakar Kushwaha
Quoted 6 lines · Prabhakar Kushwaha

IFC NAND Machine calculates ECC on 512byte sector. Same is taken care in fsl_ifc_run_command() while ECC status verification. Here buffer number is calculated assuming 512byte sector and same is passed to is_blank. However in is_blank() buffer address is calculated using mdt->writesize which is wrong. It should be calculated on basis of ecc sector size.

Quoted 6 lines

Also, in fsl_ifc_run_command() bufferpage is calculated on the basis of ecc sector size instead of hard coded value.

Signed-off-by Poonam Aggrwal <[email protected]>
Signed-off-by Prabhakar Kushwaha <[email protected]>

---

Same changes requested as in http://patchwork.ozlabs.org/patch/133500/

Also, please mention IFC in the subject.

-Scott