Re: [patch -mm 03/20] chardev: GPIO for SCx200 & PC-8736x: addplatforn_device for use w dev_dbg

[Andrew Morton]

On Sat, 17 Jun 2006 12:25:08 -0600
Jim Cromie <jim.cromie@xxxxxxxxx> 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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/