> On Tue, 2007-05-08 at 21:26 +0900, kogiidena wrote:
> To:Richard-san
> CC:all
>
> LED driver of I-O DATA LANDISK and USL-5P
> Please apply following patch
>
> Signed-off-by: kogiidena <[email protected]>
>
[...]
> +/* HDD-access-LED setting at landisk status LED */
> +static void landisk_disk_trig_activate(struct led_classdev *led_cdev)
> +{
> + unsigned long flags;
> + spin_lock_irqsave(&landisk_led_lock, flags);
> + ctrl_outb((ctrl_inb(PA_LED) & ~0x0c) | 0x04, PA_LED);
> + spin_unlock_irqrestore(&landisk_led_lock, flags);
> +}
> +
> +static void landisk_disk_trig_deactivate(struct led_classdev *led_cdev)
> +{
> + unsigned long flags;
> + spin_lock_irqsave(&landisk_led_lock, flags);
> + ctrl_outb((ctrl_inb(PA_LED) & ~0x0c) | 0x0c, PA_LED);
> + spin_unlock_irqrestore(&landisk_led_lock, flags);
> +}
> +
> +static struct led_trigger landisk_disk_led_trigger = {
> + .name = "disk",
> + .activate = landisk_disk_trig_activate,
> + .deactivate = landisk_disk_trig_deactivate,
> +};
You can't do this since the trigger will appear for all LEDs and it only
applies to a single LED. There has been previous discussion of LED
specific triggers and we'll need that support before you can make
something like this work. Nobody has sent me a patch for that yet and I
haven't had time to write one...
+static int __init landisk_led_init(void)
> +{
> + u8 orig, test;
> +
> + orig = ctrl_inb(PA_LED);
> + ctrl_outb(0x40, PA_LED);
> +
> + test = ctrl_inb(PA_LED);
> + ctrl_outb(orig, PA_LED);
> +
> + landisk_arch = (test == 0x40);
> +
> + if (landisk_arch == 0) {
> + /* arch == landisk */
> + ctrl_outb(orig | 0x07, PA_LED);
> + landisk_leds[1].default_trigger = "disk";
> + led_trigger_register(&landisk_disk_led_trigger);
> + } else {
> + /* arch == usl-5p */
> + ctrl_outb(orig | 0x03, PA_LED);
> + }
> + return platform_driver_register(&landisk_led_driver);
Broken error handling - you should check the return code of
led_trigger_register().
> +static void __exit landisk_led_exit(void)
> +{
> + if (landisk_arch == 0)
> + led_trigger_unregister(&landisk_disk_led_trigger);
> + return platform_driver_unregister(&landisk_led_driver);
> +}
void functions don't return.
Other than those issues, it looks ok.
Richard
-
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]
