From nobody Mon Feb 9 23:42:16 2026 Delivered-To: importer2@patchew.org Received-SPF: pass (zohomail.com: domain of vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; envelope-from=linux-kernel-owner@vger.kernel.org; helo=vger.kernel.org; Authentication-Results: mx.zohomail.com; dkim=fail; spf=pass (zohomail.com: domain of vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass(p=none dis=none) header.from=kernel.org ARC-Seal: i=1; a=rsa-sha256; t=1621350939; cv=none; d=zohomail.com; s=zohoarc; b=QSx0JJXCGEc7piJFeXxqRCuFiQv1a0RSVAjyail2oFysQg1RIrNu6bv0YMUvqDQ4Cx55ap7K1F2juAXRWNh95DmQT3g5hrwiX/H72iRWC1LpOlvBhewX5TkktF1BXrACwRxyf8wr/k8iI+Q+rUcPuYkNXvRqK8HdqtdYbDVGykI= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1621350939; h=Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Id:MIME-Version:Message-ID:References:Sender:Subject:To; bh=AHl59BlW75kXm/hBYNQC5HFuAalIqPIQvFR3oYu9D58=; b=fm2nJsaaE4faFA7jTpAiZW9RzY5j0n/3XQMP52vgb9iU8qmnospsEddjOlCjrqc9BJYNwndBNvIOccxIcm5HSUZYb4bcR4he36WynUMrPhalVO6rPyBZ70zkwq7Hp45ObatiPLK3UeNskvpf9CSxSmTdjJgBdrdUaEuWUeyrRCc= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=fail; spf=pass (zohomail.com: domain of vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass header.from= (p=none dis=none) header.from= Return-Path: Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mx.zohomail.com with SMTP id 1621350939012291.5667132781257; Tue, 18 May 2021 08:15:39 -0700 (PDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1350161AbhERPMo (ORCPT ); Tue, 18 May 2021 11:12:44 -0400 Received: from mail.kernel.org ([198.145.29.99]:49660 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1345056AbhERPKh (ORCPT ); Tue, 18 May 2021 11:10:37 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id 2A64561355; Tue, 18 May 2021 15:09:10 +0000 (UTC) Received: by mail.kernel.org with local (Exim 4.94.2) (envelope-from ) id 1lj1LI-007HOE-5B; Tue, 18 May 2021 17:09:08 +0200 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1621350550; bh=gLdwdgyLLaR+xP/SNPgHYHTQ7ptcIkwGqK9Ot6um84c=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Is0Csuf7ynpzMQrsLhZbi7o6qiZMtO/rOt+f8EysmnH2KLFTIj1tfcXsZ/aJNBKrR ya4+psHNNowXiqDJ9eYXQTiJbIXJusl9Xs1nxE6TIXU7fprNcgB1hTjkVkb9L1A7mo B/Ki5VrZ4Q788tAFASu7VPXubZjI0FOYOYaaWR3LP6imNaUXEOAQ2+1lpv4Fc8/AI+ 9kfoTCzSpPDtIJYZ6+d0IBCrmoKquO6D5RizmJPAHg4T2Jh33Vmm7/y+Ejv31v3a7K YYnurB0sbkBceG8YR9nS70/Y/rxDxTYj2oVjKfQzW492cP3lUoNtNv0RuQhfevUFVa Beuty5tMGZoFw== From: Mauro Carvalho Chehab Cc: linuxarm@huawei.com, mauro.chehab@huawei.com, Mauro Carvalho Chehab , Pavel Machek , gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org, linux-leds@vger.kernel.org Subject: [PATCH v2 05/17] leds: leds-nuc: add all types of brightness Date: Tue, 18 May 2021 17:08:54 +0200 Message-Id: X-Mailer: git-send-email 2.31.1 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: Mauro Carvalho Chehab To: unlisted-recipients:; (no To-header on input) Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-ZohoMail-DKIM: fail (Header signature does not verify) Content-Type: text/plain; charset="utf-8" Improve the logic in order to support not only S0 brightness, but also the brightness for other indicators and for all power states. Signed-off-by: Mauro Carvalho Chehab --- drivers/leds/leds-nuc.c | 369 +++++++++++++++++++++++++++------------- 1 file changed, 249 insertions(+), 120 deletions(-) diff --git a/drivers/leds/leds-nuc.c b/drivers/leds/leds-nuc.c index e12fa2e7a488..df65bf17e0e6 100644 --- a/drivers/leds/leds-nuc.c +++ b/drivers/leds/leds-nuc.c @@ -55,21 +55,89 @@ enum led_new_get_subcmd { LED_NEW_GET_CONTROL_ITEM =3D 0x01, }; =20 +enum led_function { + LED_FUNC_BRIGHTNESS, + LED_FUNC_COLOR1, + LED_FUNC_COLOR_GREEN, + LED_FUNC_COLOR_BLUE, + + LED_FUNC_BLINK_BEHAVIOR, + LED_FUNC_BLINK_FREQ, + + LED_FUNC_HDD_BEHAVIOR, + LED_FUNC_ETH_TYPE, + LED_FUNC_POWER_LIMIT_SCHEME, + + MAX_LED_FUNC +}; + +enum led_indicators { + LED_IND_POWER_STATE, + LED_IND_HDD_ACTIVITY, + LED_IND_ETHERNET, + LED_IND_WIFI, + LED_IND_SOFTWARE, + LED_IND_POWER_LIMIT, + LED_IND_DISABLE, + + MAX_IND =3D LED_IND_DISABLE +}; + +/* + * control items ID for each of the valid indicators on spec Rev 0.64. + */ +static const u8 led_func_rev_0_64[MAX_IND][MAX_LED_FUNC] =3D { + [LED_IND_POWER_STATE] =3D { /* Offsets for each power state */ + [LED_FUNC_BRIGHTNESS] =3D 0x00, + [LED_FUNC_BLINK_BEHAVIOR] =3D 0x01, + [LED_FUNC_BLINK_FREQ] =3D 0x02, + [LED_FUNC_COLOR1] =3D 0x03, + [LED_FUNC_COLOR_GREEN] =3D 0x04, + [LED_FUNC_COLOR_BLUE] =3D 0x05 + }, + [LED_IND_HDD_ACTIVITY] =3D { + [LED_FUNC_BRIGHTNESS] =3D 0x00, + [LED_FUNC_COLOR1] =3D 0x01, + [LED_FUNC_COLOR_GREEN] =3D 0x02, + [LED_FUNC_COLOR_BLUE] =3D 0x03, + [LED_FUNC_HDD_BEHAVIOR] =3D 0x04 + }, + [LED_IND_ETHERNET] =3D { + [LED_FUNC_ETH_TYPE] =3D 0x00, + [LED_FUNC_BRIGHTNESS] =3D 0x01, + [LED_FUNC_COLOR1] =3D 0x02, + [LED_FUNC_COLOR_GREEN] =3D 0x03, + [LED_FUNC_COLOR_BLUE] =3D 0x04 + }, + [LED_IND_WIFI] =3D { + [LED_FUNC_BRIGHTNESS] =3D 0x00, + [LED_FUNC_COLOR1] =3D 0x01, + [LED_FUNC_COLOR_GREEN] =3D 0x02, + [LED_FUNC_COLOR_BLUE] =3D 0x03 + }, + [LED_IND_SOFTWARE] =3D { + [LED_FUNC_BRIGHTNESS] =3D 0x00, + [LED_FUNC_BLINK_BEHAVIOR] =3D 0x01, + [LED_FUNC_BLINK_FREQ] =3D 0x02, + [LED_FUNC_COLOR1] =3D 0x03, + [LED_FUNC_COLOR_GREEN] =3D 0x04, + [LED_FUNC_COLOR_BLUE] =3D 0x05 + }, + [LED_IND_POWER_LIMIT] =3D { + [LED_FUNC_POWER_LIMIT_SCHEME] =3D 0x00, + [LED_FUNC_BRIGHTNESS] =3D 0x01, + [LED_FUNC_COLOR1] =3D 0x02, + [LED_FUNC_COLOR_GREEN] =3D 0x03, + [LED_FUNC_COLOR_BLUE] =3D 0x04 + }, +}; + /* LED color indicator */ #define LED_BLUE_AMBER BIT(0) #define LED_BLUE_WHITE BIT(1) #define LED_RGB BIT(2) #define LED_SINGLE_COLOR BIT(3) =20 -/* LED indicator options */ -#define LED_IND_POWER_STATE BIT(0) -#define LED_IND_HDD_ACTIVITY BIT(1) -#define LED_IND_ETHERNET BIT(2) -#define LED_IND_WIFI BIT(3) -#define LED_IND_SOFTWARE BIT(4) -#define LED_IND_POWER_LIMIT BIT(5) -#define LED_IND_DISABLE BIT(6) - static const char * const led_names[] =3D { "nuc::power", "nuc::hdd", @@ -87,7 +155,6 @@ struct nuc_nmi_led { u8 indicator; u32 color_type; u32 avail_indicators; - u32 control_items; }; =20 struct nuc_wmi { @@ -201,9 +268,9 @@ static int nuc_nmi_cmd(struct device *dev, static int nuc_wmi_query_leds(struct device *dev) { struct nuc_wmi *priv =3D dev_get_drvdata(dev); - u8 cmd, input[NUM_INPUT_ARGS] =3D { 0 }; + u8 input[NUM_INPUT_ARGS] =3D { 0 }; u8 output[NUM_OUTPUT_ARGS]; - int i, id, ret, ver =3D LED_API_UNKNOWN; + int id, ret, ver =3D LED_API_UNKNOWN; u8 leds; =20 /* @@ -214,9 +281,8 @@ static int nuc_wmi_query_leds(struct device *dev) * FIXME: Should add a fallback code for it to work with older NUCs, * as LED_QUERY returns an error on older devices like Skull Canyon. */ - cmd =3D LED_QUERY; input[0] =3D LED_QUERY_LIST_ALL; - ret =3D nuc_nmi_cmd(dev, cmd, input, output); + ret =3D nuc_nmi_cmd(dev, LED_QUERY, input, output); if (ret =3D=3D -ENOENT) { ver =3D LED_API_NUC6; } else if (ret) { @@ -255,12 +321,11 @@ static int nuc_wmi_query_leds(struct device *dev) =20 led->id =3D id; =20 - cmd =3D LED_QUERY; input[0] =3D LED_QUERY_COLOR_TYPE; input[1] =3D id; - ret =3D nuc_nmi_cmd(dev, cmd, input, output); + ret =3D nuc_nmi_cmd(dev, LED_QUERY, input, output); if (ret) { - dev_warn(dev, "error %d on led %i\n", ret, i); + dev_warn(dev, "error %d on led %i\n", ret, id); return ret; } =20 @@ -268,23 +333,11 @@ static int nuc_wmi_query_leds(struct device *dev) output[1] << 8 | output[2] << 16; =20 - cmd =3D LED_NEW_GET_STATUS; - input[0] =3D LED_NEW_GET_CURRENT_INDICATOR; - input[1] =3D i; - ret =3D nuc_nmi_cmd(dev, cmd, input, output); - if (ret) { - dev_warn(dev, "error %d on led %i\n", ret, i); - return ret; - } - - led->indicator =3D output[0]; - - cmd =3D LED_QUERY; input[0] =3D LED_QUERY_INDICATOR_OPTIONS; - input[1] =3D i; - ret =3D nuc_nmi_cmd(dev, cmd, input, output); + input[1] =3D id; + ret =3D nuc_nmi_cmd(dev, LED_QUERY, input, output); if (ret) { - dev_warn(dev, "error %d on led %i\n", ret, i); + dev_warn(dev, "error %d on led %i\n", ret, id); return ret; } =20 @@ -292,23 +345,18 @@ static int nuc_wmi_query_leds(struct device *dev) output[1] << 8 | output[2] << 16; =20 - cmd =3D LED_QUERY; - input[0] =3D LED_QUERY_CONTROL_ITEMS; - input[1] =3D i; - input[2] =3D led->indicator; - ret =3D nuc_nmi_cmd(dev, cmd, input, output); + input[0] =3D LED_NEW_GET_CURRENT_INDICATOR; + input[1] =3D id; + ret =3D nuc_nmi_cmd(dev, LED_NEW_GET_STATUS, input, output); if (ret) { - dev_warn(dev, "error %d on led %i\n", ret, i); + dev_warn(dev, "error %d on led %i\n", ret, id); return ret; } + led->indicator =3D output[0]; =20 - led->control_items =3D output[0] | - output[1] << 8 | - output[2] << 16; - - dev_dbg(dev, "%s: id: %02x, color type: %06x, indicator: %06x, control i= tems: %06x\n", - led_names[led->id], led->id, - led->color_type, led->indicator, led->control_items); + dev_dbg(dev, "%s: id: %02x, color type: %06x, indicator: %02x (avail %06= x)\n", + led_names[led->id], led->id, led->color_type, + led->indicator, led->avail_indicators); =20 priv->num_leds++; } @@ -316,6 +364,82 @@ static int nuc_wmi_query_leds(struct device *dev) return 0; } =20 +static bool nuc_wmi_test_control(struct device *dev, + struct nuc_nmi_led *led, u8 ctrl) +{ + int ret, avail_ctrls; + u8 output[NUM_OUTPUT_ARGS]; + u8 input[NUM_INPUT_ARGS] =3D { + LED_QUERY_CONTROL_ITEMS, + led->id, + led->indicator + }; + + ret =3D nuc_nmi_cmd(dev, LED_QUERY, input, output); + if (ret) + return false; + + avail_ctrls =3D output[0] | + output[1] << 8 | + output[2] << 16; + + return avail_ctrls & BIT(ctrl); +} + +static int nuc_wmi_get_brightness_offset(struct device *dev, + struct nuc_nmi_led *led, u8 offset) +{ + u8 input[NUM_INPUT_ARGS]; + u8 output[NUM_OUTPUT_ARGS]; + int ret, ctrl; + + if (led->indicator =3D=3D LED_IND_DISABLE) + return -ENODEV; + + ctrl =3D led_func_rev_0_64[led->indicator][LED_FUNC_BRIGHTNESS] + offset; + + if (!nuc_wmi_test_control(dev, led, ctrl)) + return -ENODEV; + + input[0] =3D LED_NEW_GET_CONTROL_ITEM; + input[1] =3D led->id; + input[2] =3D led->indicator; + input[3] =3D ctrl; + + ret =3D nuc_nmi_cmd(dev, LED_NEW_GET_STATUS, input, output); + if (ret) + return ret; + + dev_dbg(dev, "%s: id: %02x, brightness: %02x\n", + led_names[led->id], led->id, output[0]); + + return output[0]; +} + +static ssize_t nuc_wmi_set_brightness_offset(struct device *dev, + struct nuc_nmi_led *led, + u8 offset, + u8 val) +{ + u8 input[NUM_INPUT_ARGS]; + int ctrl; + + if (led->indicator =3D=3D LED_IND_DISABLE) + return -ENODEV; + + ctrl =3D led_func_rev_0_64[led->indicator][LED_FUNC_BRIGHTNESS] + offset; + + if (!nuc_wmi_test_control(dev, led, ctrl)) + return -ENODEV; + + input[0] =3D led->id; + input[1] =3D led->indicator; + input[2] =3D ctrl; + input[3] =3D val; + + return nuc_nmi_cmd(dev, LED_SET_VALUE, input, NULL); +} + /* * LED show/store routines */ @@ -323,6 +447,21 @@ static int nuc_wmi_query_leds(struct device *dev) #define LED_ATTR_RW(_name) \ DEVICE_ATTR(_name, 0644, show_##_name, store_##_name) =20 +#define LED_ATTR_POWER_STATE_RW(_name, offset) \ + static ssize_t show_##_name(struct device *dev, \ + struct device_attribute *attr, \ + char *buf) \ + { \ + return show_brightness_offset(dev, attr, offset, buf); \ + } \ + static ssize_t store_##_name(struct device *dev, \ + struct device_attribute *attr, \ + const char *buf, size_t len) \ + { \ + return store_brightness_offset(dev, attr, offset, buf, len); \ + } \ + static DEVICE_ATTR(_name, 0644, show_##_name, store_##_name) + static const char * const led_indicators[] =3D { "Power State", "HDD Activity", @@ -395,98 +534,93 @@ static ssize_t store_indicator(struct device *dev, return -EINVAL; } =20 -/* Get S0 brightness */ -static ssize_t show_s0_brightness(struct device *dev, - struct device_attribute *attr, - char *buf) +/* Get brightness */ +static ssize_t show_brightness_offset(struct device *dev, + struct device_attribute *attr, + u8 offset, + char *buf) { struct led_classdev *cdev =3D dev_get_drvdata(dev); struct nuc_nmi_led *led =3D container_of(cdev, struct nuc_nmi_led, cdev); - u8 cmd, input[NUM_INPUT_ARGS] =3D { 0 }; - u8 output[NUM_OUTPUT_ARGS]; int ret; =20 - cmd =3D LED_NEW_GET_STATUS; - input[0] =3D LED_NEW_GET_CONTROL_ITEM; - input[1] =3D led->id; - input[2] =3D led->indicator; - input[3] =3D 0; + if (led->indicator !=3D LED_IND_POWER_STATE) + return -ENODEV; =20 - ret =3D nuc_nmi_cmd(dev, cmd, input, output); - if (ret) + ret =3D nuc_wmi_get_brightness_offset(dev, led, offset); + if (ret < 0) return ret; =20 - /* Multicolor uses a scale from 0 to 100 */ - if (led->color_type & (LED_BLUE_AMBER | LED_BLUE_WHITE | LED_RGB)) - return scnprintf(buf, PAGE_SIZE, "%d%%\n", output[0]); - - /* single color uses 0, 50% and 100% */ - return scnprintf(buf, PAGE_SIZE, "%d%%\n", output[0] * 50); + return scnprintf(buf, PAGE_SIZE, "%d\n", ret); } =20 -/* Change S0 brightness */ -static ssize_t store_s0_brightness(struct device *dev, - struct device_attribute *attr, - const char *buf, size_t len) +/* Change brightness */ +static ssize_t store_brightness_offset(struct device *dev, + struct device_attribute *attr, + u8 offset, + const char *buf, size_t len) { struct led_classdev *cdev =3D dev_get_drvdata(dev); struct nuc_nmi_led *led =3D container_of(cdev, struct nuc_nmi_led, cdev); - u8 cmd, input[NUM_INPUT_ARGS] =3D { 0 }; int ret; u8 val; =20 - if (led->indicator =3D=3D LED_IND_DISABLE) { - dev_dbg(dev, "Led %s is disabled. ignoring it.\n", cdev->name); - return -EACCES; - } + if (led->indicator !=3D LED_IND_POWER_STATE) + return -ENODEV; =20 if (kstrtou8(buf, 0, &val) || val > 100) return -EINVAL; =20 - /* - * For single-color LEDs, the value should be between 0 to 2, but, - * in order to have a consistent API, let's always handle it as if - * it is a percentage, for both multicolor and single color LEDs. - * So: - * value =3D=3D 0 will disable the LED - * value up to 74% will set it the brightness to 50% - * value equal or above 75% will use the maximum brightness. - */ - if (!(led->color_type & (LED_BLUE_AMBER | LED_BLUE_WHITE | LED_RGB))) { - if (val > 0 && val < 75) - val =3D 1; - if (val >=3D 75) - val =3D 2; - } - - cmd =3D LED_SET_VALUE; - input[0] =3D led->id; - input[1] =3D led->indicator; - input[2] =3D 0; /* FIXME: replace by an enum */ - input[3] =3D val; - - ret =3D nuc_nmi_cmd(dev, cmd, input, NULL); + ret =3D nuc_wmi_set_brightness_offset(dev, led, offset, val); if (ret) return ret; =20 return len; } =20 +static enum led_brightness nuc_wmi_get_brightness(struct led_classdev *cde= v) +{ + struct nuc_nmi_led *led =3D container_of(cdev, struct nuc_nmi_led, cdev); + + if (led->indicator =3D=3D LED_IND_POWER_STATE) + return -ENODEV; + + return nuc_wmi_get_brightness_offset(cdev->dev, led, 0); +} + +static int nuc_wmi_set_brightness(struct led_classdev *cdev, + enum led_brightness brightness) +{ + struct nuc_nmi_led *led =3D container_of(cdev, struct nuc_nmi_led, cdev); + + if (led->indicator =3D=3D LED_IND_POWER_STATE) + return -ENODEV; + + return nuc_wmi_set_brightness_offset(cdev->dev, led, 0, brightness); +} + static LED_ATTR_RW(indicator); -static LED_ATTR_RW(s0_brightness); + +LED_ATTR_POWER_STATE_RW(s0_brightness, 0x00); +LED_ATTR_POWER_STATE_RW(s3_brightness, 0x06); +LED_ATTR_POWER_STATE_RW(s5_brightness, 0x0c); +LED_ATTR_POWER_STATE_RW(ready_mode_brightness, 0x12); =20 /* - * Attributes for multicolor LEDs + * Attributes for LEDs */ =20 -static struct attribute *nuc_wmi_multicolor_led_attr[] =3D { +static struct attribute *nuc_wmi_led_attr[] =3D { &dev_attr_indicator.attr, &dev_attr_s0_brightness.attr, + &dev_attr_s3_brightness.attr, + &dev_attr_s5_brightness.attr, + &dev_attr_ready_mode_brightness.attr, NULL, }; =20 static const struct attribute_group nuc_wmi_led_attribute_group =3D { - .attrs =3D nuc_wmi_multicolor_led_attr, + .attrs =3D nuc_wmi_led_attr, }; =20 static const struct attribute_group *nuc_wmi_led_attribute_groups[] =3D { @@ -496,30 +630,25 @@ static const struct attribute_group *nuc_wmi_led_attr= ibute_groups[] =3D { =20 static int nuc_wmi_led_register(struct device *dev, struct nuc_nmi_led *le= d) { + int brightness =3D nuc_wmi_get_brightness_offset(dev, led, 0); + led->cdev.name =3D led_names[led->id]; - led->dev =3D dev; led->cdev.groups =3D nuc_wmi_led_attribute_groups; + led->cdev.brightness_get =3D nuc_wmi_get_brightness; + led->cdev.brightness_set_blocking =3D nuc_wmi_set_brightness; =20 - /* - * It can't let the classdev to manage the brightness due to several - * reasons: - * - * 1) classdev has some internal logic to manage the brightness, - * at set_brightness_delayed(), which starts disabling the LEDs; - * While this makes sense on most cases, here, it would appear - * that the NUC was powered off, which is not what happens; - * 2) classdev unconditionally tries to set brightness for all - * leds, including the ones that were software-disabled or - * disabled disabled via BIOS menu; - * 3) There are 3 types of brightness values for each LED, depending - * on the CPU power state: S0, S3 and S5. - * - * So, the best seems to export everything via sysfs attributes - * directly. This would require some further changes at the - * LED class, though, or we would need to create our own LED - * class, which seems wrong. - */ + if (led->color_type & LED_SINGLE_COLOR) + led->cdev.max_brightness =3D 2; + else + led->cdev.max_brightness =3D 100; + + /* Ensure that the current bright will be preserved */ + if (brightness >=3D 0) + led->cdev.delayed_set_value =3D brightness; + + /* Suppress warnings for the LED(s) indicating the power state */ + led->cdev.flags =3D LED_HW_PLUGGABLE | LED_UNREGISTERING; =20 return devm_led_classdev_register(dev, &led->cdev); } --=20 2.31.1