[PATCH 0/2] [RFC] platform/x86: Fixes for Toshiba Z830

[PATCH 0/2] [RFC] platform/x86: Fixes for Toshiba Z830

[Arvid Norlander]

Hi,

NOTE: I had some trouble sending this with git send-email, I only managed
to send one copy successfully! Sorry if I managed to send it multiple
times.

I'm new to this kernel development thing, so applogies in advance for any
mistakes. I have an old Toshiba Z830-10W laptop. Unfortunately it doesn't
work well under Linux. Fortunately I spent some time figuring out what was
wrong. I had documented my findings below. I have also included patches
(see the next few emails) for some of the issues.

I would like advice on how to proceed for some of the more advanced
problems though:
* Do we want to expose all these features that I figured out? For example,
  how to set the boot order on this old BIOS-only laptop from user space.
  Or various other settings accessible via the BIOS.
* For the hardware buttons I describe below, is a solution specific to
  toshiba_acpi preferred, or should additional effort be spent on
  investigating a generic solution?

Before I start coding on these more complex issues I would like advice in
order to avoid wasting my time on bad designs. In particular, on how to
proceed with the "Hardware buttons" section below.

Best regards,
Arvid Norlander

Table of contents
=================
1. Short info on the laptop and methodology
2. Background on Toshiba ACPI communication methods
3. LED: "Eco" LED ................................ [implemented in patch 1]
4. Battery charge mode ........................... [implemented in patch 2]
5. Hardware buttons ...................................... [Advice wanted!]
6. Panel power control via HCI
7. BIOS setting control from the OS ................... [should we bother?]
   7.1 Setting boot order
   7.2 Setting USB memory emulation
   7.3 Display during boot
   7.4 CPU control
   7.5 Wake on LAN
   7.6 SATA power control
   7.8 Legacy USB
   7.9 Wake on keyboard
8. Other features .... [nothing actionable as of now, for information only]


1. Short info on the laptop and methodology
===========================================

The Toshiba Z830-10W laptop is a so-called "Ultrabook" dating from 2011.
It has BIOS, not UEFI.

The Toshiba Z830-10W laptop has several features that don't work properly
under Linux. I have performed reverse engineering by tracing ACPI calls
with WinDbg using the "AMLI" feature while performing various actions to
determine what the Windows drivers do. My user space tracing on Windows
has been limited to determine which programs interact with the driver so
that I could kill those that I was not testing behaviour of at the moment.

I have then tested these features using the "acpi_call" kernel module on
Linux to issue the relevant calls under Linux. In the following sections
is a feature by feature breakdown.


2. Background on Toshiba ACPI communication methods
===================================================

This is a short summary of the general protocol. This is already
implemented in the toshiba_acpi driver. If you are already familiar with
that you can skip this section.

Almost all vendor specific features work via the \_SB_.VALZ ACPI device
defined in the DSDT. There are a handful of interesting methods on this
object, but for the purposes of this write-up only "GHCI" is relevant. This
method takes 6 integer (32-bit) arguments and returns a buffer 6 32-bit
integers.

The general format of queries is: {OPERATION, REGISTER, ARG1, ..., ARG4 }.
The operation is one of HCI_GET/HCI_SET or SCI_GET/SCI_SET (plus SCI_OPEN
and  SCI_CLOSE). This allows for getting and setting various registers to
control features or read out data.

The data returned varies a bit, but is /generally/ on the form:
{STATUS_CODE, REGISTER_FROM_QUERY, VAL1, ..., VAL4 }

What is the difference between HCI_* and SCI_* calls? The only important
difference here is that for SCI_GET/SET you first need to call SCI_OPEN and
then follow the get or set with an SCI_CLOSE call.

Much of the rest of this write-up consists of documenting registers
previously not handled by the toshiba_acpi Linux driver.


3. LED: "Eco" LED [implemented in patch 1]
=================

The toshiba_acpi driver has support for controlling some LEDs including the
"Eco" LED. Unfortunately that LED works differently on this laptop.

The toshiba_acpi driver checks for TOS_INPUT_DATA_ERROR and tries a
different format. On the Z830 the error returned is TOS_NOT_SUPPORTED
though the different format still works.


4. Battery charge mode [implemented in patch 2]
======================

This laptop supports not charging the battery fully in order to prolong
battery life. Unlike for example ThinkPads where this control is granular
here it is just off/on. When off it charges to 100%. When on it charges to
about 80%.

According to the Windows program used to control the feature the setting
will not take effect until the battery has been discharged to around 50%.
However, in my testing it takes effect as soon as the charge drops below
80%. On Windows Toshiba branded this feature as "Eco charging"

In the following example ACPI calls I will use the following newly defined
constants:
#define HCI_BATTERY_CHARGE_MODE 0xba
#define BATTERY_CHARGE_FULL 0
#define BATTERY_CHARGE_80_PERCENT 1

To set the feature:
  {HCI_SET, HCI_BATTERY_CHARGE_MODE, charge_mode, 0, 0, 0}
To query for the existence of the feature:
  {HCI_GET, HCI_BATTERY_CHARGE_MODE, 0, 0, 0, 0}
To read the feature:
  {HCI_GET, HCI_BATTERY_CHARGE_MODE, 0, 0, 0, 1}

The read may need to be retried if TOS_DATA_NOT_AVAILABLE is returned as
the status code. This rarely happens (I have never observed it on Linux),
but I have seen it happen under Windows once, and the software did retry
it.


5. Hardware buttons [Advice wanted!]
===================

All the Fn+<key> hotkeys work. However, there are some hardware buttons
that do not. These buttons are:

* A button between space and the touchpad to turn off/on the touchpad.
* Two buttons next to the power button, one is "eco-mode", the other is
"projector".

The two buttons next to the power button both send Windows+X by default.
The touchpad control button does nothing that Linux can detect.

To enable this functionality several changes are needed.

The toshiba_acpi driver currently uses
  {HCI_SET, HCI_HOTKEY_EVENT, HCI_HOTKEY_ENABLE, 0, ...}
to enable the Fn+<key> hotkeys, where HCI_HOTKEY_ENABLE = 0x09. However on
this laptop the value 0x05 must be used instead.

This is not the whole story however, as these keys do not work like any of
the Fn-hotkeys (ACPI notification on \_SB_.VALZ). Instead, once enabled via
the above method they start sending notifications on various PNP0C32
devices. These are currently not handled by Linux. According to a search
PNP0C32 is "HIDACPI Button Device", so perhaps a generic driver should be
created.

The devices in question are:
PNP0C32 \_SB_.HS81 UID 0x03 = Enable/disable trackpad
PNP0C32 \_SB_.HS87 UID 0x01 = Eco button
PNP0C32 \_SB_.HS86 UID 0x02 = Monitor/projector button

Only the "path" and the "UID" value in the ACPI DSDT tell these devices
apart.

The notification always uses the value 0x80.

I'm not sure what the best approach to implement support for these would
be.


6. Panel power control via HCI
==============================

The toshiba_acpi driver supports controlling the panel power via SCI calls
(SCI_PANEL_POWER_ON). Based on the fact that the toshiba_acpi driver
outputs a message about reboot being needed I assume this is related to
panel power during boot?

However there is a HCI call that can turn the panel off or on immediately:

#define HCI_PANEL_POWER_ON 0x2
#define PANEL_ON 1
#define PANEL_OFF 0

To read/query existence: {HCI_GET, HCI_PANEL_POWER_ON, 0, 0, 0, 0}
To write: {HCI_SET, HCI_PANEL_POWER_ON, panel_on, 0, 0, 0}

This could be related to some backlight issues this laptop is having where
backlight control stops working after a suspend/resume.

Control via /sys/class/backlight/intel_backlight always works on this
laptop, however, most desktop environment seems to prefer the acpi_video or
vendor backlight controls if those exist.

I have tried acpi_backlight=vendor/native but that is broken in the same
way. With acpi_backlight=none, the screen never turns on after a resume.
However if I turn it on using the above HCI call, everything works
normally after that. And since only the intel_backlight driver is left,
the desktop environment controls backlight via that method.
  
I have yet to find a satisfactory solution to this problem, but I suspect
the correct solution would be to fix backlight control from acpi_video,
if that is possible. Suggestions?


7. BIOS setting control from the OS [should we bother?]
===================================
Several of the BIOS settings can be controlled from the OS. This all
happens via SCI calls. On Windows the "Hwsetup.exe" program offers this
control. I'm not sure how useful any of this is (as this is already
available via the BIOS).

If you think it is a good idea I could absolutely implement support for
this. Otherwise I might build a simple user space tool on top of acpi_call
for this. 

7.1 Setting boot order
----------------------
This is a BIOS (not UEFI) laptop, so boot order could normally not be
controlled from the OS. However here it is possible:

#define SCI_BOOT_ORDER 0x157

In this SCI register the boot order is stored as a list with each nibble
indicating a device:
#define SCI_BOOT_ORDER_FDD 0x0
#define SCI_BOOT_ORDER_HDD 0x1
#define SCI_BOOT_ORDER_LAN 0x3
#define SCI_BOOT_ORDER_USB_MEMORY 0x4
#define SCI_BOOT_ORDER_USB_CD 0x7
#define SCI_BOOT_ORDER_USB_UNUSED 0xf

These are then combined as follows:

Set boot order to USB memory, USB CD, HDD, LAN, FDD:
{SCI_SET, SCI_BOOT_ORDER, 0xfff03174, 0, 0, 0}

Each nibble indicates a device, with the lowest nibble being the first
device in the boot order. As this device doesn't have a physical FDD I
assume that this refers to USB attached devices, but I have not tested this
(I do have a USB floppy drive if anyone really cares).

When reading the data out the result is a bit surprising:
  {0x0, 0x8505, 0xfff30174, 0x5, 0xfff30741, 0x0}
Presumably these other values also mean something, the boot order in this
case is USB memory, USB CD, HDD, FDD, LAN, so the third value is the boot
order.

I'm not sure what a suitable API for exposing this setting to userspace via
sysfs would be.

7.2 Setting USB memory emulation
--------------------------------
The BIOS can either treat USB drives as HDDs or FDDs for booting purposes:

#define SCI_BOOT_FLOPPY_EMULATION 0x511
#define SCI_BOOT_FLOPPY_EMULATION_FDD 0x1
#define SCI_BOOT_FLOPPY_EMULATION_HDD 0x0

To set: {SCI_SET, SCI_BOOT_FLOPPY_EMULATION, value, 0, 0, 0}
Getting/existence query: {SCI_GET, SCI_BOOT_FLOPPY_EMULATION, 0, 0, 0, 0}

7.3 Display during boot
-----------------------
This controls if BIOS/GRUB/etc is shown on just the internal monitor or
if the montior is automatically selected.

Note: When changing this in Windows it tells me a restart is required.

#define SCI_BOOT_DISPLAY 0x300
#define SCI_BOOT_DISPLAY_INTERNAL 0x1250
#define SCI_BOOT_DISPLAY_AUTO 0x3250

To set: {SCI_SET, SCI_BOOT_DISPLAY, value, 0, 0, 0}
Getting/existence query as usual.

Note! I have not tested the effects of this, as the only external monitors
I have that are not in storage use DisplayPort, while this laptop has HDMI
and VGA. I *do* have an old VGA TFT in storage if anyone cares though...

7.4 CPU control
---------------
I presume this is only for operating systems that don't manage this
themselves, I don't know for sure. The wording in the documentation is
vague, but I believe it controls CPU frequency behaviour.

Note: When changing this in Windows it tells me a restart is required.

#define SCI_CPU_FREQUENCY 0x132
#define SCI_CPU_FREQUENCY_DYNAMIC 0x0
#define SCI_CPU_FREQUENCY_HIGH 0x1
#define SCI_CPU_FREQUENCY_LOW 0x2

Set and get as usual ({SCI_GET/SET, SCI_CPU_FREQUENCY, value, 0, 0, 0}).

7.5 Wake on LAN
---------------
Note! This only controls Wake on LAN when off/hibernated (and since this
laptop has Intel Rapid Start, presumably in that mode too). It is not
relevant to WoL when in sleep.

Here the Windows driver seem to query several possibilities until it hits
on one that works:
#define SCI_WAKE_ON_LAN 0x700

#define SCI_WAKE_ON_LAN_OFF 0x1
#define SCI_WAKE_ON_LAN_ON 0x1

#define SCI_WAKE_ON_LAN_REG1 0x0
#define SCI_WAKE_ON_LAN_REG2 0x1000
#define SCI_WAKE_ON_LAN_REG3 0x800

To set:
  {SCI_SET, SCI_WAKE_ON_LAN, value | register, 0, 0, 0}
To get/query:
  {SCI_GET, SCI_WAKE_ON_LAN, register, 0, 0, 0}

For example on this specific laptop to enable WoL:
  {SCI_SET, SCI_WAKE_ON_LAN, SCI_WAKE_ON_LAN_ON | SCI_WAKE_ON_LAN_REG3, 0, 0, 0}

REG1 and REG2 give return code TOS_INPUT_DATA_ERROR on this laptop, but
presumably they are needed on some laptops, or the Windows program would
not be attempting to use them.

7.6 SATA power control
----------------------
This is another one that I don't know what exactly it corresponds to, maybe
it is something Linux can control directly:

#define SCI_SATA_POWER 0x406
#define SCI_SATA_POWER_BATTERY_LIFE 0x1
#define SCI_SATA_POWER_PERFORMANCE 0x0

Get/set/query as expected: {SCI_SET, SCI_SATA_POWER, value, 0, 0, 0}

7.8 Legacy USB
--------------
Controls Legacy USB support in BIOS.

Note: When changing this in Windows it tells me a restart is required.

#define SCI_LEGACY_USB 0x50c
#define SCI_LEGACY_USB_ENABLED 0x1
#define SCI_LEGACY_USB_DISABLED 0x0


Get/set/query as expected: {SCI_SET, SCI_LEGACY_USB, value, 0, 0, 0}

7.9 Wake on keyboard
--------------------
This controls if pressing a key on the keyboard wakes the laptop from
sleep. Otherwise, only opening the monitor or pressing the power button
works for this.

#define SCI_WAKE_ON_KEYBOARD 0x137
#define SCI_WAKE_ON_KEYBOARD_ENABLE 0x8
#define SCI_WAKE_ON_KEYBOARD_DISABLE 0x0

Set: {SCI_SET, SCI_WAKE_ON_KEYBOARD, value, 0, 0, 0}

Get/query on the standard form but with slightly weird return values:
  {TOS_SUCCESS2, 0x800a, value, 0x38, 0x0, 0x0}


8. Other features
=================

I should note that I did discover some additional registers, but I don't
fully understand them yet. I have thus not included them here in order to
not waste your time. However I did write a blog post about this which
includes that information, which is available should you be interested:

https://vorpal.se/posts/2022/aug/21/reverse-engineering-acpi-functionality-on-a-toshiba-z830-ultrabook/#other-features_1


Arvid Norlander (2):
  platform/x86: Fix ECO LED control on Toshiba Z830
  platform/x86: Battery charge mode in toshiba_acpi

 drivers/platform/x86/toshiba_acpi.c | 115 +++++++++++++++++++++++++++-
 1 file changed, 114 insertions(+), 1 deletion(-)


base-commit: e3f259d33c0ebae1b6e4922c7cdb50e864c81928
-- 
2.37.2

[PATCH 1/2] platform/x86: Fix ECO LED control on Toshiba Z830

[Arvid Norlander]

The toshiba_acpi driver checks for TOS_INPUT_DATA_ERROR and tries a
different format. On the Z830 the error returned is TOS_NOT_SUPPORTED
though the different format still works. Allow either error.

Signed-off-by: Arvid Norlander <lkml@vorpal.se>
---
 drivers/platform/x86/toshiba_acpi.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index 0fc9e8b8827b..6cc617b2940e 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -675,12 +675,15 @@ static void toshiba_eco_mode_available(struct toshiba_acpi_dev *dev)
 		return;
 	}
 
-	if (out[0] == TOS_INPUT_DATA_ERROR) {
+	if (out[0] == TOS_INPUT_DATA_ERROR || out[0] == TOS_NOT_SUPPORTED) {
 		/*
 		 * If we receive 0x8300 (Input Data Error), it means that the
 		 * LED device is present, but that we just screwed the input
 		 * parameters.
 		 *
+		 * On some laptops 0x8000 (Not supported) is also returned in
+		 * this case, so we need to allow for that as well.
+		 *
 		 * Let's query the status of the LED to see if we really have a
 		 * success response, indicating the actual presense of the LED,
 		 * bail out otherwise.
-- 
2.37.2

[PATCH 2/2] platform/x86: Battery charge mode in toshiba_acpi

[Arvid Norlander]

Unlike for example ThinkPads where this control is granular here it is
just off/on. When off it charges to 100%. When on it charges to about 80%.

Controlling this setting is done via HCI register 0x00ba. Setting to value
1 will result in limiting the charing to 80% of the battery capacity,
while setting it to 0 will allow charging to 100%.

Reading the current state is a bit weird, and needs a 1 set in the last
position of the query for whatever reason. In addition, the read may
return 0x8d20 (Data not available) rarely, so a retry mechanism is needed.

According to the Windows program used to control the feature the setting
will not take effect until the battery has been discharged to around 50%.
However, in my testing it takes effect as soon as the charge drops below
80%. On Windows Toshiba branded this feature as "Eco charging".

Signed-off-by: Arvid Norlander <lkml@vorpal.se>
---
 drivers/platform/x86/toshiba_acpi.c | 110 ++++++++++++++++++++++++++++
 1 file changed, 110 insertions(+)

diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index 6cc617b2940e..2e13f241538a 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -112,6 +112,7 @@ MODULE_LICENSE("GPL");
 #define HCI_KBD_ILLUMINATION		0x0095
 #define HCI_ECO_MODE			0x0097
 #define HCI_ACCELEROMETER2		0x00a6
+#define HCI_BATTERY_CHARGE_MODE		0x00ba
 #define HCI_SYSTEM_INFO			0xc000
 #define SCI_PANEL_POWER_ON		0x010d
 #define SCI_ILLUMINATION		0x014e
@@ -201,6 +202,7 @@ struct toshiba_acpi_dev {
 	unsigned int usb_three_supported:1;
 	unsigned int wwan_supported:1;
 	unsigned int cooling_method_supported:1;
+	unsigned int battery_charge_mode_supported:1;
 	unsigned int sysfs_created:1;
 	unsigned int special_functions;
 
@@ -1285,6 +1287,69 @@ static int toshiba_cooling_method_set(struct toshiba_acpi_dev *dev, u32 state)
 	return (result == TOS_SUCCESS || result == TOS_SUCCESS2) ? 0 : -EIO;
 }
 
+/* Battery charge control */
+static void toshiba_battery_charge_mode_available(struct toshiba_acpi_dev *dev)
+{
+	u32 in[TCI_WORDS] = { HCI_GET, HCI_BATTERY_CHARGE_MODE, 0, 0, 0, 0 };
+	u32 out[TCI_WORDS];
+	acpi_status status;
+
+	dev->battery_charge_mode_supported = 0;
+
+	status = tci_raw(dev, in, out);
+	if (ACPI_FAILURE(status)) {
+		pr_err("ACPI call to get Battery Charge Mode failed\n");
+		return;
+	}
+
+	if (out[0] != TOS_SUCCESS && out[0] != TOS_SUCCESS2)
+		return;
+
+	dev->battery_charge_mode_supported = 1;
+}
+
+static int toshiba_battery_charge_mode_get(struct toshiba_acpi_dev *dev, u32 *state)
+{
+	u32 in[TCI_WORDS] = { HCI_GET, HCI_BATTERY_CHARGE_MODE, 0, 0, 0, 0x1 };
+	u32 out[TCI_WORDS];
+	int retries = 3;
+
+	do {
+		acpi_status status = tci_raw(dev, in, out);
+
+		if (ACPI_FAILURE(status))
+			pr_err("ACPI call to get Battery Charge Mode failed\n");
+		switch (out[0]) {
+		case TOS_SUCCESS:
+		case TOS_SUCCESS2:
+			*state = out[2];
+			return 0;
+		case TOS_NOT_SUPPORTED:
+			return -ENODEV;
+		case TOS_DATA_NOT_AVAILABLE:
+			retries--;
+			break;
+		default:
+			return -EIO;
+		}
+	} while (retries);
+
+	return -EIO;
+}
+
+static int toshiba_battery_charge_mode_set(struct toshiba_acpi_dev *dev, u32 state)
+{
+	u32 result = hci_write(dev, HCI_BATTERY_CHARGE_MODE, state);
+
+	if (result == TOS_FAILURE)
+		pr_err("ACPI call to set Battery Charge Mode failed\n");
+
+	if (result == TOS_NOT_SUPPORTED)
+		return -ENODEV;
+
+	return (result == TOS_SUCCESS || result == TOS_SUCCESS2) ? 0 : -EIO;
+}
+
 /* Transflective Backlight */
 static int get_tr_backlight_status(struct toshiba_acpi_dev *dev, u32 *status)
 {
@@ -2334,6 +2399,44 @@ static ssize_t cooling_method_store(struct device *dev,
 }
 static DEVICE_ATTR_RW(cooling_method);
 
+static ssize_t battery_charge_mode_show(struct device *dev,
+					   struct device_attribute *attr,
+					   char *buf)
+{
+	struct toshiba_acpi_dev *toshiba = dev_get_drvdata(dev);
+	int state;
+	int ret;
+
+	ret = toshiba_battery_charge_mode_get(toshiba, &state);
+	if (ret < 0)
+		return ret;
+
+	return sprintf(buf, "%d\n", state);
+}
+
+static ssize_t battery_charge_mode_store(struct device *dev,
+					    struct device_attribute *attr,
+					    const char *buf, size_t count)
+{
+	struct toshiba_acpi_dev *toshiba = dev_get_drvdata(dev);
+	int state;
+	int ret;
+
+	ret = kstrtoint(buf, 0, &state);
+	if (ret)
+		return ret;
+
+	if (state != 0 && state != 1)
+		return -EINVAL;
+
+	ret = toshiba_battery_charge_mode_set(toshiba, state);
+	if (ret)
+		return ret;
+
+	return count;
+}
+static DEVICE_ATTR_RW(battery_charge_mode);
+
 static struct attribute *toshiba_attributes[] = {
 	&dev_attr_version.attr,
 	&dev_attr_fan.attr,
@@ -2350,6 +2453,7 @@ static struct attribute *toshiba_attributes[] = {
 	&dev_attr_panel_power_on.attr,
 	&dev_attr_usb_three.attr,
 	&dev_attr_cooling_method.attr,
+	&dev_attr_battery_charge_mode.attr,
 	NULL,
 };
 
@@ -2384,6 +2488,8 @@ static umode_t toshiba_sysfs_is_visible(struct kobject *kobj,
 		exists = (drv->usb_three_supported) ? true : false;
 	else if (attr == &dev_attr_cooling_method.attr)
 		exists = (drv->cooling_method_supported) ? true : false;
+	else if (attr == &dev_attr_battery_charge_mode.attr)
+		exists = (drv->battery_charge_mode_supported) ? true : false;
 
 	return exists ? attr->mode : 0;
 }
@@ -2959,6 +3065,8 @@ static void print_supported_features(struct toshiba_acpi_dev *dev)
 		pr_cont(" wwan");
 	if (dev->cooling_method_supported)
 		pr_cont(" cooling-method");
+	if (dev->battery_charge_mode_supported)
+		pr_cont(" battery-charge-mode");
 
 	pr_cont("\n");
 }
@@ -3166,6 +3274,8 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev)
 
 	toshiba_cooling_method_available(dev);
 
+	toshiba_battery_charge_mode_available(dev);
+
 	print_supported_features(dev);
 
 	ret = sysfs_create_group(&dev->acpi_dev->dev.kobj,
-- 
2.37.2

Re: [PATCH 0/2] [RFC] platform/x86: Fixes for Toshiba Z830

[Hans de Goede]

Hi Arvid,

Nice to meet you.

On 8/21/22 22:08, Arvid Norlander wrote:
> Hi,
> 
> NOTE: I had some trouble sending this with git send-email, I only managed
> to send one copy successfully! Sorry if I managed to send it multiple
> times.
> 
> I'm new to this kernel development thing, so applogies in advance for any
> mistakes.

No worries, I think you are doing great so far.

Thank you for the detailed analysis and for all the work you are putting
into this.

> I have an old Toshiba Z830-10W laptop. Unfortunately it doesn't
> work well under Linux. Fortunately I spent some time figuring out what was
> wrong. I had documented my findings below. I have also included patches
> (see the next few emails) for some of the issues.
> 
> I would like advice on how to proceed for some of the more advanced
> problems though:
> * Do we want to expose all these features that I figured out? For example,
>   how to set the boot order on this old BIOS-only laptop from user space.
>   Or various other settings accessible via the BIOS.

I'll try to write a short reply to each feature separately,
I think we need to balance the worth of a feature to end users vs the amount
of code to write + maintain here. E.g. adding support for non working
hw buttons is generally considered worth the effort. Adding support for
changing the boot-order not so much.

Note there is a generic interface for changing BIOS options from
within Linux, so if you can change all (or almost all) BIOS options,
you could consider implementing support for:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/ABI/testing/sysfs-class-firmware-attributes

That has been implemented for Lenovo Think* and for some Dell
models, but by the vendors themselves and the used WMI APIs are
actually guaranteed to work on multiple models / generations hardware
making it worth the effort. Also this is something which their
customers want for managed rollout of these devices in big
companies.

IMHO for these Toshiba laptops I still think it is a lot of work
for something which won't see much use.

It would be good though to:
1. Write some generic documentation from an end user pov for toshiba_acpi as:
Documentation/admin-guide/laptops/toshiba_acpi.rst
see:
Documentation/admin-guide/laptops/thinkpad-acpi.rst
as an example

2. Extend the doc from 1. with toshiba_acpi firmware API documentation,
including your findings on protocol bits which we won't directly
implement/use so that this is at least written down in case it is
needed later.

For 2. you can actually just copy and paste a lot of this email,
I believe that having the info in this email in a
Documentation/admin-guide/laptops/toshiba_acpi.rst file
will make it a lot easier to find in the future then only having
it in the mailinglist archives.

> * For the hardware buttons I describe below, is a solution specific to
>   toshiba_acpi preferred, or should additional effort be spent on
>   investigating a generic solution?

Ok, this is interesting there actually is a specification from
Microsoft for this:
http://download.microsoft.com/download/9/c/5/9c5b2167-8017-4bae-9fde-d599bac8184a/dirapplaunch.docx

And there was a previous attempt to add support for the PNP0C32 devices:
https://marc.info/?l=linux-acpi&m=120550727131007
https://lkml.org/lkml/2010/5/28/327

And this even made it into drivers/staging for a while, if you do:
git revert 0be013e3dc2ee79ffab8a438bbb4e216837e3d52
you will get a: drivers/staging/quickstart/quickstart.c file.

Note this is not great code:

1. If you do:
  ls /sys/bus/platform/devices
  You should already see a couple of PNP0C32 platform devices there and the
  driver should simply bind to those rather then creating its own platform device
2. As mentioned this really should use the standard /dev/input/event interface
  for event reporting and allow userspace to change the scancode to EV_KEY*
  mapping. You can do this e.g. by using a sparse_keymap for the scancode to
  EV_KEY* mapping which will give you this for free.

> Before I start coding on these more complex issues I would like advice in
> order to avoid wasting my time on bad designs. In particular, on how to
> proceed with the "Hardware buttons" section below.

I believe that sending the magic command to make these keys generate events
should be part of toshiba_acpi, combined with a generic PNP0C32 driver
for actually reporting the key-presses.



> 
> Best regards,
> Arvid Norlander
> 
> Table of contents
> =================
> 1. Short info on the laptop and methodology
> 2. Background on Toshiba ACPI communication methods
> 3. LED: "Eco" LED ................................ [implemented in patch 1]
> 4. Battery charge mode ........................... [implemented in patch 2]
> 5. Hardware buttons ...................................... [Advice wanted!]
> 6. Panel power control via HCI
> 7. BIOS setting control from the OS ................... [should we bother?]
>    7.1 Setting boot order
>    7.2 Setting USB memory emulation
>    7.3 Display during boot
>    7.4 CPU control
>    7.5 Wake on LAN
>    7.6 SATA power control
>    7.8 Legacy USB
>    7.9 Wake on keyboard
> 8. Other features .... [nothing actionable as of now, for information only]
> 
> 
> 1. Short info on the laptop and methodology
> ===========================================
> 
> The Toshiba Z830-10W laptop is a so-called "Ultrabook" dating from 2011.
> It has BIOS, not UEFI.
> 
> The Toshiba Z830-10W laptop has several features that don't work properly
> under Linux. I have performed reverse engineering by tracing ACPI calls
> with WinDbg using the "AMLI" feature while performing various actions to
> determine what the Windows drivers do. My user space tracing on Windows
> has been limited to determine which programs interact with the driver so
> that I could kill those that I was not testing behaviour of at the moment.
> 
> I have then tested these features using the "acpi_call" kernel module on
> Linux to issue the relevant calls under Linux. In the following sections
> is a feature by feature breakdown.
> 
> 
> 2. Background on Toshiba ACPI communication methods
> ===================================================
> 
> This is a short summary of the general protocol. This is already
> implemented in the toshiba_acpi driver. If you are already familiar with
> that you can skip this section.
> 
> Almost all vendor specific features work via the \_SB_.VALZ ACPI device
> defined in the DSDT. There are a handful of interesting methods on this
> object, but for the purposes of this write-up only "GHCI" is relevant. This
> method takes 6 integer (32-bit) arguments and returns a buffer 6 32-bit
> integers.
> 
> The general format of queries is: {OPERATION, REGISTER, ARG1, ..., ARG4 }.
> The operation is one of HCI_GET/HCI_SET or SCI_GET/SCI_SET (plus SCI_OPEN
> and  SCI_CLOSE). This allows for getting and setting various registers to
> control features or read out data.
> 
> The data returned varies a bit, but is /generally/ on the form:
> {STATUS_CODE, REGISTER_FROM_QUERY, VAL1, ..., VAL4 }
> 
> What is the difference between HCI_* and SCI_* calls? The only important
> difference here is that for SCI_GET/SET you first need to call SCI_OPEN and
> then follow the get or set with an SCI_CLOSE call.
> 
> Much of the rest of this write-up consists of documenting registers
> previously not handled by the toshiba_acpi Linux driver.
> 
> 
> 3. LED: "Eco" LED [implemented in patch 1]
> =================
> 
> The toshiba_acpi driver has support for controlling some LEDs including the
> "Eco" LED. Unfortunately that LED works differently on this laptop.
> 
> The toshiba_acpi driver checks for TOS_INPUT_DATA_ERROR and tries a
> different format. On the Z830 the error returned is TOS_NOT_SUPPORTED
> though the different format still works.

This looks good I'll go and merge it into my for-next git branch
sometime this week (I usually merge patches in batches).

> 4. Battery charge mode [implemented in patch 2]
> ======================
> 
> This laptop supports not charging the battery fully in order to prolong
> battery life. Unlike for example ThinkPads where this control is granular
> here it is just off/on. When off it charges to 100%. When on it charges to
> about 80%.
> 
> According to the Windows program used to control the feature the setting
> will not take effect until the battery has been discharged to around 50%.
> However, in my testing it takes effect as soon as the charge drops below
> 80%. On Windows Toshiba branded this feature as "Eco charging"
> 
> In the following example ACPI calls I will use the following newly defined
> constants:
> #define HCI_BATTERY_CHARGE_MODE 0xba
> #define BATTERY_CHARGE_FULL 0
> #define BATTERY_CHARGE_80_PERCENT 1
> 
> To set the feature:
>   {HCI_SET, HCI_BATTERY_CHARGE_MODE, charge_mode, 0, 0, 0}
> To query for the existence of the feature:
>   {HCI_GET, HCI_BATTERY_CHARGE_MODE, 0, 0, 0, 0}
> To read the feature:
>   {HCI_GET, HCI_BATTERY_CHARGE_MODE, 0, 0, 0, 1}
> 
> The read may need to be retried if TOS_DATA_NOT_AVAILABLE is returned as
> the status code. This rarely happens (I have never observed it on Linux),
> but I have seen it happen under Windows once, and the software did retry
> it.

Hmm, this is interesting if you look at:

Documentation/ABI/testing/sysfs-class-power

You will see there already is a standard API for this in the form of
adding a "charge_control_end_threshold" attribute to the standard
ACPI /sys/class/power_supply/BAT*/ sysfs interface. See e.g.
drivers/platform/x86/thinkpad_acpi.c

For an example of how to add sysfs attributes to a battery
which is managed by the standard drivers/acpi/battery.c driver.

I think you can use this standard attribute enabling eco charging
for any writes with a value <= 90 and disabling it for values
> 90 (90 being halfway between 80 and 100).

While always showing 80 or 100 on read.

You should then also write a patch for:

Documentation/ABI/testing/sysfs-class-power

Adding something like this to the "charge_control_end_threshold"
section:

"not all hardware is capable of setting this to an arbitrary
percentage. Drivers will round written values to the nearest
supported value. Reading back the value will show the actual
threshold set by the driver."

(feel free to copy verbatim, but maybe you can do better)


> 5. Hardware buttons [Advice wanted!]
> ===================
> 
> All the Fn+<key> hotkeys work. However, there are some hardware buttons
> that do not. These buttons are:
> 
> * A button between space and the touchpad to turn off/on the touchpad.
> * Two buttons next to the power button, one is "eco-mode", the other is
> "projector".
> 
> The two buttons next to the power button both send Windows+X by default.
> The touchpad control button does nothing that Linux can detect.
> 
> To enable this functionality several changes are needed.
> 
> The toshiba_acpi driver currently uses
>   {HCI_SET, HCI_HOTKEY_EVENT, HCI_HOTKEY_ENABLE, 0, ...}
> to enable the Fn+<key> hotkeys, where HCI_HOTKEY_ENABLE = 0x09. However on
> this laptop the value 0x05 must be used instead.
> 
> This is not the whole story however, as these keys do not work like any of
> the Fn-hotkeys (ACPI notification on \_SB_.VALZ). Instead, once enabled via
> the above method they start sending notifications on various PNP0C32
> devices. These are currently not handled by Linux. According to a search
> PNP0C32 is "HIDACPI Button Device", so perhaps a generic driver should be
> created.
> 
> The devices in question are:
> PNP0C32 \_SB_.HS81 UID 0x03 = Enable/disable trackpad
> PNP0C32 \_SB_.HS87 UID 0x01 = Eco button
> PNP0C32 \_SB_.HS86 UID 0x02 = Monitor/projector button
> 
> Only the "path" and the "UID" value in the ACPI DSDT tell these devices
> apart.
> 
> The notification always uses the value 0x80.
> 
> I'm not sure what the best approach to implement support for these would
> be.

Discussed above.

> 6. Panel power control via HCI
> ==============================
> 
> The toshiba_acpi driver supports controlling the panel power via SCI calls
> (SCI_PANEL_POWER_ON). Based on the fact that the toshiba_acpi driver
> outputs a message about reboot being needed I assume this is related to
> panel power during boot?
> 
> However there is a HCI call that can turn the panel off or on immediately:
> 
> #define HCI_PANEL_POWER_ON 0x2
> #define PANEL_ON 1
> #define PANEL_OFF 0
> 
> To read/query existence: {HCI_GET, HCI_PANEL_POWER_ON, 0, 0, 0, 0}
> To write: {HCI_SET, HCI_PANEL_POWER_ON, panel_on, 0, 0, 0}
> 
> This could be related to some backlight issues this laptop is having where
> backlight control stops working after a suspend/resume.
> 
> Control via /sys/class/backlight/intel_backlight always works on this
> laptop, however, most desktop environment seems to prefer the acpi_video or
> vendor backlight controls if those exist.
> 
> I have tried acpi_backlight=vendor/native but that is broken in the same
> way. With acpi_backlight=none, the screen never turns on after a resume.
> However if I turn it on using the above HCI call, everything works
> normally after that. And since only the intel_backlight driver is left,
> the desktop environment controls backlight via that method.
>   
> I have yet to find a satisfactory solution to this problem, but I suspect
> the correct solution would be to fix backlight control from acpi_video,
> if that is possible. Suggestions?

I think this is another case of some Toshiba devices needing the
acpi_video backlight level save/restore behavior over suspend/resume
while leaving the actual backlight control to intel_backlight.

Quoting from: drivers/acpi/acpi_video.c :

         * Some machines have a broken acpi-video interface for brightness
         * control, but still need an acpi_video_device_lcd_set_level() call
         * on resume to turn the backlight power on.  We Enable backlight
         * control on these systems, but do not register a backlight sysfs
         * as brightness control does not work.
         */
        {
         /* https://bugzilla.kernel.org/show_bug.cgi?id=21012 */
         .callback = video_disable_backlight_sysfs_if,
         .ident = "Toshiba Portege R700",
         .matches = {
                DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
                DMI_MATCH(DMI_PRODUCT_NAME, "PORTEGE R700"),
                },
        },

Also:
Toshiba Portege R830:    https://bugs.freedesktop.org/show_bug.cgi?id=82634
Toshiba Satellite R830:  https://bugzilla.kernel.org/show_bug.cgi?id=21012

You can see if the same option fixes things for you by adding:
"video.disable_backlight_sysfs_if=1" to your kernel commandline while
removing all other options. If this works, please submit a patch to
add your model to the list in drivers/acpi/acpi_video.c, or provide
dmidecode output, then I can do it for you.

> 7. BIOS setting control from the OS [should we bother?]
> ===================================
> Several of the BIOS settings can be controlled from the OS. This all
> happens via SCI calls. On Windows the "Hwsetup.exe" program offers this
> control. I'm not sure how useful any of this is (as this is already
> available via the BIOS).
> 
> If you think it is a good idea I could absolutely implement support for
> this. Otherwise I might build a simple user space tool on top of acpi_call
> for this. 

As discussed above I don't really think we should bother; and IMHO
certainly not something worth spending time on before the other issues
are wrapped up.

> 7.1 Setting boot order
> ----------------------
> This is a BIOS (not UEFI) laptop, so boot order could normally not be
> controlled from the OS. However here it is possible:
> 
> #define SCI_BOOT_ORDER 0x157
> 
> In this SCI register the boot order is stored as a list with each nibble
> indicating a device:
> #define SCI_BOOT_ORDER_FDD 0x0
> #define SCI_BOOT_ORDER_HDD 0x1
> #define SCI_BOOT_ORDER_LAN 0x3
> #define SCI_BOOT_ORDER_USB_MEMORY 0x4
> #define SCI_BOOT_ORDER_USB_CD 0x7
> #define SCI_BOOT_ORDER_USB_UNUSED 0xf
> 
> These are then combined as follows:
> 
> Set boot order to USB memory, USB CD, HDD, LAN, FDD:
> {SCI_SET, SCI_BOOT_ORDER, 0xfff03174, 0, 0, 0}
> 
> Each nibble indicates a device, with the lowest nibble being the first
> device in the boot order. As this device doesn't have a physical FDD I
> assume that this refers to USB attached devices, but I have not tested this
> (I do have a USB floppy drive if anyone really cares).
> 
> When reading the data out the result is a bit surprising:
>   {0x0, 0x8505, 0xfff30174, 0x5, 0xfff30741, 0x0}
> Presumably these other values also mean something, the boot order in this
> case is USB memory, USB CD, HDD, FDD, LAN, so the third value is the boot
> order.
> 
> I'm not sure what a suitable API for exposing this setting to userspace via
> sysfs would be.
> 
> 7.2 Setting USB memory emulation
> --------------------------------
> The BIOS can either treat USB drives as HDDs or FDDs for booting purposes:
> 
> #define SCI_BOOT_FLOPPY_EMULATION 0x511
> #define SCI_BOOT_FLOPPY_EMULATION_FDD 0x1
> #define SCI_BOOT_FLOPPY_EMULATION_HDD 0x0
> 
> To set: {SCI_SET, SCI_BOOT_FLOPPY_EMULATION, value, 0, 0, 0}
> Getting/existence query: {SCI_GET, SCI_BOOT_FLOPPY_EMULATION, 0, 0, 0, 0}
> 
> 7.3 Display during boot
> -----------------------
> This controls if BIOS/GRUB/etc is shown on just the internal monitor or
> if the montior is automatically selected.
> 
> Note: When changing this in Windows it tells me a restart is required.
> 
> #define SCI_BOOT_DISPLAY 0x300
> #define SCI_BOOT_DISPLAY_INTERNAL 0x1250
> #define SCI_BOOT_DISPLAY_AUTO 0x3250
> 
> To set: {SCI_SET, SCI_BOOT_DISPLAY, value, 0, 0, 0}
> Getting/existence query as usual.
> 
> Note! I have not tested the effects of this, as the only external monitors
> I have that are not in storage use DisplayPort, while this laptop has HDMI
> and VGA. I *do* have an old VGA TFT in storage if anyone cares though...
> 
> 7.4 CPU control
> ---------------
> I presume this is only for operating systems that don't manage this
> themselves, I don't know for sure. The wording in the documentation is
> vague, but I believe it controls CPU frequency behaviour.
> 
> Note: When changing this in Windows it tells me a restart is required.
> 
> #define SCI_CPU_FREQUENCY 0x132
> #define SCI_CPU_FREQUENCY_DYNAMIC 0x0
> #define SCI_CPU_FREQUENCY_HIGH 0x1
> #define SCI_CPU_FREQUENCY_LOW 0x2
> 
> Set and get as usual ({SCI_GET/SET, SCI_CPU_FREQUENCY, value, 0, 0, 0}).
> 
> 7.5 Wake on LAN
> ---------------
> Note! This only controls Wake on LAN when off/hibernated (and since this
> laptop has Intel Rapid Start, presumably in that mode too). It is not
> relevant to WoL when in sleep.
> 
> Here the Windows driver seem to query several possibilities until it hits
> on one that works:
> #define SCI_WAKE_ON_LAN 0x700
> 
> #define SCI_WAKE_ON_LAN_OFF 0x1
> #define SCI_WAKE_ON_LAN_ON 0x1
> 
> #define SCI_WAKE_ON_LAN_REG1 0x0
> #define SCI_WAKE_ON_LAN_REG2 0x1000
> #define SCI_WAKE_ON_LAN_REG3 0x800
> 
> To set:
>   {SCI_SET, SCI_WAKE_ON_LAN, value | register, 0, 0, 0}
> To get/query:
>   {SCI_GET, SCI_WAKE_ON_LAN, register, 0, 0, 0}
> 
> For example on this specific laptop to enable WoL:
>   {SCI_SET, SCI_WAKE_ON_LAN, SCI_WAKE_ON_LAN_ON | SCI_WAKE_ON_LAN_REG3, 0, 0, 0}
> 
> REG1 and REG2 give return code TOS_INPUT_DATA_ERROR on this laptop, but
> presumably they are needed on some laptops, or the Windows program would
> not be attempting to use them.
> 
> 7.6 SATA power control
> ----------------------
> This is another one that I don't know what exactly it corresponds to, maybe
> it is something Linux can control directly:
> 
> #define SCI_SATA_POWER 0x406
> #define SCI_SATA_POWER_BATTERY_LIFE 0x1
> #define SCI_SATA_POWER_PERFORMANCE 0x0
> 
> Get/set/query as expected: {SCI_SET, SCI_SATA_POWER, value, 0, 0, 0}
> 
> 7.8 Legacy USB
> --------------
> Controls Legacy USB support in BIOS.
> 
> Note: When changing this in Windows it tells me a restart is required.
> 
> #define SCI_LEGACY_USB 0x50c
> #define SCI_LEGACY_USB_ENABLED 0x1
> #define SCI_LEGACY_USB_DISABLED 0x0
> 
> 
> Get/set/query as expected: {SCI_SET, SCI_LEGACY_USB, value, 0, 0, 0}
> 
> 7.9 Wake on keyboard
> --------------------
> This controls if pressing a key on the keyboard wakes the laptop from
> sleep. Otherwise, only opening the monitor or pressing the power button
> works for this.
> 
> #define SCI_WAKE_ON_KEYBOARD 0x137
> #define SCI_WAKE_ON_KEYBOARD_ENABLE 0x8
> #define SCI_WAKE_ON_KEYBOARD_DISABLE 0x0
> 
> Set: {SCI_SET, SCI_WAKE_ON_KEYBOARD, value, 0, 0, 0}
> 
> Get/query on the standard form but with slightly weird return values:
>   {TOS_SUCCESS2, 0x800a, value, 0x38, 0x0, 0x0}
> 
> 
> 8. Other features
> =================
> 
> I should note that I did discover some additional registers, but I don't
> fully understand them yet. I have thus not included them here in order to
> not waste your time. However I did write a blog post about this which
> includes that information, which is available should you be interested:
> 
> https://vorpal.se/posts/2022/aug/21/reverse-engineering-acpi-functionality-on-a-toshiba-z830-ultrabook/#other-features_1
> 
> 
> Arvid Norlander (2):
>   platform/x86: Fix ECO LED control on Toshiba Z830
>   platform/x86: Battery charge mode in toshiba_acpi
> 
>  drivers/platform/x86/toshiba_acpi.c | 115 +++++++++++++++++++++++++++-
>  1 file changed, 114 insertions(+), 1 deletion(-)
> 
> 
> base-commit: e3f259d33c0ebae1b6e4922c7cdb50e864c81928


Regards,

Hans

Re: [PATCH 2/2] platform/x86: Battery charge mode in toshiba_acpi

[Hans de Goede]

Hi,

On 8/21/22 22:08, Arvid Norlander wrote:
> Unlike for example ThinkPads where this control is granular here it is
> just off/on. When off it charges to 100%. When on it charges to about 80%.
> 
> Controlling this setting is done via HCI register 0x00ba. Setting to value
> 1 will result in limiting the charing to 80% of the battery capacity,
> while setting it to 0 will allow charging to 100%.
> 
> Reading the current state is a bit weird, and needs a 1 set in the last
> position of the query for whatever reason. In addition, the read may
> return 0x8d20 (Data not available) rarely, so a retry mechanism is needed.
> 
> According to the Windows program used to control the feature the setting
> will not take effect until the battery has been discharged to around 50%.
> However, in my testing it takes effect as soon as the charge drops below
> 80%. On Windows Toshiba branded this feature as "Eco charging".
> 
> Signed-off-by: Arvid Norlander <lkml@vorpal.se>

As mentioned in my reply to the cover-letter it would be better IMHO
to use the standard /sys/class/power_supply/BAT*/charge_control_end_threshold
sysfs attribute for this functionality.

Regards,

Hans


> ---
>  drivers/platform/x86/toshiba_acpi.c | 110 ++++++++++++++++++++++++++++
>  1 file changed, 110 insertions(+)
> 
> diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
> index 6cc617b2940e..2e13f241538a 100644
> --- a/drivers/platform/x86/toshiba_acpi.c
> +++ b/drivers/platform/x86/toshiba_acpi.c
> @@ -112,6 +112,7 @@ MODULE_LICENSE("GPL");
>  #define HCI_KBD_ILLUMINATION		0x0095
>  #define HCI_ECO_MODE			0x0097
>  #define HCI_ACCELEROMETER2		0x00a6
> +#define HCI_BATTERY_CHARGE_MODE		0x00ba
>  #define HCI_SYSTEM_INFO			0xc000
>  #define SCI_PANEL_POWER_ON		0x010d
>  #define SCI_ILLUMINATION		0x014e
> @@ -201,6 +202,7 @@ struct toshiba_acpi_dev {
>  	unsigned int usb_three_supported:1;
>  	unsigned int wwan_supported:1;
>  	unsigned int cooling_method_supported:1;
> +	unsigned int battery_charge_mode_supported:1;
>  	unsigned int sysfs_created:1;
>  	unsigned int special_functions;
>  
> @@ -1285,6 +1287,69 @@ static int toshiba_cooling_method_set(struct toshiba_acpi_dev *dev, u32 state)
>  	return (result == TOS_SUCCESS || result == TOS_SUCCESS2) ? 0 : -EIO;
>  }
>  
> +/* Battery charge control */
> +static void toshiba_battery_charge_mode_available(struct toshiba_acpi_dev *dev)
> +{
> +	u32 in[TCI_WORDS] = { HCI_GET, HCI_BATTERY_CHARGE_MODE, 0, 0, 0, 0 };
> +	u32 out[TCI_WORDS];
> +	acpi_status status;
> +
> +	dev->battery_charge_mode_supported = 0;
> +
> +	status = tci_raw(dev, in, out);
> +	if (ACPI_FAILURE(status)) {
> +		pr_err("ACPI call to get Battery Charge Mode failed\n");
> +		return;
> +	}
> +
> +	if (out[0] != TOS_SUCCESS && out[0] != TOS_SUCCESS2)
> +		return;
> +
> +	dev->battery_charge_mode_supported = 1;
> +}
> +
> +static int toshiba_battery_charge_mode_get(struct toshiba_acpi_dev *dev, u32 *state)
> +{
> +	u32 in[TCI_WORDS] = { HCI_GET, HCI_BATTERY_CHARGE_MODE, 0, 0, 0, 0x1 };
> +	u32 out[TCI_WORDS];
> +	int retries = 3;
> +
> +	do {
> +		acpi_status status = tci_raw(dev, in, out);
> +
> +		if (ACPI_FAILURE(status))
> +			pr_err("ACPI call to get Battery Charge Mode failed\n");
> +		switch (out[0]) {
> +		case TOS_SUCCESS:
> +		case TOS_SUCCESS2:
> +			*state = out[2];
> +			return 0;
> +		case TOS_NOT_SUPPORTED:
> +			return -ENODEV;
> +		case TOS_DATA_NOT_AVAILABLE:
> +			retries--;
> +			break;
> +		default:
> +			return -EIO;
> +		}
> +	} while (retries);
> +
> +	return -EIO;
> +}
> +
> +static int toshiba_battery_charge_mode_set(struct toshiba_acpi_dev *dev, u32 state)
> +{
> +	u32 result = hci_write(dev, HCI_BATTERY_CHARGE_MODE, state);
> +
> +	if (result == TOS_FAILURE)
> +		pr_err("ACPI call to set Battery Charge Mode failed\n");
> +
> +	if (result == TOS_NOT_SUPPORTED)
> +		return -ENODEV;
> +
> +	return (result == TOS_SUCCESS || result == TOS_SUCCESS2) ? 0 : -EIO;
> +}
> +
>  /* Transflective Backlight */
>  static int get_tr_backlight_status(struct toshiba_acpi_dev *dev, u32 *status)
>  {
> @@ -2334,6 +2399,44 @@ static ssize_t cooling_method_store(struct device *dev,
>  }
>  static DEVICE_ATTR_RW(cooling_method);
>  
> +static ssize_t battery_charge_mode_show(struct device *dev,
> +					   struct device_attribute *attr,
> +					   char *buf)
> +{
> +	struct toshiba_acpi_dev *toshiba = dev_get_drvdata(dev);
> +	int state;
> +	int ret;
> +
> +	ret = toshiba_battery_charge_mode_get(toshiba, &state);
> +	if (ret < 0)
> +		return ret;
> +
> +	return sprintf(buf, "%d\n", state);
> +}
> +
> +static ssize_t battery_charge_mode_store(struct device *dev,
> +					    struct device_attribute *attr,
> +					    const char *buf, size_t count)
> +{
> +	struct toshiba_acpi_dev *toshiba = dev_get_drvdata(dev);
> +	int state;
> +	int ret;
> +
> +	ret = kstrtoint(buf, 0, &state);
> +	if (ret)
> +		return ret;
> +
> +	if (state != 0 && state != 1)
> +		return -EINVAL;
> +
> +	ret = toshiba_battery_charge_mode_set(toshiba, state);
> +	if (ret)
> +		return ret;
> +
> +	return count;
> +}
> +static DEVICE_ATTR_RW(battery_charge_mode);
> +
>  static struct attribute *toshiba_attributes[] = {
>  	&dev_attr_version.attr,
>  	&dev_attr_fan.attr,
> @@ -2350,6 +2453,7 @@ static struct attribute *toshiba_attributes[] = {
>  	&dev_attr_panel_power_on.attr,
>  	&dev_attr_usb_three.attr,
>  	&dev_attr_cooling_method.attr,
> +	&dev_attr_battery_charge_mode.attr,
>  	NULL,
>  };
>  
> @@ -2384,6 +2488,8 @@ static umode_t toshiba_sysfs_is_visible(struct kobject *kobj,
>  		exists = (drv->usb_three_supported) ? true : false;
>  	else if (attr == &dev_attr_cooling_method.attr)
>  		exists = (drv->cooling_method_supported) ? true : false;
> +	else if (attr == &dev_attr_battery_charge_mode.attr)
> +		exists = (drv->battery_charge_mode_supported) ? true : false;
>  
>  	return exists ? attr->mode : 0;
>  }
> @@ -2959,6 +3065,8 @@ static void print_supported_features(struct toshiba_acpi_dev *dev)
>  		pr_cont(" wwan");
>  	if (dev->cooling_method_supported)
>  		pr_cont(" cooling-method");
> +	if (dev->battery_charge_mode_supported)
> +		pr_cont(" battery-charge-mode");
>  
>  	pr_cont("\n");
>  }
> @@ -3166,6 +3274,8 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev)
>  
>  	toshiba_cooling_method_available(dev);
>  
> +	toshiba_battery_charge_mode_available(dev);
> +
>  	print_supported_features(dev);
>  
>  	ret = sysfs_create_group(&dev->acpi_dev->dev.kobj,

Re: [PATCH 0/2] [RFC] platform/x86: Fixes for Toshiba Z830

[Arvid Norlander]

On 2022-08-22 13:39, Hans de Goede wrote:
> Hi Arvid,
> 
> Nice to meet you.
> 
> On 8/21/22 22:08, Arvid Norlander wrote:
>> Hi,
>>
>> NOTE: I had some trouble sending this with git send-email, I only managed
>> to send one copy successfully! Sorry if I managed to send it multiple
>> times.
>>
>> I'm new to this kernel development thing, so applogies in advance for any
>> mistakes.
> 
> No worries, I think you are doing great so far.
>
> Thank you for the detailed analysis and for all the work you are putting
> into this.

Thanks for the quick and detailed feedback. I agree with your reasoning,
it seems sound.

However, note that this will likely take me some time, as for me kernel
development is purely done as a hobby and it has to fit in between all my
other hobbies such as hiking, etc. Do not mistake radio silence for that I
have given up, just that I don't have much time.

> 
>> I have an old Toshiba Z830-10W laptop. Unfortunately it doesn't
>> work well under Linux. Fortunately I spent some time figuring out what was
>> wrong. I had documented my findings below. I have also included patches
>> (see the next few emails) for some of the issues.
>>
>> I would like advice on how to proceed for some of the more advanced
>> problems though:
>> * Do we want to expose all these features that I figured out? For example,
>>   how to set the boot order on this old BIOS-only laptop from user space.
>>   Or various other settings accessible via the BIOS.
> 
> I'll try to write a short reply to each feature separately,
> I think we need to balance the worth of a feature to end users vs the amount
> of code to write + maintain here. E.g. adding support for non working
> hw buttons is generally considered worth the effort. Adding support for
> changing the boot-order not so much.
> 
> Note there is a generic interface for changing BIOS options from
> within Linux, so if you can change all (or almost all) BIOS options,
> you could consider implementing support for:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/ABI/testing/sysfs-class-firmware-attributes
> 
> That has been implemented for Lenovo Think* and for some Dell
> models, but by the vendors themselves and the used WMI APIs are
> actually guaranteed to work on multiple models / generations hardware
> making it worth the effort. Also this is something which their
> customers want for managed rollout of these devices in big
> companies.
> 
> IMHO for these Toshiba laptops I still think it is a lot of work
> for something which won't see much use.
> 
> It would be good though to:
> 1. Write some generic documentation from an end user pov for toshiba_acpi as:
> Documentation/admin-guide/laptops/toshiba_acpi.rst
> see:
> Documentation/admin-guide/laptops/thinkpad-acpi.rst
> as an example
>
> 
> 2. Extend the doc from 1. with toshiba_acpi firmware API documentation,
> including your findings on protocol bits which we won't directly
> implement/use so that this is at least written down in case it is
> needed later.

This seems like a good idea indeed.

> For 2. you can actually just copy and paste a lot of this email,
> I believe that having the info in this email in a
> Documentation/admin-guide/laptops/toshiba_acpi.rst file
> will make it a lot easier to find in the future then only having
> it in the mailinglist archives.
> 
>> * For the hardware buttons I describe below, is a solution specific to
>>   toshiba_acpi preferred, or should additional effort be spent on
>>   investigating a generic solution?
> 
> Ok, this is interesting there actually is a specification from
> Microsoft for this:
> http://download.microsoft.com/download/9/c/5/9c5b2167-8017-4bae-9fde-d599bac8184a/dirapplaunch.docx
> 
> And there was a previous attempt to add support for the PNP0C32 devices:
> https://marc.info/?l=linux-acpi&m=120550727131007
> https://lkml.org/lkml/2010/5/28/327
> 
> And this even made it into drivers/staging for a while, if you do:
> git revert 0be013e3dc2ee79ffab8a438bbb4e216837e3d52
> you will get a: drivers/staging/quickstart/quickstart.c file.
> 
> Note this is not great code:
> 
> 1. If you do:
>   ls /sys/bus/platform/devices
>   You should already see a couple of PNP0C32 platform devices there and the
>   driver should simply bind to those rather then creating its own platform device
> 2. As mentioned this really should use the standard /dev/input/event interface
>   for event reporting and allow userspace to change the scancode to EV_KEY*
>   mapping. You can do this e.g. by using a sparse_keymap for the scancode to
>   EV_KEY* mapping which will give you this for free.

I have yet to have time to look at it. However this seems to suggest that
these buttons should work when the laptop is off. That is not the case on
the Z830. They only do anything when the computer is on and I can't find
any settings to change that.

Looking at the specification it also mentions several different
notification codes for the button. The only one used on the Z830 is 0x80.
That is, as far as I can tell from the decompilation of the DSDT.

Thus I worry I will not be able to test any sort of generic implementation
very well, if the Z830 only implements a small subset of the functionality.

>> Before I start coding on these more complex issues I would like advice in
>> order to avoid wasting my time on bad designs. In particular, on how to
>> proceed with the "Hardware buttons" section below.
> 
> I believe that sending the magic command to make these keys generate events
> should be part of toshiba_acpi, combined with a generic PNP0C32 driver
> for actually reporting the key-presses.

I guess there is no way to figure out what the buttons are supposed to mean in
this case, and we simply have to leave that to the user to map as they see fit
(as long as the IDs are stable). I'm not sure how well that works with the event
subsystem, as when I test evtest it seems to asign some sort of labels to the
events (e.g. KEY_SLEEP, KEY_BLUETOOTH, ...).

> [...]
>>
>> 3. LED: "Eco" LED [implemented in patch 1]
>> =================
>>
>> The toshiba_acpi driver has support for controlling some LEDs including the
>> "Eco" LED. Unfortunately that LED works differently on this laptop.
>>
>> The toshiba_acpi driver checks for TOS_INPUT_DATA_ERROR and tries a
>> different format. On the Z830 the error returned is TOS_NOT_SUPPORTED
>> though the different format still works.
> 
> This looks good I'll go and merge it into my for-next git branch
> sometime this week (I usually merge patches in batches).

Awsome!

> 
>> 4. Battery charge mode [implemented in patch 2]
>> ======================
>>
>> This laptop supports not charging the battery fully in order to prolong
>> battery life. Unlike for example ThinkPads where this control is granular
>> here it is just off/on. When off it charges to 100%. When on it charges to
>> about 80%.
>>
>> According to the Windows program used to control the feature the setting
>> will not take effect until the battery has been discharged to around 50%.
>> However, in my testing it takes effect as soon as the charge drops below
>> 80%. On Windows Toshiba branded this feature as "Eco charging"
>>
>> In the following example ACPI calls I will use the following newly defined
>> constants:
>> #define HCI_BATTERY_CHARGE_MODE 0xba
>> #define BATTERY_CHARGE_FULL 0
>> #define BATTERY_CHARGE_80_PERCENT 1
>>
>> To set the feature:
>>   {HCI_SET, HCI_BATTERY_CHARGE_MODE, charge_mode, 0, 0, 0}
>> To query for the existence of the feature:
>>   {HCI_GET, HCI_BATTERY_CHARGE_MODE, 0, 0, 0, 0}
>> To read the feature:
>>   {HCI_GET, HCI_BATTERY_CHARGE_MODE, 0, 0, 0, 1}
>>
>> The read may need to be retried if TOS_DATA_NOT_AVAILABLE is returned as
>> the status code. This rarely happens (I have never observed it on Linux),
>> but I have seen it happen under Windows once, and the software did retry
>> it.
> 
> Hmm, this is interesting if you look at:
> 
> Documentation/ABI/testing/sysfs-class-power
> 
> You will see there already is a standard API for this in the form of
> adding a "charge_control_end_threshold" attribute to the standard
> ACPI /sys/class/power_supply/BAT*/ sysfs interface. See e.g.
> drivers/platform/x86/thinkpad_acpi.c
> 
> For an example of how to add sysfs attributes to a battery
> which is managed by the standard drivers/acpi/battery.c driver.
> 
> I think you can use this standard attribute enabling eco charging
> for any writes with a value <= 90 and disabling it for values
>> 90 (90 being halfway between 80 and 100).
> 
> While always showing 80 or 100 on read.
> 
> You should then also write a patch for:
> 
> Documentation/ABI/testing/sysfs-class-power
> 
> Adding something like this to the "charge_control_end_threshold"
> section:
> 
> "not all hardware is capable of setting this to an arbitrary
> percentage. Drivers will round written values to the nearest
> supported value. Reading back the value will show the actual
> threshold set by the driver."
> 
> (feel free to copy verbatim, but maybe you can do better)
> 
> 

This makes perfect sense, but I don't know if it is guaranteed to be 80%
on all Toshiba laptops. Do you know of any other Toshiba laptops that
have/had this feature, and if so, what the limits are? The Windows driver
for this laptop does not document exactly what the limit is. 80% is simply
what I have observed in practice.

>> 6. Panel power control via HCI
>> ==============================
>>
>> The toshiba_acpi driver supports controlling the panel power via SCI calls
>> (SCI_PANEL_POWER_ON). Based on the fact that the toshiba_acpi driver
>> outputs a message about reboot being needed I assume this is related to
>> panel power during boot?
>>
>> However there is a HCI call that can turn the panel off or on immediately:
>>
>> #define HCI_PANEL_POWER_ON 0x2
>> #define PANEL_ON 1
>> #define PANEL_OFF 0
>>
>> To read/query existence: {HCI_GET, HCI_PANEL_POWER_ON, 0, 0, 0, 0}
>> To write: {HCI_SET, HCI_PANEL_POWER_ON, panel_on, 0, 0, 0}
>>
>> This could be related to some backlight issues this laptop is having where
>> backlight control stops working after a suspend/resume.
>>
>> Control via /sys/class/backlight/intel_backlight always works on this
>> laptop, however, most desktop environment seems to prefer the acpi_video or
>> vendor backlight controls if those exist.
>>
>> I have tried acpi_backlight=vendor/native but that is broken in the same
>> way. With acpi_backlight=none, the screen never turns on after a resume.
>> However if I turn it on using the above HCI call, everything works
>> normally after that. And since only the intel_backlight driver is left,
>> the desktop environment controls backlight via that method.
>>   
>> I have yet to find a satisfactory solution to this problem, but I suspect
>> the correct solution would be to fix backlight control from acpi_video,
>> if that is possible. Suggestions?
> 
> I think this is another case of some Toshiba devices needing the
> acpi_video backlight level save/restore behavior over suspend/resume
> while leaving the actual backlight control to intel_backlight.
> 
> Quoting from: drivers/acpi/acpi_video.c :
> 
>          * Some machines have a broken acpi-video interface for brightness
>          * control, but still need an acpi_video_device_lcd_set_level() call
>          * on resume to turn the backlight power on.  We Enable backlight
>          * control on these systems, but do not register a backlight sysfs
>          * as brightness control does not work.
>          */
>         {
>          /* https://bugzilla.kernel.org/show_bug.cgi?id=21012 */
>          .callback = video_disable_backlight_sysfs_if,
>          .ident = "Toshiba Portege R700",
>          .matches = {
>                 DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
>                 DMI_MATCH(DMI_PRODUCT_NAME, "PORTEGE R700"),
>                 },
>         },
> 
> Also:
> Toshiba Portege R830:    https://bugs.freedesktop.org/show_bug.cgi?id=82634
> Toshiba Satellite R830:  https://bugzilla.kernel.org/show_bug.cgi?id=21012
> 
> You can see if the same option fixes things for you by adding:
> "video.disable_backlight_sysfs_if=1" to your kernel commandline while
> removing all other options. If this works, please submit a patch to
> add your model to the list in drivers/acpi/acpi_video.c, or provide
> dmidecode output, then I can do it for you.

I will test this when I get some time (hopefully at the end of this
weekend, after comming from a multi-day hiking trip).

> 
>> 7. BIOS setting control from the OS [should we bother?]
>> ===================================
>> Several of the BIOS settings can be controlled from the OS. This all
>> happens via SCI calls. On Windows the "Hwsetup.exe" program offers this
>> control. I'm not sure how useful any of this is (as this is already
>> available via the BIOS).
>>
>> If you think it is a good idea I could absolutely implement support for
>> this. Otherwise I might build a simple user space tool on top of acpi_call
>> for this. 
> 
> As discussed above I don't really think we should bother; and IMHO
> certainly not something worth spending time on before the other issues
> are wrapped up.

Makes perfect sense, and kind of what I suspected.

> [...]
> 
> Regards,
> 
> Hans
> 

Re: [PATCH 0/2] [RFC] platform/x86: Fixes for Toshiba Z830

[Azael Avalos]

Hi there

Sorry for pinging in a bit late, been under a lot of work lately.

You can poke the Toshiba BIOS interface directly via /dev/toshiba_acpi
to test your findings, once it is ironed out, you can start making patches
for inclusion in the kernel.

I know there are a lot of areas where the driver is lacking features due to
hardware restrictions on the machines I had at the time, so it's good to
see a bit more movement here.

Cheers
Azael

El mié, 24 ago 2022 a la(s) 06:31, Arvid Norlander (lkml@vorpal.se) escribió:
>
> On 2022-08-22 13:39, Hans de Goede wrote:
> > Hi Arvid,
> >
> > Nice to meet you.
> >
> > On 8/21/22 22:08, Arvid Norlander wrote:
> >> Hi,
> >>
> >> NOTE: I had some trouble sending this with git send-email, I only managed
> >> to send one copy successfully! Sorry if I managed to send it multiple
> >> times.
> >>
> >> I'm new to this kernel development thing, so applogies in advance for any
> >> mistakes.
> >
> > No worries, I think you are doing great so far.
> >
> > Thank you for the detailed analysis and for all the work you are putting
> > into this.
>
> Thanks for the quick and detailed feedback. I agree with your reasoning,
> it seems sound.
>
> However, note that this will likely take me some time, as for me kernel
> development is purely done as a hobby and it has to fit in between all my
> other hobbies such as hiking, etc. Do not mistake radio silence for that I
> have given up, just that I don't have much time.
>
> >
> >> I have an old Toshiba Z830-10W laptop. Unfortunately it doesn't
> >> work well under Linux. Fortunately I spent some time figuring out what was
> >> wrong. I had documented my findings below. I have also included patches
> >> (see the next few emails) for some of the issues.
> >>
> >> I would like advice on how to proceed for some of the more advanced
> >> problems though:
> >> * Do we want to expose all these features that I figured out? For example,
> >>   how to set the boot order on this old BIOS-only laptop from user space.
> >>   Or various other settings accessible via the BIOS.
> >
> > I'll try to write a short reply to each feature separately,
> > I think we need to balance the worth of a feature to end users vs the amount
> > of code to write + maintain here. E.g. adding support for non working
> > hw buttons is generally considered worth the effort. Adding support for
> > changing the boot-order not so much.
> >
> > Note there is a generic interface for changing BIOS options from
> > within Linux, so if you can change all (or almost all) BIOS options,
> > you could consider implementing support for:
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/ABI/testing/sysfs-class-firmware-attributes
> >
> > That has been implemented for Lenovo Think* and for some Dell
> > models, but by the vendors themselves and the used WMI APIs are
> > actually guaranteed to work on multiple models / generations hardware
> > making it worth the effort. Also this is something which their
> > customers want for managed rollout of these devices in big
> > companies.
> >
> > IMHO for these Toshiba laptops I still think it is a lot of work
> > for something which won't see much use.
> >
> > It would be good though to:
> > 1. Write some generic documentation from an end user pov for toshiba_acpi as:
> > Documentation/admin-guide/laptops/toshiba_acpi.rst
> > see:
> > Documentation/admin-guide/laptops/thinkpad-acpi.rst
> > as an example
> >
> >
> > 2. Extend the doc from 1. with toshiba_acpi firmware API documentation,
> > including your findings on protocol bits which we won't directly
> > implement/use so that this is at least written down in case it is
> > needed later.
>
> This seems like a good idea indeed.
>
> > For 2. you can actually just copy and paste a lot of this email,
> > I believe that having the info in this email in a
> > Documentation/admin-guide/laptops/toshiba_acpi.rst file
> > will make it a lot easier to find in the future then only having
> > it in the mailinglist archives.
> >
> >> * For the hardware buttons I describe below, is a solution specific to
> >>   toshiba_acpi preferred, or should additional effort be spent on
> >>   investigating a generic solution?
> >
> > Ok, this is interesting there actually is a specification from
> > Microsoft for this:
> > http://download.microsoft.com/download/9/c/5/9c5b2167-8017-4bae-9fde-d599bac8184a/dirapplaunch.docx
> >
> > And there was a previous attempt to add support for the PNP0C32 devices:
> > https://marc.info/?l=linux-acpi&m=120550727131007
> > https://lkml.org/lkml/2010/5/28/327
> >
> > And this even made it into drivers/staging for a while, if you do:
> > git revert 0be013e3dc2ee79ffab8a438bbb4e216837e3d52
> > you will get a: drivers/staging/quickstart/quickstart.c file.
> >
> > Note this is not great code:
> >
> > 1. If you do:
> >   ls /sys/bus/platform/devices
> >   You should already see a couple of PNP0C32 platform devices there and the
> >   driver should simply bind to those rather then creating its own platform device
> > 2. As mentioned this really should use the standard /dev/input/event interface
> >   for event reporting and allow userspace to change the scancode to EV_KEY*
> >   mapping. You can do this e.g. by using a sparse_keymap for the scancode to
> >   EV_KEY* mapping which will give you this for free.
>
> I have yet to have time to look at it. However this seems to suggest that
> these buttons should work when the laptop is off. That is not the case on
> the Z830. They only do anything when the computer is on and I can't find
> any settings to change that.
>
> Looking at the specification it also mentions several different
> notification codes for the button. The only one used on the Z830 is 0x80.
> That is, as far as I can tell from the decompilation of the DSDT.
>
> Thus I worry I will not be able to test any sort of generic implementation
> very well, if the Z830 only implements a small subset of the functionality.
>
> >> Before I start coding on these more complex issues I would like advice in
> >> order to avoid wasting my time on bad designs. In particular, on how to
> >> proceed with the "Hardware buttons" section below.
> >
> > I believe that sending the magic command to make these keys generate events
> > should be part of toshiba_acpi, combined with a generic PNP0C32 driver
> > for actually reporting the key-presses.
>
> I guess there is no way to figure out what the buttons are supposed to mean in
> this case, and we simply have to leave that to the user to map as they see fit
> (as long as the IDs are stable). I'm not sure how well that works with the event
> subsystem, as when I test evtest it seems to asign some sort of labels to the
> events (e.g. KEY_SLEEP, KEY_BLUETOOTH, ...).
>
> > [...]
> >>
> >> 3. LED: "Eco" LED [implemented in patch 1]
> >> =================
> >>
> >> The toshiba_acpi driver has support for controlling some LEDs including the
> >> "Eco" LED. Unfortunately that LED works differently on this laptop.
> >>
> >> The toshiba_acpi driver checks for TOS_INPUT_DATA_ERROR and tries a
> >> different format. On the Z830 the error returned is TOS_NOT_SUPPORTED
> >> though the different format still works.
> >
> > This looks good I'll go and merge it into my for-next git branch
> > sometime this week (I usually merge patches in batches).
>
> Awsome!
>
> >
> >> 4. Battery charge mode [implemented in patch 2]
> >> ======================
> >>
> >> This laptop supports not charging the battery fully in order to prolong
> >> battery life. Unlike for example ThinkPads where this control is granular
> >> here it is just off/on. When off it charges to 100%. When on it charges to
> >> about 80%.
> >>
> >> According to the Windows program used to control the feature the setting
> >> will not take effect until the battery has been discharged to around 50%.
> >> However, in my testing it takes effect as soon as the charge drops below
> >> 80%. On Windows Toshiba branded this feature as "Eco charging"
> >>
> >> In the following example ACPI calls I will use the following newly defined
> >> constants:
> >> #define HCI_BATTERY_CHARGE_MODE 0xba
> >> #define BATTERY_CHARGE_FULL 0
> >> #define BATTERY_CHARGE_80_PERCENT 1
> >>
> >> To set the feature:
> >>   {HCI_SET, HCI_BATTERY_CHARGE_MODE, charge_mode, 0, 0, 0}
> >> To query for the existence of the feature:
> >>   {HCI_GET, HCI_BATTERY_CHARGE_MODE, 0, 0, 0, 0}
> >> To read the feature:
> >>   {HCI_GET, HCI_BATTERY_CHARGE_MODE, 0, 0, 0, 1}
> >>
> >> The read may need to be retried if TOS_DATA_NOT_AVAILABLE is returned as
> >> the status code. This rarely happens (I have never observed it on Linux),
> >> but I have seen it happen under Windows once, and the software did retry
> >> it.
> >
> > Hmm, this is interesting if you look at:
> >
> > Documentation/ABI/testing/sysfs-class-power
> >
> > You will see there already is a standard API for this in the form of
> > adding a "charge_control_end_threshold" attribute to the standard
> > ACPI /sys/class/power_supply/BAT*/ sysfs interface. See e.g.
> > drivers/platform/x86/thinkpad_acpi.c
> >
> > For an example of how to add sysfs attributes to a battery
> > which is managed by the standard drivers/acpi/battery.c driver.
> >
> > I think you can use this standard attribute enabling eco charging
> > for any writes with a value <= 90 and disabling it for values
> >> 90 (90 being halfway between 80 and 100).
> >
> > While always showing 80 or 100 on read.
> >
> > You should then also write a patch for:
> >
> > Documentation/ABI/testing/sysfs-class-power
> >
> > Adding something like this to the "charge_control_end_threshold"
> > section:
> >
> > "not all hardware is capable of setting this to an arbitrary
> > percentage. Drivers will round written values to the nearest
> > supported value. Reading back the value will show the actual
> > threshold set by the driver."
> >
> > (feel free to copy verbatim, but maybe you can do better)
> >
> >
>
> This makes perfect sense, but I don't know if it is guaranteed to be 80%
> on all Toshiba laptops. Do you know of any other Toshiba laptops that
> have/had this feature, and if so, what the limits are? The Windows driver
> for this laptop does not document exactly what the limit is. 80% is simply
> what I have observed in practice.
>
> >> 6. Panel power control via HCI
> >> ==============================
> >>
> >> The toshiba_acpi driver supports controlling the panel power via SCI calls
> >> (SCI_PANEL_POWER_ON). Based on the fact that the toshiba_acpi driver
> >> outputs a message about reboot being needed I assume this is related to
> >> panel power during boot?
> >>
> >> However there is a HCI call that can turn the panel off or on immediately:
> >>
> >> #define HCI_PANEL_POWER_ON 0x2
> >> #define PANEL_ON 1
> >> #define PANEL_OFF 0
> >>
> >> To read/query existence: {HCI_GET, HCI_PANEL_POWER_ON, 0, 0, 0, 0}
> >> To write: {HCI_SET, HCI_PANEL_POWER_ON, panel_on, 0, 0, 0}
> >>
> >> This could be related to some backlight issues this laptop is having where
> >> backlight control stops working after a suspend/resume.
> >>
> >> Control via /sys/class/backlight/intel_backlight always works on this
> >> laptop, however, most desktop environment seems to prefer the acpi_video or
> >> vendor backlight controls if those exist.
> >>
> >> I have tried acpi_backlight=vendor/native but that is broken in the same
> >> way. With acpi_backlight=none, the screen never turns on after a resume.
> >> However if I turn it on using the above HCI call, everything works
> >> normally after that. And since only the intel_backlight driver is left,
> >> the desktop environment controls backlight via that method.
> >>
> >> I have yet to find a satisfactory solution to this problem, but I suspect
> >> the correct solution would be to fix backlight control from acpi_video,
> >> if that is possible. Suggestions?
> >
> > I think this is another case of some Toshiba devices needing the
> > acpi_video backlight level save/restore behavior over suspend/resume
> > while leaving the actual backlight control to intel_backlight.
> >
> > Quoting from: drivers/acpi/acpi_video.c :
> >
> >          * Some machines have a broken acpi-video interface for brightness
> >          * control, but still need an acpi_video_device_lcd_set_level() call
> >          * on resume to turn the backlight power on.  We Enable backlight
> >          * control on these systems, but do not register a backlight sysfs
> >          * as brightness control does not work.
> >          */
> >         {
> >          /* https://bugzilla.kernel.org/show_bug.cgi?id=21012 */
> >          .callback = video_disable_backlight_sysfs_if,
> >          .ident = "Toshiba Portege R700",
> >          .matches = {
> >                 DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
> >                 DMI_MATCH(DMI_PRODUCT_NAME, "PORTEGE R700"),
> >                 },
> >         },
> >
> > Also:
> > Toshiba Portege R830:    https://bugs.freedesktop.org/show_bug.cgi?id=82634
> > Toshiba Satellite R830:  https://bugzilla.kernel.org/show_bug.cgi?id=21012
> >
> > You can see if the same option fixes things for you by adding:
> > "video.disable_backlight_sysfs_if=1" to your kernel commandline while
> > removing all other options. If this works, please submit a patch to
> > add your model to the list in drivers/acpi/acpi_video.c, or provide
> > dmidecode output, then I can do it for you.
>
> I will test this when I get some time (hopefully at the end of this
> weekend, after comming from a multi-day hiking trip).
>
> >
> >> 7. BIOS setting control from the OS [should we bother?]
> >> ===================================
> >> Several of the BIOS settings can be controlled from the OS. This all
> >> happens via SCI calls. On Windows the "Hwsetup.exe" program offers this
> >> control. I'm not sure how useful any of this is (as this is already
> >> available via the BIOS).
> >>
> >> If you think it is a good idea I could absolutely implement support for
> >> this. Otherwise I might build a simple user space tool on top of acpi_call
> >> for this.
> >
> > As discussed above I don't really think we should bother; and IMHO
> > certainly not something worth spending time on before the other issues
> > are wrapped up.
>
> Makes perfect sense, and kind of what I suspected.
>
> > [...]
> >
> > Regards,
> >
> > Hans
> >
>


-- 
-- El mundo apesta y vosotros apestais tambien --

Re: [PATCH 1/2] platform/x86: Fix ECO LED control on Toshiba Z830

[Hans de Goede]

Hi,

On 8/21/22 22:08, Arvid Norlander wrote:
> The toshiba_acpi driver checks for TOS_INPUT_DATA_ERROR and tries a
> different format. On the Z830 the error returned is TOS_NOT_SUPPORTED
> though the different format still works. Allow either error.
> 
> Signed-off-by: Arvid Norlander <lkml@vorpal.se>

Thank you for your patch, I've applied this patch to my review-hans 
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.

Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.

Regards,

Hans


> ---
>  drivers/platform/x86/toshiba_acpi.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
> index 0fc9e8b8827b..6cc617b2940e 100644
> --- a/drivers/platform/x86/toshiba_acpi.c
> +++ b/drivers/platform/x86/toshiba_acpi.c
> @@ -675,12 +675,15 @@ static void toshiba_eco_mode_available(struct toshiba_acpi_dev *dev)
>  		return;
>  	}
>  
> -	if (out[0] == TOS_INPUT_DATA_ERROR) {
> +	if (out[0] == TOS_INPUT_DATA_ERROR || out[0] == TOS_NOT_SUPPORTED) {
>  		/*
>  		 * If we receive 0x8300 (Input Data Error), it means that the
>  		 * LED device is present, but that we just screwed the input
>  		 * parameters.
>  		 *
> +		 * On some laptops 0x8000 (Not supported) is also returned in
> +		 * this case, so we need to allow for that as well.
> +		 *
>  		 * Let's query the status of the LED to see if we really have a
>  		 * success response, indicating the actual presense of the LED,
>  		 * bail out otherwise.

Re: [PATCH 0/2] [RFC] platform/x86: Fixes for Toshiba Z830

[Hans de Goede]

Hi,

On 8/24/22 14:31, Arvid Norlander wrote:
> On 2022-08-22 13:39, Hans de Goede wrote:

<snip>

>> For 2. you can actually just copy and paste a lot of this email,
>> I believe that having the info in this email in a
>> Documentation/admin-guide/laptops/toshiba_acpi.rst file
>> will make it a lot easier to find in the future then only having
>> it in the mailinglist archives.
>>
>>> * For the hardware buttons I describe below, is a solution specific to
>>>   toshiba_acpi preferred, or should additional effort be spent on
>>>   investigating a generic solution?
>>
>> Ok, this is interesting there actually is a specification from
>> Microsoft for this:
>> http://download.microsoft.com/download/9/c/5/9c5b2167-8017-4bae-9fde-d599bac8184a/dirapplaunch.docx
>>
>> And there was a previous attempt to add support for the PNP0C32 devices:
>> https://marc.info/?l=linux-acpi&m=120550727131007
>> https://lkml.org/lkml/2010/5/28/327
>>
>> And this even made it into drivers/staging for a while, if you do:
>> git revert 0be013e3dc2ee79ffab8a438bbb4e216837e3d52
>> you will get a: drivers/staging/quickstart/quickstart.c file.
>>
>> Note this is not great code:
>>
>> 1. If you do:
>>   ls /sys/bus/platform/devices
>>   You should already see a couple of PNP0C32 platform devices there and the
>>   driver should simply bind to those rather then creating its own platform device
>> 2. As mentioned this really should use the standard /dev/input/event interface
>>   for event reporting and allow userspace to change the scancode to EV_KEY*
>>   mapping. You can do this e.g. by using a sparse_keymap for the scancode to
>>   EV_KEY* mapping which will give you this for free.
> 
> I have yet to have time to look at it. However this seems to suggest that
> these buttons should work when the laptop is off. That is not the case on
> the Z830. They only do anything when the computer is on and I can't find
> any settings to change that.

Not necessarily fully off, but maybe when suspended ?

> Looking at the specification it also mentions several different
> notification codes for the button. The only one used on the Z830 is 0x80.
> That is, as far as I can tell from the decompilation of the DSDT.
> 
> Thus I worry I will not be able to test any sort of generic implementation
> very well, if the Z830 only implements a small subset of the functionality.

Right I understand still I think there should be a separate

drivers/platform/x86/acpi_pnp0c32_buttons.c 

driver for this IMHO. If it is only tested on your one model that
is fine (should be documented with a comment in the code though).

Then at least we have something to serve as a basis for if people
want to add support for this on more laptop models.

Does that sound reasonable ?

>>> Before I start coding on these more complex issues I would like advice in
>>> order to avoid wasting my time on bad designs. In particular, on how to
>>> proceed with the "Hardware buttons" section below.
>>
>> I believe that sending the magic command to make these keys generate events
>> should be part of toshiba_acpi, combined with a generic PNP0C32 driver
>> for actually reporting the key-presses.
> 
> I guess there is no way to figure out what the buttons are supposed to mean in
> this case, and we simply have to leave that to the user to map as they see fit
> (as long as the IDs are stable). I'm not sure how well that works with the event
> subsystem, as when I test evtest it seems to asign some sort of labels to the
> events (e.g. KEY_SLEEP, KEY_BLUETOOTH, ...).

Ack.

<snip>

>>> 4. Battery charge mode [implemented in patch 2]
>>> ======================
>>>
>>> This laptop supports not charging the battery fully in order to prolong
>>> battery life. Unlike for example ThinkPads where this control is granular
>>> here it is just off/on. When off it charges to 100%. When on it charges to
>>> about 80%.
>>>
>>> According to the Windows program used to control the feature the setting
>>> will not take effect until the battery has been discharged to around 50%.
>>> However, in my testing it takes effect as soon as the charge drops below
>>> 80%. On Windows Toshiba branded this feature as "Eco charging"
>>>
>>> In the following example ACPI calls I will use the following newly defined
>>> constants:
>>> #define HCI_BATTERY_CHARGE_MODE 0xba
>>> #define BATTERY_CHARGE_FULL 0
>>> #define BATTERY_CHARGE_80_PERCENT 1
>>>
>>> To set the feature:
>>>   {HCI_SET, HCI_BATTERY_CHARGE_MODE, charge_mode, 0, 0, 0}
>>> To query for the existence of the feature:
>>>   {HCI_GET, HCI_BATTERY_CHARGE_MODE, 0, 0, 0, 0}
>>> To read the feature:
>>>   {HCI_GET, HCI_BATTERY_CHARGE_MODE, 0, 0, 0, 1}
>>>
>>> The read may need to be retried if TOS_DATA_NOT_AVAILABLE is returned as
>>> the status code. This rarely happens (I have never observed it on Linux),
>>> but I have seen it happen under Windows once, and the software did retry
>>> it.
>>
>> Hmm, this is interesting if you look at:
>>
>> Documentation/ABI/testing/sysfs-class-power
>>
>> You will see there already is a standard API for this in the form of
>> adding a "charge_control_end_threshold" attribute to the standard
>> ACPI /sys/class/power_supply/BAT*/ sysfs interface. See e.g.
>> drivers/platform/x86/thinkpad_acpi.c
>>
>> For an example of how to add sysfs attributes to a battery
>> which is managed by the standard drivers/acpi/battery.c driver.
>>
>> I think you can use this standard attribute enabling eco charging
>> for any writes with a value <= 90 and disabling it for values
>>> 90 (90 being halfway between 80 and 100).
>>
>> While always showing 80 or 100 on read.
>>
>> You should then also write a patch for:
>>
>> Documentation/ABI/testing/sysfs-class-power
>>
>> Adding something like this to the "charge_control_end_threshold"
>> section:
>>
>> "not all hardware is capable of setting this to an arbitrary
>> percentage. Drivers will round written values to the nearest
>> supported value. Reading back the value will show the actual
>> threshold set by the driver."
>>
>> (feel free to copy verbatim, but maybe you can do better)
>>
>>
> 
> This makes perfect sense, but I don't know if it is guaranteed to be 80%
> on all Toshiba laptops. Do you know of any other Toshiba laptops that
> have/had this feature, and if so, what the limits are? The Windows driver
> for this laptop does not document exactly what the limit is. 80% is simply
> what I have observed in practice.

Right, the idea is to document that the hw/fw/driver may only
support some fixed values and that written values will be
rounded to one of the supported fixed values. There is no need
to document what those fixed values are.  The idea is that
userspace consumers will read back the value to see what
they actually got. 

Regards,

Hans

Re: [PATCH 0/2] [RFC] platform/x86: Fixes for Toshiba Z830

[Arvid Norlander]

On 2022-08-26 14:07, Hans de Goede wrote:
> Hi,
> 
> On 8/24/22 14:31, Arvid Norlander wrote:
>> On 2022-08-22 13:39, Hans de Goede wrote:
> 
> <snip>
> 
>>> For 2. you can actually just copy and paste a lot of this email,
>>> I believe that having the info in this email in a
>>> Documentation/admin-guide/laptops/toshiba_acpi.rst file
>>> will make it a lot easier to find in the future then only having
>>> it in the mailinglist archives.
>>>
>>>> * For the hardware buttons I describe below, is a solution specific to
>>>>   toshiba_acpi preferred, or should additional effort be spent on
>>>>   investigating a generic solution?
>>>
>>> Ok, this is interesting there actually is a specification from
>>> Microsoft for this:
>>> http://download.microsoft.com/download/9/c/5/9c5b2167-8017-4bae-9fde-d599bac8184a/dirapplaunch.docx
>>>
>>> And there was a previous attempt to add support for the PNP0C32 devices:
>>> https://marc.info/?l=linux-acpi&m=120550727131007
>>> https://lkml.org/lkml/2010/5/28/327
>>>
>>> And this even made it into drivers/staging for a while, if you do:
>>> git revert 0be013e3dc2ee79ffab8a438bbb4e216837e3d52
>>> you will get a: drivers/staging/quickstart/quickstart.c file.
>>>
>>> Note this is not great code:
>>>
>>> 1. If you do:
>>>   ls /sys/bus/platform/devices
>>>   You should already see a couple of PNP0C32 platform devices there and the
>>>   driver should simply bind to those rather then creating its own platform device
>>> 2. As mentioned this really should use the standard /dev/input/event interface
>>>   for event reporting and allow userspace to change the scancode to EV_KEY*
>>>   mapping. You can do this e.g. by using a sparse_keymap for the scancode to
>>>   EV_KEY* mapping which will give you this for free.
>>
>> I have yet to have time to look at it. However this seems to suggest that
>> these buttons should work when the laptop is off. That is not the case on
>> the Z830. They only do anything when the computer is on and I can't find
>> any settings to change that.
> 
> Not necessarily fully off, but maybe when suspended ?

Tested it. Nope. In fact there is no code in the DSDT to handle the wakeup
case. Or rather, only some partial lines of code are left over from that.
That functionality is definitely non-functional on this laptop.

In addition the Microsoft specification lists _PRW as being a required
method. This is missing. Only _STA (which does something funky based on
variables set based on _OSI, haven't bothered figuring that out yet), _HID,
_UID and GHID exist as methods on the button objects.

> 
>> Looking at the specification it also mentions several different
>> notification codes for the button. The only one used on the Z830 is 0x80.
>> That is, as far as I can tell from the decompilation of the DSDT.
>>
>> Thus I worry I will not be able to test any sort of generic implementation
>> very well, if the Z830 only implements a small subset of the functionality.
> 
> Right I understand still I think there should be a separate
> 
> drivers/platform/x86/acpi_pnp0c32_buttons.c 
> 
> driver for this IMHO. If it is only tested on your one model that
> is fine (should be documented with a comment in the code though).
> 
> Then at least we have something to serve as a basis for if people
> want to add support for this on more laptop models.
> 
> Does that sound reasonable ?

Sure, we may have talked past each other, as this is what I also believe I
suggested. I just don't see how I can possibly implement the wakeup
handling part of this, as no laptop I own has that. Unless you know some
else who has a laptop that would allow testing that part.

Would a sensible option be to only implement support for key presses while
the laptop is awake? If someone comes along with a laptop that has the
support for these buttons waking from sleep they can add that missing
functionality at that point. (It seems rather unlikely that will happen
though: I get the feeling that this type of button never became a hit and
is mostly a forgotten relic of the past. And if no one came along and
bothered to add support in the decade+ since it was introduced, it was
probably quite rare back then as well.)

<snip>

>>>> 4. Battery charge mode [implemented in patch 2]
>>>> ======================
>>>>
>>>> This laptop supports not charging the battery fully in order to prolong
>>>> battery life. Unlike for example ThinkPads where this control is granular
>>>> here it is just off/on. When off it charges to 100%. When on it charges to
>>>> about 80%.
>>>>
>>>> According to the Windows program used to control the feature the setting
>>>> will not take effect until the battery has been discharged to around 50%.
>>>> However, in my testing it takes effect as soon as the charge drops below
>>>> 80%. On Windows Toshiba branded this feature as "Eco charging"
>>>>
>>>> In the following example ACPI calls I will use the following newly defined
>>>> constants:
>>>> #define HCI_BATTERY_CHARGE_MODE 0xba
>>>> #define BATTERY_CHARGE_FULL 0
>>>> #define BATTERY_CHARGE_80_PERCENT 1
>>>>
>>>> To set the feature:
>>>>   {HCI_SET, HCI_BATTERY_CHARGE_MODE, charge_mode, 0, 0, 0}
>>>> To query for the existence of the feature:
>>>>   {HCI_GET, HCI_BATTERY_CHARGE_MODE, 0, 0, 0, 0}
>>>> To read the feature:
>>>>   {HCI_GET, HCI_BATTERY_CHARGE_MODE, 0, 0, 0, 1}
>>>>
>>>> The read may need to be retried if TOS_DATA_NOT_AVAILABLE is returned as
>>>> the status code. This rarely happens (I have never observed it on Linux),
>>>> but I have seen it happen under Windows once, and the software did retry
>>>> it.
>>>
>>> Hmm, this is interesting if you look at:
>>>
>>> Documentation/ABI/testing/sysfs-class-power
>>>
>>> You will see there already is a standard API for this in the form of
>>> adding a "charge_control_end_threshold" attribute to the standard
>>> ACPI /sys/class/power_supply/BAT*/ sysfs interface. See e.g.
>>> drivers/platform/x86/thinkpad_acpi.c
>>>
>>> For an example of how to add sysfs attributes to a battery
>>> which is managed by the standard drivers/acpi/battery.c driver.
>>>
>>> I think you can use this standard attribute enabling eco charging
>>> for any writes with a value <= 90 and disabling it for values
>>>> 90 (90 being halfway between 80 and 100).
>>>
>>> While always showing 80 or 100 on read.
>>>
>>> You should then also write a patch for:
>>>
>>> Documentation/ABI/testing/sysfs-class-power
>>>
>>> Adding something like this to the "charge_control_end_threshold"
>>> section:
>>>
>>> "not all hardware is capable of setting this to an arbitrary
>>> percentage. Drivers will round written values to the nearest
>>> supported value. Reading back the value will show the actual
>>> threshold set by the driver."
>>>
>>> (feel free to copy verbatim, but maybe you can do better)
>>>
>>>
>>
>> This makes perfect sense, but I don't know if it is guaranteed to be 80%
>> on all Toshiba laptops. Do you know of any other Toshiba laptops that
>> have/had this feature, and if so, what the limits are? The Windows driver
>> for this laptop does not document exactly what the limit is. 80% is simply
>> what I have observed in practice.
> 
> Right, the idea is to document that the hw/fw/driver may only
> support some fixed values and that written values will be
> rounded to one of the supported fixed values. There is no need
> to document what those fixed values are.  The idea is that
> userspace consumers will read back the value to see what
> they actually got.

I think we might be slightly talking past each other here. I absolutely
agree with your idea. My only worry is that toshiba_acpi returning 80%
might not be the right choice. This could be model dependent.

Since I only have a sample size of one, it could even depend on the current
condition of the battery for all I know (though that is probably not
likely). The Windows software and the manual do not specify any sort of
percentage. It is just documented as a mode that prolongs battery life
while reducing full charge basically. Without putting any qualifiers on
"how much".

An option would be to return 80% for the Z830, and the string "unknown" for
other models. Though I guess there is a risk of breaking user space
software that only expects numeric values, so it may be a no-go.

I guess the user space software using this interface consists of *at least*
"tlp" and whatever KDE uses to handle it ("powerdevil" I belive?).

> 
> Regards,
> 
> Hans
> 

Best regards,
Arvid Norlander

Re: [PATCH 0/2] [RFC] platform/x86: Fixes for Toshiba Z830

[Arvid Norlander]

Hi,

On 2022-08-25 19:00, Azael Avalos wrote:
> Hi there
> 
> Sorry for pinging in a bit late, been under a lot of work lately.

It happens to all of us.

> You can poke the Toshiba BIOS interface directly via /dev/toshiba_acpi
> to test your findings, once it is ironed out, you can start making patches
> for inclusion in the kernel.

Interesting. I'm new to kernel development and I can't find where in
toshiba_acpi.c this is implemented. Nor do I see any documentation for this
interface. I would love some hints with regards to this.

For now I have been using the out-of-tree acpi_call module:

https://github.com/nix-community/acpi_call

Arch Linux (which I use) packages it as a DKMS module. Handy to test with,
but probably really easy to screw up your system using it if you don't know
what you are doing.

> I know there are a lot of areas where the driver is lacking features due to
> hardware restrictions on the machines I had at the time, so it's good to
> see a bit more movement here.
> 
> Cheers
> Azael
> 

Best regards,
Arvid Norlander

<snip>

Re: [PATCH 0/2] [RFC] platform/x86: Fixes for Toshiba Z830

[Azael Avalos]

Hi there

El sáb, 27 ago 2022 a la(s) 05:51, Arvid Norlander (lkml@vorpal.se) escribió:
>
> Hi,
>
> On 2022-08-25 19:00, Azael Avalos wrote:
> > Hi there
> >
> > Sorry for pinging in a bit late, been under a lot of work lately.
>
> It happens to all of us.
>
> > You can poke the Toshiba BIOS interface directly via /dev/toshiba_acpi
> > to test your findings, once it is ironed out, you can start making patches
> > for inclusion in the kernel.
>
> Interesting. I'm new to kernel development and I can't find where in
> toshiba_acpi.c this is implemented. Nor do I see any documentation for this
> interface. I would love some hints with regards to this.
>

From line 2248 onwards

Here's a link to some documents you might find interesting:
http://www.buzzard.me.uk/toshiba/docs.html

The interface was introduced to the kernel many years ago, back when
the (char) toshiba module
was used, i just added support to the toshiba_acpi via ACPI calls

> For now I have been using the out-of-tree acpi_call module:
>
> https://github.com/nix-community/acpi_call
>
> Arch Linux (which I use) packages it as a DKMS module. Handy to test with,
> but probably really easy to screw up your system using it if you don't know
> what you are doing.

The interface is pretty straight forward, you can make a small C prog to test,
basically you just need to:
- Open the (/dev/toshiba_acpi) device for R/W (make sure you have permissions)
- Fill out the SMMRegisters struct with your query and get the results
- Rinse and repeat till you find what you're looking for

WARNING: This might (or might not) set your house on fire...

>
> > I know there are a lot of areas where the driver is lacking features due to
> > hardware restrictions on the machines I had at the time, so it's good to
> > see a bit more movement here.
> >
> > Cheers
> > Azael
> >
>
> Best regards,
> Arvid Norlander
>
> <snip>

Saludos
Azael



-- 
-- El mundo apesta y vosotros apestais tambien --

Re: [PATCH 0/2] [RFC] platform/x86: Fixes for Toshiba Z830

[Hans de Goede]

Hi,

On 8/27/22 13:42, Arvid Norlander wrote:
> On 2022-08-26 14:07, Hans de Goede wrote:
>> Hi,
>>
>> On 8/24/22 14:31, Arvid Norlander wrote:
>>> On 2022-08-22 13:39, Hans de Goede wrote:
>>
>> <snip>
>>
>>>> For 2. you can actually just copy and paste a lot of this email,
>>>> I believe that having the info in this email in a
>>>> Documentation/admin-guide/laptops/toshiba_acpi.rst file
>>>> will make it a lot easier to find in the future then only having
>>>> it in the mailinglist archives.
>>>>
>>>>> * For the hardware buttons I describe below, is a solution specific to
>>>>>   toshiba_acpi preferred, or should additional effort be spent on
>>>>>   investigating a generic solution?
>>>>
>>>> Ok, this is interesting there actually is a specification from
>>>> Microsoft for this:
>>>> http://download.microsoft.com/download/9/c/5/9c5b2167-8017-4bae-9fde-d599bac8184a/dirapplaunch.docx
>>>>
>>>> And there was a previous attempt to add support for the PNP0C32 devices:
>>>> https://marc.info/?l=linux-acpi&m=120550727131007
>>>> https://lkml.org/lkml/2010/5/28/327
>>>>
>>>> And this even made it into drivers/staging for a while, if you do:
>>>> git revert 0be013e3dc2ee79ffab8a438bbb4e216837e3d52
>>>> you will get a: drivers/staging/quickstart/quickstart.c file.
>>>>
>>>> Note this is not great code:
>>>>
>>>> 1. If you do:
>>>>   ls /sys/bus/platform/devices
>>>>   You should already see a couple of PNP0C32 platform devices there and the
>>>>   driver should simply bind to those rather then creating its own platform device
>>>> 2. As mentioned this really should use the standard /dev/input/event interface
>>>>   for event reporting and allow userspace to change the scancode to EV_KEY*
>>>>   mapping. You can do this e.g. by using a sparse_keymap for the scancode to
>>>>   EV_KEY* mapping which will give you this for free.
>>>
>>> I have yet to have time to look at it. However this seems to suggest that
>>> these buttons should work when the laptop is off. That is not the case on
>>> the Z830. They only do anything when the computer is on and I can't find
>>> any settings to change that.
>>
>> Not necessarily fully off, but maybe when suspended ?
> 
> Tested it. Nope. In fact there is no code in the DSDT to handle the wakeup
> case. Or rather, only some partial lines of code are left over from that.
> That functionality is definitely non-functional on this laptop.
> 
> In addition the Microsoft specification lists _PRW as being a required
> method. This is missing. Only _STA (which does something funky based on
> variables set based on _OSI, haven't bothered figuring that out yet), _HID,
> _UID and GHID exist as methods on the button objects.
> 
>>
>>> Looking at the specification it also mentions several different
>>> notification codes for the button. The only one used on the Z830 is 0x80.
>>> That is, as far as I can tell from the decompilation of the DSDT.
>>>
>>> Thus I worry I will not be able to test any sort of generic implementation
>>> very well, if the Z830 only implements a small subset of the functionality.
>>
>> Right I understand still I think there should be a separate
>>
>> drivers/platform/x86/acpi_pnp0c32_buttons.c 
>>
>> driver for this IMHO. If it is only tested on your one model that
>> is fine (should be documented with a comment in the code though).
>>
>> Then at least we have something to serve as a basis for if people
>> want to add support for this on more laptop models.
>>
>> Does that sound reasonable ?
> 
> Sure, we may have talked past each other, as this is what I also believe I
> suggested.

Ah ok, good :)

> I just don't see how I can possibly implement the wakeup
> handling part of this, as no laptop I own has that.

Right, I fully agree.

> Unless you know some
> else who has a laptop that would allow testing that part.
> 
> Would a sensible option be to only implement support for key presses while
> the laptop is awake?

Yes that is fine.

> If someone comes along with a laptop that has the
> support for these buttons waking from sleep they can add that missing
> functionality at that point.

Ack.

> (It seems rather unlikely that will happen
> though: I get the feeling that this type of button never became a hit and
> is mostly a forgotten relic of the past. And if no one came along and
> bothered to add support in the decade+ since it was introduced, it was
> probably quite rare back then as well.)

Also ack.

>>>>> 4. Battery charge mode [implemented in patch 2]
>>>>> ======================
>>>>>
>>>>> This laptop supports not charging the battery fully in order to prolong
>>>>> battery life. Unlike for example ThinkPads where this control is granular
>>>>> here it is just off/on. When off it charges to 100%. When on it charges to
>>>>> about 80%.
>>>>>
>>>>> According to the Windows program used to control the feature the setting
>>>>> will not take effect until the battery has been discharged to around 50%.
>>>>> However, in my testing it takes effect as soon as the charge drops below
>>>>> 80%. On Windows Toshiba branded this feature as "Eco charging"
>>>>>
>>>>> In the following example ACPI calls I will use the following newly defined
>>>>> constants:
>>>>> #define HCI_BATTERY_CHARGE_MODE 0xba
>>>>> #define BATTERY_CHARGE_FULL 0
>>>>> #define BATTERY_CHARGE_80_PERCENT 1
>>>>>
>>>>> To set the feature:
>>>>>   {HCI_SET, HCI_BATTERY_CHARGE_MODE, charge_mode, 0, 0, 0}
>>>>> To query for the existence of the feature:
>>>>>   {HCI_GET, HCI_BATTERY_CHARGE_MODE, 0, 0, 0, 0}
>>>>> To read the feature:
>>>>>   {HCI_GET, HCI_BATTERY_CHARGE_MODE, 0, 0, 0, 1}
>>>>>
>>>>> The read may need to be retried if TOS_DATA_NOT_AVAILABLE is returned as
>>>>> the status code. This rarely happens (I have never observed it on Linux),
>>>>> but I have seen it happen under Windows once, and the software did retry
>>>>> it.
>>>>
>>>> Hmm, this is interesting if you look at:
>>>>
>>>> Documentation/ABI/testing/sysfs-class-power
>>>>
>>>> You will see there already is a standard API for this in the form of
>>>> adding a "charge_control_end_threshold" attribute to the standard
>>>> ACPI /sys/class/power_supply/BAT*/ sysfs interface. See e.g.
>>>> drivers/platform/x86/thinkpad_acpi.c
>>>>
>>>> For an example of how to add sysfs attributes to a battery
>>>> which is managed by the standard drivers/acpi/battery.c driver.
>>>>
>>>> I think you can use this standard attribute enabling eco charging
>>>> for any writes with a value <= 90 and disabling it for values
>>>>> 90 (90 being halfway between 80 and 100).
>>>>
>>>> While always showing 80 or 100 on read.
>>>>
>>>> You should then also write a patch for:
>>>>
>>>> Documentation/ABI/testing/sysfs-class-power
>>>>
>>>> Adding something like this to the "charge_control_end_threshold"
>>>> section:
>>>>
>>>> "not all hardware is capable of setting this to an arbitrary
>>>> percentage. Drivers will round written values to the nearest
>>>> supported value. Reading back the value will show the actual
>>>> threshold set by the driver."
>>>>
>>>> (feel free to copy verbatim, but maybe you can do better)
>>>>
>>>>
>>>
>>> This makes perfect sense, but I don't know if it is guaranteed to be 80%
>>> on all Toshiba laptops. Do you know of any other Toshiba laptops that
>>> have/had this feature, and if so, what the limits are? The Windows driver
>>> for this laptop does not document exactly what the limit is. 80% is simply
>>> what I have observed in practice.
>>
>> Right, the idea is to document that the hw/fw/driver may only
>> support some fixed values and that written values will be
>> rounded to one of the supported fixed values. There is no need
>> to document what those fixed values are.  The idea is that
>> userspace consumers will read back the value to see what
>> they actually got.
> 
> I think we might be slightly talking past each other here. I absolutely
> agree with your idea. My only worry is that toshiba_acpi returning 80%
> might not be the right choice. This could be model dependent.

Ah I see. 80% seems to be pretty common as max-charge value for
reducing the wear on the battery so I believe that just harcoding
80% in toshiba_acpi is fine.

If people com[plain that it actually is say 85% on some models then
we can see from there.

> Since I only have a sample size of one, it could even depend on the current
> condition of the battery for all I know (though that is probably not
> likely).

Right that seems unlikely.

> The Windows software and the manual do not specify any sort of
> percentage. It is just documented as a mode that prolongs battery life
> while reducing full charge basically. Without putting any qualifiers on
> "how much".
> 
> An option would be to return 80% for the Z830, and the string "unknown" for
> other models. Though I guess there is a risk of breaking user space
> software that only expects numeric values, so it may be a no-go.

Right, reporting non-numeric values here is not allowed, so lets
not do that.

> I guess the user space software using this interface consists of *at least*
> "tlp" and whatever KDE uses to handle it ("powerdevil" I belive?).

Right and upower / GNOME is also working towards using these.

Regards,

Hans

[PATCH 0/2] [RFC] platform/x86: Fixes for Toshiba Z830

[lkml]

From: Arvid Norlander <lkml@vorpal.se>

Hi,

I'm new to this kernel development thing, so applogies in advance for any
mistakes. I have an old Toshiba Z830-10W laptop. Unfortunately it doesn't
work well under Linux. Fortunately I spent some time figuring out what was
wrong. I had documented my findings below. I have also included patches
(see the next few emails) for some of the issues.

I would like advice on how to proceed for some of the more advanced
problems though:
* Do we want to expose all these features that I figured out? For example,
  how to set the boot order on this old BIOS-only laptop from user space.
  Or various other settings accessible via the BIOS.
* For the hardware buttons I describe below, is a solution specific to
  toshiba_acpi preferred, or should additional effort be spent on
  investigating a generic solution?

Before I start coding on these more complex issues I would like advice in
order to avoid wasting my time on bad designs. In particular, on how to
proceed with the "Hardware buttons" section below.

Best regards,
Arvid Norlander

Table of contents
=================
1. Short info on the laptop and methodology
2. Background on Toshiba ACPI communication methods
3. LED: "Eco" LED ................................ [implemented in patch 1]
4. Battery charge mode ........................... [implemented in patch 2]
5. Hardware buttons ...................................... [Advice wanted!]
6. Panel power control via HCI
7. BIOS setting control from the OS ................... [should we bother?]
   7.1 Setting boot order
   7.2 Setting USB memory emulation
   7.3 Display during boot
   7.4 CPU control
   7.5 Wake on LAN
   7.6 SATA power control
   7.8 Legacy USB
   7.9 Wake on keyboard
8. Other features .... [nothing actionable as of now, for information only]


1. Short info on the laptop and methodology
===========================================

The Toshiba Z830-10W laptop is a so-called "Ultrabook" dating from 2011.
It has BIOS, not UEFI.

The Toshiba Z830-10W laptop has several features that don't work properly
under Linux. I have performed reverse engineering by tracing ACPI calls
with WinDbg using the "AMLI" feature while performing various actions to
determine what the Windows drivers do. My user space tracing on Windows
has been limited to determine which programs interact with the driver so
that I could kill those that I was not testing behaviour of at the moment.

I have then tested these features using the "acpi_call" kernel module on
Linux to issue the relevant calls under Linux. In the following sections
is a feature by feature breakdown.


2. Background on Toshiba ACPI communication methods
===================================================

This is a short summary of the general protocol. This is already
implemented in the toshiba_acpi driver. If you are already familiar with
that you can skip this section.

Almost all vendor specific features work via the \_SB_.VALZ ACPI device
defined in the DSDT. There are a handful of interesting methods on this
object, but for the purposes of this write-up only "GHCI" is relevant. This
method takes 6 integer (32-bit) arguments and returns a buffer 6 32-bit
integers.

The general format of queries is: {OPERATION, REGISTER, ARG1, ..., ARG4 }.
The operation is one of HCI_GET/HCI_SET or SCI_GET/SCI_SET (plus SCI_OPEN
and  SCI_CLOSE). This allows for getting and setting various registers to
control features or read out data.

The data returned varies a bit, but is /generally/ on the form:
{STATUS_CODE, REGISTER_FROM_QUERY, VAL1, ..., VAL4 }

What is the difference between HCI_* and SCI_* calls? The only important
difference here is that for SCI_GET/SET you first need to call SCI_OPEN and
then follow the get or set with an SCI_CLOSE call.

Much of the rest of this write-up consists of documenting registers
previously not handled by the toshiba_acpi Linux driver.


3. LED: "Eco" LED [implemented in patch 1]
=================

The toshiba_acpi driver has support for controlling some LEDs including the
"Eco" LED. Unfortunately that LED works differently on this laptop.

The toshiba_acpi driver checks for TOS_INPUT_DATA_ERROR and tries a
different format. On the Z830 the error returned is TOS_NOT_SUPPORTED
though the different format still works.


4. Battery charge mode [implemented in patch 2]
======================

This laptop supports not charging the battery fully in order to prolong
battery life. Unlike for example ThinkPads where this control is granular
here it is just off/on. When off it charges to 100%. When on it charges to
about 80%.

According to the Windows program used to control the feature the setting
will not take effect until the battery has been discharged to around 50%.
However, in my testing it takes effect as soon as the charge drops below
80%. On Windows Toshiba branded this feature as "Eco charging"

In the following example ACPI calls I will use the following newly defined
constants:
#define HCI_BATTERY_CHARGE_MODE 0xba
#define BATTERY_CHARGE_FULL 0
#define BATTERY_CHARGE_80_PERCENT 1

To set the feature:
  {HCI_SET, HCI_BATTERY_CHARGE_MODE, charge_mode, 0, 0, 0}
To query for the existence of the feature:
  {HCI_GET, HCI_BATTERY_CHARGE_MODE, 0, 0, 0, 0}
To read the feature:
  {HCI_GET, HCI_BATTERY_CHARGE_MODE, 0, 0, 0, 1}

The read may need to be retried if TOS_DATA_NOT_AVAILABLE is returned as
the status code. This rarely happens (I have never observed it on Linux),
but I have seen it happen under Windows once, and the software did retry
it.


5. Hardware buttons [Advice wanted!]
===================

All the Fn+<key> hotkeys work. However, there are some hardware buttons
that do not. These buttons are:

* A button between space and the touchpad to turn off/on the touchpad.
* Two buttons next to the power button, one is "eco-mode", the other is
"projector".

The two buttons next to the power button both send Windows+X by default.
The touchpad control button does nothing that Linux can detect.

To enable this functionality several changes are needed.

The toshiba_acpi driver currently uses
  {HCI_SET, HCI_HOTKEY_EVENT, HCI_HOTKEY_ENABLE, 0, ...}
to enable the Fn+<key> hotkeys, where HCI_HOTKEY_ENABLE = 0x09. However on
this laptop the value 0x05 must be used instead.

This is not the whole story however, as these keys do not work like any of
the Fn-hotkeys (ACPI notification on \_SB_.VALZ). Instead, once enabled via
the above method they start sending notifications on various PNP0C32
devices. These are currently not handled by Linux. According to a search
PNP0C32 is "HIDACPI Button Device", so perhaps a generic driver should be
created.

The devices in question are:
PNP0C32 \_SB_.HS81 UID 0x03 = Enable/disable trackpad
PNP0C32 \_SB_.HS87 UID 0x01 = Eco button
PNP0C32 \_SB_.HS86 UID 0x02 = Monitor/projector button

Only the "path" and the "UID" value in the ACPI DSDT tell these devices
apart.

The notification always uses the value 0x80.

I'm not sure what the best approach to implement support for these would
be.


6. Panel power control via HCI
==============================

The toshiba_acpi driver supports controlling the panel power via SCI calls
(SCI_PANEL_POWER_ON). Based on the fact that the toshiba_acpi driver
outputs a message about reboot being needed I assume this is related to
panel power during boot?

However there is a HCI call that can turn the panel off or on immediately:

#define HCI_PANEL_POWER_ON 0x2
#define PANEL_ON 1
#define PANEL_OFF 0

To read/query existence: {HCI_GET, HCI_PANEL_POWER_ON, 0, 0, 0, 0}
To write: {HCI_SET, HCI_PANEL_POWER_ON, panel_on, 0, 0, 0}

This could be related to some backlight issues this laptop is having where
backlight control stops working after a suspend/resume.

Control via /sys/class/backlight/intel_backlight always works on this
laptop, however, most desktop environment seems to prefer the acpi_video or
vendor backlight controls if those exist.

I have tried acpi_backlight=vendor/native but that is broken in the same
way. With acpi_backlight=none, the screen never turns on after a resume.
However if I turn it on using the above HCI call, everything works
normally after that. And since only the intel_backlight driver is left,
the desktop environment controls backlight via that method.
  
I have yet to find a satisfactory solution to this problem, but I suspect
the correct solution would be to fix backlight control from acpi_video,
if that is possible. Suggestions?


7. BIOS setting control from the OS [should we bother?]
===================================
Several of the BIOS settings can be controlled from the OS. This all
happens via SCI calls. On Windows the "Hwsetup.exe" program offers this
control. I'm not sure how useful any of this is (as this is already
available via the BIOS).

If you think it is a good idea I could absolutely implement support for
this. Otherwise I might build a simple user space tool on top of acpi_call
for this. 

7.1 Setting boot order
----------------------
This is a BIOS (not UEFI) laptop, so boot order could normally not be
controlled from the OS. However here it is possible:

#define SCI_BOOT_ORDER 0x157

In this SCI register the boot order is stored as a list with each nibble
indicating a device:
#define SCI_BOOT_ORDER_FDD 0x0
#define SCI_BOOT_ORDER_HDD 0x1
#define SCI_BOOT_ORDER_LAN 0x3
#define SCI_BOOT_ORDER_USB_MEMORY 0x4
#define SCI_BOOT_ORDER_USB_CD 0x7
#define SCI_BOOT_ORDER_USB_UNUSED 0xf

These are then combined as follows:

Set boot order to USB memory, USB CD, HDD, LAN, FDD:
{SCI_SET, SCI_BOOT_ORDER, 0xfff03174, 0, 0, 0}

Each nibble indicates a device, with the lowest nibble being the first
device in the boot order. As this device doesn't have a physical FDD I
assume that this refers to USB attached devices, but I have not tested this
(I do have a USB floppy drive if anyone really cares).

When reading the data out the result is a bit surprising:
  {0x0, 0x8505, 0xfff30174, 0x5, 0xfff30741, 0x0}
Presumably these other values also mean something, the boot order in this
case is USB memory, USB CD, HDD, FDD, LAN, so the third value is the boot
order.

I'm not sure what a suitable API for exposing this setting to userspace via
sysfs would be.

7.2 Setting USB memory emulation
--------------------------------
The BIOS can either treat USB drives as HDDs or FDDs for booting purposes:

#define SCI_BOOT_FLOPPY_EMULATION 0x511
#define SCI_BOOT_FLOPPY_EMULATION_FDD 0x1
#define SCI_BOOT_FLOPPY_EMULATION_HDD 0x0

To set: {SCI_SET, SCI_BOOT_FLOPPY_EMULATION, value, 0, 0, 0}
Getting/existence query: {SCI_GET, SCI_BOOT_FLOPPY_EMULATION, 0, 0, 0, 0}

7.3 Display during boot
-----------------------
This controls if BIOS/GRUB/etc is shown on just the internal monitor or
if the montior is automatically selected.

Note: When changing this in Windows it tells me a restart is required.

#define SCI_BOOT_DISPLAY 0x300
#define SCI_BOOT_DISPLAY_INTERNAL 0x1250
#define SCI_BOOT_DISPLAY_AUTO 0x3250

To set: {SCI_SET, SCI_BOOT_DISPLAY, value, 0, 0, 0}
Getting/existence query as usual.

Note! I have not tested the effects of this, as the only external monitors
I have that are not in storage use DisplayPort, while this laptop has HDMI
and VGA. I *do* have an old VGA TFT in storage if anyone cares though...

7.4 CPU control
---------------
I presume this is only for operating systems that don't manage this
themselves, I don't know for sure. The wording in the documentation is
vague, but I believe it controls CPU frequency behaviour.

Note: When changing this in Windows it tells me a restart is required.

#define SCI_CPU_FREQUENCY 0x132
#define SCI_CPU_FREQUENCY_DYNAMIC 0x0
#define SCI_CPU_FREQUENCY_HIGH 0x1
#define SCI_CPU_FREQUENCY_LOW 0x2

Set and get as usual ({SCI_GET/SET, SCI_CPU_FREQUENCY, value, 0, 0, 0}).

7.5 Wake on LAN
---------------
Note! This only controls Wake on LAN when off/hibernated (and since this
laptop has Intel Rapid Start, presumably in that mode too). It is not
relevant to WoL when in sleep.

Here the Windows driver seem to query several possibilities until it hits
on one that works:
#define SCI_WAKE_ON_LAN 0x700

#define SCI_WAKE_ON_LAN_OFF 0x1
#define SCI_WAKE_ON_LAN_ON 0x1

#define SCI_WAKE_ON_LAN_REG1 0x0
#define SCI_WAKE_ON_LAN_REG2 0x1000
#define SCI_WAKE_ON_LAN_REG3 0x800

To set:
  {SCI_SET, SCI_WAKE_ON_LAN, value | register, 0, 0, 0}
To get/query:
  {SCI_GET, SCI_WAKE_ON_LAN, register, 0, 0, 0}

For example on this specific laptop to enable WoL:
  {SCI_SET, SCI_WAKE_ON_LAN, SCI_WAKE_ON_LAN_ON | SCI_WAKE_ON_LAN_REG3, 0, 0, 0}

REG1 and REG2 give return code TOS_INPUT_DATA_ERROR on this laptop, but
presumably they are needed on some laptops, or the Windows program would
not be attempting to use them.

7.6 SATA power control
----------------------
This is another one that I don't know what exactly it corresponds to, maybe
it is something Linux can control directly:

#define SCI_SATA_POWER 0x406
#define SCI_SATA_POWER_BATTERY_LIFE 0x1
#define SCI_SATA_POWER_PERFORMANCE 0x0

Get/set/query as expected: {SCI_SET, SCI_SATA_POWER, value, 0, 0, 0}

7.8 Legacy USB
--------------
Controls Legacy USB support in BIOS.

Note: When changing this in Windows it tells me a restart is required.

#define SCI_LEGACY_USB 0x50c
#define SCI_LEGACY_USB_ENABLED 0x1
#define SCI_LEGACY_USB_DISABLED 0x0


Get/set/query as expected: {SCI_SET, SCI_LEGACY_USB, value, 0, 0, 0}

7.9 Wake on keyboard
--------------------
This controls if pressing a key on the keyboard wakes the laptop from
sleep. Otherwise, only opening the monitor or pressing the power button
works for this.

#define SCI_WAKE_ON_KEYBOARD 0x137
#define SCI_WAKE_ON_KEYBOARD_ENABLE 0x8
#define SCI_WAKE_ON_KEYBOARD_DISABLE 0x0

Set: {SCI_SET, SCI_WAKE_ON_KEYBOARD, value, 0, 0, 0}

Get/query on the standard form but with slightly weird return values:
  {TOS_SUCCESS2, 0x800a, value, 0x38, 0x0, 0x0}


8. Other features
=================

I should note that I did discover some additional registers, but I don't
fully understand them yet. I have thus not included them here in order to
not waste your time. However I did write a blog post about this which
includes that information, which is available should you be interested:

https://vorpal.se/posts/2022/aug/21/reverse-engineering-acpi-functionality-on-a-toshiba-z830-ultrabook/#other-features_1


Arvid Norlander (2):
  platform/x86: Fix ECO LED control on Toshiba Z830
  platform/x86: Battery charge mode in toshiba_acpi

 drivers/platform/x86/toshiba_acpi.c | 115 +++++++++++++++++++++++++++-
 1 file changed, 114 insertions(+), 1 deletion(-)


base-commit: e3f259d33c0ebae1b6e4922c7cdb50e864c81928
-- 
2.37.2