Re: [PATCH] Revert "platform/x86: toshiba_acpi: Add quirk for buttons on Z830"

Armin Wolf <W_Armin@xxxxxx> · Tue, 2 Jul 2024 21:17:27 +0200

Am 02.07.24 um 16:13 schrieb Hans de Goede:

Hi Armin,

On 7/2/24 2:06 PM, Armin Wolf wrote:
This reverts commit 10c66da9f87a96572ad92642ae060e827313b11c.

The associated patch depends on the availability of the ACPI
quickstart button driver, which will be available starting with
kernel 6.10. This means that the patch brings no benifit for
older kernels.

Even worse, it was found out that the patch is buggy, causing
regressions for people using older kernels.

Fix this by simply reverting the patch from the 6.9 stable tree.
The fix heading toward mainline:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/commit/?h=fixes&id=e527a6127223b644e0a27b44f4b16e16eb6c7f0a

should work fine for the stable branches too and  AFAIK the stable
maintainer prefer to have a mainline fix over a stable specific fix.

Ok, in this case i will take back the three patches.


I have added a Cc: stable to the fix and I plan to submit a PR with
this to Linus this Thursday, after which the stable scripts should
pick it up automatically for all relevant maintained branches since
it also has a Fixes: tag.

Regards,

Hans

Cc: <stable@xxxxxxxxxxxxxxx> # 6.9.x
p.s.

I believe that you could have used:

Cc: <stable@xxxxxxxxxxxxxxx> # 6.1.x, 6.6.x, 6.9.x

here instead of sending this 3 times with only the version in
the stable tag being different in the 3 versions.

The commit id of the reverted commit was different for each kernel version,
that why i decided to send three different patches.

Thanks,
Armin Wolf


Reported-by: kemal <kmal@xxxxxxx>
Signed-off-by: Armin Wolf <W_Armin@xxxxxx>
---
  drivers/platform/x86/toshiba_acpi.c | 36 +++--------------------------
  1 file changed, 3 insertions(+), 33 deletions(-)

diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index 16e941449b14..77244c9aa60d 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -57,11 +57,6 @@ 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 */
@@ -141,7 +136,6 @@ MODULE_PARM_DESC(hci_hotkey_quickstart,
  #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
@@ -2737,15 +2731,10 @@ 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 (hci_hotkey_quickstart)
-		result = hci_write(dev, HCI_HOTKEY_EVENT,
-				   HCI_HOTKEY_ENABLE_QUICKSTART);
-	else if (dev->kbd_function_keys_supported && dev->special_functions)
+	if (dev->kbd_function_keys_supported && dev->special_functions)
  		result = hci_write(dev, HCI_HOTKEY_EVENT,
  				   HCI_HOTKEY_SPECIAL_FUNCTIONS);
  	else
@@ -3269,14 +3258,7 @@ 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.
   */
-#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[] = {
+static const struct dmi_system_id turn_on_panel_on_resume_dmi_ids[] = {
  	{
  	 /* Toshiba Portégé R700 */
  	 /* https://bugzilla.kernel.org/show_bug.cgi?id=21012 */
@@ -3284,7 +3266,6 @@ static const struct dmi_system_id toshiba_dmi_quirks[] = {
  		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 */
@@ -3294,7 +3275,6 @@ static const struct dmi_system_id toshiba_dmi_quirks[] = {
  		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 */
@@ -3302,7 +3282,6 @@ static const struct dmi_system_id toshiba_dmi_quirks[] = {
  		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),
  	},
  };

@@ -3311,8 +3290,6 @@ 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)
@@ -3465,15 +3442,8 @@ 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 = !!(quirks & QUIRK_TURN_ON_PANEL_ON_RESUME);
-
-	if (hci_hotkey_quickstart == -1)
-		hci_hotkey_quickstart = !!(quirks & QUIRK_HCI_HOTKEY_QUICKSTART);
+		turn_on_panel_on_resume = dmi_check_system(turn_on_panel_on_resume_dmi_ids);

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









