lkml.org 
[lkml]   [2010]   [Feb]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] Add Dell Business Class Netbook LED driver
On Thu, Feb 11, 2010 at 05:59:14PM +0000, Richard Purdie wrote:
> On Mon, 2010-02-08 at 14:49 -0600, Bob Rodgers wrote:
> > This patch adds an LED driver to support the Dell Activity LED on the
> > Dell Latitude 2100 netbook and future products to come. The Activity LED
> > is visible externally in the lid so classroom instructors can observe it
> > from a distance. The driver uses the sysfs led_class and provides a
> > standard LED interface. This driver is ready for submission upstream.
>
> A couple of comments:
>
> > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> > index 8a0e1ec..40dd693 100644
> > --- a/drivers/leds/Kconfig
> > +++ b/drivers/leds/Kconfig
> > @@ -269,6 +269,13 @@ config LEDS_ADP5520
> > To compile this driver as a module, choose M here: the module will
> > be called leds-adp5520.
> >
> > +config LEDS_DELL_NETBOOKS
> > + tristate "External LED on Dell Business Netbooks"
> > + depends on LEDS_CLASS
> > + help
> > + This adds support for the Latitude 2100 and similar
> > + notebooks that have an external LED.
> > +
> > comment "LED Triggers"
>
> I assume this driver applies to X86 only? Is there anything else this
> config option should be depending on?
>
> > +static int __devinit dell_led_probe(struct platform_device *pdev)
> > +{
> > + return led_classdev_register(&pdev->dev, &dell_led);
> > +}
> > +
> > +static int dell_led_remove(struct platform_device *pdev)
> > +{
> > + led_classdev_unregister(&dell_led);
> > + return 0;
> > +}
> > +
> > +static struct platform_driver dell_led_driver = {
> > + .probe = dell_led_probe,
> > + .remove = dell_led_remove,
> > + .driver = {
> > + .name = KBUILD_MODNAME,
> > + .owner = THIS_MODULE,
> > + },
> > +};
> > +
> > +static struct platform_device *pdev;
> > +
> > +static int __init dell_led_init(void)
> > +{
> > + int error = 0;
> > +
> > + if (!wmi_has_guid(DELL_LED_BIOS_GUID)) {
> > + printk(KERN_DEBUG KBUILD_MODNAME
> > + ": could not find: DELL_LED_BIOS_GUID\n");
> > + return -ENODEV;
> > + }
> > +
> > + error = led_off();
> > + if (error != 0) {
> > + printk(KERN_DEBUG KBUILD_MODNAME
> > + ": could not communicate with LED"
> > + ": error %d\n", error);
> > + return -ENODEV;
> > + }
> > +
> > + error = platform_driver_register(&dell_led_driver);
> > + if (error < 0)
> > + return error;
> > +
> > + pdev = platform_device_register_simple(KBUILD_MODNAME, -1, NULL, 0);
> > + if (IS_ERR(pdev)) {
> > + error = PTR_ERR(pdev);
> > + platform_driver_unregister(&dell_led_driver);
> > + }
> > +
> > + return error;
> > +}
>
> Rather than add all this overhead of a platform device, why not just
> pass NULL as the parent to led_classdev_register()?
>

Or stuck it in dell-wmi.c...

--
Dmitry


\
 
 \ /
  Last update: 2010-02-11 19:07    [W:1.158 / U:0.016 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site