[PATCH 1/3][MTD] P4080/eLBC: Make Freescale elbc interrupt common to elbc devices

[PATCH 1/3][MTD] P4080/eLBC: Make Freescale elbc interrupt common to elbc devices

[Roy Zang]

From: Lan Chunhe-B25806 <b25806@freescale.com>

Move Freescale elbc interrupt from nand dirver to elbc driver.
Then all elbc devices can use the interrupt instead of ONLY nand.

Signed-off-by: Lan Chunhe-B25806 <b25806@freescale.com>
Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
---
send the patch to linux-mtd@lists.infradead.org
it has been posted to linuxppc-dev@ozlabs.org and do not get any comment.

 arch/powerpc/Kconfig               |    7 +-
 arch/powerpc/include/asm/fsl_lbc.h |   34 +++++-
 arch/powerpc/sysdev/fsl_lbc.c      |  254 ++++++++++++++++++++++++++++++------
 3 files changed, 253 insertions(+), 42 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 2031a28..5155b53 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -703,9 +703,12 @@ config 4xx_SOC
 	bool
 
 config FSL_LBC
-	bool
+	bool "Freescale Local Bus support"
+	depends on FSL_SOC
 	help
-	  Freescale Localbus support
+	  Enables reporting of errors from the Freescale local bus
+	  controller.  Also contains some common code used by
+	  drivers for specific local bus peripherals.
 
 config FSL_GTM
 	bool
diff --git a/arch/powerpc/include/asm/fsl_lbc.h b/arch/powerpc/include/asm/fsl_lbc.h
index 1b5a210..9b95eab 100644
--- a/arch/powerpc/include/asm/fsl_lbc.h
+++ b/arch/powerpc/include/asm/fsl_lbc.h
@@ -1,9 +1,10 @@
 /* Freescale Local Bus Controller
  *
- * Copyright (c) 2006-2007 Freescale Semiconductor
+ * Copyright (c) 2006-2007, 2010 Freescale Semiconductor
  *
  * Authors: Nick Spence <nick.spence@freescale.com>,
  *          Scott Wood <scottwood@freescale.com>
+ *          Jack Lan <jack.lan@freescale.com>
  *
  * 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
@@ -27,6 +28,9 @@
 #include <linux/types.h>
 #include <linux/io.h>
 
+#include <linux/of_platform.h>
+#include <linux/interrupt.h>
+
 struct fsl_lbc_bank {
 	__be32 br;             /**< Base Register  */
 #define BR_BA           0xFFFF8000
@@ -125,13 +129,23 @@ struct fsl_lbc_regs {
 #define LTESR_ATMW 0x00800000
 #define LTESR_ATMR 0x00400000
 #define LTESR_CS   0x00080000
+#define LTESR_UPM  0x00000002
 #define LTESR_CC   0x00000001
 #define LTESR_NAND_MASK (LTESR_FCT | LTESR_PAR | LTESR_CC)
+#define LTESR_MASK      (LTESR_BM | LTESR_FCT | LTESR_PAR | LTESR_WP \
+			 | LTESR_ATMW | LTESR_ATMR | LTESR_CS | LTESR_UPM \
+			 | LTESR_CC)
+#define LTESR_CLEAR	0xFFFFFFFF
+#define LTECCR_CLEAR	0xFFFFFFFF
+#define LTESR_STATUS	LTESR_MASK
+#define LTEIR_ENABLE	LTESR_MASK
+#define LTEDR_ENABLE	0x00000000
 	__be32 ltedr;           /**< Transfer Error Disable Register */
 	__be32 lteir;           /**< Transfer Error Interrupt Register */
 	__be32 lteatr;          /**< Transfer Error Attributes Register */
 	__be32 ltear;           /**< Transfer Error Address Register */
-	u8 res6[0xC];
+	__be32 lteccr;          /**< Transfer Error ECC Register */
+	u8 res6[0x8];
 	__be32 lbcr;            /**< Configuration Register */
 #define LBCR_LDIS  0x80000000
 #define LBCR_LDIS_SHIFT    31
@@ -265,7 +279,23 @@ static inline void fsl_upm_end_pattern(struct fsl_upm *upm)
 		cpu_relax();
 }
 
+/* overview of the fsl lbc controller */
+
+struct fsl_lbc_ctrl {
+	/* device info */
+	struct device			*dev;
+	struct fsl_lbc_regs __iomem	*regs;
+	int				irq;
+	wait_queue_head_t		irq_wait;
+	spinlock_t			lock;
+	void				*nand;
+
+	/* status read from LTESR by irq handler */
+	unsigned int			irq_status;
+};
+
 extern int fsl_upm_run_pattern(struct fsl_upm *upm, void __iomem *io_base,
 			       u32 mar);
+extern struct fsl_lbc_ctrl *fsl_lbc_ctrl_dev;
 
 #endif /* __ASM_FSL_LBC_H */
diff --git a/arch/powerpc/sysdev/fsl_lbc.c b/arch/powerpc/sysdev/fsl_lbc.c
index dceb8d1..9c9e44f 100644
--- a/arch/powerpc/sysdev/fsl_lbc.c
+++ b/arch/powerpc/sysdev/fsl_lbc.c
@@ -5,6 +5,10 @@
  *
  * Author: Anton Vorontsov <avorontsov@ru.mvista.com>
  *
+ * Copyright (c) 2010 Freescale Semiconductor
+ *
+ * Authors: Jack Lan <Jack.Lan@freescale.com>
+ *
  * 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
@@ -23,35 +27,8 @@
 #include <asm/fsl_lbc.h>
 
 static spinlock_t fsl_lbc_lock = __SPIN_LOCK_UNLOCKED(fsl_lbc_lock);
-static struct fsl_lbc_regs __iomem *fsl_lbc_regs;
-
-static char __initdata *compat_lbc[] = {
-	"fsl,pq2-localbus",
-	"fsl,pq2pro-localbus",
-	"fsl,pq3-localbus",
-	"fsl,elbc",
-};
-
-static int __init fsl_lbc_init(void)
-{
-	struct device_node *lbus;
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(compat_lbc); i++) {
-		lbus = of_find_compatible_node(NULL, NULL, compat_lbc[i]);
-		if (lbus)
-			goto found;
-	}
-	return -ENODEV;
-
-found:
-	fsl_lbc_regs = of_iomap(lbus, 0);
-	of_node_put(lbus);
-	if (!fsl_lbc_regs)
-		return -ENOMEM;
-	return 0;
-}
-arch_initcall(fsl_lbc_init);
+struct fsl_lbc_ctrl *fsl_lbc_ctrl_dev;
+EXPORT_SYMBOL(fsl_lbc_ctrl_dev);
 
 /**
  * fsl_lbc_find - find Localbus bank
@@ -66,12 +43,12 @@ int fsl_lbc_find(phys_addr_t addr_base)
 {
 	int i;
 
-	if (!fsl_lbc_regs)
+	if (!fsl_lbc_ctrl_dev || !fsl_lbc_ctrl_dev->regs)
 		return -ENODEV;
 
-	for (i = 0; i < ARRAY_SIZE(fsl_lbc_regs->bank); i++) {
-		__be32 br = in_be32(&fsl_lbc_regs->bank[i].br);
-		__be32 or = in_be32(&fsl_lbc_regs->bank[i].or);
+	for (i = 0; i < ARRAY_SIZE(fsl_lbc_ctrl_dev->regs->bank); i++) {
+		__be32 br = in_be32(&fsl_lbc_ctrl_dev->regs->bank[i].br);
+		__be32 or = in_be32(&fsl_lbc_ctrl_dev->regs->bank[i].or);
 
 		if (br & BR_V && (br & or & BR_BA) == addr_base)
 			return i;
@@ -99,17 +76,20 @@ int fsl_upm_find(phys_addr_t addr_base, struct fsl_upm *upm)
 	if (bank < 0)
 		return bank;
 
-	br = in_be32(&fsl_lbc_regs->bank[bank].br);
+	if (!fsl_lbc_ctrl_dev || !fsl_lbc_ctrl_dev->regs)
+		return -ENODEV;
+
+	br = in_be32(&fsl_lbc_ctrl_dev->regs->bank[bank].br);
 
 	switch (br & BR_MSEL) {
 	case BR_MS_UPMA:
-		upm->mxmr = &fsl_lbc_regs->mamr;
+		upm->mxmr = &fsl_lbc_ctrl_dev->regs->mamr;
 		break;
 	case BR_MS_UPMB:
-		upm->mxmr = &fsl_lbc_regs->mbmr;
+		upm->mxmr = &fsl_lbc_ctrl_dev->regs->mbmr;
 		break;
 	case BR_MS_UPMC:
-		upm->mxmr = &fsl_lbc_regs->mcmr;
+		upm->mxmr = &fsl_lbc_ctrl_dev->regs->mcmr;
 		break;
 	default:
 		return -EINVAL;
@@ -143,14 +123,18 @@ EXPORT_SYMBOL(fsl_upm_find);
  * thus UPM pattern actually executed. Note that mar usage depends on the
  * pre-programmed AMX bits in the UPM RAM.
  */
+
 int fsl_upm_run_pattern(struct fsl_upm *upm, void __iomem *io_base, u32 mar)
 {
 	int ret = 0;
 	unsigned long flags;
 
+	if (!fsl_lbc_ctrl_dev || !fsl_lbc_ctrl_dev->regs)
+		return -ENODEV;
+
 	spin_lock_irqsave(&fsl_lbc_lock, flags);
 
-	out_be32(&fsl_lbc_regs->mar, mar);
+	out_be32(&fsl_lbc_ctrl_dev->regs->mar, mar);
 
 	switch (upm->width) {
 	case 8:
@@ -172,3 +156,197 @@ int fsl_upm_run_pattern(struct fsl_upm *upm, void __iomem *io_base, u32 mar)
 	return ret;
 }
 EXPORT_SYMBOL(fsl_upm_run_pattern);
+
+static int __devinit fsl_lbc_ctrl_init(struct fsl_lbc_ctrl *ctrl)
+{
+	struct fsl_lbc_regs __iomem *lbc = ctrl->regs;
+
+	/* clear event registers */
+	setbits32(&lbc->ltesr, LTESR_CLEAR);
+	out_be32(&lbc->lteatr, 0);
+	out_be32(&lbc->ltear, 0);
+	out_be32(&lbc->lteccr, LTECCR_CLEAR);
+	out_be32(&lbc->ltedr, LTEDR_ENABLE);
+
+	/* Enable interrupts for any detected events */
+	out_be32(&lbc->lteir, LTEIR_ENABLE);
+
+	return 0;
+}
+
+static int __devexit fsl_lbc_ctrl_remove(struct of_device *ofdev)
+{
+	struct fsl_lbc_ctrl *ctrl = dev_get_drvdata(&ofdev->dev);
+
+	if (ctrl->irq)
+		free_irq(ctrl->irq, ctrl);
+
+	if (ctrl->regs)
+		iounmap(ctrl->regs);
+
+	dev_set_drvdata(&ofdev->dev, NULL);
+	kfree(ctrl);
+
+	return 0;
+}
+
+/* NOTE: This interrupt is used to report localbus events of various kinds,
+ * such as transaction errors on the chipselects.
+ */
+
+static irqreturn_t fsl_lbc_ctrl_irq(int irqno, void *data)
+{
+	struct fsl_lbc_ctrl *ctrl = data;
+	struct fsl_lbc_regs __iomem *lbc = ctrl->regs;
+	u32 status;
+
+	status = in_be32(&lbc->ltesr);
+
+	if (status) {
+		out_be32(&lbc->ltesr, LTESR_CLEAR);
+		out_be32(&lbc->lteatr, 0);
+		out_be32(&lbc->ltear, 0);
+		ctrl->irq_status = status;
+
+		if (status & LTESR_BM)
+			dev_err(ctrl->dev, "Local bus monitor time-out: "
+				"LTESR 0x%08X\n", status);
+		if (status & LTESR_WP)
+			dev_err(ctrl->dev, "Write protect error: "
+				"LTESR 0x%08X\n", status);
+		if (status & LTESR_ATMW)
+			dev_err(ctrl->dev, "Atomic write error: "
+				"LTESR 0x%08X\n", status);
+		if (status & LTESR_ATMR)
+			dev_err(ctrl->dev, "Atomic read error: "
+				"LTESR 0x%08X\n", status);
+		if (status & LTESR_CS)
+			dev_err(ctrl->dev, "Chip select error: "
+				"LTESR 0x%08X\n", status);
+		if (status & LTESR_UPM)
+			;
+		if (status & LTESR_FCT) {
+			dev_err(ctrl->dev, "FCM command time-out: "
+				"LTESR 0x%08X\n", status);
+			smp_wmb();
+			wake_up(&ctrl->irq_wait);
+		}
+		if (status & LTESR_PAR) {
+			dev_err(ctrl->dev, "Parity or Uncorrectable ECC error: "
+				"LTESR 0x%08X\n", status);
+			smp_wmb();
+			wake_up(&ctrl->irq_wait);
+		}
+		if (status & LTESR_CC) {
+			smp_wmb();
+			wake_up(&ctrl->irq_wait);
+		}
+		if (status & ~LTESR_MASK)
+			dev_err(ctrl->dev, "Unknown error: "
+				"LTESR 0x%08X\n", status);
+
+		return IRQ_HANDLED;
+	}
+
+	return IRQ_NONE;
+}
+
+/* fsl_lbc_ctrl_probe
+ *
+ * called by device layer when it finds a device matching
+ * one our driver can handled. This code allocates all of
+ * the resources needed for the controller only.  The
+ * resources for the NAND banks themselves are allocated
+ * in the chip probe function.
+*/
+
+static int __devinit fsl_lbc_ctrl_probe(struct of_device *ofdev,
+					 const struct of_device_id *match)
+{
+	int ret = 0;
+
+	fsl_lbc_ctrl_dev = kzalloc(sizeof(*fsl_lbc_ctrl_dev), GFP_KERNEL);
+	if (!fsl_lbc_ctrl_dev)
+		return -ENOMEM;
+
+	dev_set_drvdata(&ofdev->dev, fsl_lbc_ctrl_dev);
+
+	spin_lock_init(&fsl_lbc_ctrl_dev->lock);
+	init_waitqueue_head(&fsl_lbc_ctrl_dev->irq_wait);
+
+	fsl_lbc_ctrl_dev->regs = of_iomap(ofdev->dev.of_node, 0);
+	if (!fsl_lbc_ctrl_dev->regs) {
+		dev_err(&ofdev->dev, "failed to get memory region\n");
+		ret = -ENODEV;
+		goto err;
+	}
+
+	fsl_lbc_ctrl_dev->irq = of_irq_to_resource(ofdev->dev.of_node, 0, NULL);
+	if (fsl_lbc_ctrl_dev->irq == NO_IRQ) {
+		dev_err(&ofdev->dev, "failed to get irq resource\n");
+		ret = -ENODEV;
+		goto err;
+	}
+
+	fsl_lbc_ctrl_dev->dev = &ofdev->dev;
+
+	ret = fsl_lbc_ctrl_init(fsl_lbc_ctrl_dev);
+	if (ret < 0)
+		goto err;
+
+	ret = request_irq(fsl_lbc_ctrl_dev->irq, fsl_lbc_ctrl_irq, 0,
+				"fsl-lbc", fsl_lbc_ctrl_dev);
+	if (ret != 0) {
+		dev_err(&ofdev->dev, "failed to install irq (%d)\n",
+			fsl_lbc_ctrl_dev->irq);
+		ret = fsl_lbc_ctrl_dev->irq;
+		goto err;
+	}
+
+	return 0;
+
+err:
+	return ret;
+}
+
+static const struct of_device_id fsl_lbc_match[] = {
+	{
+		.compatible = "fsl,elbc",
+	},
+	{
+		.compatible = "fsl,pq3-localbus",
+	},
+	{
+		.compatible = "fsl,pq2-localbus",
+	},
+	{
+		.compatible = "fsl,pq2pro-localbus",
+	},
+	{},
+};
+
+static struct of_platform_driver fsl_lbc_ctrl_driver = {
+	.driver = {
+		.name = "fsl-lbc",
+		.of_match_table = fsl_lbc_match,
+	},
+	.probe = fsl_lbc_ctrl_probe,
+	.remove = __devexit_p(fsl_lbc_ctrl_remove),
+};
+
+static int __init fsl_lbc_init(void)
+{
+	return of_register_platform_driver(&fsl_lbc_ctrl_driver);
+}
+
+static void __exit fsl_lbc_exit(void)
+{
+	of_unregister_platform_driver(&fsl_lbc_ctrl_driver);
+}
+
+module_init(fsl_lbc_init);
+module_exit(fsl_lbc_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Freescale Semiconductor");
+MODULE_DESCRIPTION("Freescale Enhanced Local Bus Controller driver");
-- 
1.5.6.5

[PATCH 2/3][MTD] P4080/nand: Only make elbc nand driver detect nand flash partitions

[Roy Zang]

From: Lan Chunhe-B25806 <b25806@freescale.com>

The former driver had the two functions:

1. detecting nand flash partitions;
2. registering elbc interrupt.

Now, second function is removed to fsl_lbc.c.

Signed-off-by: Lan Chunhe-B25806 <b25806@freescale.com>
Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
---
 drivers/mtd/nand/Kconfig         |    1 +
 drivers/mtd/nand/fsl_elbc_nand.c |  464 ++++++++++++++------------------------
 2 files changed, 170 insertions(+), 295 deletions(-)

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index ffc3720..4b4c82e 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -459,6 +459,7 @@ config MTD_NAND_ORION
 config MTD_NAND_FSL_ELBC
 	tristate "NAND support for Freescale eLBC controllers"
 	depends on MTD_NAND && PPC_OF
+	select FSL_LBC
 	help
 	  Various Freescale chips, including the 8313, include a NAND Flash
 	  Controller Module with built-in hardware ECC capabilities.
diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c
index 5084cc5..7bbcb3f 100644
--- a/drivers/mtd/nand/fsl_elbc_nand.c
+++ b/drivers/mtd/nand/fsl_elbc_nand.c
@@ -1,9 +1,10 @@
 /* Freescale Enhanced Local Bus Controller NAND driver
  *
- * Copyright (c) 2006-2007 Freescale Semiconductor
+ * Copyright (c) 2006-2007, 2010 Freescale Semiconductor
  *
  * Authors: Nick Spence <nick.spence@freescale.com>,
  *          Scott Wood <scottwood@freescale.com>
+ *          Jack Lan <jack.lan@freescale.com>
  *
  * 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
@@ -24,32 +25,21 @@
 #include <linux/types.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
-#include <linux/string.h>
-#include <linux/ioport.h>
-#include <linux/of_platform.h>
-#include <linux/slab.h>
-#include <linux/interrupt.h>
 
-#include <linux/mtd/mtd.h>
 #include <linux/mtd/nand.h>
-#include <linux/mtd/nand_ecc.h>
 #include <linux/mtd/partitions.h>
-
-#include <asm/io.h>
 #include <asm/fsl_lbc.h>
 
 #define MAX_BANKS 8
 #define ERR_BYTE 0xFF /* Value returned for read bytes when read failed */
 #define FCM_TIMEOUT_MSECS 500 /* Maximum number of mSecs to wait for FCM */
 
-struct fsl_elbc_ctrl;
-
 /* mtd information per set */
 
 struct fsl_elbc_mtd {
 	struct mtd_info mtd;
 	struct nand_chip chip;
-	struct fsl_elbc_ctrl *ctrl;
+	struct fsl_lbc_ctrl *ctrl;
 
 	struct device *dev;
 	int bank;               /* Chip select bank number           */
@@ -58,18 +48,12 @@ struct fsl_elbc_mtd {
 	unsigned int fmr;       /* FCM Flash Mode Register value     */
 };
 
-/* overview of the fsl elbc controller */
+/* Freescale eLBC FCM controller infomation */
 
-struct fsl_elbc_ctrl {
+struct fsl_elbc_fcm_ctrl {
 	struct nand_hw_control controller;
 	struct fsl_elbc_mtd *chips[MAX_BANKS];
 
-	/* device info */
-	struct device *dev;
-	struct fsl_lbc_regs __iomem *regs;
-	int irq;
-	wait_queue_head_t irq_wait;
-	unsigned int irq_status; /* status read from LTESR by irq handler */
 	u8 __iomem *addr;        /* Address of assigned FCM buffer        */
 	unsigned int page;       /* Last page written to / read from      */
 	unsigned int read_bytes; /* Number of bytes read during command   */
@@ -82,6 +66,8 @@ struct fsl_elbc_ctrl {
 	char *oob_poi;           /* Place to write ECC after read back    */
 };
 
+static struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl;
+
 /* These map to the positions used by the FCM hardware ECC generator */
 
 /* Small Page FLASH with FMR[ECCM] = 0 */
@@ -164,11 +150,11 @@ static void set_addr(struct mtd_info *mtd, int column, int page_addr, int oob)
 {
 	struct nand_chip *chip = mtd->priv;
 	struct fsl_elbc_mtd *priv = chip->priv;
-	struct fsl_elbc_ctrl *ctrl = priv->ctrl;
+	struct fsl_lbc_ctrl *ctrl = priv->ctrl;
 	struct fsl_lbc_regs __iomem *lbc = ctrl->regs;
 	int buf_num;
 
-	ctrl->page = page_addr;
+	elbc_fcm_ctrl->page = page_addr;
 
 	out_be32(&lbc->fbar,
 	         page_addr >> (chip->phys_erase_shift - chip->page_shift));
@@ -185,16 +171,18 @@ static void set_addr(struct mtd_info *mtd, int column, int page_addr, int oob)
 		buf_num = page_addr & 7;
 	}
 
-	ctrl->addr = priv->vbase + buf_num * 1024;
-	ctrl->index = column;
+	elbc_fcm_ctrl->addr = priv->vbase + buf_num * 1024;
+	elbc_fcm_ctrl->index = column;
 
 	/* for OOB data point to the second half of the buffer */
 	if (oob)
-		ctrl->index += priv->page_size ? 2048 : 512;
+		elbc_fcm_ctrl->index += priv->page_size ? 2048 : 512;
 
-	dev_vdbg(ctrl->dev, "set_addr: bank=%d, ctrl->addr=0x%p (0x%p), "
+	dev_vdbg(priv->dev, "set_addr: bank=%d, "
+			    "elbc_fcm_ctrl->addr=0x%p (0x%p), "
 	                    "index %x, pes %d ps %d\n",
-	         buf_num, ctrl->addr, priv->vbase, ctrl->index,
+		 buf_num, elbc_fcm_ctrl->addr, priv->vbase,
+		 elbc_fcm_ctrl->index,
 	         chip->phys_erase_shift, chip->page_shift);
 }
 
@@ -205,18 +193,18 @@ static int fsl_elbc_run_command(struct mtd_info *mtd)
 {
 	struct nand_chip *chip = mtd->priv;
 	struct fsl_elbc_mtd *priv = chip->priv;
-	struct fsl_elbc_ctrl *ctrl = priv->ctrl;
+	struct fsl_lbc_ctrl *ctrl = priv->ctrl;
 	struct fsl_lbc_regs __iomem *lbc = ctrl->regs;
 
 	/* Setup the FMR[OP] to execute without write protection */
 	out_be32(&lbc->fmr, priv->fmr | 3);
-	if (ctrl->use_mdr)
-		out_be32(&lbc->mdr, ctrl->mdr);
+	if (elbc_fcm_ctrl->use_mdr)
+		out_be32(&lbc->mdr, elbc_fcm_ctrl->mdr);
 
-	dev_vdbg(ctrl->dev,
+	dev_vdbg(priv->dev,
 	         "fsl_elbc_run_command: fmr=%08x fir=%08x fcr=%08x\n",
 	         in_be32(&lbc->fmr), in_be32(&lbc->fir), in_be32(&lbc->fcr));
-	dev_vdbg(ctrl->dev,
+	dev_vdbg(priv->dev,
 	         "fsl_elbc_run_command: fbar=%08x fpar=%08x "
 	         "fbcr=%08x bank=%d\n",
 	         in_be32(&lbc->fbar), in_be32(&lbc->fpar),
@@ -229,19 +217,18 @@ static int fsl_elbc_run_command(struct mtd_info *mtd)
 	/* wait for FCM complete flag or timeout */
 	wait_event_timeout(ctrl->irq_wait, ctrl->irq_status,
 	                   FCM_TIMEOUT_MSECS * HZ/1000);
-	ctrl->status = ctrl->irq_status;
-
+	elbc_fcm_ctrl->status = ctrl->irq_status;
 	/* store mdr value in case it was needed */
-	if (ctrl->use_mdr)
-		ctrl->mdr = in_be32(&lbc->mdr);
+	if (elbc_fcm_ctrl->use_mdr)
+		elbc_fcm_ctrl->mdr = in_be32(&lbc->mdr);
 
-	ctrl->use_mdr = 0;
+	elbc_fcm_ctrl->use_mdr = 0;
 
-	if (ctrl->status != LTESR_CC) {
-		dev_info(ctrl->dev,
+	if (elbc_fcm_ctrl->status != LTESR_CC) {
+		dev_info(priv->dev,
 		         "command failed: fir %x fcr %x status %x mdr %x\n",
 		         in_be32(&lbc->fir), in_be32(&lbc->fcr),
-		         ctrl->status, ctrl->mdr);
+			 elbc_fcm_ctrl->status, elbc_fcm_ctrl->mdr);
 		return -EIO;
 	}
 
@@ -251,7 +238,7 @@ static int fsl_elbc_run_command(struct mtd_info *mtd)
 static void fsl_elbc_do_read(struct nand_chip *chip, int oob)
 {
 	struct fsl_elbc_mtd *priv = chip->priv;
-	struct fsl_elbc_ctrl *ctrl = priv->ctrl;
+	struct fsl_lbc_ctrl *ctrl = priv->ctrl;
 	struct fsl_lbc_regs __iomem *lbc = ctrl->regs;
 
 	if (priv->page_size) {
@@ -284,15 +271,15 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
 {
 	struct nand_chip *chip = mtd->priv;
 	struct fsl_elbc_mtd *priv = chip->priv;
-	struct fsl_elbc_ctrl *ctrl = priv->ctrl;
+	struct fsl_lbc_ctrl *ctrl = priv->ctrl;
 	struct fsl_lbc_regs __iomem *lbc = ctrl->regs;
 
-	ctrl->use_mdr = 0;
+	elbc_fcm_ctrl->use_mdr = 0;
 
 	/* clear the read buffer */
-	ctrl->read_bytes = 0;
+	elbc_fcm_ctrl->read_bytes = 0;
 	if (command != NAND_CMD_PAGEPROG)
-		ctrl->index = 0;
+		elbc_fcm_ctrl->index = 0;
 
 	switch (command) {
 	/* READ0 and READ1 read the entire buffer to use hardware ECC. */
@@ -301,7 +288,7 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
 
 	/* fall-through */
 	case NAND_CMD_READ0:
-		dev_dbg(ctrl->dev,
+		dev_dbg(priv->dev,
 		        "fsl_elbc_cmdfunc: NAND_CMD_READ0, page_addr:"
 		        " 0x%x, column: 0x%x.\n", page_addr, column);
 
@@ -309,8 +296,8 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
 		out_be32(&lbc->fbcr, 0); /* read entire page to enable ECC */
 		set_addr(mtd, 0, page_addr, 0);
 
-		ctrl->read_bytes = mtd->writesize + mtd->oobsize;
-		ctrl->index += column;
+		elbc_fcm_ctrl->read_bytes = mtd->writesize + mtd->oobsize;
+		elbc_fcm_ctrl->index += column;
 
 		fsl_elbc_do_read(chip, 0);
 		fsl_elbc_run_command(mtd);
@@ -318,14 +305,14 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
 
 	/* READOOB reads only the OOB because no ECC is performed. */
 	case NAND_CMD_READOOB:
-		dev_vdbg(ctrl->dev,
+		dev_vdbg(priv->dev,
 		         "fsl_elbc_cmdfunc: NAND_CMD_READOOB, page_addr:"
 			 " 0x%x, column: 0x%x.\n", page_addr, column);
 
 		out_be32(&lbc->fbcr, mtd->oobsize - column);
 		set_addr(mtd, column, page_addr, 1);
 
-		ctrl->read_bytes = mtd->writesize + mtd->oobsize;
+		elbc_fcm_ctrl->read_bytes = mtd->writesize + mtd->oobsize;
 
 		fsl_elbc_do_read(chip, 1);
 		fsl_elbc_run_command(mtd);
@@ -333,7 +320,7 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
 
 	/* READID must read all 5 possible bytes while CEB is active */
 	case NAND_CMD_READID:
-		dev_vdbg(ctrl->dev, "fsl_elbc_cmdfunc: NAND_CMD_READID.\n");
+		dev_vdbg(priv->dev, "fsl_elbc_cmdfunc: NAND_CMD_READID.\n");
 
 		out_be32(&lbc->fir, (FIR_OP_CM0 << FIR_OP0_SHIFT) |
 		                    (FIR_OP_UA  << FIR_OP1_SHIFT) |
@@ -341,9 +328,9 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
 		out_be32(&lbc->fcr, NAND_CMD_READID << FCR_CMD0_SHIFT);
 		/* 5 bytes for manuf, device and exts */
 		out_be32(&lbc->fbcr, 5);
-		ctrl->read_bytes = 5;
-		ctrl->use_mdr = 1;
-		ctrl->mdr = 0;
+		elbc_fcm_ctrl->read_bytes = 5;
+		elbc_fcm_ctrl->use_mdr = 1;
+		elbc_fcm_ctrl->mdr = 0;
 
 		set_addr(mtd, 0, 0, 0);
 		fsl_elbc_run_command(mtd);
@@ -351,7 +338,7 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
 
 	/* ERASE1 stores the block and page address */
 	case NAND_CMD_ERASE1:
-		dev_vdbg(ctrl->dev,
+		dev_vdbg(priv->dev,
 		         "fsl_elbc_cmdfunc: NAND_CMD_ERASE1, "
 		         "page_addr: 0x%x.\n", page_addr);
 		set_addr(mtd, 0, page_addr, 0);
@@ -359,7 +346,7 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
 
 	/* ERASE2 uses the block and page address from ERASE1 */
 	case NAND_CMD_ERASE2:
-		dev_vdbg(ctrl->dev, "fsl_elbc_cmdfunc: NAND_CMD_ERASE2.\n");
+		dev_vdbg(priv->dev, "fsl_elbc_cmdfunc: NAND_CMD_ERASE2.\n");
 
 		out_be32(&lbc->fir,
 		         (FIR_OP_CM0 << FIR_OP0_SHIFT) |
@@ -374,8 +361,8 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
 		         (NAND_CMD_ERASE2 << FCR_CMD2_SHIFT));
 
 		out_be32(&lbc->fbcr, 0);
-		ctrl->read_bytes = 0;
-		ctrl->use_mdr = 1;
+		elbc_fcm_ctrl->read_bytes = 0;
+		elbc_fcm_ctrl->use_mdr = 1;
 
 		fsl_elbc_run_command(mtd);
 		return;
@@ -383,14 +370,12 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
 	/* SEQIN sets up the addr buffer and all registers except the length */
 	case NAND_CMD_SEQIN: {
 		__be32 fcr;
-		dev_vdbg(ctrl->dev,
-		         "fsl_elbc_cmdfunc: NAND_CMD_SEQIN/PAGE_PROG, "
+		dev_vdbg(priv->dev,
+			 "fsl_elbc_cmdfunc: NAND_CMD_SEQIN/PAGE_PROG, "
 		         "page_addr: 0x%x, column: 0x%x.\n",
 		         page_addr, column);
 
-		ctrl->column = column;
-		ctrl->oob = 0;
-		ctrl->use_mdr = 1;
+		elbc_fcm_ctrl->use_mdr = 1;
 
 		fcr = (NAND_CMD_STATUS   << FCR_CMD1_SHIFT) |
 		      (NAND_CMD_SEQIN    << FCR_CMD2_SHIFT) |
@@ -420,7 +405,7 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
 				/* OOB area --> READOOB */
 				column -= mtd->writesize;
 				fcr |= NAND_CMD_READOOB << FCR_CMD0_SHIFT;
-				ctrl->oob = 1;
+				elbc_fcm_ctrl->oob = 1;
 			} else {
 				WARN_ON(column != 0);
 				/* First 256 bytes --> READ0 */
@@ -429,24 +414,24 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
 		}
 
 		out_be32(&lbc->fcr, fcr);
-		set_addr(mtd, column, page_addr, ctrl->oob);
+		set_addr(mtd, column, page_addr, elbc_fcm_ctrl->oob);
 		return;
 	}
 
 	/* PAGEPROG reuses all of the setup from SEQIN and adds the length */
 	case NAND_CMD_PAGEPROG: {
 		int full_page;
-		dev_vdbg(ctrl->dev,
+		dev_vdbg(priv->dev,
 		         "fsl_elbc_cmdfunc: NAND_CMD_PAGEPROG "
-		         "writing %d bytes.\n", ctrl->index);
+			 "writing %d bytes.\n", elbc_fcm_ctrl->index);
 
 		/* if the write did not start at 0 or is not a full page
 		 * then set the exact length, otherwise use a full page
 		 * write so the HW generates the ECC.
 		 */
-		if (ctrl->oob || ctrl->column != 0 ||
-		    ctrl->index != mtd->writesize + mtd->oobsize) {
-			out_be32(&lbc->fbcr, ctrl->index);
+		if (elbc_fcm_ctrl->oob || elbc_fcm_ctrl->column != 0 ||
+		    elbc_fcm_ctrl->index != mtd->writesize + mtd->oobsize) {
+			out_be32(&lbc->fbcr, elbc_fcm_ctrl->index);
 			full_page = 0;
 		} else {
 			out_be32(&lbc->fbcr, 0);
@@ -458,21 +443,21 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
 		/* Read back the page in order to fill in the ECC for the
 		 * caller.  Is this really needed?
 		 */
-		if (full_page && ctrl->oob_poi) {
+		if (full_page && elbc_fcm_ctrl->oob_poi) {
 			out_be32(&lbc->fbcr, 3);
 			set_addr(mtd, 6, page_addr, 1);
 
-			ctrl->read_bytes = mtd->writesize + 9;
+			elbc_fcm_ctrl->read_bytes = mtd->writesize + 9;
 
 			fsl_elbc_do_read(chip, 1);
 			fsl_elbc_run_command(mtd);
 
-			memcpy_fromio(ctrl->oob_poi + 6,
-			              &ctrl->addr[ctrl->index], 3);
-			ctrl->index += 3;
+			memcpy_fromio(elbc_fcm_ctrl->oob_poi + 6,
+				&elbc_fcm_ctrl->addr[elbc_fcm_ctrl->index], 3);
+			elbc_fcm_ctrl->index += 3;
 		}
 
-		ctrl->oob_poi = NULL;
+		elbc_fcm_ctrl->oob_poi = NULL;
 		return;
 	}
 
@@ -485,26 +470,26 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
 		out_be32(&lbc->fcr, NAND_CMD_STATUS << FCR_CMD0_SHIFT);
 		out_be32(&lbc->fbcr, 1);
 		set_addr(mtd, 0, 0, 0);
-		ctrl->read_bytes = 1;
+		elbc_fcm_ctrl->read_bytes = 1;
 
 		fsl_elbc_run_command(mtd);
 
 		/* The chip always seems to report that it is
 		 * write-protected, even when it is not.
 		 */
-		setbits8(ctrl->addr, NAND_STATUS_WP);
+		setbits8(elbc_fcm_ctrl->addr, NAND_STATUS_WP);
 		return;
 
 	/* RESET without waiting for the ready line */
 	case NAND_CMD_RESET:
-		dev_dbg(ctrl->dev, "fsl_elbc_cmdfunc: NAND_CMD_RESET.\n");
+		dev_dbg(priv->dev, "fsl_elbc_cmdfunc: NAND_CMD_RESET.\n");
 		out_be32(&lbc->fir, FIR_OP_CM0 << FIR_OP0_SHIFT);
 		out_be32(&lbc->fcr, NAND_CMD_RESET << FCR_CMD0_SHIFT);
 		fsl_elbc_run_command(mtd);
 		return;
 
 	default:
-		dev_err(ctrl->dev,
+		dev_err(priv->dev,
 		        "fsl_elbc_cmdfunc: error, unsupported command 0x%x.\n",
 		        command);
 	}
@@ -524,24 +509,23 @@ static void fsl_elbc_write_buf(struct mtd_info *mtd, const u8 *buf, int len)
 {
 	struct nand_chip *chip = mtd->priv;
 	struct fsl_elbc_mtd *priv = chip->priv;
-	struct fsl_elbc_ctrl *ctrl = priv->ctrl;
 	unsigned int bufsize = mtd->writesize + mtd->oobsize;
 
 	if (len <= 0) {
-		dev_err(ctrl->dev, "write_buf of %d bytes", len);
-		ctrl->status = 0;
+		dev_err(priv->dev, "write_buf of %d bytes", len);
+		elbc_fcm_ctrl->status = 0;
 		return;
 	}
 
-	if ((unsigned int)len > bufsize - ctrl->index) {
-		dev_err(ctrl->dev,
+	if ((unsigned int)len > bufsize - elbc_fcm_ctrl->index) {
+		dev_err(priv->dev,
 		        "write_buf beyond end of buffer "
 		        "(%d requested, %u available)\n",
-		        len, bufsize - ctrl->index);
-		len = bufsize - ctrl->index;
+			len, bufsize - elbc_fcm_ctrl->index);
+		len = bufsize - elbc_fcm_ctrl->index;
 	}
 
-	memcpy_toio(&ctrl->addr[ctrl->index], buf, len);
+	memcpy_toio(&elbc_fcm_ctrl->addr[elbc_fcm_ctrl->index], buf, len);
 	/*
 	 * This is workaround for the weird elbc hangs during nand write,
 	 * Scott Wood says: "...perhaps difference in how long it takes a
@@ -549,9 +533,9 @@ static void fsl_elbc_write_buf(struct mtd_info *mtd, const u8 *buf, int len)
 	 * is causing problems, and sync isn't helping for some reason."
 	 * Reading back the last byte helps though.
 	 */
-	in_8(&ctrl->addr[ctrl->index] + len - 1);
+	in_8(&elbc_fcm_ctrl->addr[elbc_fcm_ctrl->index] + len - 1);
 
-	ctrl->index += len;
+	elbc_fcm_ctrl->index += len;
 }
 
 /*
@@ -562,13 +546,12 @@ static u8 fsl_elbc_read_byte(struct mtd_info *mtd)
 {
 	struct nand_chip *chip = mtd->priv;
 	struct fsl_elbc_mtd *priv = chip->priv;
-	struct fsl_elbc_ctrl *ctrl = priv->ctrl;
 
 	/* If there are still bytes in the FCM, then use the next byte. */
-	if (ctrl->index < ctrl->read_bytes)
-		return in_8(&ctrl->addr[ctrl->index++]);
+	if (elbc_fcm_ctrl->index < elbc_fcm_ctrl->read_bytes)
+		return in_8(&elbc_fcm_ctrl->addr[elbc_fcm_ctrl->index++]);
 
-	dev_err(ctrl->dev, "read_byte beyond end of buffer\n");
+	dev_err(priv->dev, "read_byte beyond end of buffer\n");
 	return ERR_BYTE;
 }
 
@@ -579,18 +562,18 @@ static void fsl_elbc_read_buf(struct mtd_info *mtd, u8 *buf, int len)
 {
 	struct nand_chip *chip = mtd->priv;
 	struct fsl_elbc_mtd *priv = chip->priv;
-	struct fsl_elbc_ctrl *ctrl = priv->ctrl;
 	int avail;
 
 	if (len < 0)
 		return;
 
-	avail = min((unsigned int)len, ctrl->read_bytes - ctrl->index);
-	memcpy_fromio(buf, &ctrl->addr[ctrl->index], avail);
-	ctrl->index += avail;
+	avail = min((unsigned int)len,
+			elbc_fcm_ctrl->read_bytes - elbc_fcm_ctrl->index);
+	memcpy_fromio(buf, &elbc_fcm_ctrl->addr[elbc_fcm_ctrl->index], avail);
+	elbc_fcm_ctrl->index += avail;
 
 	if (len > avail)
-		dev_err(ctrl->dev,
+		dev_err(priv->dev,
 		        "read_buf beyond end of buffer "
 		        "(%d requested, %d available)\n",
 		        len, avail);
@@ -603,30 +586,31 @@ static int fsl_elbc_verify_buf(struct mtd_info *mtd, const u_char *buf, int len)
 {
 	struct nand_chip *chip = mtd->priv;
 	struct fsl_elbc_mtd *priv = chip->priv;
-	struct fsl_elbc_ctrl *ctrl = priv->ctrl;
 	int i;
 
 	if (len < 0) {
-		dev_err(ctrl->dev, "write_buf of %d bytes", len);
+		dev_err(priv->dev, "write_buf of %d bytes", len);
 		return -EINVAL;
 	}
 
-	if ((unsigned int)len > ctrl->read_bytes - ctrl->index) {
-		dev_err(ctrl->dev,
-		        "verify_buf beyond end of buffer "
-		        "(%d requested, %u available)\n",
-		        len, ctrl->read_bytes - ctrl->index);
+	if ((unsigned int)len >
+			elbc_fcm_ctrl->read_bytes - elbc_fcm_ctrl->index) {
+		dev_err(priv->dev,
+			"verify_buf beyond end of buffer "
+			"(%d requested, %u available)\n",
+			len, elbc_fcm_ctrl->read_bytes - elbc_fcm_ctrl->index);
 
-		ctrl->index = ctrl->read_bytes;
+		elbc_fcm_ctrl->index = elbc_fcm_ctrl->read_bytes;
 		return -EINVAL;
 	}
 
 	for (i = 0; i < len; i++)
-		if (in_8(&ctrl->addr[ctrl->index + i]) != buf[i])
+		if (in_8(&elbc_fcm_ctrl->addr[elbc_fcm_ctrl->index + i])
+				!= buf[i])
 			break;
 
-	ctrl->index += len;
-	return i == len && ctrl->status == LTESR_CC ? 0 : -EIO;
+	elbc_fcm_ctrl->index += len;
+	return i == len && elbc_fcm_ctrl->status == LTESR_CC ? 0 : -EIO;
 }
 
 /* This function is called after Program and Erase Operations to
@@ -634,23 +618,20 @@ static int fsl_elbc_verify_buf(struct mtd_info *mtd, const u_char *buf, int len)
  */
 static int fsl_elbc_wait(struct mtd_info *mtd, struct nand_chip *chip)
 {
-	struct fsl_elbc_mtd *priv = chip->priv;
-	struct fsl_elbc_ctrl *ctrl = priv->ctrl;
-
-	if (ctrl->status != LTESR_CC)
+	if (elbc_fcm_ctrl->status != LTESR_CC)
 		return NAND_STATUS_FAIL;
 
 	/* The chip always seems to report that it is
 	 * write-protected, even when it is not.
 	 */
-	return (ctrl->mdr & 0xff) | NAND_STATUS_WP;
+	return (elbc_fcm_ctrl->mdr & 0xff) | NAND_STATUS_WP;
 }
 
 static int fsl_elbc_chip_init_tail(struct mtd_info *mtd)
 {
 	struct nand_chip *chip = mtd->priv;
 	struct fsl_elbc_mtd *priv = chip->priv;
-	struct fsl_elbc_ctrl *ctrl = priv->ctrl;
+	struct fsl_lbc_ctrl *ctrl = priv->ctrl;
 	struct fsl_lbc_regs __iomem *lbc = ctrl->regs;
 	unsigned int al;
 
@@ -665,41 +646,41 @@ static int fsl_elbc_chip_init_tail(struct mtd_info *mtd)
 	priv->fmr |= (12 << FMR_CWTO_SHIFT) |  /* Timeout > 12 ms */
 	             (al << FMR_AL_SHIFT);
 
-	dev_dbg(ctrl->dev, "fsl_elbc_init: nand->numchips = %d\n",
+	dev_dbg(priv->dev, "fsl_elbc_init: nand->numchips = %d\n",
 	        chip->numchips);
-	dev_dbg(ctrl->dev, "fsl_elbc_init: nand->chipsize = %lld\n",
+	dev_dbg(priv->dev, "fsl_elbc_init: nand->chipsize = %lld\n",
 	        chip->chipsize);
-	dev_dbg(ctrl->dev, "fsl_elbc_init: nand->pagemask = %8x\n",
+	dev_dbg(priv->dev, "fsl_elbc_init: nand->pagemask = %8x\n",
 	        chip->pagemask);
-	dev_dbg(ctrl->dev, "fsl_elbc_init: nand->chip_delay = %d\n",
+	dev_dbg(priv->dev, "fsl_elbc_init: nand->chip_delay = %d\n",
 	        chip->chip_delay);
-	dev_dbg(ctrl->dev, "fsl_elbc_init: nand->badblockpos = %d\n",
+	dev_dbg(priv->dev, "fsl_elbc_init: nand->badblockpos = %d\n",
 	        chip->badblockpos);
-	dev_dbg(ctrl->dev, "fsl_elbc_init: nand->chip_shift = %d\n",
+	dev_dbg(priv->dev, "fsl_elbc_init: nand->chip_shift = %d\n",
 	        chip->chip_shift);
-	dev_dbg(ctrl->dev, "fsl_elbc_init: nand->page_shift = %d\n",
+	dev_dbg(priv->dev, "fsl_elbc_init: nand->page_shift = %d\n",
 	        chip->page_shift);
-	dev_dbg(ctrl->dev, "fsl_elbc_init: nand->phys_erase_shift = %d\n",
+	dev_dbg(priv->dev, "fsl_elbc_init: nand->phys_erase_shift = %d\n",
 	        chip->phys_erase_shift);
-	dev_dbg(ctrl->dev, "fsl_elbc_init: nand->ecclayout = %p\n",
+	dev_dbg(priv->dev, "fsl_elbc_init: nand->ecclayout = %p\n",
 	        chip->ecclayout);
-	dev_dbg(ctrl->dev, "fsl_elbc_init: nand->ecc.mode = %d\n",
+	dev_dbg(priv->dev, "fsl_elbc_init: nand->ecc.mode = %d\n",
 	        chip->ecc.mode);
-	dev_dbg(ctrl->dev, "fsl_elbc_init: nand->ecc.steps = %d\n",
+	dev_dbg(priv->dev, "fsl_elbc_init: nand->ecc.steps = %d\n",
 	        chip->ecc.steps);
-	dev_dbg(ctrl->dev, "fsl_elbc_init: nand->ecc.bytes = %d\n",
+	dev_dbg(priv->dev, "fsl_elbc_init: nand->ecc.bytes = %d\n",
 	        chip->ecc.bytes);
-	dev_dbg(ctrl->dev, "fsl_elbc_init: nand->ecc.total = %d\n",
+	dev_dbg(priv->dev, "fsl_elbc_init: nand->ecc.total = %d\n",
 	        chip->ecc.total);
-	dev_dbg(ctrl->dev, "fsl_elbc_init: nand->ecc.layout = %p\n",
+	dev_dbg(priv->dev, "fsl_elbc_init: nand->ecc.layout = %p\n",
 	        chip->ecc.layout);
-	dev_dbg(ctrl->dev, "fsl_elbc_init: mtd->flags = %08x\n", mtd->flags);
-	dev_dbg(ctrl->dev, "fsl_elbc_init: mtd->size = %lld\n", mtd->size);
-	dev_dbg(ctrl->dev, "fsl_elbc_init: mtd->erasesize = %d\n",
+	dev_dbg(priv->dev, "fsl_elbc_init: mtd->flags = %08x\n", mtd->flags);
+	dev_dbg(priv->dev, "fsl_elbc_init: mtd->size = %lld\n", mtd->size);
+	dev_dbg(priv->dev, "fsl_elbc_init: mtd->erasesize = %d\n",
 	        mtd->erasesize);
-	dev_dbg(ctrl->dev, "fsl_elbc_init: mtd->writesize = %d\n",
+	dev_dbg(priv->dev, "fsl_elbc_init: mtd->writesize = %d\n",
 	        mtd->writesize);
-	dev_dbg(ctrl->dev, "fsl_elbc_init: mtd->oobsize = %d\n",
+	dev_dbg(priv->dev, "fsl_elbc_init: mtd->oobsize = %d\n",
 	        mtd->oobsize);
 
 	/* adjust Option Register and ECC to match Flash page size */
@@ -719,7 +700,7 @@ static int fsl_elbc_chip_init_tail(struct mtd_info *mtd)
 			chip->badblock_pattern = &largepage_memorybased;
 		}
 	} else {
-		dev_err(ctrl->dev,
+		dev_err(priv->dev,
 		        "fsl_elbc_init: page size %d is not supported\n",
 		        mtd->writesize);
 		return -1;
@@ -749,18 +730,15 @@ static void fsl_elbc_write_page(struct mtd_info *mtd,
                                 struct nand_chip *chip,
                                 const uint8_t *buf)
 {
-	struct fsl_elbc_mtd *priv = chip->priv;
-	struct fsl_elbc_ctrl *ctrl = priv->ctrl;
-
 	fsl_elbc_write_buf(mtd, buf, mtd->writesize);
 	fsl_elbc_write_buf(mtd, chip->oob_poi, mtd->oobsize);
 
-	ctrl->oob_poi = chip->oob_poi;
+	elbc_fcm_ctrl->oob_poi = chip->oob_poi;
 }
 
 static int fsl_elbc_chip_init(struct fsl_elbc_mtd *priv)
 {
-	struct fsl_elbc_ctrl *ctrl = priv->ctrl;
+	struct fsl_lbc_ctrl *ctrl = priv->ctrl;
 	struct fsl_lbc_regs __iomem *lbc = ctrl->regs;
 	struct nand_chip *chip = &priv->chip;
 
@@ -790,7 +768,7 @@ static int fsl_elbc_chip_init(struct fsl_elbc_mtd *priv)
 	chip->options = NAND_NO_READRDY | NAND_NO_AUTOINCR |
 			NAND_USE_FLASH_BBT;
 
-	chip->controller = &ctrl->controller;
+	chip->controller = &elbc_fcm_ctrl->controller;
 	chip->priv = priv;
 
 	chip->ecc.read_page = fsl_elbc_read_page;
@@ -815,8 +793,6 @@ static int fsl_elbc_chip_init(struct fsl_elbc_mtd *priv)
 
 static int fsl_elbc_chip_remove(struct fsl_elbc_mtd *priv)
 {
-	struct fsl_elbc_ctrl *ctrl = priv->ctrl;
-
 	nand_release(&priv->mtd);
 
 	kfree(priv->mtd.name);
@@ -824,16 +800,16 @@ static int fsl_elbc_chip_remove(struct fsl_elbc_mtd *priv)
 	if (priv->vbase)
 		iounmap(priv->vbase);
 
-	ctrl->chips[priv->bank] = NULL;
+	elbc_fcm_ctrl->chips[priv->bank] = NULL;
 	kfree(priv);
 
 	return 0;
 }
 
-static int __devinit fsl_elbc_chip_probe(struct fsl_elbc_ctrl *ctrl,
-					 struct device_node *node)
+static int __devinit fsl_elbc_nand_probe(struct of_device *dev,
+					 const struct of_device_id *match)
 {
-	struct fsl_lbc_regs __iomem *lbc = ctrl->regs;
+	struct fsl_lbc_regs __iomem *lbc;
 	struct fsl_elbc_mtd *priv;
 	struct resource res;
 #ifdef CONFIG_MTD_PARTITIONS
@@ -843,11 +819,16 @@ static int __devinit fsl_elbc_chip_probe(struct fsl_elbc_ctrl *ctrl,
 #endif
 	int ret;
 	int bank;
+	struct device_node *node = dev->dev.of_node;
+
+	if (!fsl_lbc_ctrl_dev || !fsl_lbc_ctrl_dev->regs)
+		return -ENODEV;
+	lbc = fsl_lbc_ctrl_dev->regs;
 
 	/* get, allocate and map the memory resource */
 	ret = of_address_to_resource(node, 0, &res);
 	if (ret) {
-		dev_err(ctrl->dev, "failed to get resource\n");
+		dev_err(fsl_lbc_ctrl_dev->dev, "failed to get resource\n");
 		return ret;
 	}
 
@@ -861,7 +842,8 @@ static int __devinit fsl_elbc_chip_probe(struct fsl_elbc_ctrl *ctrl,
 			break;
 
 	if (bank >= MAX_BANKS) {
-		dev_err(ctrl->dev, "address did not match any chip selects\n");
+		dev_err(fsl_lbc_ctrl_dev->dev, "address did not match any "
+			"chip selects\n");
 		return -ENODEV;
 	}
 
@@ -869,14 +851,28 @@ static int __devinit fsl_elbc_chip_probe(struct fsl_elbc_ctrl *ctrl,
 	if (!priv)
 		return -ENOMEM;
 
-	ctrl->chips[bank] = priv;
+	if (fsl_lbc_ctrl_dev->nand == NULL) {
+		elbc_fcm_ctrl = kzalloc(sizeof(*elbc_fcm_ctrl), GFP_KERNEL);
+		if (!elbc_fcm_ctrl)
+			return -ENOMEM;
+
+		elbc_fcm_ctrl->read_bytes = 0;
+		elbc_fcm_ctrl->index = 0;
+		elbc_fcm_ctrl->addr = NULL;
+
+		spin_lock_init(&elbc_fcm_ctrl->controller.lock);
+		init_waitqueue_head(&elbc_fcm_ctrl->controller.wq);
+		fsl_lbc_ctrl_dev->nand = elbc_fcm_ctrl;
+	}
+
+	elbc_fcm_ctrl->chips[bank] = priv;
 	priv->bank = bank;
-	priv->ctrl = ctrl;
-	priv->dev = ctrl->dev;
+	priv->ctrl = fsl_lbc_ctrl_dev;
+	priv->dev = fsl_lbc_ctrl_dev->dev;
 
 	priv->vbase = ioremap(res.start, resource_size(&res));
 	if (!priv->vbase) {
-		dev_err(ctrl->dev, "failed to map chip region\n");
+		dev_err(fsl_lbc_ctrl_dev->dev, "failed to map chip region\n");
 		ret = -ENOMEM;
 		goto err;
 	}
@@ -933,171 +929,49 @@ err:
 	return ret;
 }
 
-static int __devinit fsl_elbc_ctrl_init(struct fsl_elbc_ctrl *ctrl)
+static int fsl_elbc_nand_remove(struct of_device *ofdev)
 {
-	struct fsl_lbc_regs __iomem *lbc = ctrl->regs;
-
-	/*
-	 * NAND transactions can tie up the bus for a long time, so set the
-	 * bus timeout to max by clearing LBCR[BMT] (highest base counter
-	 * value) and setting LBCR[BMTPS] to the highest prescaler value.
-	 */
-	clrsetbits_be32(&lbc->lbcr, LBCR_BMT, 15);
-
-	/* clear event registers */
-	setbits32(&lbc->ltesr, LTESR_NAND_MASK);
-	out_be32(&lbc->lteatr, 0);
-
-	/* Enable interrupts for any detected events */
-	out_be32(&lbc->lteir, LTESR_NAND_MASK);
-
-	ctrl->read_bytes = 0;
-	ctrl->index = 0;
-	ctrl->addr = NULL;
-
-	return 0;
-}
-
-static int fsl_elbc_ctrl_remove(struct of_device *ofdev)
-{
-	struct fsl_elbc_ctrl *ctrl = dev_get_drvdata(&ofdev->dev);
 	int i;
 
 	for (i = 0; i < MAX_BANKS; i++)
-		if (ctrl->chips[i])
-			fsl_elbc_chip_remove(ctrl->chips[i]);
-
-	if (ctrl->irq)
-		free_irq(ctrl->irq, ctrl);
-
-	if (ctrl->regs)
-		iounmap(ctrl->regs);
-
-	dev_set_drvdata(&ofdev->dev, NULL);
-	kfree(ctrl);
-	return 0;
-}
+		if (elbc_fcm_ctrl->chips[i])
+			fsl_elbc_chip_remove(elbc_fcm_ctrl->chips[i]);
 
-/* NOTE: This interrupt is also used to report other localbus events,
- * such as transaction errors on other chipselects.  If we want to
- * capture those, we'll need to move the IRQ code into a shared
- * LBC driver.
- */
-
-static irqreturn_t fsl_elbc_ctrl_irq(int irqno, void *data)
-{
-	struct fsl_elbc_ctrl *ctrl = data;
-	struct fsl_lbc_regs __iomem *lbc = ctrl->regs;
-	__be32 status = in_be32(&lbc->ltesr) & LTESR_NAND_MASK;
-
-	if (status) {
-		out_be32(&lbc->ltesr, status);
-		out_be32(&lbc->lteatr, 0);
-
-		ctrl->irq_status = status;
-		smp_wmb();
-		wake_up(&ctrl->irq_wait);
-
-		return IRQ_HANDLED;
-	}
-
-	return IRQ_NONE;
-}
-
-/* fsl_elbc_ctrl_probe
- *
- * called by device layer when it finds a device matching
- * one our driver can handled. This code allocates all of
- * the resources needed for the controller only.  The
- * resources for the NAND banks themselves are allocated
- * in the chip probe function.
-*/
-
-static int __devinit fsl_elbc_ctrl_probe(struct of_device *ofdev,
-                                         const struct of_device_id *match)
-{
-	struct device_node *child;
-	struct fsl_elbc_ctrl *ctrl;
-	int ret;
-
-	ctrl = kzalloc(sizeof(*ctrl), GFP_KERNEL);
-	if (!ctrl)
-		return -ENOMEM;
-
-	dev_set_drvdata(&ofdev->dev, ctrl);
-
-	spin_lock_init(&ctrl->controller.lock);
-	init_waitqueue_head(&ctrl->controller.wq);
-	init_waitqueue_head(&ctrl->irq_wait);
-
-	ctrl->regs = of_iomap(ofdev->dev.of_node, 0);
-	if (!ctrl->regs) {
-		dev_err(&ofdev->dev, "failed to get memory region\n");
-		ret = -ENODEV;
-		goto err;
-	}
-
-	ctrl->irq = of_irq_to_resource(ofdev->dev.of_node, 0, NULL);
-	if (ctrl->irq == NO_IRQ) {
-		dev_err(&ofdev->dev, "failed to get irq resource\n");
-		ret = -ENODEV;
-		goto err;
-	}
-
-	ctrl->dev = &ofdev->dev;
-
-	ret = fsl_elbc_ctrl_init(ctrl);
-	if (ret < 0)
-		goto err;
-
-	ret = request_irq(ctrl->irq, fsl_elbc_ctrl_irq, 0, "fsl-elbc", ctrl);
-	if (ret != 0) {
-		dev_err(&ofdev->dev, "failed to install irq (%d)\n",
-		        ctrl->irq);
-		ret = ctrl->irq;
-		goto err;
-	}
-
-	for_each_child_of_node(ofdev->dev.of_node, child)
-		if (of_device_is_compatible(child, "fsl,elbc-fcm-nand"))
-			fsl_elbc_chip_probe(ctrl, child);
+	fsl_lbc_ctrl_dev->nand = NULL;
+	kfree(elbc_fcm_ctrl);
 
 	return 0;
-
-err:
-	fsl_elbc_ctrl_remove(ofdev);
-	return ret;
 }
 
-static const struct of_device_id fsl_elbc_match[] = {
+static const struct of_device_id fsl_elbc_nand_match[] = {
 	{
-		.compatible = "fsl,elbc",
+		.compatible = "fsl,elbc-fcm-nand",
 	},
 	{}
 };
 
-static struct of_platform_driver fsl_elbc_ctrl_driver = {
+static struct of_platform_driver fsl_elbc_nand_driver = {
 	.driver = {
-		.name = "fsl-elbc",
+		.name = "fsl,elbc-fcm-nand",
 		.owner = THIS_MODULE,
-		.of_match_table = fsl_elbc_match,
+		.of_match_table = fsl_elbc_nand_match,
 	},
-	.probe = fsl_elbc_ctrl_probe,
-	.remove = fsl_elbc_ctrl_remove,
+	.probe = fsl_elbc_nand_probe,
+	.remove = fsl_elbc_nand_remove,
 };
 
-static int __init fsl_elbc_init(void)
+static int __init fsl_elbc_nand_init(void)
 {
-	return of_register_platform_driver(&fsl_elbc_ctrl_driver);
+	return of_register_platform_driver(&fsl_elbc_nand_driver);
 }
 
-static void __exit fsl_elbc_exit(void)
+static void __exit fsl_elbc_nand_exit(void)
 {
-	of_unregister_platform_driver(&fsl_elbc_ctrl_driver);
+	of_unregister_platform_driver(&fsl_elbc_nand_driver);
 }
 
-module_init(fsl_elbc_init);
-module_exit(fsl_elbc_exit);
+module_init(fsl_elbc_nand_init);
+module_exit(fsl_elbc_nand_exit);
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Freescale");
-- 
1.5.6.5

[PATCH 3/3][MTD] P4080/nand: Fix the freescale lbc issue with 36bit mode

[Roy Zang]

From: Lan Chunhe-B25806 <b25806@freescale.com>

When system uses 36bit physical address, res.start is 36bit
physical address. But the function of in_be32 returns 32bit
physical address. Then both of them compared each other is
wrong. So by converting the address of res.start into
the right format fixes this issue.

Signed-off-by: Lan Chunhe-B25806 <b25806@freescale.com>
Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
---
 arch/powerpc/include/asm/fsl_lbc.h |    1 +
 arch/powerpc/sysdev/fsl_lbc.c      |   33 ++++++++++++++++++++++++++++++++-
 drivers/mtd/nand/fsl_elbc_nand.c   |    2 +-
 3 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/fsl_lbc.h b/arch/powerpc/include/asm/fsl_lbc.h
index 9b95eab..28dcf63 100644
--- a/arch/powerpc/include/asm/fsl_lbc.h
+++ b/arch/powerpc/include/asm/fsl_lbc.h
@@ -249,6 +249,7 @@ struct fsl_upm {
 	int width;
 };
 
+extern unsigned int convert_lbc_address(phys_addr_t addr_base);
 extern int fsl_lbc_find(phys_addr_t addr_base);
 extern int fsl_upm_find(phys_addr_t addr_base, struct fsl_upm *upm);
 
diff --git a/arch/powerpc/sysdev/fsl_lbc.c b/arch/powerpc/sysdev/fsl_lbc.c
index 9c9e44f..08f98d8 100644
--- a/arch/powerpc/sysdev/fsl_lbc.c
+++ b/arch/powerpc/sysdev/fsl_lbc.c
@@ -31,6 +31,36 @@ struct fsl_lbc_ctrl *fsl_lbc_ctrl_dev;
 EXPORT_SYMBOL(fsl_lbc_ctrl_dev);
 
 /**
+ * convert_lbc_address - convert the base address
+ * @addr_base:	base address of the memory bank
+ *
+ * This function converts a base address of lbc into the right format for the BR
+ * registers. If the SOC has eLBC then it returns 32bit physical address else
+ * it returns 34bit physical address for local bus(Example: MPC8641).
+ */
+unsigned int convert_lbc_address(phys_addr_t addr_base)
+{
+	void *dev;
+	int compatible;
+
+	dev = of_find_node_by_name(NULL, "localbus");
+	if (!dev) {
+		printk(KERN_INFO "fsl-lbc: can't find localbus node\n");
+		of_node_put(dev);
+		return 0;
+	}
+
+	compatible = of_device_is_compatible(dev, "fsl,elbc");
+	of_node_put(dev);
+	if (compatible)
+		return addr_base & 0xffff8000;
+	else
+		return (addr_base & 0x0ffff8000ull) \
+			| ((addr_base & 0x300000000ull) >> 19);
+}
+EXPORT_SYMBOL(convert_lbc_address);
+
+/**
  * fsl_lbc_find - find Localbus bank
  * @addr_base:	base address of the memory bank
  *
@@ -50,7 +80,8 @@ int fsl_lbc_find(phys_addr_t addr_base)
 		__be32 br = in_be32(&fsl_lbc_ctrl_dev->regs->bank[i].br);
 		__be32 or = in_be32(&fsl_lbc_ctrl_dev->regs->bank[i].or);
 
-		if (br & BR_V && (br & or & BR_BA) == addr_base)
+		if (br & BR_V && (br & or & BR_BA) \
+				== convert_lbc_address(addr_base))
 			return i;
 	}
 
diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c
index 7bbcb3f..0e8dc40 100644
--- a/drivers/mtd/nand/fsl_elbc_nand.c
+++ b/drivers/mtd/nand/fsl_elbc_nand.c
@@ -838,7 +838,7 @@ static int __devinit fsl_elbc_nand_probe(struct of_device *dev,
 		    (in_be32(&lbc->bank[bank].br) & BR_MSEL) == BR_MS_FCM &&
 		    (in_be32(&lbc->bank[bank].br) &
 		     in_be32(&lbc->bank[bank].or) & BR_BA)
-		     == res.start)
+		     == convert_lbc_address(res.start))
 			break;
 
 	if (bank >= MAX_BANKS) {
-- 
1.5.6.5

RE: [PATCH 1/3][MTD] P4080/eLBC: Make Freescale elbc interrupt common to elbc devices

[Zang Roy-R61911]

 

> -----Original Message-----
> From: Zang Roy-R61911 
> Sent: Friday, August 06, 2010 10:52 AM
> To: linux-mtd@lists.infradead.org
> Cc: linuxppc-dev@ozlabs.org; akpm@linux-foundation.org; Gala 
> Kumar-B11780; Lan Chunhe-B25806
> Subject: [PATCH 1/3][MTD] P4080/eLBC: Make Freescale elbc 
> interrupt common to elbc devices
> 
> From: Lan Chunhe-B25806 <b25806@freescale.com>
> 
> Move Freescale elbc interrupt from nand dirver to elbc driver.
> Then all elbc devices can use the interrupt instead of ONLY nand.
> 
> Signed-off-by: Lan Chunhe-B25806 <b25806@freescale.com>
> Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
> ---
> send the patch to linux-mtd@lists.infradead.org
> it has been posted to linuxppc-dev@ozlabs.org and do not get 
> any comment.
Any comment about this serial patches?
If none, I'd ask Andrew to merge to his mm tree.
Thanks.
Roy

RE: [PATCH 1/3][MTD] P4080/eLBC: Make Freescale elbc interrupt common to elbc devices

[Zang Roy-R61911]

 

> -----Original Message-----
> From: Zang Roy-R61911 
> Sent: Friday, August 06, 2010 10:52 AM
> To: linux-mtd@lists.infradead.org
> Cc: linuxppc-dev@ozlabs.org; akpm@linux-foundation.org; Gala 
> Kumar-B11780; Lan Chunhe-B25806
> Subject: [PATCH 1/3][MTD] P4080/eLBC: Make Freescale elbc 
> interrupt common to elbc devices
> 
> From: Lan Chunhe-B25806 <b25806@freescale.com>
> 
> Move Freescale elbc interrupt from nand dirver to elbc driver.
> Then all elbc devices can use the interrupt instead of ONLY nand.
> 
> Signed-off-by: Lan Chunhe-B25806 <b25806@freescale.com>
> Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
> ---
> send the patch to linux-mtd@lists.infradead.org
> it has been posted to linuxppc-dev@ozlabs.org and do not get 
> any comment.
> 
>  arch/powerpc/Kconfig               |    7 +-
>  arch/powerpc/include/asm/fsl_lbc.h |   34 +++++-
>  arch/powerpc/sysdev/fsl_lbc.c      |  254 
> ++++++++++++++++++++++++++++++------
>  3 files changed, 253 insertions(+), 42 deletions(-)
Who is response to review the nand driver in mtd list?
Please help to  comment on this serial patch.
Thanks.
Roy

RE: [PATCH 1/3][MTD] P4080/eLBC: Make Freescale elbc interrupt common to elbc devices

[Artem Bityutskiy]

On Mon, 2010-08-09 at 15:33 +0800, Zang Roy-R61911 wrote:
> Any comment about this serial patches?
> If none, I'd ask Andrew to merge to his mm tree.

Could you please separate out MTD stuff, to the extent it is possible to
do?
-- 
Best Regards,
Artem Bityutskiy (Битюцкий Артём)

RE: [PATCH 1/3][MTD] P4080/eLBC: Make Freescale elbc interrupt common to elbc devices

[Zang Roy-R61911]

> -----Original Message-----
> From: Zang Roy-R61911
> Sent: Friday, August 06, 2010 10:52 AM
> To: linux-mtd@lists.infradead.org
> Cc: linuxppc-dev@ozlabs.org; akpm@linux-foundation.org; Gala
Kumar-B11780; Lan
> Chunhe-B25806
> Subject: [PATCH 1/3][MTD] P4080/eLBC: Make Freescale elbc interrupt
common to
> elbc devices
> 
> From: Lan Chunhe-B25806 <b25806@freescale.com>
> 
> Move Freescale elbc interrupt from nand dirver to elbc driver.
> Then all elbc devices can use the interrupt instead of ONLY nand.
> 
> Signed-off-by: Lan Chunhe-B25806 <b25806@freescale.com>
> Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
> ---
> send the patch to linux-mtd@lists.infradead.org
> it has been posted to linuxppc-dev@ozlabs.org and do not get any
comment.

Hi, Kumar

I can see that this patch delegates to you in the mail list.
Do you have any comment?
Today, I rebase the patch to Linux 2.6.26-rc3.
1/3, 3/3 are still OK for the latest tree.
For patch 2/3, some minor fix needs to change for of_dev to
platform_device.

Should I resend the patch?
Thanks.
Roy

Re: [PATCH 1/3][MTD] P4080/eLBC: Make Freescale elbc interrupt common to elbc devices

[Anton Vorontsov]

On Fri, Aug 06, 2010 at 10:51:34AM +0800, Roy Zang wrote:
> From: Lan Chunhe-B25806 <b25806@freescale.com>
> 
> Move Freescale elbc interrupt from nand dirver to elbc driver.
> Then all elbc devices can use the interrupt instead of ONLY nand.
> 
> Signed-off-by: Lan Chunhe-B25806 <b25806@freescale.com>
> Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
> ---
> send the patch to linux-mtd@lists.infradead.org
> it has been posted to linuxppc-dev@ozlabs.org and do not get any comment.
> 
>  arch/powerpc/Kconfig               |    7 +-
>  arch/powerpc/include/asm/fsl_lbc.h |   34 +++++-
>  arch/powerpc/sysdev/fsl_lbc.c      |  254 ++++++++++++++++++++++++++++++------
>  3 files changed, 253 insertions(+), 42 deletions(-)
> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 2031a28..5155b53 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -703,9 +703,12 @@ config 4xx_SOC
>  	bool
>  
>  config FSL_LBC
> -	bool
> +	bool "Freescale Local Bus support"
> +	depends on FSL_SOC
>  	help
> -	  Freescale Localbus support
> +	  Enables reporting of errors from the Freescale local bus
> +	  controller.  Also contains some common code used by
> +	  drivers for specific local bus peripherals.
>  
>  config FSL_GTM
>  	bool
[...]
> diff --git a/arch/powerpc/sysdev/fsl_lbc.c b/arch/powerpc/sysdev/fsl_lbc.c
> index dceb8d1..9c9e44f 100644
> --- a/arch/powerpc/sysdev/fsl_lbc.c
> +++ b/arch/powerpc/sysdev/fsl_lbc.c
> @@ -5,6 +5,10 @@
>   *
>   * Author: Anton Vorontsov <avorontsov@ru.mvista.com>
>   *
> + * Copyright (c) 2010 Freescale Semiconductor
> + *
> + * Authors: Jack Lan <Jack.Lan@freescale.com>

Would be prettier if you'd group copyright and authorship notices.
I.e.

Copyright 2008 MV
Copyright 2010 FSL

Authors: Anton
         Jack

> + *
>   * 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
> @@ -23,35 +27,8 @@
>  #include <asm/fsl_lbc.h>
>  
>  static spinlock_t fsl_lbc_lock = __SPIN_LOCK_UNLOCKED(fsl_lbc_lock);
> -static struct fsl_lbc_regs __iomem *fsl_lbc_regs;
> -
> -static char __initdata *compat_lbc[] = {
> -	"fsl,pq2-localbus",
> -	"fsl,pq2pro-localbus",
> -	"fsl,pq3-localbus",
> -	"fsl,elbc",
> -};
> -
> -static int __init fsl_lbc_init(void)
> -{
> -	struct device_node *lbus;
> -	int i;
> -
> -	for (i = 0; i < ARRAY_SIZE(compat_lbc); i++) {
> -		lbus = of_find_compatible_node(NULL, NULL, compat_lbc[i]);
> -		if (lbus)
> -			goto found;
> -	}
> -	return -ENODEV;
> -
> -found:
> -	fsl_lbc_regs = of_iomap(lbus, 0);
> -	of_node_put(lbus);
> -	if (!fsl_lbc_regs)
> -		return -ENOMEM;
> -	return 0;
> -}
> -arch_initcall(fsl_lbc_init);
> +struct fsl_lbc_ctrl *fsl_lbc_ctrl_dev;
> +EXPORT_SYMBOL(fsl_lbc_ctrl_dev);
>  
>  /**
>   * fsl_lbc_find - find Localbus bank
> @@ -66,12 +43,12 @@ int fsl_lbc_find(phys_addr_t addr_base)
>  {
>  	int i;
>  
> -	if (!fsl_lbc_regs)
> +	if (!fsl_lbc_ctrl_dev || !fsl_lbc_ctrl_dev->regs)
>  		return -ENODEV;
>  
> -	for (i = 0; i < ARRAY_SIZE(fsl_lbc_regs->bank); i++) {
> -		__be32 br = in_be32(&fsl_lbc_regs->bank[i].br);
> -		__be32 or = in_be32(&fsl_lbc_regs->bank[i].or);
> +	for (i = 0; i < ARRAY_SIZE(fsl_lbc_ctrl_dev->regs->bank); i++) {
> +		__be32 br = in_be32(&fsl_lbc_ctrl_dev->regs->bank[i].br);
> +		__be32 or = in_be32(&fsl_lbc_ctrl_dev->regs->bank[i].or);

A dedicated variable for regs would make this much more readable?

>  
>  		if (br & BR_V && (br & or & BR_BA) == addr_base)
>  			return i;
> @@ -99,17 +76,20 @@ int fsl_upm_find(phys_addr_t addr_base, struct fsl_upm *upm)
>  	if (bank < 0)
>  		return bank;
>  
> -	br = in_be32(&fsl_lbc_regs->bank[bank].br);
> +	if (!fsl_lbc_ctrl_dev || !fsl_lbc_ctrl_dev->regs)
> +		return -ENODEV;
> +
> +	br = in_be32(&fsl_lbc_ctrl_dev->regs->bank[bank].br);
>  
>  	switch (br & BR_MSEL) {
>  	case BR_MS_UPMA:
> -		upm->mxmr = &fsl_lbc_regs->mamr;
> +		upm->mxmr = &fsl_lbc_ctrl_dev->regs->mamr;

Ditto, a very repetitive stuff, desires a variable for regs?

>  		break;
>  	case BR_MS_UPMB:
> -		upm->mxmr = &fsl_lbc_regs->mbmr;
> +		upm->mxmr = &fsl_lbc_ctrl_dev->regs->mbmr;
>  		break;
>  	case BR_MS_UPMC:
> -		upm->mxmr = &fsl_lbc_regs->mcmr;
> +		upm->mxmr = &fsl_lbc_ctrl_dev->regs->mcmr;
>  		break;
>  	default:
>  		return -EINVAL;
> @@ -143,14 +123,18 @@ EXPORT_SYMBOL(fsl_upm_find);
>   * thus UPM pattern actually executed. Note that mar usage depends on the
>   * pre-programmed AMX bits in the UPM RAM.
>   */
> +
>  int fsl_upm_run_pattern(struct fsl_upm *upm, void __iomem *io_base, u32 mar)
>  {
>  	int ret = 0;
>  	unsigned long flags;
>  
> +	if (!fsl_lbc_ctrl_dev || !fsl_lbc_ctrl_dev->regs)
> +		return -ENODEV;
> +
>  	spin_lock_irqsave(&fsl_lbc_lock, flags);
>  
> -	out_be32(&fsl_lbc_regs->mar, mar);
> +	out_be32(&fsl_lbc_ctrl_dev->regs->mar, mar);
>  
>  	switch (upm->width) {
>  	case 8:
> @@ -172,3 +156,197 @@ int fsl_upm_run_pattern(struct fsl_upm *upm, void __iomem *io_base, u32 mar)
>  	return ret;
>  }
>  EXPORT_SYMBOL(fsl_upm_run_pattern);
> +
> +static int __devinit fsl_lbc_ctrl_init(struct fsl_lbc_ctrl *ctrl)
> +{
> +	struct fsl_lbc_regs __iomem *lbc = ctrl->regs;

Yep, something like this for the functions above.

> +
> +	/* clear event registers */
> +	setbits32(&lbc->ltesr, LTESR_CLEAR);
> +	out_be32(&lbc->lteatr, 0);
> +	out_be32(&lbc->ltear, 0);
> +	out_be32(&lbc->lteccr, LTECCR_CLEAR);
> +	out_be32(&lbc->ltedr, LTEDR_ENABLE);
> +
> +	/* Enable interrupts for any detected events */
> +	out_be32(&lbc->lteir, LTEIR_ENABLE);
> +
> +	return 0;
> +}
> +
> +static int __devexit fsl_lbc_ctrl_remove(struct of_device *ofdev)
> +{
> +	struct fsl_lbc_ctrl *ctrl = dev_get_drvdata(&ofdev->dev);
> +
> +	if (ctrl->irq)
> +		free_irq(ctrl->irq, ctrl);
> +
> +	if (ctrl->regs)
> +		iounmap(ctrl->regs);
> +
> +	dev_set_drvdata(&ofdev->dev, NULL);
> +	kfree(ctrl);
> +
> +	return 0;
> +}
> +
> +/* NOTE: This interrupt is used to report localbus events of various kinds,
> + * such as transaction errors on the chipselects.
> + */

/*
 * multi line comments
 * should be like this
 */

[...]
> +/* fsl_lbc_ctrl_probe
> + *
> + * called by device layer when it finds a device matching
> + * one our driver can handled. This code allocates all of
> + * the resources needed for the controller only.  The
> + * resources for the NAND banks themselves are allocated
> + * in the chip probe function.
> +*/

ditto.

> +static int __devinit fsl_lbc_ctrl_probe(struct of_device *ofdev,
> +					 const struct of_device_id *match)
> +{
> +	int ret = 0;

no need for the initial value here.

> +
> +	fsl_lbc_ctrl_dev = kzalloc(sizeof(*fsl_lbc_ctrl_dev), GFP_KERNEL);
> +	if (!fsl_lbc_ctrl_dev)
> +		return -ENOMEM;
> +
> +	dev_set_drvdata(&ofdev->dev, fsl_lbc_ctrl_dev);
> +
> +	spin_lock_init(&fsl_lbc_ctrl_dev->lock);
> +	init_waitqueue_head(&fsl_lbc_ctrl_dev->irq_wait);
> +
> +	fsl_lbc_ctrl_dev->regs = of_iomap(ofdev->dev.of_node, 0);
> +	if (!fsl_lbc_ctrl_dev->regs) {
> +		dev_err(&ofdev->dev, "failed to get memory region\n");
> +		ret = -ENODEV;
> +		goto err;
> +	}
> +
> +	fsl_lbc_ctrl_dev->irq = of_irq_to_resource(ofdev->dev.of_node, 0, NULL);
> +	if (fsl_lbc_ctrl_dev->irq == NO_IRQ) {
> +		dev_err(&ofdev->dev, "failed to get irq resource\n");
> +		ret = -ENODEV;
> +		goto err;
> +	}
> +
> +	fsl_lbc_ctrl_dev->dev = &ofdev->dev;
> +
> +	ret = fsl_lbc_ctrl_init(fsl_lbc_ctrl_dev);
> +	if (ret < 0)
> +		goto err;
> +
> +	ret = request_irq(fsl_lbc_ctrl_dev->irq, fsl_lbc_ctrl_irq, 0,
> +				"fsl-lbc", fsl_lbc_ctrl_dev);
> +	if (ret != 0) {
> +		dev_err(&ofdev->dev, "failed to install irq (%d)\n",
> +			fsl_lbc_ctrl_dev->irq);
> +		ret = fsl_lbc_ctrl_dev->irq;
> +		goto err;
> +	}
> +
> +	return 0;
> +
> +err:

fsl_lbc_ctrl_dev leaked. Plus, after freeing it, you should
NULLify it as well, as other functions checks it for !NULL.

fsl_lbc_ctrl_dev->regs leaks too.

> +	return ret;
> +}
> +
> +static const struct of_device_id fsl_lbc_match[] = {
> +	{
> +		.compatible = "fsl,elbc",
> +	},
> +	{
> +		.compatible = "fsl,pq3-localbus",
> +	},
> +	{
> +		.compatible = "fsl,pq2-localbus",
> +	},
> +	{
> +		.compatible = "fsl,pq2pro-localbus",
> +	},
> +	{},

Could save 8 lines by writing "{ .compatible = "...", },".

> +};
> +
> +static struct of_platform_driver fsl_lbc_ctrl_driver = {

I think you shouldn't use of_platform for new code. just platform
will work (there's platform_driver.driver.of_match_table nowadays).

> +	.driver = {
> +		.name = "fsl-lbc",
> +		.of_match_table = fsl_lbc_match,
> +	},
> +	.probe = fsl_lbc_ctrl_probe,
> +	.remove = __devexit_p(fsl_lbc_ctrl_remove),

The device is not removable, I think you don't need this.

> +};
> +
> +static int __init fsl_lbc_init(void)
> +{
> +	return of_register_platform_driver(&fsl_lbc_ctrl_driver);
> +}
> +
> +static void __exit fsl_lbc_exit(void)
> +{
> +	of_unregister_platform_driver(&fsl_lbc_ctrl_driver);
> +}
> +
> +module_init(fsl_lbc_init);
> +module_exit(fsl_lbc_exit);

fsl_lbc is not a module, so you don't need _exit().

> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Freescale Semiconductor");
> +MODULE_DESCRIPTION("Freescale Enhanced Local Bus Controller driver");

ditto, no need for this.

Thanks,

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Re: [PATCH 3/3][MTD] P4080/nand: Fix the freescale lbc issue with 36bit mode

[Anton Vorontsov]

On Fri, Aug 06, 2010 at 10:51:36AM +0800, Roy Zang wrote:
[...]
>  /**
> + * convert_lbc_address - convert the base address
> + * @addr_base:	base address of the memory bank
> + *
> + * This function converts a base address of lbc into the right format for the BR
> + * registers. If the SOC has eLBC then it returns 32bit physical address else
> + * it returns 34bit physical address for local bus(Example: MPC8641).
> + */
> +unsigned int convert_lbc_address(phys_addr_t addr_base)
> +{
> +	void *dev;
> +	int compatible;
> +
> +	dev = of_find_node_by_name(NULL, "localbus");

Nope, you shouldn't do this. Never search by name.

Also, aren't there already a global dev, which was found by the
_probe() stuff?

> +	if (!dev) {
> +		printk(KERN_INFO "fsl-lbc: can't find localbus node\n");
> +		of_node_put(dev);
> +		return 0;
> +	}
> +
> +	compatible = of_device_is_compatible(dev, "fsl,elbc");
> +	of_node_put(dev);
> +	if (compatible)
> +		return addr_base & 0xffff8000;
> +	else
> +		return (addr_base & 0x0ffff8000ull) \
> +			| ((addr_base & 0x300000000ull) >> 19);
> +}
> +EXPORT_SYMBOL(convert_lbc_address);
> +
> +/**
>   * fsl_lbc_find - find Localbus bank
>   * @addr_base:	base address of the memory bank
>   *
> @@ -50,7 +80,8 @@ int fsl_lbc_find(phys_addr_t addr_base)
>  		__be32 br = in_be32(&fsl_lbc_ctrl_dev->regs->bank[i].br);
>  		__be32 or = in_be32(&fsl_lbc_ctrl_dev->regs->bank[i].or);
>  
> -		if (br & BR_V && (br & or & BR_BA) == addr_base)
> +		if (br & BR_V && (br & or & BR_BA) \

No need for "\" at the end of the line, keep == on the same line.

> +				== convert_lbc_address(addr_base))

Would be prettier if you name it fsl_lbc_addr(). Keeps prefix
the same for the rest of the file, plus makes it shorter (so
there probably won't be any need for breaking the line).

>  			return i;
>  	}
>  
> diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c
> index 7bbcb3f..0e8dc40 100644
> --- a/drivers/mtd/nand/fsl_elbc_nand.c
> +++ b/drivers/mtd/nand/fsl_elbc_nand.c
> @@ -838,7 +838,7 @@ static int __devinit fsl_elbc_nand_probe(struct of_device *dev,
>  		    (in_be32(&lbc->bank[bank].br) & BR_MSEL) == BR_MS_FCM &&
>  		    (in_be32(&lbc->bank[bank].br) &
>  		     in_be32(&lbc->bank[bank].or) & BR_BA)
> -		     == res.start)
> +		     == convert_lbc_address(res.start))
>  			break;
>  
>  	if (bank >= MAX_BANKS) {
> -- 
> 1.5.6.5

Thanks,

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Re: [PATCH 2/3][MTD] P4080/nand: Only make elbc nand driver detect nand flash partitions

[Anton Vorontsov]

On Fri, Aug 06, 2010 at 10:51:35AM +0800, Roy Zang wrote:
[...]
>  
> +static struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl;
> +

Are you sure that you want it as a global var? A bit scary change.

Oh, you probably don't need it, as you can get it from
fsl_lbc_ctrl_dev->nand?

I wonder if Scott saw these patches? Cc'ed.

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Re: [PATCH 2/3][MTD] P4080/nand: Only make elbc nand driver detect nand flash partitions

[Scott Wood]

On Fri, 3 Sep 2010 15:43:57 +0400
Anton Vorontsov <cbouatmailru@gmail.com> wrote:

> On Fri, Aug 06, 2010 at 10:51:35AM +0800, Roy Zang wrote:
> [...]
> >  
> > +static struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl;
> > +
> 
> Are you sure that you want it as a global var? A bit scary change.
> 
> Oh, you probably don't need it, as you can get it from
> fsl_lbc_ctrl_dev->nand?
> 
> I wonder if Scott saw these patches? Cc'ed.

I saw many iterations of these patches. :-)

I had the same reaction to this, but during internal review it did not
seem to be the most pressing concern to focus on.

In practice, there will only be one eLBC, and properly handling multiple
devices and binding the right one across this new boundary would have
been unnecessary complexity.

-Scott

RE: [PATCH 1/3][MTD] P4080/eLBC: Make Freescale elbc interrupt common to elbc devices

[Zang Roy-R61911]

> -----Original Message-----
> From: Anton Vorontsov [mailto:cbouatmailru@gmail.com]
> Sent: Friday, September 03, 2010 19:28 PM
> To: Zang Roy-R61911
> Cc: linux-mtd@lists.infradead.org; Lan Chunhe-B25806; linuxppc-dev@ozlabs.org;
> akpm@linux-foundation.org; Gala Kumar-B11780
> Subject: Re: [PATCH 1/3][MTD] P4080/eLBC: Make Freescale elbc interrupt common
> to elbc devices
> 
> On Fri, Aug 06, 2010 at 10:51:34AM +0800, Roy Zang wrote:
> > From: Lan Chunhe-B25806 <b25806@freescale.com>
> >
> > Move Freescale elbc interrupt from nand dirver to elbc driver.
> > Then all elbc devices can use the interrupt instead of ONLY nand.
> >
> > Signed-off-by: Lan Chunhe-B25806 <b25806@freescale.com>
> > Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
> > ---
> > send the patch to linux-mtd@lists.infradead.org
> > it has been posted to linuxppc-dev@ozlabs.org and do not get any comment.
> >
> >  arch/powerpc/Kconfig               |    7 +-
> >  arch/powerpc/include/asm/fsl_lbc.h |   34 +++++-
> >  arch/powerpc/sysdev/fsl_lbc.c      |  254 ++++++++++++++++++++++++++++++---
> ---
> >  3 files changed, 253 insertions(+), 42 deletions(-)
> >
> > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> > index 2031a28..5155b53 100644
> > --- a/arch/powerpc/Kconfig
> > +++ b/arch/powerpc/Kconfig
> > @@ -703,9 +703,12 @@ config 4xx_SOC
> >  	bool
> >
> >  config FSL_LBC
> > -	bool
> > +	bool "Freescale Local Bus support"
> > +	depends on FSL_SOC
> >  	help
> > -	  Freescale Localbus support
> > +	  Enables reporting of errors from the Freescale local bus
> > +	  controller.  Also contains some common code used by
> > +	  drivers for specific local bus peripherals.
> >
> >  config FSL_GTM
> >  	bool
> [...]
> > diff --git a/arch/powerpc/sysdev/fsl_lbc.c b/arch/powerpc/sysdev/fsl_lbc.c
> > index dceb8d1..9c9e44f 100644
> > --- a/arch/powerpc/sysdev/fsl_lbc.c
> > +++ b/arch/powerpc/sysdev/fsl_lbc.c
> > @@ -5,6 +5,10 @@
> >   *
> >   * Author: Anton Vorontsov <avorontsov@ru.mvista.com>
> >   *
> > + * Copyright (c) 2010 Freescale Semiconductor
> > + *
> > + * Authors: Jack Lan <Jack.Lan@freescale.com>
> 
> Would be prettier if you'd group copyright and authorship notices.
> I.e.
> 
> Copyright 2008 MV
> Copyright 2010 FSL
> 
> Authors: Anton
>          Jack
Then how to reflect the relationship of the Author and the company? from email address?

> 
> > + *
> >   * 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
> > @@ -23,35 +27,8 @@
> >  #include <asm/fsl_lbc.h>
> >
> >  static spinlock_t fsl_lbc_lock = __SPIN_LOCK_UNLOCKED(fsl_lbc_lock);
> > -static struct fsl_lbc_regs __iomem *fsl_lbc_regs;
> > -
> > -static char __initdata *compat_lbc[] = {
> > -	"fsl,pq2-localbus",
> > -	"fsl,pq2pro-localbus",
> > -	"fsl,pq3-localbus",
> > -	"fsl,elbc",
> > -};
> > -
> > -static int __init fsl_lbc_init(void)
> > -{
> > -	struct device_node *lbus;
> > -	int i;
> > -
> > -	for (i = 0; i < ARRAY_SIZE(compat_lbc); i++) {
> > -		lbus = of_find_compatible_node(NULL, NULL, compat_lbc[i]);
> > -		if (lbus)
> > -			goto found;
> > -	}
> > -	return -ENODEV;
> > -
> > -found:
> > -	fsl_lbc_regs = of_iomap(lbus, 0);
> > -	of_node_put(lbus);
> > -	if (!fsl_lbc_regs)
> > -		return -ENOMEM;
> > -	return 0;
> > -}
> > -arch_initcall(fsl_lbc_init);
> > +struct fsl_lbc_ctrl *fsl_lbc_ctrl_dev;
> > +EXPORT_SYMBOL(fsl_lbc_ctrl_dev);
> >
> >  /**
> >   * fsl_lbc_find - find Localbus bank
> > @@ -66,12 +43,12 @@ int fsl_lbc_find(phys_addr_t addr_base)
> >  {
> >  	int i;
> >
> > -	if (!fsl_lbc_regs)
> > +	if (!fsl_lbc_ctrl_dev || !fsl_lbc_ctrl_dev->regs)
> >  		return -ENODEV;
> >
> > -	for (i = 0; i < ARRAY_SIZE(fsl_lbc_regs->bank); i++) {
> > -		__be32 br = in_be32(&fsl_lbc_regs->bank[i].br);
> > -		__be32 or = in_be32(&fsl_lbc_regs->bank[i].or);
> > +	for (i = 0; i < ARRAY_SIZE(fsl_lbc_ctrl_dev->regs->bank); i++) {
> > +		__be32 br = in_be32(&fsl_lbc_ctrl_dev->regs->bank[i].br);
> > +		__be32 or = in_be32(&fsl_lbc_ctrl_dev->regs->bank[i].or);
> 
> A dedicated variable for regs would make this much more readable?
Yes. considering the following condition line.
> 
> >
> >  		if (br & BR_V && (br & or & BR_BA) == addr_base)
> >  			return i;
> > @@ -99,17 +76,20 @@ int fsl_upm_find(phys_addr_t addr_base, struct fsl_upm
> *upm)
> >  	if (bank < 0)
> >  		return bank;
> >
> > -	br = in_be32(&fsl_lbc_regs->bank[bank].br);
> > +	if (!fsl_lbc_ctrl_dev || !fsl_lbc_ctrl_dev->regs)
> > +		return -ENODEV;
> > +
> > +	br = in_be32(&fsl_lbc_ctrl_dev->regs->bank[bank].br);
> >
> >  	switch (br & BR_MSEL) {
> >  	case BR_MS_UPMA:
> > -		upm->mxmr = &fsl_lbc_regs->mamr;
> > +		upm->mxmr = &fsl_lbc_ctrl_dev->regs->mamr;
> 
> Ditto, a very repetitive stuff, desires a variable for regs?
But the fact is that the variable represents different reg address according to the condition. It will be ugly to use the reg address directoly.
> 
> >  		break;
> >  	case BR_MS_UPMB:
> > -		upm->mxmr = &fsl_lbc_regs->mbmr;
> > +		upm->mxmr = &fsl_lbc_ctrl_dev->regs->mbmr;
> >  		break;
> >  	case BR_MS_UPMC:
> > -		upm->mxmr = &fsl_lbc_regs->mcmr;
> > +		upm->mxmr = &fsl_lbc_ctrl_dev->regs->mcmr;
> >  		break;
> >  	default:
> >  		return -EINVAL;
> > @@ -143,14 +123,18 @@ EXPORT_SYMBOL(fsl_upm_find);
> >   * thus UPM pattern actually executed. Note that mar usage depends on the
> >   * pre-programmed AMX bits in the UPM RAM.
> >   */
> > +
> >  int fsl_upm_run_pattern(struct fsl_upm *upm, void __iomem *io_base, u32 mar)
> >  {
> >  	int ret = 0;
> >  	unsigned long flags;
> >
> > +	if (!fsl_lbc_ctrl_dev || !fsl_lbc_ctrl_dev->regs)
> > +		return -ENODEV;
> > +
> >  	spin_lock_irqsave(&fsl_lbc_lock, flags);
> >
> > -	out_be32(&fsl_lbc_regs->mar, mar);
> > +	out_be32(&fsl_lbc_ctrl_dev->regs->mar, mar);
> >
> >  	switch (upm->width) {
> >  	case 8:
> > @@ -172,3 +156,197 @@ int fsl_upm_run_pattern(struct fsl_upm *upm, void
> __iomem *io_base, u32 mar)
> >  	return ret;
> >  }
> >  EXPORT_SYMBOL(fsl_upm_run_pattern);
> > +
> > +static int __devinit fsl_lbc_ctrl_init(struct fsl_lbc_ctrl *ctrl)
> > +{
> > +	struct fsl_lbc_regs __iomem *lbc = ctrl->regs;
> 
> Yep, something like this for the functions above.
I do not see any harm to add a variable lbc in the function to increase the readable.
> 
> > +
> > +	/* clear event registers */
> > +	setbits32(&lbc->ltesr, LTESR_CLEAR);
> > +	out_be32(&lbc->lteatr, 0);
> > +	out_be32(&lbc->ltear, 0);
> > +	out_be32(&lbc->lteccr, LTECCR_CLEAR);
> > +	out_be32(&lbc->ltedr, LTEDR_ENABLE);
> > +
> > +	/* Enable interrupts for any detected events */
> > +	out_be32(&lbc->lteir, LTEIR_ENABLE);
> > +
> > +	return 0;
> > +}
> > +
> > +static int __devexit fsl_lbc_ctrl_remove(struct of_device *ofdev)
> > +{
> > +	struct fsl_lbc_ctrl *ctrl = dev_get_drvdata(&ofdev->dev);
> > +
> > +	if (ctrl->irq)
> > +		free_irq(ctrl->irq, ctrl);
> > +
> > +	if (ctrl->regs)
> > +		iounmap(ctrl->regs);
> > +
> > +	dev_set_drvdata(&ofdev->dev, NULL);
> > +	kfree(ctrl);
> > +
> > +	return 0;
> > +}
> > +
> > +/* NOTE: This interrupt is used to report localbus events of various kinds,
> > + * such as transaction errors on the chipselects.
> > + */
> 
> /*
>  * multi line comments
>  * should be like this
>  */
OK.

> 
> [...]
> > +/* fsl_lbc_ctrl_probe
> > + *
> > + * called by device layer when it finds a device matching
> > + * one our driver can handled. This code allocates all of
> > + * the resources needed for the controller only.  The
> > + * resources for the NAND banks themselves are allocated
> > + * in the chip probe function.
> > +*/
> 
> ditto.
OK.
> 
> > +static int __devinit fsl_lbc_ctrl_probe(struct of_device *ofdev,
> > +					 const struct of_device_id *match)
> > +{
> > +	int ret = 0;
> 
> no need for the initial value here.
Any harm?

> 
> > +
> > +	fsl_lbc_ctrl_dev = kzalloc(sizeof(*fsl_lbc_ctrl_dev), GFP_KERNEL);
> > +	if (!fsl_lbc_ctrl_dev)
> > +		return -ENOMEM;
> > +
> > +	dev_set_drvdata(&ofdev->dev, fsl_lbc_ctrl_dev);
> > +
> > +	spin_lock_init(&fsl_lbc_ctrl_dev->lock);
> > +	init_waitqueue_head(&fsl_lbc_ctrl_dev->irq_wait);
> > +
> > +	fsl_lbc_ctrl_dev->regs = of_iomap(ofdev->dev.of_node, 0);
> > +	if (!fsl_lbc_ctrl_dev->regs) {
> > +		dev_err(&ofdev->dev, "failed to get memory region\n");
> > +		ret = -ENODEV;
> > +		goto err;
> > +	}
> > +
> > +	fsl_lbc_ctrl_dev->irq = of_irq_to_resource(ofdev->dev.of_node, 0, NULL);
> > +	if (fsl_lbc_ctrl_dev->irq == NO_IRQ) {
> > +		dev_err(&ofdev->dev, "failed to get irq resource\n");
> > +		ret = -ENODEV;
> > +		goto err;
> > +	}
> > +
> > +	fsl_lbc_ctrl_dev->dev = &ofdev->dev;
> > +
> > +	ret = fsl_lbc_ctrl_init(fsl_lbc_ctrl_dev);
> > +	if (ret < 0)
> > +		goto err;
> > +
> > +	ret = request_irq(fsl_lbc_ctrl_dev->irq, fsl_lbc_ctrl_irq, 0,
> > +				"fsl-lbc", fsl_lbc_ctrl_dev);
> > +	if (ret != 0) {
> > +		dev_err(&ofdev->dev, "failed to install irq (%d)\n",
> > +			fsl_lbc_ctrl_dev->irq);
> > +		ret = fsl_lbc_ctrl_dev->irq;
> > +		goto err;
> > +	}
> > +
> > +	return 0;
> > +
> > +err:
> 
> fsl_lbc_ctrl_dev leaked. Plus, after freeing it, you should
> NULLify it as well, as other functions checks it for !NULL.
Agree.
> 
> fsl_lbc_ctrl_dev->regs leaks too.
Agree.
> 
> > +	return ret;
> > +}
> > +
> > +static const struct of_device_id fsl_lbc_match[] = {
> > +	{
> > +		.compatible = "fsl,elbc",
> > +	},
> > +	{
> > +		.compatible = "fsl,pq3-localbus",
> > +	},
> > +	{
> > +		.compatible = "fsl,pq2-localbus",
> > +	},
> > +	{
> > +		.compatible = "fsl,pq2pro-localbus",
> > +	},
> > +	{},
> 
> Could save 8 lines by writing "{ .compatible = "...", },".

Agree.
> 
> > +};
> > +
> > +static struct of_platform_driver fsl_lbc_ctrl_driver = {
> 
> I think you shouldn't use of_platform for new code. just platform
> will work (there's platform_driver.driver.of_match_table nowadays).
> 
> > +	.driver = {
> > +		.name = "fsl-lbc",
> > +		.of_match_table = fsl_lbc_match,
> > +	},
> > +	.probe = fsl_lbc_ctrl_probe,
> > +	.remove = __devexit_p(fsl_lbc_ctrl_remove),
> 
> The device is not removable, I think you don't need this.
Agree.

> 
> > +};
> > +
> > +static int __init fsl_lbc_init(void)
> > +{
> > +	return of_register_platform_driver(&fsl_lbc_ctrl_driver);
> > +}
> > +
> > +static void __exit fsl_lbc_exit(void)
> > +{
> > +	of_unregister_platform_driver(&fsl_lbc_ctrl_driver);
> > +}
> > +
> > +module_init(fsl_lbc_init);
> > +module_exit(fsl_lbc_exit);
> 
> fsl_lbc is not a module, so you don't need _exit().
Agree.
> 
> > +MODULE_LICENSE("GPL");
> > +MODULE_AUTHOR("Freescale Semiconductor");
> > +MODULE_DESCRIPTION("Freescale Enhanced Local Bus Controller driver");
> 
> ditto, no need for this.
Agree.
Thanks.
Roy

RE: [PATCH 2/3][MTD] P4080/nand: Only make elbc nand driver detect nand flash partitions

[Zang Roy-R61911]

> -----Original Message-----
> From: Anton Vorontsov [mailto:cbouatmailru@gmail.com]
> Sent: Friday, September 03, 2010 19:44 PM
> To: Zang Roy-R61911
> Cc: linux-mtd@lists.infradead.org; Lan Chunhe-B25806; linuxppc-dev@ozlabs.org;
> akpm@linux-foundation.org; Gala Kumar-B11780; Wood Scott-B07421
> Subject: Re: [PATCH 2/3][MTD] P4080/nand: Only make elbc nand driver detect
> nand flash partitions
> 
> On Fri, Aug 06, 2010 at 10:51:35AM +0800, Roy Zang wrote:
> [...]
> >
> > +static struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl;
> > +
> 
> Are you sure that you want it as a global var? A bit scary change.
> 
> Oh, you probably don't need it, as you can get it from
> fsl_lbc_ctrl_dev->nand?
> 
> I wonder if Scott saw these patches? Cc'ed.
Yes.
Roy

RE: [PATCH 3/3][MTD] P4080/nand: Fix the freescale lbc issue with 36bit mode

[Zang Roy-R61911]

> -----Original Message-----
> From: Anton Vorontsov [mailto:cbouatmailru@gmail.com]
> Sent: Friday, September 03, 2010 19:36 PM
> To: Zang Roy-R61911
> Cc: linux-mtd@lists.infradead.org; Lan Chunhe-B25806; linuxppc-dev@ozlabs.org;
> akpm@linux-foundation.org; Gala Kumar-B11780
> Subject: Re: [PATCH 3/3][MTD] P4080/nand: Fix the freescale lbc issue with
> 36bit mode
> 
> On Fri, Aug 06, 2010 at 10:51:36AM +0800, Roy Zang wrote:
> [...]
> >  /**
> > + * convert_lbc_address - convert the base address
> > + * @addr_base:	base address of the memory bank
> > + *
> > + * This function converts a base address of lbc into the right format for
> the BR
> > + * registers. If the SOC has eLBC then it returns 32bit physical address
> else
> > + * it returns 34bit physical address for local bus(Example: MPC8641).
> > + */
> > +unsigned int convert_lbc_address(phys_addr_t addr_base)
> > +{
> > +	void *dev;
> > +	int compatible;
> > +
> > +	dev = of_find_node_by_name(NULL, "localbus");
> 
> Nope, you shouldn't do this. Never search by name.
OK. I will search by compatible.
> 
> Also, aren't there already a global dev, which was found by the
> _probe() stuff?
I do not see the global dev can be used in probe.

> 
> > +	if (!dev) {
> > +		printk(KERN_INFO "fsl-lbc: can't find localbus node\n");
> > +		of_node_put(dev);
> > +		return 0;
> > +	}
> > +
> > +	compatible = of_device_is_compatible(dev, "fsl,elbc");
> > +	of_node_put(dev);
> > +	if (compatible)
> > +		return addr_base & 0xffff8000;
> > +	else
> > +		return (addr_base & 0x0ffff8000ull) \
> > +			| ((addr_base & 0x300000000ull) >> 19);
> > +}
> > +EXPORT_SYMBOL(convert_lbc_address);
> > +
> > +/**
> >   * fsl_lbc_find - find Localbus bank
> >   * @addr_base:	base address of the memory bank
> >   *
> > @@ -50,7 +80,8 @@ int fsl_lbc_find(phys_addr_t addr_base)
> >  		__be32 br = in_be32(&fsl_lbc_ctrl_dev->regs->bank[i].br);
> >  		__be32 or = in_be32(&fsl_lbc_ctrl_dev->regs->bank[i].or);
> >
> > -		if (br & BR_V && (br & or & BR_BA) == addr_base)
> > +		if (br & BR_V && (br & or & BR_BA) \
> 
> No need for "\" at the end of the line, keep == on the same line.
> 
> > +				== convert_lbc_address(addr_base))
> 
> Would be prettier if you name it fsl_lbc_addr(). Keeps prefix
> the same for the rest of the file, plus makes it shorter (so
> there probably won't be any need for breaking the line).
Agree the new name.
Thanks.
Roy

RE: [PATCH 2/3][MTD] P4080/nand: Only make elbc nand driver detect nand flash partitions

[Zang Roy-R61911]

> -----Original Message-----
> From: Anton Vorontsov [mailto:cbouatmailru@gmail.com]
> Sent: Friday, September 03, 2010 19:44 PM
> To: Zang Roy-R61911
> Cc: linux-mtd@lists.infradead.org; Lan Chunhe-B25806; linuxppc-dev@ozlabs.org;
> akpm@linux-foundation.org; Gala Kumar-B11780; Wood Scott-B07421
> Subject: Re: [PATCH 2/3][MTD] P4080/nand: Only make elbc nand driver detect
> nand flash partitions
> 
> On Fri, Aug 06, 2010 at 10:51:35AM +0800, Roy Zang wrote:
> [...]
> >
> > +static struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl;
> > +
> 
> Are you sure that you want it as a global var? A bit scary change.
> 
> Oh, you probably don't need it, as you can get it from
> fsl_lbc_ctrl_dev->nand?
Get it form fsl_lbc_ctrl_dev->nand or assign it to fsl_lbc_ctrl_dev->nand in probe?
Thanks.
Roy

Re: [PATCH 2/3][MTD] P4080/nand: Only make elbc nand driver detect nand flash partitions

[Anton Vorontsov]

On Mon, Sep 06, 2010 at 12:49:17PM +0800, Zang Roy-R61911 wrote:
> > On Fri, Aug 06, 2010 at 10:51:35AM +0800, Roy Zang wrote:
> > [...]
> > >
> > > +static struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl;
> > > +
> > 
> > Are you sure that you want it as a global var? A bit scary change.
> > 
> > Oh, you probably don't need it, as you can get it from
> > fsl_lbc_ctrl_dev->nand?
> Get it form fsl_lbc_ctrl_dev->nand or assign it to
> fsl_lbc_ctrl_dev->nand in probe?

I meant to get it from fsl_lbc_ctrl_dev->nand. I.e. in
probe() you do: fsl_lbc_ctrl_dev->nand = elbc_fcm_ctrl, so
you probably don't need the global var.

(fsl_lbc_ctrl_dev seems to be global as well, duh. Well,
one variable less in the global name space. But I'd
probably use lbc_np->data to store the LBC private
struct).

Scott seem to be fine with it as there are probably no
plans to to add several localbus controllers into the SoCs.

But, I saw a custom board with two MPC82xx SoCs connected
together, one as a master (core + peripheral devs), and
other as a slave (its core was halted, and only slave's
CPM peripheral devices were used by the master CPU).

I think it is possible to connect two (or more) SoCs in
a such way so that two or more LBC controllers would
be visible for the Linux.

Thanks,

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Re: [PATCH 1/3][MTD] P4080/eLBC: Make Freescale elbc interrupt common to elbc devices

[Anton Vorontsov]

On Mon, Sep 06, 2010 at 11:38:09AM +0800, Zang Roy-R61911 wrote:
[...]
> > >  	switch (br & BR_MSEL) {
> > >  	case BR_MS_UPMA:
> > > -		upm->mxmr = &fsl_lbc_regs->mamr;
> > > +		upm->mxmr = &fsl_lbc_ctrl_dev->regs->mamr;
> > 
> > Ditto, a very repetitive stuff, desires a variable for regs?
> But the fact is that the variable represents different reg
> address according to the condition. It will be ugly to use
> the reg address directoly.

I meant a dedicated var for 'fsl_lbc_ctrl_dev->regs'.
I.e.

regs = fsl_lbc_ctrl_dev->regs;
...
mxmr = &regs->mamr;
...
mxmr = &regs->mbmr;
..
mxmr = &regs->mcmr;

Instead of

mxmr = &fsl_lbc_ctrl_dev->regs->mamr;
...
mxmr = &fsl_lbc_ctrl_dev->regs->mbmr;
..
mxmr = &fsl_lbc_ctrl_dev->regs->mcmr;

[...]
> > > +static int __devinit fsl_lbc_ctrl_probe(struct of_device *ofdev,
> > > +					 const struct of_device_id *match)
> > > +{
> > > +	int ret = 0;
> > 
> > no need for the initial value here.
> Any harm?

Probably not as gcc will likely optimize it away,
but it's not needed, so why keep it there?

Thanks,

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

RE: [PATCH 2/3][MTD] P4080/nand: Only make elbc nand driver detect nand flash partitions

[Zang Roy-R61911]

> -----Original Message-----
> From: Anton Vorontsov [mailto:cbouatmailru@gmail.com]
> Sent: Monday, September 06, 2010 16:10 PM
> To: Zang Roy-R61911
> Cc: linux-mtd@lists.infradead.org; Lan Chunhe-B25806; linuxppc-dev@ozlabs.org;
> akpm@linux-foundation.org; Gala Kumar-B11780; Wood Scott-B07421
> Subject: Re: [PATCH 2/3][MTD] P4080/nand: Only make elbc nand driver detect
> nand flash partitions
> 
> On Mon, Sep 06, 2010 at 12:49:17PM +0800, Zang Roy-R61911 wrote:
> > > On Fri, Aug 06, 2010 at 10:51:35AM +0800, Roy Zang wrote:
> > > [...]
> > > >
> > > > +static struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl;
> > > > +
> > >
> > > Are you sure that you want it as a global var? A bit scary change.
> > >
> > > Oh, you probably don't need it, as you can get it from
> > > fsl_lbc_ctrl_dev->nand?
> > Get it form fsl_lbc_ctrl_dev->nand or assign it to
> > fsl_lbc_ctrl_dev->nand in probe?
> 
> I meant to get it from fsl_lbc_ctrl_dev->nand. I.e. in
> probe() you do: fsl_lbc_ctrl_dev->nand = elbc_fcm_ctrl, so
> you probably don't need the global var.
> 
> (fsl_lbc_ctrl_dev seems to be global as well, duh. Well,
> one variable less in the global name space. But I'd
> probably use lbc_np->data to store the LBC private
> struct).
That makes sense.
> 
> Scott seem to be fine with it as there are probably no
> plans to to add several localbus controllers into the SoCs.
> 
> But, I saw a custom board with two MPC82xx SoCs connected
> together, one as a master (core + peripheral devs), and
> other as a slave (its core was halted, and only slave's
> CPM peripheral devices were used by the master CPU).
> 
> I think it is possible to connect two (or more) SoCs in
> a such way so that two or more LBC controllers would
> be visible for the Linux.
That is an interesting case. Do you have any thought here?
Thanks.
Roy

RE: [PATCH 1/3][MTD] P4080/eLBC: Make Freescale elbc interrupt common to elbc devices

[Zang Roy-R61911]

> -----Original Message-----
> From: Anton Vorontsov [mailto:cbouatmailru@gmail.com]
> Sent: Monday, September 06, 2010 16:22 PM
> To: Zang Roy-R61911
> Cc: linux-mtd@lists.infradead.org; Lan Chunhe-B25806; linuxppc-dev@ozlabs.org;
> akpm@linux-foundation.org; Gala Kumar-B11780; Wood Scott-B07421
> Subject: Re: [PATCH 1/3][MTD] P4080/eLBC: Make Freescale elbc interrupt common
> to elbc devices
> 
> On Mon, Sep 06, 2010 at 11:38:09AM +0800, Zang Roy-R61911 wrote:
> [...]
> > > >  	switch (br & BR_MSEL) {
> > > >  	case BR_MS_UPMA:
> > > > -		upm->mxmr = &fsl_lbc_regs->mamr;
> > > > +		upm->mxmr = &fsl_lbc_ctrl_dev->regs->mamr;
> > >
> > > Ditto, a very repetitive stuff, desires a variable for regs?
> > But the fact is that the variable represents different reg
> > address according to the condition. It will be ugly to use
> > the reg address directoly.
> 
> I meant a dedicated var for 'fsl_lbc_ctrl_dev->regs'.
> I.e.
> 
> regs = fsl_lbc_ctrl_dev->regs;
> ...
> mxmr = &regs->mamr;
> ...
> mxmr = &regs->mbmr;
> ..
> mxmr = &regs->mcmr;
> 
> Instead of
> 
> mxmr = &fsl_lbc_ctrl_dev->regs->mamr;
> ...
> mxmr = &fsl_lbc_ctrl_dev->regs->mbmr;
> ..
> mxmr = &fsl_lbc_ctrl_dev->regs->mcmr;
That makes sense.  A global or local variable for fsl_lbc_ctrl_dev->regs? Which one is better?

> 
> [...]
> > > > +static int __devinit fsl_lbc_ctrl_probe(struct of_device *ofdev,
> > > > +					 const struct of_device_id *match)
> > > > +{
> > > > +	int ret = 0;
> > >
> > > no need for the initial value here.
> > Any harm?
> 
> Probably not as gcc will likely optimize it away,
> but it's not needed, so why keep it there?
habit.
Thanks.
Roy

Re: [PATCH 1/3][MTD] P4080/eLBC: Make Freescale elbc interrupt common to elbc devices

[Anton Vorontsov]

On Mon, Sep 06, 2010 at 05:24:35PM +0800, Zang Roy-R61911 wrote:
[..]
> > mxmr = &fsl_lbc_ctrl_dev->regs->mcmr;
> That makes sense.  A global or local variable for fsl_lbc_ctrl_dev->regs? Which one is better?

The less global variables, the better. So, I'd vote for
a local one.

> > [...]
> > > > > +static int __devinit fsl_lbc_ctrl_probe(struct of_device *ofdev,
> > > > > +					 const struct of_device_id *match)
> > > > > +{
> > > > > +	int ret = 0;
> > > >
> > > > no need for the initial value here.
> > > Any harm?
> > 
> > Probably not as gcc will likely optimize it away,
> > but it's not needed, so why keep it there?
> habit.

;-)

Thanks,

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

RE: [PATCH 1/3][MTD] P4080/eLBC: Make Freescale elbc interrupt common to elbc devices

[Zang Roy-R61911]

> -----Original Message-----
> From: Anton Vorontsov [mailto:cbouatmailru@gmail.com]
> Sent: Monday, September 06, 2010 17:44 PM
> To: Zang Roy-R61911
> Cc: linux-mtd@lists.infradead.org; Lan Chunhe-B25806; linuxppc-dev@ozlabs.org;
> akpm@linux-foundation.org; Gala Kumar-B11780; Wood Scott-B07421
> Subject: Re: [PATCH 1/3][MTD] P4080/eLBC: Make Freescale elbc interrupt common
> to elbc devices
> 
> On Mon, Sep 06, 2010 at 05:24:35PM +0800, Zang Roy-R61911 wrote:
> [..]
> > > mxmr = &fsl_lbc_ctrl_dev->regs->mcmr;
> > That makes sense.  A global or local variable for fsl_lbc_ctrl_dev->regs?
> Which one is better?
> 
> The less global variables, the better. So, I'd vote for
> a local one.
I also prefer local one.
> 
> > > [...]
> > > > > > +static int __devinit fsl_lbc_ctrl_probe(struct of_device *ofdev,
> > > > > > +					 const struct of_device_id *match)
> > > > > > +{
> > > > > > +	int ret = 0;
> > > > >
> > > > > no need for the initial value here.
> > > > Any harm?
> > >
> > > Probably not as gcc will likely optimize it away,
> > > but it's not needed, so why keep it there?
> > habit.
> 
> ;-)
:-(.
Thanks for all your comments. That is valuable.
I will update the patch according to your comment and the new platform device arch.
Roy