> -----Original Message-----
> From: Anton Vorontsov [mailto:cbouatmai...@gmail.com]
> Sent: Monday, September 20, 2010 23:37 PM
> To: Zang Roy-R61911
> Cc: linux-...@lists.infradead.org; dw...@infradead.org; dedeki...@gmail.com;
> a...@linux-foundation.org; Lan Chunhe-B25806; Wood Scott-B07421; Gala Kumar-
> B11780; linuxppc-...@ozlabs.org
> Subject: Re: [PATCH 1/3 v4] P4080/eLBC: Make Freescale elbc interrupt common
> to elbc devices
>
> On Fri, Sep 17, 2010 at 03:01:07PM +0800, Roy Zang wrote:
[snip]
> > 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.
What struct?
assign the global variable in nand driver?
I'd prefer init it in the lbc code.
There may be other lbc device, who will use this global variable.
>
> > + 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).
Nand driver does not need to request irq. It will use the irq requested by lbc.
remember, other lbc device may also need to use this registered irq.
It should not be removed in nand driver.
Thanks.
Roy
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
