You can subscribe to this list here.
| 2006 |
Jan
|
Feb
|
Mar
|
Apr
|
May
|
Jun
|
Jul
|
Aug
|
Sep
|
Oct
|
Nov
(74) |
Dec
(26) |
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| 2007 |
Jan
(14) |
Feb
(70) |
Mar
(116) |
Apr
(64) |
May
(118) |
Jun
(39) |
Jul
(243) |
Aug
(68) |
Sep
(77) |
Oct
(136) |
Nov
(48) |
Dec
(38) |
| 2008 |
Jan
(100) |
Feb
(84) |
Mar
(21) |
Apr
(56) |
May
(24) |
Jun
(19) |
Jul
(47) |
Aug
(27) |
Sep
(20) |
Oct
(65) |
Nov
(54) |
Dec
(14) |
| 2009 |
Jan
(45) |
Feb
(12) |
Mar
(15) |
Apr
(42) |
May
(21) |
Jun
(41) |
Jul
(15) |
Aug
(25) |
Sep
(49) |
Oct
(20) |
Nov
(61) |
Dec
(139) |
| 2010 |
Jan
(42) |
Feb
(56) |
Mar
(30) |
Apr
(27) |
May
(40) |
Jun
(16) |
Jul
(28) |
Aug
(14) |
Sep
(16) |
Oct
(11) |
Nov
(9) |
Dec
(10) |
| 2011 |
Jan
(51) |
Feb
(15) |
Mar
(29) |
Apr
(17) |
May
(70) |
Jun
(37) |
Jul
(6) |
Aug
(20) |
Sep
(7) |
Oct
(18) |
Nov
(28) |
Dec
(9) |
| 2012 |
Jan
(11) |
Feb
(12) |
Mar
(25) |
Apr
(8) |
May
(11) |
Jun
(27) |
Jul
(25) |
Aug
(22) |
Sep
(7) |
Oct
(6) |
Nov
(8) |
Dec
(29) |
| 2013 |
Jan
(11) |
Feb
(16) |
Mar
(41) |
Apr
(5) |
May
(4) |
Jun
(27) |
Jul
(10) |
Aug
(21) |
Sep
(13) |
Oct
(16) |
Nov
(31) |
Dec
(13) |
| 2014 |
Jan
(2) |
Feb
(37) |
Mar
(39) |
Apr
(62) |
May
(13) |
Jun
(6) |
Jul
(3) |
Aug
(4) |
Sep
(15) |
Oct
(13) |
Nov
(45) |
Dec
(2) |
| 2015 |
Jan
(7) |
Feb
(85) |
Mar
(66) |
Apr
(14) |
May
(24) |
Jun
(127) |
Jul
(7) |
Aug
(9) |
Sep
|
Oct
(11) |
Nov
(2) |
Dec
(12) |
| 2016 |
Jan
(71) |
Feb
(17) |
Mar
(8) |
Apr
(26) |
May
(8) |
Jun
(12) |
Jul
|
Aug
(2) |
Sep
(11) |
Oct
(25) |
Nov
(139) |
Dec
(7) |
| 2017 |
Jan
(12) |
Feb
(12) |
Mar
(4) |
Apr
(8) |
May
(37) |
Jun
(12) |
Jul
(5) |
Aug
(5) |
Sep
(18) |
Oct
(21) |
Nov
(10) |
Dec
(91) |
| 2018 |
Jan
(35) |
Feb
(31) |
Mar
(1) |
Apr
(33) |
May
(15) |
Jun
(50) |
Jul
(10) |
Aug
(14) |
Sep
(5) |
Oct
(8) |
Nov
(53) |
Dec
(9) |
| 2019 |
Jan
|
Feb
|
Mar
(7) |
Apr
(14) |
May
(14) |
Jun
(5) |
Jul
(5) |
Aug
|
Sep
(2) |
Oct
(2) |
Nov
|
Dec
(2) |
| 2020 |
Jan
(9) |
Feb
(8) |
Mar
|
Apr
(11) |
May
(14) |
Jun
(35) |
Jul
(53) |
Aug
(23) |
Sep
(15) |
Oct
(6) |
Nov
(14) |
Dec
(8) |
| 2021 |
Jan
(9) |
Feb
(6) |
Mar
(14) |
Apr
(4) |
May
(3) |
Jun
(1) |
Jul
(7) |
Aug
(2) |
Sep
(10) |
Oct
(4) |
Nov
(26) |
Dec
(9) |
| 2022 |
Jan
(13) |
Feb
(1) |
Mar
(1) |
Apr
(7) |
May
(7) |
Jun
(17) |
Jul
(6) |
Aug
(9) |
Sep
|
Oct
(8) |
Nov
(6) |
Dec
|
| 2023 |
Jan
(1) |
Feb
|
Mar
|
Apr
(8) |
May
|
Jun
|
Jul
(4) |
Aug
(1) |
Sep
(13) |
Oct
(1) |
Nov
(9) |
Dec
(4) |
| 2024 |
Jan
(9) |
Feb
|
Mar
(5) |
Apr
(118) |
May
(4) |
Jun
(13) |
Jul
|
Aug
(4) |
Sep
(4) |
Oct
(16) |
Nov
(20) |
Dec
(3) |
| 2025 |
Jan
(7) |
Feb
(1) |
Mar
(3) |
Apr
(8) |
May
(15) |
Jun
(38) |
Jul
(2) |
Aug
(7) |
Sep
|
Oct
|
Nov
|
Dec
|
|
From: Ognjen G. <smc...@gm...> - 2018-06-10 07:26:08
|
On Thu, May 24, 2018 at 08:35:55AM -0300, Henrique de Moraes Holschuh wrote: > On Mon, 21 May 2018, Kevin Locke wrote: > > as-expected. The only oddity I noticed is that force_discharge has a > > delay taking effect (<1 sec) transitioning from 0 to 1 (but not 1 to > > 0). > > The EC can take its sweet time to obey any such requests ;-) > What is the status on this patch? Do I need some revisional work or? > -- > Henrique Holschuh |
|
From: Henrique de M. H. <hm...@hm...> - 2018-06-09 03:04:08
|
On Fri, 08 Jun 2018, Pali Rohár wrote: > which has only 4.9 kernel and LEN0268 identifier was added in git commit > a3c42a467a254a17236ab817d5c7c6bc054e4f84 for 4.10. You could always request it to be backported to -stable *if* you manually apply it to a 4.9 kernel and test the heck out of it... Debian would pick it up from -stable after a while. -- Henrique Holschuh |
|
From: Pali R. <pal...@gm...> - 2018-06-08 11:11:06
|
Hi! With up-to-date thinkpad_acpi.ko driver on ThinkPad T480s I'm seeing a strange behavior of LEDs which are integrated into mic mute (Fn+F4) and mute (Fn+F1) keys. When thinkpad_acpi.ko is not loaded, then mute key is working fine. When pressed, it correctly generates KEY_MUTE on AT Translated Set 2 keyboard input device and also turn on/of mute led. But when micmute key is pressed then, nothing happen. No key event is reported and also led is not turned on/off. On the other hand, when thinkpad_acpi.ko is loaded, then both buttons mute and micmute correctly generates input events; mute via AT keyboard and micmute via ThinkPad Extra Buttons. But led is not changed. When thinkpad_acpi.ko is loaded it turn off both leds (mute and micmute) and leds after pressing any of those buttons, leds are not turned on again. When thinkpad_acpi.ko is unloaded, then pressing mute button again start switching led on/off. So it seems that some init sequence of thinkpad_acpi.ko breaks mute led. And fini sequence of thinkpad_acpi.ko makes mute led working again. -- Pali Rohár pal...@gm... |
|
From: Pali R. <pal...@gm...> - 2018-06-08 10:29:20
|
On Friday 08 June 2018 12:40:32 Andy Shevchenko wrote: > On Fri, Jun 8, 2018 at 11:12 AM, Pali Rohár <pal...@gm...> wrote: > > Hi! I just found out that thinkpad_acpi.ko is not automatically loaded > > on new ThinkPad T480s. Is there any reason for it? Manually calling > > modprobe thinkpad_acpi is working fine, but I would expect that it > > should be loaded automatically. > > > > Here is dmesg output after loading it manually: > > > > thinkpad_acpi: ThinkPad ACPI Extras v0.25 > > thinkpad_acpi: http://ibm-acpi.sf.net/ > > thinkpad_acpi: ThinkPad BIOS N22ET34W (1.11 ), EC unknown > > thinkpad_acpi: Lenovo ThinkPad T480s, model 20L7S03000 > > thinkpad_acpi: radio switch found; radios are enabled > > thinkpad_acpi: This ThinkPad has standard ACPI backlight brightness control, supported by the ACPI video driver > > thinkpad_acpi: Disabling thinkpad-acpi brightness events by default... > > thinkpad_acpi: rfkill switch tpacpi_bluetooth_sw: radio is unblocked > > thinkpad_acpi: Standard ACPI backlight interface available, not loading native one > > Did you run depmod -a? > Is it present in the output? > > There are three ways of enumeration as far as I can see from the driver's code: > - by short driver name "tpacpi" (not likely our case) > - by DMI matching (only for old BIOS'es) > - by ACPI ID > > So, does the ACPI contain one of the listed IDs? > > #define TPACPI_ACPI_IBM_HKEY_HID "IBM0068" > #define TPACPI_ACPI_LENOVO_HKEY_HID "LEN0068" > #define TPACPI_ACPI_LENOVO_HKEY_V2_HID "LEN0268" > ? DSDT contains only LEN0268. And so I found reason. I'm using Debian which has only 4.9 kernel and LEN0268 identifier was added in git commit a3c42a467a254a17236ab817d5c7c6bc054e4f84 for 4.10. -- Pali Rohár pal...@gm... |
|
From: Andy S. <and...@gm...> - 2018-06-08 09:40:40
|
On Fri, Jun 8, 2018 at 11:12 AM, Pali Rohár <pal...@gm...> wrote: > Hi! I just found out that thinkpad_acpi.ko is not automatically loaded > on new ThinkPad T480s. Is there any reason for it? Manually calling > modprobe thinkpad_acpi is working fine, but I would expect that it > should be loaded automatically. > > Here is dmesg output after loading it manually: > > thinkpad_acpi: ThinkPad ACPI Extras v0.25 > thinkpad_acpi: http://ibm-acpi.sf.net/ > thinkpad_acpi: ThinkPad BIOS N22ET34W (1.11 ), EC unknown > thinkpad_acpi: Lenovo ThinkPad T480s, model 20L7S03000 > thinkpad_acpi: radio switch found; radios are enabled > thinkpad_acpi: This ThinkPad has standard ACPI backlight brightness control, supported by the ACPI video driver > thinkpad_acpi: Disabling thinkpad-acpi brightness events by default... > thinkpad_acpi: rfkill switch tpacpi_bluetooth_sw: radio is unblocked > thinkpad_acpi: Standard ACPI backlight interface available, not loading native one Did you run depmod -a? Is it present in the output? There are three ways of enumeration as far as I can see from the driver's code: - by short driver name "tpacpi" (not likely our case) - by DMI matching (only for old BIOS'es) - by ACPI ID So, does the ACPI contain one of the listed IDs? #define TPACPI_ACPI_IBM_HKEY_HID "IBM0068" #define TPACPI_ACPI_LENOVO_HKEY_HID "LEN0068" #define TPACPI_ACPI_LENOVO_HKEY_V2_HID "LEN0268" ? -- With Best Regards, Andy Shevchenko |
|
From: Pali R. <pal...@gm...> - 2018-06-08 08:12:14
|
Hi! I just found out that thinkpad_acpi.ko is not automatically loaded on new ThinkPad T480s. Is there any reason for it? Manually calling modprobe thinkpad_acpi is working fine, but I would expect that it should be loaded automatically. Here is dmesg output after loading it manually: thinkpad_acpi: ThinkPad ACPI Extras v0.25 thinkpad_acpi: http://ibm-acpi.sf.net/ thinkpad_acpi: ThinkPad BIOS N22ET34W (1.11 ), EC unknown thinkpad_acpi: Lenovo ThinkPad T480s, model 20L7S03000 thinkpad_acpi: radio switch found; radios are enabled thinkpad_acpi: This ThinkPad has standard ACPI backlight brightness control, supported by the ACPI video driver thinkpad_acpi: Disabling thinkpad-acpi brightness events by default... thinkpad_acpi: rfkill switch tpacpi_bluetooth_sw: radio is unblocked thinkpad_acpi: Standard ACPI backlight interface available, not loading native one -- Pali Rohár pal...@gm... |
|
From: Alex G. <gs...@gm...> - 2018-06-04 05:28:12
|
Repro instructions: - Lenovo Carbon X1, 6th gen - OS: Linux newgro 4.15.0-22-generic #24-Ubuntu SMP Wed May 16 12:15:17 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux - The message appears in dmesg when I close and re-open the laptop lid (i.e., apparently when it tries to enter sleep mode). [ 46.803995] acpi INT3400:00: Unsupported event [0x86] [ 46.808977] thinkpad_acpi: unknown possible thermal alarm or keyboard event received [ 46.808978] thinkpad_acpi: unhandled HKEY event 0x6032 [ 46.808978] thinkpad_acpi: please report the conditions when this event happened to ibm...@li... |
|
From: Adam R. <ad...@ra...> - 2018-06-02 21:59:03
|
Just found this in my dmesg, it asked me to report so hre it is. Hope i can help. this is everything before and after: [56811.353116] thinkpad_acpi: unknown possible thermal alarm or keyboard event received [56811.353122] thinkpad_acpi: unhandled HKEY event 0x6032 [56811.353125] thinkpad_acpi: please report the conditions when this event happened to ibm...@li... [56819.716530] PM: suspend entry (deep) [56819.716532] PM: Syncing filesystems ... done. [56819.741278] Freezing user space processes ... (elapsed 0.002 seconds) done. [56819.743405] OOM killer disabled. [56819.743405] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done. [56819.744782] Suspending console(s) (use no_console_suspend to debug) [56819.983197] psmouse serio2: Failed to disable mouse on synaptics-pt/serio0 [56820.124484] e1000e: EEE TX LPI TIMER: 00000011 [56820.487561] ACPI: EC: interrupt blocked [56820.537895] ACPI: Preparing to enter system sleep state S3 [56820.546149] ACPI: EC: event blocked [56820.546150] ACPI: EC: EC stopped [56820.546152] PM: Saving platform NVS memory [56820.546159] Disabling non-boot CPUs ... [56820.592517] smpboot: CPU 1 is now offline [56820.644704] smpboot: CPU 2 is now offline [56820.674611] smpboot: CPU 3 is now offline [56820.687624] ACPI: Low-level resume complete [56820.687903] ACPI: EC: EC started [56820.687906] PM: Restoring platform NVS memory [56820.689243] Enabling non-boot CPUs ... [56820.689368] x86: Booting SMP configuration: [56820.689373] smpboot: Booting Node 0 Processor 1 APIC 0x2 [56820.692447] cache: parent cpu1 should not be sleeping [56820.693291] CPU1 is up [56820.693371] smpboot: Booting Node 0 Processor 2 APIC 0x1 [56820.696787] cache: parent cpu2 should not be sleeping [56820.697733] CPU2 is up [56820.697843] smpboot: Booting Node 0 Processor 3 APIC 0x3 [56820.700820] cache: parent cpu3 should not be sleeping [56820.701820] CPU3 is up [56820.724729] ACPI: Waking up from system sleep state S3 [56820.789921] thinkpad_acpi: unknown possible thermal alarm or keyboard event received [56820.789931] thinkpad_acpi: unhandled HKEY event 0x6032 [56820.789937] thinkpad_acpi: please report the conditions when this event happened to ibm...@li... [56820.794622] ACPI: EC: interrupt unblocked [56820.845653] ACPI: EC: event unblocked [56820.853860] usb 2-3: Disable of device-initiated U1 failed. [56820.857332] usb 2-3: Disable of device-initiated U2 failed. [56820.860828] usb 2-3: usb_reset_and_verify_device Failed to disable LTM [56821.085766] usb 1-7: reset full-speed USB device number 5 using xhci_hcd [56821.349276] usb 1-5: reset full-speed USB device number 3 using xhci_hcd [56821.588174] psmouse serio1: synaptics: queried max coordinates: x [..5676], y [..4762] [56821.612567] usb 1-6: reset high-speed USB device number 13 using xhci_hcd [56821.620978] psmouse serio1: synaptics: queried min coordinates: x [1266..], y [1094..] [56821.752595] usb 1-6: device firmware changed [56821.875742] usb 1-8: reset high-speed USB device number 6 using xhci_hcd [56822.022883] restoring control 00000000-0000-0000-0000-000000000101/10/5 [56822.022887] restoring control 00000000-0000-0000-0000-000000000101/12/11 [56822.139341] usb 1-9: reset full-speed USB device number 7 using xhci_hcd [56822.281751] acpi LNXPOWER:02: Turning OFF [56822.281854] acpi LNXPOWER:01: Turning OFF [56822.282303] OOM killer enabled. [56822.282306] Restarting tasks ... [56822.282688] Bluetooth: hci0: Bootloader revision 0.0 build 26 week 38 2015 [56822.282719] usb 1-6: USB disconnect, device number 13 [56822.282802] cdc_mbim 1-6:1.12 wwan0: unregister 'cdc_mbim' usb-0000:00:14.0-6, CDC MBIM [56822.283985] Bluetooth: hci0: Device revision is 16 [56822.283989] Bluetooth: hci0: Secure boot is enabled [56822.283992] Bluetooth: hci0: OTP lock is enabled [56822.283994] Bluetooth: hci0: API lock is enabled [56822.283997] Bluetooth: hci0: Debug lock is disabled [56822.284001] Bluetooth: hci0: Minimum firmware build 1 week 10 2014 [56822.284009] Bluetooth: hci0: Found device firmware: intel/ibt-12-16.sfi [56822.348775] done. [56822.350585] thermal thermal_zone3: failed to read out thermal zone (-61) [56822.391582] PM: suspend exit [56822.455348] usb 2-3: USB disconnect, device number 5 [56822.462008] usb 1-6: new high-speed USB device number 14 using xhci_hcd [56822.583502] e1000e: eth0 NIC Link is Down [56822.595417] IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready [56822.602500] usb 1-6: config 1 interface 0 altsetting 0 bulk endpoint 0x1 has invalid maxpacket 64 [56822.602504] usb 1-6: config 1 interface 0 altsetting 0 bulk endpoint 0x81 has invalid maxpacket 64 [56822.603198] usb 1-6: New USB device found, idVendor=1199, idProduct=9078 [56822.603202] usb 1-6: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [56822.603204] usb 1-6: Product: Sierra Wireless EM7455 Qualcomm [56822.603206] usb 1-6: Manufacturer: Sierra Wireless, Incorporated [56822.603208] usb 1-6: SerialNumber: LF71560711021020 [56822.635378] usb 1-6: USB disconnect, device number 14 [56822.728806] usb 2-3: new SuperSpeed USB device number 6 using xhci_hcd [56822.756261] usb 2-3: New USB device found, idVendor=0bda, idProduct=0316 [56822.756266] usb 2-3: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [56822.756269] usb 2-3: Product: USB3.0-CRW [56822.756271] usb 2-3: Manufacturer: Generic [56822.756273] usb 2-3: SerialNumber: 20120501030900000 [56822.761231] usb-storage 2-3:1.0: USB Mass Storage device detected [56822.772022] scsi host1: usb-storage 2-3:1.0 [56822.836727] IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready [56822.838248] IPv6: ADDRCONF(NETDEV_UP): wlan0: link is not ready [56823.091956] IPv6: ADDRCONF(NETDEV_UP): wlan0: link is not ready [56823.367414] IPv6: ADDRCONF(NETDEV_UP): wlan0: link is not ready [56823.492438] IPv6: ADDRCONF(NETDEV_UP): wlan0: link is not ready [56823.716415] Bluetooth: hci0: Waiting for firmware download to complete [56823.716543] Bluetooth: hci0: Firmware loaded in 1400662 usecs [56823.716563] Bluetooth: hci0: Waiting for device to boot [56823.728593] Bluetooth: hci0: Device booted in 11756 usecs [56823.728607] Bluetooth: hci0: Found Intel DDC parameters: intel/ibt-12-16.ddc [56823.731572] Bluetooth: hci0: Applying Intel DDC parameters completed [56823.800528] scsi 1:0:0:0: Direct-Access Generic- SD/MMC 1.00 PQ: 0 ANSI: 6 [56824.774593] sd 1:0:0:0: [sda] 124735488 512-byte logical blocks: (63.9 GB/59.5 GiB) [56824.774826] sd 1:0:0:0: [sda] Write Protect is off [56824.774829] sd 1:0:0:0: [sda] Mode Sense: 2f 00 00 00 [56824.775055] sd 1:0:0:0: [sda] Write cache: disabled, read cache: enabled, doesn't support DPO or FUA [56824.778095] sda: sda1 Best regards, Adam Rabjerg |
|
From: Gianni C. <gia...@gm...> - 2018-06-02 08:49:02
|
executing a dmesg I have found the following after a resume [ 574.499328] ACPI: Waking up from system sleep state S3 [ 574.773704] thinkpad_acpi: unknown possible thermal alarm or keyboard event received [ 574.773764] thinkpad_acpi: temperatures (Celsius): 0 48 0 0 0 0 33 29 [ 574.773771] thinkpad_acpi: unhandled HKEY event 0x6032 [ 574.773772] thinkpad_acpi: please report the conditions when this event happened to ibm...@li... [ 574.842431] xhci_hcd 0000:00:14.0: System wakeup disabled by ACPI [ 574.842553] PM: noirq resume of devices complete after 56.801 msecs [ 574.846309] PM: early resume of devices complete after 3.669 msecs [ 574.846431] ACPI : EC: event unblocked [ 574.847352] iwlwifi 0000:02:00.0: reporting RF_KILL (radio enabled) [ 574.847678] iwlwifi 0000:02:00.0: RF_KILL bit toggled to enable radio. [ 574.848343] ACPI : button: The lid device is not compliant to SW_LID. [ 574.848384] rtc_cmos 00:03: System wakeup disabled by ACPI [ 575.081915] usb 1-7: reset full-speed USB device number 4 using xhci_hcd [ 575.357981] usb 1-5: reset high-speed USB device number 3 using xhci_hcd [ 575.514178] PM: resume of devices complete after 667.861 msecs [ 575.515801] PM: Finishing wakeup. the hardware is a lenovo L380 dual boot with windows 10 and OpenSuse LEAP 15 wifi is working only after a resume or if the system is hibernate After normal boot the wifi antenna is off best regards Gianni -- Gianni Cardini |
|
From: Henrique de M. H. <hm...@hm...> - 2018-05-24 11:36:21
|
On Mon, 21 May 2018, Kevin Locke wrote: > as-expected. The only oddity I noticed is that force_discharge has a > delay taking effect (<1 sec) transitioning from 0 to 1 (but not 1 to > 0). The EC can take its sweet time to obey any such requests ;-) -- Henrique Holschuh |
|
From: Matthias P. <mat...@et...> - 2018-05-22 14:42:50
|
Hello, I've had this message in my dmesg logs when hitting a key on my laptop : [529401.717204] thinkpad_acpi: unknown possible thermal alarm or keyboard event received [529401.717209] thinkpad_acpi: unhandled HKEY event 0x6020 [529401.717211] thinkpad_acpi: please report the conditions when this event happened to ibm...@li... I use a thinkpad yoga 15". The key in question is the lock key on the right side of the computer. Hope this is helpful in some way. -- Matthias Paulmier |
|
From: Kevin L. <ke...@ke...> - 2018-05-21 13:50:23
|
On Sun, 2018-05-13 at 17:30 +0200, Ognjen Galic wrote: > Lenovo ThinkPad systems have a feature that lets you > force the battery to discharge regardless if AC is attached > or not. > > This patch implements that feature and exposes it via the generic > ACPI battery driver. On a T430 (2342-CTO) I can confirm that both force_discharge and inhibit_charge behave as expected, both when the battery is above and below charge_start_threshold. Input validation also works as-expected. The only oddity I noticed is that force_discharge has a delay taking effect (<1 sec) transitioning from 0 to 1 (but not 1 to 0). Tested-by: Kevin Locke <ke...@ke...> Thanks for working on this! Kevin |
|
From: Henrique de M. H. <hm...@hm...> - 2018-05-14 15:41:15
|
On Mon, 14 May 2018, Ognjen Galić wrote: > On Mon, May 14, 2018 at 08:39:28AM -0300, Henrique de Moraes Holschuh wrote: > > On Mon, 14 May 2018, Christoph Böhmwalder wrote: > > > > + case INHIBIT_CHARGE: > > > > + if ACPI_FAILURE(tpacpi_battery_acpi_eval(GET_INHIBIT, ret, battery)) > > > > + return -ENODEV; > > > > + > > > > + /* The inhibit charge status is in the first bit */ > > > > + *ret = *ret & 0x01; > > > > + return 0; > > > > Do we know what is in the other bits? If so, please document the ACPI > > method using a comment somewhere in the driver code, like you did for > > SET_INHIBIT. > > I got the specs for the methods in a Lenovo doc I was told not to share > around, including information in it. Hmm, ok. Please send to me in private the email address of the person I should ask for the documentation since you cannot distribute the documentation itself. This isn't the first time it happened. The usual rule I follow is: document what is either already being used in the driver or which very likely wil have to be used in the driver. Unrelated stuff (e.g. for some unrelated functionality the driver does not implement) doesn't have to be documented. In this case, we need to at least know what bits need to be RMW, or always-zero when calling SET_* methods... > So what do you people want me to do? Should I fix the comments or leave > as-is? Since the subsystem maintainer said it should be kept as-is, and I don't much care, please keep it as-is. It won't matter for my driver maintainer ack :-) > > > > + case INHIBIT_CHARGE: > > > > + if (!battery_info.batteries[battery].inhibit_support) > > > > + return -ENODEV; > > > > + /* The only valid values are 1 and 0 */ > > > > + if (value != 0 && value != 1) > > > > > > I'm not sure, but maybe `if (value < 2)` is better here? > > > > Indeed... with a comment that says 0 = main battery, 1 = extra/dock > > battery or something. > > That seems like obfuscation to me, this way its clear that it must be > either 1 or 0. And inhibiting is set per-battery so 1 is on and not > battery 1. Hmm, yes I got that wrong. "battery" should use 0 <= battery <2 for checking (if needed), but value can only be 0 or 1. I'd personally have used a bitmask test for "value", ensuring all other bits are zero, though. But that's just a matter of taste and I don't think any of the choices (including the one you used) are "bad taste", so all of them are fine as far as I am concerned. -- Henrique Holschuh |
|
From: Ognjen G. <smc...@gm...> - 2018-05-14 13:41:29
|
On Mon, May 14, 2018 at 09:03:02AM -0300, Henrique de Moraes Holschuh wrote: > On Sun, 13 May 2018, Ognjen Galic wrote: > > +Battery force discharging > > +-------------------------- > > + > > +There is also support for AC overriding. This means that you can force the battery to discharge > > +even when AC is attached. This is also exposed via the generic ACPI driver: > > + > > +/sys/class/power_supply/BATx/force_discharge [int, 1, 0] > > + > > +Setting it to 1 forces the battery to discharge on AC. > > This is used to run a battery "fuel gauge" callibration cycle. > > One has to set the limits to 0,100 (i.e. disable the anti-wear > limiters), trigger a force_discharge, and wait until it discharges to > zero. At that point, the EC is supposed to reset the force_discharge > bit by itself, and then you have to wait the battery to charge back to > full. That's the intended use case for this patches via an upcoming program I am writing for userspace. > > (and at least on older thinkpads, where you had to use SMAPI to do the > above, it didn't need much help from the operating system. Once a > couple years you could just set force_discharge on both batteries (with > the thing connected to AC), shutdown the operating system, and go to > sleep. By morning, it would be fully charged and both gauges > calibrated, still powered off, ready to go :P ). > > -- > Henrique Holschuh |
|
From: Ognjen G. <smc...@gm...> - 2018-05-14 13:40:53
|
On Mon, May 14, 2018 at 08:39:28AM -0300, Henrique de Moraes Holschuh wrote:
> On Mon, 14 May 2018, Christoph Böhmwalder wrote:
> > > + case INHIBIT_CHARGE:
> > > + if ACPI_FAILURE(tpacpi_battery_acpi_eval(GET_INHIBIT, ret, battery))
> > > + return -ENODEV;
> > > +
> > > + /* The inhibit charge status is in the first bit */
> > > + *ret = *ret & 0x01;
> > > + return 0;
>
> Do we know what is in the other bits? If so, please document the ACPI
> method using a comment somewhere in the driver code, like you did for
> SET_INHIBIT.
I got the specs for the methods in a Lenovo doc I was told not to share
around, including information in it.
>
> > > default:
> > > pr_crit("wrong parameter: %d", what);
> > > return -EINVAL;
> > > @@ -9343,6 +9357,21 @@ static int tpacpi_battery_set(int what, int battery, int value)
> > > return -ENODEV;
> > > }
> > > return 0;
> > > + case INHIBIT_CHARGE:
> > > + /* When setting inhbitit charge, we set a default vaulue of
> >
> > This comment does not adhere to the Linux coding style
>
> Much on the driver doesn't, because it is _OLD_. But yeah, it is
> preferrable to fix this as we add code, so it would be good to have all
> new (and modified) comments switched to modern kernel style.
So what do you people want me to do? Should I fix the comments or leave
as-is?
>
> > > + case INHIBIT_CHARGE:
> > > + if (!battery_info.batteries[battery].inhibit_support)
> > > + return -ENODEV;
> > > + /* The only valid values are 1 and 0 */
> > > + if (value != 0 && value != 1)
> >
> > I'm not sure, but maybe `if (value < 2)` is better here?
>
> Indeed... with a comment that says 0 = main battery, 1 = extra/dock
> battery or something.
That seems like obfuscation to me, this way its clear that it must be
either 1 or 0. And inhibiting is set per-battery so 1 is on and not
battery 1.
>
> --
> Henrique Holschuh
|
|
From: Ognjen G. <smc...@gm...> - 2018-05-14 13:38:55
|
On Mon, May 14, 2018 at 01:41:33PM +0300, Andy Shevchenko wrote: > On Mon, May 14, 2018 at 12:46 PM, Christoph Böhmwalder > <chr...@bo...> wrote: > > On Sun, May 13, 2018 at 05:29:37PM +0200, Ognjen Galic wrote: > >> Lenovo ThinkPad systems support the prevention of > >> battery charging via a manual override called Inhibit Charge. > >> > >> This patch adds support for that attribute and exposes it via the > >> battery ACPI driver in the generic location: > >> > >> /sys/class/power_supply/BATX/inhibit_charge > > >> + /* When setting inhbitit charge, we set a default vaulue of > > > > This comment does not adhere to the Linux coding style > > While you are right in principle, the whole driver is so old and uses > this style. So, for such cases we, as maintainers, prefer less > deviation work, i.e. keeping the > style is a good thing to do. That's what I did, follow the driver style. > > >> + /* The only valid values are 1 and 0 */ > >> + if (value != 0 && value != 1) > > > > I'm not sure, but maybe `if (value < 2)` is better here? > > Since it's about integer-as-a-boolean, test for bit 0 would be > sufficient, i.e. ~BIT(0). That seems uncessarily complicated. Whats wrong with LT and GT operators? > Though, I find this form not so readable > since the input comes actually from the user. I agree. > It would be nice to have just kstrtobool() called instead for such > options, but see above. It would need a (huge) refactoring of the > driver first. That needs a whole lot of refactoring for no real functional benefit. > > -- > With Best Regards, > Andy Shevchenko |
|
From: Ognjen G. <smc...@gm...> - 2018-05-14 13:37:04
|
On Mon, May 14, 2018 at 11:46:20AM +0200, Christoph Böhmwalder wrote:
> On Sun, May 13, 2018 at 05:29:37PM +0200, Ognjen Galic wrote:
> > Lenovo ThinkPad systems support the prevention of
> > battery charging via a manual override called Inhibit Charge.
> >
> > This patch adds support for that attribute and exposes it via the
> > battery ACPI driver in the generic location:
> >
> > /sys/class/power_supply/BATX/inhibit_charge
> >
> > Signed-off-by: Ognjen Galic <smc...@gm...>
> > ---
> > drivers/platform/x86/thinkpad_acpi.c | 66 +++++++++++++++++++++++++++-
> > 1 file changed, 64 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> > index da1ca485..b8b74889 100644
> > --- a/drivers/platform/x86/thinkpad_acpi.c
> > +++ b/drivers/platform/x86/thinkpad_acpi.c
> > @@ -9233,6 +9233,11 @@ static struct ibm_struct mute_led_driver_data = {
> > #define GET_STOP "BCSG"
> > #define SET_STOP "BCSS"
> >
> > +#define SET_INHIBIT "BICS"
> > +#define GET_INHIBIT "BICG"
> > +
> > +#define INHIBIT_ATTR "inhibit_charge"
> > +
> > #define START_ATTR "charge_start_threshold"
> > #define STOP_ATTR "charge_stop_threshold"
> >
> > @@ -9251,6 +9256,7 @@ enum {
> > /* This is used in the get/set helpers */
> > THRESHOLD_START,
> > THRESHOLD_STOP,
> > + INHIBIT_CHARGE
> > };
> >
> > struct tpacpi_battery_data {
> > @@ -9258,6 +9264,7 @@ struct tpacpi_battery_data {
> > int start_support;
> > int charge_stop;
> > int stop_support;
> > + int inhibit_support;
> > };
> >
> > struct tpacpi_battery_driver_data {
> > @@ -9315,6 +9322,13 @@ static int tpacpi_battery_get(int what, int battery, int *ret)
> > if (*ret == 0)
> > *ret = 100;
> > return 0;
> > + case INHIBIT_CHARGE:
> > + if ACPI_FAILURE(tpacpi_battery_acpi_eval(GET_INHIBIT, ret, battery))
> > + return -ENODEV;
> > +
> > + /* The inhibit charge status is in the first bit */
> > + *ret = *ret & 0x01;
> > + return 0;
> > default:
> > pr_crit("wrong parameter: %d", what);
> > return -EINVAL;
> > @@ -9343,6 +9357,21 @@ static int tpacpi_battery_set(int what, int battery, int value)
> > return -ENODEV;
> > }
> > return 0;
> > + case INHIBIT_CHARGE:
> > + /* When setting inhbitit charge, we set a default vaulue of
>
> This comment does not adhere to the Linux coding style
I followed the style in the driver.
>
> > + * always breaking on AC detach and the effective time is set to
> > + * be permanent.
> > + * The battery ID is in bits 4-5, 2 bits and the effective time
> > + * is in bits 8-23, 2 bytes. A time of FFFF indicates forever.
> > + */
> > + param = value;
> > + param |= battery << 4;
> > + param |= 0xFFFF << 8;
> > + if ACPI_FAILURE(tpacpi_battery_acpi_eval(SET_INHIBIT, &ret, param)) {
> > + pr_err("failed to set inhibit charge on %d", battery);
> > + return -ENODEV;
> > + }
> > + return 0;
> > default:
> > pr_crit("wrong parameter: %d", what);
> > return -EINVAL;
> > @@ -9359,6 +9388,8 @@ static int tpacpi_battery_probe(int battery)
> > * 2) Check for support
> > * 3) Get the current stop threshold
> > * 4) Check for support
> > + * 5) Check for inhibit charge support
> > + * 6) Get the current inhibit charge status
> > */
> > if (acpi_has_method(hkey_handle, GET_START)) {
> > if ACPI_FAILURE(tpacpi_battery_acpi_eval(GET_START, &ret, battery)) {
> > @@ -9395,10 +9426,16 @@ static int tpacpi_battery_probe(int battery)
> > return -ENODEV;
> > }
> > }
> > - pr_info("battery %d registered (start %d, stop %d)",
> > + if (acpi_has_method(hkey_handle, GET_INHIBIT))
> > + if (!ACPI_FAILURE(tpacpi_battery_acpi_eval(GET_INHIBIT, &ret, battery)))
> > + /* Support is marked in bit 5 */
> > + battery_info.batteries[battery].inhibit_support = ret & BIT(5);
> > +
> > + pr_info("battery %d registered (start %d, stop %d, inhibit: %d)",
> > battery,
> > battery_info.batteries[battery].charge_start,
> > - battery_info.batteries[battery].charge_stop);
> > + battery_info.batteries[battery].charge_stop,
> > + battery_info.batteries[battery].inhibit_support);
> >
> > return 0;
> > }
> > @@ -9484,6 +9521,15 @@ static ssize_t tpacpi_battery_store(int what,
> > if (tpacpi_battery_set(THRESHOLD_STOP, battery, value))
> > return -EINVAL;
> > return count;
> > + case INHIBIT_CHARGE:
> > + if (!battery_info.batteries[battery].inhibit_support)
> > + return -ENODEV;
> > + /* The only valid values are 1 and 0 */
> > + if (value != 0 && value != 1)
>
> I'm not sure, but maybe `if (value < 2)` is better here?
I think the exisiting validation is just fine. Anything other is
cosmetic and obfuscation IMO.
>
> > + return -EINVAL;
> > + if (tpacpi_battery_set(INHIBIT_CHARGE, battery, value))
> > + return -ENODEV;
> > + return count;
> > default:
> > pr_crit("Wrong parameter: %d", what);
> > return -EINVAL;
> > @@ -9546,12 +9592,28 @@ static ssize_t charge_stop_threshold_store(struct device *dev,
> > return tpacpi_battery_store(THRESHOLD_STOP, dev, buf, count);
> > }
> >
> > +static ssize_t inhibit_charge_store(struct device *dev,
> > + struct device_attribute *attr,
> > + const char *buf, size_t count)
> > +{
> > + return tpacpi_battery_store(INHIBIT_CHARGE, dev, buf, count);
> > +}
> > +
> > +static ssize_t inhibit_charge_show(struct device *device,
> > + struct device_attribute *attr,
> > + char *buf)
> > +{
> > + return tpacpi_battery_show(INHIBIT_CHARGE, device, buf);
> > +}
> > +
> > static DEVICE_ATTR_RW(charge_start_threshold);
> > static DEVICE_ATTR_RW(charge_stop_threshold);
> > +static DEVICE_ATTR_RW(inhibit_charge);
> >
> > static struct attribute *tpacpi_battery_attrs[] = {
> > &dev_attr_charge_start_threshold.attr,
> > &dev_attr_charge_stop_threshold.attr,
> > + &dev_attr_inhibit_charge.attr,
> > NULL,
> > };
> >
> > --
> > 2.17.0
> >
|
|
From: Henrique de M. H. <hm...@hm...> - 2018-05-14 12:03:16
|
On Sun, 13 May 2018, Ognjen Galic wrote: > +Battery force discharging > +-------------------------- > + > +There is also support for AC overriding. This means that you can force the battery to discharge > +even when AC is attached. This is also exposed via the generic ACPI driver: > + > +/sys/class/power_supply/BATx/force_discharge [int, 1, 0] > + > +Setting it to 1 forces the battery to discharge on AC. This is used to run a battery "fuel gauge" callibration cycle. One has to set the limits to 0,100 (i.e. disable the anti-wear limiters), trigger a force_discharge, and wait until it discharges to zero. At that point, the EC is supposed to reset the force_discharge bit by itself, and then you have to wait the battery to charge back to full. (and at least on older thinkpads, where you had to use SMAPI to do the above, it didn't need much help from the operating system. Once a couple years you could just set force_discharge on both batteries (with the thing connected to AC), shutdown the operating system, and go to sleep. By morning, it would be fully charged and both gauges calibrated, still powered off, ready to go :P ). -- Henrique Holschuh |
|
From: Henrique de M. H. <hm...@hm...> - 2018-05-14 11:39:43
|
On Mon, 14 May 2018, Christoph Böhmwalder wrote:
> > + case INHIBIT_CHARGE:
> > + if ACPI_FAILURE(tpacpi_battery_acpi_eval(GET_INHIBIT, ret, battery))
> > + return -ENODEV;
> > +
> > + /* The inhibit charge status is in the first bit */
> > + *ret = *ret & 0x01;
> > + return 0;
Do we know what is in the other bits? If so, please document the ACPI
method using a comment somewhere in the driver code, like you did for
SET_INHIBIT.
> > default:
> > pr_crit("wrong parameter: %d", what);
> > return -EINVAL;
> > @@ -9343,6 +9357,21 @@ static int tpacpi_battery_set(int what, int battery, int value)
> > return -ENODEV;
> > }
> > return 0;
> > + case INHIBIT_CHARGE:
> > + /* When setting inhbitit charge, we set a default vaulue of
>
> This comment does not adhere to the Linux coding style
Much on the driver doesn't, because it is _OLD_. But yeah, it is
preferrable to fix this as we add code, so it would be good to have all
new (and modified) comments switched to modern kernel style.
> > + case INHIBIT_CHARGE:
> > + if (!battery_info.batteries[battery].inhibit_support)
> > + return -ENODEV;
> > + /* The only valid values are 1 and 0 */
> > + if (value != 0 && value != 1)
>
> I'm not sure, but maybe `if (value < 2)` is better here?
Indeed... with a comment that says 0 = main battery, 1 = extra/dock
battery or something.
--
Henrique Holschuh
|
|
From: Andy S. <and...@gm...> - 2018-05-14 10:41:43
|
On Mon, May 14, 2018 at 12:46 PM, Christoph Böhmwalder <chr...@bo...> wrote: > On Sun, May 13, 2018 at 05:29:37PM +0200, Ognjen Galic wrote: >> Lenovo ThinkPad systems support the prevention of >> battery charging via a manual override called Inhibit Charge. >> >> This patch adds support for that attribute and exposes it via the >> battery ACPI driver in the generic location: >> >> /sys/class/power_supply/BATX/inhibit_charge >> + /* When setting inhbitit charge, we set a default vaulue of > > This comment does not adhere to the Linux coding style While you are right in principle, the whole driver is so old and uses this style. So, for such cases we, as maintainers, prefer less deviation work, i.e. keeping the style is a good thing to do. >> + /* The only valid values are 1 and 0 */ >> + if (value != 0 && value != 1) > > I'm not sure, but maybe `if (value < 2)` is better here? Since it's about integer-as-a-boolean, test for bit 0 would be sufficient, i.e. ~BIT(0). Though, I find this form not so readable since the input comes actually from the user. It would be nice to have just kstrtobool() called instead for such options, but see above. It would need a (huge) refactoring of the driver first. -- With Best Regards, Andy Shevchenko |
|
From: Christoph B. <chr...@bo...> - 2018-05-14 10:11:44
|
On Sun, May 13, 2018 at 05:29:37PM +0200, Ognjen Galic wrote:
> Lenovo ThinkPad systems support the prevention of
> battery charging via a manual override called Inhibit Charge.
>
> This patch adds support for that attribute and exposes it via the
> battery ACPI driver in the generic location:
>
> /sys/class/power_supply/BATX/inhibit_charge
>
> Signed-off-by: Ognjen Galic <smc...@gm...>
> ---
> drivers/platform/x86/thinkpad_acpi.c | 66 +++++++++++++++++++++++++++-
> 1 file changed, 64 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index da1ca485..b8b74889 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -9233,6 +9233,11 @@ static struct ibm_struct mute_led_driver_data = {
> #define GET_STOP "BCSG"
> #define SET_STOP "BCSS"
>
> +#define SET_INHIBIT "BICS"
> +#define GET_INHIBIT "BICG"
> +
> +#define INHIBIT_ATTR "inhibit_charge"
> +
> #define START_ATTR "charge_start_threshold"
> #define STOP_ATTR "charge_stop_threshold"
>
> @@ -9251,6 +9256,7 @@ enum {
> /* This is used in the get/set helpers */
> THRESHOLD_START,
> THRESHOLD_STOP,
> + INHIBIT_CHARGE
> };
>
> struct tpacpi_battery_data {
> @@ -9258,6 +9264,7 @@ struct tpacpi_battery_data {
> int start_support;
> int charge_stop;
> int stop_support;
> + int inhibit_support;
> };
>
> struct tpacpi_battery_driver_data {
> @@ -9315,6 +9322,13 @@ static int tpacpi_battery_get(int what, int battery, int *ret)
> if (*ret == 0)
> *ret = 100;
> return 0;
> + case INHIBIT_CHARGE:
> + if ACPI_FAILURE(tpacpi_battery_acpi_eval(GET_INHIBIT, ret, battery))
> + return -ENODEV;
> +
> + /* The inhibit charge status is in the first bit */
> + *ret = *ret & 0x01;
> + return 0;
> default:
> pr_crit("wrong parameter: %d", what);
> return -EINVAL;
> @@ -9343,6 +9357,21 @@ static int tpacpi_battery_set(int what, int battery, int value)
> return -ENODEV;
> }
> return 0;
> + case INHIBIT_CHARGE:
> + /* When setting inhbitit charge, we set a default vaulue of
This comment does not adhere to the Linux coding style
> + * always breaking on AC detach and the effective time is set to
> + * be permanent.
> + * The battery ID is in bits 4-5, 2 bits and the effective time
> + * is in bits 8-23, 2 bytes. A time of FFFF indicates forever.
> + */
> + param = value;
> + param |= battery << 4;
> + param |= 0xFFFF << 8;
> + if ACPI_FAILURE(tpacpi_battery_acpi_eval(SET_INHIBIT, &ret, param)) {
> + pr_err("failed to set inhibit charge on %d", battery);
> + return -ENODEV;
> + }
> + return 0;
> default:
> pr_crit("wrong parameter: %d", what);
> return -EINVAL;
> @@ -9359,6 +9388,8 @@ static int tpacpi_battery_probe(int battery)
> * 2) Check for support
> * 3) Get the current stop threshold
> * 4) Check for support
> + * 5) Check for inhibit charge support
> + * 6) Get the current inhibit charge status
> */
> if (acpi_has_method(hkey_handle, GET_START)) {
> if ACPI_FAILURE(tpacpi_battery_acpi_eval(GET_START, &ret, battery)) {
> @@ -9395,10 +9426,16 @@ static int tpacpi_battery_probe(int battery)
> return -ENODEV;
> }
> }
> - pr_info("battery %d registered (start %d, stop %d)",
> + if (acpi_has_method(hkey_handle, GET_INHIBIT))
> + if (!ACPI_FAILURE(tpacpi_battery_acpi_eval(GET_INHIBIT, &ret, battery)))
> + /* Support is marked in bit 5 */
> + battery_info.batteries[battery].inhibit_support = ret & BIT(5);
> +
> + pr_info("battery %d registered (start %d, stop %d, inhibit: %d)",
> battery,
> battery_info.batteries[battery].charge_start,
> - battery_info.batteries[battery].charge_stop);
> + battery_info.batteries[battery].charge_stop,
> + battery_info.batteries[battery].inhibit_support);
>
> return 0;
> }
> @@ -9484,6 +9521,15 @@ static ssize_t tpacpi_battery_store(int what,
> if (tpacpi_battery_set(THRESHOLD_STOP, battery, value))
> return -EINVAL;
> return count;
> + case INHIBIT_CHARGE:
> + if (!battery_info.batteries[battery].inhibit_support)
> + return -ENODEV;
> + /* The only valid values are 1 and 0 */
> + if (value != 0 && value != 1)
I'm not sure, but maybe `if (value < 2)` is better here?
> + return -EINVAL;
> + if (tpacpi_battery_set(INHIBIT_CHARGE, battery, value))
> + return -ENODEV;
> + return count;
> default:
> pr_crit("Wrong parameter: %d", what);
> return -EINVAL;
> @@ -9546,12 +9592,28 @@ static ssize_t charge_stop_threshold_store(struct device *dev,
> return tpacpi_battery_store(THRESHOLD_STOP, dev, buf, count);
> }
>
> +static ssize_t inhibit_charge_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + return tpacpi_battery_store(INHIBIT_CHARGE, dev, buf, count);
> +}
> +
> +static ssize_t inhibit_charge_show(struct device *device,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + return tpacpi_battery_show(INHIBIT_CHARGE, device, buf);
> +}
> +
> static DEVICE_ATTR_RW(charge_start_threshold);
> static DEVICE_ATTR_RW(charge_stop_threshold);
> +static DEVICE_ATTR_RW(inhibit_charge);
>
> static struct attribute *tpacpi_battery_attrs[] = {
> &dev_attr_charge_start_threshold.attr,
> &dev_attr_charge_stop_threshold.attr,
> + &dev_attr_inhibit_charge.attr,
> NULL,
> };
>
> --
> 2.17.0
>
|
|
From: Ognjen G. <smc...@gm...> - 2018-05-13 15:30:36
|
This documents the new battery controls and wear leveling support
for Lenovo ThinkPad laptops.
Signed-off-by: Ognjen Galic <smc...@gm...>
---
Documentation/laptops/thinkpad-acpi.txt | 70 ++++++++++++++++++++++++-
1 file changed, 69 insertions(+), 1 deletion(-)
diff --git a/Documentation/laptops/thinkpad-acpi.txt b/Documentation/laptops/thinkpad-acpi.txt
index 00b6dfed..3491d46f 100644
--- a/Documentation/laptops/thinkpad-acpi.txt
+++ b/Documentation/laptops/thinkpad-acpi.txt
@@ -1,6 +1,6 @@
ThinkPad ACPI Extras Driver
- Version 0.25
+ Version 0.27
October 16th, 2013
Borislav Deianov <bor...@us...>
@@ -46,6 +46,9 @@ detailed description):
- Fan control and monitoring: fan speed, fan enable/disable
- WAN enable and disable
- UWB enable and disable
+ - Battery wear control (battery start and stop threshold)
+ - Battery inhibit charge
+ - Battery force discharge
A compatibility table by model and feature is maintained on the web
site, http://ibm-acpi.sf.net/. I appreciate any success or failure
@@ -1483,3 +1486,68 @@ Sysfs interface changelog:
0x030000: Thermal and fan sysfs attributes were moved to the hwmon
device instead of being attached to the backing platform
device.
+
+
+Battery wear control (charge and stop thresholds)
+--------------------------------------------------
+
+Since Linux 4.17 tpacpi supports setting of charge start and stop thresholds on certain
+Lenovo ThinkPad laptops. This feature is exposed via a new API from the ACPI battery driver
+also new in Linux 4.17.
+
+The new battery API allows developers to extended the standard attributes offered by the
+generic ACPI battery driver and add custom attributes from other drivers. thinkpad_acpi
+leverages this API and exposes a few custom attributes for wear control of the battery.
+
+Wear control is designed to stop the charging of the battery at a certain threshold and thus
+stop the cell from wearing out from constant charge cycles. So if you put the threshold at
+50%, the charge will only charge the battery when it is below 50% and it will stop at 50%.
+
+A lower level is also exposed, thus creating a charger "dead zone", a zone where the charger does
+nothing and the battery is in idle as if it was removed from the Thinkpad.
+
+For example, this is what happens when you set the values to 40% and 65%:
+
+ ┌ Discharging ┐
+ │ │
+[0%--------40% 65%-----------100%] Battery 1
+ │ │
+ └ Charging ┘
+
+When plugged into AC and if the percentage is between 40% and 100% the battery is in idle.
+
+The attributes:
+---------------
+
+/sys/class/power_supply/BATx/charge_start_threshold [int, <0, >99]
+
+This sets the lower charge stop threshold for the battery. Its a integer and valid values are from
+0% to 99%.
+
+/sys/class/power_supply/BATx/charge_stop_threshold [int, <1, >100]
+
+This sets the upper charge threshold for the battery. Its a integer and the value values are from
+1% to 100%.
+
+Setting the lower limit to 0% and the upper limit to 100% disables the wear leveling control feature.
+
+
+Battery charge inhibiting
+--------------------------
+
+Preventing the battery from charging is also available as a manual override over the settings. Its exposed via
+the generic ACPI driver:
+
+/sys/class/power_supply/BATx/inhibit_charge [int, 1, 0]
+
+Setting it to 1 prevents the battery from charging.
+
+Battery force discharging
+--------------------------
+
+There is also support for AC overriding. This means that you can force the battery to discharge
+even when AC is attached. This is also exposed via the generic ACPI driver:
+
+/sys/class/power_supply/BATx/force_discharge [int, 1, 0]
+
+Setting it to 1 forces the battery to discharge on AC.
--
2.17.0
|
|
From: Ognjen G. <smc...@gm...> - 2018-05-13 15:30:12
|
Lenovo ThinkPad systems have a feature that lets you
force the battery to discharge regardless if AC is attached
or not.
This patch implements that feature and exposes it via the generic
ACPI battery driver.
Signed-off-by: Ognjen Galic <smc...@gm...>
---
drivers/platform/x86/thinkpad_acpi.c | 56 ++++++++++++++++++++++++++--
1 file changed, 53 insertions(+), 3 deletions(-)
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index b8b74889..28a87640 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -9236,7 +9236,11 @@ static struct ibm_struct mute_led_driver_data = {
#define SET_INHIBIT "BICS"
#define GET_INHIBIT "BICG"
+#define GET_DISCHARGE "BDSG"
+#define SET_DISCHARGE "PSBS"
+
#define INHIBIT_ATTR "inhibit_charge"
+#define DISCHARGE_ATTR "force_discharge"
#define START_ATTR "charge_start_threshold"
#define STOP_ATTR "charge_stop_threshold"
@@ -9256,7 +9260,8 @@ enum {
/* This is used in the get/set helpers */
THRESHOLD_START,
THRESHOLD_STOP,
- INHIBIT_CHARGE
+ INHIBIT_CHARGE,
+ FORCE_DISCHARGE
};
struct tpacpi_battery_data {
@@ -9265,6 +9270,7 @@ struct tpacpi_battery_data {
int charge_stop;
int stop_support;
int inhibit_support;
+ int discharge_support;
};
struct tpacpi_battery_driver_data {
@@ -9329,6 +9335,12 @@ static int tpacpi_battery_get(int what, int battery, int *ret)
/* The inhibit charge status is in the first bit */
*ret = *ret & 0x01;
return 0;
+ case FORCE_DISCHARGE:
+ if ACPI_FAILURE(tpacpi_battery_acpi_eval(GET_DISCHARGE, ret, battery))
+ return -ENODEV;
+ /* The force discharge status is in the first bit */
+ *ret = *ret & 0x01;
+ return 0;
default:
pr_crit("wrong parameter: %d", what);
return -EINVAL;
@@ -9372,6 +9384,14 @@ static int tpacpi_battery_set(int what, int battery, int value)
return -ENODEV;
}
return 0;
+ case FORCE_DISCHARGE:
+ param = battery;
+ param |= value << 8;
+ if ACPI_FAILURE(tpacpi_battery_acpi_eval(SET_DISCHARGE, &ret, param)) {
+ pr_err("failed to set force dischrage on %d", battery);
+ return -ENODEV;
+ }
+ return 0;
default:
pr_crit("wrong parameter: %d", what);
return -EINVAL;
@@ -9431,11 +9451,16 @@ static int tpacpi_battery_probe(int battery)
/* Support is marked in bit 5 */
battery_info.batteries[battery].inhibit_support = ret & BIT(5);
- pr_info("battery %d registered (start %d, stop %d, inhibit: %d)",
+ if (acpi_has_method(hkey_handle, GET_DISCHARGE))
+ if (!ACPI_FAILURE(tpacpi_battery_acpi_eval(GET_DISCHARGE, &ret, battery)))
+ battery_info.batteries[battery].discharge_support = ret & BIT(8);
+
+ pr_info("battery %d registered (start %d, stop %d, inhibit: %d, force: %d)",
battery,
battery_info.batteries[battery].charge_start,
battery_info.batteries[battery].charge_stop,
- battery_info.batteries[battery].inhibit_support);
+ battery_info.batteries[battery].inhibit_support,
+ battery_info.batteries[battery].discharge_support);
return 0;
}
@@ -9530,6 +9555,15 @@ static ssize_t tpacpi_battery_store(int what,
if (tpacpi_battery_set(INHIBIT_CHARGE, battery, value))
return -ENODEV;
return count;
+ case FORCE_DISCHARGE:
+ if (!battery_info.batteries[battery].discharge_support)
+ return -ENODEV;
+ /* The only valid values are 1 and 0 */
+ if (value != 0 && value != 1)
+ return -EINVAL;
+ if (tpacpi_battery_set(FORCE_DISCHARGE, battery, value))
+ return -ENODEV;
+ return count;
default:
pr_crit("Wrong parameter: %d", what);
return -EINVAL;
@@ -9606,14 +9640,30 @@ static ssize_t inhibit_charge_show(struct device *device,
return tpacpi_battery_show(INHIBIT_CHARGE, device, buf);
}
+static ssize_t force_discharge_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ return tpacpi_battery_store(FORCE_DISCHARGE, dev, buf, count);
+}
+
+static ssize_t force_discharge_show(struct device *device,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return tpacpi_battery_show(FORCE_DISCHARGE, device, buf);
+}
+
static DEVICE_ATTR_RW(charge_start_threshold);
static DEVICE_ATTR_RW(charge_stop_threshold);
static DEVICE_ATTR_RW(inhibit_charge);
+static DEVICE_ATTR_RW(force_discharge);
static struct attribute *tpacpi_battery_attrs[] = {
&dev_attr_charge_start_threshold.attr,
&dev_attr_charge_stop_threshold.attr,
&dev_attr_inhibit_charge.attr,
+ &dev_attr_force_discharge.attr,
NULL,
};
--
2.17.0
|
|
From: Ognjen G. <smc...@gm...> - 2018-05-13 15:29:49
|
Lenovo ThinkPad systems support the prevention of
battery charging via a manual override called Inhibit Charge.
This patch adds support for that attribute and exposes it via the
battery ACPI driver in the generic location:
/sys/class/power_supply/BATX/inhibit_charge
Signed-off-by: Ognjen Galic <smc...@gm...>
---
drivers/platform/x86/thinkpad_acpi.c | 66 +++++++++++++++++++++++++++-
1 file changed, 64 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index da1ca485..b8b74889 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -9233,6 +9233,11 @@ static struct ibm_struct mute_led_driver_data = {
#define GET_STOP "BCSG"
#define SET_STOP "BCSS"
+#define SET_INHIBIT "BICS"
+#define GET_INHIBIT "BICG"
+
+#define INHIBIT_ATTR "inhibit_charge"
+
#define START_ATTR "charge_start_threshold"
#define STOP_ATTR "charge_stop_threshold"
@@ -9251,6 +9256,7 @@ enum {
/* This is used in the get/set helpers */
THRESHOLD_START,
THRESHOLD_STOP,
+ INHIBIT_CHARGE
};
struct tpacpi_battery_data {
@@ -9258,6 +9264,7 @@ struct tpacpi_battery_data {
int start_support;
int charge_stop;
int stop_support;
+ int inhibit_support;
};
struct tpacpi_battery_driver_data {
@@ -9315,6 +9322,13 @@ static int tpacpi_battery_get(int what, int battery, int *ret)
if (*ret == 0)
*ret = 100;
return 0;
+ case INHIBIT_CHARGE:
+ if ACPI_FAILURE(tpacpi_battery_acpi_eval(GET_INHIBIT, ret, battery))
+ return -ENODEV;
+
+ /* The inhibit charge status is in the first bit */
+ *ret = *ret & 0x01;
+ return 0;
default:
pr_crit("wrong parameter: %d", what);
return -EINVAL;
@@ -9343,6 +9357,21 @@ static int tpacpi_battery_set(int what, int battery, int value)
return -ENODEV;
}
return 0;
+ case INHIBIT_CHARGE:
+ /* When setting inhbitit charge, we set a default vaulue of
+ * always breaking on AC detach and the effective time is set to
+ * be permanent.
+ * The battery ID is in bits 4-5, 2 bits and the effective time
+ * is in bits 8-23, 2 bytes. A time of FFFF indicates forever.
+ */
+ param = value;
+ param |= battery << 4;
+ param |= 0xFFFF << 8;
+ if ACPI_FAILURE(tpacpi_battery_acpi_eval(SET_INHIBIT, &ret, param)) {
+ pr_err("failed to set inhibit charge on %d", battery);
+ return -ENODEV;
+ }
+ return 0;
default:
pr_crit("wrong parameter: %d", what);
return -EINVAL;
@@ -9359,6 +9388,8 @@ static int tpacpi_battery_probe(int battery)
* 2) Check for support
* 3) Get the current stop threshold
* 4) Check for support
+ * 5) Check for inhibit charge support
+ * 6) Get the current inhibit charge status
*/
if (acpi_has_method(hkey_handle, GET_START)) {
if ACPI_FAILURE(tpacpi_battery_acpi_eval(GET_START, &ret, battery)) {
@@ -9395,10 +9426,16 @@ static int tpacpi_battery_probe(int battery)
return -ENODEV;
}
}
- pr_info("battery %d registered (start %d, stop %d)",
+ if (acpi_has_method(hkey_handle, GET_INHIBIT))
+ if (!ACPI_FAILURE(tpacpi_battery_acpi_eval(GET_INHIBIT, &ret, battery)))
+ /* Support is marked in bit 5 */
+ battery_info.batteries[battery].inhibit_support = ret & BIT(5);
+
+ pr_info("battery %d registered (start %d, stop %d, inhibit: %d)",
battery,
battery_info.batteries[battery].charge_start,
- battery_info.batteries[battery].charge_stop);
+ battery_info.batteries[battery].charge_stop,
+ battery_info.batteries[battery].inhibit_support);
return 0;
}
@@ -9484,6 +9521,15 @@ static ssize_t tpacpi_battery_store(int what,
if (tpacpi_battery_set(THRESHOLD_STOP, battery, value))
return -EINVAL;
return count;
+ case INHIBIT_CHARGE:
+ if (!battery_info.batteries[battery].inhibit_support)
+ return -ENODEV;
+ /* The only valid values are 1 and 0 */
+ if (value != 0 && value != 1)
+ return -EINVAL;
+ if (tpacpi_battery_set(INHIBIT_CHARGE, battery, value))
+ return -ENODEV;
+ return count;
default:
pr_crit("Wrong parameter: %d", what);
return -EINVAL;
@@ -9546,12 +9592,28 @@ static ssize_t charge_stop_threshold_store(struct device *dev,
return tpacpi_battery_store(THRESHOLD_STOP, dev, buf, count);
}
+static ssize_t inhibit_charge_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ return tpacpi_battery_store(INHIBIT_CHARGE, dev, buf, count);
+}
+
+static ssize_t inhibit_charge_show(struct device *device,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return tpacpi_battery_show(INHIBIT_CHARGE, device, buf);
+}
+
static DEVICE_ATTR_RW(charge_start_threshold);
static DEVICE_ATTR_RW(charge_stop_threshold);
+static DEVICE_ATTR_RW(inhibit_charge);
static struct attribute *tpacpi_battery_attrs[] = {
&dev_attr_charge_start_threshold.attr,
&dev_attr_charge_stop_threshold.attr,
+ &dev_attr_inhibit_charge.attr,
NULL,
};
--
2.17.0
|
|
From: Henrique de M. H. <hm...@hm...> - 2018-04-27 13:20:21
|
On Thu, 26 Apr 2018, Jordan Glover via ibm-acpi-devel wrote: > I have to admit that I can't handle booting and checking this in tablet mode > (without keyboard) so I won't confirm it, sorry. FWIW, any USB keyboard should allow you to do this easily if there is at least one USB port still exposed while in tablet mode... That said, thanks for the help you gave us thus far! -- Henrique Holschuh |
