leds-clevo-mail: driver for Clevo mail LED
Subsystems (2)
PATCH 3/3 leds-clevo-mail: driver for Clevo mail LED
From: Márton Németh <[email protected]>
The driver supports the mail LED commonly found on different Clevo notebooks. The driver access the LED through the i8042 hardware and implements the support for hardware accelerated blink function.
---
drivers/leds/Kconfig +20 -0
@@ -101,6 +101,26 @@ config LEDS_GPIO outputs. To be useful the particular board must have LEDs and they must be connected to the GPIO lines. +config LEDS_CLEVO_MAIL + tristate "Mail LED on Clevo notebook (EXPERIMENTAL)" + depends on LEDS_CLASS && X86 && SERIO_I8042 && DMI && EXPERIMENTAL + help + This driver makes the mail LED accessible from userspace + programs through the leds subsystem. This LED can blink at + about 0.5Hz and 1Hz. + + This module can drive the mail LED for the following notebooks: + + Clevo D410J + Clevo D410V + Clevo D400V/D470V (not tested, but might work) + Clevo M540N + Clevo M5x0N (not tested, but might work) + Positivo Mobile (Clevo M5x0V) + + To compile this driver as a module, choose M here: the + module will be called leds-clevo-mail. + comment "LED Triggers" config LEDS_TRIGGERS
drivers/leds/leds-clevo-mail.c +226 -0
226 lines not on this page · raw message
drivers/leds/Makefile +1 -0
@@ -17,6 +17,7 @@ obj-$(CONFIG_LEDS_WRAP) += leds-wrap.o obj-$(CONFIG_LEDS_H1940) += leds-h1940.o obj-$(CONFIG_LEDS_COBALT) += leds-cobalt.o obj-$(CONFIG_LEDS_GPIO) += leds-gpio.o +obj-$(CONFIG_LEDS_CLEVO_MAIL) += leds-clevo-mail.o # LED Triggers obj-$(CONFIG_LEDS_TRIGGER_TIMER) += ledtrig-timer.o
Quoted 12 lines · On Fri, 19 Oct 2007 20:52:52 +0200 Németh Márton wrote:
From: Márton Németh <[email protected]>
The driver supports the mail LED commonly found on different Clevo notebooks. The driver access the LED through the i8042 hardware and implements the support for hardware accelerated blink function.
Signed-off-by Márton Németh <[email protected]>---
diff -uprN linux-2.6.23.orig/drivers/leds/Kconfig linux-2.6.23/drivers/leds/Kconfig
drivers/leds/Kconfig config LEDS_GPIO · line 110
Quoted 10 lines
drivers/leds/Kconfig +7
-0@@ -101,6 +101,26 @@ config LEDS_GPIO outputs. To be useful the particular board must have LEDs and they must be connected to the GPIO lines. +config LEDS_CLEVO_MAIL + tristate "Mail LED on Clevo notebook (EXPERIMENTAL)" + depends on LEDS_CLASS && X86 && SERIO_I8042 && DMI && EXPERIMENTAL + help + This driver makes the mail LED accessible from userspace + programs through the leds subsystem. This LED can blink at + about 0.5Hz and 1Hz.
?:
can blink 1 time every 1 or 2 seconds.
Quoted 24 lines
drivers/leds/Kconfig +12
-0+ This module can drive the mail LED for the following notebooks: + + Clevo D410J + Clevo D410V + Clevo D400V/D470V (not tested, but might work) + Clevo M540N + Clevo M5x0N (not tested, but might work) + Positivo Mobile (Clevo M5x0V) + + To compile this driver as a module, choose M here: the + module will be called leds-clevo-mail. + comment "LED Triggers" config LEDS_TRIGGERSdiff -uprN
linux-2.6.23.orig/drivers/leds/leds-clevo-mail.clinux-2.6.23/drivers/leds/leds-clevo-mail.cdrivers/leds/leds-clevo-mail.c +5
-0@@ -0,0 +1,226 @@ + +#include <linux/module.h> + +#include <linux/platform_device.h> +#include <linux/err.h>
drivers/leds/leds-clevo-mail.c line 15
Quoted 10 lines
drivers/leds/leds-clevo-mail.c +10
-0+#include <linux/leds.h> + +#include <linux/io.h> +#include <linux/dmi.h> + +#include <linux/i8042.h> + + +#define TRUE 1 +#define FALSE 0
Just use true / false.
drivers/leds/leds-clevo-mail.c line 24
Quoted 8 lines
drivers/leds/leds-clevo-mail.c +8
-0+#define HANDLED 0 + +#define CLEVO_MAIL_LED_OFF 0x0084 +#define CLEVO_MAIL_LED_BLINK_1HZ 0x008A +#define CLEVO_MAIL_LED_BLINK_0_5HZ 0x0083 + +#define MODULE_FNAME "leds-clevo-mail.c" +#define DRVNAME "clevo-mail-led"
Kbuild provides KBUILD_BASENAME and KBUILD_MODNAME. Can you use one of those?
drivers/leds/leds-clevo-mail.c line 25
Quoted 1 line
drivers/leds/leds-clevo-mail.c +1
-0+#define NO_RESOURCES NULL
Just use NULL.
drivers/leds/leds-clevo-mail.c line 41
Quoted 8 lines
drivers/leds/leds-clevo-mail.c +8
-0+MODULE_AUTHOR ("Márton Németh <[email protected]>" ); +MODULE_DESCRIPTION ("Clevo mail LED driver" ); +MODULE_LICENSE ("GPL" ); + + +static int __init clevo_mail_led_dmi_callback (struct dmi_system_id *id) +{ + printk(KERN_INFO MODULE_FNAME ": '%s' found\n", id->ident);
MODULE_FNAME string looks too long and out of place here.
drivers/leds/leds-clevo-mail.c line 45
Quoted 4 lines
drivers/leds/leds-clevo-mail.c +4
-0+return 1 ; +} + +/**
/** introduces kernel-doc, but there is no following kernel-doc...., so just use /*, please.
Quoted 10 lines
drivers/leds/leds-clevo-mail.c +10
-0+* struct mail_led_whitelist - List of known good models +* +* Contains the known good models this driver is compatible with. +* When adding a new model try to be as strict as possible. This +* makes it possible to keep the false positives (the model is +* detected as working, but in reality it is not) as low as +* possible. +*/ +static struct dmi_system_id __initdata mail_led_whitelist[] = { + {
...
drivers/leds/leds-clevo-mail.c line 135
Quoted 7 lines
drivers/leds/leds-clevo-mail.c +7
-0+ { } +}; + + + +static struct led_classdev clevo_mail_led = { + .name ="clevo::mail" ,
What's the purpose of the "::"?
Quoted 6 lines
drivers/leds/leds-clevo-mail.c +6
-0+ .brightness_set = clevo_mail_led_set, + .blink_set = clevo_mail_led_blink, +}; + + +static struct platform_driver clevo_mail_led_driver = {
drivers/leds/leds-clevo-mail.c line 178
Quoted 10 lines
drivers/leds/leds-clevo-mail.c +10
-0+ .probe = clevo_mail_led_probe, + .remove = clevo_mail_led_remove, +#ifdef CONFIG_PM + .suspend = clevo_mail_led_suspend, + .resume = clevo_mail_led_resume, +#endif + .driver = { + .name = DRVNAME, + }, +};
We prefer to have clevo_mail_led_susped() and _resume() defined all the time, so that the above ifdef & endif can go away, so do more like:
#ifdef CONFIG_PM
/* those functions as you have them */
#else
#define clevo_mail_led_suspend NULL
#define clevo_mail_led_resume NULL
#endif
Quoted 9 lines
drivers/leds/leds-clevo-mail.c +9
-0+static int __init clevo_mail_led_init (void ) +{ +int error =0 ; +int count =0 ; + +/* Check with the help of DMI if we are running on supported hardware */ +if (!nodetect) { + count =dmi_check_system (mail_led_whitelist); + }else {
drivers/leds/leds-clevo-mail.c line 199
Quoted 10 lines
drivers/leds/leds-clevo-mail.c +10
-0+ count =1 ; + printk(KERN_ERR MODULE_FNAME ": Skipping DMI detection. " +"If the driver works on your hardware please " +"report model and the output of dmidecode in tracker " + "at http://sourceforge.net/projects/clevo-mailled/\n"); + } + +if (!count) { +return -ENODEV ; + }
Don't use braces on one-statement/one-line "blocks."
drivers/leds/leds-clevo-mail.c line 206
Quoted 6 lines
drivers/leds/leds-clevo-mail.c +6
-0+ pdev =platform_device_register_simple (DRVNAME ,-1 ,NO_RESOURCES ,0 ); +if (!IS_ERR (pdev)) { + error =platform_driver_probe (&clevo_mail_led_driver, + clevo_mail_led_probe); +if (error) { + printk(KERN_ERR MODULE_FNAME
Use the module logical name, not source file name.
drivers/leds/leds-clevo-mail.c line 215
Quoted 9 lines
drivers/leds/leds-clevo-mail.c +9
-0+ ": Can't probe platform driver\n"); +platform_device_unregister (pdev); + } + }else { + error =PTR_ERR (pdev); + } + +return error; +}
---
~Randy
Quoted 17 lines · Randy Dunlap
Quoted 15 lines · On Fri, 19 Oct 2007 20:52:52 +0200 Németh Márton wrote:
From: Márton Németh <[email protected]>
The driver supports the mail LED commonly found on different Clevo notebooks. The driver access the LED through the i8042 hardware and implements the support for hardware accelerated blink function.
Signed-off-by Márton Németh <[email protected]>---
diff -uprN linux-2.6.23.orig/drivers/leds/Kconfig linux-2.6.23/drivers/leds/Kconfigdrivers/leds/Kconfig +0
-0@@ -101,6 +101,26 @@ config LEDS_GPIO outputs. To be useful the particular board must have LEDs and they must be connected to the GPIO lines.
Quoted 10 lines
drivers/leds/Kconfig config LEDS_GPIO · line 110
Quoted 7 lines
drivers/leds/Kconfig +7
-0+config LEDS_CLEVO_MAIL + tristate "Mail LED on Clevo notebook (EXPERIMENTAL)" + depends on LEDS_CLASS && X86 && SERIO_I8042 && DMI && EXPERIMENTAL + help + This driver makes the mail LED accessible from userspace + programs through the leds subsystem. This LED can blink at + about 0.5Hz and 1Hz.?:
can blink 1 time every 1 or 2 seconds.
It has three modes, I'll try to explain this in more detail in my next patch version.
Quoted 15 lines
drivers/leds/Kconfig config LEDS_GPIO · line 123
Quoted 13 lines
drivers/leds/Kconfig +12
-0+ This module can drive the mail LED for the following notebooks: + + Clevo D410J + Clevo D410V + Clevo D400V/D470V (not tested, but might work) + Clevo M540N + Clevo M5x0N (not tested, but might work) + Positivo Mobile (Clevo M5x0V) + + To compile this driver as a module, choose M here: the + module will be called leds-clevo-mail. +comment "LED Triggers"
Quoted 1 line
config
LEDS_TRIGGERS
I'll document the led trigger extension in more detail in the next version of the "[PATCH 2/3] leds-clevo-mail: hw accelerated LED blink extension", in drivers/leds/Kconfig.
Quoted 4 lines
drivers/leds/Kconfig config LEDS_GPIO · line 124
Quoted 2 lines
drivers/leds/Kconfig +2
-0+#define TRUE 1 +#define FALSE 0Just use true / false.
I'll delete these #defines.
Quoted 11 lines
drivers/leds/Kconfig config LEDS_GPIO
Quoted 8 lines
drivers/leds/Kconfig +8
-0+#define HANDLED 0 + +#define CLEVO_MAIL_LED_OFF 0x0084 +#define CLEVO_MAIL_LED_BLINK_1HZ 0x008A +#define CLEVO_MAIL_LED_BLINK_0_5HZ 0x0083 + +#define MODULE_FNAME "leds-clevo-mail.c" +#define DRVNAME "clevo-mail-led"
KbuildprovidesKBUILD_BASENAMEandKBUILD_MODNAME. Can you use one of those?
I'll use KBUILD_MODNAME instead.
Quoted 3 lines
Quoted 1 line
+#define
NO_RESOURCESNULLJust use NULL.
I'll delete NO_RESOURCES.
Quoted 5 lines
drivers/leds/Kconfig
Quoted 3 lines
drivers/leds/Kconfig +3
-0+static int __init clevo_mail_led_dmi_callback(struct dmi_system_id *id) +{ + printk(KERN_INFO MODULE_FNAME ": '%s' found\n", id->ident);
MODULE_FNAMEstring looks too long and out of place here.
I'll replace MODULE_FNAME with KBUILD_MODNAME. Is this what you mean?
Quoted 4 lines
Quoted 1 line
+/**
/** introduces kernel-doc, but there is no following kernel-doc...., so just use /*, please.
OK, I'll replace.
Quoted 4 lines
drivers/leds/Kconfig
Quoted 2 lines
drivers/leds/Kconfig +2
-0+static struct led_classdev clevo_mail_led = { + .name = "clevo::mail",What's the purpose of the "::"?
Here I have the problem that the different laptop models have different colors of mail LED. So I left out the color and introduced the type of the LED which is "mail". I'll try to document this in Documents/leds-class.txt
Quoted 16 lines
drivers/leds/Kconfig
Quoted 11 lines
drivers/leds/Kconfig +11
-0+static struct platform_driver clevo_mail_led_driver = { + .probe = clevo_mail_led_probe, + .remove = clevo_mail_led_remove, +#ifdef CONFIG_PM + .suspend = clevo_mail_led_suspend, + .resume = clevo_mail_led_resume, +#endif + .driver = { + .name = DRVNAME, + }, +};We prefer to have
clevo_mail_led_susped()and_resume()defined all the time, so that the above ifdef & endif can go away, so do more like:
Quoted 6 lines
#ifdef
CONFIG_PM
/* those functions as you have them */
#else
#defineclevo_mail_led_suspendNULL
#defineclevo_mail_led_resumeNULL
#endif
I'll change this to what you proposed.
Quoted 5 lines
drivers/leds/Kconfig
Quoted 3 lines
drivers/leds/Kconfig +3
-0+ if (!count) { + return -ENODEV; + }Don't use braces on one-statement/one-line "blocks."
I'll remove the braces for the on-line expression.
Quoted 8 lines
drivers/leds/Kconfig
Quoted 6 lines
drivers/leds/Kconfig +6
-0+ pdev = platform_device_register_simple(DRVNAME, -1, NO_RESOURCES, 0); + if (!IS_ERR(pdev)) { + error = platform_driver_probe(&clevo_mail_led_driver, + clevo_mail_led_probe); + if (error) { + printk(KERN_ERR MODULE_FNAMEUse the module logical name, not source file name.
I'll use KBUILD_MODNAME instead.
Quoted 2 lines
---
~Randy
Márton Németh
From: Márton Németh <[email protected]>
The driver supports the mail LED commonly found on different Clevo notebooks. The driver access the LED through the i8042 hardware and implements the support for hardware accelerated blink function.
---
Documentation/leds-class.txt +3 -1
@@ -39,7 +39,9 @@ LED Device Naming Is currently of the form: -"devicename:colour" +"devicename:colour" or +"devicename:colour:type" or +"devicename::type" There have been calls for LED properties such as colour to be exported as individual led class attributes. As a solution which doesn't incur as much
drivers/leds/Kconfig +33 -0
@@ -101,6 +101,39 @@ config LEDS_GPIO outputs. To be useful the particular board must have LEDs and they must be connected to the GPIO lines. +config LEDS_CLEVO_MAIL + tristate "Mail LED on Clevo notebook (EXPERIMENTAL)" + depends on LEDS_CLASS && X86 && SERIO_I8042 && DMI && EXPERIMENTAL + help + This driver makes the mail LED accessible from userspace + programs through the leds subsystem. This LED have three + known mode: off, blink at 0.5Hz and blink at 1Hz. + + The driver supports two kinds of interface: using ledtrig-timer + or through /sys/class/leds/clevo::mail/brightness. As this LED + cannot change it's brightness it blinks instead. The brightness + value 0 means off, 1..127 means blink at 0.5Hz and 128..255 means + blink at 1Hz. + + Example to start blink at 0.5Hz: + echo 1 >/sys/class/leds/clevo::mail/brightness + - or - + echo timer >/sys/class/leds/clevo::mail/trigger + echo 1000 >/sys/class/leds/clevo::mail/delay_on + echo 1000 >/sys/class/leds/clevo::mail/delay_off + + This module can drive the mail LED for the following notebooks: + + Clevo D410J + Clevo D410V + Clevo D400V/D470V (not tested, but might work) + Clevo M540N + Clevo M5x0N (not tested, but might work) + Positivo Mobile (Clevo M5x0V) + + To compile this driver as a module, choose M here: the + module will be called leds-clevo-mail. + comment "LED Triggers" config LEDS_TRIGGERS
drivers/leds/leds-clevo-mail.c +210 -0
210 lines not on this page · raw message
drivers/leds/Makefile +1 -0
@@ -17,6 +17,7 @@ obj-$(CONFIG_LEDS_WRAP) += leds-wrap.o obj-$(CONFIG_LEDS_H1940) += leds-h1940.o obj-$(CONFIG_LEDS_COBALT) += leds-cobalt.o obj-$(CONFIG_LEDS_GPIO) += leds-gpio.o +obj-$(CONFIG_LEDS_CLEVO_MAIL) += leds-clevo-mail.o # LED Triggers obj-$(CONFIG_LEDS_TRIGGER_TIMER) += ledtrig-timer.o
Documentation/leds-class.txt LED Device Naming · line 44
Quoted 11 lines · Németh Márton
diff -uprN
linux-2.6.23.orig/Documentation/leds-class.txtlinux-2.6.23/Documentation/leds-class.txtDocumentation/leds-class.txt +3
-1@@ -39,7 +39,9 @@ LED Device Naming Is currently of the form: -"devicename:colour" +"devicename:colour" or +"devicename:colour:type" or +"devicename::type"
Adding a the idea of function here was proposed a while back and we probably should do that but it needs to be a separate patch and some of the other drivers probably need tweaking for consistency. I will take care of this for the next merge window.
Quoted 10 lines
diff -uprN linux-2.6.23.orig/drivers/leds/Kconfig linux-2.6.23/drivers/leds/Kconfig
drivers/leds/Kconfig +3
-0@@ -101,6 +101,39 @@ config LEDS_GPIO outputs. To be useful the particular board must have LEDs and they must be connected to the GPIO lines. +config LEDS_CLEVO_MAIL + tristate "Mail LED on Clevo notebook (EXPERIMENTAL)" + depends on LEDS_CLASS && X86 && SERIO_I8042 && DMI && EXPERIMENTAL
drivers/leds/Kconfig config LEDS_GPIO · line 116
Quoted 10 lines
drivers/leds/Kconfig +10
-0+ help + This driver makes the mail LED accessible from userspace + programs through the leds subsystem. This LED have three + known mode: off, blink at 0.5Hz and blink at 1Hz. + + The driver supports two kinds of interface: using ledtrig-timer + or through /sys/class/leds/clevo::mail/brightness. As this LED + cannot change it's brightness it blinks instead. The brightness + value 0 means off, 1..127 means blink at 0.5Hz and 128..255 means + blink at 1Hz.
Hmm, there really is no way to just turn the LED on? :(
drivers/leds/Kconfig config LEDS_GPIO · line 123
Quoted 6 lines
drivers/leds/Kconfig +6
-0+ Example to start blink at 0.5Hz: + echo 1 >/sys/class/leds/clevo::mail/brightness + - or - + echo timer >/sys/class/leds/clevo::mail/trigger + echo 1000 >/sys/class/leds/clevo::mail/delay_on + echo 1000 >/sys/class/leds/clevo::mail/delay_off
The Kconfig option shouldn't need to go into this much detail. If we need this much detail, we should probably enhance the LED documentation in Documentation/ instead.
Quoted 7 lines
diff -uprN
linux-2.6.23.orig/drivers/leds/leds-clevo-mail.clinux-2.6.23/drivers/leds/leds-clevo-mail.cdrivers/leds/leds-clevo-mail.c +3
-0@@ -0,0 +1,210 @@ + +#include <linux/module.h> +
drivers/leds/leds-clevo-mail.c line 13
Quoted 10 lines
drivers/leds/leds-clevo-mail.c +10
-0+#include <linux/platform_device.h> +#include <linux/err.h> +#include <linux/leds.h> + +#include <linux/io.h> +#include <linux/dmi.h> + +#include <linux/i8042.h> + +#define HANDLED 0
Probably easiest to turn the two references to this into "return 0"
Other than that the driver looks ok and I'm happy to add a version with the above minor tweaks to the LED tree.
Cheers,
Richard
Quoted 8 lines · Richard Purdie
Quoted 7 lines · Németh Márton
diff -uprN linux-2.6.23.orig/drivers/leds/Kconfig linux-2.6.23/drivers/leds/Kconfig
drivers/leds/Kconfig +0
-0@@ -101,6 +101,39 @@ config LEDS_GPIO outputs. To be useful the particular board must have LEDs and they must be connected to the GPIO lines.
Quoted 15 lines
drivers/leds/Kconfig config LEDS_GPIO · line 116
Quoted 13 lines
drivers/leds/Kconfig +13
-0+config LEDS_CLEVO_MAIL + tristate "Mail LED on Clevo notebook (EXPERIMENTAL)" + depends on LEDS_CLASS && X86 && SERIO_I8042 && DMI && EXPERIMENTAL + help + This driver makes the mail LED accessible from userspace + programs through the leds subsystem. This LED have three + known mode: off, blink at 0.5Hz and blink at 1Hz. + + The driver supports two kinds of interface: using ledtrig-timer + or through /sys/class/leds/clevo::mail/brightness. As this LED + cannot change it's brightness it blinks instead. The brightness + value 0 means off, 1..127 means blink at 0.5Hz and 128..255 means + blink at 1Hz.Hmm, there really is no way to just turn the LED on? :(
I looked up the printed manual (which can be also found at ftp://ftp.clevo.com.tw/USRMANUAL/D4x0J/D4x0J_CUG.zip ), page 9 contains the following information:
LED Power & Communication Indicators:
Icon | Color | Description
-----+------------------+-----------------------------------------------
Mail | Blinking Green & | New mail has arrived
LED | Orange |
+------------------+-----------------------------------------------
| Fast Blinking | New mail has arrived from users defined
| Green & Orange | in the Special Group in the Auto Mail Checker
+------------------+-----------------------------------------------
| Green | The (optional) Wireless LAN module is On
+------------------+-----------------------------------------------
| Orange | The (optional) Bluetooth module is On
+------------------+-----------------------------------------------
| Half Orange/ | Both the (optional) Wireless LAN and Bluetooth
| Half Green | modules are On (see sidebar)-----+------------------+-----------------------------------------------
Unfortunately I do not have these optional modules so I never saw the LED just on without blinking. I don't know if there is any hardware limitation turning on the LED without the optional modules.
I will send a new patch collection which addresses the other mentioned topics.
Márton Németh
Quoted 6 lines · Németh Márton
Quoted 1 line · Richard Purdie
Hmm, there really is no way to just turn the LED on? :(
Unfortunately I do not have these optional modules so I never saw the LED just on without blinking. I don't know if there is any hardware limitation turning on the LED without the optional modules.
Ok, if we don't know how to do it or even if its possible, so be it. If you ever do find out please do update the driver.
Quoted 1 line
I will send a new patch collection which addresses the other mentioned topics.
Thanks, I've applied the patches although I've rewritten the second one to update the drivers with LED names that don't match the naming convention. They'll appear in the led git tree and hence -mm.
Thanks,
Richard