Re: [PATCH v3 1/7] leds: convert IDE trigger to common disk trigger
From: Jacek Anaszewski
Date: Thu Jun 09 2016 - 03:30:00 EST
Hi Stephan,
Thanks for the patch.
Generally it looks ok, with one exception: we have to keep
ide-disk trigger, so as not to break existing users.
Please just register two triggers in ledtrig_disk_init,
similarly as it was done for mtd trigger:
drivers/leds/trigger/ledtrig-mtd.c
Thanks,
Jacek Anaszewski
On 06/09/2016 12:29 AM, Stephan Linz wrote:
This patch converts the IDE specific LED trigger to a generic disk
activity LED trigger. The libata core is now a trigger source just
like before the IDE disk driver. It's merely a replacement of the
string ide by disk.
The patch is taken from http://dev.gentoo.org/~josejx/ata.patch and is
widely used by any ibook/powerbook owners with great satisfaction.
Likewise, it is very often used successfully on different ARM platforms.
The original patch was split into different parts to clarify platform
independent and dependent changes.
Cc: Joseph Jezak <josejx@xxxxxxxxxx>
Cc: Nico Macrionitis <acrux@xxxxxxxxxxx>
Cc: JÃrg Sommer <joerg@xxxxxxxxxxxx>
Cc: Richard Purdie <rpurdie@xxxxxxxxx>
Cc: Jacek Anaszewski <j.anaszewski@xxxxxxxxxxx>
Signed-off-by: Stephan Linz <linz@xxxxxxxxxx>
---
Changes in v3:
- Port to kernel 4.x
- Split into platform independent and dependent parts.
v2: https://patchwork.ozlabs.org/patch/117485/
v1: http://dev.gentoo.org/~josejx/ata.patch
---
drivers/ata/libata-core.c | 4 ++++
drivers/ide/ide-disk.c | 2 +-
drivers/leds/leds-hp6xx.c | 2 +-
drivers/leds/trigger/Kconfig | 8 ++++----
drivers/leds/trigger/Makefile | 2 +-
.../trigger/{ledtrig-ide-disk.c => ledtrig-disk.c} | 20 ++++++++++----------
include/linux/leds.h | 6 +++---
7 files changed, 24 insertions(+), 20 deletions(-)
rename drivers/leds/trigger/{ledtrig-ide-disk.c => ledtrig-disk.c} (50%)
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 6be7770..2eca572 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -69,6 +69,7 @@
#include <asm/unaligned.h>
#include <linux/cdrom.h>
#include <linux/ratelimit.h>
+#include <linux/leds.h>
#include <linux/pm_runtime.h>
#include <linux/platform_device.h>
@@ -5072,6 +5073,9 @@ void ata_qc_complete(struct ata_queued_cmd *qc)
{
struct ata_port *ap = qc->ap;
+ /* Trigger the LED (if available) */
+ ledtrig_disk_activity();
+
/* 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 05dbcce..5ceb176 100644
--- a/drivers/ide/ide-disk.c
+++ b/drivers/ide/ide-disk.c
@@ -186,7 +186,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(rq->cmd_type != REQ_TYPE_FS);
- ledtrig_ide_activity();
+ ledtrig_disk_activity();
pr_debug("%s: %sing: block=%llu, sectors=%u\n",
drive->name, rq_data_dir(rq) == READ ? "read" : "writ",
diff --git a/drivers/leds/leds-hp6xx.c b/drivers/leds/leds-hp6xx.c
index a6b8db0..137969f 100644
--- a/drivers/leds/leds-hp6xx.c
+++ b/drivers/leds/leds-hp6xx.c
@@ -50,7 +50,7 @@ static struct led_classdev hp6xx_red_led = {
static struct led_classdev hp6xx_green_led = {
.name = "hp6xx:green",
- .default_trigger = "ide-disk",
+ .default_trigger = "disk-activity",
.brightness_set = hp6xxled_green_set,
.flags = LED_CORE_SUSPENDRESUME,
};
diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig
index 9893d91..3f9ddb9 100644
--- a/drivers/leds/trigger/Kconfig
+++ b/drivers/leds/trigger/Kconfig
@@ -33,12 +33,12 @@ config LEDS_TRIGGER_ONESHOT
If unsure, say Y.
-config LEDS_TRIGGER_IDE_DISK
- bool "LED IDE Disk Trigger"
- depends on IDE_GD_ATA
+config LEDS_TRIGGER_DISK
+ bool "LED Disk Trigger"
+ depends on IDE_GD_ATA || ATA
depends on LEDS_TRIGGERS
help
- This allows LEDs to be controlled by IDE disk activity.
+ This allows LEDs to be controlled by disk activity.
If unsure, say Y.
config LEDS_TRIGGER_MTD
diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile
index 8cc64a4..a72c43c 100644
--- a/drivers/leds/trigger/Makefile
+++ b/drivers/leds/trigger/Makefile
@@ -1,6 +1,6 @@
obj-$(CONFIG_LEDS_TRIGGER_TIMER) += ledtrig-timer.o
obj-$(CONFIG_LEDS_TRIGGER_ONESHOT) += ledtrig-oneshot.o
-obj-$(CONFIG_LEDS_TRIGGER_IDE_DISK) += ledtrig-ide-disk.o
+obj-$(CONFIG_LEDS_TRIGGER_DISK) += ledtrig-disk.o
obj-$(CONFIG_LEDS_TRIGGER_MTD) += ledtrig-mtd.o
obj-$(CONFIG_LEDS_TRIGGER_HEARTBEAT) += ledtrig-heartbeat.o
obj-$(CONFIG_LEDS_TRIGGER_BACKLIGHT) += ledtrig-backlight.o
diff --git a/drivers/leds/trigger/ledtrig-ide-disk.c b/drivers/leds/trigger/ledtrig-disk.c
similarity index 50%
rename from drivers/leds/trigger/ledtrig-ide-disk.c
rename to drivers/leds/trigger/ledtrig-disk.c
index 15123d3..7a1fe44 100644
--- a/drivers/leds/trigger/ledtrig-ide-disk.c
+++ b/drivers/leds/trigger/ledtrig-disk.c
@@ -1,5 +1,5 @@
/*
- * LED IDE-Disk Activity Trigger
+ * LED Disk Activity Trigger
*
* Copyright 2006 Openedhand Ltd.
*
@@ -17,20 +17,20 @@
#define BLINK_DELAY 30
-DEFINE_LED_TRIGGER(ledtrig_ide);
+DEFINE_LED_TRIGGER(ledtrig_disk);
-void ledtrig_ide_activity(void)
+void ledtrig_disk_activity(void)
{
- unsigned long ide_blink_delay = BLINK_DELAY;
+ unsigned long disk_blink_delay = BLINK_DELAY;
- led_trigger_blink_oneshot(ledtrig_ide,
- &ide_blink_delay, &ide_blink_delay, 0);
+ led_trigger_blink_oneshot(ledtrig_disk,
+ &disk_blink_delay, &disk_blink_delay, 0);
}
-EXPORT_SYMBOL(ledtrig_ide_activity);
+EXPORT_SYMBOL(ledtrig_disk_activity);
-static int __init ledtrig_ide_init(void)
+static int __init ledtrig_disk_init(void)
{
- led_trigger_register_simple("ide-disk", &ledtrig_ide);
+ led_trigger_register_simple("disk-activity", &ledtrig_disk);
return 0;
}
-device_initcall(ledtrig_ide_init);
+device_initcall(ledtrig_disk_init);
diff --git a/include/linux/leds.h b/include/linux/leds.h
index d2b1306..28a3ef5 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -324,10 +324,10 @@ static inline void *led_get_trigger_data(struct led_classdev *led_cdev)
#endif /* CONFIG_LEDS_TRIGGERS */
/* Trigger specific functions */
-#ifdef CONFIG_LEDS_TRIGGER_IDE_DISK
-extern void ledtrig_ide_activity(void);
+#ifdef CONFIG_LEDS_TRIGGER_DISK
+extern void ledtrig_disk_activity(void);
#else
-static inline void ledtrig_ide_activity(void) {}
+static inline void ledtrig_disk_activity(void) {}
#endif
#ifdef CONFIG_LEDS_TRIGGER_MTD