NAND ECC in linux-omap

[Vimal Singh]

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).


-- 
Regards,
Vimal Singh