[U-Boot] [PATCH 1/2] mtd/nand:Fix wrong address read in is_blank()

[U-Boot] [PATCH 1/2] mtd/nand:Fix wrong address read in is_blank()

[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.

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 <poonam.aggrwal@freescale.com>
Signed-off-by: Prabhakar Kushwaha <prabhakar@freescale.com>
---
 Based upon git://git.denx.de/u-boot.git (branch master)
 
 Top commit:
            commit bfcc40bb09b05c90cc3b1496abb270eb8aa72134
            Merge: cba9a89 1895420
            Author: Wolfgang Denk <wd@denx.de>
            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(-)

diff --git a/drivers/mtd/nand/fsl_ifc_nand.c b/drivers/mtd/nand/fsl_ifc_nand.c
index b3f3c3c..3a28ab8 100644
--- a/drivers/mtd/nand/fsl_ifc_nand.c
+++ b/drivers/mtd/nand/fsl_ifc_nand.c
@@ -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;
 
-- 
1.7.5.4

[U-Boot] [PATCH 1/2] mtd/nand:Fix wrong address read in is_blank()

[Marek Vasut]

> 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 <poonam.aggrwal@freescale.com>
> Signed-off-by: Prabhakar Kushwaha <prabhakar@freescale.com>

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
> ---
>  Based upon git://git.denx.de/u-boot.git (branch master)
> 
>  Top commit:
>             commit bfcc40bb09b05c90cc3b1496abb270eb8aa72134
>             Merge: cba9a89 1895420
>             Author: Wolfgang Denk <wd@denx.de>
>             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(-)
> 
> diff --git a/drivers/mtd/nand/fsl_ifc_nand.c
> b/drivers/mtd/nand/fsl_ifc_nand.c index b3f3c3c..3a28ab8 100644
> --- a/drivers/mtd/nand/fsl_ifc_nand.c
> +++ b/drivers/mtd/nand/fsl_ifc_nand.c
> @@ -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;

[U-Boot] [PATCH 1/2] mtd/nand:Fix wrong address read in is_blank()

[Kushwaha Prabhakar-B32579]

> > 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 <poonam.aggrwal@freescale.com>
> > Signed-off-by: Prabhakar Kushwaha <prabhakar@freescale.com>
> 
> 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

> 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.

> 3) Cc Scott Wood
I will take care this point in future

--Prabhakar

> > ---
> >  Based upon git://git.denx.de/u-boot.git (branch master)
> >
> >  Top commit:
> >             commit bfcc40bb09b05c90cc3b1496abb270eb8aa72134
> >             Merge: cba9a89 1895420
> >             Author: Wolfgang Denk <wd@denx.de>
> >             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(-)
> >
> > diff --git a/drivers/mtd/nand/fsl_ifc_nand.c
> > b/drivers/mtd/nand/fsl_ifc_nand.c index b3f3c3c..3a28ab8 100644
> > --- a/drivers/mtd/nand/fsl_ifc_nand.c
> > +++ b/drivers/mtd/nand/fsl_ifc_nand.c
> > @@ -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;

[U-Boot] [PATCH 1/2] mtd/nand:Fix wrong address read in is_blank()

[Marek Vasut]

> > > 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 <poonam.aggrwal@freescale.com>
> > > Signed-off-by: Prabhakar Kushwaha <prabhakar@freescale.com>
> > 
> > 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
> 
> > 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.
> 
> > 3) Cc Scott Wood
> 
> I will take care this point in future
> 
> --Prabhakar

Awesome, good going, thanks!

M

[U-Boot] [PATCH 1/2] mtd/nand:Fix wrong address read in is_blank()

[Scott Wood]

On 12/27/2011 11:22 PM, Prabhakar Kushwaha wrote:
> 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 <poonam.aggrwal@freescale.com>
> Signed-off-by: Prabhakar Kushwaha <prabhakar@freescale.com>
> ---

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

Also, please mention IFC in the subject.

-Scott