On Mon, Dec 05, 2005 at 01:09:26PM +0000, Richard Purdie wrote:
> +static void leds_class_release(struct class_device *dev)
> +{
> + kfree(dev);
> +}
> +
> +static struct class leds_class = {
> + .name = "leds",
> + .release = leds_class_release,
> +};
Instead of this, just use class_create(). Then you don't need to
specify a release function at all.
> +/**
> + * leds_device_register - register a new object of led_device class.
> + * @dev: The device to register.
> + * @led_dev: the led_device structure for this device.
> + */
> +int leds_device_register(struct device *dev, struct led_device *led_dev)
> +{
> + int rc;
> +
> + led_dev->class_dev = kzalloc (sizeof (struct class_device), GFP_KERNEL);
> + if (unlikely (!led_dev->class_dev))
> + return -ENOMEM;
> +
> + rwlock_init(&led_dev->lock);
> +
> + led_dev->class_dev->class = &leds_class;
> + led_dev->class_dev->dev = dev;
> + led_dev->class_dev->class_data = led_dev;
> +
> + /* assign this led its name */
> + strncpy(led_dev->class_dev->class_id, led_dev->name, sizeof(led_dev->class_dev->class_id));
> +
> + rc = class_device_register (led_dev->class_dev);
Use class_device_create() instead, it's simpler, and will work better
than creating your own class device in the future.
> + if (unlikely (rc)) {
> + kfree (led_dev->class_dev);
> + return rc;
> + }
> +
> + /* register the attributes */
> + class_device_create_file(led_dev->class_dev, &class_device_attr_brightness);
> +
> + /* add to the list of leds */
> + write_lock(&leds_list_lock);
> + list_add_tail(&led_dev->node, &leds_list);
> + write_unlock(&leds_list_lock);
> +
> + printk(KERN_INFO "Registered led device: %s\n", led_dev->class_dev->class_id);
> +
> + return 0;
> +}
> +
> +/**
> + * leds_device_unregister - unregisters a object of led_properties class.
> + * @led_dev: the led device to unreigister
> + *
> + * Unregisters a previously registered via leds_device_register object.
> + */
> +void leds_device_unregister(struct led_device *led_dev)
> +{
> + class_device_remove_file (led_dev->class_dev, &class_device_attr_brightness);
> +
> + class_device_unregister(led_dev->class_dev);
> +
> + write_lock(&leds_list_lock);
> + list_del(&led_dev->node);
> + write_unlock(&leds_list_lock);
> +}
> +
> +EXPORT_SYMBOL(leds_device_suspend);
> +EXPORT_SYMBOL(leds_device_resume);
> +EXPORT_SYMBOL(leds_device_register);
> +EXPORT_SYMBOL(leds_device_unregister);
EXPORT_SYMBOL_GPL() perhaps?
> +
> +static int __init leds_init(void)
> +{
> + return class_register(&leds_class);
> +}
> +
> +static void __exit leds_exit(void)
> +{
> + class_unregister(&leds_class);
class_destroy() instead, if you use class_create().
Other than these minor things, looks good to me.
thanks,
greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
