[PATH] NAND Flash support for Intel IXP4xx platform

[PATH] NAND Flash support for Intel IXP4xx platform

[Ruslan V. Sushko]

The patch adds glue logic to support MTD for NAND flash devices on Intel 
IXP4xx platform

Signed-off-by: rsushko@rus.mvista.com

--------

Index: linux-2.6/drivers/mtd/nand/Kconfig
===================================================================
--- linux-2.6.orig/drivers/mtd/nand/Kconfig
+++ linux-2.6/drivers/mtd/nand/Kconfig
@@ -143,6 +143,13 @@ config MTD_NAND_S3C2410_CLKSTOP
          when the is NAND chip selected or released, but will save
          approximately 5mA of power when there is nothing happening.

+config MTD_NAND_IXP4XX
+       tristate "Support for NAND Flash on Intel IXP4XX based systems"
+       depends on MTD_NAND && ARCH_IXP4XX
+       help
+         This enable the use of NAND flash on Intel IXP400 based platforms.
+         Currently this is only supported on KIXRP435 platform.
+
 config MTD_NAND_DISKONCHIP
        tristate "DiskOnChip 2000, Millennium and Millennium Plus (NAND 
reimplementation) (EXPERIMENTAL)"
        depends on MTD_NAND && EXPERIMENTAL
Index: linux-2.6/drivers/mtd/nand/Makefile
===================================================================
--- linux-2.6.orig/drivers/mtd/nand/Makefile
+++ linux-2.6/drivers/mtd/nand/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_MTD_NAND_CS553X)         += cs553x
 obj-$(CONFIG_MTD_NAND_NDFC)            += ndfc.o
 obj-$(CONFIG_MTD_NAND_AT91)            += at91_nand.o
 obj-$(CONFIG_MTD_NAND_BASLER_EXCITE)   += excite_nandflash.o
+obj-$(CONFIG_MTD_NAND_IXP4XX)          += ixp4xx_nand.o

 nand-objs := nand_base.o nand_bbt.o
 cafe_nand-objs := cafe.o cafe_ecc.o
Index: linux-2.6/drivers/mtd/nand/ixp4xx_nand.c
===================================================================
--- /dev/null
+++ linux-2.6/drivers/mtd/nand/ixp4xx_nand.c
@@ -0,0 +1,217 @@
+/*
+ * drivers/mtd/nand/ixp4xx_nand.c
+ *
+ * Copyright (C) 2006 Intel Corporation.
+ * Copyright (C) 2007 MontaVista Software, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/slab.h>
+#include <linux/init.h>
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/nand.h>
+#include <linux/mtd/partitions.h>
+
+#include <asm/io.h>
+#include <asm/arch/nand.h>
+#include <asm/hardware.h>
+#include <asm/delay.h>
+
+/*
+ * MTD structure for NAND controller
+ */
+static struct mtd_info *ixp4xx_mtd = NULL;
+static struct mtd_partition *ixp4xx_nand_parts = NULL;
+const char *part_probes[] = { "cmdlinepart", NULL };
+
+static struct ixp4xx_faddr_info_t {
+       int offset;
+       void (*chip_select) (unsigned int ctrl);
+} ixp4xx_faddr_info;
+
+/**
+ * ixp_write_buf -  write buffer to chip
+ * @mtd:       MTD device structure
+ * @buf:       data buffer
+ * @len:       number of bytes to write
+ *
+ *  write function for 8bit buswith
+ */
+static void ixp_write_buf(struct mtd_info *mtd, const u_char * buf, int 
len)
+{
+       int i;
+       struct nand_chip *this = mtd->priv;
+
+       for (i = 0; i < len; i++)
+               writeb(buf[i], this->IO_ADDR_W);
+}
+
+static void ixp4xx_hwcontrol(struct mtd_info *mtd, int cmd, unsigned 
int ctrl)
+{
+       struct nand_chip *this = mtd->priv;
+       struct ixp4xx_faddr_info_t *addr_info = this->priv;
+       if (ctrl & NAND_CTRL_CHANGE) {
+               addr_info->offset = (ctrl & NAND_CLE) ? IXP4XX_NAND_CLE : 0;
+               addr_info->offset |= (ctrl & NAND_ALE) ? IXP4XX_NAND_ALE 
: 0;
+               if (addr_info->chip_select)
+                       addr_info->chip_select(ctrl);
+       }
+
+       if (cmd != NAND_CMD_NONE)
+               writeb(cmd, this->IO_ADDR_W + addr_info->offset);
+}
+
+static int ixp4xx_nand_flash_probe(struct platform_device *dev)
+{
+       int err;
+       int nb_of_parts;
+       struct nand_chip *this;
+       void __iomem *nand_io_base;
+       struct ixp4xx_nand_platform_data *plat = dev->dev.platform_data;
+
+       if (!plat)
+               return -ENODEV;
+
+       if (plat->width != 1) {
+               printk(KERN_ERR "%s: %d bits bus width is not support for "
+                      "ixp4xx driver\n", __FUNCTION__, plat->width * 8);
+               return -EINVAL;
+       }
+
+       if (plat->init) {
+               err = plat->init();
+               if (err)
+                       return err;
+       }
+
+       /* Allocate memory for MTD device structure and private data */
+       ixp4xx_mtd = kmalloc(sizeof(struct mtd_info) + sizeof(struct 
nand_chip),
+                            GFP_KERNEL);
+
+       if (ixp4xx_mtd == NULL) {
+               printk(KERN_ERR "Fail on mem alloc for NAND MTD dev\n");
+               return -ENOMEM;
+       }
+
+       /* Initialize structures and build the linkage to private data */
+       memset(ixp4xx_mtd, 0,
+              sizeof(struct mtd_info) + sizeof(struct nand_chip));
+       this = (struct nand_chip *)(&ixp4xx_mtd[1]);
+
+       ixp4xx_mtd->priv = this;
+
+       ixp4xx_mtd->owner = THIS_MODULE;
+       ixp4xx_mtd->name = "ixp4xx-nand";
+
+       /* ioremap the io of NAND Flash */
+       nand_io_base = ioremap(dev->resource->start,
+                              dev->resource->end - dev->resource->start 
+ 1);
+       if (!nand_io_base) {
+               printk(KERN_ERR "Unable to map nand flash IO space\n");
+               err = -EIO;
+               goto error;
+       }
+
+       ixp4xx_faddr_info.offset = 0;
+       ixp4xx_faddr_info.chip_select = plat->chip_select;
+
+       /* Set address of hardware control function */
+       this->cmd_ctrl = ixp4xx_hwcontrol;
+       this->dev_ready = NULL; /* KIXRP435 has no GPIO pins route to R/B */
+       this->chip_delay = 30;  /* 30 us command delay time */
+       this->ecc.mode = NAND_ECC_SOFT;
+       this->options = NAND_NO_AUTOINCR;
+       this->IO_ADDR_R = nand_io_base;
+       this->IO_ADDR_W = nand_io_base;
+       this->priv = &ixp4xx_faddr_info;
+       this->write_buf = ixp_write_buf;
+
+       /* Scan to find existence of the device */
+       if (nand_scan(ixp4xx_mtd, 1)) {
+               err = -ENXIO;
+               goto error;
+       }
+
+       nb_of_parts = parse_mtd_partitions(ixp4xx_mtd, part_probes,
+                                          &ixp4xx_nand_parts, 0);
+
+       if (nb_of_parts <= 0) { /* No partition from parsing, use default */
+               printk(KERN_INFO "Loading default partition table\n");
+               ixp4xx_nand_parts = plat->parts;
+               nb_of_parts = plat->nr_parts;
+       }
+
+       /* Register the partitions */
+       err = add_mtd_partitions(ixp4xx_mtd, ixp4xx_nand_parts, 
nb_of_parts);
+       if (err) {
+               printk(KERN_ERR "Failed to add MTD partitions\n");
+               if (ixp4xx_nand_parts != plat->parts)
+                       kfree(ixp4xx_nand_parts);
+               goto error;
+       }
+       return 0;
+
+      error:
+       if (nand_io_base)
+               iounmap((void *)nand_io_base);
+
+       if (ixp4xx_mtd)
+               kfree(ixp4xx_mtd);
+       return err;
+}
+
+static int ixp4xx_nand_flash_remove(struct platform_device *dev)
+{
+       struct ixp4xx_nand_platform_data *plat = dev->dev.platform_data;
+       struct nand_chip *this = (struct nand_chip *)&ixp4xx_mtd[1];
+
+       if (plat->exit)
+               plat->exit();
+
+       /* Unmap IO */
+       iounmap((void *)this->IO_ADDR_W);
+
+       /* Release resources, unregister device */
+       nand_release(ixp4xx_mtd);
+
+       /* Free the MTD device structure */
+       kfree(ixp4xx_mtd);
+
+       /* Free the parsed partition table */
+       if (ixp4xx_nand_parts != plat->parts)
+               kfree(ixp4xx_nand_parts);
+       return 0;
+}
+
+static struct platform_driver ixp4xx_nand_flash_driver = {
+       .probe = ixp4xx_nand_flash_probe,
+       .remove = ixp4xx_nand_flash_remove,
+       .driver = {
+                  .name = "IXP4XX-NAND-Flash",
+                  },
+};
+
+static int __init ixp4xx_nand_flash_init(void)
+{
+       return platform_driver_register(&ixp4xx_nand_flash_driver);
+}
+
+static void __exit ixp4xx_nand_flash_exit(void)
+{
+       platform_driver_unregister(&ixp4xx_nand_flash_driver);
+}
+
+module_init(ixp4xx_nand_flash_init);
+module_exit(ixp4xx_nand_flash_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Intel Corporation");
+MODULE_DESCRIPTION("Board-specific glue layer for NAND flash on "
+                  "IXP4xx-based platform");

Re: [PATH] NAND Flash support for Intel IXP4xx platform

[David Woodhouse]

On Sat, 2007-04-28 at 15:41 +0400, Ruslan V. Sushko wrote:
> The patch adds glue logic to support MTD for NAND flash devices on
> Intel IXP4xx platform

http://david.woodhou.se/email.html#test-patches

Why did you send me something you hadn't tested for yourself?

-- 
dwmw2

Re: [PATH] NAND Flash support for Intel IXP4xx platform

[Ruslan V. Sushko]

David,
I'm really sorry. I don't know but thunderbird change all tabs to 
spaces. I'm already prepare new patch in accordance with Tomas Gleixner 
suggestion,  which I've already tested as you suggest in E-Mail 
etiquette. Should I send the patch in this thread?

Sorry for inconvenient.
Ruslan

David Woodhouse wrote:
> On Sat, 2007-04-28 at 15:41 +0400, Ruslan V. Sushko wrote:
>   
>> The patch adds glue logic to support MTD for NAND flash devices on
>> Intel IXP4xx platform
>>     
>
> http://david.woodhou.se/email.html#test-patches
>
> Why did you send me something you hadn't tested for yourself?
>
>   

Re: [PATH] NAND Flash support for Intel IXP4xx platform

[Thomas Gleixner]

On Sat, 2007-04-28 at 15:41 +0400, Ruslan V. Sushko wrote:
> +static void ixp_write_buf(struct mtd_info *mtd, const u_char * buf, int 
> len)
> +{
> +       int i;
> +       struct nand_chip *this = mtd->priv;
> +
> +       for (i = 0; i < len; i++)
> +               writeb(buf[i], this->IO_ADDR_W);
> +}

How excatly is this functionally different from the generic write_buf
function in nand_base.c ?

static void nand_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
{
        int i;
        struct nand_chip *chip = mtd->priv;

        for (i = 0; i < len; i++)
                writeb(buf[i], chip->IO_ADDR_W);
}

> +static void ixp4xx_hwcontrol(struct mtd_info *mtd, int cmd, unsigned 
> int ctrl)
> +{
> +       struct nand_chip *this = mtd->priv;
> +       struct ixp4xx_faddr_info_t *addr_info = this->priv;

Newline between variables and code !

> +       if (ctrl & NAND_CTRL_CHANGE) {
> +               addr_info->offset = (ctrl & NAND_CLE) ? IXP4XX_NAND_CLE : 0;
> +               addr_info->offset |= (ctrl & NAND_ALE) ? IXP4XX_NAND_ALE 
> : 0;

Your patch is word wrapped !

> +               if (addr_info->chip_select)
> +                       addr_info->chip_select(ctrl);
> +       }
> +
> +       if (cmd != NAND_CMD_NONE)
> +               writeb(cmd, this->IO_ADDR_W + addr_info->offset);
> +}

Aside of that I agree with Lennert, that we really need to get around
and make this real platform code.

	tglx

Re: [PATH] NAND Flash support for Intel IXP4xx platform

[Ruslan V. Sushko]

Thomas,
please see my comments below

Thomas Gleixner wrote:
> On Sat, 2007-04-28 at 15:41 +0400, Ruslan V. Sushko wrote:
>   
>> +static void ixp_write_buf(struct mtd_info *mtd, const u_char * buf, int 
>> len)
>> +{
>> +       int i;
>> +       struct nand_chip *this = mtd->priv;
>> +
>> +       for (i = 0; i < len; i++)
>> +               writeb(buf[i], this->IO_ADDR_W);
>> +}
>>     
>
> How excatly is this functionally different from the generic write_buf
> function in nand_base.c ?
>
> static void nand_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
> {
>         int i;
>         struct nand_chip *chip = mtd->priv;
>
>         for (i = 0; i < len; i++)
>                 writeb(buf[i], chip->IO_ADDR_W);
> }
>
>   
This function should be removed. It was different for originally. ALE 
and CLE signal assertion was herre, but I decide this functionality is 
not necessary for data writing, so I've remove that, but forget to check 
remainder with generic code.
>> +static void ixp4xx_hwcontrol(struct mtd_info *mtd, int cmd, unsigned 
>> int ctrl)
>> +{
>> +       struct nand_chip *this = mtd->priv;
>> +       struct ixp4xx_faddr_info_t *addr_info = this->priv;
>>     
>
> Newline between variables and code !
>   
will fixed.
>   
>> +       if (ctrl & NAND_CTRL_CHANGE) {
>> +               addr_info->offset = (ctrl & NAND_CLE) ? IXP4XX_NAND_CLE : 0;
>> +               addr_info->offset |= (ctrl & NAND_ALE) ? IXP4XX_NAND_ALE 
>> : 0;
>>     
>
> Your patch is word wrapped !
>   
will fixed
>   
>> +               if (addr_info->chip_select)
>> +                       addr_info->chip_select(ctrl);
>> +       }
>> +
>> +       if (cmd != NAND_CMD_NONE)
>> +               writeb(cmd, this->IO_ADDR_W + addr_info->offset);
>> +}
>>     
>
> Aside of that I agree with Lennert, that we really need to get around
> and make this real platform code.
>   
Sorry don't understand
Do you propose to move all these (especially hw_ctrl function) 
functionality to platform/arch code?


Best regards,
Ruslan
> 	tglx
>
>
>   

Re: [PATH] NAND Flash support for Intel IXP4xx platform

[Thomas Gleixner]

Ruslan,

On Sat, 2007-04-28 at 17:42 +0400, Ruslan V. Sushko wrote:
> > How excatly is this functionally different from the generic write_buf
> > function in nand_base.c ?
> >
> This function should be removed. It was different for originally. ALE 
> and CLE signal assertion was herre, but I decide this functionality is 
> not necessary for data writing, so I've remove that, but forget to check 
> remainder with generic code.

I did a review and pointed this out. No need for lengthy explanaitions.
Just remove it.

> > Aside of that I agree with Lennert, that we really need to get around
> > and make this real platform code.
> >   
> Sorry don't understand
> Do you propose to move all these (especially hw_ctrl function) 
> functionality to platform/arch code?

That's what Lennert said. And I agree. Many of the drivers just differ
in the hardware control function and the pyhsical address where the
device is located.

We have already platform support for NAND:

See include/linux/mtd/nand.h:

struct platform_nand_chip / struct platform_nand_ctrl

As far as I can tell, there is everything covered what you need for a
generic platform driver.

Again. Lennert is right and another post of your driver does not change
anything unless you listen to his and my advise.

	tglx

Re: [PATH] NAND Flash support for Intel IXP4xx platform

[Ruslan V. Sushko]

Thomas,

I have tried to fit driver's functionality into common platform 
structures. Unfortunately the device hasn't NAND controller and some 
signals (ALE/CLE(pins on data bus) and CS(gpio)) are driven in custom 
way. Current platform structures have not enough fields to carry 
platform dependent data for this driver without hack (I guess use fields 
not used in driver but designed for other things is not good idea). 
Additionally due to device haven't NAND controller future version of 
board may change the way which is used to drive CS signal, So I move 
this functionality completely to hardware dependent part of driver (sent 
to LAKML)

Thank you,
Ruslan


Thomas Gleixner wrote:
> Ruslan,
>
> On Sat, 2007-04-28 at 17:42 +0400, Ruslan V. Sushko wrote:
>   
>>> How excatly is this functionally different from the generic write_buf
>>> function in nand_base.c ?
>>>
>>>       
>> This function should be removed. It was different for originally. ALE 
>> and CLE signal assertion was herre, but I decide this functionality is 
>> not necessary for data writing, so I've remove that, but forget to check 
>> remainder with generic code.
>>     
>
> I did a review and pointed this out. No need for lengthy explanaitions.
> Just remove it.
>
>   
>>> Aside of that I agree with Lennert, that we really need to get around
>>> and make this real platform code.
>>>   
>>>       
>> Sorry don't understand
>> Do you propose to move all these (especially hw_ctrl function) 
>> functionality to platform/arch code?
>>     
>
> That's what Lennert said. And I agree. Many of the drivers just differ
> in the hardware control function and the pyhsical address where the
> device is located.
>
> We have already platform support for NAND:
>
> See include/linux/mtd/nand.h:
>
> struct platform_nand_chip / struct platform_nand_ctrl
>
> As far as I can tell, there is everything covered what you need for a
> generic platform driver.
>
> Again. Lennert is right and another post of your driver does not change
> anything unless you listen to his and my advise.
>
> 	tglx
>
>
>   

Re: [PATH] NAND Flash support for Intel IXP4xx platform

[Thomas Gleixner]

Ruslan,

Can you please read, understand and comply with
http://www.infradead.org/~dwmw2/email.html

On Mon, 2007-04-30 at 11:36 +0400, Ruslan V. Sushko wrote:
> Thomas,
>
> I have tried to fit driver's functionality into common platform 
> structures. Unfortunately the device hasn't NAND controller and some 
> signals (ALE/CLE(pins on data bus) and CS(gpio)) are driven in custom 
> way. Current platform structures have not enough fields to carry 
> platform dependent data for this driver without hack (I guess use fields 
> not used in driver but designed for other things is not good idea). 
> Additionally due to device haven't NAND controller future version of 
> board may change the way which is used to drive CS signal, 

I really do not understand what you are talking about. There is no need
for extra fields and no need for hacks at all.

A generic platform driver, which can be used for any board with a
similar simple interface structure as yours, is just a basic interface
to the nand driver and provides common code like initialization, .....

The platform dependent code (in arch/whatever) has the hardware
dependent functions, which are given via the platform mechanism to the
generic platform driver:

struct platform_nand_ctrl {
        void            (*hwcontrol)(struct mtd_info *mtd, int cmd);
        int             (*dev_ready)(struct mtd_info *mtd);
        void            (*select_chip)(struct mtd_info *mtd, int chip);
        void            *priv;
};

It does not matter whether your board has a special NAND controller or
not. A GPIO based interface fits into this as well, simply because it is
the hardware interface, which controls the NAND flash.

> So I move 
> this functionality completely to hardware dependent part of driver (sent 
> to LAKML)

Sigh. You need a platform driver in drivers/mtd/nand first to move
anything related to nand into your arch code.

	tglx

Re: [PATH] NAND Flash support for Intel IXP4xx platform

[Vitaly Wool]

On 4/30/07, Thomas Gleixner <tglx@linutronix.de> wrote:
> > So I move
> > this functionality completely to hardware dependent part of driver (sent
> > to LAKML)
>
> Sigh. You need a platform driver in drivers/mtd/nand first to move
> anything related to nand into your arch code.

Ruslan, may I suggest that you post both parts of the driver (ie
platform-specific part and hardware-independent part) just as you see
those to this list? It's gonna be a lot easier to discuss things then,

Thanks,
   Vitaly

Re: [PATH] NAND Flash support for Intel IXP4xx platform

[Deepak Saxena]

On Apr 30 2007, at 11:36, Ruslan V. Sushko was caught saying:
> Thomas,
> 
> I have tried to fit driver's functionality into common platform 
> structures. Unfortunately the device hasn't NAND controller and some 
> signals (ALE/CLE(pins on data bus) and CS(gpio)) are driven in custom 
> way. Current platform structures have not enough fields to carry 
> platform dependent data for this driver without hack (I guess use fields 
> not used in driver but designed for other things is not good idea). 
> Additionally due to device haven't NAND controller future version of 
> board may change the way which is used to drive CS signal, So I move 
> this functionality completely to hardware dependent part of driver (sent 
> to LAKML)

Can't that extra functionality be stuffed into a structure that you
point to via the priv member of platform_nand_chip?


~Deepak

-- 
Deepak Saxena - dsaxena@plexity.net - http://www.plexity.net

In the end, they will not say, "those were dark times,"  they will ask
"why were their poets silent?" - Bertolt Brecht

Re: [PATH] NAND Flash support for Intel IXP4xx platform

[Vitaly Wool]

On 5/2/07, Deepak Saxena <dsaxena@plexity.net> wrote:
> Can't that extra functionality be stuffed into a structure that you
> point to via the priv member of platform_nand_chip?

Please see my recent posts/patches -- everything fits well in the
updated model, so either Ruslan finishes my work on generic NAND
driver or he'll have to wait till I clean up things.

Vitaly