Thread (22 messages) 22 messages, 7 authors, 2010-09-14

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

From: Anton Vorontsov <hidden>
Date: 2010-09-09 11:23:22

On Thu, Sep 09, 2010 at 06:20:31PM +0800, Roy Zang wrote:
[...]
 #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>
+#include <linux/slab.h>
Why move slab.h?

[...]
quoted hunk ↗ jump to hunk
 	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
@@ -635,22 +625,21 @@ 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;
-
Please keep the empty line between variables declaration and the code.
-	if (ctrl->status != LTESR_CC)
+	struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl = priv->ctrl->nand;
+	if (elbc_fcm_ctrl->status != LTESR_CC)
 		return NAND_STATUS_FAIL;
[...]
quoted hunk ↗ jump to hunk
@@ -750,18 +739,18 @@ static void fsl_elbc_write_page(struct mtd_info *mtd,
                                 const uint8_t *buf)
 {
 	struct fsl_elbc_mtd *priv = chip->priv;
-	struct fsl_elbc_ctrl *ctrl = priv->ctrl;
-
+	struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl = priv->ctrl->nand;
Ditto.
 	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 __devinit fsl_elbc_chip_probe(struct fsl_elbc_ctrl *ctrl,
-					 struct device_node *node)
+static int __devinit fsl_elbc_nand_probe(struct platform_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;
+	struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl = NULL;
(***1)
quoted hunk ↗ jump to hunk
 #ifdef CONFIG_MTD_PARTITIONS
 	static const char *part_probe_types[]
 		= { "cmdlinepart", "RedBoot", NULL };
@@ -843,11 +832,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 +855,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 +864,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;
'priv' leaked.
+
+		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;
The driver will oops on the second probe.

You probably meant

  struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl = fsl_lbc_ctrl_dev->nand;

at (***1).

Also, nowadays the kernel may probe devices in parallel, which
means that you probably need a mutex for fsl_lbc_ctrl_dev->nand.

[...]
-static const struct of_device_id fsl_elbc_match[] = {
+static const struct of_device_id fsl_elbc_nand_match[] = {
linux/mod_devicetable.h is needed for this.
 	{
-		.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 = {
If you write of_platform_driver, you need linux/of_platform.h (which
you removed in this patch).

But I think that you need just 'struct platform_driver' here, and
include linux/platform_device.h.
 	.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,
 };
Thanks,

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help