From patchwork Fri Dec 23 09:00:08 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?TWljaGHFgiBLxJlwaWXFhA==?= X-Patchwork-Id: 9487277 X-Patchwork-Delegate: andy.shevchenko@gmail.com Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 90CE8601D7 for ; Fri, 23 Dec 2016 09:00:23 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 78D352807B for ; Fri, 23 Dec 2016 09:00:23 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6BC0F2807E; Fri, 23 Dec 2016 09:00:23 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.3 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 5F04D2808C for ; Fri, 23 Dec 2016 09:00:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753624AbcLWJAT (ORCPT ); Fri, 23 Dec 2016 04:00:19 -0500 Received: from mail-lf0-f65.google.com ([209.85.215.65]:35327 "EHLO mail-lf0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751340AbcLWJAS (ORCPT ); Fri, 23 Dec 2016 04:00:18 -0500 Received: by mail-lf0-f65.google.com with SMTP id x140so4610256lfa.2 for ; Fri, 23 Dec 2016 01:00:17 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kempniu.pl; s=google; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=WZBnTJqlapvOobg+l8cIY6/hMusOpjz3CDicirp1hmA=; b=o2vWNke1W46QTWQDLhd/iEglme8jmv9jPwBGzLUPfKq+VED2KYTHHxnqdBRvBvVpXz wkXb4rzXQjsozGW1IGuvnDSiWKNOY4CrfExoce9xCegRmXuKX8/2e/ORwL8VTZIAyS7m Ir6phy3GWjb34WqnJxvwDA53k/w6PrC03DHqM= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=WZBnTJqlapvOobg+l8cIY6/hMusOpjz3CDicirp1hmA=; b=NjcSPPetuwEbrBANDrLfq5wkG2zORiy33j4xFqdRys3rQOPpEEQncNe6xb7FfiZOv8 st3lNRkBh2yzM+23cY1PhAP0JOTj5W8eHgPfAg7rinEWrLUx2plcMXCYRXJXZEiWUhc8 V2JO+lkicACEfmCI5jBUHCCSDrmkImCCutR1f2wug3o5boVWcHx9wU1j7USBn3BofO3h z7Bw+a3ghr0f1ch0ocHVCHdXTYBS5D4CWt8bSUloAoGnmu9bRb1ABjb1RW+d6IBHJNEC 9XXFn9foKA/AQidJzmYTUqxczT45izNxK/3DzmwZWne6vVoIy75Gb7pvqN4eFqVigEgu CVMg== X-Gm-Message-State: AIkVDXIrp3rEaKDRHn98Dvg7sFwayXCS6IlFr3w3OlT3JeuTMCcin4T2122gcB787I7KwQ== X-Received: by 10.25.206.3 with SMTP id e3mr5755713lfg.42.1482483616401; Fri, 23 Dec 2016 01:00:16 -0800 (PST) Received: from kmp-mobile.hq.kempniu.pl (kmp-mobile.hq.kempniu.pl. [2001:6a0:200:83b0::2d90]) by smtp.googlemail.com with ESMTPSA id y25sm7684498lja.13.2016.12.23.01.00.15 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 23 Dec 2016 01:00:15 -0800 (PST) From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= To: Jonathan Woithe , Darren Hart Cc: platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH] platform/x86: fujitsu-laptop: use brightness_set_blocking for LED-setting callbacks Date: Fri, 23 Dec 2016 10:00:08 +0100 Message-Id: <20161223090008.10445-1-kernel@kempniu.pl> X-Mailer: git-send-email 2.11.0 MIME-Version: 1.0 Sender: platform-driver-x86-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: platform-driver-x86@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP All LED-setting functions in fujitsu-laptop are currently assigned to the brightness_set callback, which is incorrect because they can sleep (due to their use of call_fext_func(), which in turn issues ACPI calls) and the documentation (in include/linux/leds.h) clearly states they must not. Assign them to brightness_set_blocking instead and change them to match the expected function prototype. This change makes it possible to use Fujitsu-specific LEDs with "heavy" triggers, like disk-activity or phy0rx. Fixes: 3a407086090b ("fujitsu-laptop: Add BL power, LED control and radio state information") Fixes: 4f62568c1fcf ("fujitsu-laptop: Support radio LED") Fixes: d6b88f64b0d4 ("fujitsu-laptop: Add support for eco LED") Signed-off-by: Michał Kępień Acked-by: Jonathan Woithe --- Even back in 2.6.29, when the first Fujitsu-specific LED was introduced, the brightness_set callback was annotated with the following text: "Must not sleep, use a workqueue if needed". To make things easier for LED drivers, the LED core introduced (in 3.19) brightness_set_sync, which was later renamed to brightness_set_blocking (in 4.5). It allows easy use of brightness-setting callbacks that may sleep, which is exactly what we need here. Without this patch, here is a quick way to lock your system up within seconds: # echo disk-activity > "/sys/class/leds/fujitsu::radio_led/trigger" # dd if=/dev/sda of=/dev/null bs=1M count=10000 Applying this patch allowed me to successfully perform the above stress test without a visible influence on system stability on a Lifebook E744 with an SSD. Two notes about the patch itself: - As fujitsu-laptop is already at odds with checkpatch rules, I decided to stick with its current coding style so that the functional changes introduced by this patch do not blend with changes related to coding style. - In logolamp_set(), returning the result of the second call_fext_func() call as the function return value is obviously pretty arbitrary, but nobody has cared about these return values at all since day one, so I decided to keep this patch simple. I can submit another one which properly handles call_fext_func() failures in that function, if you think that would be helpful. drivers/platform/x86/fujitsu-laptop.c | 42 +++++++++++++++++------------------ 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c index 61f39abf5dc8..82d67715ce76 100644 --- a/drivers/platform/x86/fujitsu-laptop.c +++ b/drivers/platform/x86/fujitsu-laptop.c @@ -177,43 +177,43 @@ static void acpi_fujitsu_hotkey_notify(struct acpi_device *device, u32 event); #if IS_ENABLED(CONFIG_LEDS_CLASS) static enum led_brightness logolamp_get(struct led_classdev *cdev); -static void logolamp_set(struct led_classdev *cdev, +static int logolamp_set(struct led_classdev *cdev, enum led_brightness brightness); static struct led_classdev logolamp_led = { .name = "fujitsu::logolamp", .brightness_get = logolamp_get, - .brightness_set = logolamp_set + .brightness_set_blocking = logolamp_set }; static enum led_brightness kblamps_get(struct led_classdev *cdev); -static void kblamps_set(struct led_classdev *cdev, +static int kblamps_set(struct led_classdev *cdev, enum led_brightness brightness); static struct led_classdev kblamps_led = { .name = "fujitsu::kblamps", .brightness_get = kblamps_get, - .brightness_set = kblamps_set + .brightness_set_blocking = kblamps_set }; static enum led_brightness radio_led_get(struct led_classdev *cdev); -static void radio_led_set(struct led_classdev *cdev, +static int radio_led_set(struct led_classdev *cdev, enum led_brightness brightness); static struct led_classdev radio_led = { .name = "fujitsu::radio_led", .brightness_get = radio_led_get, - .brightness_set = radio_led_set + .brightness_set_blocking = radio_led_set }; static enum led_brightness eco_led_get(struct led_classdev *cdev); -static void eco_led_set(struct led_classdev *cdev, +static int eco_led_set(struct led_classdev *cdev, enum led_brightness brightness); static struct led_classdev eco_led = { .name = "fujitsu::eco_led", .brightness_get = eco_led_get, - .brightness_set = eco_led_set + .brightness_set_blocking = eco_led_set }; #endif @@ -267,48 +267,48 @@ static int call_fext_func(int cmd, int arg0, int arg1, int arg2) #if IS_ENABLED(CONFIG_LEDS_CLASS) /* LED class callbacks */ -static void logolamp_set(struct led_classdev *cdev, +static int logolamp_set(struct led_classdev *cdev, enum led_brightness brightness) { if (brightness >= LED_FULL) { call_fext_func(FUNC_LEDS, 0x1, LOGOLAMP_POWERON, FUNC_LED_ON); - call_fext_func(FUNC_LEDS, 0x1, LOGOLAMP_ALWAYS, FUNC_LED_ON); + return call_fext_func(FUNC_LEDS, 0x1, LOGOLAMP_ALWAYS, FUNC_LED_ON); } else if (brightness >= LED_HALF) { call_fext_func(FUNC_LEDS, 0x1, LOGOLAMP_POWERON, FUNC_LED_ON); - call_fext_func(FUNC_LEDS, 0x1, LOGOLAMP_ALWAYS, FUNC_LED_OFF); + return call_fext_func(FUNC_LEDS, 0x1, LOGOLAMP_ALWAYS, FUNC_LED_OFF); } else { - call_fext_func(FUNC_LEDS, 0x1, LOGOLAMP_POWERON, FUNC_LED_OFF); + return call_fext_func(FUNC_LEDS, 0x1, LOGOLAMP_POWERON, FUNC_LED_OFF); } } -static void kblamps_set(struct led_classdev *cdev, +static int kblamps_set(struct led_classdev *cdev, enum led_brightness brightness) { if (brightness >= LED_FULL) - call_fext_func(FUNC_LEDS, 0x1, KEYBOARD_LAMPS, FUNC_LED_ON); + return call_fext_func(FUNC_LEDS, 0x1, KEYBOARD_LAMPS, FUNC_LED_ON); else - call_fext_func(FUNC_LEDS, 0x1, KEYBOARD_LAMPS, FUNC_LED_OFF); + return call_fext_func(FUNC_LEDS, 0x1, KEYBOARD_LAMPS, FUNC_LED_OFF); } -static void radio_led_set(struct led_classdev *cdev, +static int radio_led_set(struct led_classdev *cdev, enum led_brightness brightness) { if (brightness >= LED_FULL) - call_fext_func(FUNC_RFKILL, 0x5, RADIO_LED_ON, RADIO_LED_ON); + return call_fext_func(FUNC_RFKILL, 0x5, RADIO_LED_ON, RADIO_LED_ON); else - call_fext_func(FUNC_RFKILL, 0x5, RADIO_LED_ON, 0x0); + return call_fext_func(FUNC_RFKILL, 0x5, RADIO_LED_ON, 0x0); } -static void eco_led_set(struct led_classdev *cdev, +static int eco_led_set(struct led_classdev *cdev, enum led_brightness brightness) { int curr; curr = call_fext_func(FUNC_LEDS, 0x2, ECO_LED, 0x0); if (brightness >= LED_FULL) - call_fext_func(FUNC_LEDS, 0x1, ECO_LED, curr | ECO_LED_ON); + return call_fext_func(FUNC_LEDS, 0x1, ECO_LED, curr | ECO_LED_ON); else - call_fext_func(FUNC_LEDS, 0x1, ECO_LED, curr & ~ECO_LED_ON); + return call_fext_func(FUNC_LEDS, 0x1, ECO_LED, curr & ~ECO_LED_ON); } static enum led_brightness logolamp_get(struct led_classdev *cdev)