[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-id: <57591AE9.7060009@samsung.com>
Date: Thu, 09 Jun 2016 09:29:45 +0200
From: Jacek Anaszewski <j.anaszewski@...sung.com>
To: Stephan Linz <linz@...pro.net>
Cc: linux-leds@...r.kernel.org, linux-ide@...r.kernel.org,
Joseph Jezak <josejx@...too.org>,
Nico Macrionitis <acrux@...xppc.org>,
Jörg Sommer <joerg@...a.gnuu.de>,
Richard Purdie <rpurdie@...ys.net>, Tejun Heo <tj@...nel.org>,
"David S. Miller" <davem@...emloft.net>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/7] leds: convert IDE trigger to common disk trigger
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@...too.org>
> Cc: Nico Macrionitis <acrux@...xppc.org>
> Cc: Jörg Sommer <joerg@...a.gnuu.de>
> Cc: Richard Purdie <rpurdie@...ys.net>
> Cc: Jacek Anaszewski <j.anaszewski@...sung.com>
> Signed-off-by: Stephan Linz <linz@...pro.net>
> ---
> 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
>
Powered by blists - more mailing lists