lkml.org 
[lkml]   [2006]   [Feb]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 11/12] LED: Add IDE disk activity LED trigger
Hi,

Generally it looks fine, some minor comments below.

On 2/5/06, Richard Purdie <rpurdie@rpsys.net> wrote:
> Add an LED trigger for IDE disk activity to the ide-disk driver.

filesystem activity

> Signed-off-by: Richard Purdie <rpurdie@rpsys.net>
>
> Index: linux-2.6.15/drivers/ide/ide-disk.c
> ===================================================================
> --- linux-2.6.15.orig/drivers/ide/ide-disk.c 2006-02-04 13:35:37.000000000 +0000
> +++ linux-2.6.15/drivers/ide/ide-disk.c 2006-02-04 15:17:18.000000000 +0000
> @@ -60,6 +60,7 @@
> #include <linux/genhd.h>
> #include <linux/slab.h>
> #include <linux/delay.h>
> +#include <linux/leds.h>
>
> #define _IDE_DISK
>
> @@ -80,6 +81,8 @@
>
> static DECLARE_MUTEX(idedisk_ref_sem);
>
> +INIT_LED_TRIGGER(ide_led_trigger);
> +
> #define to_ide_disk(obj) container_of(obj, struct ide_disk_obj, kref)
>
> #define ide_disk_g(disk) \
> @@ -311,10 +314,12 @@
>
> if (!blk_fs_request(rq)) {
> blk_dump_rq_flags(rq, "ide_do_rw_disk - bad command");
> - ide_end_request(drive, 0, 0);
> + ide_end_rw_disk(drive, 0, 0);
> return ide_stopped;
> }
>
> + led_trigger_event(ide_led_trigger, LED_FULL);
> +
> pr_debug("%s: %sing: block=%llu, sectors=%lu, buffer=0x%08lx\n",
> drive->name, rq_data_dir(rq) == READ ? "read" : "writ",
> block, rq->nr_sectors, (unsigned long)rq->buffer);
> @@ -325,6 +330,12 @@
> return __ide_do_rw_disk(drive, rq, block);
> }
>
> +static int ide_end_rw_disk(ide_drive_t *drive, int uptodate, int nr_sectors)

ide_end_rw_disk() is used before it is declared.

Does it compile?

> +{
> + led_trigger_event(ide_led_trigger, LED_OFF);

It should check for blk_fs_request().

->end_request() can be used for other request types.

> + ide_end_request(drive, uptodate, nr_sectors);
> +}
> +
> /*
> * Queries for true maximum capacity of the drive.
> * Returns maximum LBA address (> 0) of the drive, 0 if failed.
> @@ -1097,7 +1108,7 @@
> .media = ide_disk,
> .supports_dsc_overlap = 0,
> .do_request = ide_do_rw_disk,
> - .end_request = ide_end_request,
> + .end_request = ide_end_rw_disk,
> .error = __ide_error,
> .abort = __ide_abort,
> .proc = idedisk_proc,
> @@ -1259,11 +1270,13 @@
>
> static void __exit idedisk_exit (void)
> {
> + led_trigger_unregister_simple(ide_led_trigger);
> driver_unregister(&idedisk_driver.gen_driver);

Shouldn't ordering be reverse to this in idedisk_init()?
First driver_unregister(), then led_trigger_unregister_simple()?

> }
>
> static int __init idedisk_init(void)
> {
> + led_trigger_register_simple("ide-disk", &ide_led_trigger);
> return driver_register(&idedisk_driver.gen_driver);

What does happen if driver_register() fails?

Bartlomiej
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2006-02-06 17:56    [W:0.039 / U:0.544 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site