On Wed, 21 Mar 2018 09:55:35 +0100
Boris Brezillon <boris.brezillon@xxxxxxxxxxx> wrote:
> On Wed, 21 Mar 2018 05:51:46 +0530
> Jagdish Gediya <jagdish.gediya@xxxxxxx> wrote:
>
> > 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.
> >
> > Fixes: bccb06c353af ("mtd: nand: ifc: update bufnum mask for ver >= 2.0.0")
> > Cc: stable@xxxxxxxxxxxxxxx # 3.18+
> > Signed-off-by: Prabhakar Kushwaha <prabhakar.kushwaha@xxxxxxx>
> > Signed-off-by: Jagdish Gediya <jagdish.gediya@xxxxxxx>
> > ---
> > Changes for v2: Incorporated comments from Miquel Raynal and Boris Brezillon
> > - Updated patch subject
> > - Remove usage of eccstat array
> > - Added Cc: stable@xxxxxxxxxxxxxxx
> >
> > Changes for v3: Incorporated comments from Boris Brezillon
> > - Added fixes tag
> >
> > Changes for v4: Incorporated comments from Boris Brezillon
> >
> > drivers/mtd/nand/fsl_ifc_nand.c | 23 ++++++++++-------------
> > 1 file changed, 10 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/mtd/nand/fsl_ifc_nand.c b/drivers/mtd/nand/fsl_ifc_nand.c
> > index 4872a7b..9a01309 100644
> > --- a/drivers/mtd/nand/fsl_ifc_nand.c
> > +++ b/drivers/mtd/nand/fsl_ifc_nand.c
> > @@ -173,14 +173,9 @@ static void set_addr(struct mtd_info *mtd, int column, int page_addr, int oob)
> >
> > /* 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;
> > }
> >
> > /*
> > @@ -193,7 +188,7 @@ static void fsl_ifc_run_command(struct mtd_info *mtd)
> > struct fsl_ifc_ctrl *ctrl = priv->ctrl;
> > struct fsl_ifc_nand_ctrl *nctrl = ifc_nand_ctrl;
> > struct fsl_ifc_runtime __iomem *ifc = ctrl->rregs;
> > - u32 eccstat[4];
> > + u32 eccstat;
> > int i;
> >
> > /* set the chip select for NAND Transaction */
> > @@ -228,8 +223,8 @@ static void fsl_ifc_run_command(struct mtd_info *mtd)
> > if (nctrl->eccread) {
> > int errors;
> > int bufnum = nctrl->page & priv->bufnum_mask;
> > - int sector = bufnum * chip->ecc.steps;
> > - int sector_end = sector + chip->ecc.steps - 1;
> > + int sector_start = bufnum * chip->ecc.steps;
> > + int sector_end = sector_start + chip->ecc.steps - 1;
> > __be32 *eccstat_regs;
> >
> > if (ctrl->version >= FSL_IFC_VERSION_2_0_0)
> > @@ -237,10 +232,12 @@ static void fsl_ifc_run_command(struct mtd_info *mtd)
> > else
> > eccstat_regs = ifc->ifc_nand.v1_nand_eccstat;
> >
> > - for (i = sector / 4; i <= sector_end / 4; i++)
> > - eccstat[i] = ifc_in32(&eccstat_regs[i]);
> > + 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]);
>
> So now you're reading eccstat_regs[sector_start / 4] twice if
> sector_start is aligned on 4. Why don't you want the test I proposed
> in my last review?
>
> if (i != sector_start && !(i % 4))
>
Applied with this adjustment.
Thanks,
Boris
> >
> > - for (i = sector; i <= sector_end; i++) {
> > errors = check_read_ecc(mtd, ctrl, eccstat, i);
> >
> > if (errors == 15) {
>
>
>
--
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com