Re: [PATCH v3 1/7] leds: convert IDE trigger to common disk trigger

From: Stephan Linz
Date: Thu Jun 09 2016 - 18:35:29 EST


Hi Jacek,

to keep the original ide-disk trigger is good idea to be backward
compatible. I'll submit a new v4 patch set.


br,
Stephan

Am 09.06.2016 um 09:29 schrieb Jacek Anaszewski:
> 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
>>
>
>
>

Attachment: signature.asc
Description: OpenPGP digital signature