Patchwork [U-Boot,06/13] mxc nand: Access all ecc_status_result fields

login
register
mail settings
Submitter Benoît Thébaudeau
Date Aug. 13, 2012, 8:49 p.m.
Message ID <1372434962.2374989.1344890982845.JavaMail.root@advansee.com>
Download mbox | patch
Permalink /patch/177090/
State Accepted
Commit c1db8dd62b337372a08942e1c5945a8590afbc58
Delegated to: Scott Wood
Headers show

Comments

Benoît Thébaudeau - Aug. 13, 2012, 8:49 p.m.
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(-)
Scott Wood - Sept. 18, 2012, 12:39 a.m.
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
Scott Wood - Sept. 18, 2012, 12:50 a.m.
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

Patch

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)