NAND ECC in linux-omap

[Ghorai, Sukumar]

Vimal,

> -----Original Message-----
> From: Vimal Singh [mailto:vimal.newwork at gmail.com]
> Sent: Saturday, August 28, 2010 2:07 AM
> To: Cliff Brake
> Cc: Ghorai, Sukumar; Kamat, Nishant; linux-omap at vger.kernel.org; Linux MTD
> Subject: Re: NAND ECC in linux-omap
> 
> Adding LO and MTD list too for more comments.
> 
> On Sat, Aug 28, 2010 at 1:40 AM, Vimal Singh <vimal.newwork at gmail.com>
> wrote:
> > On Sat, Aug 28, 2010 at 12:00 AM, Cliff Brake <cliff.brake at gmail.com>
> wrote:
> >> On Fri, Aug 27, 2010 at 2:29 PM, Cliff Brake <cliff.brake at gmail.com>
> wrote:
> >>> On Fri, Aug 27, 2010 at 11:13 AM, Ghorai, Sukumar <s-ghorai at ti.com>
> wrote:
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: Vimal Singh [mailto:vimal.newwork at gmail.com]
> >>>>> Sent: Tuesday, August 24, 2010 10:41 PM
> >>>>> To: Cliff Brake
> >>>>> Cc: Ghorai, Sukumar; Linux OMAP Users; Kamat, Nishant
> >>>>> Subject: Re: NAND ECC in linux-omap
> >>>>>
> >>>>> On Tue, Aug 24, 2010 at 8:04 PM, Cliff Brake <cliff.brake at gmail.com>
> >>>>> wrote:
> >>>>> >
> >>>>> > flash_erasall -j /dev/mtd4
> >>>>>
> >>>>> Give a try without using '-j' option... If ECC layout is the
> suspect..
> >>>>> this may help.
> >>>> [Ghorai]
> >>>> Cliff,
> >>>> I did not able to spend much time on it to get into the problem.
> >>>> But would you please try using these additional patch(4) that yet to
> upstream?
> >>>> https://patchwork.kernel.org/patch/116554/
> >>>> https://patchwork.kernel.org/patch/116553/
> >>>> https://patchwork.kernel.org/patch/116555/
> >>>> https://patchwork.kernel.org/patch/116556/
> >>>> And select/change the proper ECC in -
> >>>> arch/arm/mach-omap2/board-flash.c: line No.148;
> >>>> board_nand_data.ecc_opt         = OMAP_ECC_HAMMING_CODE_HW;
> >>>
> >>> Thanks for the patches.  Now I get the following:
> >>>
> >>> root at ts3:~# flash_eraseall /dev/mtd1
> >>> Erasing 128 Kibyte @ 1c0000 -- 100 % complete.
> >>> root at ts3:~# nandwrite -p /dev/mtd1 /media/mmcblk0p1/u-boot.bin
> >>> Writing data to block 0 at offset 0x0
> >>>
> >>> And the operation hangs.
> >
> > Drop these patches. Please just give one more try after reverting last
> > 2 commits in omap2.c driver:
> > 1. omap3 nand: cleanup virtual address usages
> > and
> > 2. omap3 gpmc: functionality enhancement
> >
> > I suspect something wrong with patch '2'. I will look into it more.
> 
> I can see the problem in '1' patch only. In this patch, see below change:
> 
> ------------------------------------------
> 
> static void omap_hwcontrol(struct mtd_info *mtd, int cmd, unsigned int
> ctrl)
>  {
>         struct omap_nand_info *info = container_of(mtd,
>                                         struct omap_nand_info, mtd);
> -       switch (ctrl) {
> -       case NAND_CTRL_CHANGE | NAND_CTRL_CLE:
> -               info->nand.IO_ADDR_W = info->gpmc_cs_baseaddr +
> -                                               GPMC_CS_NAND_COMMAND;
> -               info->nand.IO_ADDR_R = info->gpmc_cs_baseaddr +
> -                                               GPMC_CS_NAND_DATA;
> -               break;
> -
> -       case NAND_CTRL_CHANGE | NAND_CTRL_ALE:
> -               info->nand.IO_ADDR_W = info->gpmc_cs_baseaddr +
> -                                               GPMC_CS_NAND_ADDRESS;
> -               info->nand.IO_ADDR_R = info->gpmc_cs_baseaddr +
> -                                               GPMC_CS_NAND_DATA;
> -               break;
> -
> -       case NAND_CTRL_CHANGE | NAND_NCE:
> -               info->nand.IO_ADDR_W = info->gpmc_cs_baseaddr +
> -                                               GPMC_CS_NAND_DATA;
> -               info->nand.IO_ADDR_R = info->gpmc_cs_baseaddr +
> -                                               GPMC_CS_NAND_DATA;
> -               break;
> -       }
> 
> -       if (cmd != NAND_CMD_NONE)
> -               __raw_writeb(cmd, info->nand.IO_ADDR_W);
> +       if (cmd != NAND_CMD_NONE) {
> +               if (ctrl & NAND_CLE)
> +                       gpmc_nand_write(info->gpmc_cs, GPMC_NAND_COMMAND,
> cmd);
> +
> +               else if (ctrl & NAND_ALE)
> +                       gpmc_nand_write(info->gpmc_cs, GPMC_NAND_ADDRESS,
> cmd);
> +
> +               else /* NAND_NCE */
> +                       gpmc_nand_write(info->gpmc_cs, GPMC_NAND_DATA,
> cmd);
> +       }
>  }
> 
> ---------------------------------------------
> 
> The new (changed) hwcontrol routine still can latch command and
> address to correct gpmc nand registers. But it is failing to set '
> info->nand.IO_ADDR_W' and 'info->nand.IO_ADDR_R'.
> Which will be used by write/read routines to:  write to (IO_ADDR_W) or
> read from (IO_ADDR_R).
[Ghorai]
1. You mean revert the old code will solve the issue mentioned by Cliff? 
2. Can you check if the problem already exists before applying these patch itself?
 
> 
> 
> --
> Regards,
> Vimal Singh