| Message ID | 20180224224556.24297-1-linus.walleij@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | leds: Extends disk trigger for reads and writes | expand |
On Sat, Feb 24, 2018 at 11:45:56PM +0100, 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@kernel.org> > Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > Cc: Jacek Anaszewski <jacek.anaszewski@gmail.com> > Cc: Pavel Machek <pavel@ucw.cz> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> For libata part, Acked-by: Tejun Heo <tj@kernel.org> Thanks. -- tejun
On Tue, Feb 27, 2018 at 10:15:31AM -0800, Tejun Heo wrote: > On Sat, Feb 24, 2018 at 11:45:56PM +0100, 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@kernel.org> > > Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > > Cc: Jacek Anaszewski <jacek.anaszewski@gmail.com> > > Cc: Pavel Machek <pavel@ucw.cz> > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > > For libata part, > > Acked-by: Tejun Heo <tj@kernel.org> Ooh, btw, we have READ and WRITE macros defined in linux/kernel.h. It might be a better idea to use them instead of bool. Thanks. -- tejun
[ added DaveM to Cc: ] On Saturday, February 24, 2018 11:45:56 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@kernel.org> > Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > Cc: Jacek Anaszewski <jacek.anaszewski@gmail.com> > Cc: Pavel Machek <pavel@ucw.cz> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > --- > 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 Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
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@kernel.org> > Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > Cc: Jacek Anaszewski <jacek.anaszewski@gmail.com> > Cc: Pavel Machek <pavel@ucw.cz> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > 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 >
On Sat 2018-02-24 23:45:56, 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@kernel.org> > Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > Cc: Jacek Anaszewski <jacek.anaszewski@gmail.com> Acked-by: Pavel Machek <pavel@ucw.cz> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Thu, Mar 1, 2018 at 8:39 PM, Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote: > Thank you for the patch. It looks good to me, just > waiting for DaveM ack to merge the patch via LED tree. The patch doesn't touch any networking code, and I don't see why Dave would pay any interest in this new disk activity trigger, but if he likes to review LED triggers, fine with me :) Yours, Linus Walleij
On Friday, March 02, 2018 10:37:20 AM Linus Walleij wrote: > On Thu, Mar 1, 2018 at 8:39 PM, Jacek Anaszewski > <jacek.anaszewski@gmail.com> wrote: > > > Thank you for the patch. It looks good to me, just > > waiting for DaveM ack to merge the patch via LED tree. > > The patch doesn't touch any networking code, and I don't see > why Dave would pay any interest in this new disk activity trigger, Dave is also drivers/ide/ Maintainer.. > but if he likes to review LED triggers, fine with me :) Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
On Fri, Mar 2, 2018 at 11:24 AM, Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> wrote: > On Friday, March 02, 2018 10:37:20 AM Linus Walleij wrote: >> On Thu, Mar 1, 2018 at 8:39 PM, Jacek Anaszewski >> <jacek.anaszewski@gmail.com> wrote: >> >> > Thank you for the patch. It looks good to me, just >> > waiting for DaveM ack to merge the patch via LED tree. >> >> The patch doesn't touch any networking code, and I don't see >> why Dave would pay any interest in this new disk activity trigger, > > Dave is also drivers/ide/ Maintainer.. Bah sorry for my hopeless ignorance. Should have read the MAINTAINERS better. Yours, Linus Walleij
Hi Dave, Can we have your ack for the change in drivers/ide/ide-disk.c? Best regards, Jacek Anaszewski On 02/28/2018 04:03 PM, Bartlomiej Zolnierkiewicz wrote: > > [ added DaveM to Cc: ] > > On Saturday, February 24, 2018 11:45:56 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@kernel.org> >> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> >> Cc: Jacek Anaszewski <jacek.anaszewski@gmail.com> >> Cc: Pavel Machek <pavel@ucw.cz> >> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > > Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > >> --- >> 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 > > Best regards, > -- > Bartlomiej Zolnierkiewicz > Samsung R&D Institute Poland > Samsung Electronics > >
From: Jacek Anaszewski <jacek.anaszewski@gmail.com> Date: Sat, 10 Mar 2018 21:12:02 +0100 > Hi Dave, > > Can we have your ack for the change in drivers/ide/ide-disk.c? Sure: Acked-by: David S. Miller <davem@davemloft.net>
Hi Linus, 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@kernel.org> > Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > Cc: Jacek Anaszewski <jacek.anaszewski@gmail.com> > Cc: Pavel Machek <pavel@ucw.cz> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > 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 > Applied. thanks. -- Best regards, Jacek Anaszewski
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
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@kernel.org> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> Cc: Jacek Anaszewski <jacek.anaszewski@gmail.com> Cc: Pavel Machek <pavel@ucw.cz> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- 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(-) -- 2.14.3