[PATCH v2 0/9] HID: Add hid-plugin framework & convert haptic into a plugin

[PATCH v2 0/9] HID: Add hid-plugin framework & convert haptic into a plugin

[Rong Zhang]

Recently I am working on adding HID LampArray support for hid-input [1].
I would like it to be a loadable plugin module. While looking through
LKML, I found hid-haptic, which is a plugin-like support code collection
that must be built into vmlinux. Therefore, I decided to develop a
generic framework for loadable plugin modules, so that it can be used by
multiple drivers.

This series aims to introduce the said framework and convert hid-haptic
into the first plugin, as it's easy to convert and perfectly
demonstrates how the framework works.

The first five patches fix pre-existing bugs in hid-haptic and
hid-multitouch that may become more frigile once the former is
converted. The last four patches refactor hid-haptic, implement the
hid-plugin framework and finish the conversion.

The plugin framework is designed for devices with additional HID usage
pages which enhance their capabilities. The support code for these usage
pages is called "HID plugin".

The corresponding HID driver calls functions provided by the plugin to
take the advantage of such enhancements. Without the framework, the HID
driver must have a linkage dependence on the plugin, which is not
optimal because the plugin is always loaded into memory even if no
device actually supports it.

Logically, the dependence should be reversed, i.e., the plugin should be
an optional component of the HID driver, which is only loaded when the
driver finds that the device needs it.

Introduce hid-plugin framework for loadable plugin modules to achieve
the said logical dependence. Each plugin has a unique hid_plugin_id when
enabled, so that HID drivers can request it accordingly. The framework
aims to be lightweight. It loads the plugin module when needed. Once
loaded (or if the plugin is builtin), requesting the plugin is just an
RCU dereference plusing an module refcount increment.

When a plugin is disabled, its id maps to HID_PLUGIN_DISABLED, and calls
to the plugin helper functions are eliminated by the compiler.

With it, hid-haptic can be easily converted into a loadable plugin
module, so that hid-multitouch only requests it on demand.

Thanks Benjamin Tissoires for the inspiration for the implementation.

[1]: I know Texudo folks are working on adding hid-lamparray to connect
it to hid-generic. I am not in favor of their design, so I decided to
write a new one. In my local tree, I've had an independent
implementation with HIDRAW sniff, autonomous mode as an LED trigger,
RGB/non-RGB LampArray support, per-key LED device support, etc. I will
submit it once this series settles.

Suggested-by: Benjamin Tissoires <bentiss@kernel.org>
Link: https://lore.kernel.org/linux-input/auypydfkhx2eg7vp764way4batdilzc35inqda3exwzs3tk3ff@oagat6g46zto/
Signed-off-by: Rong Zhang <i@rong.moe>
---
Changes in v2:
- Reject touchpad applications with non-force units (thanks Sashiko)
- Only call runtime methods if the event source is the same input device
  as the stored one (ditto)
- Rebase on top of v7.4-rc2 due to the devm_kzalloc_obj() changes
- Link to v1: https://patch.msgid.link/20261102-hid-haptic-kmod-v1-0-a3de5626dade@rong.moe

---
Rong Zhang (9):
      HID: haptic: Fix use-after-free when draining workqueue
      HID: haptic: Get rid of input_ff_destroy() in error paths
      HID: haptic, multitouch: Check haptic prerequisite properly
      HID: haptic, multitouch: Fix haptic device double-free and unnecessary allocation
      HID: haptic: Postpone GET_REPORT to init
      HID: haptic: Convert simple methods into static inline ones
      HID: Add hid-plugin framework for loadable plugin modules
      HID: haptic: Convert into a loadable plugin module
      HID: haptic: Remove stubs in favor of inlined dead code elimination

 MAINTAINERS                  |   7 ++
 drivers/hid/Kconfig          |  21 +++-
 drivers/hid/Makefile         |   4 +-
 drivers/hid/hid-haptic.c     | 206 ++++++++++----------------------------
 drivers/hid/hid-haptic.h     | 233 +++++++++++++++++++++++++++++++------------
 drivers/hid/hid-multitouch.c |  75 ++++++++------
 drivers/hid/hid-plugin.c     | 108 ++++++++++++++++++++
 drivers/hid/hid-plugin.h     | 145 +++++++++++++++++++++++++++
 8 files changed, 552 insertions(+), 247 deletions(-)
---
base-commit: 7475ea8b58616b33c04afb976cf0fbaad9dd63fb
change-id: 9644469a-hid-haptic-kmod-4e8e420f8e5f

Thanks,
Rong

[PATCH v2 1/9] HID: haptic: Fix use-after-free when draining workqueue

[Rong Zhang]

The haptic->wq is destroyed after freeing effects and their report
buffers. This is too late. If any work is still pending or running
before the workqueue destruction, haptic_work_handler() will access the
just-freed contents, causing a use-after-free. If put_device() happens
to decrement the reference count to zero, it may access freed devres-
managed resources, too.

Fix it by destroying the workqueue before putting the HID device and
freeing haptic resources. The allocation sequence is also adjusted to
the reverse.

Since the workqueue allocation is anyway touched, also replace the
legacy creation API with the modern alloc_ordered_workqueue(). In this
manner, the temporary buffer on heap can be replaced by the API's
formatting.

Also call __clear_bit(EV_FF, dev->evbit) in the bail-out path to match
the behavior of input_ff_destroy().

Fixes: 344ff3584957 ("HID: haptic: initialize haptic device")
Signed-off-by: Rong Zhang <i@rong.moe>
---
 drivers/hid/hid-haptic.c | 36 ++++++++++++++++--------------------
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/drivers/hid/hid-haptic.c b/drivers/hid/hid-haptic.c
index b7fadfbddc4c..73446a123b65 100644
--- a/drivers/hid/hid-haptic.c
+++ b/drivers/hid/hid-haptic.c
@@ -383,6 +383,9 @@ static void hid_haptic_destroy(struct ff_device *ff)
 	struct hid_device *hdev = haptic->hdev;
 	int r;
 
+	destroy_workqueue(haptic->wq);
+	haptic->wq = NULL;
+
 	if (hdev)
 		put_device(&hdev->dev);
 
@@ -396,9 +399,6 @@ static void hid_haptic_destroy(struct ff_device *ff)
 	}
 	haptic->effect = NULL;
 
-	destroy_workqueue(haptic->wq);
-	haptic->wq = NULL;
-
 	kfree(haptic->duration_map);
 	haptic->duration_map = NULL;
 
@@ -417,8 +417,6 @@ int hid_haptic_init(struct hid_device *hdev,
 	struct ff_haptic_effect stop_effect = {
 		.hid_usage = HID_HP_WAVEFORMSTOP & HID_USAGE,
 	};
-	const char *prefix = "hid-haptic";
-	char *name;
 	int (*flush)(struct input_dev *dev, struct file *file);
 	int (*event)(struct input_dev *dev, unsigned int type, unsigned int code, int value);
 
@@ -455,20 +453,11 @@ int hid_haptic_init(struct hid_device *hdev,
 	haptic->manual_trigger_report_len =
 		hid_report_len(haptic->manual_trigger_report);
 	mutex_init(&haptic->manual_trigger_mutex);
-	name = kmalloc(strlen(prefix) + strlen(hdev->name) + 2, GFP_KERNEL);
-	if (name) {
-		sprintf(name, "%s %s", prefix, hdev->name);
-		haptic->wq = create_singlethread_workqueue(name);
-		kfree(name);
-	}
-	if (!haptic->wq) {
-		ret = -ENOMEM;
-		goto duration_map;
-	}
+
 	haptic->effect = kzalloc_objs(struct hid_haptic_effect, FF_MAX_EFFECTS);
 	if (!haptic->effect) {
 		ret = -ENOMEM;
-		goto output_queue;
+		goto duration_map;
 	}
 	for (r = 0; r < FF_MAX_EFFECTS; r++) {
 		haptic->effect[r].report_buf =
@@ -497,6 +486,12 @@ int hid_haptic_init(struct hid_device *hdev,
 	fill_effect_buf(haptic, &stop_effect, &haptic->stop_effect,
 			HID_HAPTIC_ORDINAL_WAVEFORMSTOP);
 
+	haptic->wq = alloc_ordered_workqueue("hid-haptic %s", WQ_MEM_RECLAIM, hdev->name);
+	if (!haptic->wq) {
+		ret = -ENOMEM;
+		goto stop_buffer_free;
+	}
+
 	input_set_capability(dev, EV_FF, FF_HAPTIC);
 
 	flush = dev->flush;
@@ -504,7 +499,7 @@ int hid_haptic_init(struct hid_device *hdev,
 	ret = input_ff_create(dev, FF_MAX_EFFECTS);
 	if (ret) {
 		dev_err(&hdev->dev, "Failed to create ff device.\n");
-		goto stop_buffer_free;
+		goto reset_evbit;
 	}
 
 	ff = dev->ff;
@@ -530,6 +525,10 @@ int hid_haptic_init(struct hid_device *hdev,
 	dev->flush = flush;
 	dev->event = event;
 	return ret;
+reset_evbit:
+	__clear_bit(EV_FF, dev->evbit);
+	destroy_workqueue(haptic->wq);
+	haptic->wq = NULL;
 stop_buffer_free:
 	kfree(haptic->stop_effect.report_buf);
 	haptic->stop_effect.report_buf = NULL;
@@ -538,9 +537,6 @@ int hid_haptic_init(struct hid_device *hdev,
 		kfree(haptic->effect[r].report_buf);
 	kfree(haptic->effect);
 	haptic->effect = NULL;
-output_queue:
-	destroy_workqueue(haptic->wq);
-	haptic->wq = NULL;
 duration_map:
 	kfree(haptic->duration_map);
 	haptic->duration_map = NULL;

-- 
2.55.0

[PATCH v2 2/9] HID: haptic: Get rid of input_ff_destroy() in error paths

[Rong Zhang]

It's painful to have error paths after calling input_ff_create(),
because input_ff_destroy() frees too much things but does not restore
some other things.

On get_device failure, the module's bail-out path calls module_put()
right before hid_haptic_destroy() calling it again, causing refcount
underflow.

Besides, every error path calling input_ff_destroy() frees ff->private,
while the others don't. This divergence has caused quite a lot of
trouble.

Get the module and the device beforehand, so that there is no error path
after input_ff_create(), eliminating the call to input_ff_destroy(). In
this manner, we don't need to restore dev->flush and dev->event either.

Fixes: 344ff3584957 ("HID: haptic: initialize haptic device")
Signed-off-by: Rong Zhang <i@rong.moe>
---
 drivers/hid/hid-haptic.c | 40 +++++++++++++++++-----------------------
 1 file changed, 17 insertions(+), 23 deletions(-)

diff --git a/drivers/hid/hid-haptic.c b/drivers/hid/hid-haptic.c
index 73446a123b65..9817d661f93d 100644
--- a/drivers/hid/hid-haptic.c
+++ b/drivers/hid/hid-haptic.c
@@ -386,9 +386,6 @@ static void hid_haptic_destroy(struct ff_device *ff)
 	destroy_workqueue(haptic->wq);
 	haptic->wq = NULL;
 
-	if (hdev)
-		put_device(&hdev->dev);
-
 	kfree(haptic->stop_effect.report_buf);
 	haptic->stop_effect.report_buf = NULL;
 
@@ -405,6 +402,9 @@ static void hid_haptic_destroy(struct ff_device *ff)
 	kfree(haptic->hid_usage_map);
 	haptic->hid_usage_map = NULL;
 
+	if (hdev)
+		put_device(&hdev->dev);
+
 	module_put(THIS_MODULE);
 }
 
@@ -417,8 +417,16 @@ int hid_haptic_init(struct hid_device *hdev,
 	struct ff_haptic_effect stop_effect = {
 		.hid_usage = HID_HP_WAVEFORMSTOP & HID_USAGE,
 	};
-	int (*flush)(struct input_dev *dev, struct file *file);
-	int (*event)(struct input_dev *dev, unsigned int type, unsigned int code, int value);
+
+	if (!try_module_get(THIS_MODULE)) {
+		dev_err(&hdev->dev, "Failed to increase module count.\n");
+		return -ENXIO;
+	}
+	if (!get_device(&hdev->dev)) {
+		dev_err(&hdev->dev, "Failed to get hdev device.\n");
+		ret = -ENXIO;
+		goto exit;
+	}
 
 	haptic->hdev = hdev;
 	haptic->max_waveform_id = max(2u, haptic->max_waveform_id);
@@ -427,7 +435,7 @@ int hid_haptic_init(struct hid_device *hdev,
 	haptic->hid_usage_map = kzalloc_objs(u16, haptic->max_waveform_id + 1);
 	if (!haptic->hid_usage_map) {
 		ret = -ENOMEM;
-		goto exit;
+		goto put_device;
 	}
 	haptic->duration_map = kzalloc_objs(u32, haptic->max_duration_id + 1);
 	if (!haptic->duration_map) {
@@ -494,8 +502,6 @@ int hid_haptic_init(struct hid_device *hdev,
 
 	input_set_capability(dev, EV_FF, FF_HAPTIC);
 
-	flush = dev->flush;
-	event = dev->event;
 	ret = input_ff_create(dev, FF_MAX_EFFECTS);
 	if (ret) {
 		dev_err(&hdev->dev, "Failed to create ff device.\n");
@@ -508,23 +514,8 @@ int hid_haptic_init(struct hid_device *hdev,
 	ff->playback = hid_haptic_playback;
 	ff->erase = hid_haptic_erase;
 	ff->destroy = hid_haptic_destroy;
-	if (!try_module_get(THIS_MODULE)) {
-		dev_err(&hdev->dev, "Failed to increase module count.\n");
-		goto input_free;
-	}
-	if (!get_device(&hdev->dev)) {
-		dev_err(&hdev->dev, "Failed to get hdev device.\n");
-		module_put(THIS_MODULE);
-		goto input_free;
-	}
 	return 0;
 
-input_free:
-	input_ff_destroy(dev);
-	/* Restore dev flush and event */
-	dev->flush = flush;
-	dev->event = event;
-	return ret;
 reset_evbit:
 	__clear_bit(EV_FF, dev->evbit);
 	destroy_workqueue(haptic->wq);
@@ -543,7 +534,10 @@ int hid_haptic_init(struct hid_device *hdev,
 usage_map:
 	kfree(haptic->hid_usage_map);
 	haptic->hid_usage_map = NULL;
+put_device:
+	put_device(&hdev->dev);
 exit:
+	module_put(THIS_MODULE);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(hid_haptic_init);

-- 
2.55.0

[PATCH v2 3/9] HID: haptic, multitouch: Check haptic prerequisite properly

[Rong Zhang]

The td->is_haptic_touchpad boolean is multiplexed through during the
probe- and run-time procedures of hid-multitouch. However, the way it
flips is fragile on faulty devices.

MT touchpads have multiple HID_DG_TIPPRESSURE usages, but the value of
td->is_haptic_touchpad only depends on the last usage, and doesn't check
if all usages have the same critical annotations. Moreover, if the HID
device happens to have multiple HID_DG_TOUCHPAD applications, the haptic
device may be initialized more than once.

Fix them by storing the relevant HID_DG_TOUCHPAD input device (must have
a pressure sensor) during hid_haptic_check_pressure_unit(), with proper
sanity checks that reject annotation mismatches and duplicated input
devices. Later, hid_haptic_input_configured() continues the
initialization only if the input device matches the previously stored
one.

With that, the td->is_haptic_touchpad is converted into a one-way (false
=> true) flipping manner. All calls to hid_haptic_*() during the
probe-time procedures are now guarded by the nullability of td->haptic
instead of td->is_haptic_touchpad or no guard, preparing to skip
allocating td->haptic in !CONFIG_HID_HAPTIC builds in the following
changes.

At runtime, relevant haptic methods are called only if the event source
is the same input device as the stored one.

Fixes: 7939f787f450 ("HID: haptic: move FF initialization into .input_configured()")
Signed-off-by: Rong Zhang <i@rong.moe>
---
Changes in v2:
- Reject touchpad applications with non-force units (thanks Sashiko)
- Only call runtime methods if the event source is the same input device
  as the stored one (ditto)
---
 drivers/hid/hid-haptic.c     | 64 ++++++++++++++++++++++++++------------------
 drivers/hid/hid-haptic.h     | 35 ++++++++++++++----------
 drivers/hid/hid-multitouch.c | 36 ++++++++++++-------------
 3 files changed, 76 insertions(+), 59 deletions(-)

diff --git a/drivers/hid/hid-haptic.c b/drivers/hid/hid-haptic.c
index 9817d661f93d..84985ad61d45 100644
--- a/drivers/hid/hid-haptic.c
+++ b/drivers/hid/hid-haptic.c
@@ -48,17 +48,40 @@ void hid_haptic_feature_mapping(struct hid_device *hdev,
 }
 EXPORT_SYMBOL_GPL(hid_haptic_feature_mapping);
 
-bool hid_haptic_check_pressure_unit(struct hid_haptic_device *haptic,
+void hid_haptic_check_pressure_unit(struct hid_haptic_device *haptic,
 				    struct hid_input *hi, struct hid_field *field)
 {
-	if (field->unit == HID_UNIT_GRAM || field->unit == HID_UNIT_NEWTON) {
-		haptic->force_logical_minimum = field->logical_minimum;
-		haptic->force_physical_minimum = field->physical_minimum;
-		haptic->force_resolution = input_abs_get_res(hi->input,
-							     ABS_MT_PRESSURE);
-		return true;
+	if (hi->application != HID_DG_TOUCHPAD)
+		return;
+
+	if (IS_ERR(haptic->input_dev))
+		return;
+
+	if (field->unit != HID_UNIT_GRAM && field->unit != HID_UNIT_NEWTON)
+		goto reject;
+
+	if (!haptic->input_dev) {
+		haptic->input_dev = hi->input;
+		haptic->pressure_field = field;
+		haptic->force_resolution = input_abs_get_res(hi->input, ABS_MT_PRESSURE);
+		return;
 	}
-	return false;
+
+	if (hid_haptic_connected_to_input(haptic, hi->input) &&
+	    haptic->pressure_field->unit == field->unit &&
+	    haptic->pressure_field->logical_minimum == field->logical_minimum &&
+	    haptic->pressure_field->physical_minimum == field->physical_minimum &&
+	    haptic->force_resolution == input_abs_get_res(hi->input, ABS_MT_PRESSURE))
+		return;
+
+reject:
+	/*
+	 * Reject if:
+	 * - there are multiple input devices with pressure sensors
+	 * - there is any touchpad application with a non-force unit
+	 * - pressure annotations mismatch among HID_DG_TIPPRESSURE usages
+	 */
+	haptic->input_dev = ERR_PTR(-EPROTO);
 }
 EXPORT_SYMBOL_GPL(hid_haptic_check_pressure_unit);
 
@@ -78,28 +101,16 @@ int hid_haptic_input_mapping(struct hid_device *hdev,
 }
 EXPORT_SYMBOL_GPL(hid_haptic_input_mapping);
 
-int hid_haptic_input_configured(struct hid_device *hdev,
-				struct hid_haptic_device *haptic,
-				struct hid_input *hi)
+bool hid_haptic_input_configured(struct hid_device *hdev,
+				 struct hid_haptic_device *haptic, struct hid_input *hi)
 {
-	int error;
-
-	if (hi->application != HID_DG_TOUCHPAD)
-		return -1;
-
-	if (!haptic->auto_trigger_report || !haptic->manual_trigger_report)
-		return 0;
+	if (IS_ERR_OR_NULL(haptic->input_dev) || haptic->input_dev != hi->input ||
+	    !haptic->auto_trigger_report || !haptic->manual_trigger_report)
+		return false;
 
 	__set_bit(INPUT_PROP_PRESSUREPAD, hi->input->propbit);
 
-	error = hid_haptic_init(hdev, haptic, hi->input);
-	if (error) {
-		dev_warn(&hdev->dev, "Cannot allocate haptic for %s\n",
-			 hdev->name);
-		return 0;
-	}
-
-	return 1;
+	return hid_haptic_init(hdev, haptic, hi->input) == 0;
 }
 EXPORT_SYMBOL_GPL(hid_haptic_input_configured);
 
@@ -537,6 +548,7 @@ int hid_haptic_init(struct hid_device *hdev,
 put_device:
 	put_device(&hdev->dev);
 exit:
+	dev_warn(&hdev->dev, "Cannot allocate haptic for %s: %d\n", hdev->name, ret);
 	module_put(THIS_MODULE);
 	return ret;
 }
diff --git a/drivers/hid/hid-haptic.h b/drivers/hid/hid-haptic.h
index 6332991a7844..cd4d9c74d2dc 100644
--- a/drivers/hid/hid-haptic.h
+++ b/drivers/hid/hid-haptic.h
@@ -33,12 +33,11 @@ struct hid_haptic_device {
 	struct mutex auto_trigger_mutex;
 	struct workqueue_struct *wq;
 	struct hid_report *manual_trigger_report;
+	struct hid_field *pressure_field;
 	struct mutex manual_trigger_mutex;
 	size_t manual_trigger_report_len;
 	int pressed_state;
 	s32 pressure_sum;
-	s32 force_logical_minimum;
-	s32 force_physical_minimum;
 	s32 force_resolution;
 	u32 mode;
 	u32 default_auto_trigger;
@@ -55,20 +54,25 @@ struct hid_haptic_device {
 };
 
 #if IS_ENABLED(CONFIG_HID_HAPTIC)
+static inline bool hid_haptic_connected_to_input(struct hid_haptic_device *haptic,
+						 struct input_dev *input)
+{
+	return haptic->input_dev == input;
+}
+
 void hid_haptic_feature_mapping(struct hid_device *hdev,
 				struct hid_haptic_device *haptic,
 				struct hid_field *field, struct hid_usage
 				*usage);
-bool hid_haptic_check_pressure_unit(struct hid_haptic_device *haptic,
+void hid_haptic_check_pressure_unit(struct hid_haptic_device *haptic,
 				    struct hid_input *hi, struct hid_field *field);
 int hid_haptic_input_mapping(struct hid_device *hdev,
 			     struct hid_haptic_device *haptic,
 			     struct hid_input *hi,
 			     struct hid_field *field, struct hid_usage *usage,
 			     unsigned long **bit, int *max);
-int hid_haptic_input_configured(struct hid_device *hdev,
-				struct hid_haptic_device *haptic,
-				struct hid_input *hi);
+bool hid_haptic_input_configured(struct hid_device *hdev,
+				 struct hid_haptic_device *haptic, struct hid_input *hi);
 int hid_haptic_init(struct hid_device *hdev, struct hid_haptic_device *haptic,
 		    struct input_dev *dev);
 void hid_haptic_handle_press_release(struct hid_haptic_device *haptic);
@@ -76,6 +80,12 @@ void hid_haptic_pressure_reset(struct hid_haptic_device *haptic);
 void hid_haptic_pressure_increase(struct hid_haptic_device *haptic,
 				  __s32 pressure);
 #else
+static inline bool hid_haptic_connected_to_input(struct hid_haptic_device *haptic,
+						 struct input_dev *input)
+{
+	return false;
+}
+
 static inline
 void hid_haptic_feature_mapping(struct hid_device *hdev,
 				struct hid_haptic_device *haptic,
@@ -83,11 +93,9 @@ void hid_haptic_feature_mapping(struct hid_device *hdev,
 				*usage)
 {}
 static inline
-bool hid_haptic_check_pressure_unit(struct hid_haptic_device *haptic,
+void hid_haptic_check_pressure_unit(struct hid_haptic_device *haptic,
 				    struct hid_input *hi, struct hid_field *field)
-{
-	return false;
-}
+{}
 static inline
 int hid_haptic_input_mapping(struct hid_device *hdev,
 			     struct hid_haptic_device *haptic,
@@ -98,11 +106,10 @@ int hid_haptic_input_mapping(struct hid_device *hdev,
 	return 0;
 }
 static inline
-int hid_haptic_input_configured(struct hid_device *hdev,
-				struct hid_haptic_device *haptic,
-				struct hid_input *hi)
+bool hid_haptic_input_configured(struct hid_device *hdev,
+				 struct hid_haptic_device *haptic, struct hid_input *hi)
 {
-	return 0;
+	return false;
 }
 static inline
 void hid_haptic_reset(struct hid_device *hdev, struct hid_haptic_device *haptic)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 3afca07025ea..5eea774dee44 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -605,7 +605,8 @@ static void mt_feature_mapping(struct hid_device *hdev,
 		break;
 	}
 
-	hid_haptic_feature_mapping(hdev, td->haptic, field, usage);
+	if (td->haptic)
+		hid_haptic_feature_mapping(hdev, td->haptic, field, usage);
 }
 
 static void set_abs(struct input_dev *input, unsigned int code,
@@ -961,9 +962,8 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 		case HID_DG_TIPPRESSURE:
 			set_abs(hi->input, ABS_MT_PRESSURE, field,
 				cls->sn_pressure);
-			td->is_haptic_touchpad =
-				hid_haptic_check_pressure_unit(td->haptic,
-							       hi, field);
+			if (td->haptic)
+				hid_haptic_check_pressure_unit(td->haptic, hi, field);
 			MT_STORE_FIELD(p);
 			return 1;
 		case HID_DG_SCANTIME:
@@ -1085,7 +1085,7 @@ static void mt_sync_frame(struct mt_device *td, struct mt_application *app,
 
 	app->num_received = 0;
 	app->left_button_state = 0;
-	if (td->is_haptic_touchpad)
+	if (td->is_haptic_touchpad && hid_haptic_connected_to_input(td->haptic, input))
 		hid_haptic_pressure_reset(td->haptic);
 }
 
@@ -1238,7 +1238,7 @@ static int mt_process_slot(struct mt_device *td, struct input_dev *input,
 			minor = minor >> 1;
 		}
 
-		if (td->is_haptic_touchpad)
+		if (td->is_haptic_touchpad && hid_haptic_connected_to_input(td->haptic, input))
 			hid_haptic_pressure_increase(td->haptic, *slot->p);
 
 		x = hdev->quirks & HID_QUIRK_X_INVERT ?
@@ -1441,7 +1441,7 @@ static int mt_touch_input_configured(struct hid_device *hdev,
 	if (cls->is_indirect)
 		app->mt_flags |= INPUT_MT_POINTER;
 
-	if (td->is_haptic_touchpad)
+	if (td->is_haptic_touchpad && hid_haptic_connected_to_input(td->haptic, input))
 		app->mt_flags |= INPUT_MT_TOTAL_FORCE;
 
 	if (app->quirks & MT_QUIRK_NOT_SEEN_MEANS_UP)
@@ -1553,10 +1553,11 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 	if (field->physical == HID_DG_STYLUS)
 		hi->application = HID_DG_STYLUS;
 
-	ret = hid_haptic_input_mapping(hdev, td->haptic, hi, field, usage, bit,
-				       max);
-	if (ret != 0)
-		return ret;
+	if (td->haptic) {
+		ret = hid_haptic_input_mapping(hdev, td->haptic, hi, field, usage, bit, max);
+		if (ret)
+			return ret;
+	}
 
 	/* let hid-core decide for the others */
 	return 0;
@@ -1954,14 +1955,11 @@ static int mt_input_configured(struct hid_device *hdev, struct hid_input *hi)
 	struct hid_report *report;
 	int ret;
 
-	if (td->is_haptic_touchpad && (td->mtclass.name == MT_CLS_WIN_8 ||
-	    td->mtclass.name == MT_CLS_WIN_8_FORCE_MULTI_INPUT ||
-	    td->mtclass.name == MT_CLS_WIN_8_KEEP_LATENCY_ON_CLOSE)) {
-		if (hid_haptic_input_configured(hdev, td->haptic, hi) == 0)
-			td->is_haptic_touchpad = false;
-	} else {
-		td->is_haptic_touchpad = false;
-	}
+	if (td->haptic && !td->is_haptic_touchpad &&
+	    (td->mtclass.name == MT_CLS_WIN_8 ||
+	     td->mtclass.name == MT_CLS_WIN_8_FORCE_MULTI_INPUT ||
+	     td->mtclass.name == MT_CLS_WIN_8_KEEP_LATENCY_ON_CLOSE))
+		td->is_haptic_touchpad = hid_haptic_input_configured(hdev, td->haptic, hi);
 
 	list_for_each_entry(report, &hi->reports, hidinput_list) {
 		rdata = mt_find_report_data(td, report);

-- 
2.55.0

[PATCH v2 4/9] HID: haptic, multitouch: Fix haptic device double-free and unnecessary allocation

[Rong Zhang]

The haptic device is allocated by devm_kzalloc(), which will be freed by
devres on HID device release. At that time, the ff core has already
freed it on input device release previously, causing double-free.

Allocate the haptic device using kzalloc_obj() to ensure its release can
be safely delegates to the ff core.

The allocation code is also moved to hid-haptic.h, so that it can be
replaced by a stub in !CONFIG_HID_HAPTIC builds, eliminating the
meaningless alloc-and-free sequence.

Fixes: 344ff3584957 ("HID: haptic: initialize haptic device")
Signed-off-by: Rong Zhang <i@rong.moe>
---
 drivers/hid/hid-haptic.h     | 17 +++++++++++++++++
 drivers/hid/hid-multitouch.c | 39 +++++++++++++++++++++++++++++----------
 2 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/drivers/hid/hid-haptic.h b/drivers/hid/hid-haptic.h
index cd4d9c74d2dc..7878cd73ccdb 100644
--- a/drivers/hid/hid-haptic.h
+++ b/drivers/hid/hid-haptic.h
@@ -5,6 +5,7 @@
  *  Copyright (c) 2021 Angela Czubak <acz@semihalf.com>
  */
 
+#include <linux/err.h>
 #include <linux/hid.h>
 
 #define HID_HAPTIC_ORDINAL_WAVEFORMNONE 1
@@ -54,6 +55,17 @@ struct hid_haptic_device {
 };
 
 #if IS_ENABLED(CONFIG_HID_HAPTIC)
+static inline struct hid_haptic_device *hid_haptic_alloc(struct hid_device *hdev)
+{
+	struct hid_haptic_device *haptic;
+
+	haptic = kzalloc_obj(struct hid_haptic_device);
+	if (haptic)
+		haptic->hdev = hdev;
+
+	return haptic;
+}
+
 static inline bool hid_haptic_connected_to_input(struct hid_haptic_device *haptic,
 						 struct input_dev *input)
 {
@@ -80,6 +92,11 @@ void hid_haptic_pressure_reset(struct hid_haptic_device *haptic);
 void hid_haptic_pressure_increase(struct hid_haptic_device *haptic,
 				  __s32 pressure);
 #else
+static inline struct hid_haptic_device *hid_haptic_alloc(struct hid_device *hdev)
+{
+	return ERR_PTR(-EOPNOTSUPP);
+}
+
 static inline bool hid_haptic_connected_to_input(struct hid_haptic_device *haptic,
 						 struct input_dev *input)
 {
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 5eea774dee44..a9c8396c804e 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -34,6 +34,7 @@
 #include <linux/bitmap.h>
 #include <linux/bits.h>
 #include <linux/device.h>
+#include <linux/err.h>
 #include <linux/hid.h>
 #include <linux/module.h>
 #include <linux/slab.h>
@@ -2129,11 +2130,18 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		dev_err(&hdev->dev, "cannot allocate multitouch data\n");
 		return -ENOMEM;
 	}
-	td->haptic = devm_kzalloc_obj(&hdev->dev, *(td->haptic));
-	if (!td->haptic)
+
+	td->haptic = hid_haptic_alloc(hdev);
+	if (IS_ERR(td->haptic)) {
+		ret = PTR_ERR(td->haptic);
+		if (ret == -EOPNOTSUPP)
+			td->haptic = NULL;
+		else
+			return ret;
+	} else if (!td->haptic) {
 		return -ENOMEM;
+	}
 
-	td->haptic->hdev = hdev;
 	td->hdev = hdev;
 	td->mtclass = *mtclass;
 	td->inputmode_value = MT_INPUTMODE_TOUCHSCREEN;
@@ -2178,12 +2186,14 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
 
 	ret = hid_parse(hdev);
 	if (ret != 0)
-		return ret;
+		goto out;
 
 	if (mtclass->name == MT_CLS_APPLE_TOUCHBAR &&
 	    !hid_find_field(hdev, HID_INPUT_REPORT,
-			    HID_DG_TOUCHPAD, HID_DG_TRANSDUCER_INDEX))
-		return -ENODEV;
+			    HID_DG_TOUCHPAD, HID_DG_TRANSDUCER_INDEX)) {
+		ret = -ENODEV;
+		goto out;
+	}
 
 	if (mtclass->quirks & MT_QUIRK_FIX_CONST_CONTACT_ID)
 		mt_fix_const_fields(hdev, HID_DG_CONTACTID);
@@ -2193,7 +2203,7 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
 
 	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
 	if (ret)
-		return ret;
+		goto out;
 
 	ret = sysfs_create_group(&hdev->dev.kobj, &mt_attribute_group);
 	if (ret)
@@ -2202,10 +2212,19 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
 
 	mt_set_modes(hdev, HID_LATENCY_NORMAL, TOUCHPAD_REPORT_ALL);
 
-	if (!td->is_haptic_touchpad)
-		devm_kfree(&hdev->dev, td->haptic);
+	ret = 0;
 
-	return 0;
+out:
+	/*
+	 * Don't double free when the haptic device has been delegated to the ff
+	 * core.
+	 */
+	if (td->haptic && !td->is_haptic_touchpad) {
+		kfree(td->haptic);
+		td->haptic = NULL;
+	}
+
+	return ret;
 }
 
 static int mt_suspend(struct hid_device *hdev, pm_message_t state)

-- 
2.55.0

[PATCH v2 5/9] HID: haptic: Postpone GET_REPORT to init

[Rong Zhang]

Currently, the driver calls GET_REPORT within the feature_mapping
callback. This is not optimal, as the driver works only if manual
trigger is also available and the if the device is a touchpad. It makes
no sense to initiate device IO if the device is not supported by the
driver.

Hence, postpone GET_REPORT to hid_haptic_init(), and parse default
trigger in parse_auto_trigger_field() where many fields are already
parsed there.

Signed-off-by: Rong Zhang <i@rong.moe>
---
 drivers/hid/hid-haptic.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/hid/hid-haptic.c b/drivers/hid/hid-haptic.c
index 84985ad61d45..9f2af082d2ea 100644
--- a/drivers/hid/hid-haptic.c
+++ b/drivers/hid/hid-haptic.c
@@ -22,13 +22,6 @@ void hid_haptic_feature_mapping(struct hid_device *hdev,
 				"HID_HP_AUTOTRIGGER out of range\n");
 			return;
 		}
-
-		hid_device_io_start(hdev);
-		hid_hw_request(hdev, field->report, HID_REQ_GET_REPORT);
-		hid_hw_wait(hdev);
-		hid_device_io_stop(hdev);
-		haptic->default_auto_trigger =
-			field->value[usage->usage_index];
 		haptic->auto_trigger_report = field->report;
 	} else if ((usage->hid & HID_USAGE_PAGE) == HID_UP_ORDINAL) {
 		usage_hid = usage->hid & HID_USAGE;
@@ -146,6 +139,9 @@ static void parse_auto_trigger_field(struct hid_haptic_device *haptic,
 			break;
 		case HID_UP_HAPTIC:
 			switch (field->usage[n].hid) {
+			case HID_HP_AUTOTRIGGER:
+				haptic->default_auto_trigger = field->value[n];
+				break;
 			case HID_HP_WAVEFORMVENDORID:
 				haptic->vendor_id = field->value[n];
 				break;
@@ -465,6 +461,12 @@ int hid_haptic_init(struct hid_device *hdev,
 		HID_HP_WAVEFORMSTOP & HID_USAGE;
 
 	mutex_init(&haptic->auto_trigger_mutex);
+
+	hid_device_io_start(hdev);
+	hid_hw_request(hdev, haptic->auto_trigger_report, HID_REQ_GET_REPORT);
+	hid_hw_wait(hdev);
+	hid_device_io_stop(hdev);
+
 	for (r = 0; r < haptic->auto_trigger_report->maxfield; r++)
 		parse_auto_trigger_field(haptic, haptic->auto_trigger_report->field[r]);
 

-- 
2.55.0

[PATCH v2 6/9] HID: haptic: Convert simple methods into static inline ones

[Rong Zhang]

Some methods of hid-haptic are meant to check the prerequisites before
really initializing the haptic device. Most of them are quite simple and
straightforward. To prepare for the following changes that convert
hid-haptic into a loadable plugin module, factor them out of the module
and convert them into static inline functions.

Some unused declarations and stubs in the header are also removed, i.e.,
hid_haptic_handle_press_release(), hid_haptic_reset(),
hid_haptic_handle_input(). They seemed to be remnants from the patch
revisioning process.

No functional change intended.

Signed-off-by: Rong Zhang <i@rong.moe>
---
 drivers/hid/hid-haptic.c | 110 ---------------------------------------
 drivers/hid/hid-haptic.h | 131 +++++++++++++++++++++++++++++++++++++----------
 2 files changed, 104 insertions(+), 137 deletions(-)

diff --git a/drivers/hid/hid-haptic.c b/drivers/hid/hid-haptic.c
index 9f2af082d2ea..a6f3ec514504 100644
--- a/drivers/hid/hid-haptic.c
+++ b/drivers/hid/hid-haptic.c
@@ -10,103 +10,6 @@
 
 #include "hid-haptic.h"
 
-void hid_haptic_feature_mapping(struct hid_device *hdev,
-				struct hid_haptic_device *haptic,
-				struct hid_field *field, struct hid_usage *usage)
-{
-	u16 usage_hid;
-
-	if (usage->hid == HID_HP_AUTOTRIGGER) {
-		if (usage->usage_index >= field->report_count) {
-			dev_err(&hdev->dev,
-				"HID_HP_AUTOTRIGGER out of range\n");
-			return;
-		}
-		haptic->auto_trigger_report = field->report;
-	} else if ((usage->hid & HID_USAGE_PAGE) == HID_UP_ORDINAL) {
-		usage_hid = usage->hid & HID_USAGE;
-		switch (field->logical) {
-		case HID_HP_WAVEFORMLIST:
-			if (usage_hid > haptic->max_waveform_id)
-				haptic->max_waveform_id = usage_hid;
-			break;
-		case HID_HP_DURATIONLIST:
-			if (usage_hid > haptic->max_duration_id)
-				haptic->max_duration_id = usage_hid;
-			break;
-		default:
-			break;
-		}
-	}
-}
-EXPORT_SYMBOL_GPL(hid_haptic_feature_mapping);
-
-void hid_haptic_check_pressure_unit(struct hid_haptic_device *haptic,
-				    struct hid_input *hi, struct hid_field *field)
-{
-	if (hi->application != HID_DG_TOUCHPAD)
-		return;
-
-	if (IS_ERR(haptic->input_dev))
-		return;
-
-	if (field->unit != HID_UNIT_GRAM && field->unit != HID_UNIT_NEWTON)
-		goto reject;
-
-	if (!haptic->input_dev) {
-		haptic->input_dev = hi->input;
-		haptic->pressure_field = field;
-		haptic->force_resolution = input_abs_get_res(hi->input, ABS_MT_PRESSURE);
-		return;
-	}
-
-	if (hid_haptic_connected_to_input(haptic, hi->input) &&
-	    haptic->pressure_field->unit == field->unit &&
-	    haptic->pressure_field->logical_minimum == field->logical_minimum &&
-	    haptic->pressure_field->physical_minimum == field->physical_minimum &&
-	    haptic->force_resolution == input_abs_get_res(hi->input, ABS_MT_PRESSURE))
-		return;
-
-reject:
-	/*
-	 * Reject if:
-	 * - there are multiple input devices with pressure sensors
-	 * - there is any touchpad application with a non-force unit
-	 * - pressure annotations mismatch among HID_DG_TIPPRESSURE usages
-	 */
-	haptic->input_dev = ERR_PTR(-EPROTO);
-}
-EXPORT_SYMBOL_GPL(hid_haptic_check_pressure_unit);
-
-int hid_haptic_input_mapping(struct hid_device *hdev,
-			     struct hid_haptic_device *haptic,
-			     struct hid_input *hi,
-			     struct hid_field *field, struct hid_usage *usage,
-			     unsigned long **bit, int *max)
-{
-	if (usage->hid == HID_HP_MANUALTRIGGER) {
-		haptic->manual_trigger_report = field->report;
-		/* we don't really want to map these fields */
-		return -1;
-	}
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(hid_haptic_input_mapping);
-
-bool hid_haptic_input_configured(struct hid_device *hdev,
-				 struct hid_haptic_device *haptic, struct hid_input *hi)
-{
-	if (IS_ERR_OR_NULL(haptic->input_dev) || haptic->input_dev != hi->input ||
-	    !haptic->auto_trigger_report || !haptic->manual_trigger_report)
-		return false;
-
-	__set_bit(INPUT_PROP_PRESSUREPAD, hi->input->propbit);
-
-	return hid_haptic_init(hdev, haptic, hi->input) == 0;
-}
-EXPORT_SYMBOL_GPL(hid_haptic_input_configured);
-
 static void parse_auto_trigger_field(struct hid_haptic_device *haptic,
 				     struct hid_field *field)
 {
@@ -555,16 +458,3 @@ int hid_haptic_init(struct hid_device *hdev,
 	return ret;
 }
 EXPORT_SYMBOL_GPL(hid_haptic_init);
-
-void hid_haptic_pressure_reset(struct hid_haptic_device *haptic)
-{
-	haptic->pressure_sum = 0;
-}
-EXPORT_SYMBOL_GPL(hid_haptic_pressure_reset);
-
-void hid_haptic_pressure_increase(struct hid_haptic_device *haptic,
-				 __s32 pressure)
-{
-	haptic->pressure_sum += pressure;
-}
-EXPORT_SYMBOL_GPL(hid_haptic_pressure_increase);
diff --git a/drivers/hid/hid-haptic.h b/drivers/hid/hid-haptic.h
index 7878cd73ccdb..6196041eb4f8 100644
--- a/drivers/hid/hid-haptic.h
+++ b/drivers/hid/hid-haptic.h
@@ -72,25 +72,112 @@ static inline bool hid_haptic_connected_to_input(struct hid_haptic_device *hapti
 	return haptic->input_dev == input;
 }
 
-void hid_haptic_feature_mapping(struct hid_device *hdev,
-				struct hid_haptic_device *haptic,
-				struct hid_field *field, struct hid_usage
-				*usage);
-void hid_haptic_check_pressure_unit(struct hid_haptic_device *haptic,
-				    struct hid_input *hi, struct hid_field *field);
-int hid_haptic_input_mapping(struct hid_device *hdev,
-			     struct hid_haptic_device *haptic,
-			     struct hid_input *hi,
-			     struct hid_field *field, struct hid_usage *usage,
-			     unsigned long **bit, int *max);
-bool hid_haptic_input_configured(struct hid_device *hdev,
-				 struct hid_haptic_device *haptic, struct hid_input *hi);
+static inline void hid_haptic_feature_mapping(struct hid_device *hdev,
+					      struct hid_haptic_device *haptic,
+					      struct hid_field *field, struct hid_usage *usage)
+{
+	u16 usage_hid;
+
+	if (usage->hid == HID_HP_AUTOTRIGGER) {
+		if (usage->usage_index >= field->report_count) {
+			dev_err(&hdev->dev, "HID_HP_AUTOTRIGGER out of range\n");
+			return;
+		}
+
+		haptic->auto_trigger_report = field->report;
+	} else if ((usage->hid & HID_USAGE_PAGE) == HID_UP_ORDINAL) {
+		usage_hid = usage->hid & HID_USAGE;
+		switch (field->logical) {
+		case HID_HP_WAVEFORMLIST:
+			if (usage_hid > haptic->max_waveform_id)
+				haptic->max_waveform_id = usage_hid;
+			break;
+		case HID_HP_DURATIONLIST:
+			if (usage_hid > haptic->max_duration_id)
+				haptic->max_duration_id = usage_hid;
+			break;
+		default:
+			break;
+		}
+	}
+}
+
+static inline void hid_haptic_check_pressure_unit(struct hid_haptic_device *haptic,
+						  struct hid_input *hi, struct hid_field *field)
+{
+	if (hi->application != HID_DG_TOUCHPAD)
+		return;
+
+	if (IS_ERR(haptic->input_dev))
+		return;
+
+	if (field->unit != HID_UNIT_GRAM && field->unit != HID_UNIT_NEWTON)
+		goto reject;
+
+	if (!haptic->input_dev) {
+		haptic->input_dev = hi->input;
+		haptic->pressure_field = field;
+		haptic->force_resolution = input_abs_get_res(hi->input, ABS_MT_PRESSURE);
+		return;
+	}
+
+	if (hid_haptic_connected_to_input(haptic, hi->input) &&
+	    haptic->pressure_field->unit == field->unit &&
+	    haptic->pressure_field->logical_minimum == field->logical_minimum &&
+	    haptic->pressure_field->physical_minimum == field->physical_minimum &&
+	    haptic->force_resolution == input_abs_get_res(hi->input, ABS_MT_PRESSURE))
+		return;
+
+reject:
+	/*
+	 * Reject if:
+	 * - there are multiple input devices with pressure sensors
+	 * - there is any touchpad application with a non-force unit
+	 * - pressure annotations mismatch among HID_DG_TIPPRESSURE usages
+	 */
+	haptic->input_dev = ERR_PTR(-EPROTO);
+}
+
+static inline int hid_haptic_input_mapping(struct hid_device *hdev,
+					   struct hid_haptic_device *haptic,
+					   struct hid_input *hi,
+					   struct hid_field *field, struct hid_usage *usage,
+					   unsigned long **bit, int *max)
+{
+	if (usage->hid == HID_HP_MANUALTRIGGER) {
+		haptic->manual_trigger_report = field->report;
+		/* we don't really want to map these fields */
+		return -1;
+	}
+	return 0;
+}
+
 int hid_haptic_init(struct hid_device *hdev, struct hid_haptic_device *haptic,
 		    struct input_dev *dev);
-void hid_haptic_handle_press_release(struct hid_haptic_device *haptic);
-void hid_haptic_pressure_reset(struct hid_haptic_device *haptic);
-void hid_haptic_pressure_increase(struct hid_haptic_device *haptic,
-				  __s32 pressure);
+
+static inline bool hid_haptic_input_configured(struct hid_device *hdev,
+					       struct hid_haptic_device *haptic,
+					       struct hid_input *hi)
+{
+	if (IS_ERR_OR_NULL(haptic->input_dev) || haptic->input_dev != hi->input ||
+	    !haptic->auto_trigger_report || !haptic->manual_trigger_report)
+		return false;
+
+	__set_bit(INPUT_PROP_PRESSUREPAD, hi->input->propbit);
+
+	return hid_haptic_init(hdev, haptic, hi->input) == 0;
+}
+
+static inline void hid_haptic_pressure_reset(struct hid_haptic_device *haptic)
+{
+	haptic->pressure_sum = 0;
+}
+
+static inline void hid_haptic_pressure_increase(struct hid_haptic_device *haptic,
+						__s32 pressure)
+{
+	haptic->pressure_sum += pressure;
+}
 #else
 static inline struct hid_haptic_device *hid_haptic_alloc(struct hid_device *hdev)
 {
@@ -129,22 +216,12 @@ bool hid_haptic_input_configured(struct hid_device *hdev,
 	return false;
 }
 static inline
-void hid_haptic_reset(struct hid_device *hdev, struct hid_haptic_device *haptic)
-{}
-static inline
 int hid_haptic_init(struct hid_device *hdev, struct hid_haptic_device *haptic,
 		    struct input_dev *dev)
 {
 	return 0;
 }
 static inline
-void hid_haptic_handle_press_release(struct hid_haptic_device *haptic) {}
-static inline
-bool hid_haptic_handle_input(struct hid_haptic_device *haptic)
-{
-	return false;
-}
-static inline
 void hid_haptic_pressure_reset(struct hid_haptic_device *haptic) {}
 static inline
 void hid_haptic_pressure_increase(struct hid_haptic_device *haptic,

-- 
2.55.0

[PATCH v2 7/9] HID: Add hid-plugin framework for loadable plugin modules

[Rong Zhang]

Some devices support some additional HID usage pages which enhance their
capabilities. The support code for these usage pages is called "HID plugin".

The corresponding HID driver calls functions provided by the plugin to
take the advantage of such enhancements. Without the framework, the HID
driver must have a linkage dependence on the plugin, which is not
optimal because the plugin is always loaded into memory even if no
device actually supports it.

Logically, the dependence should be reversed, i.e., the plugin should be
an optional component of the HID driver, which is only loaded when the
driver finds that the device needs it.

Introduce hid-plugin framework for loadable plugin modules to achieve
the said logical dependence. Each plugin has a unique hid_plugin_id when
enabled, so that HID drivers can request it accordingly. The framework
aims to be lightweight. It loads the plugin module when needed. Once
loaded (or if the plugin is builtin), requesting the plugin is just an
RCU dereference plusing an module refcount increment.

When a plugin is disabled, its id maps to HID_PLUGIN_DISABLED, and calls
to the plugin helper functions are eliminated by the compiler.

Thanks Benjamin Tissoires for the inspiration for the implementation.

Suggested-by: Benjamin Tissoires <bentiss@kernel.org>
Link: https://lore.kernel.org/linux-input/auypydfkhx2eg7vp764way4batdilzc35inqda3exwzs3tk3ff@oagat6g46zto/
Signed-off-by: Rong Zhang <i@rong.moe>
---
 MAINTAINERS              |   6 ++
 drivers/hid/Kconfig      |  14 +++++
 drivers/hid/Makefile     |   2 +
 drivers/hid/hid-plugin.c | 105 +++++++++++++++++++++++++++++++++++
 drivers/hid/hid-plugin.h | 139 +++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 266 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 222b7fc2c3aa..06c82b4a7614 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11905,6 +11905,12 @@ L:	linux-input@vger.kernel.org
 S:	Supported
 F:	drivers/hid/hid-playstation.c
 
+HID PLUGINS
+M:	Rong Zhang <i@rong.moe>
+L:	linux-input@vger.kernel.org
+S:	Maintained
+F:	drivers/hid/hid-plugin.*
+
 HID PULSAR DRIVER
 M:	Nikolas Koesling <nikolas@koesling.info>
 L:	linux-input@vger.kernel.org
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 2481bc01c435..604683b4a4a9 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -92,6 +92,20 @@ config HID_GENERIC
 
 	If unsure, say Y.
 
+menuconfig HID_PLUGIN
+	bool "HID plugins"
+	help
+	Support for HID plugins.
+
+	Say Y here if you want to enhance HID drivers with additional
+	functionalities provided by corresponding plugins.
+
+	Plugins can be either builtin or modules. A plugin compiled as a module
+	is made available to HID drivers by automatically loading it when
+	requested.
+
+	If unsure, say N.
+
 config HID_HAPTIC
 	bool "Haptic touchpad support"
 	default n
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 43a8eb0f389b..b41aa3f52a94 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -4,6 +4,8 @@
 #
 hid-y			:= hid-core.o hid-input.o hid-quirks.o
 hid-$(CONFIG_DEBUG_FS)		+= hid-debug.o
+hid-$(CONFIG_HID_PLUGIN)	+= hid-plugin.o
+
 hid-$(CONFIG_HID_HAPTIC)	+= hid-haptic.o
 
 obj-$(CONFIG_HID_BPF)		+= bpf/
diff --git a/drivers/hid/hid-plugin.c b/drivers/hid/hid-plugin.c
new file mode 100644
index 000000000000..a83275df038c
--- /dev/null
+++ b/drivers/hid/hid-plugin.c
@@ -0,0 +1,105 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  HID plugin helpers
+ *
+ *  Copyright (c) 2026 Rong Zhang <i@rong.moe>
+ */
+
+#define pr_fmt(fmt) "hid-plugin: " fmt
+
+#include <linux/bug.h>
+#include <linux/cleanup.h>
+#include <linux/compiler.h>
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/printk.h>
+#include <linux/rcupdate.h>
+
+#include "hid-plugin.h"
+
+static const char * const hid_plugin_mod_names[NR_HID_PLUGINS] = {
+};
+
+static struct hid_plugin __rcu *hid_plugins[NR_HID_PLUGINS];
+
+static struct hid_plugin *hid_plugin_try_module_get(enum hid_plugin_id id)
+{
+	guard(rcu)();
+
+	struct hid_plugin *plugin = rcu_dereference(hid_plugins[id]);
+
+	if (!plugin)
+		return NULL;
+
+	/* Is the module unloading? */
+	return try_module_get(plugin->owner) ? plugin : ERR_PTR(-ENXIO);
+}
+
+struct hid_plugin *__hid_plugin_get(enum hid_plugin_id id)
+{
+	struct hid_plugin *plugin;
+	const char *mod_name;
+	int ret;
+
+	/*
+	 * Compile the function as a stub if plugin support is enabled but no
+	 * plugin is actually enabled. Utilizing dead code elimination is the
+	 * best thing we can do as it's impossible to evaluate an enumeration in
+	 * macros.
+	 */
+	if (NR_HID_PLUGINS <= 0)
+		return ERR_PTR(-ENOENT);
+
+	plugin = hid_plugin_try_module_get(id);
+	if (plugin)
+		return plugin;
+
+	mod_name = hid_plugin_mod_names[id];
+	if (WARN_ON(!mod_name)) /* Huh? Builtin plugins can't reach here. */
+		return ERR_PTR(-ENXIO);
+
+	ret = request_module(mod_name);
+	if (ret) {
+		pr_warn("failed to load [%d]%s: %d\n", id, mod_name, ret);
+		return ERR_PTR(-ENXIO); /* Don't return positive error code. */
+	}
+
+	plugin = hid_plugin_try_module_get(id);
+	return plugin ?: ERR_PTR(-ENXIO);
+}
+EXPORT_SYMBOL_GPL(__hid_plugin_get);
+
+int __hid_plugin_register(enum hid_plugin_id id, struct hid_plugin *plugin)
+{
+	if (NR_HID_PLUGINS <= 0)
+		return -ENOENT;
+
+	/*
+	 * The corresponding plugin is the only writer of hid_plugins[id], so
+	 * there is no need to protect the read access with RCU or to protect
+	 * the write access with an extra lock.
+	 *
+	 * However, try our best to do some sanity checks.
+	 */
+	if (WARN_ON(rcu_access_pointer(hid_plugins[id])))
+		return -EEXIST;
+
+	rcu_assign_pointer(hid_plugins[id], plugin);
+	synchronize_rcu();
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(__hid_plugin_register);
+
+void __hid_plugin_unregister(enum hid_plugin_id id, struct hid_plugin *plugin)
+{
+	if (NR_HID_PLUGINS <= 0)
+		return;
+
+	if (WARN_ON(rcu_access_pointer(hid_plugins[id]) != plugin))
+		return;
+
+	rcu_assign_pointer(hid_plugins[id], NULL);
+	synchronize_rcu();
+}
+EXPORT_SYMBOL_GPL(__hid_plugin_unregister);
diff --git a/drivers/hid/hid-plugin.h b/drivers/hid/hid-plugin.h
new file mode 100644
index 000000000000..4038325c51a0
--- /dev/null
+++ b/drivers/hid/hid-plugin.h
@@ -0,0 +1,139 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ *  HID plugin helpers
+ *
+ *  Copyright (c) 2026 Rong Zhang <i@rong.moe>
+ */
+
+#ifndef __HID_PLUGIN_H
+#define __HID_PLUGIN_H
+
+#include <linux/bug.h>
+#include <linux/build_bug.h>
+#include <linux/compiler.h>
+#include <linux/err.h>
+#include <linux/kconfig.h>
+#include <linux/init.h>
+#include <linux/module.h>
+
+enum hid_plugin_id {
+	HID_PLUGIN_DISABLED	= -1,
+
+	NR_HID_PLUGINS,
+};
+
+#if !IS_ENABLED(CONFIG_HID_PLUGIN)
+static_assert(NR_HID_PLUGINS == 0);
+#endif
+
+struct hid_plugin {
+	struct module *owner;
+};
+
+struct hid_plugin *__hid_plugin_get(enum hid_plugin_id id);
+int __hid_plugin_register(enum hid_plugin_id id, struct hid_plugin *plugin);
+void __hid_plugin_unregister(enum hid_plugin_id id, struct hid_plugin *plugin);
+
+/**
+ * hid_plugin_enabled() - Check if an HID plugin is enabled.
+ * @id: Unique ID of the desired HID plugin
+ *
+ * Return: boolean.
+ */
+static __always_inline bool hid_plugin_enabled(enum hid_plugin_id id)
+{
+	BUILD_BUG_ON(!__builtin_constant_p(id) || id >= NR_HID_PLUGINS || id < HID_PLUGIN_DISABLED);
+
+	return id != HID_PLUGIN_DISABLED;
+}
+
+/**
+ * hid_plugin_get() - Get an HID plugin and increment its refcount.
+ * @id: Unique ID of the desired HID plugin
+ *
+ * If the plugin is builtin or loaded, a pointer to the plugin is returned with
+ * the plugin's module refcount incremented. If the plugin is unloading during
+ * the process, ERR_PTR(-ENXIO) is returned.
+ *
+ * If the plugin is disabled, ERR_PTR(-ENOENT) is returned.
+ *
+ * Otherwise, try loading the module with request_module() and then increment
+ * its refcount. If the plugin fails to load, ERR_PTR(-ENXIO) is returned.
+ *
+ * Return: The HID plugin corresponding to @id on success, or an ERR_PTR().
+ * Never NULL.
+ */
+static __always_inline struct hid_plugin *hid_plugin_get(enum hid_plugin_id id)
+{
+	if (hid_plugin_enabled(id))
+		return __hid_plugin_get(id);
+
+	return ERR_PTR(-ENOENT);
+}
+
+/**
+ * hid_plugin_put() - Decrement an HID plugin's refcount.
+ * @id: Unique ID of @plugin
+ * @plugin: Pointer to the HID plugin
+ *
+ * Decrement @plugin's refcount, unless it's disabled.
+ */
+static __always_inline void hid_plugin_put(enum hid_plugin_id id, struct hid_plugin *plugin)
+{
+	if (hid_plugin_enabled(id) && !IS_ERR_OR_NULL(plugin))
+		module_put(plugin->owner);
+}
+
+/**
+ * hid_plugin_register() - Register an HID plugin.
+ * @id: Unique ID of @plugin
+ * @plugin: Pointer to the HID plugin
+ */
+static __always_inline int hid_plugin_register(enum hid_plugin_id id, struct hid_plugin *plugin)
+{
+	BUILD_BUG_ON(!hid_plugin_enabled(id));
+
+	return __hid_plugin_register(id, plugin);
+}
+
+/**
+ * hid_plugin_unregister() - Unregister an HID plugin.
+ * @id: Unique ID of @plugin
+ * @plugin: Pointer to the HID plugin
+ */
+static __always_inline void hid_plugin_unregister(enum hid_plugin_id id, struct hid_plugin *plugin)
+{
+	BUILD_BUG_ON(!hid_plugin_enabled(id));
+
+	__hid_plugin_unregister(id, plugin);
+}
+
+/*
+ * Usage:
+ *
+ *	struct foo_plugin {
+ *		struct hid_plugin hp;
+ *		void (*bar)(struct hid_device *hdev);
+ *	};
+ *
+ *	static struct foo_plugin foo_plugin = {
+ *		.hp = {
+ *			.owner = THIS_MODULE,
+ *		},
+ *		.bar = hid_foo_bar,
+ *	};
+ *	module_hid_plugin(foo, HID_PLUGIN_FOO, &foo_plugin.hp);
+ */
+#define module_hid_plugin(_name, _id, _plugin)		\
+static int __init hid_plugin_##_name##_init(void)	\
+{							\
+	return hid_plugin_register(_id, _plugin);	\
+}							\
+subsys_initcall(hid_plugin_##_name##_init);		\
+static void __exit hid_plugin_##_name##_exit(void)	\
+{							\
+	hid_plugin_unregister(_id, _plugin);		\
+}							\
+module_exit(hid_plugin_##_name##_exit)
+
+#endif /* __HID_PLUGIN_H */

-- 
2.55.0

[PATCH v2 8/9] HID: haptic: Convert into a loadable plugin module

[Rong Zhang]

Commit 083a4f3f3cc7 ("HID: Kconfig: Fix build error from
CONFIG_HID_HAPTIC") built hid-haptic into vmlinux or hid.ko. This is not
optimal for generic kernel images, as a platform with no multitouch
device doesn't need it anyway. It's also a bit harsh for enabling it in
distribution kernels.

With the new hid-plugin framework, hid-haptic can be easily converted
into a loadable plugin module, so that hid-multitouch only requests it
on demand.

The redaundent `default n' statement is also removed for tidiness since
we are anyway touching its context.

Suggested-by: Benjamin Tissoires <bentiss@kernel.org>
Link: https://lore.kernel.org/linux-input/auypydfkhx2eg7vp764way4batdilzc35inqda3exwzs3tk3ff@oagat6g46zto/
Signed-off-by: Rong Zhang <i@rong.moe>
---
 MAINTAINERS              |  1 +
 drivers/hid/Kconfig      |  7 +++++--
 drivers/hid/Makefile     |  2 +-
 drivers/hid/hid-haptic.c | 28 ++++++++++++++++++----------
 drivers/hid/hid-haptic.h | 29 +++++++++++++++++++++++++++--
 drivers/hid/hid-plugin.c |  3 +++
 drivers/hid/hid-plugin.h |  6 ++++++
 7 files changed, 61 insertions(+), 15 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 06c82b4a7614..b772148c031c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11909,6 +11909,7 @@ HID PLUGINS
 M:	Rong Zhang <i@rong.moe>
 L:	linux-input@vger.kernel.org
 S:	Maintained
+F:	drivers/hid/hid-haptic.*
 F:	drivers/hid/hid-plugin.*
 
 HID PULSAR DRIVER
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 604683b4a4a9..97cb4caf9d7e 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -106,9 +106,10 @@ menuconfig HID_PLUGIN
 
 	If unsure, say N.
 
+if HID_PLUGIN
+
 config HID_HAPTIC
-	bool "Haptic touchpad support"
-	default n
+	tristate "Haptic touchpad support"
 	help
 	Support for touchpads with force sensors and haptic actuators instead of a
 	traditional button.
@@ -117,6 +118,8 @@ config HID_HAPTIC
 
 	If unsure, say N.
 
+endif # HID_PLUGIN
+
 menu "Special HID drivers"
 
 config HID_A4TECH
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index b41aa3f52a94..3360347c1cb8 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -6,7 +6,7 @@ hid-y			:= hid-core.o hid-input.o hid-quirks.o
 hid-$(CONFIG_DEBUG_FS)		+= hid-debug.o
 hid-$(CONFIG_HID_PLUGIN)	+= hid-plugin.o
 
-hid-$(CONFIG_HID_HAPTIC)	+= hid-haptic.o
+obj-$(CONFIG_HID_HAPTIC)	+= hid-haptic.o
 
 obj-$(CONFIG_HID_BPF)		+= bpf/
 
diff --git a/drivers/hid/hid-haptic.c b/drivers/hid/hid-haptic.c
index a6f3ec514504..0e9708d1f3a9 100644
--- a/drivers/hid/hid-haptic.c
+++ b/drivers/hid/hid-haptic.c
@@ -315,12 +315,11 @@ static void hid_haptic_destroy(struct ff_device *ff)
 	if (hdev)
 		put_device(&hdev->dev);
 
-	module_put(THIS_MODULE);
+	hid_plugin_put(HID_PLUGIN_HAPTIC, &haptic->self_plugin->hp);
 }
 
-int hid_haptic_init(struct hid_device *hdev,
-		    struct hid_haptic_device *haptic,
-		    struct input_dev *dev)
+static int __hid_haptic_init(struct hid_device *hdev, struct hid_haptic_device *haptic,
+			     struct input_dev *dev)
 {
 	struct ff_device *ff;
 	int ret = 0, r;
@@ -328,10 +327,6 @@ int hid_haptic_init(struct hid_device *hdev,
 		.hid_usage = HID_HP_WAVEFORMSTOP & HID_USAGE,
 	};
 
-	if (!try_module_get(THIS_MODULE)) {
-		dev_err(&hdev->dev, "Failed to increase module count.\n");
-		return -ENXIO;
-	}
 	if (!get_device(&hdev->dev)) {
 		dev_err(&hdev->dev, "Failed to get hdev device.\n");
 		ret = -ENXIO;
@@ -454,7 +449,20 @@ int hid_haptic_init(struct hid_device *hdev,
 	put_device(&hdev->dev);
 exit:
 	dev_warn(&hdev->dev, "Cannot allocate haptic for %s: %d\n", hdev->name, ret);
-	module_put(THIS_MODULE);
+	hid_plugin_put(HID_PLUGIN_HAPTIC, &haptic->self_plugin->hp);
 	return ret;
 }
-EXPORT_SYMBOL_GPL(hid_haptic_init);
+
+static struct hid_plugin_haptic hid_plugin_haptic = {
+	.hp = {
+		.owner = THIS_MODULE,
+	},
+	.init = __hid_haptic_init,
+};
+module_hid_plugin(haptic, HID_PLUGIN_HAPTIC, &hid_plugin_haptic.hp);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Angela Czubak <aczubak@google.com>");
+MODULE_AUTHOR("Jonathan Denose <jdenose@google.com>");
+MODULE_AUTHOR("Rong Zhang <i@rong.moe>");
+MODULE_DESCRIPTION("HID helpers for Haptics Page");
diff --git a/drivers/hid/hid-haptic.h b/drivers/hid/hid-haptic.h
index 6196041eb4f8..a71ccee569f3 100644
--- a/drivers/hid/hid-haptic.h
+++ b/drivers/hid/hid-haptic.h
@@ -5,9 +5,12 @@
  *  Copyright (c) 2021 Angela Czubak <acz@semihalf.com>
  */
 
+#include <linux/container_of.h>
 #include <linux/err.h>
 #include <linux/hid.h>
 
+#include "hid-plugin.h"
+
 #define HID_HAPTIC_ORDINAL_WAVEFORMNONE 1
 #define HID_HAPTIC_ORDINAL_WAVEFORMSTOP 2
 
@@ -52,6 +55,14 @@ struct hid_haptic_device {
 	u16 release_ordinal;
 	struct hid_haptic_effect *effect;
 	struct hid_haptic_effect stop_effect;
+
+	struct hid_plugin_haptic *self_plugin;
+};
+
+struct hid_plugin_haptic {
+	struct hid_plugin hp;
+	int (*init)(struct hid_device *hdev, struct hid_haptic_device *haptic,
+		    struct input_dev *dev);
 };
 
 #if IS_ENABLED(CONFIG_HID_HAPTIC)
@@ -152,8 +163,22 @@ static inline int hid_haptic_input_mapping(struct hid_device *hdev,
 	return 0;
 }
 
-int hid_haptic_init(struct hid_device *hdev, struct hid_haptic_device *haptic,
-		    struct input_dev *dev);
+static inline int hid_haptic_init(struct hid_device *hdev, struct hid_haptic_device *haptic,
+				  struct input_dev *dev)
+{
+	struct hid_plugin_haptic *self_plugin;
+	struct hid_plugin *plugin;
+
+	plugin = hid_plugin_get(HID_PLUGIN_HAPTIC);
+	if (IS_ERR(plugin))
+		return PTR_ERR(plugin);
+
+	self_plugin = container_of(plugin, struct hid_plugin_haptic, hp);
+
+	haptic->self_plugin = self_plugin;
+
+	return self_plugin->init(hdev, haptic, dev);
+}
 
 static inline bool hid_haptic_input_configured(struct hid_device *hdev,
 					       struct hid_haptic_device *haptic,
diff --git a/drivers/hid/hid-plugin.c b/drivers/hid/hid-plugin.c
index a83275df038c..7357f81c3aac 100644
--- a/drivers/hid/hid-plugin.c
+++ b/drivers/hid/hid-plugin.c
@@ -18,6 +18,9 @@
 #include "hid-plugin.h"
 
 static const char * const hid_plugin_mod_names[NR_HID_PLUGINS] = {
+#if IS_MODULE(CONFIG_HID_HAPTIC)
+	[HID_PLUGIN_HAPTIC] = "hid-haptic",
+#endif
 };
 
 static struct hid_plugin __rcu *hid_plugins[NR_HID_PLUGINS];
diff --git a/drivers/hid/hid-plugin.h b/drivers/hid/hid-plugin.h
index 4038325c51a0..20d1b80a98c8 100644
--- a/drivers/hid/hid-plugin.h
+++ b/drivers/hid/hid-plugin.h
@@ -19,6 +19,12 @@
 enum hid_plugin_id {
 	HID_PLUGIN_DISABLED	= -1,
 
+#if IS_ENABLED(CONFIG_HID_HAPTIC)
+	HID_PLUGIN_HAPTIC,
+#else
+#define HID_PLUGIN_HAPTIC	HID_PLUGIN_DISABLED
+#endif
+
 	NR_HID_PLUGINS,
 };
 

-- 
2.55.0

[PATCH v2 9/9] HID: haptic: Remove stubs in favor of inlined dead code elimination

[Rong Zhang]

All stubs in hid-haptic.h can be replaced by equivalent
!hid_plugin_enabled(HID_PLUGIN_HAPTIC) checks. The compiler will
eliminate calls to all methods when the plugin is disabled.

Signed-off-by: Rong Zhang <i@rong.moe>
---
 drivers/hid/hid-haptic.h | 83 +++++++++++++++++++-----------------------------
 1 file changed, 32 insertions(+), 51 deletions(-)

diff --git a/drivers/hid/hid-haptic.h b/drivers/hid/hid-haptic.h
index a71ccee569f3..070b56189ccb 100644
--- a/drivers/hid/hid-haptic.h
+++ b/drivers/hid/hid-haptic.h
@@ -65,11 +65,18 @@ struct hid_plugin_haptic {
 		    struct input_dev *dev);
 };
 
-#if IS_ENABLED(CONFIG_HID_HAPTIC)
+static inline bool hid_haptic_enabled(void)
+{
+	return hid_plugin_enabled(HID_PLUGIN_HAPTIC);
+}
+
 static inline struct hid_haptic_device *hid_haptic_alloc(struct hid_device *hdev)
 {
 	struct hid_haptic_device *haptic;
 
+	if (!hid_haptic_enabled())
+		return ERR_PTR(-EOPNOTSUPP);
+
 	haptic = kzalloc_obj(struct hid_haptic_device);
 	if (haptic)
 		haptic->hdev = hdev;
@@ -80,6 +87,9 @@ static inline struct hid_haptic_device *hid_haptic_alloc(struct hid_device *hdev
 static inline bool hid_haptic_connected_to_input(struct hid_haptic_device *haptic,
 						 struct input_dev *input)
 {
+	if (!hid_haptic_enabled())
+		return ERR_PTR(-EOPNOTSUPP);
+
 	return haptic->input_dev == input;
 }
 
@@ -89,6 +99,9 @@ static inline void hid_haptic_feature_mapping(struct hid_device *hdev,
 {
 	u16 usage_hid;
 
+	if (!hid_haptic_enabled())
+		return;
+
 	if (usage->hid == HID_HP_AUTOTRIGGER) {
 		if (usage->usage_index >= field->report_count) {
 			dev_err(&hdev->dev, "HID_HP_AUTOTRIGGER out of range\n");
@@ -116,6 +129,9 @@ static inline void hid_haptic_feature_mapping(struct hid_device *hdev,
 static inline void hid_haptic_check_pressure_unit(struct hid_haptic_device *haptic,
 						  struct hid_input *hi, struct hid_field *field)
 {
+	if (!hid_haptic_enabled())
+		return;
+
 	if (hi->application != HID_DG_TOUCHPAD)
 		return;
 
@@ -155,6 +171,9 @@ static inline int hid_haptic_input_mapping(struct hid_device *hdev,
 					   struct hid_field *field, struct hid_usage *usage,
 					   unsigned long **bit, int *max)
 {
+	if (!hid_haptic_enabled())
+		return 0;
+
 	if (usage->hid == HID_HP_MANUALTRIGGER) {
 		haptic->manual_trigger_report = field->report;
 		/* we don't really want to map these fields */
@@ -169,6 +188,9 @@ static inline int hid_haptic_init(struct hid_device *hdev, struct hid_haptic_dev
 	struct hid_plugin_haptic *self_plugin;
 	struct hid_plugin *plugin;
 
+	if (!hid_haptic_enabled())
+		return 0;
+
 	plugin = hid_plugin_get(HID_PLUGIN_HAPTIC);
 	if (IS_ERR(plugin))
 		return PTR_ERR(plugin);
@@ -184,6 +206,9 @@ static inline bool hid_haptic_input_configured(struct hid_device *hdev,
 					       struct hid_haptic_device *haptic,
 					       struct hid_input *hi)
 {
+	if (!hid_haptic_enabled())
+		return false;
+
 	if (IS_ERR_OR_NULL(haptic->input_dev) || haptic->input_dev != hi->input ||
 	    !haptic->auto_trigger_report || !haptic->manual_trigger_report)
 		return false;
@@ -195,61 +220,17 @@ static inline bool hid_haptic_input_configured(struct hid_device *hdev,
 
 static inline void hid_haptic_pressure_reset(struct hid_haptic_device *haptic)
 {
+	if (!hid_haptic_enabled())
+		return;
+
 	haptic->pressure_sum = 0;
 }
 
 static inline void hid_haptic_pressure_increase(struct hid_haptic_device *haptic,
 						__s32 pressure)
 {
-	haptic->pressure_sum += pressure;
-}
-#else
-static inline struct hid_haptic_device *hid_haptic_alloc(struct hid_device *hdev)
-{
-	return ERR_PTR(-EOPNOTSUPP);
-}
-
-static inline bool hid_haptic_connected_to_input(struct hid_haptic_device *haptic,
-						 struct input_dev *input)
-{
-	return false;
-}
+	if (!hid_haptic_enabled())
+		return;
 
-static inline
-void hid_haptic_feature_mapping(struct hid_device *hdev,
-				struct hid_haptic_device *haptic,
-				struct hid_field *field, struct hid_usage
-				*usage)
-{}
-static inline
-void hid_haptic_check_pressure_unit(struct hid_haptic_device *haptic,
-				    struct hid_input *hi, struct hid_field *field)
-{}
-static inline
-int hid_haptic_input_mapping(struct hid_device *hdev,
-			     struct hid_haptic_device *haptic,
-			     struct hid_input *hi,
-			     struct hid_field *field, struct hid_usage *usage,
-			     unsigned long **bit, int *max)
-{
-	return 0;
-}
-static inline
-bool hid_haptic_input_configured(struct hid_device *hdev,
-				 struct hid_haptic_device *haptic, struct hid_input *hi)
-{
-	return false;
-}
-static inline
-int hid_haptic_init(struct hid_device *hdev, struct hid_haptic_device *haptic,
-		    struct input_dev *dev)
-{
-	return 0;
+	haptic->pressure_sum += pressure;
 }
-static inline
-void hid_haptic_pressure_reset(struct hid_haptic_device *haptic) {}
-static inline
-void hid_haptic_pressure_increase(struct hid_haptic_device *haptic,
-				  __s32 pressure)
-{}
-#endif

-- 
2.55.0

Re: [PATCH v2 4/9] HID: haptic, multitouch: Fix haptic device double-free and unnecessary allocation

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] HID: multitouch: Fixes trailer cites wrong commit for td->haptic
  allocation
--

commit d6cf865be3d7088ec7c4976c069b5aabfd168691
Author: Rong Zhang <i@rong.moe>

> HID: haptic, multitouch: Fix haptic device double-free and unnecessary allocation
>
> The haptic device is allocated by devm_kzalloc(), which will be freed by
> devres on HID device release. At that time, the ff core has already
> freed it on input device release previously, causing double-free.
>
> Allocate the haptic device using kzalloc_obj() to ensure its release can
> be safely delegates to the ff core.
>
> The allocation code is also moved to hid-haptic.h, so that it can be
> replaced by a stub in !CONFIG_HID_HAPTIC builds, eliminating the
> meaningless alloc-and-free sequence.
>
> Fixes: 344ff3584957 ("HID: haptic: initialize haptic device")
> Signed-off-by: Rong Zhang <i@rong.moe>

[Severity: Low]
Should the Fixes: tag reference 8d0bf7908b5a ("HID: multitouch: add
haptic multitouch support") instead, or in addition to 344ff3584957
("HID: haptic: initialize haptic device")?

344ff3584957 ("HID: haptic: initialize haptic device") only added
hid_haptic_init() in hid-haptic, whereas 8d0bf7908b5a ("HID: multitouch:
add haptic multitouch support") introduced the devm_kzalloc() allocation
of td->haptic and the hid_haptic_init() call in mt_probe().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110-hid-haptic-kmod-v2-0-b835a067cb62@rong.moe?part=4

Re: [PATCH v2 7/9] HID: Add hid-plugin framework for loadable plugin modules

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] HID: hid-plugin: missing Return section in hid_plugin_register()
  kernel-doc
--

commit ea30651fd5d31e4684e4f118c37b9199cf1158fc
Author: Rong Zhang <i@rong.moe>

HID: Add hid-plugin framework for loadable plugin modules

This commit introduces a hid-plugin framework that allows HID drivers to
request optional loadable or builtin plugin modules via RCU and module
reference counting.

Link: https://lore.kernel.org/linux-input/auypydfkhx2eg7vp764way4batdilzc35inqda3exwzs3tk3ff@oagat6g46zto/

> diff --git a/drivers/hid/hid-plugin.h b/drivers/hid/hid-plugin.h
> new file mode 100644
> index 0000000000000..4038325c51a08
> --- /dev/null
> +++ b/drivers/hid/hid-plugin.h

[ ... ]

> +/**
> + * hid_plugin_register() - Register an HID plugin.
> + * @id: Unique ID of @plugin
> + * @plugin: Pointer to the HID plugin
> + */
> +static __always_inline int hid_plugin_register(enum hid_plugin_id id, struct hid_plugin *plugin)
> +{
> +	BUILD_BUG_ON(!hid_plugin_enabled(id));
> +
> +	return __hid_plugin_register(id, plugin);
> +}

[Severity: Low]
The kernel-doc comment for hid_plugin_register() omits a Return: section,
which causes a kernel-doc warning when building with W=1.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110-hid-haptic-kmod-v2-0-b835a067cb62@rong.moe?part=7

Re: [PATCH v2 9/9] HID: haptic: Remove stubs in favor of inlined dead code elimination

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] HID: haptic: hid_haptic_connected_to_input() returns ERR_PTR
  instead of false
--

commit 3914d83f5b9ce7ec32449543dd125ffa8f399224
Author: Rong Zhang <i@rong.moe>

HID: haptic: Remove stubs in favor of inlined dead code elimination

This commit removes the #else stub implementations in
drivers/hid/hid-haptic.h and replaces them with hid_haptic_enabled()
checks inside the inline helpers.

> diff --git a/drivers/hid/hid-haptic.h b/drivers/hid/hid-haptic.h
> index a71ccee569f31..070b56189ccbe 100644
> --- a/drivers/hid/hid-haptic.h
> +++ b/drivers/hid/hid-haptic.h

[ ... ]

> @@ -80,6 +87,9 @@ static inline struct hid_haptic_device *hid_haptic_alloc(struct hid_device *hdev
>  static inline bool hid_haptic_connected_to_input(struct hid_haptic_device *haptic,
>  						 struct input_dev *input)
>  {
> +	if (!hid_haptic_enabled())
> +		return ERR_PTR(-EOPNOTSUPP);
> +
>  	return haptic->input_dev == input;
>  }

[Severity: Medium]
Should hid_haptic_connected_to_input() return false instead of
ERR_PTR(-EOPNOTSUPP) when !hid_haptic_enabled()?  Because the return type
is bool, the non-NULL error pointer converts to true, inverting the return
value of the removed stub and preventing dead-code elimination in callers
such as mt_touch_input_configured().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110-hid-haptic-kmod-v2-0-b835a067cb62@rong.moe?part=9

Re: [PATCH v2 9/9] HID: haptic: Remove stubs in favor of inlined dead code elimination

[Rong Zhang]

Hi All,

On Wed, 2026-11-11 at 00:45 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue to consider:
> - [Medium] HID: haptic: hid_haptic_connected_to_input() returns ERR_PTR
>   instead of false
> --
> 
> commit 3914d83f5b9ce7ec32449543dd125ffa8f399224
> Author: Rong Zhang <i@rong.moe>
> 
> HID: haptic: Remove stubs in favor of inlined dead code elimination
> 
> This commit removes the #else stub implementations in
> drivers/hid/hid-haptic.h and replaces them with hid_haptic_enabled()
> checks inside the inline helpers.
> 
> > diff --git a/drivers/hid/hid-haptic.h b/drivers/hid/hid-haptic.h
> > index a71ccee569f31..070b56189ccbe 100644
> > --- a/drivers/hid/hid-haptic.h
> > +++ b/drivers/hid/hid-haptic.h
> 
> [ ... ]
> 
> > @@ -80,6 +87,9 @@ static inline struct hid_haptic_device *hid_haptic_alloc(struct hid_device *hdev
> >  static inline bool hid_haptic_connected_to_input(struct hid_haptic_device *haptic,
> >  						 struct input_dev *input)
> >  {
> > +	if (!hid_haptic_enabled())
> > +		return ERR_PTR(-EOPNOTSUPP);
> > +
> >  	return haptic->input_dev == input;
> >  }
> 
> [Severity: Medium]
> Should hid_haptic_connected_to_input() return false instead of
> ERR_PTR(-EOPNOTSUPP) when !hid_haptic_enabled()?  
> 

Oops, bad copy-and-paste when rebasing...

I will fix it (and other issues found by Sashiko) in v3.

Thanks,
Rong

> Because the return type
> is bool, the non-NULL error pointer converts to true, inverting the return
> value of the removed stub and preventing dead-code elimination in callers
> such as mt_touch_input_configured().