Linux Input/HID development
 help / color / mirror / Atom feed
* Re: [PATCH] Input: touchscreen: tsc2007: Reduce I2C transactions for Z2 read
From: Dmitry Torokhov @ 2026-05-05  0:29 UTC (permalink / raw)
  To: Andreas Kemnade; +Cc: Yuki Horii, clamor95, johannes.kirchmair, linux-input
In-Reply-To: <20260502125313.3e1c296a@kemnade.info>

On Sat, May 02, 2026 at 12:53:13PM +0200, Andreas Kemnade wrote:
> Hi,
> 
> On Sun, 19 Apr 2026 17:18:23 -0700
> Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> 
> > Hi Yuki,
> > 
> > On Fri, Apr 10, 2026 at 04:41:00PM +0900, Yuki Horii wrote:
> > > From: Yuki Horii <yuuki198708@gmail.com>
> > > 
> > > The current implementation sends a separate power-down command
> > > after reading the Z2 value, resulting in an extra I2C
> > > transaction per measurement cycle.
> > > 
> > > The TSC2007 command byte contains a 2-bit power-down mode
> > > selection field. By selecting the power-down state in the Z2
> > > measurement command, the device powers down after the Z2 A/D
> > > conversion completes, eliminating the subsequent power-down
> > > transaction.
> > > 
> > > This reduces the number of I2C transactions by one per touch
> > > measurement cycle, decreasing I2C bus overhead and improving
> > > touch sampling performance.
> > > 
> > > Signed-off-by: Yuki Horii <yuuki198708@gmail.com>
> > > ---
> > >  drivers/input/touchscreen/tsc2007_core.c | 6 ++----
> > >  1 file changed, 2 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/input/touchscreen/tsc2007_core.c b/drivers/input/touchscreen/tsc2007_core.c
> > > index 948935de894b..ff60245baa96 100644
> > > --- a/drivers/input/touchscreen/tsc2007_core.c
> > > +++ b/drivers/input/touchscreen/tsc2007_core.c
> > > @@ -61,10 +61,8 @@ static void tsc2007_read_values(struct tsc2007 *tsc, struct ts_event *tc)
> > >  
> > >  	/* turn y+ off, x- on; we'll use formula #1 */
> > >  	tc->z1 = tsc2007_xfer(tsc, READ_Z1);
> > > -	tc->z2 = tsc2007_xfer(tsc, READ_Z2);
> > > -
> > > -	/* Prepare for next touch reading - power down ADC, enable PENIRQ */
> > > -	tsc2007_xfer(tsc, PWRDOWN);
> > > +	/* Read Z2 and power down ADC after A/D conversion, enable PENIRQ */
> > > +	tc->z2 = tsc2007_xfer(tsc, (TSC2007_POWER_OFF_IRQ_EN | TSC2007_MEASURE_Z2));
> > >  }
> > >  
> > >  u32 tsc2007_calculate_resistance(struct tsc2007 *tsc, struct ts_event *tc)  
> > 
> > Thank you for the patch.
> > 
> > I'd like people using this part to chime in (CCed).
> > 
> This looks like that there might be some reason behind it. But I cannot find any.
> Either by reading the datasheet nor by testing.
> 
> I have checked the interrupt counter. Output seems still be same.
> Interrupts appear when finger is moving.
> 
> So 
> Tested-by: Andreas Kemnade <andreas@kemnade.info> # GTA04

Thank you for giving it a spin Andreas.

Applied, thank you Yuki.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v3 4/4] HID: wacom: use __free(kfree) to clean up temporary buffers
From: Dmitry Torokhov @ 2026-05-04 23:15 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Jiri Kosina, Filipe Laíns, Bastien Nocera, Ping Cheng,
	Jason Gerecke, Viresh Kumar, Johan Hovold, Alex Elder,
	Greg Kroah-Hartman, Lee Jones, Icenowy Zheng, linux-input,
	linux-kernel, greybus-dev, linux-staging, linux-usb
In-Reply-To: <20260504-wip-fix-core-v3-4-ce1f11f4968f@kernel.org>

Hi Benjamin,

On Mon, May 04, 2026 at 10:47:25AM +0200, Benjamin Tissoires wrote:
> @@ -386,10 +381,11 @@ static void wacom_feature_mapping(struct hid_device *hdev,
>  	case WACOM_HID_WD_OFFSETRIGHT:
>  	case WACOM_HID_WD_OFFSETBOTTOM:
>  		/* read manually */
> -		n = hid_report_len(field->report);
> -		data = hid_alloc_report_buf(field->report, GFP_KERNEL);
> +		u8 *data __free(kfree) = hid_alloc_report_buf(field->report, GFP_KERNEL);
> +
>  		if (!data)
>  			break;
> +		n = hid_report_len(field->report);
>  		data[0] = field->report->id;
>  		ret = wacom_get_report(hdev, HID_FEATURE_REPORT,
>  					data, n, WAC_CMD_RETRIES);
> @@ -400,7 +396,6 @@ static void wacom_feature_mapping(struct hid_device *hdev,
>  			hid_warn(hdev, "%s: could not retrieve sensor offsets\n",
>  				 __func__);
>  		}
> -		kfree(data);
>  		break;
>  	}

I'd recommend establishing a new scope for the "data", otherwise it is
fragile. If there was another label below then this cleanup would
explode since current scope of "data" is from the declaration point
until the end of the switch statement.

Having a dedicated scope makes lifertime explicit.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH 2/3] Input: atmel_mxt_ts - check mem_size before calculating config memory size
From: Dmitry Torokhov @ 2026-05-04 22:59 UTC (permalink / raw)
  To: Nick Dyer, linux-input; +Cc: Ricardo Ribalda, linux-kernel
In-Reply-To: <20260504185448.4055973-2-dmitry.torokhov@gmail.com>

On Mon, May 04, 2026 at 11:54:46AM -0700, Dmitry Torokhov wrote:
> In mxt_update_cfg(), the driver calculates the memory size needed to store
> the configuration as data->mem_size - cfg.start_ofs. If data->mem_size is
> less than or equal to cfg.start_ofs, this calculation will underflow or
> result in a zero-size buffer, neither of which is valid for a configuration
> update.
> 
> Add a check to return -EINVAL if data->mem_size is too small. While at it,
> change the types of start_ofs and mem_size in struct mxt_cfg to u16 to
> match the device address space.
> 
> Assisted-by: Gemini:gemini-3.1-pro
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  drivers/input/touchscreen/atmel_mxt_ts.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index 28b2bd889c70..d660cc5b5fe3 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -275,8 +275,8 @@ struct mxt_cfg {
>  	off_t raw_pos;
>  
>  	u8 *mem;
> -	size_t mem_size;
> -	int start_ofs;
> +	u16 mem_size;
> +	u16 start_ofs;
>  
>  	struct mxt_info info;
>  };
> @@ -1657,6 +1657,13 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *fw)
>  	cfg.start_ofs = MXT_OBJECT_START +
>  			data->info->object_num * sizeof(struct mxt_object) +
>  			MXT_INFO_CHECKSUM_SIZE;
> +
> +	if (data->mem_size < cfg.start_ofs) {

This is supposed to be "<=", like the commit message says.

> +		dev_err(dev, "Memory size too small: %u < %u\n",
> +			data->mem_size, cfg.start_ofs);
> +		return -EINVAL;
> +	}
> +
>  	cfg.mem_size = data->mem_size - cfg.start_ofs;
>  
>  	u8 *mem_buf __free(kfree) = cfg.mem = kzalloc(cfg.mem_size, GFP_KERNEL);

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH 6/6] leds: led-class: mark classdev as unregistering early
From: kernel test robot @ 2026-05-04 21:49 UTC (permalink / raw)
  To: James Ye, jikos, bentiss, lee, pavel
  Cc: llvm, oe-kbuild-all, linux-input, linux-leds, linux-kernel,
	denis.benato, James Ye
In-Reply-To: <20260503072643.2774762-7-jye836@gmail.com>

Hi James,

kernel test robot noticed the following build errors:

[auto build test ERROR on hid/for-next]
[also build test ERROR on lee-leds/for-leds-next linus/master v6.16-rc1 next-20260430]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/James-Ye/HID-input-delete-hid_battery-on-disconnect/20260504-013406
base:   https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
patch link:    https://lore.kernel.org/r/20260503072643.2774762-7-jye836%40gmail.com
patch subject: [PATCH 6/6] leds: led-class: mark classdev as unregistering early
config: x86_64-kexec (https://download.01.org/0day-ci/archive/20260504/202605042355.hVEd8jjX-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260504/202605042355.hVEd8jjX-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202605042355.hVEd8jjX-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/hid/hid-input.c:2423:48: error: no member named 'batteries' in 'struct hid_device'
    2423 |         list_for_each_entry_safe(bat, bat_next, &hid->batteries, list) {
         |                                                  ~~~  ^
   include/linux/list.h:869:30: note: expanded from macro 'list_for_each_entry_safe'
     869 |         for (pos = list_first_entry(head, typeof(*pos), member),        \
         |                                     ^~~~
   include/linux/list.h:620:14: note: expanded from macro 'list_first_entry'
     620 |         list_entry((ptr)->next, type, member)
         |                     ^~~
   include/linux/list.h:609:15: note: expanded from macro 'list_entry'
     609 |         container_of(ptr, type, member)
         |                      ^~~
   include/linux/container_of.h:20:26: note: expanded from macro 'container_of'
      20 |         void *__mptr = (void *)(ptr);                                   \
         |                                 ^~~
>> drivers/hid/hid-input.c:2423:48: error: no member named 'batteries' in 'struct hid_device'
    2423 |         list_for_each_entry_safe(bat, bat_next, &hid->batteries, list) {
         |                                                  ~~~  ^
   include/linux/list.h:869:30: note: expanded from macro 'list_for_each_entry_safe'
     869 |         for (pos = list_first_entry(head, typeof(*pos), member),        \
         |                                     ^~~~
   include/linux/list.h:620:14: note: expanded from macro 'list_first_entry'
     620 |         list_entry((ptr)->next, type, member)
         |                     ^~~
   include/linux/list.h:609:15: note: expanded from macro 'list_entry'
     609 |         container_of(ptr, type, member)
         |                      ^~~
   note: (skipping 1 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:610:63: note: expanded from macro '__same_type'
     610 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
         |                                                               ^
   include/linux/build_bug.h:79:50: note: expanded from macro 'static_assert'
      79 | #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
         |                                                  ^~~~
   include/linux/build_bug.h:80:56: note: expanded from macro '__static_assert'
      80 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
         |                                                        ^~~~
>> drivers/hid/hid-input.c:2423:48: error: no member named 'batteries' in 'struct hid_device'
    2423 |         list_for_each_entry_safe(bat, bat_next, &hid->batteries, list) {
         |                                                  ~~~  ^
   include/linux/list.h:869:30: note: expanded from macro 'list_for_each_entry_safe'
     869 |         for (pos = list_first_entry(head, typeof(*pos), member),        \
         |                                     ^~~~
   include/linux/list.h:620:14: note: expanded from macro 'list_first_entry'
     620 |         list_entry((ptr)->next, type, member)
         |                     ^~~
   include/linux/list.h:609:15: note: expanded from macro 'list_entry'
     609 |         container_of(ptr, type, member)
         |                      ^~~
   note: (skipping 1 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:610:63: note: expanded from macro '__same_type'
     610 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
         |                                                               ^
   include/linux/build_bug.h:79:50: note: expanded from macro 'static_assert'
      79 | #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
         |                                                  ^~~~
   include/linux/build_bug.h:80:56: note: expanded from macro '__static_assert'
      80 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
         |                                                        ^~~~
>> drivers/hid/hid-input.c:2423:48: error: no member named 'batteries' in 'struct hid_device'
    2423 |         list_for_each_entry_safe(bat, bat_next, &hid->batteries, list) {
         |                                                  ~~~  ^
   include/linux/list.h:871:32: note: expanded from macro 'list_for_each_entry_safe'
     871 |              !list_entry_is_head(pos, head, member);                    \
         |                                       ^~~~
   include/linux/list.h:773:30: note: expanded from macro 'list_entry_is_head'
     773 |         list_is_head(&pos->member, (head))
         |                                     ^~~~
   4 errors generated.


vim +2423 drivers/hid/hid-input.c

  2407	
  2408	void hidinput_disconnect(struct hid_device *hid)
  2409	{
  2410		struct hid_input *hidinput, *next;
  2411		struct hid_battery *bat, *bat_next;
  2412	
  2413		list_for_each_entry_safe(hidinput, next, &hid->inputs, list) {
  2414			list_del(&hidinput->list);
  2415			if (hidinput->registered)
  2416				input_unregister_device(hidinput->input);
  2417			else
  2418				input_free_device(hidinput->input);
  2419			kfree(hidinput->name);
  2420			kfree(hidinput);
  2421		}
  2422	
> 2423		list_for_each_entry_safe(bat, bat_next, &hid->batteries, list) {
  2424			list_del(&bat->list);
  2425		}
  2426	
  2427		/* led_work is spawned by input_dev callbacks, but doesn't access the
  2428		 * parent input_dev at all. Once all input devices are removed, we
  2429		 * know that led_work will never get restarted, so we can cancel it
  2430		 * synchronously and are safe. */
  2431		cancel_work_sync(&hid->led_work);
  2432	}
  2433	EXPORT_SYMBOL_GPL(hidinput_disconnect);
  2434	

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply

* [PATCH 3/3] Input: atmel_mxt_ts - use __free() for obuf in mxt_object_show
From: Dmitry Torokhov @ 2026-05-04 18:54 UTC (permalink / raw)
  To: Nick Dyer, linux-input; +Cc: Ricardo Ribalda, linux-kernel
In-Reply-To: <20260504185448.4055973-1-dmitry.torokhov@gmail.com>

Use the __free(kfree) macro for the obuf allocation in mxt_object_show()
to simplify the code.

Assisted-by: Gemini:gemini-3.1-pro
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index d660cc5b5fe3..6af4db5ed117 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -2870,14 +2870,12 @@ static ssize_t mxt_object_show(struct device *dev,
 	int count = 0;
 	int i, j;
 	int error;
-	u8 *obuf;
 
 	/* Pre-allocate buffer large enough to hold max sized object. */
-	obuf = kmalloc(256, GFP_KERNEL);
+	u8 *obuf __free(kfree) = kmalloc(256, GFP_KERNEL);
 	if (!obuf)
 		return -ENOMEM;
 
-	error = 0;
 	for (i = 0; i < data->info->object_num; i++) {
 		object = data->object_table + i;
 
@@ -2892,15 +2890,13 @@ static ssize_t mxt_object_show(struct device *dev,
 
 			error = __mxt_read_reg(data->client, addr, size, obuf);
 			if (error)
-				goto done;
+				return error;
 
 			count = mxt_show_instance(buf, count, object, j, obuf);
 		}
 	}
 
-done:
-	kfree(obuf);
-	return error ?: count;
+	return count;
 }
 
 static int mxt_check_firmware_format(struct device *dev,
-- 
2.54.0.545.g6539524ca2-goog


^ permalink raw reply related

* [PATCH 2/3] Input: atmel_mxt_ts - check mem_size before calculating config memory size
From: Dmitry Torokhov @ 2026-05-04 18:54 UTC (permalink / raw)
  To: Nick Dyer, linux-input; +Cc: Ricardo Ribalda, linux-kernel
In-Reply-To: <20260504185448.4055973-1-dmitry.torokhov@gmail.com>

In mxt_update_cfg(), the driver calculates the memory size needed to store
the configuration as data->mem_size - cfg.start_ofs. If data->mem_size is
less than or equal to cfg.start_ofs, this calculation will underflow or
result in a zero-size buffer, neither of which is valid for a configuration
update.

Add a check to return -EINVAL if data->mem_size is too small. While at it,
change the types of start_ofs and mem_size in struct mxt_cfg to u16 to
match the device address space.

Assisted-by: Gemini:gemini-3.1-pro
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 28b2bd889c70..d660cc5b5fe3 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -275,8 +275,8 @@ struct mxt_cfg {
 	off_t raw_pos;
 
 	u8 *mem;
-	size_t mem_size;
-	int start_ofs;
+	u16 mem_size;
+	u16 start_ofs;
 
 	struct mxt_info info;
 };
@@ -1657,6 +1657,13 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *fw)
 	cfg.start_ofs = MXT_OBJECT_START +
 			data->info->object_num * sizeof(struct mxt_object) +
 			MXT_INFO_CHECKSUM_SIZE;
+
+	if (data->mem_size < cfg.start_ofs) {
+		dev_err(dev, "Memory size too small: %u < %u\n",
+			data->mem_size, cfg.start_ofs);
+		return -EINVAL;
+	}
+
 	cfg.mem_size = data->mem_size - cfg.start_ofs;
 
 	u8 *mem_buf __free(kfree) = cfg.mem = kzalloc(cfg.mem_size, GFP_KERNEL);
-- 
2.54.0.545.g6539524ca2-goog


^ permalink raw reply related

* [PATCH 1/3] Input: atmel_mxt_ts - fix boundary check in mxt_prepare_cfg_mem
From: Dmitry Torokhov @ 2026-05-04 18:54 UTC (permalink / raw)
  To: Nick Dyer, linux-input; +Cc: Ricardo Ribalda, linux-kernel, stable

When a configuration file provides an object size that is larger than the
driver's known mxt_obj_size(object), the driver intends to discard the
extra bytes.

The loop iterates using for (i = 0; i < size; i++). Inside the loop, the
condition to skip processing extra bytes is:

    if (i > mxt_obj_size(object))
        continue;

Since i is a 0-based index, the valid indices for the object are 0 through
mxt_obj_size(object) - 1.

When i == mxt_obj_size(object), the condition evaluates to false, and the
code processes the byte instead of discarding it.

This causes the code to calculate byte_offset = reg + i - cfg->start_ofs
and writes the byte there, overwriting exactly one byte of the adjacent
instance or object.

Update the boundary check to skip extra bytes correctly by using >=.

Fixes: 50a77c658b80 ("Input: atmel_mxt_ts - download device config using firmware loader")
Cc: stable@vger.kernel.org
Assisted-by: Gemini:gemini-3.1-pro
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index d62bf2c95578..28b2bd889c70 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -1503,7 +1503,7 @@ static int mxt_prepare_cfg_mem(struct mxt_data *data, struct mxt_cfg *cfg)
 			}
 			cfg->raw_pos += offset;
 
-			if (i > mxt_obj_size(object))
+			if (i >= mxt_obj_size(object))
 				continue;
 
 			byte_offset = reg + i - cfg->start_ofs;
-- 
2.54.0.545.g6539524ca2-goog


^ permalink raw reply related

* Re: [PATCH] Input: atmel_mxt_ts - Set byte_offset as signed
From: Dmitry Torokhov @ 2026-05-04 17:00 UTC (permalink / raw)
  To: Ricardo Ribalda; +Cc: Nick Dyer, linux-input, linux-kernel, linux-media
In-Reply-To: <20260504-fix-sparse-v1-1-1071137cd280@chromium.org>

On Mon, May 04, 2026 at 10:55:03AM +0000, Ricardo Ribalda wrote:
> The calculations done to obtain byte_offset can result into a negative
> number, fix its type.
> 
> This patch fixes the following sparse error:
> 
> drivers/input/touchscreen/atmel_mxt_ts.c:1481:44: warning: unsigned value that used to be signed checked against zero?
> drivers/input/touchscreen/atmel_mxt_ts.c:1479:49: signed value source
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>

Applied, thank you.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v3 1/4] HID: pass the buffer size to hid_report_raw_event
From: Greg Kroah-Hartman @ 2026-05-04 12:21 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Jiri Kosina, Filipe Laíns, Bastien Nocera, Ping Cheng,
	Jason Gerecke, Viresh Kumar, Johan Hovold, Alex Elder, Lee Jones,
	Icenowy Zheng, linux-input, linux-kernel, greybus-dev,
	linux-staging, linux-usb, stable
In-Reply-To: <20260504-wip-fix-core-v3-1-ce1f11f4968f@kernel.org>

On Mon, May 04, 2026 at 10:47:22AM +0200, Benjamin Tissoires wrote:
> commit 0a3fe972a7cb ("HID: core: Mitigate potential OOB by removing
> bogus memset()") enforced the provided data to be at least the size of
> the declared buffer in the report descriptor to prevent a buffer
> overflow. However, we can try to be smarter by providing both the buffer
> size and the data size, meaning that hid_report_raw_event() can make
> better decision whether we should plaining reject the buffer (buffer
> overflow attempt) or if we can safely memset it to 0 and pass it to the
> rest of the stack.
> 
> Fixes: 0a3fe972a7cb ("HID: core: Mitigate potential OOB by removing bogus memset()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply

* [PATCH] Input: atmel_mxt_ts - Set byte_offset as signed
From: Ricardo Ribalda @ 2026-05-04 10:55 UTC (permalink / raw)
  To: Nick Dyer, Dmitry Torokhov
  Cc: linux-input, linux-kernel, linux-media, Ricardo Ribalda

The calculations done to obtain byte_offset can result into a negative
number, fix its type.

This patch fixes the following sparse error:

drivers/input/touchscreen/atmel_mxt_ts.c:1481:44: warning: unsigned value that used to be signed checked against zero?
drivers/input/touchscreen/atmel_mxt_ts.c:1479:49: signed value source

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 87c6a10381f2..26ba82fb60b6 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -1397,7 +1397,8 @@ static int mxt_prepare_cfg_mem(struct mxt_data *data, struct mxt_cfg *cfg)
 {
 	struct device *dev = &data->client->dev;
 	struct mxt_object *object;
-	unsigned int type, instance, size, byte_offset;
+	unsigned int type, instance, size;
+	int byte_offset;
 	int offset;
 	int ret;
 	int i;

---
base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
change-id: 20260504-fix-sparse-3bfa6ba62e5b

Best regards,
-- 
Ricardo Ribalda <ribalda@chromium.org>


^ permalink raw reply related

* Re: [PATCH v3 1/4] HID: pass the buffer size to hid_report_raw_event
From: Johan Hovold @ 2026-05-04  9:31 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Jiri Kosina, Filipe Laíns, Bastien Nocera, Ping Cheng,
	Jason Gerecke, Viresh Kumar, Alex Elder, Greg Kroah-Hartman,
	Lee Jones, Icenowy Zheng, linux-input, linux-kernel, greybus-dev,
	linux-staging, linux-usb, stable
In-Reply-To: <20260504-wip-fix-core-v3-1-ce1f11f4968f@kernel.org>

On Mon, May 04, 2026 at 10:47:22AM +0200, Benjamin Tissoires wrote:
> commit 0a3fe972a7cb ("HID: core: Mitigate potential OOB by removing
> bogus memset()") enforced the provided data to be at least the size of
> the declared buffer in the report descriptor to prevent a buffer
> overflow. However, we can try to be smarter by providing both the buffer
> size and the data size, meaning that hid_report_raw_event() can make
> better decision whether we should plaining reject the buffer (buffer
> overflow attempt) or if we can safely memset it to 0 and pass it to the
> rest of the stack.
> 
> Fixes: 0a3fe972a7cb ("HID: core: Mitigate potential OOB by removing bogus memset()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
> ---
>  drivers/hid/bpf/hid_bpf_dispatch.c |  6 ++++--
>  drivers/hid/hid-core.c             | 42 +++++++++++++++++++++++++-------------
>  drivers/hid/hid-gfrm.c             |  4 ++--
>  drivers/hid/hid-logitech-hidpp.c   |  2 +-
>  drivers/hid/hid-multitouch.c       |  2 +-
>  drivers/hid/hid-primax.c           |  2 +-
>  drivers/hid/hid-vivaldi-common.c   |  2 +-
>  drivers/hid/wacom_sys.c            |  6 +++---
>  drivers/staging/greybus/hid.c      |  2 +-

The Greybus change builds fine now:

Acked-by: Johan Hovold <johan@kernel.org>

>  include/linux/hid.h                |  4 ++--
>  include/linux/hid_bpf.h            | 14 ++++++++-----
>  11 files changed, 53 insertions(+), 33 deletions(-)

Johan

^ permalink raw reply

* [PATCH v3 4/4] HID: wacom: use __free(kfree) to clean up temporary buffers
From: Benjamin Tissoires @ 2026-05-04  8:47 UTC (permalink / raw)
  To: Jiri Kosina, Filipe Laíns, Bastien Nocera, Ping Cheng,
	Jason Gerecke, Viresh Kumar, Johan Hovold, Alex Elder,
	Greg Kroah-Hartman, Lee Jones
  Cc: Icenowy Zheng, linux-input, linux-kernel, greybus-dev,
	linux-staging, linux-usb, Benjamin Tissoires
In-Reply-To: <20260504-wip-fix-core-v3-0-ce1f11f4968f@kernel.org>

This simplifies error handling and protects against memory leaks.

Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
 drivers/hid/hid-core.c  |  4 ++--
 drivers/hid/wacom_sys.c | 40 +++++++++++++---------------------------
 2 files changed, 15 insertions(+), 29 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index b3596851c719..7fc20e0405ad 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2050,8 +2050,8 @@ int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *
 		return 0;
 
 	if (unlikely(bsize < csize)) {
-		hid_warn_ratelimited(hid, "Event data for report %d is incorrect (%d vs %ld)\n",
-				     report->id, csize, bsize);
+		hid_warn_ratelimited(hid, "Event data for report %d is incorrect (%d vs %d)\n",
+				     report->id, csize, (unsigned int)bsize);
 		return -EINVAL;
 	}
 
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index a32320b351e3..adb31f54e524 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -70,11 +70,10 @@ static void wacom_wac_queue_flush(struct hid_device *hdev,
 {
 	while (!kfifo_is_empty(fifo)) {
 		int size = kfifo_peek_len(fifo);
-		u8 *buf;
 		unsigned int count;
 		int err;
 
-		buf = kzalloc(size, GFP_KERNEL);
+		u8 *buf __free(kfree) = kzalloc(size, GFP_KERNEL);
 		if (!buf) {
 			kfifo_skip(fifo);
 			continue;
@@ -87,7 +86,6 @@ static void wacom_wac_queue_flush(struct hid_device *hdev,
 			// to flush seems reasonable enough, however.
 			hid_warn(hdev, "%s: removed fifo entry with unexpected size\n",
 				 __func__);
-			kfree(buf);
 			continue;
 		}
 		err = hid_report_raw_event(hdev, HID_INPUT_REPORT, buf, size, size, false);
@@ -95,8 +93,6 @@ static void wacom_wac_queue_flush(struct hid_device *hdev,
 			hid_warn(hdev, "%s: unable to flush event due to error %d\n",
 				 __func__, err);
 		}
-
-		kfree(buf);
 	}
 }
 
@@ -311,7 +307,6 @@ static void wacom_feature_mapping(struct hid_device *hdev,
 	struct wacom_features *features = &wacom->wacom_wac.features;
 	struct hid_data *hid_data = &wacom->wacom_wac.hid_data;
 	unsigned int equivalent_usage = wacom_equivalent_usage(usage->hid);
-	u8 *data;
 	int ret;
 	u32 n;
 
@@ -325,10 +320,11 @@ static void wacom_feature_mapping(struct hid_device *hdev,
 		/* leave touch_max as is if predefined */
 		if (!features->touch_max) {
 			/* read manually */
-			n = hid_report_len(field->report);
-			data = hid_alloc_report_buf(field->report, GFP_KERNEL);
+			u8 *data __free(kfree) = hid_alloc_report_buf(field->report, GFP_KERNEL);
+
 			if (!data)
 				break;
+			n = hid_report_len(field->report);
 			data[0] = field->report->id;
 			ret = wacom_get_report(hdev, HID_FEATURE_REPORT,
 					       data, n, WAC_CMD_RETRIES);
@@ -344,7 +340,6 @@ static void wacom_feature_mapping(struct hid_device *hdev,
 					 "defaulting to %d\n",
 					  features->touch_max);
 			}
-			kfree(data);
 		}
 		break;
 	case HID_DG_INPUTMODE:
@@ -386,10 +381,11 @@ static void wacom_feature_mapping(struct hid_device *hdev,
 	case WACOM_HID_WD_OFFSETRIGHT:
 	case WACOM_HID_WD_OFFSETBOTTOM:
 		/* read manually */
-		n = hid_report_len(field->report);
-		data = hid_alloc_report_buf(field->report, GFP_KERNEL);
+		u8 *data __free(kfree) = hid_alloc_report_buf(field->report, GFP_KERNEL);
+
 		if (!data)
 			break;
+		n = hid_report_len(field->report);
 		data[0] = field->report->id;
 		ret = wacom_get_report(hdev, HID_FEATURE_REPORT,
 					data, n, WAC_CMD_RETRIES);
@@ -400,7 +396,6 @@ static void wacom_feature_mapping(struct hid_device *hdev,
 			hid_warn(hdev, "%s: could not retrieve sensor offsets\n",
 				 __func__);
 		}
-		kfree(data);
 		break;
 	}
 }
@@ -581,7 +576,6 @@ static int wacom_hid_set_device_mode(struct hid_device *hdev)
 static int wacom_set_device_mode(struct hid_device *hdev,
 				 struct wacom_wac *wacom_wac)
 {
-	u8 *rep_data;
 	struct hid_report *r;
 	struct hid_report_enum *re;
 	u32 length;
@@ -595,7 +589,7 @@ static int wacom_set_device_mode(struct hid_device *hdev,
 	if (!r)
 		return -EINVAL;
 
-	rep_data = hid_alloc_report_buf(r, GFP_KERNEL);
+	u8 *rep_data __free(kfree) = hid_alloc_report_buf(r, GFP_KERNEL);
 	if (!rep_data)
 		return -ENOMEM;
 
@@ -614,8 +608,6 @@ static int wacom_set_device_mode(struct hid_device *hdev,
 		 rep_data[1] != wacom_wac->mode_report &&
 		 limit++ < WAC_MSG_RETRIES);
 
-	kfree(rep_data);
-
 	return error < 0 ? error : 0;
 }
 
@@ -921,7 +913,6 @@ static int wacom_add_shared_data(struct hid_device *hdev)
 
 static int wacom_led_control(struct wacom *wacom)
 {
-	unsigned char *buf;
 	int retval;
 	unsigned char report_id = WAC_CMD_LED_CONTROL;
 	int buf_size = 9;
@@ -940,7 +931,8 @@ static int wacom_led_control(struct wacom *wacom)
 		report_id = WAC_CMD_WL_INTUOSP2;
 		buf_size = 51;
 	}
-	buf = kzalloc(buf_size, GFP_KERNEL);
+
+	unsigned char *buf __free(kfree) = kzalloc(buf_size, GFP_KERNEL);
 	if (!buf)
 		return -ENOMEM;
 
@@ -996,7 +988,6 @@ static int wacom_led_control(struct wacom *wacom)
 
 	retval = wacom_set_report(wacom->hdev, HID_FEATURE_REPORT, buf, buf_size,
 				  WAC_CMD_RETRIES);
-	kfree(buf);
 
 	return retval;
 }
@@ -1004,11 +995,10 @@ static int wacom_led_control(struct wacom *wacom)
 static int wacom_led_putimage(struct wacom *wacom, int button_id, u8 xfer_id,
 		const unsigned len, const void *img)
 {
-	unsigned char *buf;
 	int i, retval;
 	const unsigned chunk_len = len / 4; /* 4 chunks are needed to be sent */
 
-	buf = kzalloc(chunk_len + 3 , GFP_KERNEL);
+	unsigned char *buf __free(kfree) = kzalloc(chunk_len + 3, GFP_KERNEL);
 	if (!buf)
 		return -ENOMEM;
 
@@ -1018,7 +1008,7 @@ static int wacom_led_putimage(struct wacom *wacom, int button_id, u8 xfer_id,
 	retval = wacom_set_report(wacom->hdev, HID_FEATURE_REPORT, buf, 2,
 				  WAC_CMD_RETRIES);
 	if (retval < 0)
-		goto out;
+		return retval;
 
 	buf[0] = xfer_id;
 	buf[1] = button_id & 0x07;
@@ -1038,8 +1028,6 @@ static int wacom_led_putimage(struct wacom *wacom, int button_id, u8 xfer_id,
 	wacom_set_report(wacom->hdev, HID_FEATURE_REPORT, buf, 2,
 			 WAC_CMD_RETRIES);
 
-out:
-	kfree(buf);
 	return retval;
 }
 
@@ -1948,10 +1936,9 @@ static int wacom_remote_create_attr_group(struct wacom *wacom, __u32 serial,
 static int wacom_cmd_unpair_remote(struct wacom *wacom, unsigned char selector)
 {
 	const size_t buf_size = 2;
-	unsigned char *buf;
 	int retval;
 
-	buf = kzalloc(buf_size, GFP_KERNEL);
+	unsigned char *buf __free(kfree) = kzalloc(buf_size, GFP_KERNEL);
 	if (!buf)
 		return -ENOMEM;
 
@@ -1960,7 +1947,6 @@ static int wacom_cmd_unpair_remote(struct wacom *wacom, unsigned char selector)
 
 	retval = wacom_set_report(wacom->hdev, HID_OUTPUT_REPORT, buf,
 				  buf_size, WAC_CMD_RETRIES);
-	kfree(buf);
 
 	return retval;
 }

-- 
2.54.0


^ permalink raw reply related

* [PATCH v3 3/4] HID: multitouch: use __free(kfree) to clean up temporary buffers
From: Benjamin Tissoires @ 2026-05-04  8:47 UTC (permalink / raw)
  To: Jiri Kosina, Filipe Laíns, Bastien Nocera, Ping Cheng,
	Jason Gerecke, Viresh Kumar, Johan Hovold, Alex Elder,
	Greg Kroah-Hartman, Lee Jones
  Cc: Icenowy Zheng, linux-input, linux-kernel, greybus-dev,
	linux-staging, linux-usb, Benjamin Tissoires
In-Reply-To: <20260504-wip-fix-core-v3-0-ce1f11f4968f@kernel.org>

This simplifies error handling and protects against memory leaks.

Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
 drivers/hid/hid-multitouch.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index eeab0b6e32cc..b19463e545d6 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -507,7 +507,6 @@ static void mt_get_feature(struct hid_device *hdev, struct hid_report *report)
 {
 	int ret;
 	u32 size = hid_report_len(report);
-	u8 *buf;
 
 	/*
 	 * Do not fetch the feature report if the device has been explicitly
@@ -516,7 +515,7 @@ static void mt_get_feature(struct hid_device *hdev, struct hid_report *report)
 	if (hdev->quirks & HID_QUIRK_NO_INIT_REPORTS)
 		return;
 
-	buf = hid_alloc_report_buf(report, GFP_KERNEL);
+	u8 *buf __free(kfree) = hid_alloc_report_buf(report, GFP_KERNEL);
 	if (!buf)
 		return;
 
@@ -529,7 +528,7 @@ static void mt_get_feature(struct hid_device *hdev, struct hid_report *report)
 		/* The report ID in the request and the response should match */
 		if (report->id != buf[0]) {
 			hid_err(hdev, "Returned feature report did not match the request\n");
-			goto free;
+			return;
 		}
 
 		ret = hid_report_raw_event(hdev, HID_FEATURE_REPORT, buf,
@@ -537,9 +536,6 @@ static void mt_get_feature(struct hid_device *hdev, struct hid_report *report)
 		if (ret)
 			dev_warn(&hdev->dev, "failed to report feature\n");
 	}
-
-free:
-	kfree(buf);
 }
 
 static void mt_feature_mapping(struct hid_device *hdev,
@@ -1658,7 +1654,6 @@ static bool mt_need_to_apply_feature(struct hid_device *hdev,
 	struct mt_class *cls = &td->mtclass;
 	struct hid_report *report = field->report;
 	unsigned int index = usage->usage_index;
-	char *buf;
 	u32 report_len;
 	int max;
 
@@ -1673,17 +1668,18 @@ static bool mt_need_to_apply_feature(struct hid_device *hdev,
 			return false;
 
 		if (cls->quirks & MT_QUIRK_FORCE_GET_FEATURE) {
-			report_len = hid_report_len(report);
-			buf = hid_alloc_report_buf(report, GFP_KERNEL);
+			char *buf __free(kfree) = hid_alloc_report_buf(report, GFP_KERNEL);
+
 			if (!buf) {
 				hid_err(hdev,
 					"failed to allocate buffer for report\n");
 				return false;
 			}
+
+			report_len = hid_report_len(report);
 			hid_hw_raw_request(hdev, report->id, buf, report_len,
 					   HID_FEATURE_REPORT,
 					   HID_REQ_GET_REPORT);
-			kfree(buf);
 		}
 
 		field->value[index] = td->inputmode_value;

-- 
2.54.0


^ permalink raw reply related

* [PATCH v3 2/4] HID: core: introduce hid_safe_input_report()
From: Benjamin Tissoires @ 2026-05-04  8:47 UTC (permalink / raw)
  To: Jiri Kosina, Filipe Laíns, Bastien Nocera, Ping Cheng,
	Jason Gerecke, Viresh Kumar, Johan Hovold, Alex Elder,
	Greg Kroah-Hartman, Lee Jones
  Cc: Icenowy Zheng, linux-input, linux-kernel, greybus-dev,
	linux-staging, linux-usb, Benjamin Tissoires, stable
In-Reply-To: <20260504-wip-fix-core-v3-0-ce1f11f4968f@kernel.org>

hid_input_report() is used in too many places to have a commit that
doesn't cross subsystem borders. Instead of changing the API, introduce
a new one when things matters in the transport layers:
- usbhid
- i2chid

This effectively revert to the old behavior for those two transport
layers.

Fixes: 0a3fe972a7cb ("HID: core: Mitigate potential OOB by removing bogus memset()")
Cc: stable@vger.kernel.org
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
 drivers/hid/hid-core.c             | 25 +++++++++++++++++++++++++
 drivers/hid/i2c-hid/i2c-hid-core.c |  7 ++++---
 drivers/hid/usbhid/hid-core.c      | 11 ++++++-----
 include/linux/hid.h                |  2 ++
 4 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index a806820df7e5..b3596851c719 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2181,6 +2181,7 @@ static int __hid_input_report(struct hid_device *hid, enum hid_report_type type,
  * @interrupt: distinguish between interrupt and control transfers
  *
  * This is data entry for lower layers.
+ * Legacy, please use hid_safe_input_report() instead.
  */
 int hid_input_report(struct hid_device *hid, enum hid_report_type type, u8 *data, u32 size,
 		     int interrupt)
@@ -2191,6 +2192,30 @@ int hid_input_report(struct hid_device *hid, enum hid_report_type type, u8 *data
 }
 EXPORT_SYMBOL_GPL(hid_input_report);
 
+/**
+ * hid_safe_input_report - report data from lower layer (usb, bt...)
+ *
+ * @hid: hid device
+ * @type: HID report type (HID_*_REPORT)
+ * @data: report contents
+ * @bufsize: allocated size of the data buffer
+ * @size: useful size of data parameter
+ * @interrupt: distinguish between interrupt and control transfers
+ *
+ * This is data entry for lower layers.
+ * Please use this function instead of the non safe version because we provide
+ * here the size of the buffer, allowing hid-core to make smarter decisions
+ * regarding the incoming buffer.
+ */
+int hid_safe_input_report(struct hid_device *hid, enum hid_report_type type, u8 *data,
+			  size_t bufsize, u32 size, int interrupt)
+{
+	return __hid_input_report(hid, type, data, bufsize, size, interrupt, 0,
+				  false, /* from_bpf */
+				  false /* lock_already_taken */);
+}
+EXPORT_SYMBOL_GPL(hid_safe_input_report);
+
 bool hid_match_one_id(const struct hid_device *hdev,
 		      const struct hid_device_id *id)
 {
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index 5a183af3d5c6..e0a302544cef 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -574,9 +574,10 @@ static void i2c_hid_get_input(struct i2c_hid *ihid)
 		if (ihid->hid->group != HID_GROUP_RMI)
 			pm_wakeup_event(&ihid->client->dev, 0);
 
-		hid_input_report(ihid->hid, HID_INPUT_REPORT,
-				ihid->inbuf + sizeof(__le16),
-				ret_size - sizeof(__le16), 1);
+		hid_safe_input_report(ihid->hid, HID_INPUT_REPORT,
+				      ihid->inbuf + sizeof(__le16),
+				      ihid->bufsize - sizeof(__le16),
+				      ret_size - sizeof(__le16), 1);
 	}
 
 	return;
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index fbbfc0f60829..5af93b9b1fb5 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -283,9 +283,9 @@ static void hid_irq_in(struct urb *urb)
 			break;
 		usbhid_mark_busy(usbhid);
 		if (!test_bit(HID_RESUME_RUNNING, &usbhid->iofl)) {
-			hid_input_report(urb->context, HID_INPUT_REPORT,
-					 urb->transfer_buffer,
-					 urb->actual_length, 1);
+			hid_safe_input_report(urb->context, HID_INPUT_REPORT,
+					      urb->transfer_buffer, urb->transfer_buffer_length,
+					      urb->actual_length, 1);
 			/*
 			 * autosuspend refused while keys are pressed
 			 * because most keyboards don't wake up when
@@ -482,9 +482,10 @@ static void hid_ctrl(struct urb *urb)
 	switch (status) {
 	case 0:			/* success */
 		if (usbhid->ctrl[usbhid->ctrltail].dir == USB_DIR_IN)
-			hid_input_report(urb->context,
+			hid_safe_input_report(urb->context,
 				usbhid->ctrl[usbhid->ctrltail].report->type,
-				urb->transfer_buffer, urb->actual_length, 0);
+				urb->transfer_buffer, urb->transfer_buffer_length,
+				urb->actual_length, 0);
 		break;
 	case -ESHUTDOWN:	/* unplug */
 		unplug = 1;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index ac432a2ef415..bfb9859f391e 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -1030,6 +1030,8 @@ struct hid_field *hid_find_field(struct hid_device *hdev, unsigned int report_ty
 int hid_set_field(struct hid_field *, unsigned, __s32);
 int hid_input_report(struct hid_device *hid, enum hid_report_type type, u8 *data, u32 size,
 		     int interrupt);
+int hid_safe_input_report(struct hid_device *hid, enum hid_report_type type, u8 *data,
+			  size_t bufsize, u32 size, int interrupt);
 struct hid_field *hidinput_get_led_field(struct hid_device *hid);
 unsigned int hidinput_count_leds(struct hid_device *hid);
 __s32 hidinput_calc_abs_res(const struct hid_field *field, __u16 code);

-- 
2.54.0


^ permalink raw reply related

* [PATCH v3 1/4] HID: pass the buffer size to hid_report_raw_event
From: Benjamin Tissoires @ 2026-05-04  8:47 UTC (permalink / raw)
  To: Jiri Kosina, Filipe Laíns, Bastien Nocera, Ping Cheng,
	Jason Gerecke, Viresh Kumar, Johan Hovold, Alex Elder,
	Greg Kroah-Hartman, Lee Jones
  Cc: Icenowy Zheng, linux-input, linux-kernel, greybus-dev,
	linux-staging, linux-usb, Benjamin Tissoires, stable
In-Reply-To: <20260504-wip-fix-core-v3-0-ce1f11f4968f@kernel.org>

commit 0a3fe972a7cb ("HID: core: Mitigate potential OOB by removing
bogus memset()") enforced the provided data to be at least the size of
the declared buffer in the report descriptor to prevent a buffer
overflow. However, we can try to be smarter by providing both the buffer
size and the data size, meaning that hid_report_raw_event() can make
better decision whether we should plaining reject the buffer (buffer
overflow attempt) or if we can safely memset it to 0 and pass it to the
rest of the stack.

Fixes: 0a3fe972a7cb ("HID: core: Mitigate potential OOB by removing bogus memset()")
Cc: stable@vger.kernel.org
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
 drivers/hid/bpf/hid_bpf_dispatch.c |  6 ++++--
 drivers/hid/hid-core.c             | 42 +++++++++++++++++++++++++-------------
 drivers/hid/hid-gfrm.c             |  4 ++--
 drivers/hid/hid-logitech-hidpp.c   |  2 +-
 drivers/hid/hid-multitouch.c       |  2 +-
 drivers/hid/hid-primax.c           |  2 +-
 drivers/hid/hid-vivaldi-common.c   |  2 +-
 drivers/hid/wacom_sys.c            |  6 +++---
 drivers/staging/greybus/hid.c      |  2 +-
 include/linux/hid.h                |  4 ++--
 include/linux/hid_bpf.h            | 14 ++++++++-----
 11 files changed, 53 insertions(+), 33 deletions(-)

diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c
index 50c7b45c59e3..d0130658091b 100644
--- a/drivers/hid/bpf/hid_bpf_dispatch.c
+++ b/drivers/hid/bpf/hid_bpf_dispatch.c
@@ -24,7 +24,8 @@ EXPORT_SYMBOL(hid_ops);
 
 u8 *
 dispatch_hid_bpf_device_event(struct hid_device *hdev, enum hid_report_type type, u8 *data,
-			      u32 *size, int interrupt, u64 source, bool from_bpf)
+			      size_t *buf_size, u32 *size, int interrupt, u64 source,
+			      bool from_bpf)
 {
 	struct hid_bpf_ctx_kern ctx_kern = {
 		.ctx = {
@@ -74,6 +75,7 @@ dispatch_hid_bpf_device_event(struct hid_device *hdev, enum hid_report_type type
 		*size = ret;
 	}
 
+	*buf_size = ctx_kern.ctx.allocated_size;
 	return ctx_kern.data;
 }
 EXPORT_SYMBOL_GPL(dispatch_hid_bpf_device_event);
@@ -505,7 +507,7 @@ __hid_bpf_input_report(struct hid_bpf_ctx *ctx, enum hid_report_type type, u8 *b
 	if (ret)
 		return ret;
 
-	return hid_ops->hid_input_report(ctx->hid, type, buf, size, 0, (u64)(long)ctx, true,
+	return hid_ops->hid_input_report(ctx->hid, type, buf, size, size, 0, (u64)(long)ctx, true,
 					 lock_already_taken);
 }
 
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 61afec5915ec..a806820df7e5 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2033,24 +2033,32 @@ int __hid_request(struct hid_device *hid, struct hid_report *report,
 }
 EXPORT_SYMBOL_GPL(__hid_request);
 
-int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *data, u32 size,
-			 int interrupt)
+int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *data,
+			 size_t bufsize, u32 size, int interrupt)
 {
 	struct hid_report_enum *report_enum = hid->report_enum + type;
 	struct hid_report *report;
 	struct hid_driver *hdrv;
 	int max_buffer_size = HID_MAX_BUFFER_SIZE;
 	u32 rsize, csize = size;
+	size_t bsize = bufsize;
 	u8 *cdata = data;
 	int ret = 0;
 
 	report = hid_get_report(report_enum, data);
 	if (!report)
-		goto out;
+		return 0;
+
+	if (unlikely(bsize < csize)) {
+		hid_warn_ratelimited(hid, "Event data for report %d is incorrect (%d vs %ld)\n",
+				     report->id, csize, bsize);
+		return -EINVAL;
+	}
 
 	if (report_enum->numbered) {
 		cdata++;
 		csize--;
+		bsize--;
 	}
 
 	rsize = hid_compute_report_size(report);
@@ -2063,11 +2071,16 @@ int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *
 	else if (rsize > max_buffer_size)
 		rsize = max_buffer_size;
 
+	if (bsize < rsize) {
+		hid_warn_ratelimited(hid, "Event data for report %d was too short (%d vs %ld)\n",
+				     report->id, rsize, bsize);
+		return -EINVAL;
+	}
+
 	if (csize < rsize) {
-		hid_warn_ratelimited(hid, "Event data for report %d was too short (%d vs %d)\n",
-				     report->id, rsize, csize);
-		ret = -EINVAL;
-		goto out;
+		dbg_hid("report %d is too short, (%d < %d)\n", report->id,
+			csize, rsize);
+		memset(cdata + csize, 0, rsize - csize);
 	}
 
 	if ((hid->claimed & HID_CLAIMED_HIDDEV) && hid->hiddev_report_event)
@@ -2075,7 +2088,7 @@ int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *
 	if (hid->claimed & HID_CLAIMED_HIDRAW) {
 		ret = hidraw_report_event(hid, data, size);
 		if (ret)
-			goto out;
+			return ret;
 	}
 
 	if (hid->claimed != HID_CLAIMED_HIDRAW && report->maxfield) {
@@ -2087,15 +2100,15 @@ int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *
 
 	if (hid->claimed & HID_CLAIMED_INPUT)
 		hidinput_report_event(hid, report);
-out:
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(hid_report_raw_event);
 
 
 static int __hid_input_report(struct hid_device *hid, enum hid_report_type type,
-			      u8 *data, u32 size, int interrupt, u64 source, bool from_bpf,
-			      bool lock_already_taken)
+			      u8 *data, size_t bufsize, u32 size, int interrupt, u64 source,
+			      bool from_bpf, bool lock_already_taken)
 {
 	struct hid_report_enum *report_enum;
 	struct hid_driver *hdrv;
@@ -2120,7 +2133,8 @@ static int __hid_input_report(struct hid_device *hid, enum hid_report_type type,
 	report_enum = hid->report_enum + type;
 	hdrv = hid->driver;
 
-	data = dispatch_hid_bpf_device_event(hid, type, data, &size, interrupt, source, from_bpf);
+	data = dispatch_hid_bpf_device_event(hid, type, data, &bufsize, &size, interrupt,
+					     source, from_bpf);
 	if (IS_ERR(data)) {
 		ret = PTR_ERR(data);
 		goto unlock;
@@ -2149,7 +2163,7 @@ static int __hid_input_report(struct hid_device *hid, enum hid_report_type type,
 			goto unlock;
 	}
 
-	ret = hid_report_raw_event(hid, type, data, size, interrupt);
+	ret = hid_report_raw_event(hid, type, data, bufsize, size, interrupt);
 
 unlock:
 	if (!lock_already_taken)
@@ -2171,7 +2185,7 @@ static int __hid_input_report(struct hid_device *hid, enum hid_report_type type,
 int hid_input_report(struct hid_device *hid, enum hid_report_type type, u8 *data, u32 size,
 		     int interrupt)
 {
-	return __hid_input_report(hid, type, data, size, interrupt, 0,
+	return __hid_input_report(hid, type, data, size, size, interrupt, 0,
 				  false, /* from_bpf */
 				  false /* lock_already_taken */);
 }
diff --git a/drivers/hid/hid-gfrm.c b/drivers/hid/hid-gfrm.c
index 699186ff2349..d2a56bf92b41 100644
--- a/drivers/hid/hid-gfrm.c
+++ b/drivers/hid/hid-gfrm.c
@@ -66,7 +66,7 @@ static int gfrm_raw_event(struct hid_device *hdev, struct hid_report *report,
 	switch (data[1]) {
 	case GFRM100_SEARCH_KEY_DOWN:
 		ret = hid_report_raw_event(hdev, HID_INPUT_REPORT, search_key_dn,
-					   sizeof(search_key_dn), 1);
+					   sizeof(search_key_dn), sizeof(search_key_dn), 1);
 		break;
 
 	case GFRM100_SEARCH_KEY_AUDIO_DATA:
@@ -74,7 +74,7 @@ static int gfrm_raw_event(struct hid_device *hdev, struct hid_report *report,
 
 	case GFRM100_SEARCH_KEY_UP:
 		ret = hid_report_raw_event(hdev, HID_INPUT_REPORT, search_key_up,
-					   sizeof(search_key_up), 1);
+					   sizeof(search_key_up), sizeof(search_key_up), 1);
 		break;
 
 	default:
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index b1330d23bd2d..b3ff9265377b 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -3673,7 +3673,7 @@ static int hidpp10_consumer_keys_raw_event(struct hidpp_device *hidpp,
 	memcpy(&consumer_report[1], &data[3], 4);
 	/* We are called from atomic context */
 	hid_report_raw_event(hidpp->hid_dev, HID_INPUT_REPORT,
-			     consumer_report, 5, 1);
+			     consumer_report, sizeof(consumer_report), 5, 1);
 
 	return 1;
 }
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index e82a3c4e5b44..eeab0b6e32cc 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -533,7 +533,7 @@ static void mt_get_feature(struct hid_device *hdev, struct hid_report *report)
 		}
 
 		ret = hid_report_raw_event(hdev, HID_FEATURE_REPORT, buf,
-					   size, 0);
+					   size, size, 0);
 		if (ret)
 			dev_warn(&hdev->dev, "failed to report feature\n");
 	}
diff --git a/drivers/hid/hid-primax.c b/drivers/hid/hid-primax.c
index e44d79dff8de..8db054280afb 100644
--- a/drivers/hid/hid-primax.c
+++ b/drivers/hid/hid-primax.c
@@ -44,7 +44,7 @@ static int px_raw_event(struct hid_device *hid, struct hid_report *report,
 			data[0] |= (1 << (data[idx] - 0xE0));
 			data[idx] = 0;
 		}
-		hid_report_raw_event(hid, HID_INPUT_REPORT, data, size, 0);
+		hid_report_raw_event(hid, HID_INPUT_REPORT, data, size, size, 0);
 		return 1;
 
 	default:	/* unknown report */
diff --git a/drivers/hid/hid-vivaldi-common.c b/drivers/hid/hid-vivaldi-common.c
index bf734055d4b6..b12bb5cc091a 100644
--- a/drivers/hid/hid-vivaldi-common.c
+++ b/drivers/hid/hid-vivaldi-common.c
@@ -85,7 +85,7 @@ void vivaldi_feature_mapping(struct hid_device *hdev,
 	}
 
 	ret = hid_report_raw_event(hdev, HID_FEATURE_REPORT, report_data,
-				   report_len, 0);
+				   report_len, report_len, 0);
 	if (ret) {
 		dev_warn(&hdev->dev, "failed to report feature %d\n",
 			 field->report->id);
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 0d1c6d90fe21..a32320b351e3 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -90,7 +90,7 @@ static void wacom_wac_queue_flush(struct hid_device *hdev,
 			kfree(buf);
 			continue;
 		}
-		err = hid_report_raw_event(hdev, HID_INPUT_REPORT, buf, size, false);
+		err = hid_report_raw_event(hdev, HID_INPUT_REPORT, buf, size, size, false);
 		if (err) {
 			hid_warn(hdev, "%s: unable to flush event due to error %d\n",
 				 __func__, err);
@@ -334,7 +334,7 @@ static void wacom_feature_mapping(struct hid_device *hdev,
 					       data, n, WAC_CMD_RETRIES);
 			if (ret == n && features->type == HID_GENERIC) {
 				ret = hid_report_raw_event(hdev,
-					HID_FEATURE_REPORT, data, n, 0);
+					HID_FEATURE_REPORT, data, n, n, 0);
 			} else if (ret == 2 && features->type != HID_GENERIC) {
 				features->touch_max = data[1];
 			} else {
@@ -395,7 +395,7 @@ static void wacom_feature_mapping(struct hid_device *hdev,
 					data, n, WAC_CMD_RETRIES);
 		if (ret == n) {
 			ret = hid_report_raw_event(hdev, HID_FEATURE_REPORT,
-						   data, n, 0);
+						   data, n, n, 0);
 		} else {
 			hid_warn(hdev, "%s: could not retrieve sensor offsets\n",
 				 __func__);
diff --git a/drivers/staging/greybus/hid.c b/drivers/staging/greybus/hid.c
index 1f58c907c036..f1f9f6fbc00e 100644
--- a/drivers/staging/greybus/hid.c
+++ b/drivers/staging/greybus/hid.c
@@ -201,7 +201,7 @@ static void gb_hid_init_report(struct gb_hid *ghid, struct hid_report *report)
 	 * we just need to setup the input fields, so using
 	 * hid_report_raw_event is safe.
 	 */
-	hid_report_raw_event(ghid->hid, report->type, ghid->inbuf, size, 1);
+	hid_report_raw_event(ghid->hid, report->type, ghid->inbuf, ghid->bufsize, size, 1);
 }
 
 static void gb_hid_init_reports(struct gb_hid *ghid)
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 442a80d79e89..ac432a2ef415 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -1298,8 +1298,8 @@ static inline u32 hid_report_len(struct hid_report *report)
 	return DIV_ROUND_UP(report->size, 8) + (report->id > 0);
 }
 
-int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *data, u32 size,
-			 int interrupt);
+int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *data,
+			 size_t bufsize, u32 size, int interrupt);
 
 /* HID quirks API */
 unsigned long hid_lookup_quirk(const struct hid_device *hdev);
diff --git a/include/linux/hid_bpf.h b/include/linux/hid_bpf.h
index a2e47dbcf82c..19fffa4574a4 100644
--- a/include/linux/hid_bpf.h
+++ b/include/linux/hid_bpf.h
@@ -72,8 +72,8 @@ struct hid_ops {
 	int (*hid_hw_output_report)(struct hid_device *hdev, __u8 *buf, size_t len,
 				    u64 source, bool from_bpf);
 	int (*hid_input_report)(struct hid_device *hid, enum hid_report_type type,
-				u8 *data, u32 size, int interrupt, u64 source, bool from_bpf,
-				bool lock_already_taken);
+				u8 *data, size_t bufsize, u32 size, int interrupt, u64 source,
+				bool from_bpf, bool lock_already_taken);
 	struct module *owner;
 	const struct bus_type *bus_type;
 };
@@ -200,7 +200,8 @@ struct hid_bpf {
 
 #ifdef CONFIG_HID_BPF
 u8 *dispatch_hid_bpf_device_event(struct hid_device *hid, enum hid_report_type type, u8 *data,
-				  u32 *size, int interrupt, u64 source, bool from_bpf);
+				  size_t *buf_size, u32 *size, int interrupt, u64 source,
+				  bool from_bpf);
 int dispatch_hid_bpf_raw_requests(struct hid_device *hdev,
 				  unsigned char reportnum, __u8 *buf,
 				  u32 size, enum hid_report_type rtype,
@@ -215,8 +216,11 @@ int hid_bpf_device_init(struct hid_device *hid);
 const u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, const u8 *rdesc, unsigned int *size);
 #else /* CONFIG_HID_BPF */
 static inline u8 *dispatch_hid_bpf_device_event(struct hid_device *hid, enum hid_report_type type,
-						u8 *data, u32 *size, int interrupt,
-						u64 source, bool from_bpf) { return data; }
+						u8 *data, size_t *buf_size, u32 *size,
+						int interrupt, u64 source, bool from_bpf)
+{
+	return data;
+}
 static inline int dispatch_hid_bpf_raw_requests(struct hid_device *hdev,
 						unsigned char reportnum, u8 *buf,
 						u32 size, enum hid_report_type rtype,

-- 
2.54.0


^ permalink raw reply related

* [PATCH v3 0/4] HID: Proper fix for OOM in hid-core
From: Benjamin Tissoires @ 2026-05-04  8:47 UTC (permalink / raw)
  To: Jiri Kosina, Filipe Laíns, Bastien Nocera, Ping Cheng,
	Jason Gerecke, Viresh Kumar, Johan Hovold, Alex Elder,
	Greg Kroah-Hartman, Lee Jones
  Cc: Icenowy Zheng, linux-input, linux-kernel, greybus-dev,
	linux-staging, linux-usb, Benjamin Tissoires, stable

Commit 0a3fe972a7cb ("HID: core: Mitigate potential OOB by removing
bogus memset()") enforced the provided data to be at least the size of
the declared buffer in the report descriptor to prevent a buffer
overflow.

We only had corner cases of malicious devices exposing the OOM because
in most cases, the buffer provided by the transport layer needs to be
allocated at probe time and is large enough to handle all the possible
reports.

However, the patch from above, which enforces the spec a little bit more
introduced both regressions for devices not following the spec (not
necesserally malicious), but also a stream of errors for those devices.

Let's revert to the old behavior by giving more information to HID core
to be able to decide whether it can or not memset the rest of the buffer
to 0 and continue the processing.

Note that the first commit makes an API change, but the callers are
relatively limited, so it should be fine on its own. The second patch
can't really make the same kind of API change because we have too many
callers in various subsystems. We can switch them one by one to the safe
approach when needed.

The last 2 patches are small cleanups I initially put together with the
2 first patches, but they can be applied on their own and don't need to
be pulled in stable like the first 2.

Cheers,
Benjamin

Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
Changes in v3:
- fixed ghib -> ghid in greybus
- fixed i386 size_t debug size reported by kernel-bot
- Link to v2: https://lore.kernel.org/r/20260416-wip-fix-core-v2-0-be92570e5627@kernel.org

Changes in v2:
- added a small blurb explaining the difference between the safe and the
  non safe version of hid_safe_input_report
- Link to v1: https://lore.kernel.org/r/20260415-wip-fix-core-v1-0-ed3c4c823175@kernel.org

---
Benjamin Tissoires (4):
      HID: pass the buffer size to hid_report_raw_event
      HID: core: introduce hid_safe_input_report()
      HID: multitouch: use __free(kfree) to clean up temporary buffers
      HID: wacom: use __free(kfree) to clean up temporary buffers

 drivers/hid/bpf/hid_bpf_dispatch.c |  6 ++--
 drivers/hid/hid-core.c             | 67 ++++++++++++++++++++++++++++++--------
 drivers/hid/hid-gfrm.c             |  4 +--
 drivers/hid/hid-logitech-hidpp.c   |  2 +-
 drivers/hid/hid-multitouch.c       | 18 ++++------
 drivers/hid/hid-primax.c           |  2 +-
 drivers/hid/hid-vivaldi-common.c   |  2 +-
 drivers/hid/i2c-hid/i2c-hid-core.c |  7 ++--
 drivers/hid/usbhid/hid-core.c      | 11 ++++---
 drivers/hid/wacom_sys.c            | 46 +++++++++-----------------
 drivers/staging/greybus/hid.c      |  2 +-
 include/linux/hid.h                |  6 ++--
 include/linux/hid_bpf.h            | 14 +++++---
 13 files changed, 109 insertions(+), 78 deletions(-)
---
base-commit: 7df6572f1cb381d6b89ceed58e3b076c233c2cd0
change-id: 20260415-wip-fix-core-7d85c8516ed0

Best regards,
-- 
Benjamin Tissoires <bentiss@kernel.org>


^ permalink raw reply

* Re: [PATCH 5/6] HID: asus: add microphone mute LED support for T3304
From: James Ye @ 2026-05-04  1:27 UTC (permalink / raw)
  To: Denis Benato
  Cc: jikos, bentiss, lee, pavel, linux-input, linux-leds, linux-kernel
In-Reply-To: <769ce05a-3148-408f-b012-74a85c1a2343@linux.dev>

On Mon, 4 May 2026 at 08:45, Denis Benato <denis.benato@linux.dev> wrote:
> Isn't QUIRK_T3304_KEYBOARD enough? Or are you saying you know there is more than one keyboard
> model that requires this?

I don't know for certain if there are any other such keyboards with
LEDs that need to be handled like this, but I can imagine that there
are.

I decided to introduce an additional quirk because:
- I don't know how to detect if a micmute LED is present.
- The quirk could be used to implement support in other devices.
- asus_kbd_register_leds is also gated by QUIRK_USE_KBD_BACKLIGHT,
adding a device-specific quirk didn't feel appropriate.

If there turns out to be a widely-supported detection method and T330
doesn't support this, then just QUIRK_T3304_KEYBOARD would be better.

If you would prefer then I shall remove QUIRK_HID_MICMUTE. This can be
revisited when more than one device uses this code.

>
> Do we really want to use strlen?
>
> Do we really want to use strlen on the result of a function without additional checks?

My understanding is that this is safe, as this name is set by the
kernel. Existing use of dev_name appears to not involve additional
checks.

> This doesn't look like a good idea to change this for all laptops under the sun...
>
>
> Perhaps might make sense to move this check before? Maybe not?

Returning an error from asus_kbd_register_leds is currently non-fatal,
so the only difference in behaviour is that devices with
QUIRK_USE_KBD_BACKLIGHT that do not report supporting a backlight will
no longer log a warning.

T3304 indeed lacks a backlight, so we can't proceed with the backlight
setup code, but asus_kbd_get_functions() is still a prerequisite for
the micmute LED.

I think the alternative is:
- Move the asus_kbd_get_functions() call site from
asus_kbd_register_leds() to asus_probe(), and
- Separate asus_kbd_register_leds() into two functions for backlight
and micmute LEDs respectively, keeping the current behaviour for
backlight registration.

Your thoughts?

Cheers,
James

>
> Denis

^ permalink raw reply

* Re: [PATCH 5/6] HID: asus: add microphone mute LED support for T3304
From: Denis Benato @ 2026-05-03 22:45 UTC (permalink / raw)
  To: James Ye, jikos, bentiss, lee, pavel
  Cc: linux-input, linux-leds, linux-kernel
In-Reply-To: <20260503072643.2774762-6-jye836@gmail.com>


On 5/3/26 09:26, James Ye wrote:
> T3304 keyboard has a microphone mute LED indicator on the same key
> (Fn+F9). Look this up with a led_classdev.
>
> It does not have backlight LEDs, so return without failure from
> asus_kbd_register_leds if backlight support is not present.
>
> Signed-off-by: James Ye <jye836@gmail.com>
> ---
>  drivers/hid/hid-asus.c | 51 +++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 48 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index e4c97fddfaf1..4f68bc40b8bf 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -100,6 +100,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
>  #define QUIRK_ROG_ALLY_XPAD		BIT(13)
>  #define QUIRK_HID_FN_LOCK		BIT(14)
>  #define QUIRK_T3304_KEYBOARD		BIT(15)
> +#define QUIRK_HID_MICMUTE		BIT(16)
>  
Isn't QUIRK_T3304_KEYBOARD enough? Or are you saying you know there is more than one keyboard
model that requires this?
>  #define I2C_KEYBOARD_QUIRKS			(QUIRK_FIX_NOTEBOOK_REPORT | \
>  						 QUIRK_NO_INIT_REPORTS | \
> @@ -144,6 +145,7 @@ struct asus_drvdata {
>  	unsigned long battery_next_query;
>  	struct work_struct fn_lock_sync_work;
>  	bool fn_lock;
> +	struct led_classdev led_micmute;
>  };
>  
>  static int asus_report_battery(struct asus_drvdata *, u8 *, int);
> @@ -578,6 +580,26 @@ static int asus_kbd_disable_oobe(struct hid_device *hdev)
>  	return 0;
>  }
>  
> +static int asus_kbd_set_micmute_led(struct hid_device *hdev, bool enabled)
> +{
> +	u8 buf[] = { FEATURE_KBD_REPORT_ID, 0xd0, 0x7c, enabled };
> +
> +	return asus_kbd_set_report(hdev, buf, sizeof(buf));
> +}
> +
> +static int asus_led_brightness_set(struct led_classdev *led_cdev,
> +		enum led_brightness value)
> +{
> +	struct device *dev = led_cdev->dev->parent;
> +	struct hid_device *hdev = to_hid_device(dev);
> +	struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
> +
> +	if (led_cdev == &drvdata->led_micmute)
> +		return asus_kbd_set_micmute_led(hdev, !!value);
> +
> +	return -ENODEV;
> +}
> +
>  static int asus_kbd_set_fn_lock(struct hid_device *hdev, bool enabled)
>  {
>  	u8 buf[] = { FEATURE_KBD_REPORT_ID, 0xd0, 0x4e, !!enabled };
> @@ -752,9 +774,31 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
>  	if (ret < 0)
>  		return ret;
>  
> +	if (drvdata->quirks & QUIRK_HID_MICMUTE) {
> +		char *name_micmute;
> +		size_t name_sz = strlen(dev_name(&hdev->dev)) + 16;
> +
Do we really want to use strlen?

Do we really want to use strlen on the result of a function without additional checks?
> +		name_micmute = devm_kzalloc(&hdev->dev, name_sz, GFP_KERNEL);
> +		if (name_micmute == NULL)
> +			return -ENOMEM;
> +
> +		snprintf(name_micmute, name_sz, "%s::micmute", dev_name(&hdev->dev));
> +		drvdata->led_micmute.name = name_micmute;
> +		drvdata->led_micmute.default_trigger = "audio-micmute";
> +		drvdata->led_micmute.brightness_set_blocking = asus_led_brightness_set;
> +		drvdata->led_micmute.max_brightness = 1;
> +		drvdata->led_micmute.flags = LED_CORE_SUSPENDRESUME | LED_HW_PLUGGABLE;
> +		drvdata->led_micmute.dev = &hdev->dev;
> +		ret = devm_led_classdev_register(&hdev->dev, &drvdata->led_micmute);
> +		if (ret < 0) {
> +			hid_err(hdev, "Failed to register LED: %d.\n", ret);
> +			return ret;
> +		}
> +	}
> +
>  	/* Check for backlight support */
>  	if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
> -		return -ENODEV;
> +		return 0;
>  
This doesn't look like a good idea to change this for all laptops under the sun...


Perhaps might make sense to move this check before? Maybe not?

Denis
>  	if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
>  		ret = asus_kbd_disable_oobe(hdev);
> @@ -1315,7 +1359,8 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  	}
>  
>  	/* Laptops keyboard backlight is always at 0x5a */
> -	if (is_vendor && (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT) &&
> +	if (is_vendor &&
> +	    (drvdata->quirks & (QUIRK_USE_KBD_BACKLIGHT | QUIRK_HID_MICMUTE)) &&
>  	    (asus_has_report_id(hdev, FEATURE_KBD_REPORT_ID)) &&
>  		(asus_kbd_register_leds(hdev)))
>  		hid_warn(hdev, "Failed to initialize backlight.\n");
> @@ -1587,7 +1632,7 @@ static const struct hid_device_id asus_devices[] = {
>  		USB_VENDOR_ID_ASUSTEK, USB_DEVICE_ID_ASUSTEK_T101HA_KEYBOARD) },
>  	{ HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
>  		USB_VENDOR_ID_ASUSTEK, USB_DEVICE_ID_ASUSTEK_T3304_KEYBOARD),
> -	  QUIRK_T3304_KEYBOARD },
> +	  QUIRK_T3304_KEYBOARD | QUIRK_HID_MICMUTE },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(hid, asus_devices);

^ permalink raw reply

* Re: [PATCH 3/6] HID: asus: add support for T3304 detachable keyboard
From: Denis Benato @ 2026-05-03 22:23 UTC (permalink / raw)
  To: James Ye, jikos, bentiss, lee, pavel
  Cc: linux-input, linux-leds, linux-kernel
In-Reply-To: <20260503072643.2774762-4-jye836@gmail.com>


On 5/3/26 09:26, James Ye wrote:
> ASUSTek Computer, Inc. T3304 Soft Keyboard [0b05:1aad] is the detachable
> keyboard of the ASUS Vivobook 13 Slate OLED (T3304). It presents as a
> USB device with two interfaces: a keyboard and a pointing device
> (touchpad).
>
> Basic keyboard and full touchpad functionality work out-of-the-box with
> hid-generic and hid-multitouch, but function key combos e.g. volume,
> brightness control, home/end/pgup/pgdown require initialization.
>
> Bind the keyboard interface to hid-asus for initialization. The
> OEM-specific report descriptors required for this are present only on
> the touchpad interface, not the keyboard, so a quirk is used to add the
> required feature descriptor.
Reviewed-By: Denis Benato <denis.benato@linux.dev>
> Signed-off-by: James Ye <jye836@gmail.com>
> ---
>  drivers/hid/hid-asus.c | 58 +++++++++++++++++++++++++++++++++++++++---
>  drivers/hid/hid-ids.h  |  1 +
>  2 files changed, 56 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index ef9d5eba4dc9..e4c97fddfaf1 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -99,6 +99,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
>  #define QUIRK_ROG_CLAYMORE_II_KEYBOARD	BIT(12)
>  #define QUIRK_ROG_ALLY_XPAD		BIT(13)
>  #define QUIRK_HID_FN_LOCK		BIT(14)
> +#define QUIRK_T3304_KEYBOARD		BIT(15)
>
>  #define I2C_KEYBOARD_QUIRKS			(QUIRK_FIX_NOTEBOOK_REPORT | \
>  						 QUIRK_NO_INIT_REPORTS | \
> @@ -494,6 +495,14 @@ static int asus_kbd_init(struct hid_device *hdev, u8 report_id)
>  		return ret;
>  	}
>
> +	struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
> +
> +	/* T3304 keyboard always replies with 16 0xff bytes. Don't check for
> +	 * acknowledgment.
> +	 */
> +	if (drvdata->quirks & QUIRK_T3304_KEYBOARD)
> +		return 0;
> +
>  	u8 *readbuf __free(kfree) = kzalloc(FEATURE_KBD_REPORT_SIZE, GFP_KERNEL);
>  	if (!readbuf)
>  		return -ENOMEM;
> @@ -1312,10 +1321,12 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  		hid_warn(hdev, "Failed to initialize backlight.\n");
>
>  	/*
> -	 * For ROG keyboards, skip rename for consistency and ->input check as
> -	 * some devices do not have inputs.
> +	 * For ROG and T3304 keyboards, skip rename for consistency.
> +	 * For ROG keyboards, skip ->input check as some devices do not have
> +	 * inputs.
>  	 */
> -	if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD)
> +	if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD ||
> +	    drvdata->quirks & QUIRK_T3304_KEYBOARD)
>  		return 0;
>
>  	/*
> @@ -1369,6 +1380,22 @@ static const __u8 asus_g752_fixed_rdesc[] = {
>          0x2A, 0xFF, 0x00,		/*   Usage Maximum (0xFF)       */
>  };
>
> +static const __u8 asus_t3304_fixed_rdesc[] = {
> +	0x06, 0x31, 0xff,              // Usage Page (Vendor Usage Page 0xff31)
> +	0x09, 0x76,                    // Usage (Vendor Usage 0x76)
> +	0xa1, 0x01,                    // Collection (Application)
> +	0x05, 0xff,                    //  Usage Page (Vendor Usage Page 0xff)
> +	0x85, 0x5a,                    //  Report ID (90)
> +	0x19, 0x00,                    //  Usage Minimum (0)
> +	0x2a, 0xff, 0x00,              //  Usage Maximum (255)
> +	0x15, 0x00,                    //  Logical Minimum (0)
> +	0x26, 0xff, 0x00,              //  Logical Maximum (255)
> +	0x75, 0x08,                    //  Report Size (8)
> +	0x95, 0x0f,                    //  Report Count (15)
> +	0xb1, 0x02,                    //  Feature (Data,Var,Abs)
> +	0xc0,                          // End Collection
> +};
> +
>  static const __u8 *asus_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>  		unsigned int *rsize)
>  {
> @@ -1473,6 +1500,28 @@ static const __u8 *asus_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>  		}
>  	}
>
> +	/* T3304 keyboard's vendor descriptors are on the touchpad interface,
> +	 * not the keyboard. But we need hid-multitouch to handle the touchpad,
> +	 * Add a descriptor with only the config report so that this driver can
> +	 * perform initialization.
> +	 */
> +	if (drvdata->quirks & QUIRK_T3304_KEYBOARD) {
> +		__u8 *new_rdesc;
> +		size_t new_size = *rsize + sizeof(asus_t3304_fixed_rdesc);
> +
> +		new_rdesc = devm_kzalloc(&hdev->dev, new_size, GFP_KERNEL);
> +		if (new_rdesc == NULL)
> +			return rdesc;
> +
> +		hid_info(hdev, "Fixing up Asus T3304 keyboard report descriptor\n");
> +		memcpy(new_rdesc, rdesc, *rsize);
> +		memcpy(new_rdesc + *rsize, asus_t3304_fixed_rdesc,
> +		       sizeof(asus_t3304_fixed_rdesc));
> +
> +		*rsize = new_size;
> +		rdesc = new_rdesc;
> +	}
> +
>  	return rdesc;
>  }
>
> @@ -1536,6 +1585,9 @@ static const struct hid_device_id asus_devices[] = {
>  	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
>  	{ HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
>  		USB_VENDOR_ID_ASUSTEK, USB_DEVICE_ID_ASUSTEK_T101HA_KEYBOARD) },
> +	{ HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
> +		USB_VENDOR_ID_ASUSTEK, USB_DEVICE_ID_ASUSTEK_T3304_KEYBOARD),
> +	  QUIRK_T3304_KEYBOARD },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(hid, asus_devices);
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 0cf63742315b..ecf30e36a99d 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -219,6 +219,7 @@
>  #define USB_DEVICE_ID_ASUSTEK_T100CHI_KEYBOARD	0x8502
>  #define USB_DEVICE_ID_ASUSTEK_T101HA_KEYBOARD	0x183d
>  #define USB_DEVICE_ID_ASUSTEK_T304_KEYBOARD	0x184a
> +#define USB_DEVICE_ID_ASUSTEK_T3304_KEYBOARD	0x1aad
>  #define USB_DEVICE_ID_ASUSTEK_I2C_KEYBOARD	0x8585
>  #define USB_DEVICE_ID_ASUSTEK_I2C_TOUCHPAD	0x0101
>  #define USB_DEVICE_ID_ASUSTEK_ROG_KEYBOARD1 0x1854
> --
> 2.54.0

^ permalink raw reply

* [PATCH v3 2/2] Input: isa1200 - new driver for Imagis ISA1200
From: Svyatoslav Ryhel @ 2026-05-03 16:52 UTC (permalink / raw)
  To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Linus Walleij, Svyatoslav Ryhel
  Cc: linux-input, devicetree, linux-kernel
In-Reply-To: <20260503165243.215979-1-clamor95@gmail.com>

From: Linus Walleij <linusw@kernel.org>

The ISA1200 is a haptic feedback unit from Imagis Technology using two
motors for haptic feedback in mobile phones. Used in many mobile devices
c. 2012 including Samsung Galxy S Advance GT-I9070 (Janice), Samsung Beam
GT-I8350 (Gavini), LG Optimus 4X P880 and LG Optimus Vu P895.

The exact datasheet for the ISA1200 is not available; all data was modeled
based on available downstream kernel sources for various devices and
fragments of information scattered across the internet.

Tested-by: Linus Walleij <linusw@kernel.org> # GT-I9070 Janice
Signed-off-by: Linus Walleij <linusw@kernel.org>
Co-developed-by: Svyatoslav Ryhel <clamor95@gmail.com>
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 drivers/input/misc/Kconfig   |  11 +
 drivers/input/misc/Makefile  |   1 +
 drivers/input/misc/isa1200.c | 535 +++++++++++++++++++++++++++++++++++
 3 files changed, 547 insertions(+)
 create mode 100644 drivers/input/misc/isa1200.c

diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 94a753fcb64f..5e5d46f44b91 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -852,6 +852,17 @@ config INPUT_IQS7222
 	  To compile this driver as a module, choose M here: the
 	  module will be called iqs7222.
 
+config INPUT_ISA1200_HAPTIC
+	tristate "Imagis ISA1200 haptic feedback unit"
+	depends on I2C
+	select REGMAP_I2C
+	help
+	  Say Y to enable support for the Imagis ISA1200 haptic
+	  feedback unit.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called isa1200.
+
 config INPUT_CMA3000
 	tristate "VTI CMA3000 Tri-axis accelerometer"
 	help
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 415fc4e2918b..d62bf2e9d85f 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -49,6 +49,7 @@ obj-$(CONFIG_INPUT_IMS_PCU)		+= ims-pcu.o
 obj-$(CONFIG_INPUT_IQS269A)		+= iqs269a.o
 obj-$(CONFIG_INPUT_IQS626A)		+= iqs626a.o
 obj-$(CONFIG_INPUT_IQS7222)		+= iqs7222.o
+obj-$(CONFIG_INPUT_ISA1200_HAPTIC)	+= isa1200.o
 obj-$(CONFIG_INPUT_KEYSPAN_REMOTE)	+= keyspan_remote.o
 obj-$(CONFIG_INPUT_KXTJ9)		+= kxtj9.o
 obj-$(CONFIG_INPUT_M68K_BEEP)		+= m68kspkr.o
diff --git a/drivers/input/misc/isa1200.c b/drivers/input/misc/isa1200.c
new file mode 100644
index 000000000000..2c7b5ef0532c
--- /dev/null
+++ b/drivers/input/misc/isa1200.c
@@ -0,0 +1,535 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include <linux/array_size.h>
+#include <linux/bitmap.h>
+#include <linux/bits.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/devm-helpers.h>
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/pwm.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/units.h>
+
+/*
+ * System control (LDO regulator)
+ *
+ * LDO voltage to register mapping is linear, but it is split in two parts:
+ * 2.3V - 3.0V map to 0x08 - 0x0f; 3.1V - 3.8V map to 0x00 - 0x7
+ */
+
+#define ISA1200_SCTRL			0x00
+#define ISA1200_LDO_VOLTAGE_BASE	0x08
+#define ISA1200_LDO_VOLTAGE_STEP	100000
+#define ISA1200_LDO_VOLTAGE_2V3		23
+#define ISA1200_LDO_VOLTAGE_3V1		31
+
+/*
+ * The output frequency is calculated with this formula:
+ *
+ *                 base clock frequency
+ * fout = -----------------------------------------
+ *        (128 - PWM_FREQ) * 2 * PLLDIV * PWM_PERIOD
+ *
+ * The base clock frequency is the clock frequency provided on the
+ * clock input to the chip, divided by the value in HCTRL0
+ *
+ * PWM_FREQ is configured in register HCTRL4, it is common to set this
+ * to 0 to get only two variables to calculate.
+ *
+ * PLLDIV is configured in register HCTRL3 (bits 7..4, so 0..15)
+ * PWM_PERIOD is configured in register HCTRL6
+ * Further the duty cycle can be configured in HCTRL5
+ */
+
+/*
+ * HCTRL0 configures clock or PWM input and selects the divider for
+ * the clock input.
+ */
+#define ISA1200_HCTRL0			0x30
+#define ISA1200_HCTRL0_HAP_ENABLE	BIT(7)
+#define ISA1200_HCTRL0_PWM_GEN_MODE	BIT(4)
+#define ISA1200_HCTRL0_PWM_INPUT_MODE	BIT(3)
+#define ISA1200_HCTRL0_CLKDIV_128	128
+
+/*
+ * HCTRL1 configures the motor type and clock sourse
+ */
+#define ISA1200_HCTRL1			0x31
+#define ISA1200_HCTRL1_EXT_CLOCK	BIT(7)
+#define ISA1200_HCTRL1_DAC_INVERT	BIT(6)
+#define ISA1200_HCTRL1_MODE(n)		(((n) & 1) << 5)
+
+/* HCTRL2 controls software reset of the chip */
+#define ISA1200_HCTRL2			0x32
+#define ISA1200_HCTRL2_SW_RESET		BIT(0)
+
+/*
+ * HCTRL3 controls the PLL divisor
+ *
+ * Bits [0,1] are always set to 1 (we don't know what they are
+ * used for) and bit 4 and upward control the PLL divisor.
+ */
+#define ISA1200_HCTRL3			0x33
+#define ISA1200_HCTRL3_DEFAULT		0x03
+#define ISA1200_HCTRL3_PLLDIV(n)	(((n) & 0xf) << 4)
+
+/* HCTRL4 controls the PWM frequency of external channel */
+#define ISA1200_HCTRL4			0x34
+
+/* HCTRL5 controls the PWM high duty cycle of internal channel */
+#define ISA1200_HCTRL5			0x35
+
+/* HCTRL6 controls the PWM period of internal channel */
+#define ISA1200_HCTRL6			0x36
+#define ISA1200_HCTRL6_PERIOD_SCALE	100
+
+/* The use for these registers is unknown but they exist */
+#define ISA1200_HCTRL7			0x37
+#define ISA1200_HCTRL8			0x38
+#define ISA1200_HCTRL9			0x39
+#define ISA1200_HCTRLA			0x3a
+#define ISA1200_HCTRLB			0x3b
+#define ISA1200_HCTRLC			0x3c
+#define ISA1200_HCTRLD			0x3d
+
+#define ISA1200_EN_PINS_MAX		2
+
+struct isa1200_config {
+	u32 ldo_voltage;
+	u32 mode;
+	u32 clkdiv;
+	u32 plldiv;
+	u32 freq;
+	u32 period;
+	u32 duty;
+};
+
+struct isa1200 {
+	struct input_dev *input;
+	struct regmap *map;
+
+	struct clk *clk;
+	struct pwm_device *pwm;
+	struct gpio_descs *enable_gpios;
+
+	struct work_struct play_work;
+	struct isa1200_config config;
+
+	int level;
+	bool clk_on;
+};
+
+static const struct regmap_config isa1200_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = ISA1200_HCTRLD,
+};
+
+static void isa1200_start(struct isa1200 *isa)
+{
+	struct isa1200_config *config = &isa->config;
+	struct pwm_state state;
+	u8 hctrl0 = 0, hctrl1 = 0;
+	DECLARE_BITMAP(values, ISA1200_EN_PINS_MAX);
+	int ret;
+
+	if (!isa->clk_on) {
+		ret = clk_prepare_enable(isa->clk);
+		if (ret < 0)
+			return;
+
+		isa->clk_on = true;
+	}
+
+	bitmap_fill(values, ISA1200_EN_PINS_MAX);
+	gpiod_multi_set_value_cansleep(isa->enable_gpios, values);
+
+	usleep_range(200, 300);
+
+	regmap_write(isa->map, ISA1200_SCTRL, config->ldo_voltage);
+
+	if (isa->clk) {
+		hctrl0 = ISA1200_HCTRL0_PWM_GEN_MODE;
+		hctrl1 = ISA1200_HCTRL1_EXT_CLOCK;
+	}
+
+	if (isa->pwm) {
+		hctrl0 = ISA1200_HCTRL0_PWM_INPUT_MODE;
+		hctrl1 = 0;
+	}
+
+	hctrl0 |= __ffs(config->clkdiv / ISA1200_HCTRL0_CLKDIV_128);
+	hctrl1 |= ISA1200_HCTRL1_DAC_INVERT;
+	hctrl1 |= ISA1200_HCTRL1_MODE(config->mode);
+
+	regmap_write(isa->map, ISA1200_HCTRL0, hctrl0);
+	regmap_write(isa->map, ISA1200_HCTRL1, hctrl1);
+
+	/* Make sure to de-assert software reset */
+	regmap_write(isa->map, ISA1200_HCTRL2, 0x00);
+
+	/* PLL divisor */
+	regmap_write(isa->map, ISA1200_HCTRL3,
+		     ISA1200_HCTRL3_PLLDIV(config->plldiv) |
+		     ISA1200_HCTRL3_DEFAULT);
+
+	/* Frequency */
+	regmap_write(isa->map, ISA1200_HCTRL4, config->freq);
+	/* Duty cycle */
+	regmap_write(isa->map, ISA1200_HCTRL5, config->period >> 1);
+	/* Period */
+	regmap_write(isa->map, ISA1200_HCTRL6, config->period);
+
+	hctrl0 |= ISA1200_HCTRL0_HAP_ENABLE;
+	regmap_write(isa->map, ISA1200_HCTRL0, hctrl0);
+
+	if (isa->clk)
+		regmap_write(isa->map, ISA1200_HCTRL5, config->duty);
+
+	if (isa->pwm) {
+		pwm_get_state(isa->pwm, &state);
+		state.duty_cycle = config->duty;
+		state.enabled = true;
+		pwm_apply_might_sleep(isa->pwm, &state);
+	}
+}
+
+static void isa1200_power_off(void *data)
+{
+	struct isa1200 *isa = data;
+
+	DECLARE_BITMAP(values, ISA1200_EN_PINS_MAX);
+
+	bitmap_zero(values, ISA1200_EN_PINS_MAX);
+	gpiod_multi_set_value_cansleep(isa->enable_gpios, values);
+
+	if (isa->clk_on)
+		clk_disable_unprepare(isa->clk);
+}
+
+static void isa1200_stop(struct isa1200 *isa)
+{
+	struct pwm_state state;
+
+	if (isa->pwm) {
+		pwm_get_state(isa->pwm, &state);
+		state.duty_cycle = 0;
+		state.enabled = false;
+		pwm_apply_might_sleep(isa->pwm, &state);
+	}
+
+	regmap_write(isa->map, ISA1200_HCTRL0, 0x00);
+	isa1200_power_off(isa);
+}
+
+static void isa1200_play_work(struct work_struct *work)
+{
+	struct isa1200 *isa =
+		container_of(work, struct isa1200, play_work);
+
+	if (isa->level)
+		isa1200_start(isa);
+	else
+		isa1200_stop(isa);
+}
+
+static int isa1200_vibrator_play_effect(struct input_dev *input, void *data,
+					struct ff_effect *effect)
+{
+	struct isa1200 *isa = input_get_drvdata(input);
+	int level;
+
+	/*
+	 * TODO: we currently only support rumble.
+	 * The ISA1200 can control two motors and some devices
+	 * also have two motors mounted.
+	 */
+	level = effect->u.rumble.strong_magnitude;
+	if (!level)
+		level = effect->u.rumble.weak_magnitude;
+
+	dev_dbg(&input->dev, "FF effect type %d level %d\n",
+		effect->type, level);
+
+	if (isa->level != level) {
+		isa->level = level;
+		schedule_work(&isa->play_work);
+	}
+
+	return 0;
+}
+
+static void isa1200_vibrator_close(struct input_dev *input)
+{
+	struct isa1200 *isa = input_get_drvdata(input);
+
+	cancel_work_sync(&isa->play_work);
+	isa1200_stop(isa);
+	isa->level = 0;
+}
+
+static int isa1200_of_probe(struct i2c_client *client)
+{
+	static const char * const isa1200_supplies[] = { "vdd", "vddp" };
+	struct isa1200 *isa = i2c_get_clientdata(client);
+	struct isa1200_config *config = &isa->config;
+	struct device *dev = &client->dev;
+	struct fwnode_handle *ldo_node;
+	int ret;
+
+	isa->clk = devm_clk_get_optional(dev, NULL);
+	if (IS_ERR(isa->clk))
+		return dev_err_probe(dev, PTR_ERR(isa->clk),
+				     "failed to get clock\n");
+
+	isa->pwm = devm_pwm_get(dev, NULL);
+	if (IS_ERR(isa->pwm)) {
+		ret = PTR_ERR(isa->pwm);
+		if (ret == -ENODEV || ret == -EINVAL)
+			isa->pwm = NULL;
+		else
+			return dev_err_probe(dev, ret, "getting PWM\n");
+	}
+
+	if (!isa->clk && !isa->pwm)
+		return dev_err_probe(dev, -EINVAL,
+				     "clock or PWM are required, none were provided\n");
+
+	ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(isa1200_supplies),
+					     isa1200_supplies);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to set up supplies\n");
+
+	isa->enable_gpios = devm_gpiod_get_array_optional(dev, "control",
+							  GPIOD_OUT_LOW);
+	if (IS_ERR(isa->enable_gpios))
+		return dev_err_probe(dev, PTR_ERR(isa->enable_gpios),
+				     "failed to get enable gpios\n");
+
+	ldo_node = device_get_named_child_node(dev, "ldo");
+	if (!ldo_node)
+		return dev_err_probe(dev, -ENODEV,
+				     "failed to get embedded LDO node\n");
+
+	config->ldo_voltage = 2300000;
+	ret = fwnode_property_read_u32(ldo_node, "regulator-min-microvolt",
+				       &config->ldo_voltage);
+	fwnode_handle_put(ldo_node);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "failed to get ldo voltage\n");
+
+	/* Device supports only 2.3V - 3.8V range */
+	if (config->ldo_voltage < 2300000 || config->ldo_voltage > 3800000)
+		return dev_err_probe(dev, -EINVAL,
+				     "voltage is out of 2.3V - 3.8V range\n");
+
+	config->ldo_voltage /= ISA1200_LDO_VOLTAGE_STEP;
+	if (config->ldo_voltage < ISA1200_LDO_VOLTAGE_3V1)
+		config->ldo_voltage = config->ldo_voltage -
+				      ISA1200_LDO_VOLTAGE_2V3 +
+				      ISA1200_LDO_VOLTAGE_BASE;
+	else
+		config->ldo_voltage -= ISA1200_LDO_VOLTAGE_3V1;
+
+	config->mode = 0; /* LRA_MODE */
+	device_property_read_u32(dev, "imagis,mode", &config->mode);
+
+	config->clkdiv = ISA1200_HCTRL0_CLKDIV_128;
+	device_property_read_u32(dev, "imagis,clk-div", &config->clkdiv);
+	if (!config->clkdiv)
+		return dev_err_probe(dev, -EINVAL, "clk-div cannot be zero\n");
+
+	config->clkdiv = clamp(config->clkdiv, ISA1200_HCTRL0_CLKDIV_128,
+			       ISA1200_HCTRL0_CLKDIV_128 << 3);
+
+	device_property_read_u32(dev, "imagis,pll-div", &config->plldiv);
+	if (ret < 0 || !config->clkdiv)
+		config->plldiv = 1;
+
+	config->period = 0;
+	config->freq = 0;
+	config->duty = 0;
+
+	if (isa->clk) {
+		ret = device_property_read_u32(dev, "imagis,period-ns",
+					       &config->period);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "failed to get period\n");
+
+		/*
+		 * TODO: The scale value is arbitrary, but it fits observations
+		 * quite well, and the exact conversion method is unknown.
+		 * The period property value returned above is the HCTRL6
+		 * register value set by the vendor code, multiplied by 100.
+		 */
+		config->period /= ISA1200_HCTRL6_PERIOD_SCALE;
+		config->duty = config->period >> 1;
+	}
+
+	if (isa->pwm) {
+		struct pwm_state state;
+
+		pwm_init_state(isa->pwm, &state);
+
+		if (!state.period)
+			return dev_err_probe(dev, -EINVAL,
+					     "PWM period cannot be zero\n");
+
+		config->freq = div64_u64(NANO, state.period * config->clkdiv);
+		config->duty = state.period >> 1;
+
+		ret = pwm_apply_might_sleep(isa->pwm, &state);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "failed to apply initial PWM state\n");
+	}
+
+	/*
+	 * TODO: If device is using a clock, this property should return the
+	 * value written to the HCTRL5 register by downstrem code. It likely
+	 * needs to be converted into a meaningful duty cycle value, though
+	 * unfortunately the exact conversion mechanism is unknown. If the
+	 * device uses PWM, this property will return the correct duty cycle
+	 * in nanoseconds.
+	 */
+	device_property_read_u32(dev, "imagis,duty-cycle-ns", &config->duty);
+
+	return 0;
+}
+
+static int isa1200_probe(struct i2c_client *client)
+{
+	struct isa1200 *isa;
+	struct device *dev = &client->dev;
+	DECLARE_BITMAP(values, ISA1200_EN_PINS_MAX);
+	u32 val;
+	int ret;
+
+	isa = devm_kzalloc(dev, sizeof(*isa), GFP_KERNEL);
+	if (!isa)
+		return -ENOMEM;
+
+	isa->input = devm_input_allocate_device(dev);
+	if (!isa->input)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, isa);
+
+	ret = isa1200_of_probe(client);
+	if (ret)
+		return ret;
+
+	isa->map = devm_regmap_init_i2c(client, &isa1200_regmap_config);
+	if (IS_ERR(isa->map))
+		return dev_err_probe(dev, PTR_ERR(isa->map),
+				     "failed to initialize register map\n");
+
+	ret = clk_prepare_enable(isa->clk);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "failed to enable clock\n");
+
+	isa->clk_on = true;
+
+	bitmap_fill(values, ISA1200_EN_PINS_MAX);
+	gpiod_multi_set_value_cansleep(isa->enable_gpios, values);
+
+	ret = devm_add_action_or_reset(dev, isa1200_power_off, isa);
+	if (ret)
+		return ret;
+
+	usleep_range(200, 300);
+
+	/* Read a register so we know that regmap and I2C transport works */
+	ret = regmap_read(isa->map, ISA1200_SCTRL, &val);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to read SCTRL\n");
+
+	ret = devm_work_autocancel(dev, &isa->play_work, isa1200_play_work);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to init work\n");
+
+	isa->input->name = "isa1200-haptic";
+	isa->input->id.bustype = BUS_HOST;
+	isa->input->dev.parent = dev;
+	isa->input->close = isa1200_vibrator_close;
+
+	input_set_drvdata(isa->input, isa);
+
+	/* TODO: this hardware can likely support more than rumble */
+	input_set_capability(isa->input, EV_FF, FF_RUMBLE);
+
+	ret = input_ff_create_memless(isa->input, NULL,
+				      isa1200_vibrator_play_effect);
+	if (ret)
+		return dev_err_probe(dev, ret, "couldn't create FF dev\n");
+
+	ret = input_register_device(isa->input);
+	if (ret)
+		return dev_err_probe(dev, ret, "couldn't register input dev\n");
+
+	return ret;
+}
+
+static int isa1200_suspend(struct device *dev)
+{
+	struct isa1200 *isa = dev_get_drvdata(dev);
+	struct input_dev *input_dev = isa->input;
+
+	guard(mutex)(&input_dev->mutex);
+
+	if (input_device_enabled(input_dev)) {
+		cancel_work_sync(&isa->play_work);
+		if (isa->level)
+			isa1200_stop(isa);
+	}
+
+	return 0;
+}
+
+static int isa1200_resume(struct device *dev)
+{
+	struct isa1200 *isa = dev_get_drvdata(dev);
+	struct input_dev *input_dev = isa->input;
+
+	guard(mutex)(&input_dev->mutex);
+
+	if (input_device_enabled(input_dev))
+		if (isa->level)
+			isa1200_start(isa);
+
+	return 0;
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(isa1200_pm_ops, isa1200_suspend, isa1200_resume);
+
+static const struct of_device_id isa1200_of_match[] = {
+	{ .compatible = "imagis,isa1200" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, isa1200_of_match);
+
+static struct i2c_driver isa1200_i2c_driver = {
+	.driver = {
+		.name = "isa1200",
+		.of_match_table = isa1200_of_match,
+		.pm = pm_sleep_ptr(&isa1200_pm_ops),
+	},
+	.probe = isa1200_probe,
+};
+module_i2c_driver(isa1200_i2c_driver);
+
+MODULE_AUTHOR("Linus Walleij <linusw@kernel.org>");
+MODULE_AUTHOR("Svyatoslav Ryhel <clamor95@gmail.com>");
+MODULE_DESCRIPTION("Imagis ISA1200 haptic feedback unit");
+MODULE_LICENSE("GPL");
-- 
2.51.0


^ permalink raw reply related

* [PATCH v3 1/2] dt-bindings: input: Document Imagis ISA1200 haptic motor driver
From: Svyatoslav Ryhel @ 2026-05-03 16:52 UTC (permalink / raw)
  To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Linus Walleij, Svyatoslav Ryhel
  Cc: linux-input, devicetree, linux-kernel
In-Reply-To: <20260503165243.215979-1-clamor95@gmail.com>

Document the Imagis ISA1200 haptic motor driver, used primarily in mobile
handheld devices and capable of supporting up to two motors.

The exact datasheet for the ISA1200 is not available; all data was modeled
based on available downstream kernel sources for various devices and
fragments of information scattered across the internet.

Tested-by: Linus Walleij <linusw@kernel.org> # Samsung GT-I9070 Janice
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 .../bindings/input/imagis,isa1200.yaml        | 140 ++++++++++++++++++
 1 file changed, 140 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/imagis,isa1200.yaml

diff --git a/Documentation/devicetree/bindings/input/imagis,isa1200.yaml b/Documentation/devicetree/bindings/input/imagis,isa1200.yaml
new file mode 100644
index 000000000000..bbe6f99d39c1
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/imagis,isa1200.yaml
@@ -0,0 +1,140 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/imagis,isa1200.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Imagis ISA1200 haptic motor driver
+
+maintainers:
+  - Svyatoslav Ryhel <clamor95@gmail.com>
+  - Linus Walleij <linusw@kernel.org>
+
+description:
+  The ISA1200 is a high-performance enhanced haptic motor driver designed
+  for mobile hand-held devices. It supports various voltages for both ERM
+  (Eccentric Rotating Mass) and LRA (Linear Resonant Actuator) type
+  actuators. Thanks to an embedded LDO, battery power can be used directly
+  in handheld applications.
+
+properties:
+  compatible:
+    const: imagis,isa1200
+
+  reg:
+    maxItems: 1
+
+  control-gpios:
+    description:
+      One or two GPIOs flagged as active high linked to HEN and LEN pins
+    maxItems: 2
+
+  clocks:
+    maxItems: 1
+
+  pwms:
+    maxItems: 1
+
+  vdd-supply:
+    description:
+      Regulator for 2.4V - 5.5V power supply
+
+  vddp-supply:
+    description:
+      Regulator for 2.4V - 3.6V IO power supply
+
+  imagis,clk-div:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Divider for the external input clock/PWM
+    enum: [128, 256, 512, 1024]
+    default: 128
+
+  imagis,pll-div:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Divider for the internal PLL clock
+    minimum: 1
+    maximum: 15
+    default: 1
+
+  imagis,mode:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      Defines the motor type isa1200 drives
+      0 - LRA (Linear Resonant Actuator)
+      1 - ERM (Eccentric Rotating Mass)
+    enum: [0, 1]
+    default: 0
+
+  imagis,period-ns:
+    description:
+      Period of the internal PWM channel in nanoseconds.
+    minimum: 10000
+    maximum: 30000
+
+  imagis,duty-cycle-ns:
+    description:
+      Duty cycle of the external/internal PWM channel in nanoseconds,
+      defaults to 50% of the channel's period
+
+  ldo:
+    $ref: /schemas/regulator/regulator.yaml#
+    type: object
+    description:
+      Embedded LDO regulator with voltage range 2.3V - 3.8V
+    unevaluatedProperties: false
+
+    required:
+      - regulator-min-microvolt
+      - regulator-max-microvolt
+
+required:
+  - compatible
+  - reg
+  - ldo
+
+anyOf:
+  - required:
+      - clocks
+      - imagis,period-ns
+  - required:
+      - pwms
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        haptic-engine@49 {
+            compatible = "imagis,isa1200";
+            reg = <0x49>;
+
+            clocks = <&isa1200_refclk>;
+
+            control-gpios = <&gpio 22 GPIO_ACTIVE_HIGH>,
+                            <&gpio 23 GPIO_ACTIVE_HIGH>;
+
+            vdd-supply = <&vdd_3v3_vbat>;
+            vddp-supply = <&vdd_2v8_vvib>;
+
+            imagis,clk-div = <256>;
+            imagis,pll-div = <2>;
+
+            imagis,mode = <0>; /* LRA_MODE */
+
+            imagis,period-ns = <13400>;
+            imagis,duty-cycle-ns = <100>;
+
+            ldo {
+                regulator-name = "vdd_vib";
+                regulator-min-microvolt = <2300000>;
+                regulator-max-microvolt = <2300000>;
+            };
+        };
+    };
-- 
2.51.0


^ permalink raw reply related

* [PATCH v3 0/2] input: misc: add support for Imagis ISA1200 haptic motor driver
From: Svyatoslav Ryhel @ 2026-05-03 16:52 UTC (permalink / raw)
  To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Linus Walleij, Svyatoslav Ryhel
  Cc: linux-input, devicetree, linux-kernel

The ISA1200 is a haptic feedback unit from Imagis Technology using two
motors for haptic feedback in mobile phones. Used in many mobile devices
c. 2012 including Samsung Galxy S Advance GT-I9070 (Janice), Samsung Beam
GT-I8350 (Gavini), LG Optimus 4X P880 and LG Optimus Vu P895.

The exact datasheet for the ISA1200 is not available; all data was modeled
based on available downstream kernel sources for various devices and
fragments of information scattered across the internet.

---
Changes in v3:
- added clock state tracking
- dropped level check in vibrator close
- added clkdiv clamping
- added comments regarding registers 5 and 6

Changes in v2:
- imagis,clk-div switched to accept actual divider value
- dropped DT header
- adjusted imagis,period-ns range
- initiated hctrl0 and hctrl1 values in isa1200_start
- fixed situation when PWM might return -EPROBE_DEFER to be
  treated properly
- added chech a clock or PWM is available
- fixed regulator voltages check being off by 10
- added chech if state.period is not zero
- added action call to disable clock and gpios on error
- used managed version of work init
- added work cancel on suspend
- PW calls are done under mutex lock
---

Linus Walleij (1):
  Input: isa1200 - new driver for Imagis ISA1200

Svyatoslav Ryhel (1):
  dt-bindings: input: Document Imagis ISA1200 haptic motor driver

 .../bindings/input/imagis,isa1200.yaml        | 140 +++++
 drivers/input/misc/Kconfig                    |  11 +
 drivers/input/misc/Makefile                   |   1 +
 drivers/input/misc/isa1200.c                  | 535 ++++++++++++++++++
 4 files changed, 687 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/imagis,isa1200.yaml
 create mode 100644 drivers/input/misc/isa1200.c

-- 
2.51.0


^ permalink raw reply

* [PATCH 6/6] leds: led-class: mark classdev as unregistering early
From: James Ye @ 2026-05-03  7:26 UTC (permalink / raw)
  To: jikos, bentiss, lee, pavel
  Cc: linux-input, linux-leds, linux-kernel, denis.benato, James Ye
In-Reply-To: <20260503072643.2774762-1-jye836@gmail.com>

The classdev was marked as unregistering after disabling the trigger.
Disabling the trigger attempts to turn the LED off, and the device may
already be inaccessible. This generates a "Setting an LED's brightness
failed" log, which is avoided by changing this order.

Signed-off-by: James Ye <jye836@gmail.com>
---
 drivers/leds/led-class.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 9e14ae588f78..a00986ffd9f6 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -601,6 +601,8 @@ void led_classdev_unregister(struct led_classdev *led_cdev)
 	if (IS_ERR_OR_NULL(led_cdev->dev))
 		return;
 
+	led_cdev->flags |= LED_UNREGISTERING;
+
 #ifdef CONFIG_LEDS_TRIGGERS
 	down_write(&led_cdev->trigger_lock);
 	if (led_cdev->trigger)
@@ -608,8 +610,6 @@ void led_classdev_unregister(struct led_classdev *led_cdev)
 	up_write(&led_cdev->trigger_lock);
 #endif
 
-	led_cdev->flags |= LED_UNREGISTERING;
-
 	/* Stop blinking */
 	led_stop_software_blink(led_cdev);
 
-- 
2.54.0


^ permalink raw reply related

* [PATCH 5/6] HID: asus: add microphone mute LED support for T3304
From: James Ye @ 2026-05-03  7:26 UTC (permalink / raw)
  To: jikos, bentiss, lee, pavel
  Cc: linux-input, linux-leds, linux-kernel, denis.benato, James Ye
In-Reply-To: <20260503072643.2774762-1-jye836@gmail.com>

T3304 keyboard has a microphone mute LED indicator on the same key
(Fn+F9). Look this up with a led_classdev.

It does not have backlight LEDs, so return without failure from
asus_kbd_register_leds if backlight support is not present.

Signed-off-by: James Ye <jye836@gmail.com>
---
 drivers/hid/hid-asus.c | 51 +++++++++++++++++++++++++++++++++++++++---
 1 file changed, 48 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index e4c97fddfaf1..4f68bc40b8bf 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -100,6 +100,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
 #define QUIRK_ROG_ALLY_XPAD		BIT(13)
 #define QUIRK_HID_FN_LOCK		BIT(14)
 #define QUIRK_T3304_KEYBOARD		BIT(15)
+#define QUIRK_HID_MICMUTE		BIT(16)
 
 #define I2C_KEYBOARD_QUIRKS			(QUIRK_FIX_NOTEBOOK_REPORT | \
 						 QUIRK_NO_INIT_REPORTS | \
@@ -144,6 +145,7 @@ struct asus_drvdata {
 	unsigned long battery_next_query;
 	struct work_struct fn_lock_sync_work;
 	bool fn_lock;
+	struct led_classdev led_micmute;
 };
 
 static int asus_report_battery(struct asus_drvdata *, u8 *, int);
@@ -578,6 +580,26 @@ static int asus_kbd_disable_oobe(struct hid_device *hdev)
 	return 0;
 }
 
+static int asus_kbd_set_micmute_led(struct hid_device *hdev, bool enabled)
+{
+	u8 buf[] = { FEATURE_KBD_REPORT_ID, 0xd0, 0x7c, enabled };
+
+	return asus_kbd_set_report(hdev, buf, sizeof(buf));
+}
+
+static int asus_led_brightness_set(struct led_classdev *led_cdev,
+		enum led_brightness value)
+{
+	struct device *dev = led_cdev->dev->parent;
+	struct hid_device *hdev = to_hid_device(dev);
+	struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+
+	if (led_cdev == &drvdata->led_micmute)
+		return asus_kbd_set_micmute_led(hdev, !!value);
+
+	return -ENODEV;
+}
+
 static int asus_kbd_set_fn_lock(struct hid_device *hdev, bool enabled)
 {
 	u8 buf[] = { FEATURE_KBD_REPORT_ID, 0xd0, 0x4e, !!enabled };
@@ -752,9 +774,31 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
 	if (ret < 0)
 		return ret;
 
+	if (drvdata->quirks & QUIRK_HID_MICMUTE) {
+		char *name_micmute;
+		size_t name_sz = strlen(dev_name(&hdev->dev)) + 16;
+
+		name_micmute = devm_kzalloc(&hdev->dev, name_sz, GFP_KERNEL);
+		if (name_micmute == NULL)
+			return -ENOMEM;
+
+		snprintf(name_micmute, name_sz, "%s::micmute", dev_name(&hdev->dev));
+		drvdata->led_micmute.name = name_micmute;
+		drvdata->led_micmute.default_trigger = "audio-micmute";
+		drvdata->led_micmute.brightness_set_blocking = asus_led_brightness_set;
+		drvdata->led_micmute.max_brightness = 1;
+		drvdata->led_micmute.flags = LED_CORE_SUSPENDRESUME | LED_HW_PLUGGABLE;
+		drvdata->led_micmute.dev = &hdev->dev;
+		ret = devm_led_classdev_register(&hdev->dev, &drvdata->led_micmute);
+		if (ret < 0) {
+			hid_err(hdev, "Failed to register LED: %d.\n", ret);
+			return ret;
+		}
+	}
+
 	/* Check for backlight support */
 	if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
-		return -ENODEV;
+		return 0;
 
 	if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
 		ret = asus_kbd_disable_oobe(hdev);
@@ -1315,7 +1359,8 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	}
 
 	/* Laptops keyboard backlight is always at 0x5a */
-	if (is_vendor && (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT) &&
+	if (is_vendor &&
+	    (drvdata->quirks & (QUIRK_USE_KBD_BACKLIGHT | QUIRK_HID_MICMUTE)) &&
 	    (asus_has_report_id(hdev, FEATURE_KBD_REPORT_ID)) &&
 		(asus_kbd_register_leds(hdev)))
 		hid_warn(hdev, "Failed to initialize backlight.\n");
@@ -1587,7 +1632,7 @@ static const struct hid_device_id asus_devices[] = {
 		USB_VENDOR_ID_ASUSTEK, USB_DEVICE_ID_ASUSTEK_T101HA_KEYBOARD) },
 	{ HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
 		USB_VENDOR_ID_ASUSTEK, USB_DEVICE_ID_ASUSTEK_T3304_KEYBOARD),
-	  QUIRK_T3304_KEYBOARD },
+	  QUIRK_T3304_KEYBOARD | QUIRK_HID_MICMUTE },
 	{ }
 };
 MODULE_DEVICE_TABLE(hid, asus_devices);
-- 
2.54.0


^ permalink raw reply related

* [PATCH 4/6] HID: multitouch: add support for ASUS T3304 media keys
From: James Ye @ 2026-05-03  7:26 UTC (permalink / raw)
  To: jikos, bentiss, lee, pavel
  Cc: linux-input, linux-leds, linux-kernel, denis.benato, James Ye
In-Reply-To: <20260503072643.2774762-1-jye836@gmail.com>

Touchpad functionality already works with hid-multitouch, but media key
events are emitted from the touchpad interface. Add MT_CLS_ASUS to
handle these, and also add missing key mappings.

Signed-off-by: James Ye <jye836@gmail.com>
---
 drivers/hid/hid-multitouch.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index e82a3c4e5b44..a49930496c5b 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -1510,6 +1510,10 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 		case 0x35: mt_map_key_clear(KEY_DISPLAY_OFF);		break;
 		case 0x6b: mt_map_key_clear(KEY_F21);			break;
 		case 0x6c: mt_map_key_clear(KEY_SLEEP);			break;
+		case 0x7c: mt_map_key_clear(KEY_MICMUTE);		break;
+		case 0x4e: mt_map_key_clear(KEY_FN_ESC);		break;
+
+		case 0x86: mt_map_key_clear(KEY_PROG1);		break; /* MyASUS key */
 		default:
 			return -1;
 		}
@@ -2145,6 +2149,12 @@ static const struct hid_device_id mt_devices[] = {
 			USB_VENDOR_ID_ASUSTEK,
 			USB_DEVICE_ID_ASUSTEK_T304_KEYBOARD) },
 
+	/* Asus T3304 */
+	{ .driver_data = MT_CLS_ASUS,
+		HID_DEVICE(BUS_USB, HID_GROUP_MULTITOUCH_WIN_8,
+			USB_VENDOR_ID_ASUSTEK,
+			USB_DEVICE_ID_ASUSTEK_T3304_KEYBOARD) },
+
 	/* Atmel panels */
 	{ .driver_data = MT_CLS_SERIAL,
 		MT_USB_DEVICE(USB_VENDOR_ID_ATMEL,
-- 
2.54.0


^ permalink raw reply related


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