Hi Todd,

Thank you for review.
I attached recasted patch with the additional fix for wrong write in the 
OOB area.

The driver is tested with bonnie++ test tool:
This patches fixes the issue of the bug.
This is tested by bonnie++ test on NAND flash formatted with jffs2 fs:

root@192.168.0.8:~# flash_eraseall -j /dev/mtd4
Erasing 16 Kibyte @ 90c000 -- 56 % complete. Cleanmarker written at 90c000.
Skipping bad block at 0x00910000
Erasing 16 Kibyte @ ffc000 -- 99 % complete. Cleanmarker written at ffc000.
root@192.168.0.8:~# mount -t jffs2 /dev/mtdblock4 /mnt
root@192.168.0.8:~# mount
rootfs on / type rootfs (rw)
/dev/root on / type nfs 
(rw,v2,rsize=4096,wsize=4096,hard,udp,nolock,addr=192.168.0.1)
proc on /proc type proc (rw,nodiratime)
sysfs on /sys type sysfs (rw)
tmpfs on /tmp type tmpfs (rw)
tmpfs on /dev/shm type tmpfs (rw)
/dev/mtdblock4 on /mnt type jffs2 (rw,noatime)
root@192.168.0.8:~# cat /proc/mtd
dev:    size   erasesize  name
mtd0: 00004000 00004000 "microBTM"
mtd1: 0003c000 00004000 "bootloader"
mtd2: 005c0000 00004000 "ROMFS-Tools"
mtd3: 00a00000 00004000 "ROMFS-User"
mtd4: 01000000 00004000 "User"
root@192.168.0.8:~#
root@192.168.0.8:~# bonnie++ -u root -d /mnt -s 16 -r 0
Using uid:0, gid:0.
Writing with putc()...done
Writing intelligently...done
Rewriting...done
Reading with getc()...mtd->read(0x78 bytes from 0x64154) returned ECC error
done
Reading intelligently...done
start 'em...done...done...done...
Create files in sequential order...done.
Stat files in sequential order...done.
Delete files in sequential order...done.
Create files in random order...done.
Stat files in random order...done.
Delete files in random order...done.
Version  1.03       ------Sequential Output------ --Sequential Input- 
--Random-
                    -Per Chr- --Block-- -Rewrite- -Per Chr- --Block-- 
--Seeks--
Machine        Size K/sec %CP K/sec %CP K/sec %CP K/sec %CP K/sec %CP  
/sec %CP
192.168.0.8     16M   286  99  2379  99  2253  98   357  99 +++++ +++  
1961  98
                    ------Sequential Create------ --------Random 
Create--------
                    -Create-- --Read--- -Delete-- -Create-- --Read--- 
-Delete--
              files  /sec %CP  /sec %CP  /sec %CP  /sec %CP  /sec %CP  
/sec %CP
                 16   285  99  3013  99   292  97   275  95  3199 100   
295  99
192.168.0.8,16M,286,99,2379,99,2253,98,357,99,+++++,+++,1960.8,98,16,285,99,3013,99,292,97,275,95,3199,100,295,99
root@192.168.0.8:~#

Vladimir

Todd Poynor wrote:

> Vladimir A. Barinov wrote:
>
>> Hi All,
>>
>> Attached patch is NAND flash driver for PNX8550 based platforms.
>> Any comments and suggestions are highly appreciated.
>>
>> Vladimir
>>
>>
>> +    if ((u32) from & 3) {
>> +        printk
>> +            ("%s: from buffer not 32bit aligned, will not use 
>> fastest transfer mechanism\n",
>> +             __FUNCTION__);
>> +    }
>> +    if ((u32) to & 3) {
>> +        printk
>> +            ("%s: to buffer not 32bit aligned, will not use fastest 
>> transfer mechanism\n",
>> +             __FUNCTION__);
>
>
> Those printks could get old pretty fast.  Debugging info, not needed 
> for normal operation.

Thanks. Removed in the attached patch.

>
>> +    }
>> +
>> +    if (((bytes & 3) || (bytes < 16)) || ((u32) to & 3) || ((u32) 
>> from & 3)) {
>> +        if (((bytes & 1) == 0) &&
>> +            (((u32) to & 1) == 0) && (((u32) from & 1) == 0)) {
>> +            int words = bytes / 2;
>> +
>> +            local_irq_disable();
>> +            for (i = 0; i < words; i++) {
>> +                to16[i] = from16[i];
>> +            }
>> +            local_irq_enable();
>
>
> Really necessary to disable all irqs around this transfer?  How long 
> can interrupts be off during that time?

That is needed since the NAND device is binded to the external XIO bus 
that could be used by another devices.

>
>
>> +
>> +    local_irq_disable();
>> +    PNX8550_DMA_TRANS_SIZE = bytes >> 2;    /* Length in words */
>> +    PNX8550_DMA_EXT_ADDR = external;
>> +    PNX8550_DMA_INT_ADDR = internal;
>> +    PNX8550_DMA_INT_CLEAR = 0xffff;
>> +    PNX8550_DMA_CTRL = PNX8550_DMA_CTRL_BURST_512 |
>> +        PNX8550_DMA_CTRL_SND2XIO | PNX8550_DMA_CTRL_INIT_DMA | cmd;
>> +
>> +    while ((PNX8550_DMA_INT_STATUS & PNX8550_DMA_INT_COMPL) == 0) ;
>> +
>> +    if (!toxio) {
>> +        dma_cache_inv(to, bytes);
>> +    }
>> +    local_irq_enable();
>
>
> Again, necessary to prevent interrupts?
>
>> +} +    /* Scan to find existence of the device */
>> +    if (nand_scan(&pnx8550_mtd, 1)) {
>> +        printk("%s: Exiting No Devices\n", __FUNCTION__);
>> +        return -ENXIO;
>
>
> Need kfree(transferBuffer)
>
>> +    }
>> +
>> +    /* Register the partitions */
>> +    add_mtd_partitions(&pnx8550_mtd, partition_info, NUM_PARTITIONS);
>
>
> Need Kconfig to select MTD_PARTITIONS if required.

Added "ifdef". Thanks.

>
>> +static void __exit pnx8550_nand_cleanup(void)
>> +{
>> +    /* Unregister the device */
>> +    del_mtd_device(&pnx8550_mtd);
>
>
> Need del_mtd_partitions I think?

No. That is in the del_mtd_device().

>
>> +    if (transferBuffer) {
>> +        kfree(transferBuffer);
>
>
> "if (transferBuffer)" not needed and is discouraged

Already removed.