All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH][v2] mtd: nand: fsl_ifc: Fix eccstat array overflow for IFC ver >= 2.0.0
@ 2018-03-21 17:32 Jagdish Gediya
  2018-03-21 17:32 ` [U-Boot] [PATCH][v2] mtd: nand: fsl_ifc: Fix nand waitfunc return value Jagdish Gediya
  0 siblings, 1 reply; 6+ messages in thread
From: Jagdish Gediya @ 2018-03-21 17:32 UTC (permalink / raw)
  To: u-boot

Number of ECC status registers i.e. (ECCSTATx) has been increased in IFC
version 2.0.0 due to increase in SRAM size. This is causing eccstat
array to over flow.

So, replace eccstat array with u32 variable to make it fail-safe and
independent of number of ECC status registers or SRAM size.

Signed-off-by: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
Signed-off-by: Jagdish Gediya <jagdish.gediya@nxp.com>
---
Changes for v2:
	- Resolve checkpatch error
	- Give suitable name to variable and do proper initialization.

 drivers/mtd/nand/fsl_ifc_nand.c | 32 ++++++++++++--------------------
 include/fsl_ifc.h               |  4 ++--
 2 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/drivers/mtd/nand/fsl_ifc_nand.c b/drivers/mtd/nand/fsl_ifc_nand.c
index d1165f7..6eb44c3 100644
--- a/drivers/mtd/nand/fsl_ifc_nand.c
+++ b/drivers/mtd/nand/fsl_ifc_nand.c
@@ -270,14 +270,9 @@ static int is_blank(struct mtd_info *mtd, struct fsl_ifc_ctrl *ctrl,
 
 /* returns nonzero if entire page is blank */
 static int check_read_ecc(struct mtd_info *mtd, struct fsl_ifc_ctrl *ctrl,
-			  u32 *eccstat, unsigned int bufnum)
+			  u32 eccstat, unsigned int bufnum)
 {
-	u32 reg = eccstat[bufnum / 4];
-	int errors;
-
-	errors = (reg >> ((3 - bufnum % 4) * 8)) & 15;
-
-	return errors;
+	return (eccstat >> ((3 - bufnum % 4) * 8)) & 15;
 }
 
 /*
@@ -291,7 +286,7 @@ static int fsl_ifc_run_command(struct mtd_info *mtd)
 	struct fsl_ifc_runtime *ifc = ctrl->regs.rregs;
 	u32 timeo = (CONFIG_SYS_HZ * 10) / 1000;
 	u32 time_start;
-	u32 eccstat[8] = {0};
+	u32 eccstat;
 	int i;
 
 	/* set the chip select for NAND Transaction */
@@ -321,20 +316,17 @@ static int fsl_ifc_run_command(struct mtd_info *mtd)
 	if (ctrl->eccread) {
 		int errors;
 		int bufnum = ctrl->page & priv->bufnum_mask;
-		int sector = bufnum * chip->ecc.steps;
-		int sector_end = sector + chip->ecc.steps - 1;
-
-		for (i = sector / 4; i <= sector_end / 4; i++) {
-			if (i >= ARRAY_SIZE(eccstat)) {
-				printf("%s: eccstat too small for %d\n",
-				       __func__, i);
-				return -EIO;
-			}
+		int sector_start = bufnum * chip->ecc.steps;
+		int sector_end = sector_start + chip->ecc.steps - 1;
+		u32 *eccstat_regs;
 
-			eccstat[i] = ifc_in32(&ifc->ifc_nand.nand_eccstat[i]);
-		}
+		eccstat_regs = ifc->ifc_nand.nand_eccstat;
+		eccstat = ifc_in32(&eccstat_regs[sector_start / 4]);
+
+		for (i = sector_start; i <= sector_end; i++) {
+			if (!(i % 4))
+				eccstat = ifc_in32(&eccstat_regs[i / 4]);
 
-		for (i = sector; i <= sector_end; i++) {
 			errors = check_read_ecc(mtd, ctrl, eccstat, i);
 
 			if (errors == 15) {
diff --git a/include/fsl_ifc.h b/include/fsl_ifc.h
index 29aa687..4bff019 100644
--- a/include/fsl_ifc.h
+++ b/include/fsl_ifc.h
@@ -892,8 +892,8 @@ struct fsl_ifc_nand {
 	u32 nand_erattr1;
 	u32 res19[0x10];
 	u32 nand_fsr;
-	u32 res20[0x3];
-	u32 nand_eccstat[6];
+	u32 res20[0x1];
+	u32 nand_eccstat[8];
 	u32 res21[0x1c];
 	u32 nanndcr;
 	u32 res22[0x2];
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH][v2] mtd: nand: fsl_ifc: Fix nand waitfunc return value
  2018-03-21 17:32 [U-Boot] [PATCH][v2] mtd: nand: fsl_ifc: Fix eccstat array overflow for IFC ver >= 2.0.0 Jagdish Gediya
@ 2018-03-21 17:32 ` Jagdish Gediya
  2018-04-26 21:01   ` York Sun
  0 siblings, 1 reply; 6+ messages in thread
From: Jagdish Gediya @ 2018-03-21 17:32 UTC (permalink / raw)
  To: u-boot

As per the IFC hardware manual, Most significant 2 bytes in
nand_fsr register are the outcome of NAND READ STATUS command.

So status value need to be shifted and aligned as per the nand
framework requirement.

Signed-off-by: Jagdish Gediya <jagdish.gediya@nxp.com>
Reviewed-by: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
---
Changes for v2:
	- Change the waitfunc return value according to semantic
	  enforced by framework.

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

diff --git a/drivers/mtd/nand/fsl_ifc_nand.c b/drivers/mtd/nand/fsl_ifc_nand.c
index 6eb44c3..7f487e7 100644
--- a/drivers/mtd/nand/fsl_ifc_nand.c
+++ b/drivers/mtd/nand/fsl_ifc_nand.c
@@ -701,6 +701,7 @@ static int fsl_ifc_wait(struct mtd_info *mtd, struct nand_chip *chip)
 	struct fsl_ifc_ctrl *ctrl = priv->ctrl;
 	struct fsl_ifc_runtime *ifc = ctrl->regs.rregs;
 	u32 nand_fsr;
+	int status;
 
 	if (ctrl->status != IFC_NAND_EVTER_STAT_OPC)
 		return NAND_STATUS_FAIL;
@@ -721,10 +722,10 @@ static int fsl_ifc_wait(struct mtd_info *mtd, struct nand_chip *chip)
 		return NAND_STATUS_FAIL;
 
 	nand_fsr = ifc_in32(&ifc->ifc_nand.nand_fsr);
+	status = nand_fsr >> 24;
 
 	/* Chip sometimes reporting write protect even when it's not */
-	nand_fsr = nand_fsr | NAND_STATUS_WP;
-	return nand_fsr;
+	return status | NAND_STATUS_WP;
 }
 
 static int fsl_ifc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH][v2] mtd: nand: fsl_ifc: Fix nand waitfunc return value
  2018-03-21 17:32 ` [U-Boot] [PATCH][v2] mtd: nand: fsl_ifc: Fix nand waitfunc return value Jagdish Gediya
@ 2018-04-26 21:01   ` York Sun
  2018-04-27  2:58     ` Prabhakar Kushwaha
  0 siblings, 1 reply; 6+ messages in thread
From: York Sun @ 2018-04-26 21:01 UTC (permalink / raw)
  To: u-boot

On 03/20/2018 10:24 PM, Jagdish Gediya wrote:
> As per the IFC hardware manual, Most significant 2 bytes in
> nand_fsr register are the outcome of NAND READ STATUS command.
> 
> So status value need to be shifted and aligned as per the nand
> framework requirement.
> 
> Signed-off-by: Jagdish Gediya <jagdish.gediya@nxp.com>
> Reviewed-by: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
> ---
> Changes for v2:
> 	- Change the waitfunc return value according to semantic
> 	  enforced by framework.
> 
>  drivers/mtd/nand/fsl_ifc_nand.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/nand/fsl_ifc_nand.c b/drivers/mtd/nand/fsl_ifc_nand.c
> index 6eb44c3..7f487e7 100644
> --- a/drivers/mtd/nand/fsl_ifc_nand.c
> +++ b/drivers/mtd/nand/fsl_ifc_nand.c
> @@ -701,6 +701,7 @@ static int fsl_ifc_wait(struct mtd_info *mtd, struct nand_chip *chip)
>  	struct fsl_ifc_ctrl *ctrl = priv->ctrl;
>  	struct fsl_ifc_runtime *ifc = ctrl->regs.rregs;
>  	u32 nand_fsr;
> +	int status;
>  
>  	if (ctrl->status != IFC_NAND_EVTER_STAT_OPC)
>  		return NAND_STATUS_FAIL;
> @@ -721,10 +722,10 @@ static int fsl_ifc_wait(struct mtd_info *mtd, struct nand_chip *chip)
>  		return NAND_STATUS_FAIL;
>  
>  	nand_fsr = ifc_in32(&ifc->ifc_nand.nand_fsr);
> +	status = nand_fsr >> 24;
>  

You said most significant 2 bytes are the outcome, and you shift 24
bits. Did you intentionally use one byte?

York

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH][v2] mtd: nand: fsl_ifc: Fix nand waitfunc return value
  2018-04-26 21:01   ` York Sun
@ 2018-04-27  2:58     ` Prabhakar Kushwaha
  2018-04-27  3:12       ` York Sun
  0 siblings, 1 reply; 6+ messages in thread
From: Prabhakar Kushwaha @ 2018-04-27  2:58 UTC (permalink / raw)
  To: u-boot

Hi York,

> -----Original Message-----
> From: U-Boot [mailto:u-boot-bounces at lists.denx.de] On Behalf Of York Sun
> Sent: Friday, April 27, 2018 2:31 AM
> To: Jagdish Gediya <jagdish.gediya@nxp.com>; u-boot at lists.denx.de
> Cc: oss at buserror.net
> Subject: Re: [U-Boot] [PATCH][v2] mtd: nand: fsl_ifc: Fix nand waitfunc
> return value
> 
> On 03/20/2018 10:24 PM, Jagdish Gediya wrote:
> > As per the IFC hardware manual, Most significant 2 bytes in nand_fsr
> > register are the outcome of NAND READ STATUS command.
> >
> > So status value need to be shifted and aligned as per the nand
> > framework requirement.
> >
> > Signed-off-by: Jagdish Gediya <jagdish.gediya@nxp.com>
> > Reviewed-by: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
> > ---
> > Changes for v2:
> > 	- Change the waitfunc return value according to semantic
> > 	  enforced by framework.
> >
> >  drivers/mtd/nand/fsl_ifc_nand.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mtd/nand/fsl_ifc_nand.c
> > b/drivers/mtd/nand/fsl_ifc_nand.c index 6eb44c3..7f487e7 100644
> > --- a/drivers/mtd/nand/fsl_ifc_nand.c
> > +++ b/drivers/mtd/nand/fsl_ifc_nand.c
> > @@ -701,6 +701,7 @@ static int fsl_ifc_wait(struct mtd_info *mtd, struct
> nand_chip *chip)
> >  	struct fsl_ifc_ctrl *ctrl = priv->ctrl;
> >  	struct fsl_ifc_runtime *ifc = ctrl->regs.rregs;
> >  	u32 nand_fsr;
> > +	int status;
> >
> >  	if (ctrl->status != IFC_NAND_EVTER_STAT_OPC)
> >  		return NAND_STATUS_FAIL;
> > @@ -721,10 +722,10 @@ static int fsl_ifc_wait(struct mtd_info *mtd, struct
> nand_chip *chip)
> >  		return NAND_STATUS_FAIL;
> >
> >  	nand_fsr = ifc_in32(&ifc->ifc_nand.nand_fsr);
> > +	status = nand_fsr >> 24;
> >
> 
> You said most significant 2 bytes are the outcome, and you shift 24 bits. Did
> you intentionally use one byte?
> 

Even though there are 2 bytes. 
But NAND sub-system only consume 1 byte because all NAND flash has 1 byte status register. 

--pk  

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH][v2] mtd: nand: fsl_ifc: Fix nand waitfunc return value
  2018-04-27  2:58     ` Prabhakar Kushwaha
@ 2018-04-27  3:12       ` York Sun
  2018-05-01  8:40         ` Jagdish Gediya
  0 siblings, 1 reply; 6+ messages in thread
From: York Sun @ 2018-04-27  3:12 UTC (permalink / raw)
  To: u-boot


> On Apr 26, 2018, at 19:58, Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com> wrote:
>> 

<snip>

> 
> Even though there are 2 bytes. 
> But NAND sub-system only consume 1 byte because all NAND flash has 1 byte status register. 
> 

Then the commit message should describe the position of primary status field, instead of saying two bytes.

York

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH][v2] mtd: nand: fsl_ifc: Fix nand waitfunc return value
  2018-04-27  3:12       ` York Sun
@ 2018-05-01  8:40         ` Jagdish Gediya
  0 siblings, 0 replies; 6+ messages in thread
From: Jagdish Gediya @ 2018-05-01  8:40 UTC (permalink / raw)
  To: u-boot

Hi York,

> -----Original Message-----
> From: York Sun
> Sent: Friday, April 27, 2018 8:43 AM
> To: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
> Cc: Jagdish Gediya <jagdish.gediya@nxp.com>; u-boot at lists.denx.de;
> oss at buserror.net
> Subject: Re: [U-Boot] [PATCH][v2] mtd: nand: fsl_ifc: Fix nand waitfunc return
> value
> 
> 
> > On Apr 26, 2018, at 19:58, Prabhakar Kushwaha
> <prabhakar.kushwaha@nxp.com> wrote:
> >>
> 
> <snip>
> 
> >
> > Even though there are 2 bytes.
> > But NAND sub-system only consume 1 byte because all NAND flash has 1 byte
> status register.
> >
> 
> Then the commit message should describe the position of primary status field,
> instead of saying two bytes.
I will send the new revision of the patch.

Thanks,
Jagdish

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-05-01  8:40 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-21 17:32 [U-Boot] [PATCH][v2] mtd: nand: fsl_ifc: Fix eccstat array overflow for IFC ver >= 2.0.0 Jagdish Gediya
2018-03-21 17:32 ` [U-Boot] [PATCH][v2] mtd: nand: fsl_ifc: Fix nand waitfunc return value Jagdish Gediya
2018-04-26 21:01   ` York Sun
2018-04-27  2:58     ` Prabhakar Kushwaha
2018-04-27  3:12       ` York Sun
2018-05-01  8:40         ` Jagdish Gediya

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.