Thread (14 messages) flat view 14 messages, 4 authors, 2010-10-15

Re: [PATCH 1/3 v4] P4080/eLBC: Make Freescale elbc interrupt common to elbc devices

From: Anton Vorontsov <hidden>
Date: 2010-09-20 15:37:26

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>

[...]
quoted hunk ↗ jump to hunk
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>
 
[...]
quoted hunk ↗ jump to hunk
 	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: 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