On Fri, Sep 17, 2010 at 03:01:07PM +0800, Roy Zang wrote:
[...]
> +struct fsl_lbc_ctrl {
> + /* device info */
> + struct device *dev;
Forward declaration for struct device is needed (and enough).
> + struct fsl_lbc_regs __iomem *regs;
> + int irq;
> + wait_queue_head_t irq_wait;
#include <linux/wait.h> is needed for this.
> + spinlock_t lock;
#include <linux/spinlock.h>
[...]
> diff --git a/arch/powerpc/sysdev/fsl_lbc.c b/arch/powerpc/sysdev/fsl_lbc.c
> index dceb8d1..4920cd3 100644
> --- a/arch/powerpc/sysdev/fsl_lbc.c
> +++ b/arch/powerpc/sysdev/fsl_lbc.c
[...]
> +#include <linux/slab.h>
> +#include <linux/of_platform.h>
I guess of_platform.h is not needed any longer.
> +#include <linux/platform_device.h>
> +#include <linux/interrupt.h>
>
[...]
> default:
> return -EINVAL;
> @@ -143,14 +130,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.
> */
> +
No need for this empty line.
> int fsl_upm_run_pattern(struct fsl_upm *upm, void __iomem *io_base, u32 mar)
> {
> int ret = 0;
> unsigned long flags;
[...]
> +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);
Btw, the code in the NAND driver checks for !fsl_lbc_ctrl_dev.
So it might make sense to assign the global variable after the
struct is fully initialized.
> + 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;
> + }
> +
> + fsl_lbc_ctrl_dev->irq = irq_of_parse_and_map(dev->dev.of_node, 0);
> + if (fsl_lbc_ctrl_dev->irq == NO_IRQ) {
> + dev_err(&dev->dev, "failed to get irq resource\n");
> + ret = -ENODEV;
> + goto err;
> + }
> +
> + fsl_lbc_ctrl_dev->dev = &dev->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(&dev->dev, "failed to install irq (%d)\n",
> + fsl_lbc_ctrl_dev->irq);
> + ret = fsl_lbc_ctrl_dev->irq;
> + goto err;
> + }
> +
> + return 0;
> +
> +err:
> + iounmap(fsl_lbc_ctrl_dev->regs);
> + kfree(fsl_lbc_ctrl_dev);
> + return ret;
> +}
> +
> +static const struct of_device_id fsl_lbc_match[] = {
#include <linux/mod_devicetable.h> is needed for this.
Plus, I think the patch is not runtime bisectable (i.e. you
now do request_irq() here, but not removing it from the nand
driver, so nand will fail to probe).
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
