Skip to contentSkip to conversation

PowerPC: Add 44x NDFC device-tree aware support

PATCH NAK Valentine Barshak 6 participants 12 messages Oct 26, 2007 – Oct 29, 2007

Subsystems (5)
lore
Share & tools
lore
#0 lore raw

cover PowerPC: Add 44x NDFC device-tree aware support

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.

#1 lore raw

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

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.

---
 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(+)
drivers/mtd/nand/Kconfig +7 -0
diff -pruN linux-2.6.orig/drivers/mtd/nand/Kconfig linux-2.6/drivers/mtd/nand/Kconfig
@@ -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
drivers/mtd/nand/Makefile +1 -0
diff -pruN linux-2.6.orig/drivers/mtd/nand/Makefile linux-2.6/drivers/mtd/nand/Makefile
@@ -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
drivers/mtd/nand/ndfc_of.c +449 -0

449 lines not on this page · raw message

include/linux/mtd/ndfc.h +4 -0
diff -pruN linux-2.6.orig/include/linux/mtd/ndfc.h linux-2.6/include/linux/mtd/ndfc.h
@@ -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
 
#2 lore raw
Quoted 12 lines · On Fri, 26 Oct 2007 20:39:58 +0400 Valentine Barshak <[email protected]>…
drivers/mtd/nand/ndfc_of.c +11 -0
+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 */

drivers/mtd/nand/ndfc_of.c line 215

Quoted 10 lines
drivers/mtd/nand/ndfc_of.c +10 -0
+	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))

drivers/mtd/nand/ndfc_of.c line 226

Quoted 11 lines
drivers/mtd/nand/ndfc_of.c +11 -0
+		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++)

drivers/mtd/nand/ndfc_of.c line 231

Quoted 5 lines
drivers/mtd/nand/ndfc_of.c +5 -0
+		const u32 *reg;
+		int len;
+
+		reg = of_get_property(pp, "reg", &len);
+		if (!reg || (len != 2*sizeof(u32))) {
			of_node_put(pp);
Quoted 13 lines
drivers/mtd/nand/ndfc_of.c +13 -0
+			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;

drivers/mtd/nand/ndfc_of.c line 254

Quoted 10 lines
drivers/mtd/nand/ndfc_of.c +10 -0
+
+		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;
}
Quoted 9 lines
drivers/mtd/nand/ndfc_of.c +8 -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;

drivers/mtd/nand/ndfc_of.c line 338

Quoted 10 lines
drivers/mtd/nand/ndfc_of.c +10 -0
+	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 ?

Signature · 3 lines
Cheers,
Stephen Rothwell                    [email protected]
http://www.canb.auug.org.au/~sfr/
#3 lore raw
In reply to Valentine Barshak

Hi Valentine,

Quoted 11 lines · 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.

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

Quoted 135 lines

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

drivers/mtd/nand/Kconfig +7 -0
+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.
+ *
+ */

drivers/mtd/nand/ndfc_of.c +57 -0
+#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) +{

drivers/mtd/nand/ndfc_of.c +20 -0
+	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;
+}
+

drivers/mtd/nand/ndfc_of.c line 102

Quoted 10 lines
drivers/mtd/nand/ndfc_of.c +10 -0
+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();

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

drivers/mtd/nand/ndfc_of.c line 114

Quoted 12 lines
drivers/mtd/nand/ndfc_of.c +12 -0
+}
+
+
+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();

Same here.

Quoted 127 lines
drivers/mtd/nand/ndfc_of.c +27 -0
+	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) +{

drivers/mtd/nand/ndfc_of.c +9 -0
+	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) +{

drivers/mtd/nand/ndfc_of.c +89 -0
+	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);

drivers/mtd/nand/ndfc_of.c line 251

Quoted 10 lines
drivers/mtd/nand/ndfc_of.c +10 -0
+		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;
+}

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

Quoted 203 lines
drivers/mtd/nand/ndfc_of.c +198 -0
+#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

include/linux/mtd/ndfc.h line 60

Quoted 9 lines
include/linux/mtd/ndfc.h +4 -0
@@ -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

#4 lore raw
Quoted 12 lines · Stefan Roese

Hi Valentine,

Quoted 9 lines · 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.

Quoted 4 lines
Quoted 2 lines

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

Definitely not.

tglx

#5 lore raw

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

In reply to Valentine Barshak

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

---
 arch/powerpc/boot/dts/sequoia.dts |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+)
arch/powerpc/boot/dts/sequoia.dts +0 -0
diff -pruN linux-2.6.orig/arch/powerpc/boot/dts/sequoia.dts linux-2.6.bld/arch/powerpc/boot/dts/sequoia.dts

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>;

#6 lore raw

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

Hello.

Quoted 4 lines · Valentine Barshak

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

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

WBR, Sergei

#7 lore raw

PATCH 1/1 PowerPC: 44x device-tree aware NDFC bindings

In reply to Valentine Barshak

PowerPC 44x NAND Flash Controller (NDFC) bindings

---
 Documentation/powerpc/booting-without-of.txt |   43 +++++++++++++++++++++++++++
 1 files changed, 43 insertions(+)
Documentation/powerpc/booting-without-of.txt +43 -0
@@ -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
#8 lore raw
In reply to Valentine Barshak
Quoted 6 lines · On Fri, 26 Oct 2007 20:30:58 +0400 Valentine Barshak <[email protected]>…

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

#9 lore raw
In reply to Valentine Barshak
Quoted 4 lines · Valentine Barshak

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 ?

Quoted 2 lines

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.

Quoted 2 lines

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

Why not fixing the original driver ?

Quoted 1 line

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

#10 lore raw

Hi Valentine,

Quoted 8 lines · Thomas Gleixner
Quoted 5 lines · Valentine Barshak

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.

Quoted 5 lines

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.

Quoted 9 lines

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

Quoted 3 lines

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

#11 lore raw
In reply to Thomas Gleixner
Quoted 27 lines · Thomas Gleixner
Quoted 4 lines · Valentine Barshak

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 ?

Quoted 2 lines

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.

Quoted 1 line

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

Why not fixing the original driver ?

Quoted 1 line

Any comments are greatly appreciated.

Quoted 8 lines

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.