Linux Input/HID development

Linux Input/HID development
 help / color / mirror / Atom feed

* Re: [PATCH 01/32] Input: cyttsp5 - Use %pe format specifier
From: Frank Li @ 2025-10-13 15:35 UTC (permalink / raw)
  To: Ricardo Ribalda
  Cc: Linus Walleij, Dmitry Torokhov, Mauro Carvalho Chehab,
	Hans Verkuil, Sakari Ailus, Krzysztof Hałasa, Tomi Valkeinen,
	Leon Luo, Kieran Bingham, Jacopo Mondi, Kieran Bingham,
	Laurent Pinchart, Niklas Söderlund, Julien Massot,
	Jacopo Mondi, Daniel Scally, Dave Stevenson, Benjamin Mugnier,
	Sylvain Petinot, Yong Zhi, Bingbu Cao, Tianshu Qiu, Tiffany Lin,
	Andrew-CT Chen, Yunfei Dong, Matthias Brugger,
	AngeloGioacchino Del Regno, Rui Miguel Silva, Laurent Pinchart,
	Martin Kepplinger, Purism Kernel Team, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Dafna Hirschfeld,
	Heiko Stuebner, Sylwester Nawrocki, Krzysztof Kozlowski,
	Alim Akhtar, Yemike Abhilash Chandra, Greg Kroah-Hartman,
	linux-input, linux-kernel, linux-media, linux-arm-kernel,
	linux-mediatek, imx, linux-renesas-soc, linux-rockchip,
	linux-samsung-soc, linux-staging
In-Reply-To: <20251013-ptr_err-v1-1-2c5efbd82952@chromium.org>

On Mon, Oct 13, 2025 at 02:14:41PM +0000, Ricardo Ribalda wrote:
> The %pe format specifier is designed to print error pointers. It prints
> a symbolic error name (eg. -EINVAL) and it makes the code simpler by
> omitting PTR_ERR()
>
> This patch fixes this cocci report:
> ./cyttsp5.c:927:3-10: WARNING: Consider using %pe to print PTR_ERR()
>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  drivers/input/touchscreen/cyttsp5.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Suppose it will go though input subsystem intead of media.
Need post seperated at difference thread?

Frank

>
> diff --git a/drivers/input/touchscreen/cyttsp5.c b/drivers/input/touchscreen/cyttsp5.c
> index 071b7c9bf566eb0b58e302a941ec085be1eb5683..47f4271395a69b8350f9be7266b57fe11d442ee3 100644
> --- a/drivers/input/touchscreen/cyttsp5.c
> +++ b/drivers/input/touchscreen/cyttsp5.c
> @@ -923,8 +923,8 @@ static int cyttsp5_i2c_probe(struct i2c_client *client)
>
>  	regmap = devm_regmap_init_i2c(client, &config);
>  	if (IS_ERR(regmap)) {
> -		dev_err(&client->dev, "regmap allocation failed: %ld\n",
> -			PTR_ERR(regmap));
> +		dev_err(&client->dev, "regmap allocation failed: %pe\n",
> +			regmap);
>  		return PTR_ERR(regmap);
>  	}
>
>
> --
> 2.51.0.760.g7b8bcc2412-goog
>

^ permalink raw reply

* [PATCH] input: Add marine keycodes for radar control.
From: Hunter Moore @ 2025-10-13 15:23 UTC (permalink / raw)
  To: dmitry.torokhov; +Cc: linux-input, linux-kernel, Hunter Moore, Hunter Moore

From: Hunter Moore <hunter.moore@garmin.com>

We are looking into adding some additional keycodes to add support for
radar control. In total we are adding 29 keycodes which seems to
exceed the KEY_MAX limit set, so we have also bumped this value to
0x3ff to double the available keycodes.

Signed-off-by: Hunter Moore <Hunter.Moore@garmin.com>
---
 include/linux/mod_devicetable.h        |  2 +-
 include/uapi/linux/input-event-codes.h | 32 +++++++++++++++++++++++++-
 2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 6077972e8b45..97f8867955de 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -333,7 +333,7 @@ struct pcmcia_device_id {
 /* Input */
 #define INPUT_DEVICE_ID_EV_MAX         0x1f
 #define INPUT_DEVICE_ID_KEY_MIN_INTERESTING    0x71
-#define INPUT_DEVICE_ID_KEY_MAX                0x2ff
+#define INPUT_DEVICE_ID_KEY_MAX                0x3ff
 #define INPUT_DEVICE_ID_REL_MAX                0x0f
 #define INPUT_DEVICE_ID_ABS_MAX                0x3f
 #define INPUT_DEVICE_ID_MSC_MAX                0x07
diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
index ca5851e97fac..49a7fd7c0e23 100644
--- a/include/uapi/linux/input-event-codes.h
+++ b/include/uapi/linux/input-event-codes.h
@@ -815,9 +815,39 @@
 #define BTN_TRIGGER_HAPPY39            0x2e6
 #define BTN_TRIGGER_HAPPY40            0x2e7

+#define KEY_RADAR_TRANSMIT             0x2e8
+#define KEY_RADAR_RANGE_UP             0x2e9
+#define KEY_RADAR_RANGE_DOWN           0x2ea
+#define KEY_RADAR_GAIN_UP              0x2eb
+#define KEY_RADAR_GAIN_DOWN            0x2ec
+#define KEY_RADAR_SEA_UP               0x2ed
+#define KEY_RADAR_SEA_DOWN             0x2ee
+#define KEY_RADAR_RAIN_UP              0x2ef
+#define KEY_RADAR_RAIN_DOWN            0x2f0
+#define KEY_RADAR_VRM1_UP              0x2f1
+#define KEY_RADAR_VRM1_DOWN            0x2f2
+#define KEY_RADAR_VRM2_UP              0x2f3
+#define KEY_RADAR_VRM2_DOWN            0x2f4
+#define KEY_RADAR_EBL1_UP              0x2f5
+#define KEY_RADAR_EBL1_DOWN            0x2f6
+#define KEY_RADAR_EBL2_UP              0x2f7
+#define KEY_RADAR_EBL2_DOWN            0x2f8
+#define KEY_RADAR_VRM_TOGGLE           0x2f9
+#define KEY_RADAR_VRM_UP               0x2fa
+#define KEY_RADAR_VRM_DOWN             0x2fb
+#define KEY_RADAR_EBL_TOGGLE           0x2fc
+#define KEY_RADAR_EBL_UP               0x2fd
+#define KEY_RADAR_EBL_DOWN             0x2fe
+#define KEY_RADAR_SCANNER_TOGGLE       0x2ff
+#define KEY_RADAR_SCANNER1             0x300
+#define KEY_RADAR_SCANNER2             0x301
+#define KEY_RADAR_CHANNEL_TOGGLE       0x302
+#define KEY_RADAR_CHANNEL_A            0x303
+#define KEY_RADAR_CHANNEL_B            0x304
+
 /* We avoid low common keys in module aliases so they don't get huge. */
 #define KEY_MIN_INTERESTING    KEY_MUTE
-#define KEY_MAX                        0x2ff
+#define KEY_MAX                        0x3ff
 #define KEY_CNT                        (KEY_MAX+1)

 /*
--
2.43.0

________________________________

CONFIDENTIALITY NOTICE: This email and any attachments are for the sole use of the intended recipient(s) and contain information that may be Garmin confidential and/or Garmin legally privileged. If you have received this email in error, please notify the sender by reply email and delete the message. Any disclosure, copying, distribution or use of this communication (including attachments) by someone other than the intended recipient is prohibited. Thank you.

________________________________

CONFIDENTIALITY NOTICE: This email and any attachments are for the sole use of the intended recipient(s) and contain information that may be Garmin confidential and/or Garmin legally privileged. If you have received this email in error, please notify the sender by reply email and delete the message. Any disclosure, copying, distribution or use of this communication (including attachments) by someone other than the intended recipient is prohibited. Thank you.

^ permalink raw reply related

* Re: [PATCH 01/32] Input: cyttsp5 - Use %pe format specifier
From: Ricardo Ribalda @ 2025-10-13 15:50 UTC (permalink / raw)
  To: Frank Li
  Cc: Linus Walleij, Dmitry Torokhov, Mauro Carvalho Chehab,
	Hans Verkuil, Sakari Ailus, Krzysztof Hałasa, Tomi Valkeinen,
	Leon Luo, Kieran Bingham, Jacopo Mondi, Kieran Bingham,
	Laurent Pinchart, Niklas Söderlund, Julien Massot,
	Jacopo Mondi, Daniel Scally, Dave Stevenson, Benjamin Mugnier,
	Sylvain Petinot, Yong Zhi, Bingbu Cao, Tianshu Qiu, Tiffany Lin,
	Andrew-CT Chen, Yunfei Dong, Matthias Brugger,
	AngeloGioacchino Del Regno, Rui Miguel Silva, Laurent Pinchart,
	Martin Kepplinger, Purism Kernel Team, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Dafna Hirschfeld,
	Heiko Stuebner, Sylwester Nawrocki, Krzysztof Kozlowski,
	Alim Akhtar, Yemike Abhilash Chandra, Greg Kroah-Hartman,
	linux-input, linux-kernel, linux-media, linux-arm-kernel,
	linux-mediatek, imx, linux-renesas-soc, linux-rockchip,
	linux-samsung-soc, linux-staging
In-Reply-To: <aO0cXYeGLwwDABP6@lizhi-Precision-Tower-5810>

Hi Frank

On Mon, 13 Oct 2025 at 17:36, Frank Li <Frank.li@nxp.com> wrote:
>
> On Mon, Oct 13, 2025 at 02:14:41PM +0000, Ricardo Ribalda wrote:
> > The %pe format specifier is designed to print error pointers. It prints
> > a symbolic error name (eg. -EINVAL) and it makes the code simpler by
> > omitting PTR_ERR()
> >
> > This patch fixes this cocci report:
> > ./cyttsp5.c:927:3-10: WARNING: Consider using %pe to print PTR_ERR()
> >
> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > ---
> >  drivers/input/touchscreen/cyttsp5.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
>
> Suppose it will go though input subsystem intead of media.
> Need post seperated at difference thread?

Indeed, it belongs to input. if there is a v2 I will move it to a
different thread.

Thanks!

>
> Frank
>
> >
> > diff --git a/drivers/input/touchscreen/cyttsp5.c b/drivers/input/touchscreen/cyttsp5.c
> > index 071b7c9bf566eb0b58e302a941ec085be1eb5683..47f4271395a69b8350f9be7266b57fe11d442ee3 100644
> > --- a/drivers/input/touchscreen/cyttsp5.c
> > +++ b/drivers/input/touchscreen/cyttsp5.c
> > @@ -923,8 +923,8 @@ static int cyttsp5_i2c_probe(struct i2c_client *client)
> >
> >       regmap = devm_regmap_init_i2c(client, &config);
> >       if (IS_ERR(regmap)) {
> > -             dev_err(&client->dev, "regmap allocation failed: %ld\n",
> > -                     PTR_ERR(regmap));
> > +             dev_err(&client->dev, "regmap allocation failed: %pe\n",
> > +                     regmap);
> >               return PTR_ERR(regmap);
> >       }
> >
> >
> > --
> > 2.51.0.760.g7b8bcc2412-goog
> >



-- 
Ricardo Ribalda

^ permalink raw reply

* Re: [PATCH 10/32] media: i2c: imx335: Use %pe format specifier
From: Ricardo Ribalda @ 2025-10-13 15:55 UTC (permalink / raw)
  To: Kieran Bingham
  Cc: Alim Akhtar, Andrew-CT Chen, AngeloGioacchino Del Regno,
	Benjamin Mugnier, Bingbu Cao, Dafna Hirschfeld, Daniel Scally,
	Dave Stevenson, Dmitry Torokhov, Fabio Estevam,
	Greg Kroah-Hartman, Hans Verkuil, Heiko Stuebner, Jacopo Mondi,
	Jacopo Mondi, Julien Massot, Krzysztof Hałasa,
	Krzysztof Kozlowski, Laurent Pinchart, Laurent Pinchart, Leon Luo,
	Linus Walleij, Matthias Brugger, Mauro Carvalho Chehab,
	Niklas Söderlund, Pengutronix Kernel Team,
	Purism Kernel Team, Rui Miguel Silva, Sakari Ailus, Sascha Hauer,
	Shawn Guo, Sylvain Petinot, Sylwester Nawrocki, Tianshu Qiu,
	Tiffany Lin, Tomi Valkeinen, Yemike Abhilash Chandra, Yong Zhi,
	Yunfei Dong, linux-input, linux-kernel, linux-media,
	linux-arm-kernel, linux-mediatek, imx, linux-renesas-soc,
	linux-rockchip, linux-samsung-soc, linux-staging
In-Reply-To: <176036780330.559803.287308146210017676@ping.linuxembedded.co.uk>

Hi Kieran

On Mon, 13 Oct 2025 at 17:03, Kieran Bingham
<kieran.bingham@ideasonboard.com> wrote:
>
> Quoting Ricardo Ribalda (2025-10-13 15:14:50)
> > The %pe format specifier is designed to print error pointers. It prints
> > a symbolic error name (eg. -EINVAL) and it makes the code simpler by
> > omitting PTR_ERR().
> >
> > This patch fixes this cocci report:
> > ./i2c/imx335.c:1013:3-10: WARNING: Consider using %pe to print PTR_ERR()
>
> Ohhh nice. Is this new ? First I've come across it.

It is actually from 2019:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=57f5677e535ba24b8926a7125be2ef8d7f09323c

I just learned about it because there is a new check in coccinelle :).

It is pretty cool, but you need to be careful to check IS_ERR(ptr)
before doing the printk, otherwise %pe will print the pointer value.

Regards!

>
>
>
> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > ---
> >  drivers/media/i2c/imx335.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/i2c/imx335.c b/drivers/media/i2c/imx335.c
> > index c043df2f15fb25b3a56422092f99a1fd9a508fa9..71ed9a0d84a252ee362621c4d38001508fb86d28 100644
> > --- a/drivers/media/i2c/imx335.c
> > +++ b/drivers/media/i2c/imx335.c
> > @@ -1009,8 +1009,8 @@ static int imx335_parse_hw_config(struct imx335 *imx335)
> >         imx335->reset_gpio = devm_gpiod_get_optional(imx335->dev, "reset",
> >                                                      GPIOD_OUT_HIGH);
> >         if (IS_ERR(imx335->reset_gpio)) {
> > -               dev_err(imx335->dev, "failed to get reset gpio %ld\n",
> > -                       PTR_ERR(imx335->reset_gpio));
> > +               dev_err(imx335->dev, "failed to get reset gpio %pe\n",
> > +                       imx335->reset_gpio);
>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>
> >                 return PTR_ERR(imx335->reset_gpio);
> >         }
> >
> >
> > --
> > 2.51.0.760.g7b8bcc2412-goog
> >



-- 
Ricardo Ribalda

^ permalink raw reply

* Re: [PATCH] input: Add marine keycodes for radar control.
From: Dmitry Torokhov @ 2025-10-13 16:05 UTC (permalink / raw)
  To: Hunter Moore; +Cc: linux-input, linux-kernel
In-Reply-To: <20251013152309.880-1-Hunter.Moore@garmin.com>

Hi Hunter,

On Mon, Oct 13, 2025 at 10:23:09AM -0500, Hunter Moore wrote:
> From: Hunter Moore <hunter.moore@garmin.com>
> 
> We are looking into adding some additional keycodes to add support for
> radar control. In total we are adding 29 keycodes which seems to
> exceed the KEY_MAX limit set, so we have also bumped this value to
> 0x3ff to double the available keycodes.

No, we will not be adding these new keys since I do not see any users of
the previously defined ones anywhere, not in kernel sources and not in
the HID specification.

You seem to be creating a purpose-built devices where you control your
userspace, and I do not think the new keycodes will be of any use to
anyone but your specific application. You are also unlikely to be
running anything else besides the software that you are developing on
these devices, so I'd recommend simply reuse parts of the existing key
code space for your purposes (KEY_MACRO*, BTN_TRIGGER_HAPPY*, etc).

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH resend] Input: goodix - add support for ACPI ID GDIX1003
From: Dmitry Torokhov @ 2025-10-13 16:35 UTC (permalink / raw)
  To: Hans de Goede; +Cc: Bastien Nocera, linux-input, Hans de Goede, Weikang Guo
In-Reply-To: <20251013121022.44333-1-hansg@kernel.org>

On Mon, Oct 13, 2025 at 02:10:22PM +0200, Hans de Goede wrote:
> From: Hans de Goede <hdegoede@redhat.com>
> 
> Some newer devices use an ACPI hardware ID of GDIX1003 for their Goodix
> touchscreen controller, instead of GDIX1001 / GDIX1002. Add GDIX1003
> to the goodix_acpi_match[] table.
> 
> Reported-by: Weikang Guo <guoweikang.kernel@gmail.com>
> Closes: https://lore.kernel.org/linux-input/20250225024409.1467040-1-guoweikang.kernel@gmail.com/
> Tested-by: Weikang Guo <guoweikang.kernel@gmail.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Applied, thank you.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v2] input: goodix: Remove setting of RST pin to input
From: Dmitry Torokhov @ 2025-10-13 16:38 UTC (permalink / raw)
  To: Martyn Welch; +Cc: Hans de Goede, kernel, linux-input, linux-kernel
In-Reply-To: <20251009134138.686215-1-martyn.welch@collabora.com>

On Thu, Oct 09, 2025 at 02:41:32PM +0100, Martyn Welch wrote:
> The reset line is being set to input on non-ACPI devices apparently to
> save power. This isn't being done on ACPI devices as it's been found
> that some ACPI devices don't have a pull-up resistor fitted. This can
> also be the case for non-ACPI devices, resulting in:
> 
> [  941.672207] Goodix-TS 1-0014: Error reading 10 bytes from 0x814e: -110
> [  942.696168] Goodix-TS 1-0014: Error reading 10 bytes from 0x814e: -110
> [  945.832208] Goodix-TS 1-0014: Error reading 10 bytes from 0x814e: -110
> 
> This behaviour appears to have been initialing introduced in
> ec6e1b4082d9. This doesn't seem to be based on information in either the
> GT911 or GT9271 datasheets cited as sources of information for this
> change. Thus it seems likely that it is based on functionality in the
> Android driver which it also lists. This behaviour may be viable in very
> specific instances where the hardware is well known, but seems unwise in
> the upstream kernel where such hardware requirements can't be
> guaranteed.
> 
> Remove this over optimisation to improve reliability on non-ACPI
> devices.
> 
> Signed-off-by: Martyn Welch <martyn.welch@collabora.com>
> 

Applied, thank you.

-- 
Dmitry

^ permalink raw reply

* [PATCH v6 1/7] HID: asus: refactor init sequence per spec
From: Antheas Kapenekakis @ 2025-10-13 20:15 UTC (permalink / raw)
  To: platform-driver-x86, linux-input
  Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Corentin Chary,
	Luke D . Jones, Hans de Goede, Ilpo Järvinen, Denis Benato,
	Antheas Kapenekakis
In-Reply-To: <20251013201535.6737-1-lkml@antheas.dev>

Currently, asus_kbd_init() uses a reverse engineered init sequence
from Windows, which contains the handshakes from multiple programs.
Keep the main one, which is 0x5a (meant for brightness drivers).

In addition, perform a get_response and check if the response is the
same. To avoid regressions, print an error if the response does not
match instead of rejecting device.

Then, refactor asus_kbd_get_functions() to use the same ID it is called
with, instead of hardcoding it to 0x5a so that it may be used for 0x0d
in the future.

Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
 drivers/hid/hid-asus.c | 91 ++++++++++++++++++++++--------------------
 1 file changed, 48 insertions(+), 43 deletions(-)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index a444d41e53b6..d0c783df99bc 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -48,7 +48,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
 #define FEATURE_REPORT_ID 0x0d
 #define INPUT_REPORT_ID 0x5d
 #define FEATURE_KBD_REPORT_ID 0x5a
-#define FEATURE_KBD_REPORT_SIZE 16
+#define FEATURE_KBD_REPORT_SIZE 64
 #define FEATURE_KBD_LED_REPORT_ID1 0x5d
 #define FEATURE_KBD_LED_REPORT_ID2 0x5e
 
@@ -393,14 +393,37 @@ static int asus_kbd_set_report(struct hid_device *hdev, const u8 *buf, size_t bu
 
 static int asus_kbd_init(struct hid_device *hdev, u8 report_id)
 {
-	const u8 buf[] = { report_id, 0x41, 0x53, 0x55, 0x53, 0x20, 0x54,
-		     0x65, 0x63, 0x68, 0x2e, 0x49, 0x6e, 0x63, 0x2e, 0x00 };
+	/*
+	 * The handshake is first sent as a set_report, then retrieved
+	 * from a get_report. They should be equal.
+	 */
+	const u8 buf[] = { report_id, 0x41, 0x53, 0x55, 0x53, 0x20,
+		0x54, 0x65, 0x63, 0x68, 0x2e, 0x49, 0x6e, 0x63, 0x2e, 0x00 };
+	u8 *readbuf;
 	int ret;
 
 	ret = asus_kbd_set_report(hdev, buf, sizeof(buf));
-	if (ret < 0)
-		hid_err(hdev, "Asus failed to send init command: %d\n", ret);
+	if (ret < 0) {
+		hid_err(hdev, "Asus failed to send handshake: %d\n", ret);
+		return ret;
+	}
+
+	readbuf = kzalloc(FEATURE_KBD_REPORT_SIZE, GFP_KERNEL);
+	if (!readbuf)
+		return -ENOMEM;
+
+	ret = hid_hw_raw_request(hdev, report_id, readbuf,
+				 FEATURE_KBD_REPORT_SIZE, HID_FEATURE_REPORT,
+				 HID_REQ_GET_REPORT);
+	if (ret < 0) {
+		hid_err(hdev, "Asus failed to receive handshake ack: %d\n", ret);
+	} else if (memcmp(readbuf, buf, sizeof(buf)) != 0) {
+		hid_warn(hdev, "Asus handshake returned invalid response: %*ph\n",
+			FEATURE_KBD_REPORT_SIZE, readbuf);
+		// Do not return error if handshake is wrong to avoid regressions
+	}
 
+	kfree(readbuf);
 	return ret;
 }
 
@@ -422,7 +445,7 @@ static int asus_kbd_get_functions(struct hid_device *hdev,
 	if (!readbuf)
 		return -ENOMEM;
 
-	ret = hid_hw_raw_request(hdev, FEATURE_KBD_REPORT_ID, readbuf,
+	ret = hid_hw_raw_request(hdev, report_id, readbuf,
 				 FEATURE_KBD_REPORT_SIZE, HID_FEATURE_REPORT,
 				 HID_REQ_GET_REPORT);
 	if (ret < 0) {
@@ -638,50 +661,32 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
 	unsigned char kbd_func;
 	int ret;
 
-	if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) {
-		/* Initialize keyboard */
-		ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
-		if (ret < 0)
-			return ret;
-
-		/* The LED endpoint is initialised in two HID */
-		ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID1);
-		if (ret < 0)
-			return ret;
-
-		ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID2);
-		if (ret < 0)
-			return ret;
-
-		if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
-			ret = asus_kbd_disable_oobe(hdev);
-			if (ret < 0)
-				return ret;
-		}
-
-		if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
-			intf = to_usb_interface(hdev->dev.parent);
-			udev = interface_to_usbdev(intf);
-			validate_mcu_fw_version(hdev,
-				le16_to_cpu(udev->descriptor.idProduct));
-		}
+	ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
+	if (ret < 0)
+		return ret;
 
-	} else {
-		/* Initialize keyboard */
-		ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
-		if (ret < 0)
-			return ret;
+	/* Get keyboard functions */
+	ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
+	if (ret < 0)
+		return ret;
 
-		/* Get keyboard functions */
-		ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
+	if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
+		ret = asus_kbd_disable_oobe(hdev);
 		if (ret < 0)
 			return ret;
+	}
 
-		/* Check for backlight support */
-		if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
-			return -ENODEV;
+	if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
+		intf = to_usb_interface(hdev->dev.parent);
+		udev = interface_to_usbdev(intf);
+		validate_mcu_fw_version(
+			hdev, le16_to_cpu(udev->descriptor.idProduct));
 	}
 
+	/* Check for backlight support */
+	if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
+		return -ENODEV;
+
 	drvdata->kbd_backlight = devm_kzalloc(&hdev->dev,
 					      sizeof(struct asus_kbd_leds),
 					      GFP_KERNEL);
-- 
2.51.0



^ permalink raw reply related

* [PATCH v6 4/7] HID: asus: listen to the asus-wmi brightness device instead of creating one
From: Antheas Kapenekakis @ 2025-10-13 20:15 UTC (permalink / raw)
  To: platform-driver-x86, linux-input
  Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Corentin Chary,
	Luke D . Jones, Hans de Goede, Ilpo Järvinen, Denis Benato,
	Antheas Kapenekakis
In-Reply-To: <20251013201535.6737-1-lkml@antheas.dev>

Some ROG laptops expose multiple interfaces for controlling the
keyboard/RGB brightness. This creates a name conflict under
asus::kbd_brightness, where the second device ends up being
named asus::kbd_brightness_1 and they are both broken.

Therefore, register a listener to the asus-wmi brightness device
instead of creating a new one.

Reviewed-by: Luke D. Jones <luke@ljones.dev>
Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
 drivers/hid/hid-asus.c | 64 +++++++-----------------------------------
 1 file changed, 10 insertions(+), 54 deletions(-)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index a62559e3e064..0af19c8ef035 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -102,7 +102,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
 #define TRKID_SGN       ((TRKID_MAX + 1) >> 1)
 
 struct asus_kbd_leds {
-	struct led_classdev cdev;
+	struct asus_hid_listener listener;
 	struct hid_device *hdev;
 	struct work_struct work;
 	unsigned int brightness;
@@ -495,11 +495,11 @@ static void asus_schedule_work(struct asus_kbd_leds *led)
 	spin_unlock_irqrestore(&led->lock, flags);
 }
 
-static void asus_kbd_backlight_set(struct led_classdev *led_cdev,
-				   enum led_brightness brightness)
+static void asus_kbd_backlight_set(struct asus_hid_listener *listener,
+				   int brightness)
 {
-	struct asus_kbd_leds *led = container_of(led_cdev, struct asus_kbd_leds,
-						 cdev);
+	struct asus_kbd_leds *led = container_of(listener, struct asus_kbd_leds,
+						 listener);
 	unsigned long flags;
 
 	spin_lock_irqsave(&led->lock, flags);
@@ -509,20 +509,6 @@ static void asus_kbd_backlight_set(struct led_classdev *led_cdev,
 	asus_schedule_work(led);
 }
 
-static enum led_brightness asus_kbd_backlight_get(struct led_classdev *led_cdev)
-{
-	struct asus_kbd_leds *led = container_of(led_cdev, struct asus_kbd_leds,
-						 cdev);
-	enum led_brightness brightness;
-	unsigned long flags;
-
-	spin_lock_irqsave(&led->lock, flags);
-	brightness = led->brightness;
-	spin_unlock_irqrestore(&led->lock, flags);
-
-	return brightness;
-}
-
 static void asus_kbd_backlight_work(struct work_struct *work)
 {
 	struct asus_kbd_leds *led = container_of(work, struct asus_kbd_leds, work);
@@ -539,34 +525,6 @@ static void asus_kbd_backlight_work(struct work_struct *work)
 		hid_err(led->hdev, "Asus failed to set keyboard backlight: %d\n", ret);
 }
 
-/* WMI-based keyboard backlight LED control (via asus-wmi driver) takes
- * precedence. We only activate HID-based backlight control when the
- * WMI control is not available.
- */
-static bool asus_kbd_wmi_led_control_present(struct hid_device *hdev)
-{
-	struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
-	u32 value;
-	int ret;
-
-	if (!IS_ENABLED(CONFIG_ASUS_WMI))
-		return false;
-
-	if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD &&
-			dmi_check_system(asus_use_hid_led_dmi_ids)) {
-		hid_info(hdev, "using HID for asus::kbd_backlight\n");
-		return false;
-	}
-
-	ret = asus_wmi_evaluate_method(ASUS_WMI_METHODID_DSTS,
-				       ASUS_WMI_DEVID_KBD_BACKLIGHT, 0, &value);
-	hid_dbg(hdev, "WMI backlight check: rc %d value %x", ret, value);
-	if (ret)
-		return false;
-
-	return !!(value & ASUS_WMI_DSTS_PRESENCE_BIT);
-}
-
 /*
  * We don't care about any other part of the string except the version section.
  * Example strings: FGA80100.RC72LA.312_T01, FGA80100.RC71LS.318_T01
@@ -701,14 +659,11 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
 	drvdata->kbd_backlight->removed = false;
 	drvdata->kbd_backlight->brightness = 0;
 	drvdata->kbd_backlight->hdev = hdev;
-	drvdata->kbd_backlight->cdev.name = "asus::kbd_backlight";
-	drvdata->kbd_backlight->cdev.max_brightness = 3;
-	drvdata->kbd_backlight->cdev.brightness_set = asus_kbd_backlight_set;
-	drvdata->kbd_backlight->cdev.brightness_get = asus_kbd_backlight_get;
+	drvdata->kbd_backlight->listener.brightness_set = asus_kbd_backlight_set;
 	INIT_WORK(&drvdata->kbd_backlight->work, asus_kbd_backlight_work);
 	spin_lock_init(&drvdata->kbd_backlight->lock);
 
-	ret = devm_led_classdev_register(&hdev->dev, &drvdata->kbd_backlight->cdev);
+	ret = asus_hid_register_listener(&drvdata->kbd_backlight->listener);
 	if (ret < 0) {
 		/* No need to have this still around */
 		devm_kfree(&hdev->dev, drvdata->kbd_backlight);
@@ -1105,7 +1060,7 @@ static int __maybe_unused asus_resume(struct hid_device *hdev) {
 
 	if (drvdata->kbd_backlight) {
 		const u8 buf[] = { FEATURE_KBD_REPORT_ID, 0xba, 0xc5, 0xc4,
-				drvdata->kbd_backlight->cdev.brightness };
+				drvdata->kbd_backlight->brightness };
 		ret = asus_kbd_set_report(hdev, buf, sizeof(buf));
 		if (ret < 0) {
 			hid_err(hdev, "Asus failed to set keyboard backlight: %d\n", ret);
@@ -1241,7 +1196,6 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	}
 
 	if (is_vendor && (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT) &&
-	    !asus_kbd_wmi_led_control_present(hdev) &&
 	    asus_kbd_register_leds(hdev))
 		hid_warn(hdev, "Failed to initialize backlight.\n");
 
@@ -1282,6 +1236,8 @@ static void asus_remove(struct hid_device *hdev)
 	unsigned long flags;
 
 	if (drvdata->kbd_backlight) {
+		asus_hid_unregister_listener(&drvdata->kbd_backlight->listener);
+
 		spin_lock_irqsave(&drvdata->kbd_backlight->lock, flags);
 		drvdata->kbd_backlight->removed = true;
 		spin_unlock_irqrestore(&drvdata->kbd_backlight->lock, flags);
-- 
2.51.0



^ permalink raw reply related

* [PATCH v6 2/7] HID: asus: prevent binding to all HID devices on ROG
From: Antheas Kapenekakis @ 2025-10-13 20:15 UTC (permalink / raw)
  To: platform-driver-x86, linux-input
  Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Corentin Chary,
	Luke D . Jones, Hans de Goede, Ilpo Järvinen, Denis Benato,
	Antheas Kapenekakis
In-Reply-To: <20251013201535.6737-1-lkml@antheas.dev>

ROG keyboards are HID compliant and only care about the endpoint that
produces vendor events (e.g., fan mode) and has the keyboard backlight.

Therefore, handle all of the endpoints of ROG keyboards as compliant,
by adding HID_QUIRK_INPUT_PER_APP and, for devices other than the vendor
one, by adding QUIRK_HANDLE_GENERIC to stop mutating them.

Due to HID_QUIRK_INPUT_PER_APP, rgb register is moved into probe, as
the input_* functions are called multiple times (4 for the Z13).

Reviewed-by: Luke D. Jones <luke@ljones.dev>
Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
 drivers/hid/hid-asus.c | 71 +++++++++++++++++++++++++++++-------------
 1 file changed, 50 insertions(+), 21 deletions(-)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index d0c783df99bc..a62559e3e064 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -47,6 +47,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
 #define T100CHI_MOUSE_REPORT_ID 0x06
 #define FEATURE_REPORT_ID 0x0d
 #define INPUT_REPORT_ID 0x5d
+#define HID_USAGE_PAGE_VENDOR 0xff310000
 #define FEATURE_KBD_REPORT_ID 0x5a
 #define FEATURE_KBD_REPORT_SIZE 64
 #define FEATURE_KBD_LED_REPORT_ID1 0x5d
@@ -89,6 +90,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
 #define QUIRK_ROG_NKEY_KEYBOARD		BIT(11)
 #define QUIRK_ROG_CLAYMORE_II_KEYBOARD BIT(12)
 #define QUIRK_ROG_ALLY_XPAD		BIT(13)
+#define QUIRK_HANDLE_GENERIC		BIT(14)
 
 #define I2C_KEYBOARD_QUIRKS			(QUIRK_FIX_NOTEBOOK_REPORT | \
 						 QUIRK_NO_INIT_REPORTS | \
@@ -125,7 +127,6 @@ struct asus_drvdata {
 	struct input_dev *tp_kbd_input;
 	struct asus_kbd_leds *kbd_backlight;
 	const struct asus_touchpad_info *tp;
-	bool enable_backlight;
 	struct power_supply *battery;
 	struct power_supply_desc battery_desc;
 	int battery_capacity;
@@ -316,7 +317,7 @@ static int asus_e1239t_event(struct asus_drvdata *drvdat, u8 *data, int size)
 static int asus_event(struct hid_device *hdev, struct hid_field *field,
 		      struct hid_usage *usage, __s32 value)
 {
-	if ((usage->hid & HID_USAGE_PAGE) == 0xff310000 &&
+	if ((usage->hid & HID_USAGE_PAGE) == HID_USAGE_PAGE_VENDOR &&
 	    (usage->hid & HID_USAGE) != 0x00 &&
 	    (usage->hid & HID_USAGE) != 0xff && !usage->type) {
 		hid_warn(hdev, "Unmapped Asus vendor usagepage code 0x%02x\n",
@@ -331,6 +332,10 @@ static int asus_raw_event(struct hid_device *hdev,
 {
 	struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
 
+	/* NOOP on generic HID devices to avoid side effects. */
+	if (drvdata->quirks & QUIRK_HANDLE_GENERIC)
+		return 0;
+
 	if (drvdata->battery && data[0] == BATTERY_REPORT_ID)
 		return asus_report_battery(drvdata, data, size);
 
@@ -875,6 +880,10 @@ static int asus_input_configured(struct hid_device *hdev, struct hid_input *hi)
 	struct input_dev *input = hi->input;
 	struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
 
+	/* NOOP on generic HID devices to avoid side effects. */
+	if (drvdata->quirks & QUIRK_HANDLE_GENERIC)
+		return 0;
+
 	/* T100CHI uses MULTI_INPUT, bind the touchpad to the mouse hid_input */
 	if (drvdata->quirks & QUIRK_T100CHI &&
 	    hi->report->id != T100CHI_MOUSE_REPORT_ID)
@@ -928,11 +937,6 @@ static int asus_input_configured(struct hid_device *hdev, struct hid_input *hi)
 
 	drvdata->input = input;
 
-	if (drvdata->enable_backlight &&
-	    !asus_kbd_wmi_led_control_present(hdev) &&
-	    asus_kbd_register_leds(hdev))
-		hid_warn(hdev, "Failed to initialize backlight.\n");
-
 	return 0;
 }
 
@@ -952,6 +956,10 @@ static int asus_input_mapping(struct hid_device *hdev,
 		return -1;
 	}
 
+	/* NOOP on generic HID devices to avoid side effects. */
+	if (drvdata->quirks & QUIRK_HANDLE_GENERIC)
+		return 0;
+
 	/*
 	 * Ignore a bunch of bogus collections in the T100CHI descriptor.
 	 * This avoids a bunch of non-functional hid_input devices getting
@@ -1005,15 +1013,6 @@ static int asus_input_mapping(struct hid_device *hdev,
 			return -1;
 		}
 
-		/*
-		 * Check and enable backlight only on devices with UsagePage ==
-		 * 0xff31 to avoid initializing the keyboard firmware multiple
-		 * times on devices with multiple HID descriptors but same
-		 * PID/VID.
-		 */
-		if (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT)
-			drvdata->enable_backlight = true;
-
 		set_bit(EV_REP, hi->input->evbit);
 		return 1;
 	}
@@ -1130,8 +1129,10 @@ static int __maybe_unused asus_reset_resume(struct hid_device *hdev)
 
 static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
 {
-	int ret;
+	struct hid_report_enum *rep_enum;
 	struct asus_drvdata *drvdata;
+	struct hid_report *rep;
+	int ret, is_vendor = 0;
 
 	drvdata = devm_kzalloc(&hdev->dev, sizeof(*drvdata), GFP_KERNEL);
 	if (drvdata == NULL) {
@@ -1215,18 +1216,42 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		return ret;
 	}
 
+	/* Check for vendor for RGB init and handle generic devices properly. */
+	rep_enum = &hdev->report_enum[HID_INPUT_REPORT];
+	list_for_each_entry(rep, &rep_enum->report_list, list) {
+		if ((rep->application & HID_USAGE_PAGE) == HID_USAGE_PAGE_VENDOR)
+			is_vendor = true;
+	}
+
+	/*
+	 * For ROG keyboards, make them hid compliant by
+	 * creating one input per application. For interfaces other than
+	 * the vendor one, disable hid-asus handlers.
+	 */
+	if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) {
+		if (!is_vendor)
+			drvdata->quirks |= QUIRK_HANDLE_GENERIC;
+		hdev->quirks |= HID_QUIRK_INPUT_PER_APP;
+	}
+
 	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
 	if (ret) {
 		hid_err(hdev, "Asus hw start failed: %d\n", ret);
 		return ret;
 	}
 
+	if (is_vendor && (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT) &&
+	    !asus_kbd_wmi_led_control_present(hdev) &&
+	    asus_kbd_register_leds(hdev))
+		hid_warn(hdev, "Failed to initialize backlight.\n");
+
 	/*
-	 * Check that input registration succeeded. Checking that
-	 * HID_CLAIMED_INPUT is set prevents a UAF when all input devices
-	 * were freed during registration due to no usages being mapped,
-	 * leaving drvdata->input pointing to freed memory.
+	 * For ROG keyboards, skip rename for consistency and ->input check as
+	 * some devices do not have inputs.
 	 */
+	if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD)
+		return 0;
+
 	if (!drvdata->input || !(hdev->claimed & HID_CLAIMED_INPUT)) {
 		hid_err(hdev, "Asus input not registered\n");
 		ret = -ENOMEM;
@@ -1277,6 +1302,10 @@ static const __u8 *asus_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 {
 	struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
 
+	/* NOOP on generic HID devices to avoid side effects. */
+	if (drvdata->quirks & QUIRK_HANDLE_GENERIC)
+		return rdesc;
+
 	if (drvdata->quirks & QUIRK_FIX_NOTEBOOK_REPORT &&
 			*rsize >= 56 && rdesc[54] == 0x25 && rdesc[55] == 0x65) {
 		hid_info(hdev, "Fixing up Asus notebook report descriptor\n");
-- 
2.51.0



^ permalink raw reply related

* [PATCH v6 5/7] platform/x86: asus-wmi: remove unused keyboard backlight quirk
From: Antheas Kapenekakis @ 2025-10-13 20:15 UTC (permalink / raw)
  To: platform-driver-x86, linux-input
  Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Corentin Chary,
	Luke D . Jones, Hans de Goede, Ilpo Järvinen, Denis Benato,
	Antheas Kapenekakis
In-Reply-To: <20251013201535.6737-1-lkml@antheas.dev>

The quirk for selecting whether keyboard backlight should be controlled
by HID or WMI is not needed anymore, so remove it.

Reviewed-by: Luke D. Jones <luke@ljones.dev>
Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
 include/linux/platform_data/x86/asus-wmi.h | 40 ----------------------
 1 file changed, 40 deletions(-)

diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index f13a701f47a8..1f85d76387a8 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -202,44 +202,4 @@ static inline void asus_hid_unregister_listener(struct asus_hid_listener *bdev)
 }
 #endif
 
-/* To be used by both hid-asus and asus-wmi to determine which controls kbd_brightness */
-static const struct dmi_system_id asus_use_hid_led_dmi_ids[] = {
-	{
-		.matches = {
-			DMI_MATCH(DMI_PRODUCT_FAMILY, "ROG Zephyrus"),
-		},
-	},
-	{
-		.matches = {
-			DMI_MATCH(DMI_PRODUCT_FAMILY, "ROG Strix"),
-		},
-	},
-	{
-		.matches = {
-			DMI_MATCH(DMI_PRODUCT_FAMILY, "ROG Flow"),
-		},
-	},
-	{
-		.matches = {
-			DMI_MATCH(DMI_PRODUCT_FAMILY, "ProArt P16"),
-		},
-	},
-	{
-		.matches = {
-			DMI_MATCH(DMI_BOARD_NAME, "GA403U"),
-		},
-	},
-	{
-		.matches = {
-			DMI_MATCH(DMI_BOARD_NAME, "GU605M"),
-		},
-	},
-	{
-		.matches = {
-			DMI_MATCH(DMI_BOARD_NAME, "RC71L"),
-		},
-	},
-	{ },
-};
-
 #endif	/* __PLATFORM_DATA_X86_ASUS_WMI_H */
-- 
2.51.0



^ permalink raw reply related

* [PATCH v6 3/7] platform/x86: asus-wmi: Add support for multiple kbd RGB handlers
From: Antheas Kapenekakis @ 2025-10-13 20:15 UTC (permalink / raw)
  To: platform-driver-x86, linux-input
  Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Corentin Chary,
	Luke D . Jones, Hans de Goede, Ilpo Järvinen, Denis Benato,
	Antheas Kapenekakis
In-Reply-To: <20251013201535.6737-1-lkml@antheas.dev>

Some devices, such as the Z13 have multiple AURA devices connected
to them by USB. In addition, they might have a WMI interface for
RGB. In Windows, Armoury Crate exposes a unified brightness slider
for all of them, with 3 brightness levels.

Therefore, to be synergistic in Linux, and support existing tooling
such as UPower, allow adding listeners to the RGB device of the WMI
interface. If WMI does not exist, lazy initialize the interface.

Reviewed-by: Luke D. Jones <luke@ljones.dev>
Tested-by: Luke D. Jones <luke@ljones.dev>
Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
 drivers/platform/x86/asus-wmi.c            | 118 ++++++++++++++++++---
 include/linux/platform_data/x86/asus-wmi.h |  16 +++
 2 files changed, 121 insertions(+), 13 deletions(-)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index e72a2b5d158e..a2a7cd61fd59 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -258,6 +258,8 @@ struct asus_wmi {
 	int tpd_led_wk;
 	struct led_classdev kbd_led;
 	int kbd_led_wk;
+	bool kbd_led_avail;
+	bool kbd_led_registered;
 	struct led_classdev lightbar_led;
 	int lightbar_led_wk;
 	struct led_classdev micmute_led;
@@ -1530,6 +1532,53 @@ static void asus_wmi_battery_exit(struct asus_wmi *asus)
 
 /* LEDs ***********************************************************************/
 
+struct asus_hid_ref {
+	struct list_head listeners;
+	struct asus_wmi *asus;
+	spinlock_t lock;
+};
+
+struct asus_hid_ref asus_ref = {
+	.listeners = LIST_HEAD_INIT(asus_ref.listeners),
+	.asus = NULL,
+	.lock = __SPIN_LOCK_UNLOCKED(asus_ref.lock),
+};
+
+int asus_hid_register_listener(struct asus_hid_listener *bdev)
+{
+	unsigned long flags;
+	int ret = 0;
+
+	spin_lock_irqsave(&asus_ref.lock, flags);
+	list_add_tail(&bdev->list, &asus_ref.listeners);
+	if (asus_ref.asus) {
+		if (asus_ref.asus->kbd_led_registered && asus_ref.asus->kbd_led_wk >= 0)
+			bdev->brightness_set(bdev, asus_ref.asus->kbd_led_wk);
+
+		if (!asus_ref.asus->kbd_led_registered) {
+			ret = led_classdev_register(
+				&asus_ref.asus->platform_device->dev,
+				&asus_ref.asus->kbd_led);
+			if (!ret)
+				asus_ref.asus->kbd_led_registered = true;
+		}
+	}
+	spin_unlock_irqrestore(&asus_ref.lock, flags);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(asus_hid_register_listener);
+
+void asus_hid_unregister_listener(struct asus_hid_listener *bdev)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&asus_ref.lock, flags);
+	list_del(&bdev->list);
+	spin_unlock_irqrestore(&asus_ref.lock, flags);
+}
+EXPORT_SYMBOL_GPL(asus_hid_unregister_listener);
+
 /*
  * These functions actually update the LED's, and are called from a
  * workqueue. By doing this as separate work rather than when the LED
@@ -1609,6 +1658,7 @@ static int kbd_led_read(struct asus_wmi *asus, int *level, int *env)
 
 static void do_kbd_led_set(struct led_classdev *led_cdev, int value)
 {
+	struct asus_hid_listener *listener;
 	struct asus_wmi *asus;
 	int max_level;
 
@@ -1616,25 +1666,39 @@ static void do_kbd_led_set(struct led_classdev *led_cdev, int value)
 	max_level = asus->kbd_led.max_brightness;
 
 	asus->kbd_led_wk = clamp_val(value, 0, max_level);
-	kbd_led_update(asus);
+
+	if (asus->kbd_led_avail)
+		kbd_led_update(asus);
+
+	list_for_each_entry(listener, &asus_ref.listeners, list)
+		listener->brightness_set(listener, asus->kbd_led_wk);
 }
 
 static void kbd_led_set(struct led_classdev *led_cdev,
 			enum led_brightness value)
 {
+	unsigned long flags;
+
 	/* Prevent disabling keyboard backlight on module unregister */
 	if (led_cdev->flags & LED_UNREGISTERING)
 		return;
 
+	spin_lock_irqsave(&asus_ref.lock, flags);
 	do_kbd_led_set(led_cdev, value);
+	spin_unlock_irqrestore(&asus_ref.lock, flags);
 }
 
 static void kbd_led_set_by_kbd(struct asus_wmi *asus, enum led_brightness value)
 {
-	struct led_classdev *led_cdev = &asus->kbd_led;
+	struct led_classdev *led_cdev;
+	unsigned long flags;
+
+	spin_lock_irqsave(&asus_ref.lock, flags);
+	led_cdev = &asus->kbd_led;
 
 	do_kbd_led_set(led_cdev, value);
 	led_classdev_notify_brightness_hw_changed(led_cdev, asus->kbd_led_wk);
+	spin_unlock_irqrestore(&asus_ref.lock, flags);
 }
 
 static enum led_brightness kbd_led_get(struct led_classdev *led_cdev)
@@ -1644,6 +1708,9 @@ static enum led_brightness kbd_led_get(struct led_classdev *led_cdev)
 
 	asus = container_of(led_cdev, struct asus_wmi, kbd_led);
 
+	if (!asus->kbd_led_avail)
+		return asus->kbd_led_wk;
+
 	retval = kbd_led_read(asus, &value, NULL);
 	if (retval < 0)
 		return retval;
@@ -1759,7 +1826,15 @@ static int camera_led_set(struct led_classdev *led_cdev,
 
 static void asus_wmi_led_exit(struct asus_wmi *asus)
 {
-	led_classdev_unregister(&asus->kbd_led);
+	unsigned long flags;
+
+	spin_lock_irqsave(&asus_ref.lock, flags);
+	asus_ref.asus = NULL;
+	spin_unlock_irqrestore(&asus_ref.lock, flags);
+
+	if (asus->kbd_led_registered)
+		led_classdev_unregister(&asus->kbd_led);
+
 	led_classdev_unregister(&asus->tpd_led);
 	led_classdev_unregister(&asus->wlan_led);
 	led_classdev_unregister(&asus->lightbar_led);
@@ -1773,6 +1848,8 @@ static void asus_wmi_led_exit(struct asus_wmi *asus)
 static int asus_wmi_led_init(struct asus_wmi *asus)
 {
 	int rv = 0, num_rgb_groups = 0, led_val;
+	struct asus_hid_listener *listener;
+	unsigned long flags;
 
 	if (asus->kbd_rgb_dev)
 		kbd_rgb_mode_groups[num_rgb_groups++] = &kbd_rgb_mode_group;
@@ -1797,23 +1874,38 @@ static int asus_wmi_led_init(struct asus_wmi *asus)
 			goto error;
 	}
 
-	if (!kbd_led_read(asus, &led_val, NULL) && !dmi_check_system(asus_use_hid_led_dmi_ids)) {
-		pr_info("using asus-wmi for asus::kbd_backlight\n");
+	asus->kbd_led.name = "asus::kbd_backlight";
+	asus->kbd_led.flags = LED_BRIGHT_HW_CHANGED;
+	asus->kbd_led.brightness_set = kbd_led_set;
+	asus->kbd_led.brightness_get = kbd_led_get;
+	asus->kbd_led.max_brightness = 3;
+	asus->kbd_led_avail = !kbd_led_read(asus, &led_val, NULL);
+
+	if (asus->kbd_led_avail)
 		asus->kbd_led_wk = led_val;
-		asus->kbd_led.name = "asus::kbd_backlight";
-		asus->kbd_led.flags = LED_BRIGHT_HW_CHANGED;
-		asus->kbd_led.brightness_set = kbd_led_set;
-		asus->kbd_led.brightness_get = kbd_led_get;
-		asus->kbd_led.max_brightness = 3;
+	else
+		asus->kbd_led_wk = -1;
 
-		if (num_rgb_groups != 0)
-			asus->kbd_led.groups = kbd_rgb_mode_groups;
+	if (asus->kbd_led_avail && num_rgb_groups != 0)
+		asus->kbd_led.groups = kbd_rgb_mode_groups;
 
+	spin_lock_irqsave(&asus_ref.lock, flags);
+	if (asus->kbd_led_avail || !list_empty(&asus_ref.listeners)) {
 		rv = led_classdev_register(&asus->platform_device->dev,
 					   &asus->kbd_led);
-		if (rv)
+		if (rv) {
+			spin_unlock_irqrestore(&asus_ref.lock, flags);
 			goto error;
+		}
+		asus->kbd_led_registered = true;
+
+		if (asus->kbd_led_wk >= 0) {
+			list_for_each_entry(listener, &asus_ref.listeners, list)
+				listener->brightness_set(listener, asus->kbd_led_wk);
+		}
 	}
+	asus_ref.asus = asus;
+	spin_unlock_irqrestore(&asus_ref.lock, flags);
 
 	if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_WIRELESS_LED)
 			&& (asus->driver->quirks->wapf > 0)) {
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index 8a515179113d..f13a701f47a8 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -163,11 +163,19 @@ enum asus_ally_mcu_hack {
 	ASUS_WMI_ALLY_MCU_HACK_DISABLED,
 };
 
+struct asus_hid_listener {
+	struct list_head list;
+	void (*brightness_set)(struct asus_hid_listener *listener, int brightness);
+};
+
 #if IS_REACHABLE(CONFIG_ASUS_WMI)
 void set_ally_mcu_hack(enum asus_ally_mcu_hack status);
 void set_ally_mcu_powersave(bool enabled);
 int asus_wmi_set_devstate(u32 dev_id, u32 ctrl_param, u32 *retval);
 int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1, u32 *retval);
+
+int asus_hid_register_listener(struct asus_hid_listener *cdev);
+void asus_hid_unregister_listener(struct asus_hid_listener *cdev);
 #else
 static inline void set_ally_mcu_hack(enum asus_ally_mcu_hack status)
 {
@@ -184,6 +192,14 @@ static inline int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1,
 {
 	return -ENODEV;
 }
+
+static inline int asus_hid_register_listener(struct asus_hid_listener *bdev)
+{
+	return -ENODEV;
+}
+static inline void asus_hid_unregister_listener(struct asus_hid_listener *bdev)
+{
+}
 #endif
 
 /* To be used by both hid-asus and asus-wmi to determine which controls kbd_brightness */
-- 
2.51.0



^ permalink raw reply related

* [PATCH v6 6/7] platform/x86: asus-wmi: add keyboard brightness event handler
From: Antheas Kapenekakis @ 2025-10-13 20:15 UTC (permalink / raw)
  To: platform-driver-x86, linux-input
  Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Corentin Chary,
	Luke D . Jones, Hans de Goede, Ilpo Järvinen, Denis Benato,
	Antheas Kapenekakis
In-Reply-To: <20251013201535.6737-1-lkml@antheas.dev>

Currenlty, the keyboard brightness control of Asus WMI keyboards is
handled in the kernel, which leads to the shortcut going from
brightness 0, to 1, to 2, and 3.

However, for HID keyboards it is exposed as a key and handled by the
user's desktop environment. For the toggle button, this means that
brightness control becomes on/off. In addition, in the absence of a
DE, the keyboard brightness does not work.

Therefore, expose an event handler for the keyboard brightness control
which can then be used by hid-asus.

Reviewed-by: Luke D. Jones <luke@ljones.dev>
Tested-by: Luke D. Jones <luke@ljones.dev>
Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
 drivers/platform/x86/asus-wmi.c            | 41 +++++++++++++++++++++-
 include/linux/platform_data/x86/asus-wmi.h | 13 +++++++
 2 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index a2a7cd61fd59..58407a3b6d41 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -1579,6 +1579,45 @@ void asus_hid_unregister_listener(struct asus_hid_listener *bdev)
 }
 EXPORT_SYMBOL_GPL(asus_hid_unregister_listener);
 
+static void do_kbd_led_set(struct led_classdev *led_cdev, int value);
+
+int asus_hid_event(enum asus_hid_event event)
+{
+	unsigned long flags;
+	int brightness;
+
+	spin_lock_irqsave(&asus_ref.lock, flags);
+	if (!asus_ref.asus || !asus_ref.asus->kbd_led_registered) {
+		spin_unlock_irqrestore(&asus_ref.lock, flags);
+		return -EBUSY;
+	}
+	brightness = asus_ref.asus->kbd_led_wk;
+
+	switch (event) {
+	case ASUS_EV_BRTUP:
+		brightness += 1;
+		break;
+	case ASUS_EV_BRTDOWN:
+		brightness -= 1;
+		break;
+	case ASUS_EV_BRTTOGGLE:
+		if (brightness >= ASUS_EV_MAX_BRIGHTNESS)
+			brightness = 0;
+		else
+			brightness += 1;
+		break;
+	}
+
+	do_kbd_led_set(&asus_ref.asus->kbd_led, brightness);
+	led_classdev_notify_brightness_hw_changed(&asus_ref.asus->kbd_led,
+						  asus_ref.asus->kbd_led_wk);
+
+	spin_unlock_irqrestore(&asus_ref.lock, flags);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(asus_hid_event);
+
 /*
  * These functions actually update the LED's, and are called from a
  * workqueue. By doing this as separate work rather than when the LED
@@ -1878,7 +1917,7 @@ static int asus_wmi_led_init(struct asus_wmi *asus)
 	asus->kbd_led.flags = LED_BRIGHT_HW_CHANGED;
 	asus->kbd_led.brightness_set = kbd_led_set;
 	asus->kbd_led.brightness_get = kbd_led_get;
-	asus->kbd_led.max_brightness = 3;
+	asus->kbd_led.max_brightness = ASUS_EV_MAX_BRIGHTNESS;
 	asus->kbd_led_avail = !kbd_led_read(asus, &led_val, NULL);
 
 	if (asus->kbd_led_avail)
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index 1f85d76387a8..e78e0fbccede 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -168,6 +168,14 @@ struct asus_hid_listener {
 	void (*brightness_set)(struct asus_hid_listener *listener, int brightness);
 };
 
+enum asus_hid_event {
+	ASUS_EV_BRTUP,
+	ASUS_EV_BRTDOWN,
+	ASUS_EV_BRTTOGGLE,
+};
+
+#define ASUS_EV_MAX_BRIGHTNESS 3
+
 #if IS_REACHABLE(CONFIG_ASUS_WMI)
 void set_ally_mcu_hack(enum asus_ally_mcu_hack status);
 void set_ally_mcu_powersave(bool enabled);
@@ -176,6 +184,7 @@ int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1, u32 *retval);
 
 int asus_hid_register_listener(struct asus_hid_listener *cdev);
 void asus_hid_unregister_listener(struct asus_hid_listener *cdev);
+int asus_hid_event(enum asus_hid_event event);
 #else
 static inline void set_ally_mcu_hack(enum asus_ally_mcu_hack status)
 {
@@ -200,6 +209,10 @@ static inline int asus_hid_register_listener(struct asus_hid_listener *bdev)
 static inline void asus_hid_unregister_listener(struct asus_hid_listener *bdev)
 {
 }
+static inline int asus_hid_event(enum asus_hid_event event)
+{
+	return -ENODEV;
+}
 #endif
 
 #endif	/* __PLATFORM_DATA_X86_ASUS_WMI_H */
-- 
2.51.0



^ permalink raw reply related

* [PATCH v6 7/7] HID: asus: add support for the asus-wmi brightness handler
From: Antheas Kapenekakis @ 2025-10-13 20:15 UTC (permalink / raw)
  To: platform-driver-x86, linux-input
  Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Corentin Chary,
	Luke D . Jones, Hans de Goede, Ilpo Järvinen, Denis Benato,
	Antheas Kapenekakis
In-Reply-To: <20251013201535.6737-1-lkml@antheas.dev>

If the asus-wmi brightness handler is available, send the
keyboard brightness events to it instead of passing them
to userspace. If it is not, fall back to sending them to it.

Reviewed-by: Luke D. Jones <luke@ljones.dev>
Tested-by: Luke D. Jones <luke@ljones.dev>
Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
 drivers/hid/hid-asus.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 0af19c8ef035..1f904bb66396 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -324,6 +324,17 @@ static int asus_event(struct hid_device *hdev, struct hid_field *field,
 			 usage->hid & HID_USAGE);
 	}
 
+	if (usage->type == EV_KEY && value) {
+		switch (usage->code) {
+		case KEY_KBDILLUMUP:
+			return !asus_hid_event(ASUS_EV_BRTUP);
+		case KEY_KBDILLUMDOWN:
+			return !asus_hid_event(ASUS_EV_BRTDOWN);
+		case KEY_KBDILLUMTOGGLE:
+			return !asus_hid_event(ASUS_EV_BRTTOGGLE);
+		}
+	}
+
 	return 0;
 }
 
-- 
2.51.0



^ permalink raw reply related

* [PATCH v6 0/7] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
From: Antheas Kapenekakis @ 2025-10-13 20:15 UTC (permalink / raw)
  To: platform-driver-x86, linux-input
  Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Corentin Chary,
	Luke D . Jones, Hans de Goede, Ilpo Järvinen, Denis Benato,
	Antheas Kapenekakis

This is a two part series which does the following:
  - Clean-up init sequence
  - Unify backlight handling to happen under asus-wmi so that all Aura
    devices have synced brightness controls and the backlight button works
    properly when it is on a USB laptop keyboard instead of one w/ WMI.

For more context, see cover letter of V1. Since V5, I removed some patches
to make this easier to merge.

All comments with these patches had been addressed since V4.

---
V5: https://lore.kernel.org/all/20250325184601.10990-1-lkml@antheas.dev/
V4: https://lore.kernel.org/lkml/20250324210151.6042-1-lkml@antheas.dev/
V3: https://lore.kernel.org/lkml/20250322102804.418000-1-lkml@antheas.dev/
V2: https://lore.kernel.org/all/20250320220924.5023-1-lkml@antheas.dev/
V1: https://lore.kernel.org/all/20250319191320.10092-1-lkml@antheas.dev/

Changes since V5:
  - It's been a long time
  - Remove addition of RGB as that had some comments I need to work on
  - Remove folio patch (already merged)
  - Remove legacy fix patch 11 from V4. There is a small chance that
    without this patch, some old NKEY keyboards might not respond to
    RGB commands according to Luke, but the kernel driver does not do
    RGB currently. The 0x5d init is done by Armoury crate software in
    Windows. If an issue is found, we can re-add it or just remove patches
    1/2 before merging. However, init could use the cleanup.

Changes since V4:
  - Fix KConfig (reported by kernel robot)
  - Fix Ilpo's nits, if I missed anything lmk

Changes since V3:
  - Add initializer for 0x5d for old NKEY keyboards until it is verified
    that it is not needed for their media keys to function.
  - Cover init in asus-wmi with spinlock as per Hans
  - If asus-wmi registers WMI handler with brightness, init the brightness
    in USB Asus keyboards, per Hans.
  - Change hid handler name to asus-UNIQ:rgb:peripheral to match led class
  - Fix oops when unregistering asus-wmi by moving unregister outside of
    the spin lock (but after the asus reference is set to null)

Changes since V2:
  - Check lazy init succeds in asus-wmi before setting register variable
  - make explicit check in asus_hid_register_listener for listener existing
    to avoid re-init
  - rename asus_brt to asus_hid in most places and harmonize everything
  - switch to a spinlock instead of a mutex to avoid kernel ooops
  - fixup hid device quirks to avoid multiple RGB devices while still exposing
    all input vendor devices. This includes moving rgb init to probe
    instead of the input_configured callbacks.
  - Remove fan key (during retest it appears to be 0xae that is already
    supported by hid-asus)
  - Never unregister asus::kbd_backlight while asus-wmi is active, as that
  - removes fds from userspace and breaks backlight functionality. All
  - current mainline drivers do not support backlight hotplugging, so most
    userspace software (e.g., KDE, UPower) is built with that assumption.
    For the Ally, since it disconnects its controller during sleep, this
    caused the backlight slider to not work in KDE.

Changes since V1:
  - Add basic RGB support on hid-asus, (Z13/Ally) tested in KDE/Z13
  - Fix ifdef else having an invalid signature (reported by kernel robot)
  - Restore input arguments to init and keyboard function so they can
    be re-used for RGB controls.
  - Remove Z13 delay (it did not work to fix the touchpad) and replace it
    with a HID_GROUP_GENERIC quirk to allow hid-multitouch to load. Squash
    keyboard rename into it.
  - Unregister brightness listener before removing work queue to avoid
    a race condition causing corruption
  - Remove spurious mutex unlock in asus_brt_event
  - Place mutex lock in kbd_led_set after LED_UNREGISTERING check to avoid
    relocking the mutex and causing a deadlock when unregistering leds
  - Add extra check during unregistering to avoid calling unregister when
    no led device is registered.
  - Temporarily HID_QUIRK_INPUT_PER_APP from the ROG endpoint as it causes
    the driver to create 4 RGB handlers per device. I also suspect some
    extra events sneak through (KDE had the @@@@@@).

Antheas Kapenekakis (7):
  HID: asus: refactor init sequence per spec
  HID: asus: prevent binding to all HID devices on ROG
  platform/x86: asus-wmi: Add support for multiple kbd RGB handlers
  HID: asus: listen to the asus-wmi brightness device instead of
    creating one
  platform/x86: asus-wmi: remove unused keyboard backlight quirk
  platform/x86: asus-wmi: add keyboard brightness event handler
  HID: asus: add support for the asus-wmi brightness handler

 drivers/hid/hid-asus.c                     | 235 +++++++++++----------
 drivers/platform/x86/asus-wmi.c            | 157 ++++++++++++--
 include/linux/platform_data/x86/asus-wmi.h |  69 +++---
 3 files changed, 291 insertions(+), 170 deletions(-)


base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
-- 
2.51.0



^ permalink raw reply

* [PATCH] HID: Kconfig: Fix build error from CONFIG_HID_HAPTIC
From: Jonathan Denose @ 2025-10-13 20:54 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires
  Cc: Thorsten Leemhuis, Randy Dunlap, Lucas GISSOT, linux-input,
	linux-kernel, Jonathan Denose

Temporarily change CONFIG_HID_HAPTIC to be bool instead of tristate, until
we implement a permanent solution.

---
Recently the CONFIG_HID_HAPTIC Kconfig option was reported as causing
the following build errors:

  MODPOST Module.symvers
ERROR: modpost: "hid_haptic_init" [drivers/hid/hid-multitouch.ko] undefined!
ERROR: modpost: "hid_haptic_pressure_increase" [drivers/hid/hid-multitouch.ko] undefined!
ERROR: modpost: "hid_haptic_check_pressure_unit" [drivers/hid/hid-multitouch.ko] undefined!
ERROR: modpost: "hid_haptic_input_configured" [drivers/hid/hid-multitouch.ko] undefined!
ERROR: modpost: "hid_haptic_input_mapping" [drivers/hid/hid-multitouch.ko] undefined!
ERROR: modpost: "hid_haptic_feature_mapping" [drivers/hid/hid-multitouch.ko] undefined!
ERROR: modpost: "hid_haptic_pressure_reset" [drivers/hid/hid-multitouch.ko] undefined!
make[3]: *** [/home/thl/var/linux.dev/scripts/Makefile.modpost:147: Module.symvers] Error 1

when the kernel is compiled with the following configuration:

CONFIG_HID=y
CONFIG_HID_MULTITOUCH=m
CONFIG_HID_HAPTIC=m

To resolve this, temporarily change the CONFIG_HID_HAPTIC option to be
bool, until we arrive at a permanent solution to enable CONFIG_HID_HAPTIC
to be tristate.

For a more detailed discussion, see [1].

[1]: https://lore.kernel.org/linux-input/auypydfkhx2eg7vp764way4batdilzc35inqda3exwzs3tk3ff@oagat6g46zto/

Signed-off-by: Jonathan Denose <jdenose@google.com>
---
 drivers/hid/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 5341aa79f387bd0e5a76266b5928d2c978dd81cf..04420a713be085c8871b4d35255fde4cafd8de0f 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -93,7 +93,7 @@ config HID_GENERIC
 	If unsure, say Y.

 config HID_HAPTIC
-	tristate "Haptic touchpad support"
+	bool "Haptic touchpad support"
 	default n
 	help
 	Support for touchpads with force sensors and haptic actuators instead of a

---
base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
change-id: 20251013-hid-haptic-kconfig-fix-634df2bdac1d

Best regards,
-- 
Jonathan Denose <jdenose@google.com>

^ permalink raw reply related

* Re: [PATCH v3 04/11] HID: haptic: introduce hid_haptic_device
From: Jonathan Denose @ 2025-10-13 20:59 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Randy Dunlap, Thorsten Leemhuis, Jiri Kosina, Dmitry Torokhov,
	Jonathan Corbet, Henrik Rydberg, linux-input, linux-kernel,
	linux-doc, Angela Czubak, Sean O'Brien, Lucas GISSOT
In-Reply-To: <auypydfkhx2eg7vp764way4batdilzc35inqda3exwzs3tk3ff@oagat6g46zto>

On Mon, Oct 13, 2025 at 9:29 AM Benjamin Tissoires <bentiss@kernel.org> wrote:
>
> On Oct 13 2025, Jonathan Denose wrote:
> > On Fri, Oct 10, 2025 at 5:52 PM Randy Dunlap <rdunlap@infradead.org> wrote:
> > >
> > >
> > >
> > > On 10/10/25 1:30 PM, Jonathan Denose wrote:
> > > > Hi all,
> > > >
> > > > Thanks for looking into this.
> > > >
> > > > On Fri, Oct 10, 2025 at 1:55 PM Randy Dunlap <rdunlap@infradead.org> wrote:
> > > >>
> > > >> Hi,
> > > >>
> > > >> I think I found it... see below.
> > > >>
> > > >>
> > > >> On 10/9/25 11:48 PM, Thorsten Leemhuis wrote:
> > > >>> [Top-Posting for easier consumption]
> > > >>>
> > > >>> Mainly writing this mail to bring Lucas GISSOT in here, who reported the
> > > >>> same error yesterday here:
> > > >>> https://lore.kernel.org/all/aOgvA8Jiofcnk2xb@ARSENIURE.localdomain/
> > > >>>
> > > >>> Lucas there suggested:
> > > >>> """but it seems to me that the #if IS_ENABLED(CONFIG_HID_HAPTIC) in
> > > >>> hid-haptic.h should be replaced by IS_BUILTIN(CONFIG_HID_HAPTIC) and
> > > >>> Kconfig updated."""
> > > >>>
> > > >>> And Randy: Thx for the closer investigation! It explains some of the
> > > >>> "that feels odd, am I holding this wrong" feeling I had when reporting this.
> > > >>>
> > > >>> Ciao, Thorsten
> > > >>>
> > > >>> On 10/10/25 06:50, Randy Dunlap wrote:
> > > >>>> On 10/9/25 7:43 AM, Thorsten Leemhuis wrote:
> > > >>>>> On 8/19/25 01:08, Jonathan Denose wrote:
> > > >>>>>> From: Angela Czubak <aczubak@google.com>
> > > >>>>>>
> > > >>>>>> Define a new structure that contains simple haptic device configuration
> > > >>>>>> as well as current state.
> > > >>>>>> Add functions that recognize auto trigger and manual trigger reports
> > > >>>>>> as well as save their addresses.Hi,
> > > >>>>>> Verify that the pressure unit is either grams or newtons.
> > > >>>>>> Mark the input device as a haptic touchpad if the unit is correct and
> > > >>>>>> the reports are found.
> > > >>>>>>  [...]
> > > >>>>>> +config HID_HAPTIC
> > > >>>>>> +  tristate "Haptic touchpad support"
> > > >>>>>> +  default n
> > > >>>>>> +  help
> > > >>>>>> +  Support for touchpads with force sensors and haptic actuators instead of a
> > > >>>>>> +  traditional button.
> > > >>>>>> +  Adds extra parsing and FF device for the hid multitouch driver.
> > > >>>>>> +  It can be used for Elan 2703 haptic touchpad.
> > > >>>>>> +
> > > >>>>>> +  If unsure, say N.
> > > >>>>>> +
> > > >>>>>>  menu "Special HID drivers"
> > > >>>>>
> > > >>>>> I suspect this change is related to a build error I ran into today:
> > > >>>>>
> > > >>>>>   MODPOST Module.symvers
> > > >>>>> ERROR: modpost: "hid_haptic_init" [drivers/hid/hid-multitouch.ko] undefined!
> > > >>>>> ERROR: modpost: "hid_haptic_pressure_increase" [drivers/hid/hid-multitouch.ko] undefined!
> > > >>>>> ERROR: modpost: "hid_haptic_check_pressure_unit" [drivers/hid/hid-multitouch.ko] undefined!
> > > >>>>> ERROR: modpost: "hid_haptic_input_configured" [drivers/hid/hid-multitouch.ko] undefined!
> > > >>>>> ERROR: modpost: "hid_haptic_input_mapping" [drivers/hid/hid-multitouch.ko] undefined!
> > > >>>>> ERROR: modpost: "hid_haptic_feature_mapping" [drivers/hid/hid-multitouch.ko] undefined!
> > > >>>>> ERROR: modpost: "hid_haptic_pressure_reset" [drivers/hid/hid-multitouch.ko] undefined!
> > > >>>>> make[3]: *** [/home/thl/var/linux.dev/scripts/Makefile.modpost:147: Module.symvers] Error 1
> > > >>>>>
> > > >>>>> The config where this occurred had this:
> > > >>>>>
> > > >>>>> CONFIG_HID=y
> > > >>>>> CONFIG_HID_MULTITOUCH=m
> > > >>>>> CONFIG_HID_HAPTIC=m
> > > >>>>>
> > > >>>>> Changing the latter to "CONFIG_HID_HAPTIC=y" fixed the problem for me.
> > > >>>>
> > > >>>> Sure, but that's just covering up the problem.
> > > >>>>> First, I get this build error:
> > > >>>>
> > > >>>> ERROR: modpost: missing MODULE_LICENSE() in drivers/hid/hid-haptic.o
> > > >>>> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hid/hid-haptic.o
> > > >>>>
> > > >>
> > > >> ISTM that tristate is incompatible with this newly added Makefile
> > > >> line:
> > > >>
> > > >> +hid-$(CONFIG_HID_HAPTIC)       += hid-haptic.o
> > > >>
> > > >> hid-* lists files that should be builtin, not loadable modules.
> > > >> These should all be hid-y.  AFAIK, hid-m is not useful.
> > > >> (A maintainer can correct me as needed.)
> > >
> > > More correctly, any .o that is listed as being built as part of
> > > hid.o should is going to be controlled by CONFIG_HID and should
> > > not its own have a Kconfig symbol at all.
> > >
> > > E.g. here:
> > >  hid-y                  := hid-core.o hid-input.o hid-quirks.o
> > > hid-core, hid-input, and hid-quirks don't have their own
> > > Kconfig symbols.
> > >
> > >
> > >
> > > >> So adding a MODULE_LICENSE() and MODULE_DESCRIPTION() to
> > > >> hid-haptic.c and changing drivers/hid/Makefile to use
> > > >> obj-$(CONFIG_HID_HAPTIC_        += hid-haptic.o
> > > >>
> > > >> fixes the build errors for me.
> > > >>
> > > >> Angela, Jonathan D., is there any reason that
> > > >> hid-haptic needs to be builtin instead of a loadable
> > > >> module?  It's no problem for hid-multitouch.ko to call
> > > >> into hid-haptic.ko (both as loadable modules) as long as
> > > >> hid-haptic.ko is loaded first.
> > > >>
> > > > As long as hid-multitouch.ko is able to call into hid-haptic.ko I
> > > > don't see any issues, but is there a way to enforce that when
> > > > CONFIG_HID_HAPTIC is enabled, hid-haptic.ko will be loaded before
> > > > hid-multitouch.ko?
> > >
> > > I only know of two possibilities though there may be more.
> > >
> > > a. use request_module(true, "hid-haptic");
> > >
> > > This would probably be used in the hid core somewhere, afterthe device matching is done.
> > >
> > > b. use udev. When a device that needs hid-multitouch is
> > > discovered, have udev load both hid-haptic and hid-multitouch.
> > >
> > >
> > > I see that hid-haptic.h is written so that it has stubs for
> > > when CONFIG_HID_HAPTIC is not enabled. Can hid-multitouch
> > > operate (in a reduced capacity) when HID_HAPTIC is not enabled?
> > > So that they are separate modules and hid-multitouch is not
> > > dependent on hid-haptic?
> > >
> > > There is probably a use case for hid-multitouch without having
> > > hid-haptic loaded since hid-multitouch existed without having
> > > hid-haptic around at all.
> > >
> > > It seems that you want both of them loaded. And then Lucas
> > > has reported a build error when HID_HAPTIC=m and
> > > HID_MULTITOUCH=y, so either (like Lucas said) HID_HAPTIC
> > > should be bool, not tristate; or in Kconfig,
> > > HID_MULTITOUCH should depend on HID_HAPTIC, which would not
> > > allow the problem config that Lucas reported.
> > > But that forces devices that want HID_MULTITOUCH to also
> > > have HID_HAPTIC loaded, even though they may not need it.
>
> One way around it is to declare a stub struct haptic_operations and let
> hid-haptic.ko fill in the function pointers when it loads and remove
> them when it unloads. But it can be a little bit tedious, especially
> because making it properly working involves RCUs (we don't want to have
> mutexes everywhere).
>
> > >
> > The idea behind these patches was that hid-haptic would depend on
> > hid-multitouch but not the other way around. I am fine changing
> > HID_HAPTIC to bool. That's what I had it as initially, but I was asked
> > to change it.
> >
> > If everyone else is fine with that, I can send out a patch.
>
> I'd still prefer hid-haptic to be tristate, just because
> input_ff_memless is tristate as well. Right now distributions don't
> support the FF bits, so enforcing this into the kernel seems a little
> bit harsh on them and difficult to debug for early adopters.
>
> That being said, that build failure is pretty bad. So please send a
> (temporary) fix ASAP. If making it boolean solves the issue, then yes,
> send a boolean fix and then we can revisit it. But right now the urgency
> is to fix that.
>
> And if making it proper tristate is too hard, then we can leave with
> bool.
>
> Cheers,
> Benjamin

Changing to bool instead of tristate resolved the build error.

Submitted the patch:
https://lore.kernel.org/linux-input/20251013-hid-haptic-kconfig-fix-v1-1-b1ad90732625@google.com/
-- 
Jonathan

^ permalink raw reply

* Re: [PATCH] HID: Kconfig: Fix build error from CONFIG_HID_HAPTIC
From: Kuan-Wei Chiu @ 2025-10-13 21:10 UTC (permalink / raw)
  To: Jonathan Denose
  Cc: Jiri Kosina, Benjamin Tissoires, Thorsten Leemhuis, Randy Dunlap,
	Lucas GISSOT, linux-input, linux-kernel
In-Reply-To: <20251013-hid-haptic-kconfig-fix-v1-1-b1ad90732625@google.com>

On Mon, Oct 13, 2025 at 08:54:57PM +0000, Jonathan Denose wrote:
> Temporarily change CONFIG_HID_HAPTIC to be bool instead of tristate, until
> we implement a permanent solution.
> 
> ---

The "---" line here will cause many tools used for applying patches,
like git am, to discard the content below it [1].

Please don't add this line unless you don't want the following content
to appear in the commit message.

[1]: https://www.kernel.org/doc/html/v6.17/process/submitting-patches.html#commentary

Regards,
Kuan-Wei

> Recently the CONFIG_HID_HAPTIC Kconfig option was reported as causing
> the following build errors:
> 
>   MODPOST Module.symvers
> ERROR: modpost: "hid_haptic_init" [drivers/hid/hid-multitouch.ko] undefined!
> ERROR: modpost: "hid_haptic_pressure_increase" [drivers/hid/hid-multitouch.ko] undefined!
> ERROR: modpost: "hid_haptic_check_pressure_unit" [drivers/hid/hid-multitouch.ko] undefined!
> ERROR: modpost: "hid_haptic_input_configured" [drivers/hid/hid-multitouch.ko] undefined!
> ERROR: modpost: "hid_haptic_input_mapping" [drivers/hid/hid-multitouch.ko] undefined!
> ERROR: modpost: "hid_haptic_feature_mapping" [drivers/hid/hid-multitouch.ko] undefined!
> ERROR: modpost: "hid_haptic_pressure_reset" [drivers/hid/hid-multitouch.ko] undefined!
> make[3]: *** [/home/thl/var/linux.dev/scripts/Makefile.modpost:147: Module.symvers] Error 1
> 
> when the kernel is compiled with the following configuration:
> 
> CONFIG_HID=y
> CONFIG_HID_MULTITOUCH=m
> CONFIG_HID_HAPTIC=m
> 
> To resolve this, temporarily change the CONFIG_HID_HAPTIC option to be
> bool, until we arrive at a permanent solution to enable CONFIG_HID_HAPTIC
> to be tristate.
> 
> For a more detailed discussion, see [1].
> 
> [1]: https://lore.kernel.org/linux-input/auypydfkhx2eg7vp764way4batdilzc35inqda3exwzs3tk3ff@oagat6g46zto/
> 
> Signed-off-by: Jonathan Denose <jdenose@google.com>
> ---
>  drivers/hid/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 5341aa79f387bd0e5a76266b5928d2c978dd81cf..04420a713be085c8871b4d35255fde4cafd8de0f 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -93,7 +93,7 @@ config HID_GENERIC
>  	If unsure, say Y.
>  
>  config HID_HAPTIC
> -	tristate "Haptic touchpad support"
> +	bool "Haptic touchpad support"
>  	default n
>  	help
>  	Support for touchpads with force sensors and haptic actuators instead of a
> 
> ---
> base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
> change-id: 20251013-hid-haptic-kconfig-fix-634df2bdac1d
> 
> Best regards,
> -- 
> Jonathan Denose <jdenose@google.com>
> 

^ permalink raw reply

* Re: [PATCH] HID: Kconfig: Fix build error from CONFIG_HID_HAPTIC
From: Jonathan Denose @ 2025-10-13 21:23 UTC (permalink / raw)
  To: Kuan-Wei Chiu
  Cc: Jiri Kosina, Benjamin Tissoires, Thorsten Leemhuis, Randy Dunlap,
	Lucas GISSOT, linux-input, linux-kernel
In-Reply-To: <aO1q4coXPqU/K6KI@visitorckw-System-Product-Name>

On Mon, Oct 13, 2025 at 4:11 PM Kuan-Wei Chiu <visitorckw@gmail.com> wrote:
>
> On Mon, Oct 13, 2025 at 08:54:57PM +0000, Jonathan Denose wrote:
> > Temporarily change CONFIG_HID_HAPTIC to be bool instead of tristate, until
> > we implement a permanent solution.
> >
> > ---
>
> The "---" line here will cause many tools used for applying patches,
> like git am, to discard the content below it [1].
>
> Please don't add this line unless you don't want the following content
> to appear in the commit message.
>
> [1]: https://www.kernel.org/doc/html/v6.17/process/submitting-patches.html#commentary
>
> Regards,
> Kuan-Wei

Yes, that was intentional, the information below the '---' was
included as additional information and not for the commit message. The
error messages were too long, as was the link to the longer
discussion, and this caused errors in checkpatch.pl.

> > Recently the CONFIG_HID_HAPTIC Kconfig option was reported as causing
> > the following build errors:
> >
> >   MODPOST Module.symvers
> > ERROR: modpost: "hid_haptic_init" [drivers/hid/hid-multitouch.ko] undefined!
> > ERROR: modpost: "hid_haptic_pressure_increase" [drivers/hid/hid-multitouch.ko] undefined!
> > ERROR: modpost: "hid_haptic_check_pressure_unit" [drivers/hid/hid-multitouch.ko] undefined!
> > ERROR: modpost: "hid_haptic_input_configured" [drivers/hid/hid-multitouch.ko] undefined!
> > ERROR: modpost: "hid_haptic_input_mapping" [drivers/hid/hid-multitouch.ko] undefined!
> > ERROR: modpost: "hid_haptic_feature_mapping" [drivers/hid/hid-multitouch.ko] undefined!
> > ERROR: modpost: "hid_haptic_pressure_reset" [drivers/hid/hid-multitouch.ko] undefined!
> > make[3]: *** [/home/thl/var/linux.dev/scripts/Makefile.modpost:147: Module.symvers] Error 1
> >
> > when the kernel is compiled with the following configuration:
> >
> > CONFIG_HID=y
> > CONFIG_HID_MULTITOUCH=m
> > CONFIG_HID_HAPTIC=m
> >
> > To resolve this, temporarily change the CONFIG_HID_HAPTIC option to be
> > bool, until we arrive at a permanent solution to enable CONFIG_HID_HAPTIC
> > to be tristate.
> >
> > For a more detailed discussion, see [1].
> >
> > [1]: https://lore.kernel.org/linux-input/auypydfkhx2eg7vp764way4batdilzc35inqda3exwzs3tk3ff@oagat6g46zto/
> >
> > Signed-off-by: Jonathan Denose <jdenose@google.com>
> > ---
> >  drivers/hid/Kconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> > index 5341aa79f387bd0e5a76266b5928d2c978dd81cf..04420a713be085c8871b4d35255fde4cafd8de0f 100644
> > --- a/drivers/hid/Kconfig
> > +++ b/drivers/hid/Kconfig
> > @@ -93,7 +93,7 @@ config HID_GENERIC
> >       If unsure, say Y.
> >
> >  config HID_HAPTIC
> > -     tristate "Haptic touchpad support"
> > +     bool "Haptic touchpad support"
> >       default n
> >       help
> >       Support for touchpads with force sensors and haptic actuators instead of a
> >
> > ---
> > base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
> > change-id: 20251013-hid-haptic-kconfig-fix-634df2bdac1d
> >
> > Best regards,
> > --
> > Jonathan Denose <jdenose@google.com>
> >
-- 
Jonathan

^ permalink raw reply

* Re: [PATCH] HID: Kconfig: Fix build error from CONFIG_HID_HAPTIC
From: Kuan-Wei Chiu @ 2025-10-13 21:29 UTC (permalink / raw)
  To: Jonathan Denose
  Cc: Jiri Kosina, Benjamin Tissoires, Thorsten Leemhuis, Randy Dunlap,
	Lucas GISSOT, linux-input, linux-kernel
In-Reply-To: <CAMCVhVNLr+2ivRo9T4rVt4mkncwbOfXEL9bE=pDGRp=Qjy1c9A@mail.gmail.com>

On Mon, Oct 13, 2025 at 04:23:36PM -0500, Jonathan Denose wrote:
> On Mon, Oct 13, 2025 at 4:11 PM Kuan-Wei Chiu <visitorckw@gmail.com> wrote:
> >
> > On Mon, Oct 13, 2025 at 08:54:57PM +0000, Jonathan Denose wrote:
> > > Temporarily change CONFIG_HID_HAPTIC to be bool instead of tristate, until
> > > we implement a permanent solution.
> > >
> > > ---
> >
> > The "---" line here will cause many tools used for applying patches,
> > like git am, to discard the content below it [1].
> >
> > Please don't add this line unless you don't want the following content
> > to appear in the commit message.
> >
> > [1]: https://www.kernel.org/doc/html/v6.17/process/submitting-patches.html#commentary
> >
> > Regards,
> > Kuan-Wei
> 
> Yes, that was intentional, the information below the '---' was
> included as additional information and not for the commit message. The
> error messages were too long, as was the link to the longer
> discussion, and this caused errors in checkpatch.pl.
> 

Then at least your Signed-off-by tag needs to be above the --- line?

Regards,
Kuan-Wei


^ permalink raw reply

* Re: [PATCH v6 0/7] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
From: Denis Benato @ 2025-10-13 21:36 UTC (permalink / raw)
  To: Antheas Kapenekakis, platform-driver-x86, linux-input
  Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Corentin Chary,
	Luke D . Jones, Hans de Goede, Ilpo Järvinen
In-Reply-To: <20251013201535.6737-1-lkml@antheas.dev>


On 10/13/25 22:15, Antheas Kapenekakis wrote:
> This is a two part series which does the following:
>   - Clean-up init sequence
>   - Unify backlight handling to happen under asus-wmi so that all Aura
>     devices have synced brightness controls and the backlight button works
>     properly when it is on a USB laptop keyboard instead of one w/ WMI.
>
> For more context, see cover letter of V1. Since V5, I removed some patches
> to make this easier to merge.
>
> All comments with these patches had been addressed since V4.
>
> ---
> V5: https://lore.kernel.org/all/20250325184601.10990-1-lkml@antheas.dev/
> V4: https://lore.kernel.org/lkml/20250324210151.6042-1-lkml@antheas.dev/
> V3: https://lore.kernel.org/lkml/20250322102804.418000-1-lkml@antheas.dev/
> V2: https://lore.kernel.org/all/20250320220924.5023-1-lkml@antheas.dev/
> V1: https://lore.kernel.org/all/20250319191320.10092-1-lkml@antheas.dev/
>
> Changes since V5:
>   - It's been a long time
>   - Remove addition of RGB as that had some comments I need to work on
Hello and thanks for this,

I would like to see a set of objectives that this series is trying to reach clearly spelled out:
https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html
section 2, basically.

I can deduct you are trying to fix certain keyboards having keys not properly changing
the backlight, but is that all there is to it?

These commits sure touches a lot of code to fix an (unspecified? Unclear to me at least) number of bugs.

Given the fact I expect regressions (nothing personal: ASUS has a very large pool of models,
many requiring specialized quirks as can be seen in the diff)
wouldn't it be wise to take the chance and expose the "static" lighting mode,
including colors handling, for every asus rog, tuf, tx and proart laptop
in existence in a way that most already-existing programs (including the KDE widget) can handle?
At least there would be a big incentive behind all these changes.

[^ the above paragraph is valid if the number of fixed bugs is small and there is at least another way to fix it/them].

Surely data needed to check what model support
what lighting mode is known at this point, would probably be wise
to put such data in the kernel and make good use of it.

Thanks,
Denis

>   - Remove folio patch (already merged)
>   - Remove legacy fix patch 11 from V4. There is a small chance that
>     without this patch, some old NKEY keyboards might not respond to
>     RGB commands according to Luke, but the kernel driver does not do
>     RGB currently. The 0x5d init is done by Armoury crate software in
>     Windows. If an issue is found, we can re-add it or just remove patches
>     1/2 before merging. However, init could use the cleanup.
>
> Changes since V4:
>   - Fix KConfig (reported by kernel robot)
>   - Fix Ilpo's nits, if I missed anything lmk
>
> Changes since V3:
>   - Add initializer for 0x5d for old NKEY keyboards until it is verified
>     that it is not needed for their media keys to function.
>   - Cover init in asus-wmi with spinlock as per Hans
>   - If asus-wmi registers WMI handler with brightness, init the brightness
>     in USB Asus keyboards, per Hans.
>   - Change hid handler name to asus-UNIQ:rgb:peripheral to match led class
>   - Fix oops when unregistering asus-wmi by moving unregister outside of
>     the spin lock (but after the asus reference is set to null)
>
> Changes since V2:
>   - Check lazy init succeds in asus-wmi before setting register variable
>   - make explicit check in asus_hid_register_listener for listener existing
>     to avoid re-init
>   - rename asus_brt to asus_hid in most places and harmonize everything
>   - switch to a spinlock instead of a mutex to avoid kernel ooops
>   - fixup hid device quirks to avoid multiple RGB devices while still exposing
>     all input vendor devices. This includes moving rgb init to probe
>     instead of the input_configured callbacks.
>   - Remove fan key (during retest it appears to be 0xae that is already
>     supported by hid-asus)
>   - Never unregister asus::kbd_backlight while asus-wmi is active, as that
>   - removes fds from userspace and breaks backlight functionality. All
>   - current mainline drivers do not support backlight hotplugging, so most
>     userspace software (e.g., KDE, UPower) is built with that assumption.
>     For the Ally, since it disconnects its controller during sleep, this
>     caused the backlight slider to not work in KDE.
>
> Changes since V1:
>   - Add basic RGB support on hid-asus, (Z13/Ally) tested in KDE/Z13
>   - Fix ifdef else having an invalid signature (reported by kernel robot)
>   - Restore input arguments to init and keyboard function so they can
>     be re-used for RGB controls.
>   - Remove Z13 delay (it did not work to fix the touchpad) and replace it
>     with a HID_GROUP_GENERIC quirk to allow hid-multitouch to load. Squash
>     keyboard rename into it.
>   - Unregister brightness listener before removing work queue to avoid
>     a race condition causing corruption
>   - Remove spurious mutex unlock in asus_brt_event
>   - Place mutex lock in kbd_led_set after LED_UNREGISTERING check to avoid
>     relocking the mutex and causing a deadlock when unregistering leds
>   - Add extra check during unregistering to avoid calling unregister when
>     no led device is registered.
>   - Temporarily HID_QUIRK_INPUT_PER_APP from the ROG endpoint as it causes
>     the driver to create 4 RGB handlers per device. I also suspect some
>     extra events sneak through (KDE had the @@@@@@).
>
> Antheas Kapenekakis (7):
>   HID: asus: refactor init sequence per spec
>   HID: asus: prevent binding to all HID devices on ROG
>   platform/x86: asus-wmi: Add support for multiple kbd RGB handlers
>   HID: asus: listen to the asus-wmi brightness device instead of
>     creating one
>   platform/x86: asus-wmi: remove unused keyboard backlight quirk
>   platform/x86: asus-wmi: add keyboard brightness event handler
>   HID: asus: add support for the asus-wmi brightness handler
>
>  drivers/hid/hid-asus.c                     | 235 +++++++++++----------
>  drivers/platform/x86/asus-wmi.c            | 157 ++++++++++++--
>  include/linux/platform_data/x86/asus-wmi.h |  69 +++---
>  3 files changed, 291 insertions(+), 170 deletions(-)
>
>
> base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787

^ permalink raw reply

* Re: [PATCH v6 7/7] HID: asus: add support for the asus-wmi brightness handler
From: Denis Benato @ 2025-10-13 21:37 UTC (permalink / raw)
  To: Antheas Kapenekakis, platform-driver-x86, linux-input
  Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Corentin Chary,
	Luke D . Jones, Hans de Goede, Ilpo Järvinen
In-Reply-To: <20251013201535.6737-8-lkml@antheas.dev>


On 10/13/25 22:15, Antheas Kapenekakis wrote:
> If the asus-wmi brightness handler is available, send the
> keyboard brightness events to it instead of passing them
> to userspace. If it is not, fall back to sending them to it.
>
> Reviewed-by: Luke D. Jones <luke@ljones.dev>
> Tested-by: Luke D. Jones <luke@ljones.dev>
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> ---
>  drivers/hid/hid-asus.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 0af19c8ef035..1f904bb66396 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -324,6 +324,17 @@ static int asus_event(struct hid_device *hdev, struct hid_field *field,
>  			 usage->hid & HID_USAGE);
>  	}
>  
> +	if (usage->type == EV_KEY && value) {
> +		switch (usage->code) {
> +		case KEY_KBDILLUMUP:
> +			return !asus_hid_event(ASUS_EV_BRTUP);
> +		case KEY_KBDILLUMDOWN:
> +			return !asus_hid_event(ASUS_EV_BRTDOWN);
> +		case KEY_KBDILLUMTOGGLE:
> +			return !asus_hid_event(ASUS_EV_BRTTOGGLE);
> +		}
> +	}
> +
>  	return 0;
>  }
>  
Neat. I like this.


^ permalink raw reply

* Re: [PATCH v6 4/7] HID: asus: listen to the asus-wmi brightness device instead of creating one
From: Denis Benato @ 2025-10-13 21:44 UTC (permalink / raw)
  To: Antheas Kapenekakis, platform-driver-x86, linux-input
  Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Corentin Chary,
	Luke D . Jones, Hans de Goede, Ilpo Järvinen
In-Reply-To: <20251013201535.6737-5-lkml@antheas.dev>


On 10/13/25 22:15, Antheas Kapenekakis wrote:
> Some ROG laptops expose multiple interfaces for controlling the
> keyboard/RGB brightness. This creates a name conflict under
> asus::kbd_brightness, where the second device ends up being
> named asus::kbd_brightness_1 and they are both broken.
Can you please reference a bug report and/or an analysis of why they ends
up being broken?
>
> Therefore, register a listener to the asus-wmi brightness device
> instead of creating a new one.
>
> Reviewed-by: Luke D. Jones <luke@ljones.dev>
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> ---
>  drivers/hid/hid-asus.c | 64 +++++++-----------------------------------
>  1 file changed, 10 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index a62559e3e064..0af19c8ef035 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -102,7 +102,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
>  #define TRKID_SGN       ((TRKID_MAX + 1) >> 1)
>  
>  struct asus_kbd_leds {
> -	struct led_classdev cdev;
> +	struct asus_hid_listener listener;
It is my understanding from "register a listener .... instead of creating a new one"
that you are attempting to use the same listener among many devices... so why isn't
this a pointer? And more importantly: why do we have bool available, bool registered
instead of either one or the other being replaced by this field being possibly NULL?
>  	struct hid_device *hdev;
>  	struct work_struct work;
>  	unsigned int brightness;
> @@ -495,11 +495,11 @@ static void asus_schedule_work(struct asus_kbd_leds *led)
>  	spin_unlock_irqrestore(&led->lock, flags);
>  }
>  
> -static void asus_kbd_backlight_set(struct led_classdev *led_cdev,
> -				   enum led_brightness brightness)
> +static void asus_kbd_backlight_set(struct asus_hid_listener *listener,
> +				   int brightness)
>  {
> -	struct asus_kbd_leds *led = container_of(led_cdev, struct asus_kbd_leds,
> -						 cdev);
> +	struct asus_kbd_leds *led = container_of(listener, struct asus_kbd_leds,
> +						 listener);
>  	unsigned long flags;
>  
>  	spin_lock_irqsave(&led->lock, flags);
> @@ -509,20 +509,6 @@ static void asus_kbd_backlight_set(struct led_classdev *led_cdev,
>  	asus_schedule_work(led);
>  }
>  
> -static enum led_brightness asus_kbd_backlight_get(struct led_classdev *led_cdev)
> -{
> -	struct asus_kbd_leds *led = container_of(led_cdev, struct asus_kbd_leds,
> -						 cdev);
> -	enum led_brightness brightness;
> -	unsigned long flags;
> -
> -	spin_lock_irqsave(&led->lock, flags);
> -	brightness = led->brightness;
> -	spin_unlock_irqrestore(&led->lock, flags);
> -
> -	return brightness;
> -}
> -
>  static void asus_kbd_backlight_work(struct work_struct *work)
>  {
>  	struct asus_kbd_leds *led = container_of(work, struct asus_kbd_leds, work);
> @@ -539,34 +525,6 @@ static void asus_kbd_backlight_work(struct work_struct *work)
>  		hid_err(led->hdev, "Asus failed to set keyboard backlight: %d\n", ret);
>  }
>  
> -/* WMI-based keyboard backlight LED control (via asus-wmi driver) takes
> - * precedence. We only activate HID-based backlight control when the
> - * WMI control is not available.
> - */
> -static bool asus_kbd_wmi_led_control_present(struct hid_device *hdev)
> -{
> -	struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
> -	u32 value;
> -	int ret;
> -
> -	if (!IS_ENABLED(CONFIG_ASUS_WMI))
> -		return false;
> -
> -	if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD &&
> -			dmi_check_system(asus_use_hid_led_dmi_ids)) {
> -		hid_info(hdev, "using HID for asus::kbd_backlight\n");
> -		return false;
> -	}
> -
> -	ret = asus_wmi_evaluate_method(ASUS_WMI_METHODID_DSTS,
> -				       ASUS_WMI_DEVID_KBD_BACKLIGHT, 0, &value);
> -	hid_dbg(hdev, "WMI backlight check: rc %d value %x", ret, value);
> -	if (ret)
> -		return false;
> -
> -	return !!(value & ASUS_WMI_DSTS_PRESENCE_BIT);
> -}
> -
>  /*
>   * We don't care about any other part of the string except the version section.
>   * Example strings: FGA80100.RC72LA.312_T01, FGA80100.RC71LS.318_T01
> @@ -701,14 +659,11 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
>  	drvdata->kbd_backlight->removed = false;
>  	drvdata->kbd_backlight->brightness = 0;
>  	drvdata->kbd_backlight->hdev = hdev;
> -	drvdata->kbd_backlight->cdev.name = "asus::kbd_backlight";
> -	drvdata->kbd_backlight->cdev.max_brightness = 3;
> -	drvdata->kbd_backlight->cdev.brightness_set = asus_kbd_backlight_set;
> -	drvdata->kbd_backlight->cdev.brightness_get = asus_kbd_backlight_get;
> +	drvdata->kbd_backlight->listener.brightness_set = asus_kbd_backlight_set;
>  	INIT_WORK(&drvdata->kbd_backlight->work, asus_kbd_backlight_work);
>  	spin_lock_init(&drvdata->kbd_backlight->lock);
>  
> -	ret = devm_led_classdev_register(&hdev->dev, &drvdata->kbd_backlight->cdev);
> +	ret = asus_hid_register_listener(&drvdata->kbd_backlight->listener);
>  	if (ret < 0) {
>  		/* No need to have this still around */
>  		devm_kfree(&hdev->dev, drvdata->kbd_backlight);
> @@ -1105,7 +1060,7 @@ static int __maybe_unused asus_resume(struct hid_device *hdev) {
>  
>  	if (drvdata->kbd_backlight) {
>  		const u8 buf[] = { FEATURE_KBD_REPORT_ID, 0xba, 0xc5, 0xc4,
> -				drvdata->kbd_backlight->cdev.brightness };
> +				drvdata->kbd_backlight->brightness };
>  		ret = asus_kbd_set_report(hdev, buf, sizeof(buf));
>  		if (ret < 0) {
>  			hid_err(hdev, "Asus failed to set keyboard backlight: %d\n", ret);
> @@ -1241,7 +1196,6 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  	}
>  
>  	if (is_vendor && (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT) &&
> -	    !asus_kbd_wmi_led_control_present(hdev) &&
>  	    asus_kbd_register_leds(hdev))
>  		hid_warn(hdev, "Failed to initialize backlight.\n");
>  
> @@ -1282,6 +1236,8 @@ static void asus_remove(struct hid_device *hdev)
>  	unsigned long flags;
>  
>  	if (drvdata->kbd_backlight) {
> +		asus_hid_unregister_listener(&drvdata->kbd_backlight->listener);
> +
>  		spin_lock_irqsave(&drvdata->kbd_backlight->lock, flags);
>  		drvdata->kbd_backlight->removed = true;
>  		spin_unlock_irqrestore(&drvdata->kbd_backlight->lock, flags);

^ permalink raw reply

* Re: [PATCH v6 0/7] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
From: Antheas Kapenekakis @ 2025-10-13 21:45 UTC (permalink / raw)
  To: Denis Benato
  Cc: platform-driver-x86, linux-input, linux-kernel, Jiri Kosina,
	Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
	Ilpo Järvinen
In-Reply-To: <f936d9c0-ab58-4205-afd2-2d6b857cad57@gmail.com>

On Mon, 13 Oct 2025 at 23:36, Denis Benato <benato.denis96@gmail.com> wrote:
>
>
> On 10/13/25 22:15, Antheas Kapenekakis wrote:
> > This is a two part series which does the following:
> >   - Clean-up init sequence
> >   - Unify backlight handling to happen under asus-wmi so that all Aura
> >     devices have synced brightness controls and the backlight button works
> >     properly when it is on a USB laptop keyboard instead of one w/ WMI.
> >
> > For more context, see cover letter of V1. Since V5, I removed some patches
> > to make this easier to merge.
> >
> > All comments with these patches had been addressed since V4.
> >
> > ---
> > V5: https://lore.kernel.org/all/20250325184601.10990-1-lkml@antheas.dev/
> > V4: https://lore.kernel.org/lkml/20250324210151.6042-1-lkml@antheas.dev/
> > V3: https://lore.kernel.org/lkml/20250322102804.418000-1-lkml@antheas.dev/
> > V2: https://lore.kernel.org/all/20250320220924.5023-1-lkml@antheas.dev/
> > V1: https://lore.kernel.org/all/20250319191320.10092-1-lkml@antheas.dev/
> >
> > Changes since V5:
> >   - It's been a long time
> >   - Remove addition of RGB as that had some comments I need to work on
> Hello and thanks for this,
>
> I would like to see a set of objectives that this series is trying to reach clearly spelled out:
> https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html
> section 2, basically.
>
> I can deduct you are trying to fix certain keyboards having keys not properly changing
> the backlight, but is that all there is to it?

First two patches cleanup the init sequence.

Next three patches centralize the keyboard brightness handler into
asus-wmi, by having hid-asus register a listener for brightness events
to it.

This allows Asus laptops that have both WMI and Aura devices to still
work without a rerouting quirk, the Z13 to work while having both a
lightbar and a keyboard (2 Aura devices), and specifically for devices
with removable keyboards such as the Z13, the ability to maintain the
same brightness file descriptor, so that handlers like upower do not
stop working after detaching the keyboard.

Finally, the last two patches send the brightness events to asus-wmi
to process them internally. This allows for 0-33%-66%-100% brightness,
instead of 100%/0% toggle. This mirrors how the thinkpad devices work
internally. Brightness events are broadcast, so KDE reports the
brightness changing correctly.

> These commits sure touches a lot of code to fix an (unspecified? Unclear to me at least) number of bugs.
>
> Given the fact I expect regressions (nothing personal: ASUS has a very large pool of models,
> many requiring specialized quirks as can be seen in the diff)
> wouldn't it be wise to take the chance and expose the "static" lighting mode,
> including colors handling, for every asus rog, tuf, tx and proart laptop
> in existence in a way that most already-existing programs (including the KDE widget) can handle?
> At least there would be a big incentive behind all these changes.

The patches for RGB have been removed from this series.

Thanks,
Antheas

> [^ the above paragraph is valid if the number of fixed bugs is small and there is at least another way to fix it/them].
>
> Surely data needed to check what model support
> what lighting mode is known at this point, would probably be wise
> to put such data in the kernel and make good use of it.
>
> Thanks,
> Denis
>
> >   - Remove folio patch (already merged)
> >   - Remove legacy fix patch 11 from V4. There is a small chance that
> >     without this patch, some old NKEY keyboards might not respond to
> >     RGB commands according to Luke, but the kernel driver does not do
> >     RGB currently. The 0x5d init is done by Armoury crate software in
> >     Windows. If an issue is found, we can re-add it or just remove patches
> >     1/2 before merging. However, init could use the cleanup.
> >
> > Changes since V4:
> >   - Fix KConfig (reported by kernel robot)
> >   - Fix Ilpo's nits, if I missed anything lmk
> >
> > Changes since V3:
> >   - Add initializer for 0x5d for old NKEY keyboards until it is verified
> >     that it is not needed for their media keys to function.
> >   - Cover init in asus-wmi with spinlock as per Hans
> >   - If asus-wmi registers WMI handler with brightness, init the brightness
> >     in USB Asus keyboards, per Hans.
> >   - Change hid handler name to asus-UNIQ:rgb:peripheral to match led class
> >   - Fix oops when unregistering asus-wmi by moving unregister outside of
> >     the spin lock (but after the asus reference is set to null)
> >
> > Changes since V2:
> >   - Check lazy init succeds in asus-wmi before setting register variable
> >   - make explicit check in asus_hid_register_listener for listener existing
> >     to avoid re-init
> >   - rename asus_brt to asus_hid in most places and harmonize everything
> >   - switch to a spinlock instead of a mutex to avoid kernel ooops
> >   - fixup hid device quirks to avoid multiple RGB devices while still exposing
> >     all input vendor devices. This includes moving rgb init to probe
> >     instead of the input_configured callbacks.
> >   - Remove fan key (during retest it appears to be 0xae that is already
> >     supported by hid-asus)
> >   - Never unregister asus::kbd_backlight while asus-wmi is active, as that
> >   - removes fds from userspace and breaks backlight functionality. All
> >   - current mainline drivers do not support backlight hotplugging, so most
> >     userspace software (e.g., KDE, UPower) is built with that assumption.
> >     For the Ally, since it disconnects its controller during sleep, this
> >     caused the backlight slider to not work in KDE.
> >
> > Changes since V1:
> >   - Add basic RGB support on hid-asus, (Z13/Ally) tested in KDE/Z13
> >   - Fix ifdef else having an invalid signature (reported by kernel robot)
> >   - Restore input arguments to init and keyboard function so they can
> >     be re-used for RGB controls.
> >   - Remove Z13 delay (it did not work to fix the touchpad) and replace it
> >     with a HID_GROUP_GENERIC quirk to allow hid-multitouch to load. Squash
> >     keyboard rename into it.
> >   - Unregister brightness listener before removing work queue to avoid
> >     a race condition causing corruption
> >   - Remove spurious mutex unlock in asus_brt_event
> >   - Place mutex lock in kbd_led_set after LED_UNREGISTERING check to avoid
> >     relocking the mutex and causing a deadlock when unregistering leds
> >   - Add extra check during unregistering to avoid calling unregister when
> >     no led device is registered.
> >   - Temporarily HID_QUIRK_INPUT_PER_APP from the ROG endpoint as it causes
> >     the driver to create 4 RGB handlers per device. I also suspect some
> >     extra events sneak through (KDE had the @@@@@@).
> >
> > Antheas Kapenekakis (7):
> >   HID: asus: refactor init sequence per spec
> >   HID: asus: prevent binding to all HID devices on ROG
> >   platform/x86: asus-wmi: Add support for multiple kbd RGB handlers
> >   HID: asus: listen to the asus-wmi brightness device instead of
> >     creating one
> >   platform/x86: asus-wmi: remove unused keyboard backlight quirk
> >   platform/x86: asus-wmi: add keyboard brightness event handler
> >   HID: asus: add support for the asus-wmi brightness handler
> >
> >  drivers/hid/hid-asus.c                     | 235 +++++++++++----------
> >  drivers/platform/x86/asus-wmi.c            | 157 ++++++++++++--
> >  include/linux/platform_data/x86/asus-wmi.h |  69 +++---
> >  3 files changed, 291 insertions(+), 170 deletions(-)
> >
> >
> > base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
>


^ permalink raw reply

* Re: [PATCH v6 4/7] HID: asus: listen to the asus-wmi brightness device instead of creating one
From: Antheas Kapenekakis @ 2025-10-13 21:57 UTC (permalink / raw)
  To: Denis Benato
  Cc: platform-driver-x86, linux-input, linux-kernel, Jiri Kosina,
	Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
	Ilpo Järvinen
In-Reply-To: <e1e6ee09-ea29-4328-9eae-f2a4a23b3edc@gmail.com>

On Mon, 13 Oct 2025 at 23:44, Denis Benato <benato.denis96@gmail.com> wrote:
>
>
> On 10/13/25 22:15, Antheas Kapenekakis wrote:
> > Some ROG laptops expose multiple interfaces for controlling the
> > keyboard/RGB brightness. This creates a name conflict under
> > asus::kbd_brightness, where the second device ends up being
> > named asus::kbd_brightness_1 and they are both broken.
> Can you please reference a bug report and/or an analysis of why they ends
> up being broken?

You can reference the V1 description [1]

[1] https://lore.kernel.org/all/20250319191320.10092-1-lkml@antheas.dev/

> >
> > Therefore, register a listener to the asus-wmi brightness device
> > instead of creating a new one.
> >
> > Reviewed-by: Luke D. Jones <luke@ljones.dev>
> > Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> > ---
> >  drivers/hid/hid-asus.c | 64 +++++++-----------------------------------
> >  1 file changed, 10 insertions(+), 54 deletions(-)
> >
> > diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> > index a62559e3e064..0af19c8ef035 100644
> > --- a/drivers/hid/hid-asus.c
> > +++ b/drivers/hid/hid-asus.c
> > @@ -102,7 +102,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
> >  #define TRKID_SGN       ((TRKID_MAX + 1) >> 1)
> >
> >  struct asus_kbd_leds {
> > -     struct led_classdev cdev;
> > +     struct asus_hid_listener listener;
> It is my understanding from "register a listener .... instead of creating a new one"
> that you are attempting to use the same listener among many devices... so why isn't
> this a pointer? And more importantly: why do we have bool available, bool registered
> instead of either one or the other being replaced by this field being possibly NULL?

A listener is the handle that is passed to asus-wmi so that it can
communicate with hid-asus. Since the flow of communication flows from
asus-wmi -> hid-asus, the pointer is placed on asus-wmi.

The boolean kbd_led_avail is used to signify whether the BIOS supports
RGB commands. If not, we still want the common handler to be there to
link multiple hid-asus devices together. At the same time, we need to
skip calling the bios commands for brightness, and hold a value for
the previous brightness outside the bios.

The kbd_led_registered fixes the race condition that happens between
hid-asus and asus-wmi. Specifically, it ensures that the rgb listener
is only setup once, either once asus-wmi loads (if it supports RGB) or
when the first hid device loads.

Best,
Antheas

> >       struct hid_device *hdev;
> >       struct work_struct work;
> >       unsigned int brightness;
> > @@ -495,11 +495,11 @@ static void asus_schedule_work(struct asus_kbd_leds *led)
> >       spin_unlock_irqrestore(&led->lock, flags);
> >  }
> >
> > -static void asus_kbd_backlight_set(struct led_classdev *led_cdev,
> > -                                enum led_brightness brightness)
> > +static void asus_kbd_backlight_set(struct asus_hid_listener *listener,
> > +                                int brightness)
> >  {
> > -     struct asus_kbd_leds *led = container_of(led_cdev, struct asus_kbd_leds,
> > -                                              cdev);
> > +     struct asus_kbd_leds *led = container_of(listener, struct asus_kbd_leds,
> > +                                              listener);
> >       unsigned long flags;
> >
> >       spin_lock_irqsave(&led->lock, flags);
> > @@ -509,20 +509,6 @@ static void asus_kbd_backlight_set(struct led_classdev *led_cdev,
> >       asus_schedule_work(led);
> >  }
> >
> > -static enum led_brightness asus_kbd_backlight_get(struct led_classdev *led_cdev)
> > -{
> > -     struct asus_kbd_leds *led = container_of(led_cdev, struct asus_kbd_leds,
> > -                                              cdev);
> > -     enum led_brightness brightness;
> > -     unsigned long flags;
> > -
> > -     spin_lock_irqsave(&led->lock, flags);
> > -     brightness = led->brightness;
> > -     spin_unlock_irqrestore(&led->lock, flags);
> > -
> > -     return brightness;
> > -}
> > -
> >  static void asus_kbd_backlight_work(struct work_struct *work)
> >  {
> >       struct asus_kbd_leds *led = container_of(work, struct asus_kbd_leds, work);
> > @@ -539,34 +525,6 @@ static void asus_kbd_backlight_work(struct work_struct *work)
> >               hid_err(led->hdev, "Asus failed to set keyboard backlight: %d\n", ret);
> >  }
> >
> > -/* WMI-based keyboard backlight LED control (via asus-wmi driver) takes
> > - * precedence. We only activate HID-based backlight control when the
> > - * WMI control is not available.
> > - */
> > -static bool asus_kbd_wmi_led_control_present(struct hid_device *hdev)
> > -{
> > -     struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
> > -     u32 value;
> > -     int ret;
> > -
> > -     if (!IS_ENABLED(CONFIG_ASUS_WMI))
> > -             return false;
> > -
> > -     if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD &&
> > -                     dmi_check_system(asus_use_hid_led_dmi_ids)) {
> > -             hid_info(hdev, "using HID for asus::kbd_backlight\n");
> > -             return false;
> > -     }
> > -
> > -     ret = asus_wmi_evaluate_method(ASUS_WMI_METHODID_DSTS,
> > -                                    ASUS_WMI_DEVID_KBD_BACKLIGHT, 0, &value);
> > -     hid_dbg(hdev, "WMI backlight check: rc %d value %x", ret, value);
> > -     if (ret)
> > -             return false;
> > -
> > -     return !!(value & ASUS_WMI_DSTS_PRESENCE_BIT);
> > -}
> > -
> >  /*
> >   * We don't care about any other part of the string except the version section.
> >   * Example strings: FGA80100.RC72LA.312_T01, FGA80100.RC71LS.318_T01
> > @@ -701,14 +659,11 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
> >       drvdata->kbd_backlight->removed = false;
> >       drvdata->kbd_backlight->brightness = 0;
> >       drvdata->kbd_backlight->hdev = hdev;
> > -     drvdata->kbd_backlight->cdev.name = "asus::kbd_backlight";
> > -     drvdata->kbd_backlight->cdev.max_brightness = 3;
> > -     drvdata->kbd_backlight->cdev.brightness_set = asus_kbd_backlight_set;
> > -     drvdata->kbd_backlight->cdev.brightness_get = asus_kbd_backlight_get;
> > +     drvdata->kbd_backlight->listener.brightness_set = asus_kbd_backlight_set;
> >       INIT_WORK(&drvdata->kbd_backlight->work, asus_kbd_backlight_work);
> >       spin_lock_init(&drvdata->kbd_backlight->lock);
> >
> > -     ret = devm_led_classdev_register(&hdev->dev, &drvdata->kbd_backlight->cdev);
> > +     ret = asus_hid_register_listener(&drvdata->kbd_backlight->listener);
> >       if (ret < 0) {
> >               /* No need to have this still around */
> >               devm_kfree(&hdev->dev, drvdata->kbd_backlight);
> > @@ -1105,7 +1060,7 @@ static int __maybe_unused asus_resume(struct hid_device *hdev) {
> >
> >       if (drvdata->kbd_backlight) {
> >               const u8 buf[] = { FEATURE_KBD_REPORT_ID, 0xba, 0xc5, 0xc4,
> > -                             drvdata->kbd_backlight->cdev.brightness };
> > +                             drvdata->kbd_backlight->brightness };
> >               ret = asus_kbd_set_report(hdev, buf, sizeof(buf));
> >               if (ret < 0) {
> >                       hid_err(hdev, "Asus failed to set keyboard backlight: %d\n", ret);
> > @@ -1241,7 +1196,6 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
> >       }
> >
> >       if (is_vendor && (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT) &&
> > -         !asus_kbd_wmi_led_control_present(hdev) &&
> >           asus_kbd_register_leds(hdev))
> >               hid_warn(hdev, "Failed to initialize backlight.\n");
> >
> > @@ -1282,6 +1236,8 @@ static void asus_remove(struct hid_device *hdev)
> >       unsigned long flags;
> >
> >       if (drvdata->kbd_backlight) {
> > +             asus_hid_unregister_listener(&drvdata->kbd_backlight->listener);
> > +
> >               spin_lock_irqsave(&drvdata->kbd_backlight->lock, flags);
> >               drvdata->kbd_backlight->removed = true;
> >               spin_unlock_irqrestore(&drvdata->kbd_backlight->lock, flags);
>


^ permalink raw reply

page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox