> -----Original Message-----
> From: tiejun.chen [mailto:tiejun.c...@windriver.com]
> Sent: Monday, October 18, 2010 16:56 PM
> To: Zang Roy-R61911
> Cc: linux-...@lists.infradead.org; Wood Scott-B07421; dedeki...@gmail.com; Lan
> Chunhe-B25806; linuxppc-...@ozlabs.org; a...@linux-foundation.org;
> dw...@infradead.org; Gala Kumar-B11780
> Subject: Re: [PATCH 1/2] P4080/eLBC: Make Freescale elbc interrupt common to
> elbc devices
>
> Roy Zang wrote:
> > Move Freescale elbc interrupt from nand dirver to elbc driver.
> > Then all elbc devices can use the interrupt instead of ONLY nand.
> >
> > For former nand driver, it 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 <b25...@freescale.com>
> > Signed-off-by: Roy Zang <tie-fei.z...@freescale.com>
> > Reviewed-by: Anton Vorontsov <cbouatmai...@gmail.com>
> > Cc: Wood Scott-B07421 <b07...@freescale.com>
> > ---
> >
> > These two patches are based on the following commits:
> > 1. http://lists.infradead.org/pipermail/linux-mtd/2010-
> September/032112.html
> > 2. http://lists.infradead.org/pipermail/linux-mtd/2010-
> September/032110.html
> > 3. http://lists.infradead.org/pipermail/linux-mtd/2010-
> September/032111.html
> > According to Anton's comment, I merge 1 & 2 together and start a new thread.
> > Comparing the provided link:
> > 1. Merge 1 & 2 together.
> > 2. Some code style updates
> > 3. Add counter protect for elbc driver remove
> > 4. Rebase to 2.6.36-rc7
> >
> > Other histories from the links:
> > V2: Comparing with v1, according to the feedback, add some decorations.
> >
> > V3: Comparing with v2:
> > 1. according to the feedback, add some decorations.
> > 2. change of_platform_driver to platform_driver
> > 3. rebase to 2.6.36-rc4
> >
> > V4: Comparing with v3
> > 1. minor fix from type unsigned int to u32
> > 2. fix platform_driver issue.
> > 3. add mutex for nand probe
> >
> > arch/powerpc/Kconfig | 7 +-
> > arch/powerpc/include/asm/fsl_lbc.h | 33 +++-
> > arch/powerpc/sysdev/fsl_lbc.c | 229 ++++++++++++++---
> > drivers/mtd/nand/Kconfig | 1 +
> > drivers/mtd/nand/fsl_elbc_nand.c | 482 +++++++++++++++------------------
> ---
> > 5 files changed, 425 insertions(+), 327 deletions(-)
> >
> > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> > index 631e5a0..44df1ba 100644
> > --- a/arch/powerpc/Kconfig
> > +++ b/arch/powerpc/Kconfig
> > @@ -687,9 +687,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..0c40c05 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.spe...@freescale.com>,
> > * Scott Wood <scottw...@freescale.com>
> > + * Jack Lan <jack....@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
> > @@ -26,6 +27,8 @@
> > #include <linux/compiler.h>
> > #include <linux/types.h>
> > #include <linux/io.h>
> > +#include <linux/device.h>
> > +#include <linux/spinlock.h>
> >
> > struct fsl_lbc_bank {
> > __be32 br; /**< Base Register */
> > @@ -125,13 +128,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 +278,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..4bb0336 100644
> > --- a/arch/powerpc/sysdev/fsl_lbc.c
> > +++ b/arch/powerpc/sysdev/fsl_lbc.c
> > @@ -2,8 +2,11 @@
> > * Freescale LBC and UPM routines.
> > *
> > * Copyright (c) 2007-2008 MontaVista Software, Inc.
> > + * Copyright (c) 2010 Freescale Semiconductor
> > *
> > * Author: Anton Vorontsov <avoront...@ru.mvista.com>
> > + * Author: Jack Lan <jack....@freescale.com>
> > + * Author: Roy Zang <tie-fei.z...@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
> > @@ -19,39 +22,16 @@
> > #include <linux/types.h>
> > #include <linux/io.h>
> > #include <linux/of.h>
> > +#include <linux/slab.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/mod_devicetable.h>
> > #include <asm/prom.h>
> > #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
> > @@ -65,13 +45,15 @@ arch_initcall(fsl_lbc_init);
> > int fsl_lbc_find(phys_addr_t addr_base)
> > {
> > int i;
> > + struct fsl_lbc_regs __iomem *lbc;
> >
> > - 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);
> > + lbc = fsl_lbc_ctrl_dev->regs;
> > + for (i = 0; i < ARRAY_SIZE(lbc->bank); i++) {
> > + __be32 br = in_be32(&lbc->bank[i].br);
> > + __be32 or = in_be32(&lbc->bank[i].or);
> >
> > if (br & BR_V && (br & or & BR_BA) == addr_base)
> > return i;
> > @@ -94,22 +76,27 @@ int fsl_upm_find(phys_addr_t addr_base, struct fsl_upm
> *upm)
> > {
> > int bank;
> > __be32 br;
> > + struct fsl_lbc_regs __iomem *lbc;
> >
> > bank = fsl_lbc_find(addr_base);
> > 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;
> > +
> > + lbc = fsl_lbc_ctrl_dev->regs;
> > + br = in_be32(&lbc->bank[bank].br);
> >
> > switch (br & BR_MSEL) {
> > case BR_MS_UPMA:
> > - upm->mxmr = &fsl_lbc_regs->mamr;
> > + upm->mxmr = &lbc->mamr;
> > break;
> > case BR_MS_UPMB:
> > - upm->mxmr = &fsl_lbc_regs->mbmr;
> > + upm->mxmr = &lbc->mbmr;
> > break;
> > case BR_MS_UPMC:
> > - upm->mxmr = &fsl_lbc_regs->mcmr;
> > + upm->mxmr = &lbc->mcmr;
> > break;
> > default:
> > return -EINVAL;
> > @@ -148,9 +135,12 @@ 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 +162,166 @@ 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;
> > +}
> > +
> > +/*
> > + * 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)
> > + return IRQ_NONE;
> > +
> > + 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;
> > +}
> > +
> > +/*
> > + * 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 platform_device *dev)
> > +{
> > + int ret;
> > +
> > + if (!dev->dev.of_node) {
> > + dev_err(&dev->dev, "Device OF-Node is NULL");
> > + return -EFAULT;
> > + }
> > +
> > + fsl_lbc_ctrl_dev = kzalloc(sizeof(*fsl_lbc_ctrl_dev), GFP_KERNEL);
> > + if (!fsl_lbc_ctrl_dev)
> > + return -ENOMEM;
> > +
> > + dev_set_drvdata(&dev->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(dev->dev.of_node, 0);
> > + if (!fsl_lbc_ctrl_dev->regs) {
> > + dev_err(&dev->dev, "failed to get memory region\n");
> > + ret = -ENODEV;
> > + goto err;
>
> Looks you always iounmap(fsl_lbc_ctrl_dev->regs) on position 'err' but here
> of_iomap() is already failed you should skip iounmap() fsl_lbc_ctrl_dev->regs
> again. So you should improve that as the following on 'err', or layout 'err'
> in
> gain.
> ------
> if(fsl_lbc_ctrl_dev->regs)
> iounmap(fsl_lbc_ctrl_dev->regs);
>
> Tiejun
You are right!
How about
if (!fsl_lbc_ctrl_dev->regs) {
dev_err(&dev->dev, "failed to get memory region\n");
kfree(fsl_lbc_ctrl_dev);
return -ENOMEM;
}
...
Thanks.
Roy
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
