platform-driver-x86.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] [RFC] platform/x86: Fixes for Toshiba Z830
@ 2022-08-21 19:49 lkml
  2022-08-21 19:49 ` [PATCH 1/2] platform/x86: Fix ECO LED control on " lkml
  2022-08-21 19:49 ` [PATCH 2/2] platform/x86: Battery charge mode in toshiba_acpi lkml
  0 siblings, 2 replies; 5+ messages in thread
From: lkml @ 2022-08-21 19:49 UTC (permalink / raw)
  To: platform-driver-x86; +Cc: Azael Avalos, Arvid Norlander

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] platform/x86: Fix ECO LED control on Toshiba Z830
  2022-08-21 19:49 [PATCH 0/2] [RFC] platform/x86: Fixes for Toshiba Z830 lkml
@ 2022-08-21 19:49 ` lkml
  2022-08-21 19:49 ` [PATCH 2/2] platform/x86: Battery charge mode in toshiba_acpi lkml
  1 sibling, 0 replies; 5+ messages in thread
From: lkml @ 2022-08-21 19:49 UTC (permalink / raw)
  To: platform-driver-x86; +Cc: Azael Avalos, Arvid Norlander

From: Arvid Norlander <lkml@vorpal.se>

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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] platform/x86: Battery charge mode in toshiba_acpi
  2022-08-21 19:49 [PATCH 0/2] [RFC] platform/x86: Fixes for Toshiba Z830 lkml
  2022-08-21 19:49 ` [PATCH 1/2] platform/x86: Fix ECO LED control on " lkml
@ 2022-08-21 19:49 ` lkml
  1 sibling, 0 replies; 5+ messages in thread
From: lkml @ 2022-08-21 19:49 UTC (permalink / raw)
  To: platform-driver-x86; +Cc: Azael Avalos, Arvid Norlander

From: Arvid Norlander <lkml@vorpal.se>

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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] platform/x86: Fix ECO LED control on Toshiba Z830
  2022-08-21 20:08 ` [PATCH 1/2] platform/x86: Fix ECO LED control on " Arvid Norlander
@ 2022-08-26 10:58   ` Hans de Goede
  0 siblings, 0 replies; 5+ messages in thread
From: Hans de Goede @ 2022-08-26 10:58 UTC (permalink / raw)
  To: Arvid Norlander, platform-driver-x86; +Cc: Azael Avalos

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.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] platform/x86: Fix ECO LED control on Toshiba Z830
  2022-08-21 20:08 [PATCH 0/2] [RFC] platform/x86: Fixes for Toshiba Z830 Arvid Norlander
@ 2022-08-21 20:08 ` Arvid Norlander
  2022-08-26 10:58   ` Hans de Goede
  0 siblings, 1 reply; 5+ messages in thread
From: Arvid Norlander @ 2022-08-21 20:08 UTC (permalink / raw)
  To: platform-driver-x86; +Cc: Azael Avalos, 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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-08-26 11:01 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-21 19:49 [PATCH 0/2] [RFC] platform/x86: Fixes for Toshiba Z830 lkml
2022-08-21 19:49 ` [PATCH 1/2] platform/x86: Fix ECO LED control on " lkml
2022-08-21 19:49 ` [PATCH 2/2] platform/x86: Battery charge mode in toshiba_acpi lkml
2022-08-21 20:08 [PATCH 0/2] [RFC] platform/x86: Fixes for Toshiba Z830 Arvid Norlander
2022-08-21 20:08 ` [PATCH 1/2] platform/x86: Fix ECO LED control on " Arvid Norlander
2022-08-26 10:58   ` Hans de Goede

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).