Messages in this thread |  | | | Date | Mon, 19 Jun 2006 22:22:01 -0700 | | From | Andrew Morton <> | | Subject | Re: [patch -mm 03/20] chardev: GPIO for SCx200 & PC-8736x: add platforn_device for use w dev_dbg |
| |
On Sat, 17 Jun 2006 12:25:08 -0600 Jim Cromie <jim.cromie@gmail.com> wrote:
> 3/20. patch.platform-dev-2 > > Add a platform-device to scx200_gpio, and use its struct device dev > member (ie: devp) in dev_dbg() once. > > There are 2 alternatives here (Im soliciting guidance/commentary): > > - use isa_device, if/when its added to the kernel. > > - alter scx200.c to EXPORT_GPL its private devp so that both > scx200_gpio, and the (to be added) nsc_gpio module can use it. > Since the available devp is in 'grandparent', this seems like > too much 'action at a distance'. > > > @@ -121,12 +126,20 @@ static int __init scx200_gpio_init(void) > int rc, i; > dev_t dev = MKDEV(major, 0); > > - printk(KERN_DEBUG NAME ": NatSemi SCx200 GPIO Driver\n"); > - > if (!scx200_gpio_present()) { > printk(KERN_ERR NAME ": no SCx200 gpio present\n"); > return -ENODEV; > } > + > + /* support dev_dbg() with pdev->dev */ > + pdev = platform_device_alloc(DEVNAME, 0); > + if (!pdev) > + return -ENODEV;
-ENOMEM would be more accurate.
> + rc = platform_device_add(pdev); > + if (rc) > + goto undo_platform_device_add;
If the platform_device_add() didn't work, I don't think we need to undo it?
> if (major) > rc = register_chrdev_region(dev, num_devs, "scx200_gpio"); > else { > @@ -134,29 +147,31 @@ static int __init scx200_gpio_init(void) > major = MAJOR(dev); > } > if (rc < 0) { > - printk(KERN_ERR NAME ": SCx200 chrdev_region: %d\n", rc); > - return rc; > + dev_err(&pdev->dev, "SCx200 chrdev_region err: %d\n", rc); > + goto undo_platform_device_add; > } > scx200_devices = kzalloc(num_devs * sizeof(struct cdev), GFP_KERNEL); > if (!scx200_devices) { > rc = -ENOMEM; > - goto fail_malloc; > + goto undo_chrdev_region; > } > for (i = 0; i < num_devs; i++) { > struct cdev *cdev = &scx200_devices[i]; > cdev_init(cdev, &scx200_gpio_fops); > cdev->owner = THIS_MODULE; > - cdev->ops = &scx200_gpio_fops; > rc = cdev_add(cdev, MKDEV(major, i), 1); > - /* Fail gracefully if need be */ > + /* tolerate 'minor' errors */ > if (rc) > - printk(KERN_ERR NAME "Error %d on minor %d", rc, i); > + dev_err(&pdev->dev, "Error %d on minor %d", rc, i); > } > > - return 0; /* succeed */ > + return 0; /* succeed */ > > -fail_malloc: > - unregister_chrdev_region(dev, num_devs); > +undo_chrdev_region: > + unregister_chrdev_region(dev, num_devs);
needs a tab.
> +undo_platform_device_add: > + platform_device_put(pdev); > + kfree(pdev); /* undo platform_device_alloc */ > return rc; > } > > @@ -164,6 +179,9 @@ static void __exit scx200_gpio_cleanup(v > { > kfree(scx200_devices); > unregister_chrdev_region(MKDEV(major, 0), num_devs); > + platform_device_put(pdev); > + platform_device_unregister(pdev); > + /* kfree(pdev); */ > }
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|  |