Hi Linus,
Thank you for the patch. It looks good to me, just
waiting for DaveM ack to merge the patch via LED tree.
Best regards,
Jacek Anaszewski
On 02/24/2018 11:45 PM, Linus Walleij wrote:
> This adds two new disk triggers for triggering on reads
> and writes respectively, named "disk-read" and "disk-write".
>
> The use case comes from working on the D-Link DNS-313 NAS
> box. This features an RGB LED for disk activity. with
> these two triggers I can couple the green LED to read
> activity and the red LED to write activity, which gives
> the appropriate user feedback about what is happening
> on the disk. When tested it gave exactly the feedback
> desired.
>
> The in-kernel interface is simply changed to pass a bool
> indicating if the activity is write activity and update
> each trigger (and the composite "disk-activity" trigger)
> depending on what is passed in.
>
> This requires an ACK from the libata/IDE maintainers.
>
> Cc: Tejun Heo <tj@xxxxxxxxxx>
> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@xxxxxxxxxxx>
> Cc: Jacek Anaszewski <jacek.anaszewski@xxxxxxxxx>
> Cc: Pavel Machek <pavel@xxxxxx>
> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> ---
> drivers/ata/libata-core.c | 2 +-
> drivers/ide/ide-disk.c | 2 +-
> drivers/leds/trigger/ledtrig-disk.c | 12 +++++++++++-
> include/linux/leds.h | 4 ++--
> 4 files changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 3c09122bf038..fa75de6abbf1 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -5219,7 +5219,7 @@ void ata_qc_complete(struct ata_queued_cmd *qc)
> struct ata_port *ap = qc->ap;
>
> /* Trigger the LED (if available) */
> - ledtrig_disk_activity();
> + ledtrig_disk_activity(!!(qc->tf.flags & ATA_TFLAG_WRITE));
>
> /* XXX: New EH and old EH use different mechanisms to
> * synchronize EH with regular execution path.
> diff --git a/drivers/ide/ide-disk.c b/drivers/ide/ide-disk.c
> index 188d1b03715d..67bc72d78fbf 100644
> --- a/drivers/ide/ide-disk.c
> +++ b/drivers/ide/ide-disk.c
> @@ -187,7 +187,7 @@ static ide_startstop_t ide_do_rw_disk(ide_drive_t *drive, struct request *rq,
> BUG_ON(drive->dev_flags & IDE_DFLAG_BLOCKED);
> BUG_ON(blk_rq_is_passthrough(rq));
>
> - ledtrig_disk_activity();
> + ledtrig_disk_activity(rq_data_dir(rq) == WRITE);
>
> pr_debug("%s: %sing: block=%llu, sectors=%u\n",
> drive->name, rq_data_dir(rq) == READ ? "read" : "writ",
> diff --git a/drivers/leds/trigger/ledtrig-disk.c b/drivers/leds/trigger/ledtrig-disk.c
> index cd525b4125eb..9816b0d60270 100644
> --- a/drivers/leds/trigger/ledtrig-disk.c
> +++ b/drivers/leds/trigger/ledtrig-disk.c
> @@ -18,9 +18,11 @@
> #define BLINK_DELAY 30
>
> DEFINE_LED_TRIGGER(ledtrig_disk);
> +DEFINE_LED_TRIGGER(ledtrig_disk_read);
> +DEFINE_LED_TRIGGER(ledtrig_disk_write);
> DEFINE_LED_TRIGGER(ledtrig_ide);
>
> -void ledtrig_disk_activity(void)
> +void ledtrig_disk_activity(bool write)
> {
> unsigned long blink_delay = BLINK_DELAY;
>
> @@ -28,12 +30,20 @@ void ledtrig_disk_activity(void)
> &blink_delay, &blink_delay, 0);
> led_trigger_blink_oneshot(ledtrig_ide,
> &blink_delay, &blink_delay, 0);
> + if (write)
> + led_trigger_blink_oneshot(ledtrig_disk_write,
> + &blink_delay, &blink_delay, 0);
> + else
> + led_trigger_blink_oneshot(ledtrig_disk_read,
> + &blink_delay, &blink_delay, 0);
> }
> EXPORT_SYMBOL(ledtrig_disk_activity);
>
> static int __init ledtrig_disk_init(void)
> {
> led_trigger_register_simple("disk-activity", &ledtrig_disk);
> + led_trigger_register_simple("disk-read", &ledtrig_disk_read);
> + led_trigger_register_simple("disk-write", &ledtrig_disk_write);
> led_trigger_register_simple("ide-disk", &ledtrig_ide);
>
> return 0;
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index 5579c64c8fd6..b7e82550e655 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -346,9 +346,9 @@ static inline void *led_get_trigger_data(struct led_classdev *led_cdev)
>
> /* Trigger specific functions */
> #ifdef CONFIG_LEDS_TRIGGER_DISK
> -extern void ledtrig_disk_activity(void);
> +extern void ledtrig_disk_activity(bool write);
> #else
> -static inline void ledtrig_disk_activity(void) {}
> +static inline void ledtrig_disk_activity(bool write) {}
> #endif
>
> #ifdef CONFIG_LEDS_TRIGGER_MTD
>
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
