From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1424730AbcFIH3z (ORCPT ); Thu, 9 Jun 2016 03:29:55 -0400 Received: from mailout4.w1.samsung.com ([210.118.77.14]:15700 "EHLO mailout4.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1423277AbcFIH3v (ORCPT ); Thu, 9 Jun 2016 03:29:51 -0400 MIME-version: 1.0 Content-type: text/plain; charset=UTF-8; format=flowed X-AuditID: cbfec7f5-f792a6d000001302-ff-57591aea2eaa Content-transfer-encoding: 8BIT Message-id: <57591AE9.7060009@samsung.com> Date: Thu, 09 Jun 2016 09:29:45 +0200 From: Jacek Anaszewski User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130804 Thunderbird/17.0.8 To: Stephan Linz Cc: linux-leds@vger.kernel.org, linux-ide@vger.kernel.org, Joseph Jezak , Nico Macrionitis , =?UTF-8?B?SsO2cmcgU29tbWVy?= , Richard Purdie , Tejun Heo , "David S. Miller" , linux-kernel@vger.kernel.org Subject: Re: [PATCH v3 1/7] leds: convert IDE trigger to common disk trigger References: <20160608223000.433-1-linz@li-pro.net> In-reply-to: <20160608223000.433-1-linz@li-pro.net> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrBLMWRmVeSWpSXmKPExsVy+t/xq7qvpCLDDfommVnsXfCVyWLO+RYW i/PL2lktzj3oYbY4tuMRk8XlXXPYLLa+WcdocfX1IhaL3bueslr8Wn6U0YHLY0nzZiaPO1vW sXpsWXmTyePy9zfMHptWdbJ57H6b6rFn/g9Wj8+b5AI4orhsUlJzMstSi/TtErgy2r62sBWs tqi48WIbYwPjed0uRk4OCQETiZ93ljJC2GISF+6tZ+ti5OIQEljKKPFg/3WwBK+AoMSPyfdY uhg5OJgF5CWOXMoGCTMLmEk8alnHDFH/jFFi1r03bBD1WhI7525iB7FZBFQl3hxZwgxiswkY Svx88ZoJxBYViJD4c3ofK4gtIqAkMelJKzvIIGaBQ0wS1yf/BVssLOAjsefvYbAiIQEjidNv 94Et4BQwlli+bCXbBEaBWUjum4Vw3ywk9y1gZF7FKJpamlxQnJSea6RXnJhbXJqXrpecn7uJ ERIfX3cwLj1mdYhRgINRiYdXIyUiXIg1say4MvcQowQHs5IIbzF7ZLgQb0piZVVqUX58UWlO avEhRmkOFiVx3pm73ocICaQnlqRmp6YWpBbBZJk4OKUaGJ3PTtvHEyu1ZN7ssJdfV3LLBCWW fPz55OD9gkcqRn0KB9YErrI3Ftgv8Km2ODi2qjpIm71z17PICya/u9Jv1jHG3jxo/ep0eOXl f0rnVukbvTk2099IfYKIZDmf5oy3ESse3fgrPyEph7klf8ZUqVNGIrlSmxXiZaXveho4hP9S LrS7sZ3DQomlOCPRUIu5qDgRAD+QnnuLAgAA Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > Cc: Nico Macrionitis > Cc: Jörg Sommer > Cc: Richard Purdie > Cc: Jacek Anaszewski > Signed-off-by: Stephan Linz > --- > 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 > #include > #include > +#include > #include > #include > > @@ -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 >