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?
[...]
> 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;
[...]
> @@ -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)
> #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: cbouatmai...@gmail.com
irc://irc.freenode.net/bd2
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
