| Message ID | 1372434962.2374989.1344890982845.JavaMail.root@advansee.com |
|---|---|
| State | Accepted |
| Commit | c1db8dd62b337372a08942e1c5945a8590afbc58 |
| Delegated to: | Scott Wood |
| Headers | show |
On Mon, Aug 13, 2012 at 10:49:42PM +0200, Benoît Thébaudeau wrote: > On the NFC IP 1.1, the 32-bit ecc_status_result value comes from 2 > consecutive 16-bit registers. This patch reads all the fields of this value, > which makes a difference for 4-kiB NF pages. > > Signed-off-by: Benoît Thébaudeau <benoit.thebaudeau@advansee.com> > Cc: Scott Wood <scottwood@freescale.com> > Cc: Stefano Babic <sbabic@denx.de> > --- > .../drivers/mtd/nand/mxc_nand.c | 2 +- > .../include/fsl_nfc.h | 3 +-- > .../nand_spl/nand_boot_fsl_nfc.c | 4 ++++ > 3 files changed, 6 insertions(+), 3 deletions(-) > > diff --git u-boot-4d3c95f.orig/drivers/mtd/nand/mxc_nand.c u-boot-4d3c95f/drivers/mtd/nand/mxc_nand.c > index 62d8c6b..4564b25 100644 > --- u-boot-4d3c95f.orig/drivers/mtd/nand/mxc_nand.c > +++ u-boot-4d3c95f/drivers/mtd/nand/mxc_nand.c > @@ -653,7 +653,7 @@ static int mxc_nand_correct_data(struct mtd_info *mtd, u_char *dat, > { > struct nand_chip *nand_chip = mtd->priv; > struct mxc_nand_host *host = nand_chip->priv; > - uint16_t ecc_status = readw(&host->regs->ecc_status_result); > + uint32_t ecc_status = readl(&host->regs->ecc_status_result); > int subpages = mtd->writesize / nand_chip->subpagesize; > int pg2blk_shift = nand_chip->phys_erase_shift - > nand_chip->page_shift; > diff --git u-boot-4d3c95f.orig/include/fsl_nfc.h u-boot-4d3c95f/include/fsl_nfc.h > index 1c16c71..c248cef 100644 > --- u-boot-4d3c95f.orig/include/fsl_nfc.h > +++ u-boot-4d3c95f/include/fsl_nfc.h > @@ -94,8 +94,7 @@ struct fsl_nfc_regs { > u16 flash_addr; > u16 flash_cmd; > u16 config; > - u16 ecc_status_result; > - u16 ecc_status_result2; > + u32 ecc_status_result; > u16 spare_area_size; > u16 wrprot; > u16 reserved3[2]; > diff --git u-boot-4d3c95f.orig/nand_spl/nand_boot_fsl_nfc.c u-boot-4d3c95f/nand_spl/nand_boot_fsl_nfc.c > index 842943c..4c5a7fe 100644 > --- u-boot-4d3c95f.orig/nand_spl/nand_boot_fsl_nfc.c > +++ u-boot-4d3c95f/nand_spl/nand_boot_fsl_nfc.c > @@ -139,7 +139,11 @@ static void nfc_nand_data_output(void) > > static int nfc_nand_check_ecc(void) > { > +#if defined(MXC_NFC_V1) > return readw(&nfc->ecc_status_result); > +#elif defined(MXC_NFC_V1_1) > + return readl(&nfc->ecc_status_result); > +#endif > } On MXC_NFC_V1 you're using readw() on a u32 -- that's not OK, even if the ARM accessors let you get away with it. Use a union or #ifdef the struct definition, or just always readl() if you can get away with it, or do two readw()s on V1_1. -Scott
On 09/17/2012 07:39:35 PM, Scott Wood wrote: > On Mon, Aug 13, 2012 at 10:49:42PM +0200, Benoît Thébaudeau wrote: > > On the NFC IP 1.1, the 32-bit ecc_status_result value comes from 2 > > consecutive 16-bit registers. This patch reads all the fields of > this value, > > which makes a difference for 4-kiB NF pages. > > > > Signed-off-by: Benoît Thébaudeau <benoit.thebaudeau@advansee.com> > > Cc: Scott Wood <scottwood@freescale.com> > > Cc: Stefano Babic <sbabic@denx.de> > > --- > > .../drivers/mtd/nand/mxc_nand.c | 2 +- > > .../include/fsl_nfc.h | 3 +-- > > .../nand_spl/nand_boot_fsl_nfc.c | 4 ++++ > > 3 files changed, 6 insertions(+), 3 deletions(-) > > > > diff --git u-boot-4d3c95f.orig/drivers/mtd/nand/mxc_nand.c > u-boot-4d3c95f/drivers/mtd/nand/mxc_nand.c > > index 62d8c6b..4564b25 100644 > > --- u-boot-4d3c95f.orig/drivers/mtd/nand/mxc_nand.c > > +++ u-boot-4d3c95f/drivers/mtd/nand/mxc_nand.c > > @@ -653,7 +653,7 @@ static int mxc_nand_correct_data(struct > mtd_info *mtd, u_char *dat, > > { > > struct nand_chip *nand_chip = mtd->priv; > > struct mxc_nand_host *host = nand_chip->priv; > > - uint16_t ecc_status = readw(&host->regs->ecc_status_result); > > + uint32_t ecc_status = readl(&host->regs->ecc_status_result); > > int subpages = mtd->writesize / nand_chip->subpagesize; > > int pg2blk_shift = nand_chip->phys_erase_shift - > > nand_chip->page_shift; > > diff --git u-boot-4d3c95f.orig/include/fsl_nfc.h > u-boot-4d3c95f/include/fsl_nfc.h > > index 1c16c71..c248cef 100644 > > --- u-boot-4d3c95f.orig/include/fsl_nfc.h > > +++ u-boot-4d3c95f/include/fsl_nfc.h > > @@ -94,8 +94,7 @@ struct fsl_nfc_regs { > > u16 flash_addr; > > u16 flash_cmd; > > u16 config; > > - u16 ecc_status_result; > > - u16 ecc_status_result2; > > + u32 ecc_status_result; > > u16 spare_area_size; > > u16 wrprot; > > u16 reserved3[2]; > > diff --git u-boot-4d3c95f.orig/nand_spl/nand_boot_fsl_nfc.c > u-boot-4d3c95f/nand_spl/nand_boot_fsl_nfc.c > > index 842943c..4c5a7fe 100644 > > --- u-boot-4d3c95f.orig/nand_spl/nand_boot_fsl_nfc.c > > +++ u-boot-4d3c95f/nand_spl/nand_boot_fsl_nfc.c > > @@ -139,7 +139,11 @@ static void nfc_nand_data_output(void) > > > > static int nfc_nand_check_ecc(void) > > { > > +#if defined(MXC_NFC_V1) > > return readw(&nfc->ecc_status_result); > > +#elif defined(MXC_NFC_V1_1) > > + return readl(&nfc->ecc_status_result); > > +#endif > > } > > On MXC_NFC_V1 you're using readw() on a u32 -- that's not OK, even if > the > ARM accessors let you get away with it. Use a union or #ifdef the > struct > definition, or just always readl() if you can get away with it, or do > two > readw()s on V1_1. Never mind, didn't notice that the struct definition is under #ifdef MXC_NFC_V1_1. The ARM accessors still should be fixed to do type checking... -Scott
diff --git u-boot-4d3c95f.orig/drivers/mtd/nand/mxc_nand.c u-boot-4d3c95f/drivers/mtd/nand/mxc_nand.c index 62d8c6b..4564b25 100644 --- u-boot-4d3c95f.orig/drivers/mtd/nand/mxc_nand.c +++ u-boot-4d3c95f/drivers/mtd/nand/mxc_nand.c @@ -653,7 +653,7 @@ static int mxc_nand_correct_data(struct mtd_info *mtd, u_char *dat, { struct nand_chip *nand_chip = mtd->priv; struct mxc_nand_host *host = nand_chip->priv; - uint16_t ecc_status = readw(&host->regs->ecc_status_result); + uint32_t ecc_status = readl(&host->regs->ecc_status_result); int subpages = mtd->writesize / nand_chip->subpagesize; int pg2blk_shift = nand_chip->phys_erase_shift - nand_chip->page_shift; diff --git u-boot-4d3c95f.orig/include/fsl_nfc.h u-boot-4d3c95f/include/fsl_nfc.h index 1c16c71..c248cef 100644 --- u-boot-4d3c95f.orig/include/fsl_nfc.h +++ u-boot-4d3c95f/include/fsl_nfc.h @@ -94,8 +94,7 @@ struct fsl_nfc_regs { u16 flash_addr; u16 flash_cmd; u16 config; - u16 ecc_status_result; - u16 ecc_status_result2; + u32 ecc_status_result; u16 spare_area_size; u16 wrprot; u16 reserved3[2]; diff --git u-boot-4d3c95f.orig/nand_spl/nand_boot_fsl_nfc.c u-boot-4d3c95f/nand_spl/nand_boot_fsl_nfc.c index 842943c..4c5a7fe 100644 --- u-boot-4d3c95f.orig/nand_spl/nand_boot_fsl_nfc.c +++ u-boot-4d3c95f/nand_spl/nand_boot_fsl_nfc.c @@ -139,7 +139,11 @@ static void nfc_nand_data_output(void) static int nfc_nand_check_ecc(void) { +#if defined(MXC_NFC_V1) return readw(&nfc->ecc_status_result); +#elif defined(MXC_NFC_V1_1) + return readl(&nfc->ecc_status_result); +#endif } static void nfc_nand_read_page(unsigned int page_address)
On the NFC IP 1.1, the 32-bit ecc_status_result value comes from 2 consecutive 16-bit registers. This patch reads all the fields of this value, which makes a difference for 4-kiB NF pages. Signed-off-by: Benoît Thébaudeau <benoit.thebaudeau@advansee.com> Cc: Scott Wood <scottwood@freescale.com> Cc: Stefano Babic <sbabic@denx.de> --- .../drivers/mtd/nand/mxc_nand.c | 2 +- .../include/fsl_nfc.h | 3 +-- .../nand_spl/nand_boot_fsl_nfc.c | 4 ++++ 3 files changed, 6 insertions(+), 3 deletions(-)