Re: [PATCH] leds: Extends disk trigger for reads and writes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux