[PATCH 0/2] PowerPC: Add 44x NDFC device-tree aware support

[PATCH 0/2] PowerPC: Add 44x NDFC device-tree aware support

[Valentine Barshak]

I've worked in parallel with Stefan Roese on the new OF NDFC support.
This version (as well as Stefan's) is based on the original NDFC driver by Thomas Gleixner.
The major difference is that the original implements each chip connected NDFC banks as a
separate MTD device. Here I try to have one MTD device spread on all chips found.
However, the chips should have equal ID's and sizes, but I've never seen several different
chips attached to single ndfc. I'm adding ndfc_of as a separate file, since some other changes
have also been made (e.g. all i/o is made with ndfc_readl/writel inline functions).
The original version didn't handle driver removal well (it never calls del_mtd...),it's 
corrected here. I decided not to add ecc pos and oob layout to DTS, since we should be OK with
the default ones with NDFC.
This stuff has been tested on PowerPC 440EPx Sequoia board.
Any comments are greatly appreciated.
Thanks,
Valentine.

[PATCH 1/2] PowerPC: Add 44x NDFC device-tree aware support

[Valentine Barshak]

This adds device-tree aware PowerPC 44x NDFC (NAND Flash Controller) driver.
The code is based on the original ndfc.c driver by Thomas Gleixner.
The major difference is that here we try to handle all chips found as one mtd
device instead of having a separate one on each chip.
The partition handling code is based on the physmap_of one.
The the first 4 bits of the "bank-mask" property show which of the 4 NDFC
banks have chips attached. The "bank-width" property is 1 for 8-bit flash
and 2 for a 16-bit one.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Valentine Barshak <vbarshak@ru.mvista.com>
---
 drivers/mtd/nand/Kconfig   |    7 
 drivers/mtd/nand/Makefile  |    1 
 drivers/mtd/nand/ndfc_of.c |  449 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/ndfc.h   |    4 
 4 files changed, 461 insertions(+)

diff -pruN linux-2.6.orig/drivers/mtd/nand/Kconfig linux-2.6/drivers/mtd/nand/Kconfig
--- linux-2.6.orig/drivers/mtd/nand/Kconfig	2007-10-25 19:20:05.000000000 +0400
+++ linux-2.6/drivers/mtd/nand/Kconfig	2007-10-26 16:16:20.000000000 +0400
@@ -158,6 +158,13 @@ config MTD_NAND_NDFC
 	help
 	 NDFC Nand Flash Controllers are integrated in IBM/AMCC's 4xx SoCs
 
+config MTD_NAND_NDFC_OF
+	tristate "NDFC OF Nand Flash Controller"
+	depends on 44x
+	select MTD_NAND_ECC_SMC
+	help
+	 NDFC OF Nand Flash Controllers are integrated in PowerPC44x SoCs
+
 config MTD_NAND_S3C2410_CLKSTOP
 	bool "S3C2410 NAND IDLE clock stop"
 	depends on MTD_NAND_S3C2410
diff -pruN linux-2.6.orig/drivers/mtd/nand/Makefile linux-2.6/drivers/mtd/nand/Makefile
--- linux-2.6.orig/drivers/mtd/nand/Makefile	2007-10-25 19:20:05.000000000 +0400
+++ linux-2.6/drivers/mtd/nand/Makefile	2007-10-26 16:16:20.000000000 +0400
@@ -24,6 +24,7 @@ obj-$(CONFIG_MTD_NAND_TS7250)		+= ts7250
 obj-$(CONFIG_MTD_NAND_NANDSIM)		+= nandsim.o
 obj-$(CONFIG_MTD_NAND_CS553X)		+= cs553x_nand.o
 obj-$(CONFIG_MTD_NAND_NDFC)		+= ndfc.o
+obj-$(CONFIG_MTD_NAND_NDFC_OF)		+= ndfc_of.o
 obj-$(CONFIG_MTD_NAND_AT91)		+= at91_nand.o
 obj-$(CONFIG_MTD_NAND_CM_X270)		+= cmx270_nand.o
 obj-$(CONFIG_MTD_NAND_BASLER_EXCITE)	+= excite_nandflash.o
diff -pruN linux-2.6.orig/drivers/mtd/nand/ndfc_of.c linux-2.6/drivers/mtd/nand/ndfc_of.c
--- linux-2.6.orig/drivers/mtd/nand/ndfc_of.c	1970-01-01 03:00:00.000000000 +0300
+++ linux-2.6/drivers/mtd/nand/ndfc_of.c	2007-10-26 17:28:57.000000000 +0400
@@ -0,0 +1,449 @@
+/*
+ *  PowerPC 44x NDFC (NanD Flash Controller) driver
+ *  with OF device tree support.
+ *
+ *  Based on the original ndfc driver by Thomas Gleixner
+ *
+ *  Copyright 2006 IBM
+ *
+ *  This program is free software; you can redistribute	 it and/or modify it
+ *  under  the terms of	 the GNU General  Public License as published by the
+ *  Free Software Foundation;  either version 2 of the	License, or (at your
+ *  option) any later version.
+ *
+ */
+#include <linux/module.h>
+#include <linux/mtd/nand.h>
+#include <linux/mtd/nand_ecc.h>
+#include <linux/mtd/partitions.h>
+#include <linux/mtd/ndfc.h>
+#include <linux/mtd/mtd.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+
+#include <asm/io.h>
+
+
+struct of_ndfc {
+	__iomem void		*base;
+	struct resource		*res;
+	unsigned		bank_width;
+	unsigned 		chip_cnt;
+	unsigned char		chip_map[NDFC_MAX_BANKS];
+	struct nand_hw_control	control;
+	struct nand_chip	chip;
+	struct mtd_info		mtd;
+#ifdef CONFIG_MTD_PARTITIONS
+	struct mtd_partition	*parts;
+#endif
+};
+
+static inline u32 ndfc_raw_readl(struct of_ndfc *ndfc, u32 off)
+{
+	return __raw_readl(ndfc->base + off);
+}
+
+static inline void ndfc_raw_writel(struct of_ndfc *ndfc, u32 off, u32 val)
+{
+	__raw_writel(val, ndfc->base + off);
+}
+
+static inline void ndfc_writel(struct of_ndfc *ndfc, u32 off, u32 val)
+{
+	writel(val, ndfc->base + off);
+}
+
+static void ndfc_select_chip(struct mtd_info *mtd, int chip)
+{
+	struct nand_chip *this = mtd->priv;
+	struct of_ndfc *ndfc = this->priv;
+	uint32_t ccr;
+
+	ccr = ndfc_raw_readl(ndfc, NDFC_CCR);
+	if ((chip >= 0) && (chip < ndfc->chip_cnt))  {
+		ccr &= ~NDFC_CCR_BS_MASK;
+		ccr |= NDFC_CCR_BS(ndfc->chip_map[chip]);
+	} else
+		ccr |= NDFC_CCR_RESET_CE;
+	ndfc_raw_writel(ndfc, NDFC_CCR, ccr);
+}
+
+static void ndfc_hwcontrol(struct mtd_info *mtd, int cmd, unsigned int ctrl)
+{
+	struct nand_chip *this = mtd->priv;
+	struct of_ndfc *ndfc = this->priv;
+
+	if (cmd == NAND_CMD_NONE)
+		return;
+
+	if (ctrl & NAND_CLE)
+		ndfc_writel(ndfc, NDFC_CMD, cmd & 0xff);
+	else
+		ndfc_writel(ndfc, NDFC_ALE, cmd & 0xff);
+}
+
+static int ndfc_ready(struct mtd_info *mtd)
+{
+	struct nand_chip *this = mtd->priv;
+	struct of_ndfc *ndfc = this->priv;
+
+	return ndfc_raw_readl(ndfc, NDFC_STAT) & NDFC_STAT_IS_READY;
+}
+
+static void ndfc_enable_hwecc(struct mtd_info *mtd, int mode)
+{
+	uint32_t ccr;
+	struct nand_chip *this = mtd->priv;
+	struct of_ndfc *ndfc = this->priv;
+
+	ccr = ndfc_raw_readl(ndfc, NDFC_CCR);
+	ccr |= NDFC_CCR_RESET_ECC;
+	ndfc_raw_writel(ndfc, NDFC_CCR, ccr);
+	wmb();
+}
+
+
+static int ndfc_calculate_ecc(struct mtd_info *mtd,
+			      const u_char *dat, u_char *ecc_code)
+{
+	uint32_t ecc;
+	struct nand_chip *this = mtd->priv;
+	struct of_ndfc *ndfc = this->priv;
+	uint8_t *p = (uint8_t *)&ecc;
+
+	wmb();
+	ecc = ndfc_raw_readl(ndfc, NDFC_ECC);
+	ecc_code[0] = p[1];
+	ecc_code[1] = p[2];
+	ecc_code[2] = p[3];
+
+	return 0;
+}
+
+
+/*
+ * Speedups for buffer read/write/verify
+ *
+ * NDFC allows 32bit read/write of data. So we can speed up the buffer
+ * functions. No further checking, as nand_base will always read/write
+ * page aligned.
+ */
+static void ndfc_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
+{
+	struct nand_chip *this = mtd->priv;
+	struct of_ndfc *ndfc = this->priv;
+	uint32_t *p = (uint32_t *) buf;
+
+	for(;len > 0; len -= 4)
+		*p++ = ndfc_raw_readl(ndfc, NDFC_DATA);
+}
+
+static void ndfc_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
+{
+	struct nand_chip *this = mtd->priv;
+	struct of_ndfc *ndfc = this->priv;
+	uint32_t *p = (uint32_t *) buf;
+
+	for(;len > 0; len -= 4)
+		ndfc_raw_writel(ndfc, NDFC_DATA, *p++);
+}
+
+static int ndfc_verify_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
+{
+	struct nand_chip *this = mtd->priv;
+	struct of_ndfc *ndfc = this->priv;
+	uint32_t *p = (uint32_t *) buf;
+
+	for(;len > 0; len -= 4)
+		if (*p++ != ndfc_raw_readl(ndfc, NDFC_DATA))
+			return -EFAULT;
+	return 0;
+}
+
+
+
+static void ndfc_chip_init(struct nand_chip *chip, 
+				struct of_ndfc *ndfc)
+{
+	chip->IO_ADDR_R = ndfc->base + NDFC_DATA;
+	chip->IO_ADDR_W = ndfc->base + NDFC_DATA;
+	chip->cmd_ctrl = ndfc_hwcontrol;
+	chip->dev_ready = ndfc_ready;
+	chip->select_chip = ndfc_select_chip;
+	chip->chip_delay = 50;
+	chip->priv = ndfc;
+	if (ndfc->bank_width == 2)
+		chip->options |= NAND_BUSWIDTH_16;
+	chip->controller = &ndfc->control;
+	chip->read_buf = ndfc_read_buf;
+	chip->write_buf = ndfc_write_buf;
+	chip->verify_buf = ndfc_verify_buf;
+	chip->ecc.correct = nand_correct_data;
+	chip->ecc.hwctl = ndfc_enable_hwecc;
+	chip->ecc.calculate = ndfc_calculate_ecc;
+	chip->ecc.mode = NAND_ECC_HW;
+	chip->ecc.size = 256;
+	chip->ecc.bytes = 3;
+	ndfc->mtd.priv = chip;
+	ndfc->mtd.owner = THIS_MODULE;
+}
+
+
+#ifdef CONFIG_MTD_PARTITIONS
+#define OF_FLASH_PARTS(ndfc)	((ndfc)->parts)
+
+static int __devinit parse_partitions(struct of_ndfc *ndfc,
+				      struct of_device *dev)
+{
+	const char *partname;
+	static const char *part_probe_types[]
+		= { "cmdlinepart", "RedBoot", NULL };
+	struct device_node *dp = dev->node, *pp;
+	int nr_parts, i;
+
+	/* First look for RedBoot table or partitions on the command
+	 * line, these take precedence over device tree information */
+	nr_parts = parse_mtd_partitions(&ndfc->mtd, part_probe_types,
+					&ndfc->parts, 0);
+	if (nr_parts > 0) {
+		add_mtd_partitions(&ndfc->mtd, ndfc->parts, nr_parts);
+		return 0;
+	}
+
+	/* First count the subnodes */
+	nr_parts = 0;
+	for (pp = dp->child; pp; pp = pp->sibling)
+		nr_parts++;
+
+	if (nr_parts == 0)
+		return 0;
+
+	ndfc->parts = kzalloc(nr_parts * sizeof(*ndfc->parts),
+			      GFP_KERNEL);
+	if (!ndfc->parts)
+		return -ENOMEM;
+
+	for (pp = dp->child, i = 0; pp; pp = pp->sibling, i++) {
+		const u32 *reg;
+		int len;
+
+		reg = of_get_property(pp, "reg", &len);
+		if (!reg || (len != 2*sizeof(u32))) {
+			dev_err(&dev->dev, "Invalid 'reg' on %s\n",
+				dp->full_name);
+			kfree(ndfc->parts);
+			ndfc->parts = NULL;
+			return -EINVAL;
+		}
+		ndfc->parts[i].offset = reg[0];
+		ndfc->parts[i].size = reg[1];
+
+		partname = of_get_property(pp, "label", &len);
+		if (!partname)
+			partname = of_get_property(pp, "name", &len);
+		ndfc->parts[i].name = (char *)partname;
+
+		if (of_get_property(pp, "read-only", &len))
+			ndfc->parts[i].mask_flags = MTD_WRITEABLE;
+	}
+
+	return nr_parts;
+}
+#else /* MTD_PARTITIONS */
+#define	OF_FLASH_PARTS(ndfc)		(0)
+#define parse_partitions(ndfc, dev)	(0)
+#endif /* MTD_PARTITIONS */
+
+
+static int of_ndfc_remove(struct of_device *dev)
+{
+	struct of_ndfc *ndfc;
+
+	ndfc = dev_get_drvdata(&dev->dev);
+	if (!ndfc)
+		return 0;
+
+	if (OF_FLASH_PARTS(ndfc)) {
+		del_mtd_partitions(&ndfc->mtd);
+		kfree(OF_FLASH_PARTS(ndfc));
+	} else {
+		del_mtd_device(&ndfc->mtd);
+	}
+	nand_release(&ndfc->mtd);
+
+	dev_set_drvdata(&dev->dev, NULL);
+
+	if (ndfc->base)
+		iounmap(ndfc->base);
+
+	if (ndfc->res) {
+		release_resource(ndfc->res);
+		kfree(ndfc->res);
+	}
+
+	kfree(ndfc);
+
+	return 0;
+}
+
+
+static int __devinit ndfc_map_banks(struct of_ndfc *ndfc, const u32 *mask)
+{
+	unsigned cnt, i, tmp;
+	uint32_t bcr;
+
+	if (!ndfc || !mask)
+		return -EINVAL;
+
+	/* Disable all banks */
+	for (cnt = 0; cnt < NDFC_MAX_BANKS; cnt++) {
+		ndfc_raw_writel(ndfc, NDFC_BCFG0 + (cnt << 2), 0);
+	}
+
+	/* Enable bank and set default RE/WE/CE timings */
+	bcr = NDFC_BxCFG_EN | NDFC_BxCFG_RR(2) | NDFC_BxCFG_RWH(2) |
+				NDFC_BxCFG_RWP(2) | NDFC_BxCFG_CRW(2);
+	if (ndfc->bank_width == 2)
+		bcr |= NDFC_BxCFG_SZ_16BIT;
+
+	cnt = 0;
+	tmp = *mask;
+	while ((i = ffs(tmp)) && (cnt < NDFC_MAX_BANKS)) {
+		i--;
+		tmp &= ~(1 << i);
+		ndfc->chip_map[cnt++] = i;
+		ndfc_raw_writel(ndfc, NDFC_BCFG0 + (i << 2), bcr);
+	}
+	ndfc->chip_cnt = cnt;
+	return cnt;
+}
+
+
+static int __devinit of_ndfc_probe(struct of_device *dev,
+				    const struct of_device_id *match)
+{
+	struct device_node *dp = dev->node;
+	struct resource res;
+	struct of_ndfc *ndfc;
+	const u32 *prop;
+	resource_size_t rlen;
+	int err;
+
+	err = -ENXIO;
+	if (of_address_to_resource(dp, 0, &res)) {
+		dev_err(&dev->dev, "can't get IO address from device tree\n");
+		goto err_out;
+	}
+
+       	dev_dbg(&dev->dev, "regs: %.8llx-%.8llx\n",
+		(unsigned long long)res.start, (unsigned long long)res.end);
+
+	ndfc = kzalloc(sizeof(struct of_ndfc), GFP_KERNEL);
+	if (!ndfc) {
+		err = -ENOMEM;
+		goto err_out;
+	}
+
+	rlen = res.end - res.start + 1;
+	ndfc->res = request_mem_region(res.start, rlen, dev->dev.bus_id);
+	if (!ndfc->res) {
+		err = -EBUSY;
+		goto err_free_out;
+	}
+
+	ndfc->base = ioremap(res.start, rlen);
+	if (!ndfc->base) {
+		err = -ENXIO;
+		goto err_rel_out;
+	}
+
+	spin_lock_init(&ndfc->control.lock);
+	init_waitqueue_head(&ndfc->control.wq);
+
+	prop = of_get_property(dp, "bank-width", NULL);
+	ndfc->bank_width = ((prop) && (*prop) == 2) ? 2 : 1;
+
+	prop = of_get_property(dp, "bank-mask", NULL);
+	err = ndfc_map_banks(ndfc, prop);
+	if (err <= 0) {
+		dev_err(&dev->dev, "no banks found\n");
+		err = -ENODEV;
+		goto err_unmap_out;
+	}
+
+	ndfc_chip_init(&ndfc->chip, ndfc);
+	dev_set_drvdata(&dev->dev, ndfc);
+
+	dev_info(&dev->dev, "NDFC driver initialized. Chip-Rev: 0x%08x\n",
+			ndfc_raw_readl(ndfc, NDFC_REVID));
+
+	err = nand_scan_ident(&ndfc->mtd, ndfc->chip_cnt);
+	if (err)
+		goto err_dat_out;
+
+	if ((ndfc->mtd.writesize != 2048) && (ndfc->mtd.writesize != 512)) {
+		dev_err(&dev->dev, "unexpected NAND flash writesize %d",
+			ndfc->mtd.writesize);
+		goto err_dat_out;
+	}
+
+	err = nand_scan_tail(&ndfc->mtd);
+	if (err)
+		goto err_dat_out;
+
+	err = parse_partitions(ndfc, dev);
+	if (err < 0)
+		goto err_dat_out;
+
+	if (err > 0)
+		add_mtd_partitions(&ndfc->mtd, OF_FLASH_PARTS(ndfc), err);
+	else
+		add_mtd_device(&ndfc->mtd);
+
+	return 0;
+
+err_dat_out:
+	dev_set_drvdata(&dev->dev, NULL);
+err_unmap_out:
+	iounmap(ndfc->base);
+err_rel_out:
+	release_resource(ndfc->res);
+	kfree(ndfc->res);
+err_free_out:
+	kfree(ndfc);
+err_out:
+	return err;
+}
+
+static struct of_device_id of_ndfc_match[] = {
+	{
+		.compatible	= "ibm,ndfc",
+	},
+	{ },
+};
+MODULE_DEVICE_TABLE(of, of_ndfc_match);
+
+static struct of_platform_driver of_ndfc_driver = {
+	.name		= "of-ndfc",
+	.match_table	= of_ndfc_match,
+	.probe		= of_ndfc_probe,
+	.remove		= of_ndfc_remove,
+};
+
+static int __init of_ndfc_init(void)
+{
+	return of_register_platform_driver(&of_ndfc_driver);
+}
+
+static void __exit of_ndfc_exit(void)
+{
+	of_unregister_platform_driver(&of_ndfc_driver);
+}
+
+module_init(of_ndfc_init);
+module_exit(of_ndfc_exit);
+
+MODULE_LICENSE("GPL");
+
+
+MODULE_DESCRIPTION("OF driver for NDFC");
diff -pruN linux-2.6.orig/include/linux/mtd/ndfc.h linux-2.6/include/linux/mtd/ndfc.h
--- linux-2.6.orig/include/linux/mtd/ndfc.h	2007-10-25 19:20:42.000000000 +0400
+++ linux-2.6/include/linux/mtd/ndfc.h	2007-10-26 16:19:42.000000000 +0400
@@ -52,6 +52,10 @@
 #define NDFC_BxCFG_SZ_MASK	0x08000000 /* Bank Size */
 #define NDFC_BxCFG_SZ_8BIT	0x00000000 /* 8bit */
 #define NDFC_BxCFG_SZ_16BIT	0x08000000 /* 16bit */
+#define NDFC_BxCFG_RR(x)	(((x) & 0x7) << 0)
+#define NDFC_BxCFG_RWH(x)	(((x) & 0x7) << 4)
+#define NDFC_BxCFG_RWP(x)	(((x) & 0x7) << 8)
+#define NDFC_BxCFG_CRW(x)	(((x) & 0x7) << 12)
 
 #define NDFC_MAX_BANKS		4
 

Re: [PATCH 1/2] PowerPC: Add 44x NDFC device-tree aware support

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 2917 bytes --]

On Fri, 26 Oct 2007 20:39:58 +0400 Valentine Barshak <vbarshak@ru.mvista.com> wrote:
>
> +static int __devinit parse_partitions(struct of_ndfc *ndfc,
> +				      struct of_device *dev)
> +{
> +	const char *partname;
> +	static const char *part_probe_types[]
> +		= { "cmdlinepart", "RedBoot", NULL };
> +	struct device_node *dp = dev->node, *pp;
> +	int nr_parts, i;
> +
> +	/* First look for RedBoot table or partitions on the command
> +	 * line, these take precedence over device tree information */
> +	nr_parts = parse_mtd_partitions(&ndfc->mtd, part_probe_types,
> +					&ndfc->parts, 0);
> +	if (nr_parts > 0) {
> +		add_mtd_partitions(&ndfc->mtd, ndfc->parts, nr_parts);
> +		return 0;
> +	}
> +
> +	/* First count the subnodes */
> +	nr_parts = 0;
> +	for (pp = dp->child; pp; pp = pp->sibling)

For proper ref counting and locking, use:

	for (pp = of_get_next_child(dp, NULL); pp;
	     pp = of_get_next_child(dp, pp))

> +		nr_parts++;
> +
> +	if (nr_parts == 0)
> +		return 0;
> +
> +	ndfc->parts = kzalloc(nr_parts * sizeof(*ndfc->parts),
> +			      GFP_KERNEL);
> +	if (!ndfc->parts)
> +		return -ENOMEM;
> +
> +	for (pp = dp->child, i = 0; pp; pp = pp->sibling, i++) {

	for (pp = of_get_next_child(dp, NULL), i = 0; pp;
	     pp = of_get_next_child(dp, pp), i++)

> +		const u32 *reg;
> +		int len;
> +
> +		reg = of_get_property(pp, "reg", &len);
> +		if (!reg || (len != 2*sizeof(u32))) {

			of_node_put(pp);

> +			dev_err(&dev->dev, "Invalid 'reg' on %s\n",
> +				dp->full_name);
> +			kfree(ndfc->parts);
> +			ndfc->parts = NULL;
> +			return -EINVAL;
> +		}
> +		ndfc->parts[i].offset = reg[0];
> +		ndfc->parts[i].size = reg[1];
> +
> +		partname = of_get_property(pp, "label", &len);
> +		if (!partname)
> +			partname = of_get_property(pp, "name", &len);
> +		ndfc->parts[i].name = (char *)partname;
> +
> +		if (of_get_property(pp, "read-only", &len))
> +			ndfc->parts[i].mask_flags = MTD_WRITEABLE;
> +	}
> +
> +	return nr_parts;
> +}
> +#else /* MTD_PARTITIONS */
> +#define	OF_FLASH_PARTS(ndfc)		(0)
> +#define parse_partitions(ndfc, dev)	(0)

static inline int parse_partitions(struct of_ndfc *ndfc, struct of_device *dev)
{
	return 0;
}

> +#endif /* MTD_PARTITIONS */

> +static int __devinit of_ndfc_probe(struct of_device *dev,
> +				    const struct of_device_id *match)
> +{
> +	struct device_node *dp = dev->node;
> +	struct resource res;
> +	struct of_ndfc *ndfc;
> +	const u32 *prop;
> +	resource_size_t rlen;
> +	int err;
> +
> +	err = -ENXIO;
> +	if (of_address_to_resource(dp, 0, &res)) {
> +		dev_err(&dev->dev, "can't get IO address from device tree\n");
> +		goto err_out;
> +	}
> +
> +       	dev_dbg(&dev->dev, "regs: %.8llx-%.8llx\n",

indentation ?

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 1/2] PowerPC: Add 44x NDFC device-tree aware support

[Stefan Roese]

Hi Valentine,

On Friday 26 October 2007, Valentine Barshak wrote:
> This adds device-tree aware PowerPC 44x NDFC (NAND Flash Controller)
> driver. The code is based on the original ndfc.c driver by Thomas Gleixne=
r.
> The major difference is that here we try to handle all chips found as one
> mtd device instead of having a separate one on each chip.
> The partition handling code is based on the physmap_of one.
> The the first 4 bits of the "bank-mask" property show which of the 4 NDFC
> banks have chips attached. The "bank-width" property is 1 for 8-bit flash
> and 2 for a 16-bit one.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Valentine Barshak <vbarshak@ru.mvista.com>

Are you sure you have the Signed-off-by from Thomas already on this?

> ---
> =A0drivers/mtd/nand/Kconfig =A0 | =A0 =A07
> =A0drivers/mtd/nand/Makefile =A0| =A0 =A01
> =A0drivers/mtd/nand/ndfc_of.c | =A0449
> +++++++++++++++++++++++++++++++++++++++++++++ include/linux/mtd/ndfc.h =
=A0 |
> =A0 =A04
> =A04 files changed, 461 insertions(+)
>
> diff -pruN linux-2.6.orig/drivers/mtd/nand/Kconfig
> linux-2.6/drivers/mtd/nand/Kconfig ---
> linux-2.6.orig/drivers/mtd/nand/Kconfig=A0=A0=A0=A0=A02007-10-25 19:20:05=
=2E000000000
> +0400 +++ linux-2.6/drivers/mtd/nand/Kconfig=A0=A02007-10-26 16:16:20.000=
000000
> +0400 @@ -158,6 +158,13 @@ config MTD_NAND_NDFC
> =A0=A0=A0=A0=A0=A0=A0=A0help
> =A0=A0=A0=A0=A0=A0=A0=A0 NDFC Nand Flash Controllers are integrated in IB=
M/AMCC's 4xx SoCs
> =A0
> +config MTD_NAND_NDFC_OF
> +=A0=A0=A0=A0=A0=A0=A0tristate "NDFC OF Nand Flash Controller"
> +=A0=A0=A0=A0=A0=A0=A0depends on 44x
> +=A0=A0=A0=A0=A0=A0=A0select MTD_NAND_ECC_SMC
> +=A0=A0=A0=A0=A0=A0=A0help
> +=A0=A0=A0=A0=A0=A0=A0 NDFC OF Nand Flash Controllers are integrated in P=
owerPC44x SoCs
> +
> =A0config MTD_NAND_S3C2410_CLKSTOP
> =A0=A0=A0=A0=A0=A0=A0=A0bool "S3C2410 NAND IDLE clock stop"
> =A0=A0=A0=A0=A0=A0=A0=A0depends on MTD_NAND_S3C2410
> diff -pruN linux-2.6.orig/drivers/mtd/nand/Makefile
> linux-2.6/drivers/mtd/nand/Makefile ---
> linux-2.6.orig/drivers/mtd/nand/Makefile=A0=A0=A0=A02007-10-25 19:20:05.0=
00000000
> +0400 +++ linux-2.6/drivers/mtd/nand/Makefile=A02007-10-26 16:16:20.00000=
0000
> +0400 @@ -24,6 +24,7 @@ obj-$(CONFIG_MTD_NAND_TS7250)=A0=A0=A0=A0=A0=A0=
=A0=A0=A0+=3D ts7250
> =A0obj-$(CONFIG_MTD_NAND_NANDSIM)=A0=A0=A0=A0=A0=A0=A0=A0=A0+=3D nandsim.o
> =A0obj-$(CONFIG_MTD_NAND_CS553X)=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0+=3D cs553x=
_nand.o
> =A0obj-$(CONFIG_MTD_NAND_NDFC)=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0+=3D nd=
fc.o
> +obj-$(CONFIG_MTD_NAND_NDFC_OF)=A0=A0=A0=A0=A0=A0=A0=A0=A0+=3D ndfc_of.o
> =A0obj-$(CONFIG_MTD_NAND_AT91)=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0+=3D at=
91_nand.o
> =A0obj-$(CONFIG_MTD_NAND_CM_X270)=A0=A0=A0=A0=A0=A0=A0=A0=A0+=3D cmx270_n=
and.o
> =A0obj-$(CONFIG_MTD_NAND_BASLER_EXCITE)=A0=A0=A0+=3D excite_nandflash.o
> diff -pruN linux-2.6.orig/drivers/mtd/nand/ndfc_of.c
> linux-2.6/drivers/mtd/nand/ndfc_of.c ---
> linux-2.6.orig/drivers/mtd/nand/ndfc_of.c=A0=A0=A01970-01-01 03:00:00.000=
000000
> +0300 +++ linux-2.6/drivers/mtd/nand/ndfc_of.c=A0=A0=A0=A0=A0=A0=A0=A0200=
7-10-26
> 17:28:57.000000000 +0400 @@ -0,0 +1,449 @@
> +/*
> + * =A0PowerPC 44x NDFC (NanD Flash Controller) driver
> + * =A0with OF device tree support.
> + *
> + * =A0Based on the original ndfc driver by Thomas Gleixner
> + *
> + * =A0Copyright 2006 IBM
> + *
> + * =A0This program is free software; you can redistribute=A0=A0=A0=A0=A0=
=A0=A0=A0 it and/or
> modify it + * =A0under =A0the terms of=A0=A0=A0=A0=A0=A0=A0=A0 the GNU Ge=
neral =A0Public License
> as published by the + * =A0Free Software Foundation; =A0either version 2 =
of
> the=A0License, or (at your + * =A0option) any later version.
> + *
> + */
> +#include <linux/module.h>
> +#include <linux/mtd/nand.h>
> +#include <linux/mtd/nand_ecc.h>
> +#include <linux/mtd/partitions.h>
> +#include <linux/mtd/ndfc.h>
> +#include <linux/mtd/mtd.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +
> +#include <asm/io.h>
> +
> +
> +struct of_ndfc {
> +=A0=A0=A0=A0=A0=A0=A0__iomem void=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0*ba=
se;
> +=A0=A0=A0=A0=A0=A0=A0struct resource=A0=A0=A0=A0=A0=A0=A0=A0=A0*res;
> +=A0=A0=A0=A0=A0=A0=A0unsigned=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=
=A0=A0bank_width;
> +=A0=A0=A0=A0=A0=A0=A0unsigned =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=
=A0chip_cnt;
> +=A0=A0=A0=A0=A0=A0=A0unsigned char=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0chip_=
map[NDFC_MAX_BANKS];
> +=A0=A0=A0=A0=A0=A0=A0struct nand_hw_control=A0=A0control;
> +=A0=A0=A0=A0=A0=A0=A0struct nand_chip=A0=A0=A0=A0=A0=A0=A0=A0chip;
> +=A0=A0=A0=A0=A0=A0=A0struct mtd_info=A0=A0=A0=A0=A0=A0=A0=A0=A0mtd;
> +#ifdef CONFIG_MTD_PARTITIONS
> +=A0=A0=A0=A0=A0=A0=A0struct mtd_partition=A0=A0=A0=A0*parts;
> +#endif
> +};
> +
> +static inline u32 ndfc_raw_readl(struct of_ndfc *ndfc, u32 off)
> +{
> +	return __raw_readl(ndfc->base + off);
> +}
> +
> +static inline void ndfc_raw_writel(struct of_ndfc *ndfc, u32 off, u32 va=
l)
> +{
> +	__raw_writel(val, ndfc->base + off);
> +}
> +
> +static inline void ndfc_writel(struct of_ndfc *ndfc, u32 off, u32 val)
> +{
> +	writel(val, ndfc->base + off);
> +}
> +
> +static void ndfc_select_chip(struct mtd_info *mtd, int chip)
> +{
> +	struct nand_chip *this =3D mtd->priv;
> +	struct of_ndfc *ndfc =3D this->priv;
> +	uint32_t ccr;
> +
> +	ccr =3D ndfc_raw_readl(ndfc, NDFC_CCR);
> +	if ((chip >=3D 0) && (chip < ndfc->chip_cnt))  {
> +		ccr &=3D ~NDFC_CCR_BS_MASK;
> +		ccr |=3D NDFC_CCR_BS(ndfc->chip_map[chip]);
> +	} else
> +		ccr |=3D NDFC_CCR_RESET_CE;
> +	ndfc_raw_writel(ndfc, NDFC_CCR, ccr);
> +}
> +
> +static void ndfc_hwcontrol(struct mtd_info *mtd, int cmd, unsigned int
> ctrl) +{
> +	struct nand_chip *this =3D mtd->priv;
> +	struct of_ndfc *ndfc =3D this->priv;
> +
> +	if (cmd =3D=3D NAND_CMD_NONE)
> +		return;
> +
> +	if (ctrl & NAND_CLE)
> +		ndfc_writel(ndfc, NDFC_CMD, cmd & 0xff);
> +	else
> +		ndfc_writel(ndfc, NDFC_ALE, cmd & 0xff);
> +}
> +
> +static int ndfc_ready(struct mtd_info *mtd)
> +{
> +	struct nand_chip *this =3D mtd->priv;
> +	struct of_ndfc *ndfc =3D this->priv;
> +
> +	return ndfc_raw_readl(ndfc, NDFC_STAT) & NDFC_STAT_IS_READY;
> +}
> +
> +static void ndfc_enable_hwecc(struct mtd_info *mtd, int mode)
> +{
> +	uint32_t ccr;
> +	struct nand_chip *this =3D mtd->priv;
> +	struct of_ndfc *ndfc =3D this->priv;
> +
> +	ccr =3D ndfc_raw_readl(ndfc, NDFC_CCR);
> +	ccr |=3D NDFC_CCR_RESET_ECC;
> +	ndfc_raw_writel(ndfc, NDFC_CCR, ccr);
> +	wmb();

I suspect that when we use the in_be32() and friends functions for IO acces=
s,=20
the memory-barriers can go away.

> +}
> +
> +
> +static int ndfc_calculate_ecc(struct mtd_info *mtd,
> +			      const u_char *dat, u_char *ecc_code)
> +{
> +	uint32_t ecc;
> +	struct nand_chip *this =3D mtd->priv;
> +	struct of_ndfc *ndfc =3D this->priv;
> +	uint8_t *p =3D (uint8_t *)&ecc;
> +
> +	wmb();

Same here.

> +	ecc =3D ndfc_raw_readl(ndfc, NDFC_ECC);
> +	ecc_code[0] =3D p[1];
> +	ecc_code[1] =3D p[2];
> +	ecc_code[2] =3D p[3];
> +
> +	return 0;
> +}
> +
> +
> +/*
> + * Speedups for buffer read/write/verify
> + *
> + * NDFC allows 32bit read/write of data. So we can speed up the buffer
> + * functions. No further checking, as nand_base will always read/write
> + * page aligned.
> + */
> +static void ndfc_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
> +{
> +	struct nand_chip *this =3D mtd->priv;
> +	struct of_ndfc *ndfc =3D this->priv;
> +	uint32_t *p =3D (uint32_t *) buf;
> +
> +	for(;len > 0; len -=3D 4)
> +		*p++ =3D ndfc_raw_readl(ndfc, NDFC_DATA);
> +}
> +
> +static void ndfc_write_buf(struct mtd_info *mtd, const uint8_t *buf, int
> len) +{
> +	struct nand_chip *this =3D mtd->priv;
> +	struct of_ndfc *ndfc =3D this->priv;
> +	uint32_t *p =3D (uint32_t *) buf;
> +
> +	for(;len > 0; len -=3D 4)
> +		ndfc_raw_writel(ndfc, NDFC_DATA, *p++);
> +}
> +
> +static int ndfc_verify_buf(struct mtd_info *mtd, const uint8_t *buf, int
> len) +{
> +	struct nand_chip *this =3D mtd->priv;
> +	struct of_ndfc *ndfc =3D this->priv;
> +	uint32_t *p =3D (uint32_t *) buf;
> +
> +	for(;len > 0; len -=3D 4)
> +		if (*p++ !=3D ndfc_raw_readl(ndfc, NDFC_DATA))
> +			return -EFAULT;
> +	return 0;
> +}
> +
> +
> +
> +static void ndfc_chip_init(struct nand_chip *chip,
> +				struct of_ndfc *ndfc)
> +{
> +	chip->IO_ADDR_R =3D ndfc->base + NDFC_DATA;
> +	chip->IO_ADDR_W =3D ndfc->base + NDFC_DATA;
> +	chip->cmd_ctrl =3D ndfc_hwcontrol;
> +	chip->dev_ready =3D ndfc_ready;
> +	chip->select_chip =3D ndfc_select_chip;
> +	chip->chip_delay =3D 50;
> +	chip->priv =3D ndfc;
> +	if (ndfc->bank_width =3D=3D 2)
> +		chip->options |=3D NAND_BUSWIDTH_16;
> +	chip->controller =3D &ndfc->control;
> +	chip->read_buf =3D ndfc_read_buf;
> +	chip->write_buf =3D ndfc_write_buf;
> +	chip->verify_buf =3D ndfc_verify_buf;
> +	chip->ecc.correct =3D nand_correct_data;
> +	chip->ecc.hwctl =3D ndfc_enable_hwecc;
> +	chip->ecc.calculate =3D ndfc_calculate_ecc;
> +	chip->ecc.mode =3D NAND_ECC_HW;
> +	chip->ecc.size =3D 256;
> +	chip->ecc.bytes =3D 3;
> +	ndfc->mtd.priv =3D chip;
> +	ndfc->mtd.owner =3D THIS_MODULE;
> +}
> +
> +
> +#ifdef CONFIG_MTD_PARTITIONS
> +#define OF_FLASH_PARTS(ndfc)	((ndfc)->parts)
> +
> +static int __devinit parse_partitions(struct of_ndfc *ndfc,
> +				      struct of_device *dev)
> +{
> +	const char *partname;
> +	static const char *part_probe_types[]
> +		=3D { "cmdlinepart", "RedBoot", NULL };
> +	struct device_node *dp =3D dev->node, *pp;
> +	int nr_parts, i;
> +
> +	/* First look for RedBoot table or partitions on the command
> +	 * line, these take precedence over device tree information */
> +	nr_parts =3D parse_mtd_partitions(&ndfc->mtd, part_probe_types,
> +					&ndfc->parts, 0);
> +	if (nr_parts > 0) {
> +		add_mtd_partitions(&ndfc->mtd, ndfc->parts, nr_parts);
> +		return 0;
> +	}
> +
> +	/* First count the subnodes */
> +	nr_parts =3D 0;
> +	for (pp =3D dp->child; pp; pp =3D pp->sibling)
> +		nr_parts++;
> +
> +	if (nr_parts =3D=3D 0)
> +		return 0;
> +
> +	ndfc->parts =3D kzalloc(nr_parts * sizeof(*ndfc->parts),
> +			      GFP_KERNEL);
> +	if (!ndfc->parts)
> +		return -ENOMEM;
> +
> +	for (pp =3D dp->child, i =3D 0; pp; pp =3D pp->sibling, i++) {
> +		const u32 *reg;
> +		int len;
> +
> +		reg =3D of_get_property(pp, "reg", &len);
> +		if (!reg || (len !=3D 2*sizeof(u32))) {
> +			dev_err(&dev->dev, "Invalid 'reg' on %s\n",
> +				dp->full_name);
> +			kfree(ndfc->parts);
> +			ndfc->parts =3D NULL;
> +			return -EINVAL;
> +		}
> +		ndfc->parts[i].offset =3D reg[0];
> +		ndfc->parts[i].size =3D reg[1];
> +
> +		partname =3D of_get_property(pp, "label", &len);
> +		if (!partname)
> +			partname =3D of_get_property(pp, "name", &len);
> +		ndfc->parts[i].name =3D (char *)partname;
> +
> +		if (of_get_property(pp, "read-only", &len))
> +			ndfc->parts[i].mask_flags =3D MTD_WRITEABLE;
> +	}
> +
> +	return nr_parts;
> +}

This parse_partition code looks very much like the code in the physmap_of=20
driver. I think it would be a good idea not to duplicate this code, but to=
=20
extract it and use one version in both drivers.

> +#else /* MTD_PARTITIONS */
> +#define	OF_FLASH_PARTS(ndfc)		(0)
> +#define parse_partitions(ndfc, dev)	(0)
> +#endif /* MTD_PARTITIONS */
> +
> +
> +static int of_ndfc_remove(struct of_device *dev)
> +{
> +	struct of_ndfc *ndfc;
> +
> +	ndfc =3D dev_get_drvdata(&dev->dev);
> +	if (!ndfc)
> +		return 0;
> +
> +	if (OF_FLASH_PARTS(ndfc)) {
> +		del_mtd_partitions(&ndfc->mtd);
> +		kfree(OF_FLASH_PARTS(ndfc));
> +	} else {
> +		del_mtd_device(&ndfc->mtd);
> +	}
> +	nand_release(&ndfc->mtd);
> +
> +	dev_set_drvdata(&dev->dev, NULL);
> +
> +	if (ndfc->base)
> +		iounmap(ndfc->base);
> +
> +	if (ndfc->res) {
> +		release_resource(ndfc->res);
> +		kfree(ndfc->res);
> +	}
> +
> +	kfree(ndfc);
> +
> +	return 0;
> +}
> +
> +
> +static int __devinit ndfc_map_banks(struct of_ndfc *ndfc, const u32 *mas=
k)
> +{
> +	unsigned cnt, i, tmp;
> +	uint32_t bcr;
> +
> +	if (!ndfc || !mask)
> +		return -EINVAL;
> +
> +	/* Disable all banks */
> +	for (cnt =3D 0; cnt < NDFC_MAX_BANKS; cnt++) {
> +		ndfc_raw_writel(ndfc, NDFC_BCFG0 + (cnt << 2), 0);
> +	}
> +
> +	/* Enable bank and set default RE/WE/CE timings */
> +	bcr =3D NDFC_BxCFG_EN | NDFC_BxCFG_RR(2) | NDFC_BxCFG_RWH(2) |
> +				NDFC_BxCFG_RWP(2) | NDFC_BxCFG_CRW(2);
> +	if (ndfc->bank_width =3D=3D 2)
> +		bcr |=3D NDFC_BxCFG_SZ_16BIT;
> +
> +	cnt =3D 0;
> +	tmp =3D *mask;
> +	while ((i =3D ffs(tmp)) && (cnt < NDFC_MAX_BANKS)) {
> +		i--;
> +		tmp &=3D ~(1 << i);
> +		ndfc->chip_map[cnt++] =3D i;
> +		ndfc_raw_writel(ndfc, NDFC_BCFG0 + (i << 2), bcr);
> +	}
> +	ndfc->chip_cnt =3D cnt;
> +	return cnt;
> +}
> +
> +
> +static int __devinit of_ndfc_probe(struct of_device *dev,
> +				    const struct of_device_id *match)
> +{
> +	struct device_node *dp =3D dev->node;
> +	struct resource res;
> +	struct of_ndfc *ndfc;
> +	const u32 *prop;
> +	resource_size_t rlen;
> +	int err;
> +
> +	err =3D -ENXIO;
> +	if (of_address_to_resource(dp, 0, &res)) {
> +		dev_err(&dev->dev, "can't get IO address from device tree\n");
> +		goto err_out;
> +	}
> +
> +       	dev_dbg(&dev->dev, "regs: %.8llx-%.8llx\n",
> +		(unsigned long long)res.start, (unsigned long long)res.end);
> +
> +	ndfc =3D kzalloc(sizeof(struct of_ndfc), GFP_KERNEL);
> +	if (!ndfc) {
> +		err =3D -ENOMEM;
> +		goto err_out;
> +	}
> +
> +	rlen =3D res.end - res.start + 1;
> +	ndfc->res =3D request_mem_region(res.start, rlen, dev->dev.bus_id);
> +	if (!ndfc->res) {
> +		err =3D -EBUSY;
> +		goto err_free_out;
> +	}
> +
> +	ndfc->base =3D ioremap(res.start, rlen);
> +	if (!ndfc->base) {
> +		err =3D -ENXIO;
> +		goto err_rel_out;
> +	}
> +
> +	spin_lock_init(&ndfc->control.lock);
> +	init_waitqueue_head(&ndfc->control.wq);
> +
> +	prop =3D of_get_property(dp, "bank-width", NULL);
> +	ndfc->bank_width =3D ((prop) && (*prop) =3D=3D 2) ? 2 : 1;
> +
> +	prop =3D of_get_property(dp, "bank-mask", NULL);
> +	err =3D ndfc_map_banks(ndfc, prop);
> +	if (err <=3D 0) {
> +		dev_err(&dev->dev, "no banks found\n");
> +		err =3D -ENODEV;
> +		goto err_unmap_out;
> +	}
> +
> +	ndfc_chip_init(&ndfc->chip, ndfc);
> +	dev_set_drvdata(&dev->dev, ndfc);
> +
> +	dev_info(&dev->dev, "NDFC driver initialized. Chip-Rev: 0x%08x\n",
> +			ndfc_raw_readl(ndfc, NDFC_REVID));
> +
> +	err =3D nand_scan_ident(&ndfc->mtd, ndfc->chip_cnt);
> +	if (err)
> +		goto err_dat_out;
> +
> +	if ((ndfc->mtd.writesize !=3D 2048) && (ndfc->mtd.writesize !=3D 512)) {
> +		dev_err(&dev->dev, "unexpected NAND flash writesize %d",
> +			ndfc->mtd.writesize);
> +		goto err_dat_out;
> +	}
> +
> +	err =3D nand_scan_tail(&ndfc->mtd);
> +	if (err)
> +		goto err_dat_out;
> +
> +	err =3D parse_partitions(ndfc, dev);
> +	if (err < 0)
> +		goto err_dat_out;
> +
> +	if (err > 0)
> +		add_mtd_partitions(&ndfc->mtd, OF_FLASH_PARTS(ndfc), err);
> +	else
> +		add_mtd_device(&ndfc->mtd);
> +
> +	return 0;
> +
> +err_dat_out:
> +	dev_set_drvdata(&dev->dev, NULL);
> +err_unmap_out:
> +	iounmap(ndfc->base);
> +err_rel_out:
> +	release_resource(ndfc->res);
> +	kfree(ndfc->res);
> +err_free_out:
> +	kfree(ndfc);
> +err_out:
> +	return err;
> +}
> +
> +static struct of_device_id of_ndfc_match[] =3D {
> +	{
> +		.compatible	=3D "ibm,ndfc",
> +	},
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, of_ndfc_match);
> +
> +static struct of_platform_driver of_ndfc_driver =3D {
> +	.name		=3D "of-ndfc",
> +	.match_table	=3D of_ndfc_match,
> +	.probe		=3D of_ndfc_probe,
> +	.remove		=3D of_ndfc_remove,
> +};
> +
> +static int __init of_ndfc_init(void)
> +{
> +	return of_register_platform_driver(&of_ndfc_driver);
> +}
> +
> +static void __exit of_ndfc_exit(void)
> +{
> +	of_unregister_platform_driver(&of_ndfc_driver);
> +}
> +
> +module_init(of_ndfc_init);
> +module_exit(of_ndfc_exit);
> +
> +MODULE_LICENSE("GPL");
> +
> +
> +MODULE_DESCRIPTION("OF driver for NDFC");
> diff -pruN linux-2.6.orig/include/linux/mtd/ndfc.h
> linux-2.6/include/linux/mtd/ndfc.h ---
> linux-2.6.orig/include/linux/mtd/ndfc.h	2007-10-25 19:20:42.000000000 +04=
00
> +++ linux-2.6/include/linux/mtd/ndfc.h	2007-10-26 16:19:42.000000000 +0400
> @@ -52,6 +52,10 @@
>  #define NDFC_BxCFG_SZ_MASK	0x08000000 /* Bank Size */
>  #define NDFC_BxCFG_SZ_8BIT	0x00000000 /* 8bit */
>  #define NDFC_BxCFG_SZ_16BIT	0x08000000 /* 16bit */
> +#define NDFC_BxCFG_RR(x)	(((x) & 0x7) << 0)
> +#define NDFC_BxCFG_RWH(x)	(((x) & 0x7) << 4)
> +#define NDFC_BxCFG_RWP(x)	(((x) & 0x7) << 8)
> +#define NDFC_BxCFG_CRW(x)	(((x) & 0x7) << 12)
>
>  #define NDFC_MAX_BANKS		4

Best regards,
Stefan

Re: [PATCH 1/2] PowerPC: Add 44x NDFC device-tree aware support

[Thomas Gleixner]

On Sat, 27 Oct 2007, Stefan Roese wrote:

> Hi Valentine,
> 
> On Friday 26 October 2007, Valentine Barshak wrote:
> > This adds device-tree aware PowerPC 44x NDFC (NAND Flash Controller)
> > driver. The code is based on the original ndfc.c driver by Thomas Gleixner.
> > The major difference is that here we try to handle all chips found as one
> > mtd device instead of having a separate one on each chip.
> > The partition handling code is based on the physmap_of one.
> > The the first 4 bits of the "bank-mask" property show which of the 4 NDFC
> > banks have chips attached. The "bank-width" property is 1 for 8-bit flash
> > and 2 for a 16-bit one.
> >
> > Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> > Signed-off-by: Valentine Barshak <vbarshak@ru.mvista.com>
>
> Are you sure you have the Signed-off-by from Thomas already on this?

Definitely not.

	tglx

[PATCH 2/2] PowerPC: NDFC entry for PowerPC 440EPx Sequoia DTS

[Valentine Barshak]

NDFC (NAND Flash Controller) DTS entry for PowerPC 440EPx Sequoia board.
The NDFC is relocatable in EBC space.

Signed-off-by: Valentine Barshak <vbarshak@ru.mvista.com>
---
 arch/powerpc/boot/dts/sequoia.dts |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+)

diff -pruN linux-2.6.orig/arch/powerpc/boot/dts/sequoia.dts linux-2.6.bld/arch/powerpc/boot/dts/sequoia.dts
--- linux-2.6.orig/arch/powerpc/boot/dts/sequoia.dts	2007-10-22 18:33:07.000000000 +0400
+++ linux-2.6.bld/arch/powerpc/boot/dts/sequoia.dts	2007-10-22 22:19:42.000000000 +0400
 		POB0: opb {
 		  	compatible = "ibm,opb-440epx", "ibm,opb";
 			#address-cells = <1>;
@@ -141,6 +148,26 @@
 				interrupts = <5 1>;
 				interrupt-parent = <&UIC1>;
 
+				ndfc@0,0 {
+					compatible = "ibm,ndfc-440epx", "ibm,ndfc";
+					reg = <3 000000 2000>;
+					bank-width = <1>;
+					bank-mask = <8>;
+					#address-cells = <1>;
+					#size-cells = <1>;
+					partition@0 {
+						label = "u-boot-nand";
+						reg = <0 0080000>;
+					};
+					partition@80000 {
+						label = "kernel-nand";
+						reg = <0080000 0180000>;
+					};
+					partition@200000 {
+						label = "filesystem";
+						reg = <0200000 1e00000>;
+					};
+				};
 				nor_flash@0,0 {
 					compatible = "amd,s29gl256n", "cfi-flash";
 					bank-width = <2>;

Re: [PATCH 2/2] PowerPC: NDFC entry for PowerPC 440EPx Sequoia DTS

[Sergei Shtylyov]

Hello.

Valentine Barshak wrote:

> NDFC (NAND Flash Controller) DTS entry for PowerPC 440EPx Sequoia board.
> The NDFC is relocatable in EBC space.

> Signed-off-by: Valentine Barshak <vbarshak@ru.mvista.com>

    You need to documents the layout in  the 
Documentation/powerpc/booting-without-of.txt file

WBR, Sergei

[PATCH] PowerPC: 44x device-tree aware NDFC bindings

[Valentine Barshak]

PowerPC 44x NAND Flash Controller (NDFC) bindings

Signed-off-by: Valentine Barshak <vbarshak@ru.mvista.com>
---
 Documentation/powerpc/booting-without-of.txt |   43 +++++++++++++++++++++++++++
 1 files changed, 43 insertions(+)

--- linux-2.6.orig/Documentation/powerpc/booting-without-of.txt	2007-10-26 19:01:43.000000000 +0400
+++ linux-2.6/Documentation/powerpc/booting-without-of.txt	2007-10-26 21:43:33.000000000 +0400
@@ -52,6 +52,7 @@ Table of Contents
       i) Freescale QUICC Engine module (QE)
       j) CFI or JEDEC memory-mapped NOR flash
       k) Global Utilities Block
+      l) 44x NanD Flash Controller (NDFC)
 
   VII - Specifying interrupt information for devices
     1) interrupts property
@@ -2242,6 +2243,48 @@ platforms are moved over to use the flat
 			   available.
 			   For Axon: 0x0000012a
 
+  l) 44x NanD Flash Controller (NDFC)
+
+  Required properties:
+  - compatible : should be "ibm,ndfc".
+  - reg : should contain at address and length of the NDFC registers
+  - bank-width : NAND chip bus width. Should be 1 for 8-bit NAND or 
+		 2 for 16-bit NAND
+  - bank-map : The first 4 bits of this property indicate which of the
+	       4 NDFC banks have chips attached.
+  - #address-cells, #size-cells : Must be present if the flash has
+    sub-nodes representing partitions (see below).  In this case
+    both #address-cells and #size-cells must be equal to 1.
+  
+  NDFC can have partition nodes, which are described the same way
+  as for the CFI or JEDEC memory-mapped NOR flash.
+
+   Example (Sequoia 440EPx):
+   NDFC is relocatable within EBC and should have EBC as a parent node.
+   Here we have NDFC on EBC CS3 bank:
+
+		ndfc@0,0 {
+			compatible = "ibm,ndfc-440epx", "ibm,ndfc";
+			reg = <3 000000 2000>;
+			bank-width = <1>;
+			bank-mask = <8>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+			partition@0 {
+				label = "u-boot-nand";
+				reg = <0 0080000>;
+			};
+			partition@80000 {
+				label = "kernel-nand";
+				reg = <0080000 0180000>;
+			};
+			partition@200000 {
+				label = "filesystem";
+				reg = <0200000 1e00000>;
+			};
+		};
+
+
    More devices will be defined as this spec matures.
 
 VII - Specifying interrupt information for devices

Re: [PATCH 0/2] PowerPC: Add 44x NDFC device-tree aware support

[Josh Boyer]

On Fri, 26 Oct 2007 20:30:58 +0400
Valentine Barshak <vbarshak@ru.mvista.com> wrote:

> I've worked in parallel with Stefan Roese on the new OF NDFC support.
> This version (as well as Stefan's) is based on the original NDFC driver by Thomas Gleixner.
> The major difference is that the original implements each chip connected NDFC banks as a
> separate MTD device. Here I try to have one MTD device spread on all chips found.
> However, the chips should have equal ID's and sizes, but I've never seen several different
> chips attached to single ndfc. 

Hm.  Why did you do that?  It adds restrictions to the driver that
really don't need to be present.  If people want a single MTD device
across multiple chips, they can use mtdconcat.

Remember, just because you've never seen something, doesn't mean it
doesn't exist.  Bamboo even has two different sized NAND chips
hooked to the NDFC, IIRC.

josh

Re: [PATCH 0/2] PowerPC: Add 44x NDFC device-tree aware support

[Thomas Gleixner]

On Fri, 26 Oct 2007, Valentine Barshak wrote:
> The major difference is that the original implements each chip connected NDFC banks as a
> separate MTD device. Here I try to have one MTD device spread on all chips found.
> However, the chips should have equal ID's and sizes, but I've never seen several different
> chips attached to single ndfc.

Bamboo has 2 different nand chips. And I'm aware of another board
which has a 2k-page onboard NAND and sockets for SmartMedia /
PictureXd cards, which will simply break with your design.

Restricting stuff for no good reason is never a good idea.

mtdconcat can build you a big one if you want, so why adding
restrictions to a driver ?

> I'm adding ndfc_of as a separate file, since some other changes
> have also been made (e.g. all i/o is made with ndfc_readl/writel inline functions).

This should be done in the original ndfc driver and not in a separate
incarnation.

> The original version didn't handle driver removal well (it never calls del_mtd...),it's 
> corrected here.

Why not fixing the original driver ?

> Any comments are greatly appreciated.

NACK.

Please fix the existing driver and convert it to deal with OF and fix
the other short comings as well.

Duplicate code is not going anywhere near drivers/mtd/nand

	tglx

Re: [PATCH 0/2] PowerPC: Add 44x NDFC device-tree aware support

[Stefan Roese]

Hi Valentine,

On Friday 26 October 2007, Thomas Gleixner wrote:
> On Fri, 26 Oct 2007, Valentine Barshak wrote:
> > The major difference is that the original implements each chip connected
> > NDFC banks as a separate MTD device. Here I try to have one MTD device
> > spread on all chips found. However, the chips should have equal ID's and
> > sizes, but I've never seen several different chips attached to single
> > ndfc.
>
> Bamboo has 2 different nand chips.

Right. This was my first thought after seeing your mail too.

> And I'm aware of another board 
> which has a 2k-page onboard NAND and sockets for SmartMedia /
> PictureXd cards, which will simply break with your design.
>
> Restricting stuff for no good reason is never a good idea.

Add my voice here too.

> mtdconcat can build you a big one if you want, so why adding
> restrictions to a driver ?
>
> > I'm adding ndfc_of as a separate file, since some other changes
> > have also been made (e.g. all i/o is made with ndfc_readl/writel inline
> > functions).
>
> This should be done in the original ndfc driver and not in a separate
> incarnation.

Right. And I always thought it would be a good idea to switch to using the 
in_be32() functions and friends for IO access in this driver too.

Best regards,
Stefan

Re: [PATCH 0/2] PowerPC: Add 44x NDFC device-tree aware support

[Valentine Barshak]

Thomas Gleixner wrote:
> On Fri, 26 Oct 2007, Valentine Barshak wrote:
>> The major difference is that the original implements each chip connected NDFC banks as a
>> separate MTD device. Here I try to have one MTD device spread on all chips found.
>> However, the chips should have equal ID's and sizes, but I've never seen several different
>> chips attached to single ndfc.
> 
> Bamboo has 2 different nand chips. And I'm aware of another board
> which has a 2k-page onboard NAND and sockets for SmartMedia /
> PictureXd cards, which will simply break with your design.
> 
> Restricting stuff for no good reason is never a good idea.
> 
> mtdconcat can build you a big one if you want, so why adding
> restrictions to a driver ?
> 
>> I'm adding ndfc_of as a separate file, since some other changes
>> have also been made (e.g. all i/o is made with ndfc_readl/writel inline functions).
> 
> This should be done in the original ndfc driver and not in a separate
> incarnation.
> 
>> The original version didn't handle driver removal well (it never calls del_mtd...),it's 
>> corrected here.
> 
> Why not fixing the original driver ?
> 
>> Any comments are greatly appreciated.
> 
> NACK.
> 
> Please fix the existing driver and convert it to deal with OF and fix
> the other short comings as well.
> 
> Duplicate code is not going anywhere near drivers/mtd/nand
> 
> 	tglx

Thanks all for your comments.
Well, let me explain why I did this.
First of all I should have checked twice, since I was thinking bamboo 
had identical chips :) I planned to add different chip support later a 
bit, just wanted to get a simple OF driver version working first. 
Surely, mtd concat can be used, but it adds a slight overhead for 
identical chips. Eventually, I wanted to make it support both separate 
mtd devices on each chip or spread across identical ones depending on 
the device-tree settings. The other thing is that the original driver 
lacked a distinct parent-child relationship between ndfc and chips. 
Simply registering chip driver only after ndfc is successfully 
registered doesn't guarantee we initialize chip device after ndfc is 
properly initialized.
Even if we set ndfc parent for the chip platform device as suggested by 
Stefan:
static struct platform_device nand_dev = {
	.name = "ndfc-chip",
	.id = 0,
	.num_resources = 1,
	.resource = &r,
	.dev = {
		.platform_data = &nand_chip,
		.parent = &ndfc_dev.dev,
	}
};
If for some reason ndfc probe fails the kernel will crash on the chip 
probe. Of course it would work most of the time, but I think that both 
initialization and clean-up has to be reworked in the original driver.
The original driver has a tight connection to the 
platform_device/platform_data structures. Stefan suggested a 
OF-to-platform device wrapper to make both versions work:
http://ozlabs.org/pipermail/linuxppc-dev/2007-October/044019.html
It's fine to have this glue code, but right now only 1 chip is 
supported. To add support for more chips we need an array of at least 4 
ndfc-chip platform devices. And this approach looks to me like inventing 
something new (OF) and then adding glue and quirks to make it work with 
the old stuff. Why invent new stuff then?
To make the original driver work with both "platform device" and new OF 
device descriptions need additional rework of the current ndfc code (add 
some #ifdefs, or completely split platform/OF and nand core stuff into 
separate files). I know that duplicating code is no good either, but 
since the original stuff is going to die and be removed anyway should it 
be a big problem?
So, these are just my thoughts.
Thanks,
Valentine.