[PATCH 0/2] platform/x86: Add ACPI quickstart button driver

[PATCH 0/2] platform/x86: Add ACPI quickstart button driver

[Armin Wolf]

This patch series adds support for the ACPI PNP0C32 device as
proposed in 2022 by Arvid Norlander. The first patch adds support
for the device itself, while the second patch was taken from the
original series.

Both patches are compile-tested only.

Armin Wolf (1):
  platform/x86: Add ACPI quickstart button (PNP0C32) driver

Arvid Norlander (1):
  platform/x86: toshiba_acpi: Add quirk for buttons on Z830

 MAINTAINERS                         |   6 +
 drivers/platform/x86/Kconfig        |  13 ++
 drivers/platform/x86/Makefile       |   3 +
 drivers/platform/x86/quickstart.c   | 225 ++++++++++++++++++++++++++++
 drivers/platform/x86/toshiba_acpi.c |  36 ++++-
 5 files changed, 280 insertions(+), 3 deletions(-)
 create mode 100644 drivers/platform/x86/quickstart.c

--
2.39.2

[PATCH 1/2] platform/x86: Add ACPI quickstart button (PNP0C32) driver

[Armin Wolf]

This drivers supports the ACPI quickstart button device, which
is used to send manufacturer-specific events to userspace.
Since the meaning of those events is not standardized, userspace
has to use for example hwdb to decode them.

The driver itself is based on an earlier proposal, but contains
some improvements and uses the device wakeup API instead of a
custom sysfs file.

Compile-tested only.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 MAINTAINERS                       |   6 +
 drivers/platform/x86/Kconfig      |  13 ++
 drivers/platform/x86/Makefile     |   3 +
 drivers/platform/x86/quickstart.c | 225 ++++++++++++++++++++++++++++++
 4 files changed, 247 insertions(+)
 create mode 100644 drivers/platform/x86/quickstart.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 8d1052fa6a69..b71692c55f7b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -354,6 +354,12 @@ B:	https://bugzilla.kernel.org
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm
 F:	drivers/acpi/pmic/

+ACPI QUICKSTART DRIVER
+M:	Armin Wolf <W_Armin@gmx.de>
+L:	platform-driver-x86@vger.kernel.org
+S:	Maintained
+F:	drivers/platform/x86/quickstart.c
+
 ACPI SERIAL MULTI INSTANTIATE DRIVER
 M:	Hans de Goede <hdegoede@redhat.com>
 L:	platform-driver-x86@vger.kernel.org
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 6dbd40e2aeda..9cba9a410e28 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -641,6 +641,19 @@ config THINKPAD_LMI

 source "drivers/platform/x86/intel/Kconfig"

+config ACPI_QUICKSTART
+	tristate "ACPI Quickstart button driver"
+	depends on ACPI
+	depends on INPUT
+	select INPUT_SPARSE_KEYMAP
+	help
+	  This driver adds support for ACPI quickstart button (PNP0C32) devices.
+	  The button emits a manufacturer-specific key value when pressed, so
+	  userspace has to map this value to a standard key code.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called quickstart.
+
 config MSI_EC
 	tristate "MSI EC Extras"
 	depends on ACPI
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 1de432e8861e..0801ccc37e9b 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -70,6 +70,9 @@ obj-$(CONFIG_YOGABOOK)		+= lenovo-yogabook.o
 # Intel
 obj-y				+= intel/

+# Microsoft
+obj-$(CONFIG_ACPI_QUICKSTART)  += quickstart.o
+
 # MSI
 obj-$(CONFIG_MSI_EC)		+= msi-ec.o
 obj-$(CONFIG_MSI_LAPTOP)	+= msi-laptop.o
diff --git a/drivers/platform/x86/quickstart.c b/drivers/platform/x86/quickstart.c
new file mode 100644
index 000000000000..ba3a7a25dda7
--- /dev/null
+++ b/drivers/platform/x86/quickstart.c
@@ -0,0 +1,225 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * quickstart.c - ACPI Direct App Launch driver
+ *
+ * Copyright (C) 2024 Armin Wolf <W_Armin@gmx.de>
+ * Copyright (C) 2022 Arvid Norlander <lkml@vorapal.se>
+ * Copyright (C) 2007-2010 Angelo Arrifano <miknix@gmail.com>
+ *
+ * Information gathered from disassembled dsdt and from here:
+ * <https://archive.org/details/microsoft-acpi-dirapplaunch>
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/acpi.h>
+#include <linux/init.h>
+#include <linux/input.h>
+#include <linux/input/sparse-keymap.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/sysfs.h>
+#include <linux/types.h>
+
+#include <asm/unaligned.h>
+
+#define DRIVER_NAME	"quickstart"
+
+/*
+ * There will be two events:
+ * 0x02 - Button was pressed while device was off/sleeping.
+ * 0x80 - Button was pressed while device was up.
+ */
+#define QUICKSTART_EVENT_RUNTIME	0x80
+
+struct quickstart_data {
+	struct device *dev;
+	struct input_dev *input_device;
+	char input_name[32];
+	char phys[32];
+	u32 id;
+};
+
+/*
+ * Knowing what these buttons do require system specific knowledge.
+ * This could be done by matching on DMI data in a long quirk table.
+ * However, it is easier to leave it up to user space to figure this out.
+ *
+ * Using for example udev hwdb the scancode 0x1 can be remapped suitably.
+ */
+static const struct key_entry quickstart_keymap[] = {
+	{ KE_KEY, 0x1, { KEY_UNKNOWN } },
+	{ KE_END, 0 },
+};
+
+static ssize_t button_id_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct quickstart_data *data = dev_get_drvdata(dev);
+
+	return sysfs_emit(buf, "%u\n", data->id);
+}
+static DEVICE_ATTR_RO(button_id);
+
+static struct attribute *quickstart_attrs[] = {
+	&dev_attr_button_id.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(quickstart);
+
+static void quickstart_notify(acpi_handle handle, u32 event, void *context)
+{
+	struct quickstart_data *data = context;
+
+	switch (event) {
+	case QUICKSTART_EVENT_RUNTIME:
+		sparse_keymap_report_event(data->input_device, 0x1, 1, true);
+		acpi_bus_generate_netlink_event(DRIVER_NAME, dev_name(data->dev), event, 0);
+		break;
+	default:
+		dev_err(data->dev, FW_INFO "Unexpected ACPI notify event (%u)\n", event);
+		break;
+	}
+}
+
+/*
+ * The GHID ACPI method is used to indicate the "role" of the button.
+ * However, all the meanings of these values are vendor defined.
+ *
+ * We do however expose this value to user space.
+ */
+static int quickstart_get_ghid(struct quickstart_data *data)
+{
+	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+	acpi_handle handle = ACPI_HANDLE(data->dev);
+	union acpi_object *obj;
+	acpi_status status;
+	int ret = 0;
+
+	/*
+	 * This returns a buffer telling the button usage ID,
+	 * and triggers pending notify events (The ones before booting).
+	 */
+	status = acpi_evaluate_object_typed(handle, "GHID", NULL, &buffer, ACPI_TYPE_BUFFER);
+	if (ACPI_FAILURE(status))
+		return -EIO;
+
+	obj = buffer.pointer;
+	if (!obj)
+		return -ENODATA;
+
+	/*
+	 * Quoting the specification:
+	 * "The GHID method can return a BYTE, WORD, or DWORD.
+	 *  The value must be encoded in little-endian byte
+	 *  order (least significant byte first)."
+	 */
+	switch (obj->buffer.length) {
+	case 1:
+		data->id = obj->buffer.pointer[0];
+		break;
+	case 2:
+		data->id = get_unaligned_le16(obj->buffer.pointer);
+		break;
+	case 4:
+		data->id = get_unaligned_le32(obj->buffer.pointer);
+		break;
+	default:
+		dev_err(data->dev,
+			FW_BUG "GHID method returned buffer of unexpected length %u\n",
+			obj->buffer.length);
+		ret = -EIO;
+		break;
+	}
+
+	kfree(obj);
+
+	return ret;
+}
+
+static void quickstart_notify_remove(void *context)
+{
+	struct quickstart_data *data = context;
+	acpi_handle handle;
+
+	handle = ACPI_HANDLE(data->dev);
+
+	acpi_remove_notify_handler(handle, ACPI_DEVICE_NOTIFY, quickstart_notify);
+}
+
+static int quickstart_probe(struct platform_device *pdev)
+{
+	struct quickstart_data *data;
+	acpi_handle handle;
+	acpi_status status;
+	int ret;
+
+	handle = ACPI_HANDLE(&pdev->dev);
+	if (!handle)
+		return -ENODEV;
+
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->dev = &pdev->dev;
+	dev_set_drvdata(&pdev->dev, data);
+
+	/* We have to initialize the device wakeup before evaluating GHID because
+	 * doing so will notify the device if the button was used to wake the machine
+	 * from S5.
+	 */
+	device_init_wakeup(&pdev->dev, true);
+
+	ret = quickstart_get_ghid(data);
+	if (ret < 0)
+		return ret;
+
+	data->input_device = devm_input_allocate_device(&pdev->dev);
+	if (!data->input_device)
+		return -ENOMEM;
+
+	ret = sparse_keymap_setup(data->input_device, quickstart_keymap, NULL);
+	if (ret < 0)
+		return ret;
+
+	snprintf(data->input_name, sizeof(data->input_name), "Quickstart Button %u", data->id);
+	snprintf(data->phys, sizeof(data->phys), DRIVER_NAME "/input%u", data->id);
+
+	data->input_device->name = data->input_name;
+	data->input_device->phys = data->phys;
+	data->input_device->id.bustype = BUS_HOST;
+
+	ret = input_register_device(data->input_device);
+	if (ret < 0)
+		return ret;
+
+	status = acpi_install_notify_handler(handle, ACPI_DEVICE_NOTIFY, quickstart_notify, data);
+	if (ACPI_FAILURE(status))
+		return -EIO;
+
+	return devm_add_action_or_reset(&pdev->dev, quickstart_notify_remove, data);
+}
+
+static const struct acpi_device_id quickstart_device_ids[] = {
+	{ "PNP0C32", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, quickstart_device_ids);
+
+static struct platform_driver quickstart_platform_driver = {
+	.driver	= {
+		.name = DRIVER_NAME,
+		.dev_groups = quickstart_groups,
+		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
+		.acpi_match_table = quickstart_device_ids,
+	},
+	.probe = quickstart_probe,
+};
+module_platform_driver(quickstart_platform_driver);
+
+MODULE_AUTHOR("Armin Wolf <W_Armin@gmx.de>");
+MODULE_AUTHOR("Arvid Norlander <lkml@vorpal.se>");
+MODULE_AUTHOR("Angelo Arrifano");
+MODULE_DESCRIPTION("ACPI Direct App Launch driver");
+MODULE_LICENSE("GPL");
--
2.39.2

Re: [PATCH 1/2] platform/x86: Add ACPI quickstart button (PNP0C32) driver

[Ilpo Järvinen]

On Wed, 31 Jan 2024, Armin Wolf wrote:

> This drivers supports the ACPI quickstart button device, which
> is used to send manufacturer-specific events to userspace.
> Since the meaning of those events is not standardized, userspace
> has to use for example hwdb to decode them.
> 
> The driver itself is based on an earlier proposal, but contains
> some improvements and uses the device wakeup API instead of a
> custom sysfs file.
> 
> Compile-tested only.
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>

> diff --git a/drivers/platform/x86/quickstart.c b/drivers/platform/x86/quickstart.c
> new file mode 100644
> index 000000000000..ba3a7a25dda7
> --- /dev/null
> +++ b/drivers/platform/x86/quickstart.c
> @@ -0,0 +1,225 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * quickstart.c - ACPI Direct App Launch driver

ACPI Quickstart ?

> + * Copyright (C) 2024 Armin Wolf <W_Armin@gmx.de>
> + * Copyright (C) 2022 Arvid Norlander <lkml@vorapal.se>
> + * Copyright (C) 2007-2010 Angelo Arrifano <miknix@gmail.com>
> + *
> + * Information gathered from disassembled dsdt and from here:
> + * <https://archive.org/details/microsoft-acpi-dirapplaunch>
> + */

> +static void quickstart_notify(acpi_handle handle, u32 event, void *context)
> +{
> +	struct quickstart_data *data = context;
> +
> +	switch (event) {
> +	case QUICKSTART_EVENT_RUNTIME:
> +		sparse_keymap_report_event(data->input_device, 0x1, 1, true);
> +		acpi_bus_generate_netlink_event(DRIVER_NAME, dev_name(data->dev), event, 0);
> +		break;
> +	default:
> +		dev_err(data->dev, FW_INFO "Unexpected ACPI notify event (%u)\n", event);

Could this end up spamming the logs so perhaps use _ratelimited variant?

> +static int quickstart_probe(struct platform_device *pdev)
> +{
> +	struct quickstart_data *data;
> +	acpi_handle handle;
> +	acpi_status status;
> +	int ret;
> +
> +	handle = ACPI_HANDLE(&pdev->dev);
> +	if (!handle)
> +		return -ENODEV;
> +
> +	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	data->dev = &pdev->dev;
> +	dev_set_drvdata(&pdev->dev, data);
> +
> +	/* We have to initialize the device wakeup before evaluating GHID because
> +	 * doing so will notify the device if the button was used to wake the machine
> +	 * from S5.
> +	 */
> +	device_init_wakeup(&pdev->dev, true);
> +
> +	ret = quickstart_get_ghid(data);
> +	if (ret < 0)
> +		return ret;
> +
> +	data->input_device = devm_input_allocate_device(&pdev->dev);
> +	if (!data->input_device)
> +		return -ENOMEM;
> +
> +	ret = sparse_keymap_setup(data->input_device, quickstart_keymap, NULL);
> +	if (ret < 0)
> +		return ret;
> +
> +	snprintf(data->input_name, sizeof(data->input_name), "Quickstart Button %u", data->id);
> +	snprintf(data->phys, sizeof(data->phys), DRIVER_NAME "/input%u", data->id);
> +
> +	data->input_device->name = data->input_name;
> +	data->input_device->phys = data->phys;

Why not devm_kasprintf() these directly into data->input_device->xx + 
NULL check instead of storing into struct quickstart_data ?

> +static struct platform_driver quickstart_platform_driver = {
> +	.driver	= {
> +		.name = DRIVER_NAME,
> +		.dev_groups = quickstart_groups,
> +		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
> +		.acpi_match_table = quickstart_device_ids,
> +	},
> +	.probe = quickstart_probe,
> +};
> +module_platform_driver(quickstart_platform_driver);
> +
> +MODULE_AUTHOR("Armin Wolf <W_Armin@gmx.de>");
> +MODULE_AUTHOR("Arvid Norlander <lkml@vorpal.se>");
> +MODULE_AUTHOR("Angelo Arrifano");
> +MODULE_DESCRIPTION("ACPI Direct App Launch driver");

ACPI Quickstart Button ?

> +MODULE_LICENSE("GPL");
> --
> 2.39.2
> 

-- 
 i.

[PATCH 2/2] platform/x86: toshiba_acpi: Add quirk for buttons on Z830

[Armin Wolf]

From: Arvid Norlander <lkml@vorpal.se>

The Z830 has some buttons that will only work properly as "quickstart"
buttons. To enable them in that mode, a value between 1 and 7 must be
used for HCI_HOTKEY_EVENT. Windows uses 0x5 on this laptop so use that for
maximum predictability and compatibility.

As there is not yet a known way of auto detection, this patch uses a DMI
quirk table. A module parameter is exposed to allow setting this on other
models for testing.

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

diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index 291f14ef6702..2a5a651235fe 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -57,6 +57,11 @@ module_param(turn_on_panel_on_resume, int, 0644);
 MODULE_PARM_DESC(turn_on_panel_on_resume,
 	"Call HCI_PANEL_POWER_ON on resume (-1 = auto, 0 = no, 1 = yes");

+static int hci_hotkey_quickstart = -1;
+module_param(hci_hotkey_quickstart, int, 0644);
+MODULE_PARM_DESC(hci_hotkey_quickstart,
+		 "Call HCI_HOTKEY_EVENT with value 0x5 for quickstart button support (-1 = auto, 0 = no, 1 = yes");
+
 #define TOSHIBA_WMI_EVENT_GUID "59142400-C6A3-40FA-BADB-8A2652834100"

 /* Scan code for Fn key on TOS1900 models */
@@ -136,6 +141,7 @@ MODULE_PARM_DESC(turn_on_panel_on_resume,
 #define HCI_ACCEL_MASK			0x7fff
 #define HCI_ACCEL_DIRECTION_MASK	0x8000
 #define HCI_HOTKEY_DISABLE		0x0b
+#define HCI_HOTKEY_ENABLE_QUICKSTART	0x05
 #define HCI_HOTKEY_ENABLE		0x09
 #define HCI_HOTKEY_SPECIAL_FUNCTIONS	0x10
 #define HCI_LCD_BRIGHTNESS_BITS		3
@@ -2730,10 +2736,15 @@ static int toshiba_acpi_enable_hotkeys(struct toshiba_acpi_dev *dev)
 		return -ENODEV;

 	/*
+	 * Enable quickstart buttons if supported.
+	 *
 	 * Enable the "Special Functions" mode only if they are
 	 * supported and if they are activated.
 	 */
-	if (dev->kbd_function_keys_supported && dev->special_functions)
+	if (hci_hotkey_quickstart)
+		result = hci_write(dev, HCI_HOTKEY_EVENT,
+				   HCI_HOTKEY_ENABLE_QUICKSTART);
+	else if (dev->kbd_function_keys_supported && dev->special_functions)
 		result = hci_write(dev, HCI_HOTKEY_EVENT,
 				   HCI_HOTKEY_SPECIAL_FUNCTIONS);
 	else
@@ -3257,7 +3268,14 @@ static const char *find_hci_method(acpi_handle handle)
  * works. toshiba_acpi_resume() uses HCI_PANEL_POWER_ON to avoid changing
  * the configured brightness level.
  */
-static const struct dmi_system_id turn_on_panel_on_resume_dmi_ids[] = {
+#define QUIRK_TURN_ON_PANEL_ON_RESUME		BIT(0)
+/*
+ * Some Toshibas use "quickstart" keys. On these, HCI_HOTKEY_EVENT must use
+ * the value HCI_HOTKEY_ENABLE_QUICKSTART.
+ */
+#define QUIRK_HCI_HOTKEY_QUICKSTART		BIT(1)
+
+static const struct dmi_system_id toshiba_dmi_quirks[] = {
 	{
 	 /* Toshiba Portégé R700 */
 	 /* https://bugzilla.kernel.org/show_bug.cgi?id=21012 */
@@ -3265,6 +3283,7 @@ static const struct dmi_system_id turn_on_panel_on_resume_dmi_ids[] = {
 		DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
 		DMI_MATCH(DMI_PRODUCT_NAME, "PORTEGE R700"),
 		},
+	 .driver_data = (void *)QUIRK_TURN_ON_PANEL_ON_RESUME,
 	},
 	{
 	 /* Toshiba Satellite/Portégé R830 */
@@ -3274,6 +3293,7 @@ static const struct dmi_system_id turn_on_panel_on_resume_dmi_ids[] = {
 		DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
 		DMI_MATCH(DMI_PRODUCT_NAME, "R830"),
 		},
+	 .driver_data = (void *)QUIRK_TURN_ON_PANEL_ON_RESUME,
 	},
 	{
 	 /* Toshiba Satellite/Portégé Z830 */
@@ -3281,6 +3301,7 @@ static const struct dmi_system_id turn_on_panel_on_resume_dmi_ids[] = {
 		DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
 		DMI_MATCH(DMI_PRODUCT_NAME, "Z830"),
 		},
+	 .driver_data = (void *)(QUIRK_TURN_ON_PANEL_ON_RESUME | QUIRK_HCI_HOTKEY_QUICKSTART),
 	},
 };

@@ -3289,6 +3310,8 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev)
 	struct toshiba_acpi_dev *dev;
 	const char *hci_method;
 	u32 dummy;
+	const struct dmi_system_id *dmi_id;
+	long quirks = 0;
 	int ret = 0;

 	if (toshiba_acpi)
@@ -3441,8 +3464,15 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev)
 	}
 #endif

+	dmi_id = dmi_first_match(toshiba_dmi_quirks);
+	if (dmi_id)
+		quirks = (long)dmi_id->driver_data;
+
 	if (turn_on_panel_on_resume == -1)
-		turn_on_panel_on_resume = dmi_check_system(turn_on_panel_on_resume_dmi_ids);
+		turn_on_panel_on_resume = !!(quirks & QUIRK_TURN_ON_PANEL_ON_RESUME);
+
+	if (hci_hotkey_quickstart == -1)
+		hci_hotkey_quickstart = !!(quirks & QUIRK_HCI_HOTKEY_QUICKSTART);

 	toshiba_wwan_available(dev);
 	if (dev->wwan_supported)
--
2.39.2

Re: [PATCH 0/2] platform/x86: Add ACPI quickstart button driver

[Hans de Goede]

Hi Armin and Arvid,

On 1/31/24 12:16 PM, Armin Wolf wrote:
> This patch series adds support for the ACPI PNP0C32 device as
> proposed in 2022 by Arvid Norlander. The first patch adds support
> for the device itself, while the second patch was taken from the
> original series.
> 
> Both patches are compile-tested only.

Armin, thank you for creating a new cleaned up driver for the
quickstart button support.

I have managed to get my hands on a Toshiba Portege Z830 and
I have successfully tested this series. That is this makes
the 2 quickstart application and the toggle-touchpad button
work when the system is running normally.

Neither the quickstart buttons, nor the touchpad-toggle button
which also uses the PNP0C32 interface, work to wakeup
the system from sleep though.

I've also review both patches and they look good to me:

Tested-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>

So I plan to merge this series into pdx86/for-next once
6.9-rc1 is out.

Regards,

Hans



> Armin Wolf (1):
>   platform/x86: Add ACPI quickstart button (PNP0C32) driver
> 
> Arvid Norlander (1):
>   platform/x86: toshiba_acpi: Add quirk for buttons on Z830
> 
>  MAINTAINERS                         |   6 +
>  drivers/platform/x86/Kconfig        |  13 ++
>  drivers/platform/x86/Makefile       |   3 +
>  drivers/platform/x86/quickstart.c   | 225 ++++++++++++++++++++++++++++
>  drivers/platform/x86/toshiba_acpi.c |  36 ++++-
>  5 files changed, 280 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/platform/x86/quickstart.c
> 
> --
> 2.39.2
> 
> 

Re: [PATCH 0/2] platform/x86: Add ACPI quickstart button driver

[Armin Wolf]

Am 24.03.24 um 15:55 schrieb Hans de Goede:

> Hi Armin and Arvid,
>
> On 1/31/24 12:16 PM, Armin Wolf wrote:
>> This patch series adds support for the ACPI PNP0C32 device as
>> proposed in 2022 by Arvid Norlander. The first patch adds support
>> for the device itself, while the second patch was taken from the
>> original series.
>>
>> Both patches are compile-tested only.
> Armin, thank you for creating a new cleaned up driver for the
> quickstart button support.
>
> I have managed to get my hands on a Toshiba Portege Z830 and
> I have successfully tested this series. That is this makes
> the 2 quickstart application and the toggle-touchpad button
> work when the system is running normally.
>
> Neither the quickstart buttons, nor the touchpad-toggle button
> which also uses the PNP0C32 interface, work to wakeup
> the system from sleep though.
>
> I've also review both patches and they look good to me:
>
> Tested-by: Hans de Goede <hdegoede@redhat.com>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
>
> So I plan to merge this series into pdx86/for-next once
> 6.9-rc1 is out.
>
> Regards,
>
> Hans
>
Hi,

great to hear that the driver works. Can you send me the output of "acpidump" on this machine?
Maybe the quickstart buttons have no wake capabilities?

Thanks,
Armin Wolf

>> Armin Wolf (1):
>>    platform/x86: Add ACPI quickstart button (PNP0C32) driver
>>
>> Arvid Norlander (1):
>>    platform/x86: toshiba_acpi: Add quirk for buttons on Z830
>>
>>   MAINTAINERS                         |   6 +
>>   drivers/platform/x86/Kconfig        |  13 ++
>>   drivers/platform/x86/Makefile       |   3 +
>>   drivers/platform/x86/quickstart.c   | 225 ++++++++++++++++++++++++++++
>>   drivers/platform/x86/toshiba_acpi.c |  36 ++++-
>>   5 files changed, 280 insertions(+), 3 deletions(-)
>>   create mode 100644 drivers/platform/x86/quickstart.c
>>
>> --
>> 2.39.2
>>
>>
>

Re: [PATCH 0/2] platform/x86: Add ACPI quickstart button driver

[Arvid Norlander]

On 2024-03-24 15:55, Hans de Goede wrote:
> Hi Armin and Arvid,
> 
> On 1/31/24 12:16 PM, Armin Wolf wrote:
>> This patch series adds support for the ACPI PNP0C32 device as
>> proposed in 2022 by Arvid Norlander. The first patch adds support
>> for the device itself, while the second patch was taken from the
>> original series.
>>
>> Both patches are compile-tested only.
> 
> Armin, thank you for creating a new cleaned up driver for the
> quickstart button support.
> 
> I have managed to get my hands on a Toshiba Portege Z830 and
> I have successfully tested this series. That is this makes
> the 2 quickstart application and the toggle-touchpad button
> work when the system is running normally.

That is some dedication! Sorry for completely dropping the ball on this,
real life happened and I had to prioritise. My beginning "carreer" as a
kernel developer was one of the things that ended up getting cut.

I'm going to do a quick bug report while I'm writing an email here. I
noticed on my Z830 that the readouts of the charge control thresholds
didn't seem to work when I last tested it. Setting worked just fine, but
reading it always returns 100%. Didn't get around to debugging this, but I
assume it a simple error.

Also my Z830 have developed a whole horizontal line of stuck red pixels
across the middle of the screen, so it seems quite unlikely I'll use it
much more or even keep it long term.

> Neither the quickstart buttons, nor the touchpad-toggle button
> which also uses the PNP0C32 interface, work to wakeup
> the system from sleep though.

This is probably a limitation of the hardware. Though there are IIRC some
BIOS options for what should wake the laptop from power-off, including any
keyboard press whatsoever. (Fun fact: The Toshiba ACPI interface allows
changing these BIOS settings from the OS, though you would have to do it by
hand from user space currently as it is not exposed in the driver.)

Best regards,
Arvid Norlander

Re: [PATCH 0/2] platform/x86: Add ACPI quickstart button driver

[Hans de Goede]

Hi,

On 1/31/24 12:16 PM, Armin Wolf wrote:
> This patch series adds support for the ACPI PNP0C32 device as
> proposed in 2022 by Arvid Norlander. The first patch adds support
> for the device itself, while the second patch was taken from the
> original series.
> 
> Both patches are compile-tested only.

Not anymore :)


Thank you for your patch-series, I've applied the series 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






> 
> Armin Wolf (1):
>   platform/x86: Add ACPI quickstart button (PNP0C32) driver
> 
> Arvid Norlander (1):
>   platform/x86: toshiba_acpi: Add quirk for buttons on Z830
> 
>  MAINTAINERS                         |   6 +
>  drivers/platform/x86/Kconfig        |  13 ++
>  drivers/platform/x86/Makefile       |   3 +
>  drivers/platform/x86/quickstart.c   | 225 ++++++++++++++++++++++++++++
>  drivers/platform/x86/toshiba_acpi.c |  36 ++++-
>  5 files changed, 280 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/platform/x86/quickstart.c
> 
> --
> 2.39.2
>