Hi Jagdish,
On Tue, 20 Mar 2018 03:09:38 +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.
>
> Cc: stable@xxxxxxxxxxxxxxx
Can you add a Fixes tag here?
> 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
>
> drivers/mtd/nand/fsl_ifc_nand.c | 23 +++++++++++++----------
> 1 file changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/mtd/nand/fsl_ifc_nand.c b/drivers/mtd/nand/fsl_ifc_nand.c
> index 4872a7b..d9ce398 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 */
> @@ -237,10 +232,18 @@ 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 = 0;
>
> for (i = sector; i <= sector_end; i++) {
> +
> + /*
> + * if page size and sector size both are 512B then
> + * sector may not be multiple of 4 but sector and
> + * sector_end will be same.
> + */
I don't understand this comment.
> + if ((i % 4 == 0) || (sector == sector_end))
How about:
/*
* sector is not necessarily aligned on 4 and
* we need to read the eccstat in this case,
* hence the i == sector test.
*/
if (i == sector || !(i % 4))
> + eccstat = ifc_in32(&eccstat_regs[i / 4]);
> +
> errors = check_read_ecc(mtd, ctrl, eccstat, i);
>
> if (errors == 15) {
Regards,
Boris
--
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com