From: Mauro Carvalho Chehab <firstname.lastname@example.org> To: Pavel Machek <email@example.com> Cc: firstname.lastname@example.org, email@example.com, firstname.lastname@example.org, Mauro Carvalho Chehab <email@example.com>, firstname.lastname@example.org, email@example.com, firstname.lastname@example.org, email@example.com Subject: Re: [PATCH 17/17] staging: nuc-led: update the TODOs Date: Mon, 17 May 2021 14:19:35 +0200 [thread overview] Message-ID: <firstname.lastname@example.org> (raw) In-Reply-To: <20210517080527.GA18642@amd> Em Mon, 17 May 2021 10:05:27 +0200 Pavel Machek <email@example.com> escreveu: > No. Take a look at triggers; for example hdd monitor should look very > much like existing disk trigger. Hmm... after looking at triggers, I'm not sure if this is the right interface, nor if we're talking about the same thing. See, at least the way ledtrig-disk.c uses it, two drivers are triggering the LED based on software events: drivers/ata/libata-core.c: ledtrig_disk_activity(!!(qc->tf.flags & ATA_TFLAG_WRITE)); drivers/ide/ide-disk.c: ledtrig_disk_activity(rq_data_dir(rq) == WRITE); This is not how the NUC LEDs are work. On NUC, the hardware itself triggers the event and/or blinks the LED(*). (*) By default, blink is enabled only when the device is suspended or it is hibernating. There's no need to do any software emulation. The API is meant to just program the hardware (and/or the firmware) for it to do the behavior expected by the user. So, for instance, setting the indicator event that will trigger the LED is done by sending a WMI message for this GUID object: 8C5DA44C-CDC3-46b3-8619-4E26D34390B7 (somewhat similar to using the way ACPI works, but WMI is a different firmware interface). The method at the WMI API which sets the LED indicator is this one: 0x05 message (Set an Indicator option for the LED type) Such method receives two parameters. The first one is the LED number, which can be: 0 - Power button LED 1 - HDD LED 2 - Skull LED 3 - Eyes LED 4 - Front LED 1 5 - Front LED 1 6 - Front LED 1 The second one tells which hardware event will trigger the LED: ===== ============== ======================================================= Value Indicator type Meaning ===== ============== ======================================================= 0 Power State Shows if the device is powered and what power level it is (e. g. if the device is suspended or not, and on which kind of suspended level). 1 HDD Activity Indicates if the LED is measuring the hard disk (or SDD) activity. 2 Ethernet Indicates the activity Ethernet adapter(s) 3 WiFi Indicates if WiFi is enabled 4 Software Doesn't indicate any hardware level. Instead, the LED status is controlled via software. 5 Power Limit Changes the LED color when the computer is throttling its power limits. 6 Disable The LED was disabled (either by BIOS or via WMI). ===== ============== ======================================================= There is an example at page 7 of the datasheet: https://raw.githubusercontent.com/nomego/intel_nuc_led/master/specs/INTEL_WMI_LED_0.64.pdf Where it shows what happens if the WMI message is filled with: <0x05> <0x00> <0x01> On such example, it changes the behavior of the button named "Power button" to change it to monitor the disk activity, instead of monitoring if the device is powered on or not. Such setting is even persistent after rebooting, and the event is hardware/firmware triggered. So, IMO, it would only makes sense to use something else from the LED class if are there a way for the sysfs nodes to dynamically be shown/hidden when the indicator changes. At least on my eyes, that doesn't seem to be what triggers do. Thanks, Mauro
next prev parent reply other threads:[~2021-05-17 12:19 UTC|newest] Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-05-16 10:53 [PATCH 00/17] Add an experimental driver for Intel NUC leds Mauro Carvalho Chehab 2021-05-16 10:53 ` [PATCH 01/17] staging: add support for NUC WMI LEDs Mauro Carvalho Chehab 2021-05-16 16:12 ` Randy Dunlap 2021-05-17 8:20 ` Greg KH 2021-05-16 10:53 ` [PATCH 02/17] staging: nuc-wmi: detect WMI API detection Mauro Carvalho Chehab 2021-05-17 9:35 ` Dan Carpenter 2021-05-16 10:53 ` [PATCH 03/17] staging: nuc-wmi: add support for changing S0 brightness Mauro Carvalho Chehab 2021-05-16 10:53 ` [PATCH 04/17] staging: nuc-wmi: add all types of brightness Mauro Carvalho Chehab 2021-05-16 10:53 ` [PATCH 05/17] staging: nuc-wmi: allow changing the LED colors Mauro Carvalho Chehab 2021-05-16 10:53 ` [PATCH 06/17] staging: nuc-wmi: add support for WMI API version 1.0 Mauro Carvalho Chehab 2021-05-16 10:53 ` [PATCH 07/17] staging: nuc-wmi: add basic support for NUC6 WMI Mauro Carvalho Chehab 2021-05-17 9:44 ` Dan Carpenter 2021-05-16 10:53 ` [PATCH 08/17] staging: muc-wmi: add brightness and color for NUC6 API Mauro Carvalho Chehab 2021-05-16 10:53 ` [PATCH 09/17] staging: nuc-wmi: Add support to blink behavior for NUC8/10 Mauro Carvalho Chehab 2021-05-16 10:53 ` [PATCH 10/17] staging: nuc-wmi: get rid of an unused variable Mauro Carvalho Chehab 2021-05-16 10:53 ` [PATCH 11/17] staging: nuc-wmi: implement blink control for NUC6 Mauro Carvalho Chehab 2021-05-16 10:53 ` [PATCH 12/17] staging: nuc-wmi: better detect NUC6/NUC7 devices Mauro Carvalho Chehab 2021-05-16 10:53 ` [PATCH 13/17] staging: nuc-led: add support for HDD activity default Mauro Carvalho Chehab 2021-05-16 10:53 ` [PATCH 14/17] staging: nuc-wmi: fix software blink behavior logic Mauro Carvalho Chehab 2021-05-16 10:53 ` [PATCH 15/17] staging: nuc-wmi: add support for changing the ethernet type indicator Mauro Carvalho Chehab 2021-05-16 10:53 ` [PATCH 16/17] staging: nuc-wmi: add support for changing the power limit scheme Mauro Carvalho Chehab 2021-05-16 10:53 ` [PATCH 17/17] staging: nuc-led: update the TODOs Mauro Carvalho Chehab 2021-05-16 18:21 ` Pavel Machek 2021-05-17 6:30 ` Mauro Carvalho Chehab 2021-05-17 8:05 ` Pavel Machek 2021-05-17 8:57 ` Mauro Carvalho Chehab 2021-05-17 9:12 ` Mauro Carvalho Chehab 2021-05-17 12:19 ` Mauro Carvalho Chehab [this message] 2021-05-17 8:18 ` [PATCH 00/17] Add an experimental driver for Intel NUC leds Greg KH 2021-05-17 9:02 ` Mauro Carvalho Chehab 2021-05-17 9:08 ` Greg KH
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --subject='Re: [PATCH 17/17] staging: nuc-led: update the TODOs' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).