* Re: [PATCH v4] HID: generic: add LampArray support via hid-lamparray helper
From: Werner Sembach @ 2026-03-25 10:21 UTC (permalink / raw)
To: Tim Guttzeit, Jiri Kosina, Benjamin Tissoires
Cc: linux-kernel, linux-input, Hans de Goede, linux-leds
In-Reply-To: <20260223083954.131099-1-tgu@tuxedocomputers.com>
Hi,
as Tim went on to other endeavors, I'm taking over this patch. I have yet to
test it myself, but i was already in the feedback loop while he was writing it,
so I'm not unfamiliar with the code.
Is there already some initial feedback or thoughts about this?
This patch implements the
- (Optional) Implement a generic LampArray leds subsystem driver that maps to
the single zone control and ads the use_leds_uapi sysfs switch to the virtual
HID device
sub point from the rough outline described back here
https://lore.kernel.org/all/1fb08a74-62c7-4d0c-ba5d-648e23082dcb@tuxedocomputers.com/
Best regards,
Werner
Am 23.02.26 um 09:39 schrieb Tim Guttzeit:
> Add a new hid-lamparray helper module and integrate it with the
> hid-generic driver.
>
> The helper provides basic support for devices exposing a
> Lighting/LampArray application collection (usage page 0x59) and
> registers a single-zone RGB LED representation via the LED
> subsystem.
>
> hid-generic now checks for LampArray support after hid_parse() and
> optionally registers a lamparray instance. Failures in the helper
> do not abort device probe to keep the device functional.
>
> LampArray resources are released on driver remove.
>
> Signed-off-by: Tim Guttzeit <tgu@tuxedocomputers.com>
> ---
> V1 -> V2: Fix Kconfig to avoid build errors when LEDS_CLASS_MULTICOLOR is disabled
> V2 -> V3: Squash V1 and V2 into one patch
> V3 -> V4: Restrict CONFIG_HID_LAMPARRAY to built-in configurations only
> to fix additional randconfig build errors
>
> drivers/hid/Kconfig | 10 +
> drivers/hid/Makefile | 2 +
> drivers/hid/hid-generic.c | 41 +-
> drivers/hid/hid-lamparray.c | 855 ++++++++++++++++++++++++++++++++++++
> drivers/hid/hid-lamparray.h | 91 ++++
> 5 files changed, 997 insertions(+), 2 deletions(-)
> create mode 100644 drivers/hid/hid-lamparray.c
> create mode 100644 drivers/hid/hid-lamparray.h
>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 920a64b66b25..548dc708e5cd 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -92,6 +92,16 @@ config HID_GENERIC
>
> If unsure, say Y.
>
> +config HID_LAMPARRAY
> + bool "HID LampArray helper"
> + depends on HID=y && HID_GENERIC=y && LEDS_CLASS=y && LEDS_CLASS_MULTICOLOR=y
> + default y
> + help
> + Helper for HID devices exposing a Lighting/LampArray collection.
> + Treats LampArray devices as a single-zone device and exposes a sysfs
> + interface for changing color and intensity values. Also exposes a
> + sysfs flag to be disabled e.g. by a userspace driver.
> +
> config HID_HAPTIC
> bool "Haptic touchpad support"
> default n
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index 361a7daedeb8..5a14b4b0970d 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -13,6 +13,8 @@ obj-$(CONFIG_UHID) += uhid.o
>
> obj-$(CONFIG_HID_GENERIC) += hid-generic.o
>
> +obj-$(CONFIG_HID_LAMPARRAY) += hid-lamparray.o
> +
> hid-$(CONFIG_HIDRAW) += hidraw.o
>
> hid-logitech-y := hid-lg.o
> diff --git a/drivers/hid/hid-generic.c b/drivers/hid/hid-generic.c
> index c2de916747de..57b81c86982c 100644
> --- a/drivers/hid/hid-generic.c
> +++ b/drivers/hid/hid-generic.c
> @@ -20,6 +20,7 @@
> #include <asm/byteorder.h>
>
> #include <linux/hid.h>
> +#include "hid-lamparray.h"
>
> static struct hid_driver hid_generic;
>
> @@ -31,7 +32,7 @@ static int __check_hid_generic(struct device_driver *drv, void *data)
> if (hdrv == &hid_generic)
> return 0;
>
> - return hid_match_device(hdev, hdrv) != NULL;
> + return !!hid_match_device(hdev, hdrv);
> }
>
> static bool hid_generic_match(struct hid_device *hdev,
> @@ -60,6 +61,7 @@ static int hid_generic_probe(struct hid_device *hdev,
> const struct hid_device_id *id)
> {
> int ret;
> + struct lamparray *la = NULL;
>
> hdev->quirks |= HID_QUIRK_INPUT_PER_APP;
>
> @@ -67,7 +69,31 @@ static int hid_generic_probe(struct hid_device *hdev,
> if (ret)
> return ret;
>
> - return hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> + ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> + if (ret)
> + return ret;
> +
> + /*
> + * Optional: attach LampArray support if present.
> + * Never fail probe on LampArray errors; keep device functional.
> + */
> + if (IS_ENABLED(CONFIG_HID_LAMPARRAY) && lamparray_is_supported_device(hdev)) {
> + const struct lamparray_init_state init = {
> + .r = 0xff,
> + .g = 0xff,
> + .b = 0xff,
> + .brightness = LED_FULL,
> + };
> +
> + la = lamparray_register(hdev, &init);
> + if (IS_ERR(la)) {
> + hid_warn(hdev, "LampArray init failed: %ld\n", PTR_ERR(la));
> + la = NULL;
> + }
> + }
> +
> + hid_set_drvdata(hdev, la);
> + return 0;
> }
>
> static int hid_generic_reset_resume(struct hid_device *hdev)
> @@ -78,6 +104,16 @@ static int hid_generic_reset_resume(struct hid_device *hdev)
> return 0;
> }
>
> +static void hid_generic_remove(struct hid_device *hdev)
> +{
> + struct lamparray *la = hid_get_drvdata(hdev);
> +
> + if (IS_ENABLED(CONFIG_HID_LAMPARRAY) && la)
> + lamparray_unregister(la);
> +
> + hid_hw_stop(hdev);
> +}
> +
> static const struct hid_device_id hid_table[] = {
> { HID_DEVICE(HID_BUS_ANY, HID_GROUP_ANY, HID_ANY_ID, HID_ANY_ID) },
> { }
> @@ -90,6 +126,7 @@ static struct hid_driver hid_generic = {
> .match = hid_generic_match,
> .probe = hid_generic_probe,
> .reset_resume = hid_generic_reset_resume,
> + .remove = hid_generic_remove,
> };
> module_hid_driver(hid_generic);
>
> diff --git a/drivers/hid/hid-lamparray.c b/drivers/hid/hid-lamparray.c
> new file mode 100644
> index 000000000000..5be46fff0191
> --- /dev/null
> +++ b/drivers/hid/hid-lamparray.c
> @@ -0,0 +1,855 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * hid-lamparray.c - HID LampArray helper module (single-zone RGB)
> + *
> + * Helper module for HID drivers supporting devices that expose a
> + * Lighting and Illumination (LampArray) application collection
> + * (usage page 0x59).
> + *
> + * The module provides a minimal integration with the LED subsystem
> + * and treats the device as a single zone: all lamps share one RGB
> + * value and a global brightness level. It does not implement multi-
> + * zone layouts or hardware effects.
> + *
> + *
> + * If enabled, one multicolor LED class device is registered under
> + * /sys/class/leds/<HID-ID> to expose the single-zone RGB control.
> + *
> + * The use_leds_uapi sysfs attribute is attached directly to the HID device
> + * under /sys/bus/hid/devices/<HID-ID>/use_leds_uapi.Writing 0 to use_leds_uapi
> + * unregisters the LED class device. The last state is kept cached. Writing 1
> + * registers it again and restores the cached state to hardware.
> + *
> + * State is cached as last known RGB + brightness. Setting sends a HID
> + * SET_REPORT. Getting issues a HID GET_REPORT and updates the cache on
> + * mismatch. Since the device is handled as single-zone, readback only queries
> + * lamp 0 when a lamp range is available.
> + *
> + * The module does not bind to devices on its own. Instead, a HID
> + * driver may query support via lamparray_is_supported_device() after
> + * hid_parse() and create an instance using lamparray_register().
> + *
> + * Copyright (C) 2026 Tim Guttzeit <tgu@tuxedocomputers.com>
> + */
> +
> +#include "hid-lamparray.h"
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/hid.h>
> +#include <linux/leds.h>
> +#include <linux/led-class-multicolor.h>
> +#include <linux/slab.h>
> +#include <linux/sysfs.h>
> +#include <linux/mutex.h>
> +#include <linux/bitops.h>
> +#include <linux/xarray.h>
> +
> +/* Constants */
> +
> +/* HID usages (LampArray, etc.) */
> +#define HID_LIGHTING_ILLUMINATION_USAGE_PAGE 0x0059
> +
> +#define HID_LAIP_LAMP_COUNT 0x0003
> +#define HID_LAIP_LAMP_ID 0x0021
> +#define HID_LAIP_RED_UPDATE_CHANNEL 0x0051
> +#define HID_LAIP_GREEN_UPDATE_CHANNEL 0x0052
> +#define HID_LAIP_BLUE_UPDATE_CHANNEL 0x0053
> +#define HID_LAIP_INTENSITY_UPDATE_CHANNEL 0x0054
> +#define HID_LAIP_LAMP_ID_START 0x0061
> +#define HID_LAIP_LAMP_ID_END 0x0062
> +#define HID_LAIP_AUTONOMOUS_MODE 0x0071
> +
> +#define HID_APPLICATION_COLLECTION_USAGE_TYPE 0x0001
> +
> +/* Device state */
> +
> +struct lamparray_quirks {
> + unsigned long flags;
> + int fixed_lamp_count;
> +};
> +
> +#define LAMPARRAY_QUIRK_LAMPCOUNT_FIXED BIT(0)
> +
> +struct lamparray_quirk_entry {
> + u16 vendor;
> + u16 product;
> + unsigned long flags;
> + int fixed_lamp_count;
> +};
> +
> +static const struct lamparray_quirk_entry lamparray_quirk_table[] = {
> + { 0xcafe, 0x4005, LAMPARRAY_QUIRK_LAMPCOUNT_FIXED, 12 },
> + {}
> +};
> +
> +static const struct lamparray_quirk_entry *
> +lamparray_lookup_quirks(struct hid_device *hdev)
> +{
> + const struct lamparray_quirk_entry *e;
> +
> + for (e = lamparray_quirk_table; e->vendor; e++) {
> + if (hdev->vendor == e->vendor && hdev->product == e->product)
> + return e;
> + }
> + return NULL;
> +}
> +
> +struct lamparray_device {
> + const struct lamparray_quirk_entry *quirks;
> +
> + struct hid_device *hdev;
> + struct hid_report *update_report;
> +
> + struct hid_field *red_field;
> + int red_index;
> + struct hid_field *green_field;
> + int green_index;
> + struct hid_field *blue_field;
> + int blue_index;
> + struct hid_field *intensity_field;
> + int intensity_index;
> +
> + struct hid_report *autonomous_report;
> + struct hid_field *autonomous_field;
> +
> + struct hid_field *range_start_field;
> + int range_start_index;
> +
> + struct hid_field *range_end_field;
> + int range_end_index;
> +
> + struct hid_field *lamp_count_field;
> + int lamp_count;
> + int lamp_count_index;
> +
> + struct led_classdev_mc mc_cdev;
> + struct mc_subled subleds[3];
> +
> + struct mutex lock; /* protects cached state and HID access */
> +
> + u8 last_r;
> + u8 last_g;
> + u8 last_b;
> + enum led_brightness last_brightness;
> +
> + bool use_leds_uapi;
> + bool led_registered;
> +};
> +
> +/*
> + * Opaque handle exposed to callers via the header.
> + * Keep the actual state in lamparray_device, but return a stable pointer.
> + */
> +struct lamparray {
> + struct lamparray_device ldev;
> +};
> +
> +static DEFINE_XARRAY(lamparray_by_hdev);
> +
> +/* HID helper functions */
> +
> +#ifdef DEBUG
> +static void lamparray_dump_reports(struct hid_device *hdev)
> +{
> + struct hid_report_enum *re;
> + struct hid_report *report;
> + int i, j, report_type;
> +
> + for (report_type = 0; report_type < HID_REPORT_TYPES; report_type++) {
> + re = &hdev->report_enum[report_type];
> + hid_dbg(hdev, "Dumping reports for report type %d",
> + report_type);
> + list_for_each_entry(report, &re->report_list, list) {
> + hid_dbg(hdev,
> + "lamparray: report id=%u type=%d maxfield=%u\n",
> + report->id, report->type, report->maxfield);
> +
> + for (i = 0; i < report->maxfield; i++) {
> + struct hid_field *field = report->field[i];
> +
> + for (j = 0; j < field->maxusage; j++) {
> + u32 usage = field->usage[j].hid;
> + u16 page = usage >> 16;
> + u16 id = usage & 0xFFFF;
> +
> + hid_dbg(hdev,
> + "lamparray: report %u field %d usage[%d]: page=0x%04x id=0x%04x\n",
> + report->id, i, j, page, id);
> + }
> + }
> + }
> + }
> +}
> +#else
> +static inline void lamparray_dump_reports(struct hid_device *hdev)
> +{}
> +#endif
> +
> +static int lamparray_read_lamp_count(struct lamparray_device *ldev)
> +{
> + struct hid_device *hdev = ldev->hdev;
> + struct hid_report *report;
> +
> + if (ldev->quirks &&
> + (ldev->quirks->flags & LAMPARRAY_QUIRK_LAMPCOUNT_FIXED)) {
> + ldev->lamp_count = ldev->quirks->fixed_lamp_count;
> + hid_dbg(hdev, "LampCount from quirk: %d\n", ldev->lamp_count);
> + return 0;
> + }
> + if (!ldev->lamp_count_field) {
> + hid_warn(hdev, "No LampCount field found\n");
> + return -ENODEV;
> + }
> +
> + report = ldev->lamp_count_field->report;
> +
> + if (!report) {
> + hid_warn(hdev, "LampCount field has no report\n");
> + return -ENODEV;
> + }
> + hid_hw_request(hdev, report, HID_REQ_GET_REPORT);
> + ldev->lamp_count =
> + ldev->lamp_count_field->value[ldev->lamp_count_index];
> +
> + hid_dbg(hdev, "LampCount from device: %d\n", ldev->lamp_count);
> +
> + if (ldev->lamp_count <= 0) {
> + hid_warn(hdev, "LampCount is %d (invalid)\n", ldev->lamp_count);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int lamparray_parse_update_report(struct lamparray_device *ldev)
> +{
> + struct hid_device *hdev = ldev->hdev;
> + struct hid_report_enum *re;
> + struct hid_report *report;
> + struct hid_field *field;
> + int i, j;
> +
> + lamparray_dump_reports(hdev);
> +
> + re = &hdev->report_enum[HID_FEATURE_REPORT];
> +
> + list_for_each_entry(report, &re->report_list, list) {
> + for (i = 0; i < report->maxfield; i++) {
> + field = report->field[i];
> + if (!field)
> + continue;
> +
> + if (!field->usage || !field->maxusage)
> + continue;
> +
> + for (j = 0; j < field->maxusage; j++) {
> + u32 usage = field->usage[j].hid;
> + u16 page = usage >> 16;
> + u16 id = usage & 0xffff;
> +
> + if (page !=
> + HID_LIGHTING_ILLUMINATION_USAGE_PAGE)
> + continue;
> + switch (id) {
> + case HID_LAIP_LAMP_COUNT:
> + ldev->lamp_count_field = field;
> + ldev->lamp_count_index = j;
> + break;
> + case HID_LAIP_RED_UPDATE_CHANNEL:
> + ldev->update_report = report;
> + ldev->red_field = field;
> + ldev->red_index = j;
> + break;
> + case HID_LAIP_GREEN_UPDATE_CHANNEL:
> + ldev->update_report = report;
> + ldev->green_field = field;
> + ldev->green_index = j;
> + break;
> + case HID_LAIP_BLUE_UPDATE_CHANNEL:
> + ldev->update_report = report;
> + ldev->blue_field = field;
> + ldev->blue_index = j;
> + break;
> + case HID_LAIP_INTENSITY_UPDATE_CHANNEL:
> + ldev->update_report = report;
> + ldev->intensity_field = field;
> + ldev->intensity_index = j;
> + break;
> + case HID_LAIP_LAMP_ID_START:
> + ldev->range_start_field = field;
> + ldev->range_start_index = j;
> + break;
> + case HID_LAIP_LAMP_ID_END:
> + ldev->range_end_field = field;
> + ldev->range_end_index = j;
> + break;
> + case HID_LAIP_AUTONOMOUS_MODE:
> + ldev->autonomous_field = field;
> + ldev->autonomous_report = report;
> + break;
> + default:
> + break;
> + }
> + }
> + }
> + }
> +
> + if (!ldev->update_report || !ldev->red_field || !ldev->green_field ||
> + !ldev->blue_field || !ldev->autonomous_report || !ldev->autonomous_field)
> + return -ENODEV;
> +
> + return 0;
> +}
> +
> +static int lamparray_hw_set_autonomous(struct lamparray_device *ldev,
> + bool enable)
> +{
> + struct hid_device *hdev = ldev->hdev;
> + struct hid_field *field = ldev->autonomous_field;
> + struct hid_report *report = ldev->autonomous_report;
> +
> + if (!field || !report)
> + return -ENODEV;
> +
> + field->value[0] = enable ? 1 : 0;
> +
> + hid_hw_request(hdev, report, HID_REQ_SET_REPORT);
> + return 0;
> +}
> +
> +static int lamparray_hw_set_state(struct lamparray_device *ldev, u8 r, u8 g,
> + u8 b, u8 intensity)
> +{
> + struct hid_device *hdev = ldev->hdev;
> + struct hid_report *report = ldev->update_report;
> + int i, j;
> +
> + if (!report || !ldev->red_field || !ldev->green_field ||
> + !ldev->blue_field || !ldev->intensity_field)
> + return -ENODEV;
> +
> + if (ldev->range_start_field && ldev->range_end_field) {
> + ldev->range_start_field->value[ldev->range_start_index] = 0;
> + ldev->range_end_field->value[ldev->range_end_index] = ldev->lamp_count - 1;
> + }
> +
> + for (i = 0; i < report->maxfield; i++) {
> + struct hid_field *field = report->field[i];
> +
> + if (!field || !field->usage || !field->maxusage)
> + continue;
> +
> + for (j = 0; j < field->maxusage; j++) {
> + u32 usage = field->usage[j].hid;
> + u16 page = usage >> 16;
> + u16 id = usage & 0xffff;
> +
> + if (page != HID_LIGHTING_ILLUMINATION_USAGE_PAGE)
> + continue;
> +
> + switch (id) {
> + case HID_LAIP_RED_UPDATE_CHANNEL:
> + field->value[j] = r;
> + break;
> + case HID_LAIP_GREEN_UPDATE_CHANNEL:
> + field->value[j] = g;
> + break;
> + case HID_LAIP_BLUE_UPDATE_CHANNEL:
> + field->value[j] = b;
> + break;
> + case HID_LAIP_INTENSITY_UPDATE_CHANNEL:
> + field->value[j] = intensity;
> + break;
> + default:
> + break;
> + }
> + }
> + }
> +
> + hid_hw_request(hdev, report, HID_REQ_SET_REPORT);
> + return 0;
> +}
> +
> +static int lamparray_hw_get_state(struct lamparray_device *ldev, u8 *r, u8 *g,
> + u8 *b, enum led_brightness *brightness)
> +{
> + struct hid_device *hdev = ldev->hdev;
> + struct hid_report *report = ldev->update_report;
> +
> + if (!report || !ldev->red_field || !ldev->green_field ||
> + !ldev->blue_field || !ldev->intensity_field)
> + return -ENODEV;
> +
> + if (!r || !g || !b || !brightness)
> + return -EINVAL;
> +
> + /* Single-zone: Reading lamp 0 only suffices */
> + if (ldev->range_start_field && ldev->range_end_field) {
> + ldev->range_start_field->value[ldev->range_start_index] = 0;
> + ldev->range_end_field->value[ldev->range_end_index] = 0;
> + }
> +
> + hid_hw_request(hdev, report, HID_REQ_GET_REPORT);
> +
> + *r = ldev->red_field->value[ldev->red_index];
> + *g = ldev->green_field->value[ldev->green_index];
> + *b = ldev->blue_field->value[ldev->blue_index];
> + *brightness = ldev->intensity_field->value[ldev->intensity_index];
> +
> + return 0;
> +}
> +
> +/* Helper functions */
> +
> +static int lamparray_restore_state(struct lamparray_device *ldev)
> +{
> + u8 r, g, b;
> + int ret;
> + enum led_brightness brightness;
> +
> + mutex_lock(&ldev->lock);
> +
> + if (!ldev->use_leds_uapi) {
> + mutex_unlock(&ldev->lock);
> + return 0;
> + }
> +
> + r = ldev->last_r;
> + g = ldev->last_g;
> + b = ldev->last_b;
> + brightness = ldev->last_brightness;
> +
> + ldev->mc_cdev.subled_info[0].brightness = r;
> + ldev->mc_cdev.subled_info[1].brightness = g;
> + ldev->mc_cdev.subled_info[2].brightness = b;
> + ldev->mc_cdev.led_cdev.brightness = brightness;
> +
> + mutex_unlock(&ldev->lock);
> +
> + ret = lamparray_hw_set_state(ldev, r, g, b, brightness);
> + return ret;
> +}
> +
> +/* LEDs API */
> +
> +static int lamparray_led_brightness_set(struct led_classdev *cdev,
> + enum led_brightness brightness)
> +{
> + struct led_classdev_mc *mc = lcdev_to_mccdev(cdev);
> + struct lamparray_device *ldev =
> + container_of(mc, struct lamparray_device, mc_cdev);
> + struct lamparray *la = container_of(ldev, struct lamparray, ldev);
> + u8 r, g, b;
> + int ret;
> +
> + if (!la)
> + return -ENODEV;
> + ldev = &la->ldev;
> +
> + ret = led_mc_calc_color_components(mc, brightness);
> + if (ret)
> + return ret;
> +
> + r = mc->subled_info[0].brightness;
> + g = mc->subled_info[1].brightness;
> + b = mc->subled_info[2].brightness;
> +
> + ret = lamparray_hw_set_state(ldev, r, g, b, brightness);
> + if (ret)
> + hid_err(ldev->hdev, "Failed to send LampArray update: %d\n",
> + ret);
> +
> + mutex_lock(&ldev->lock);
> + ldev->last_r = r;
> + ldev->last_g = g;
> + ldev->last_b = b;
> + ldev->last_brightness = brightness;
> + mutex_unlock(&ldev->lock);
> +
> + return 0;
> +}
> +
> +static enum led_brightness
> +lamparray_led_brightness_get(struct led_classdev *cdev)
> +{
> + struct led_classdev_mc *mc = lcdev_to_mccdev(cdev);
> + struct lamparray_device *ldev =
> + container_of(mc, struct lamparray_device, mc_cdev);
> + enum led_brightness brightness;
> + struct lamparray *la = container_of(ldev, struct lamparray, ldev);
> + u8 rr, gg, bb;
> + enum led_brightness br;
> + int ret;
> +
> + /* Default: cache (also used while registering LED classdev) */
> + mutex_lock(&ldev->lock);
> + brightness = ldev->last_brightness;
> + mutex_unlock(&ldev->lock);
> +
> + /* Only do HID readback after registration completed */
> + if (READ_ONCE(ldev->led_registered)) {
> + if (!la)
> + return brightness;
> + ldev = &la->ldev;
> +
> + ret = lamparray_hw_get_state(ldev, &rr, &gg, &bb, &br);
> + if (ret) {
> + hid_warn(ldev->hdev,
> + "Failed to read LampArray state (%d), using cached brightness %u\n",
> + ret, brightness);
> + return brightness;
> + }
> +
> + mutex_lock(&ldev->lock);
> + if (ldev->last_r != rr || ldev->last_g != gg ||
> + ldev->last_b != bb || ldev->last_brightness != br) {
> + ldev->last_r = rr;
> + ldev->last_g = gg;
> + ldev->last_b = bb;
> + ldev->last_brightness = br;
> +
> + if (ldev->led_registered && ldev->mc_cdev.subled_info) {
> + ldev->mc_cdev.subled_info[0].brightness = rr;
> + ldev->mc_cdev.subled_info[1].brightness = gg;
> + ldev->mc_cdev.subled_info[2].brightness = bb;
> + }
> + }
> + mutex_unlock(&ldev->lock);
> + return br;
> + }
> + return brightness;
> +}
> +
> +static int lamparray_register_led(struct lamparray_device *ldev)
> +{
> + struct device *dev = &ldev->hdev->dev;
> + struct led_classdev *cdev = &ldev->mc_cdev.led_cdev;
> + u8 r_i, g_i, b_i;
> + int ret;
> +
> + mutex_lock(&ldev->lock);
> +
> + if (ldev->led_registered) {
> + mutex_unlock(&ldev->lock);
> + return 0;
> + }
> +
> + if (!cdev->name) {
> + cdev->name =
> + devm_kasprintf(dev, GFP_KERNEL, "%s", dev_name(dev));
> + if (!cdev->name) {
> + mutex_unlock(&ldev->lock);
> + return -ENOMEM;
> + }
> + }
> +
> + cdev->max_brightness = 255;
> + cdev->brightness_set_blocking = lamparray_led_brightness_set;
> + cdev->brightness_get = lamparray_led_brightness_get;
> + cdev->brightness = ldev->last_brightness;
> +
> + ldev->subleds[0].color_index = LED_COLOR_ID_RED;
> + ldev->subleds[1].color_index = LED_COLOR_ID_GREEN;
> + ldev->subleds[2].color_index = LED_COLOR_ID_BLUE;
> +
> + /*
> + * Initialize the color mix (multi_intensity) from the last known HW/init
> + * state so that writing only /brightness scales the expected default color
> + * instead of white.
> + *
> + * If last_brightness is non-zero, treat last_r/g/b as per-channel
> + * brightness and normalize back to intensities (0..255).
> + * If last_brightness is zero, keep last_r/g/b as the intended mix.
> + */
> + if (ldev->last_brightness) {
> + r_i = (u8)min_t(unsigned int, 255,
> + (ldev->last_r * 255u) / ldev->last_brightness);
> + g_i = (u8)min_t(unsigned int, 255,
> + (ldev->last_g * 255u) / ldev->last_brightness);
> + b_i = (u8)min_t(unsigned int, 255,
> + (ldev->last_b * 255u) / ldev->last_brightness);
> + } else {
> + r_i = ldev->last_r;
> + g_i = ldev->last_g;
> + b_i = ldev->last_b;
> + }
> +
> + ldev->subleds[0].intensity = r_i;
> + ldev->subleds[1].intensity = g_i;
> + ldev->subleds[2].intensity = b_i;
> +
> + ldev->mc_cdev.subled_info = ldev->subleds;
> + ldev->mc_cdev.num_colors = ARRAY_SIZE(ldev->subleds);
> +
> + /* Ensure subled_info[].brightness matches intensity + brightness */
> + led_mc_calc_color_components(&ldev->mc_cdev, cdev->brightness);
> +
> + ldev->mc_cdev.subled_info = ldev->subleds;
> + ldev->mc_cdev.num_colors = ARRAY_SIZE(ldev->subleds);
> +
> + mutex_unlock(&ldev->lock);
> +
> + ret = led_classdev_multicolor_register(dev, &ldev->mc_cdev);
> + if (ret)
> + return ret;
> +
> + mutex_lock(&ldev->lock);
> + ldev->led_registered = true;
> + mutex_unlock(&ldev->lock);
> +
> + return 0;
> +}
> +
> +static void lamparray_unregister_led(struct lamparray_device *ldev)
> +{
> + bool was_registered;
> +
> + mutex_lock(&ldev->lock);
> + was_registered = ldev->led_registered;
> + ldev->led_registered = false;
> + mutex_unlock(&ldev->lock);
> +
> + if (!was_registered)
> + return;
> +
> + led_classdev_multicolor_unregister(&ldev->mc_cdev);
> +}
> +
> +/* Sysfs */
> +
> +static struct lamparray_device *
> +lamparray_ldev_from_sysfs_dev(struct device *dev)
> +{
> + struct hid_device *hdev = to_hid_device(dev);
> +
> + return xa_load(&lamparray_by_hdev, (unsigned long)hdev);
> +}
> +
> +static ssize_t use_leds_uapi_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct lamparray_device *ldev = lamparray_ldev_from_sysfs_dev(dev);
> +
> + if (!ldev)
> + return -ENODEV;
> +
> + return sysfs_emit(buf, "%d\n", ldev->use_leds_uapi);
> +}
> +
> +static ssize_t use_leds_uapi_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct lamparray_device *ldev = lamparray_ldev_from_sysfs_dev(dev);
> + int val;
> + int old_val;
> + int ret;
> +
> + if (!ldev)
> + return -ENODEV;
> +
> + ret = kstrtoint(buf, 0, &val);
> + if (ret)
> + return ret;
> +
> + if (val != 0 && val != 1)
> + return -EINVAL;
> +
> + mutex_lock(&ldev->lock);
> + old_val = ldev->use_leds_uapi;
> +
> + if (val == old_val) {
> + mutex_unlock(&ldev->lock);
> + return count;
> + }
> +
> + ldev->use_leds_uapi = val;
> + mutex_unlock(&ldev->lock);
> +
> + if (val == 1) {
> + ret = lamparray_register_led(ldev);
> + if (ret) {
> + mutex_lock(&ldev->lock);
> + ldev->use_leds_uapi = old_val;
> + mutex_unlock(&ldev->lock);
> + return ret;
> + }
> + ret = lamparray_restore_state(ldev);
> + if (ret) {
> + hid_err(ldev->hdev, "Could not restore state: %d", ret);
> + return ret;
> + }
> +
> + } else {
> + lamparray_unregister_led(ldev);
> + }
> +
> + return count;
> +}
> +static DEVICE_ATTR_RW(use_leds_uapi);
> +
> +static struct attribute *lamparray_attrs[] = {
> + &dev_attr_use_leds_uapi.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group lamparray_attr_group = {
> + .attrs = lamparray_attrs,
> +};
> +
> +static int lamparray_register_sysfs(struct lamparray_device *ldev)
> +{
> + struct device *dev = &ldev->hdev->dev;
> + int ret;
> +
> + ret = sysfs_create_group(&dev->kobj, &lamparray_attr_group);
> + if (ret)
> + hid_err(ldev->hdev,
> + "Failed to create lamparray sysfs group: %d\n", ret);
> +
> + return ret;
> +}
> +
> +static void lamparray_remove_sysfs(struct lamparray_device *ldev)
> +{
> + sysfs_remove_group(&ldev->hdev->dev.kobj, &lamparray_attr_group);
> +}
> +
> +/* Public API */
> +
> +bool lamparray_is_supported_device(struct hid_device *hdev)
> +{
> + unsigned int i;
> +
> + hid_dbg(hdev, "lamparray: walking %u collections\n",
> + hdev->maxcollection);
> +
> + for (i = 0; i < hdev->maxcollection; i++) {
> + struct hid_collection *col = &hdev->collection[i];
> + u16 page = (col->usage & HID_USAGE_PAGE) >> 16;
> + u16 code = col->usage & HID_USAGE;
> +
> + hid_dbg(hdev,
> + "lamparray: collection[%u]: type=%u level=%u usage=0x%08x page=0x%04x code=0x%04x\n",
> + i, col->type, col->level, col->usage, page, code);
> +
> + if (col->type == HID_COLLECTION_APPLICATION &&
> + page == HID_LIGHTING_ILLUMINATION_USAGE_PAGE &&
> + code == HID_APPLICATION_COLLECTION_USAGE_TYPE) {
> + return true;
> + }
> + }
> + return false;
> +}
> +EXPORT_SYMBOL_GPL(lamparray_is_supported_device);
> +
> +struct lamparray *
> +lamparray_register(struct hid_device *hdev,
> + const struct lamparray_init_state *led_init_state)
> +{
> + int ret;
> + struct lamparray *la;
> + struct lamparray_device *ldev;
> +
> + if (!hdev)
> + return ERR_PTR(-ENODEV);
> +
> + la = kzalloc(sizeof(*la), GFP_KERNEL);
> + if (!la)
> + return ERR_PTR(-ENOMEM);
> +
> + ldev = &la->ldev;
> +
> + mutex_init(&ldev->lock);
> + ldev->hdev = hdev;
> + ldev->quirks = lamparray_lookup_quirks(hdev);
> + ldev->use_leds_uapi = 1;
> + ldev->led_registered = false;
> + if (!led_init_state) {
> + ldev->last_r = 255;
> + ldev->last_g = 255;
> + ldev->last_b = 255;
> + ldev->last_brightness = LED_OFF;
> + } else {
> + ldev->last_r = led_init_state->r;
> + ldev->last_g = led_init_state->g;
> + ldev->last_b = led_init_state->b;
> + ldev->last_brightness = led_init_state->brightness;
> + }
> + ret = lamparray_parse_update_report(ldev);
> + if (ret) {
> + hid_err(hdev, "No LampArray update report found: %d\n", ret);
> + goto err_free;
> + }
> +
> + ret = lamparray_read_lamp_count(ldev);
> + if (ret) {
> + hid_err(hdev,
> + "Could not determine LampCount. This device needs a quirk for a fixed LampCount: %d\n",
> + ret);
> + goto err_unregister_led;
> + }
> +
> + ret = lamparray_register_led(ldev);
> + if (ret) {
> + hid_warn(hdev, "Failed to register LED UAPI: %d\n", ret);
> + mutex_lock(&ldev->lock);
> + ldev->use_leds_uapi = 0;
> + mutex_unlock(&ldev->lock);
> + }
> +
> + ret = xa_err(xa_store(&lamparray_by_hdev, (unsigned long)hdev, ldev,
> + GFP_KERNEL));
> + if (ret)
> + goto err_unregister_led;
> +
> + ret = lamparray_register_sysfs(ldev);
> + if (ret)
> + goto err_xa_erase;
> +
> + ret = lamparray_hw_set_autonomous(ldev, false);
> + if (ret) {
> + hid_err(hdev, "Could not disable autonomous mode: %d", ret);
> + goto err_remove_sysfs;
> + }
> +
> + hid_info(hdev, "LampArray device registered\n");
> +
> + ret = lamparray_restore_state(ldev);
> + if (ret) {
> + hid_err(hdev, "Failed to set standard state: %d", ret);
> + goto err_remove_sysfs;
> + }
> + return la;
> +
> +err_remove_sysfs:
> + lamparray_remove_sysfs(ldev);
> +err_xa_erase:
> + xa_erase(&lamparray_by_hdev, (unsigned long)hdev);
> +err_unregister_led:
> + lamparray_unregister_led(ldev);
> +err_free:
> + kfree(la);
> + return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL_GPL(lamparray_register);
> +
> +void lamparray_unregister(struct lamparray *la)
> +{
> + struct lamparray_device *ldev;
> +
> + if (!la)
> + return;
> +
> + ldev = &la->ldev;
> +
> + lamparray_unregister_led(ldev);
> + lamparray_remove_sysfs(ldev);
> + xa_erase(&lamparray_by_hdev, (unsigned long)ldev->hdev);
> +
> + kfree(la);
> +}
> +EXPORT_SYMBOL_GPL(lamparray_unregister);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("HID LampArray helper module (single-zone RGB)");
> diff --git a/drivers/hid/hid-lamparray.h b/drivers/hid/hid-lamparray.h
> new file mode 100644
> index 000000000000..ac3edd366a5b
> --- /dev/null
> +++ b/drivers/hid/hid-lamparray.h
> @@ -0,0 +1,91 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +
> +#ifndef _HID_LAMPARRAY_H
> +#define _HID_LAMPARRAY_H
> +
> +#include <linux/types.h>
> +#include <linux/leds.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +
> +struct hid_device;
> +struct lamparray;
> +
> +/*
> + * Optional initial LED state for lamparray_register().
> + * Used to define the initial state of a LampArray's LEDs.
> + */
> +struct lamparray_init_state {
> + u8 r;
> + u8 g;
> + u8 b;
> + enum led_brightness brightness;
> +};
> +
> +#if IS_ENABLED(CONFIG_HID_LAMPARRAY)
> +
> +/**
> + * lamparray_is_supported_device() - check whether a HID device supports LampArray
> + * @hdev: HID device to inspect
> + *
> + * Check whether the given HID device exposes a Lighting/LampArray application
> + * collection as defined by the HID Lighting specification.
> + *
> + * This helper can be used by HID drivers to determine whether LampArray
> + * functionality should be enabled for a device.
> + *
> + * Return: %true if LampArray support is detected, %false otherwise.
> + */
> +bool lamparray_is_supported_device(struct hid_device *hdev);
> +
> +/**
> + * lamparray_register() - initialize LampArray support for a HID device
> + * @hdev: HID device
> + * @led_init_state: Optional LED state at init specification
> + *
> + * Allocate and initialize internal LampArray state for the given HID device.
> + * The function parses required HID reports and fields and registers the
> + * associated miscdevice and sysfs attributes.
> + *
> + * If enabled, a multicolor LED class device is also registered to expose the
> + * LampArray functionality via the LED subsystem. If specified, the desired
> + * initial LED state is applied. If led_init_state is NULL, a default state is
> + * applied.
> + *
> + * Return: pointer to a LampArray handle on success, or ERR_PTR() on failure.
> + */
> +struct lamparray *lamparray_register(struct hid_device *hdev,
> + const struct lamparray_init_state *led_init_state);
> +
> +/**
> + * lamparray_unregister() - tear down LampArray support
> + * @la: LampArray handle returned by lamparray_register()
> + *
> + * Remove all resources associated with a LampArray instance.
> + *
> + * This unregisters the LED class device (if present), removes the miscdevice
> + * and sysfs interfaces and frees all internal state associated with @la.
> + */
> +void lamparray_unregister(struct lamparray *la);
> +
> +#else /* !CONFIG_HID_LAMPARRAY */
> +
> +static inline bool lamparray_is_supported_device(struct hid_device *hdev)
> +{
> + return false;
> +}
> +
> +static inline struct lamparray *
> +lamparray_register(struct hid_device *hdev,
> + const struct lamparray_init_state *led_init_state)
> +{
> + return ERR_PTR(-EOPNOTSUPP);
> +}
> +
> +static inline void lamparray_unregister(struct lamparray *la)
> +{
> +}
> +
> +#endif /* CONFIG_HID_LAMPARRAY */
> +
> +#endif /* _HID_LAMPARRAY_H */
^ permalink raw reply
* Re: [PATCH v2 00/11] Add spi-hid transport driver
From: Krzysztof Kozlowski @ 2026-03-25 8:49 UTC (permalink / raw)
To: Jingyuan Liang
Cc: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, tfiga, Jarrett Schultz,
Dmitry Antipov, Angela Czubak
In-Reply-To: <20260324-send-upstream-v2-0-521ce8afff86@chromium.org>
On Tue, Mar 24, 2026 at 06:39:33AM +0000, Jingyuan Liang wrote:
> This series picks up the spi-hid driver work originally started by
> Microsoft. The patch breakdown has been modified and the implementation
> has been refactored to address upstream feedback and testing issues. We
> are submitting this as a new series while keeping the original sign-off
> chain to reflect the history.
>
> Same as the original series, there is a change to HID documentation, some
> HID core changes to support a SPI device, the SPI HID transport driver,
> and HID over SPI Device Tree binding. We have added the HID over SPI ACPI
> support, power management, panel follower, and quirks for Ilitek touch
> controllers.
>
> Original authors: Jarrett Schultz <jaschultz@microsoft.com>,
> Dmitry Antipov <dmanti@microsoft.com>
> Link: https://lore.kernel.org/r/86b63b7b-afda-d7f4-7bfa-175085d5a8ef@gmail.com
>
> Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
> ---
> Changes in v2:
> - Fix style problems and remove unnecessary fields from the DT binding file
Style and removal? So other comments were skipped?
Please write detailed changelogs, otherwise it feels you just ignore
parts of the feedback.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH] Input: gf2k: clamp hat values to the lookup table
From: Dmitry Torokhov @ 2026-03-25 3:07 UTC (permalink / raw)
To: Pengpeng Hou; +Cc: linux-input, linux-kernel, kees
In-Reply-To: <20260323074541.93413-1-pengpeng@iscas.ac.cn>
On Mon, Mar 23, 2026 at 03:45:41PM +0800, Pengpeng Hou wrote:
> gf2k_read() decodes the hat position from a 4-bit field and uses it
> directly to index gf2k_hat_to_axis[]. The lookup table only has nine
> entries, so malformed packets can read past the end of the fixed table.
>
> Clamp invalid hat values to the neutral position before indexing the
> lookup table.
>
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
> drivers/input/joystick/gf2k.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/input/joystick/gf2k.c b/drivers/input/joystick/gf2k.c
> index 5a1cdce0bc48..78fba36285dc 100644
> --- a/drivers/input/joystick/gf2k.c
> +++ b/drivers/input/joystick/gf2k.c
> @@ -164,6 +164,8 @@ static void gf2k_read(struct gf2k *gf2k, unsigned char *data)
> input_report_abs(dev, gf2k_abs[i], GB(i*9+60,8,0) | GB(i+54,1,9));
>
> t = GB(40,4,0);
> + if (t >= ARRAY_SIZE(gf2k_hat_to_axis))
> + t = 0;
I think if "t" is too bug we should skip the loop instead of reporting
the first axis.
>
> for (i = 0; i < gf2k_hats[gf2k->id]; i++)
> input_report_abs(dev, ABS_HAT0X + i, gf2k_hat_to_axis[t][i]);
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v3] dt-bindings: input: touchscreen: edt-ft5x06: Add FocalTech FT3519
From: Dmitry Torokhov @ 2026-03-25 3:02 UTC (permalink / raw)
To: Bhushan Shah
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-input,
devicetree, linux-kernel
In-Reply-To: <20260314-edt-ft3519-v3-1-5ee91b408ed6@machinesoul.in>
On Sat, Mar 14, 2026 at 08:27:58PM +0530, Bhushan Shah wrote:
> Document FocalTech FT3519 support by adding the compatible. It's 10
> point touchscreen, which is compatible with FT3518
>
> Signed-off-by: Bhushan Shah <bhushan.shah@machinesoul.in>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2] Input: penmount: bound packet buffer indices in IRQ path
From: Pengpeng Hou @ 2026-03-25 1:46 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Pengpeng Hou, andriy.shevchenko, kees, linux-input, linux-kernel
In-Reply-To: <20260324131442.27632-1-pengpeng@iscas.ac.cn>
Hi Dmitry,
You are right, thanks for pointing this out.
I rechecked the control flow and my analysis was wrong here. If the
first byte does not match a supported packet header, the left side of
the && fails and ++pm->idx is not evaluated, so pm->idx stays at 0. If
the first byte does match, pm->idx only advances until packetsize and
is then reset to 0 in the handler.
So this does not provide a path for pm->idx to grow past the packet
buffer. My analyzer missed the short-circuiting behavior of && here.
I will drop this patch.
Best regards,
Pengpeng
^ permalink raw reply
* Re: [PATCH v2] Input: penmount: bound packet buffer indices in IRQ path
From: Dmitry Torokhov @ 2026-03-25 1:36 UTC (permalink / raw)
To: Pengpeng Hou; +Cc: andriy.shevchenko, kees, linux-input, linux-kernel
In-Reply-To: <20260324131442.27632-1-pengpeng@iscas.ac.cn>
Hi Pengpeng,
On Tue, Mar 24, 2026 at 09:14:42PM +0800, Pengpeng Hou wrote:
> pm_interrupt() stores each incoming byte into pm->data[] before the
> packet parser gets a chance to reset pm->idx. If the incoming serial
> stream never matches one of the expected packet headers, pm->idx can
> advance past the fixed receive buffer and the next IRQ will write beyond
> PM_MAX_LENGTH.
How will it advance? The handlers do:
if (byte_0_check() && pm->packetsize == ++pm->idx)
...
If we never match any of the protocols then pm->idx will never advance
past 0 (and we will keep overwriting the first byte of the packet
array).
Does your analyzer miss the "short-circuiting" nature of && operator?
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH] HID: Kysona: Add support for VXE Dragonfly R1 Pro
From: Lode Willems @ 2026-03-24 20:43 UTC (permalink / raw)
To: linux-input
Cc: jikos, bentiss, linux-kernel, Lode Willems, Dominykas Svetikas
Apparently this same protocol is used by more mice from different brands.
This patch adds support for the VXE Dragonfly R1 Pro.
Tested-by: Dominykas Svetikas <dominykas@svetikas.lt>
Signed-off-by: Lode Willems <me@lodewillems.com>
---
drivers/hid/hid-ids.h | 4 ++++
drivers/hid/hid-kysona.c | 2 ++
2 files changed, 6 insertions(+)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index afcee13bad61..772f24e80baa 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -1471,6 +1471,10 @@
#define USB_VENDOR_ID_VTL 0x0306
#define USB_DEVICE_ID_VTL_MULTITOUCH_FF3F 0xff3f
+#define USB_VENDOR_ID_VXE 0x3554
+#define USB_DEVICE_ID_VXE_DRAGONFLY_R1_PRO_DONGLE 0xf58a
+#define USB_DEVICE_ID_VXE_DRAGONFLY_R1_PRO_WIRED 0xf58c
+
#define USB_VENDOR_ID_WACOM 0x056a
#define USB_DEVICE_ID_WACOM_GRAPHIRE_BLUETOOTH 0x81
#define USB_DEVICE_ID_WACOM_INTUOS4_BLUETOOTH 0x00BD
diff --git a/drivers/hid/hid-kysona.c b/drivers/hid/hid-kysona.c
index 09bfe30d02cb..ccbd8380064e 100644
--- a/drivers/hid/hid-kysona.c
+++ b/drivers/hid/hid-kysona.c
@@ -272,6 +272,8 @@ static void kysona_remove(struct hid_device *hdev)
static const struct hid_device_id kysona_devices[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_KYSONA, USB_DEVICE_ID_KYSONA_M600_DONGLE) },
{ HID_USB_DEVICE(USB_VENDOR_ID_KYSONA, USB_DEVICE_ID_KYSONA_M600_WIRED) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_VXE, USB_DEVICE_ID_VXE_DRAGONFLY_R1_PRO_DONGLE) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_VXE, USB_DEVICE_ID_VXE_DRAGONFLY_R1_PRO_WIRED) },
{ }
};
MODULE_DEVICE_TABLE(hid, kysona_devices);
--
2.53.0
^ permalink raw reply related
* [PATCH v2] HID: ft260: validate report size and payload length in raw_event
From: Sebastian Josue Alba Vives @ 2026-03-24 20:18 UTC (permalink / raw)
To: michael.zaidman, jikos, bentiss
Cc: linux-i2c, linux-input, linux-kernel, stable,
Sebastian Josue Alba Vives
In-Reply-To: <20260324173527.11321-1-sebasjosue84@gmail.com>
ft260_raw_event() casts the raw data buffer to a
ft260_i2c_input_report struct and accesses its fields without
validating the size parameter. Since __hid_input_report() invokes
the driver's raw_event callback before hid_report_raw_event()
performs its own report-size validation, a device sending a
truncated HID report can cause out-of-bounds heap reads.
Additionally, even with a full-sized report, a corrupted
xfer->length field can cause memcpy to read beyond the report
buffer. The existing check only validates against the destination
buffer size, not the source data available in the report.
Add two checks: reject reports shorter than FT260_REPORT_MAX_LENGTH,
and verify that xfer->length does not exceed the actual data
available in the report. Log warnings to aid debugging.
Cc: stable@vger.kernel.org
Signed-off-by: Sebastian Josue Alba Vives <sebasjosue84@gmail.com>
---
drivers/hid/hid-ft260.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 333341e80..68008a423 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -1068,6 +1068,17 @@ static int ft260_raw_event(struct hid_device *hdev, struct hid_report *report,
struct ft260_device *dev = hid_get_drvdata(hdev);
struct ft260_i2c_input_report *xfer = (void *)data;
+ if (size < FT260_REPORT_MAX_LENGTH) {
+ hid_warn(hdev, "short report: %d\n", size);
+ return 0;
+ }
+
+ if (xfer->length > size - offsetof(struct ft260_i2c_input_report, data)) {
+ hid_warn(hdev, "payload %d exceeds report size %d\n",
+ xfer->length, size);
+ return 0;
+ }
+
if (xfer->report >= FT260_I2C_REPORT_MIN &&
xfer->report <= FT260_I2C_REPORT_MAX) {
ft260_dbg("i2c resp: rep %#02x len %d\n", xfer->report,
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] HID: ft260: validate report size in raw_event handler
From: Michael Zaidman @ 2026-03-24 20:00 UTC (permalink / raw)
To: Sebastian Josue Alba Vives
Cc: jikos, bentiss, linux-i2c, linux-input, linux-kernel, stable
In-Reply-To: <20260324173527.11321-1-sebasjosue84@gmail.com>
Hi Sebastian,
Thanks for the patch. The report size validation gap in ft260_raw_event()
is a valid concern - the raw_event callback is indeed invoked before
hid_report_raw_event() validates the report size, so a truncated report
from a malicious or buggy device could cause OOB reads.
However, I have a couple of comments on the proposed fix:
Please use the existing FT260_REPORT_MAX_LENGTH macro instead of the
hardcoded 64.
More importantly, the size < 64 check alone is insufficient. It prevents
accessing struct fields in a truncated buffer, but does not guard against
a corrupted xfer->length field in an otherwise full-sized report.
Consider: a device sends a valid 64-byte report (passes the size check),
but with xfer->length set to, say, 100. The data payload starts at offset 2,
so only 62 bytes are available in the buffer. The existing check at line 1077
validates against the destination buffer (dev->read_len - dev->read_idx),
not the source. If read_len is large enough (e.g., 180), the check passes,
and the memcpy reads 100 bytes from a 62-byte region - a 38-byte OOB heap
read from the source side.
A more complete fix would validate xfer->length against the actual report size:
struct ft260_i2c_input_report *xfer = (void *)data;
if (size < FT260_REPORT_MAX_LENGTH) {
hid_warn(hdev, "short report: %d\n", size);
return 0;
}
if (xfer->length > size -
offsetof(struct ft260_i2c_input_report, data)) {
hid_warn(hdev, "payload %d exceeds report size %d\n",
xfer->length, size);
return 0;
}
This catches both truncated reports and corrupted length fields.
Would you like to send a v2 addressing the above?
Thanks, Michael
On Tue, Mar 24, 2026 at 7:35 PM Sebastian Josue Alba Vives
<sebasjosue84@gmail.com> wrote:
>
> ft260_raw_event() casts the raw data buffer to a
> ft260_i2c_input_report struct and accesses its fields without
> validating the size parameter. Since __hid_input_report() invokes
> the driver's raw_event callback before hid_report_raw_event()
> performs its own report-size validation, a device sending a
> truncated HID report can cause out-of-bounds heap reads in the
> kernel.
>
> In the I2C response path, xfer->length (data[1]) is used as the
> length for a memcpy into dev->read_buf. While xfer->length is
> checked against dev->read_len, there is no check that size is large
> enough to actually contain xfer->length bytes of data starting at
> offset 2. A malicious USB device could therefore cause an OOB read
> from the kernel heap, with the result accessible from userspace via
> the I2C read interface.
>
> FT260 devices use 64-byte HID reports. Add a check at the top of
> the handler to reject any report shorter than expected, and log a
> warning to aid debugging.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Sebastian Josue Alba Vives <sebasjosue84@gmail.com>
> ---
> drivers/hid/hid-ft260.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
> index 333341e80..7ca323992 100644
> --- a/drivers/hid/hid-ft260.c
> +++ b/drivers/hid/hid-ft260.c
> @@ -1068,6 +1068,12 @@ static int ft260_raw_event(struct hid_device *hdev, struct hid_report *report,
> struct ft260_device *dev = hid_get_drvdata(hdev);
> struct ft260_i2c_input_report *xfer = (void *)data;
>
> + /* FT260 always sends 64-byte reports */
> + if (size < 64) {
> + hid_warn(hdev, "report too short: %d < 64\n", size);
> + return 0;
> + }
> +
> if (xfer->report >= FT260_I2C_REPORT_MIN &&
> xfer->report <= FT260_I2C_REPORT_MAX) {
> ft260_dbg("i2c resp: rep %#02x len %d\n", xfer->report,
> --
> 2.43.0
>
^ permalink raw reply
* Re: [PATCH v8 1/7] dt-bindings: input: syna,rmi4: Document syna,rmi4-s3706b
From: Dmitry Torokhov @ 2026-03-24 19:42 UTC (permalink / raw)
To: david
Cc: Kaustabh Chakraborty, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jason A. Donenfeld, Matthias Schiffer,
Vincent Huang, Casey Connolly, linux-input, devicetree,
linux-kernel, phone-devel, Krzysztof Kozlowski
In-Reply-To: <20260324-synaptics-rmi4-v8-1-2168d2df68f5@ixit.cz>
On Tue, Mar 24, 2026 at 08:40:34PM +0100, David Heidelberg via B4 Relay wrote:
> From: David Heidelberg <david@ixit.cz>
>
> Mostly irrelevant for authentic Synaptics touchscreens, but very important
> for applying workarounds to cheap TS knockoffs.
>
> These knockoffs work well with the downstream driver, and since the user
> has no way to distinguish them, later in this patch set, we introduce
> workarounds to ensure they function as well as possible.
>
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Signed-off-by: David Heidelberg <david@ixit.cz>
> ---
> Documentation/devicetree/bindings/input/syna,rmi4.yaml | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/input/syna,rmi4.yaml b/Documentation/devicetree/bindings/input/syna,rmi4.yaml
> index 8685ef4481f4a..fb4804ac3544d 100644
> --- a/Documentation/devicetree/bindings/input/syna,rmi4.yaml
> +++ b/Documentation/devicetree/bindings/input/syna,rmi4.yaml
> @@ -18,9 +18,14 @@ description: |
>
> properties:
> compatible:
> - enum:
> - - syna,rmi4-i2c
> - - syna,rmi4-spi
> + oneOf:
> + - enum:
> + - syna,rmi4-i2c
> + - syna,rmi4-spi
> + - items:
> + - enum:
> + - syna,rmi4-s3706b # OnePlus 6/6T
I thought that all the workarounds will be keyed off this new
compatible, but I do not see that. What am I missing?
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH v8 7/7] Input: synaptics-rmi4 - support fallback values for PDT descriptor bytes
From: David Heidelberg via B4 Relay @ 2026-03-24 19:40 UTC (permalink / raw)
To: Kaustabh Chakraborty, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jason A. Donenfeld,
Matthias Schiffer, Vincent Huang, Casey Connolly
Cc: David Heidelberg, linux-input, devicetree, linux-kernel,
phone-devel
In-Reply-To: <20260324-synaptics-rmi4-v8-0-2168d2df68f5@ixit.cz>
From: Kaustabh Chakraborty <kauschluss@disroot.org>
Some replacement displays include third-party touch ICs which do not
expose the function number and the interrupt status in its PDT entries.
OnePlus 6 (original touch IC)
rmi4_i2c 12-0020: read 6 bytes at 0x00e3: 0 (2b 22 0d 06 01 01)
OnePlus 6 (aftermarket touch IC)
rmi4_i2c 12-0020: read 6 bytes at 0x00e3: 0 (2c 23 0d 06 00 00)
Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
[codeflow adjustments, checkpatch fixes, wording]
Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
Co-developed-by: David Heidelberg <david@ixit.cz>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
drivers/input/rmi4/rmi_driver.c | 63 +++++++++++++++++++++++++++++++++++------
drivers/input/rmi4/rmi_driver.h | 3 ++
include/linux/rmi.h | 3 ++
3 files changed, 61 insertions(+), 8 deletions(-)
diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index fc84799c36e04..bb1db5bbb3abb 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -462,9 +462,10 @@ static int rmi_driver_reset_handler(struct rmi_device *rmi_dev)
return 0;
}
-static int rmi_read_pdt_entry(struct rmi_device *rmi_dev,
- struct pdt_entry *entry, u16 pdt_address)
+static int rmi_read_pdt_entry(struct rmi_device *rmi_dev, struct pdt_entry *entry,
+ struct pdt_scan_state *state, u16 pdt_address)
{
+ const struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
u8 buf[RMI_PDT_ENTRY_SIZE];
int error;
@@ -475,6 +476,21 @@ static int rmi_read_pdt_entry(struct rmi_device *rmi_dev,
return error;
}
+ if (pdata->pdt_fallback_size > state->pdt_count * RMI_OF_PDT_DESC_CELLS + 1) {
+ /* Use the description bytes from the driver */
+ buf[5] = pdata->pdt_fallback_desc[state->pdt_count * RMI_OF_PDT_DESC_CELLS];
+ buf[4] = pdata->pdt_fallback_desc[state->pdt_count * RMI_OF_PDT_DESC_CELLS + 1];
+
+ error = rmi_read_block(rmi_dev, pdt_address, buf,
+ RMI_PDT_ENTRY_SIZE - 2);
+ if (error) {
+ dev_err(&rmi_dev->dev,
+ "Read PDT entry at %#06x failed, code: %d.\n",
+ pdt_address, error);
+ return error;
+ }
+ }
+
entry->page_start = pdt_address & RMI4_PAGE_MASK;
entry->query_base_addr = buf[0];
entry->command_base_addr = buf[1];
@@ -522,6 +538,7 @@ static bool rmi_pdt_entry_is_valid(struct rmi_device *rmi_dev,
}
state->pdts[fn] = true;
+ state->pdt_count++;
return true;
}
@@ -546,7 +563,7 @@ static int rmi_scan_pdt_page(struct rmi_device *rmi_dev,
int retval;
for (addr = pdt_start; addr >= pdt_end; addr -= RMI_PDT_ENTRY_SIZE) {
- error = rmi_read_pdt_entry(rmi_dev, &pdt_entry, addr);
+ error = rmi_read_pdt_entry(rmi_dev, &pdt_entry, state, addr);
if (error)
return error;
@@ -1023,9 +1040,13 @@ static int rmi_driver_remove(struct device *dev)
}
#ifdef CONFIG_OF
-static int rmi_driver_of_probe(struct device *dev,
- struct rmi_device_platform_data *pdata)
+static const u8 rmi_s3706_fallback_pdt[] = {34, 41, 01, 01, 12, 01};
+
+static int rmi_driver_of_probe(struct rmi_device *rmi_dev,
+ struct rmi_device_platform_data *pdata)
{
+ struct device *dev = rmi_dev->xport->dev;
+ u8 buf[RMI_PDT_ENTRY_SIZE];
int retval;
retval = rmi_of_property_read_u32(dev, &pdata->reset_delay_ms,
@@ -1033,11 +1054,37 @@ static int rmi_driver_of_probe(struct device *dev,
if (retval)
return retval;
+ /*
+ * In some aftermerket touch ICs, the first PDT entry is empty and
+ * the function number register is 0. If so, the driver
+ * may have provide backup PDT entries.
+ */
+
+ retval = rmi_read_block(rmi_dev, PDT_START_SCAN_LOCATION,
+ buf, RMI_PDT_ENTRY_SIZE);
+ if (retval) {
+ dev_err(dev, "Read PDT entry at %#06x failed, code: %d.\n",
+ PDT_START_SCAN_LOCATION, retval);
+ return retval;
+ }
+
+ if (!RMI4_END_OF_PDT(buf[5]))
+ return 0;
+
+ /* List of known PDT entries per compatible. */
+ if (of_device_is_compatible(dev->of_node, "syna,rmi4-s3706b")) {
+ pdata->pdt_fallback_desc = rmi_s3706_fallback_pdt;
+ pdata->pdt_fallback_size = ARRAY_SIZE(rmi_s3706_fallback_pdt);
+ } else {
+ dev_err(dev, "First PDT entry is empty and no backup values provided.\n");
+ return -EINVAL;
+ }
+
return 0;
}
#else
-static inline int rmi_driver_of_probe(struct device *dev,
- struct rmi_device_platform_data *pdata)
+static inline int rmi_driver_of_probe(struct rmi_device *rmi_dev,
+ struct rmi_device_platform_data *pdata)
{
return -ENODEV;
}
@@ -1158,7 +1205,7 @@ static int rmi_driver_probe(struct device *dev)
pdata = rmi_get_platform_data(rmi_dev);
if (rmi_dev->xport->dev->of_node) {
- retval = rmi_driver_of_probe(rmi_dev->xport->dev, pdata);
+ retval = rmi_driver_of_probe(rmi_dev, pdata);
if (retval)
return retval;
}
diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
index 2c47373c3d177..a6a29fde49f35 100644
--- a/drivers/input/rmi4/rmi_driver.h
+++ b/drivers/input/rmi4/rmi_driver.h
@@ -31,6 +31,8 @@
#define RMI_PDT_FUNCTION_VERSION_MASK 0x60
#define RMI_PDT_INT_SOURCE_COUNT_MASK 0x07
+#define RMI_OF_PDT_DESC_CELLS 2
+
#define PDT_START_SCAN_LOCATION 0x00e9
#define PDT_END_SCAN_LOCATION 0x0005
#define RMI4_END_OF_PDT(id) ((id) == 0x00 || (id) == 0xff)
@@ -50,6 +52,7 @@ struct pdt_entry {
struct pdt_scan_state {
u8 empty_pages;
+ u8 pdt_count;
bool pdts[RMI_PDT_MAX + 1];
};
diff --git a/include/linux/rmi.h b/include/linux/rmi.h
index ab7eea01ab427..4ba2cefac8558 100644
--- a/include/linux/rmi.h
+++ b/include/linux/rmi.h
@@ -214,6 +214,9 @@ struct rmi_device_platform_data {
int reset_delay_ms;
int irq;
+ unsigned int pdt_fallback_size;
+ const u8 *pdt_fallback_desc;
+
struct rmi_device_platform_data_spi spi_data;
/* function handler pdata */
--
2.53.0
^ permalink raw reply related
* [PATCH v8 6/7] Input: synaptics-rmi4 - read product ID on aftermarket touch ICs
From: David Heidelberg via B4 Relay @ 2026-03-24 19:40 UTC (permalink / raw)
To: Kaustabh Chakraborty, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jason A. Donenfeld,
Matthias Schiffer, Vincent Huang, Casey Connolly
Cc: David Heidelberg, linux-input, devicetree, linux-kernel,
phone-devel
In-Reply-To: <20260324-synaptics-rmi4-v8-0-2168d2df68f5@ixit.cz>
From: Kaustabh Chakraborty <kauschluss@disroot.org>
Some replacement displays include third-party touch ICs which do not
report the product ID correctly unless we read directly from the
product ID register. Add a check and a fallback read to handle this.
Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
drivers/input/rmi4/rmi_f01.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
index 47be64284b25e..2278e9b6a9207 100644
--- a/drivers/input/rmi4/rmi_f01.c
+++ b/drivers/input/rmi4/rmi_f01.c
@@ -250,6 +250,20 @@ static int rmi_f01_read_properties(struct rmi_device *rmi_dev,
}
}
+ /*
+ * Some aftermarket ICs put garbage into the product id field unless
+ * we read directly from the product id register.
+ */
+ if (props->product_id[0] < 0x20) {
+ ret = rmi_read_block(rmi_dev, query_base_addr + 11,
+ props->product_id, RMI_PRODUCT_ID_LENGTH);
+ if (ret) {
+ dev_err(&rmi_dev->dev,
+ "Failed to read product id: %d\n", ret);
+ return ret;
+ }
+ }
+
return 0;
}
--
2.53.0
^ permalink raw reply related
* [PATCH v8 5/7] Input: synaptics-rmi4 - don't do unaligned reads in IRQ context
From: David Heidelberg via B4 Relay @ 2026-03-24 19:40 UTC (permalink / raw)
To: Kaustabh Chakraborty, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jason A. Donenfeld,
Matthias Schiffer, Vincent Huang, Casey Connolly
Cc: David Heidelberg, linux-input, devicetree, linux-kernel,
phone-devel
In-Reply-To: <20260324-synaptics-rmi4-v8-0-2168d2df68f5@ixit.cz>
From: Kaustabh Chakraborty <kauschluss@disroot.org>
Some replacement displays include third-party touch ICs which incur a
significant penalty (1-2 seconds) when doing certain unaligned reads.
This is enough to break functionality when it happens in the hot path,
so adjust the interrupt handler to not read from an unaligned address.
Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
drivers/input/rmi4/rmi_driver.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index dea04cf076eb4..fc84799c36e04 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -137,9 +137,14 @@ static int rmi_process_interrupt_requests(struct rmi_device *rmi_dev)
return 0;
if (!data->attn_data.data) {
+ /*
+ * Read the device status register as well and ignore it.
+ * Some aftermarket ICs have issues with interrupt requests
+ * otherwise.
+ */
error = rmi_read_block(rmi_dev,
- data->f01_container->fd.data_base_addr + 1,
- data->irq_status, data->num_of_irq_regs);
+ data->f01_container->fd.data_base_addr,
+ (u8 *)data->irq_status - 1, data->num_of_irq_regs + 1);
if (error < 0) {
dev_err(dev, "Failed to read irqs, code=%d\n", error);
return error;
@@ -1078,16 +1083,17 @@ int rmi_probe_interrupts(struct rmi_driver_data *data)
data->num_of_irq_regs = (data->irq_count + 7) / 8;
size = BITS_TO_LONGS(data->irq_count) * sizeof(unsigned long);
- data->irq_memory = devm_kcalloc(dev, size, 4, GFP_KERNEL);
+ data->irq_memory = devm_kzalloc(dev, size * 4 + 1, GFP_KERNEL);
if (!data->irq_memory) {
dev_err(dev, "Failed to allocate memory for irq masks.\n");
return -ENOMEM;
}
- data->irq_status = data->irq_memory + size * 0;
- data->fn_irq_bits = data->irq_memory + size * 1;
- data->current_irq_mask = data->irq_memory + size * 2;
- data->new_irq_mask = data->irq_memory + size * 3;
+ /* The first byte is reserved for the device status register */
+ data->irq_status = data->irq_memory + size * 0 + 1;
+ data->fn_irq_bits = data->irq_memory + size * 1 + 1;
+ data->current_irq_mask = data->irq_memory + size * 2 + 1;
+ data->new_irq_mask = data->irq_memory + size * 3 + 1;
return retval;
}
--
2.53.0
^ permalink raw reply related
* [PATCH v8 4/7] Input: synaptics-rmi4 - f55: handle zero electrode count
From: David Heidelberg via B4 Relay @ 2026-03-24 19:40 UTC (permalink / raw)
To: Kaustabh Chakraborty, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jason A. Donenfeld,
Matthias Schiffer, Vincent Huang, Casey Connolly
Cc: David Heidelberg, linux-input, devicetree, linux-kernel,
phone-devel
In-Reply-To: <20260324-synaptics-rmi4-v8-0-2168d2df68f5@ixit.cz>
From: Kaustabh Chakraborty <kauschluss@disroot.org>
Some third party ICs claim to support f55 but report an electrode count
of 0. Catch this and bail out early so that we don't confuse the i2c bus
with 0 sized reads.
Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
[simplify code, adjust wording]
Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
drivers/input/rmi4/rmi_f55.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/input/rmi4/rmi_f55.c b/drivers/input/rmi4/rmi_f55.c
index 488adaca4dd00..776c915b82e72 100644
--- a/drivers/input/rmi4/rmi_f55.c
+++ b/drivers/input/rmi4/rmi_f55.c
@@ -52,6 +52,11 @@ static int rmi_f55_detect(struct rmi_function *fn)
f55->num_rx_electrodes = f55->qry[F55_NUM_RX_OFFSET];
f55->num_tx_electrodes = f55->qry[F55_NUM_TX_OFFSET];
+ if (!f55->num_rx_electrodes || !f55->num_tx_electrodes) {
+ dev_err(&fn->dev, "%s: F55 query returned no electrodes, giving up\n",
+ __func__);
+ return -EINVAL;
+ }
f55->cfg_num_rx_electrodes = f55->num_rx_electrodes;
f55->cfg_num_tx_electrodes = f55->num_rx_electrodes;
--
2.53.0
^ permalink raw reply related
* [PATCH v8 3/7] Input: synaptics-rmi4 - f12: use hardcoded values for aftermarket touch ICs
From: David Heidelberg via B4 Relay @ 2026-03-24 19:40 UTC (permalink / raw)
To: Kaustabh Chakraborty, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jason A. Donenfeld,
Matthias Schiffer, Vincent Huang, Casey Connolly
Cc: David Heidelberg, linux-input, devicetree, linux-kernel,
phone-devel
In-Reply-To: <20260324-synaptics-rmi4-v8-0-2168d2df68f5@ixit.cz>
From: Kaustabh Chakraborty <kauschluss@disroot.org>
Some replacement displays include third-party touch ICs which are
devoid of register descriptors. Create a fake data register descriptor
for such ICs and provide hardcoded default values.
It isn't possible to reliably determine if the touch IC is original or
not, so these fallback values are offered as an alternative to the error
path when register descriptors aren't available.
Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
[changes for readability / codeflow, checkpatch fixes]
Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
drivers/input/rmi4/rmi_f12.c | 117 +++++++++++++++++++++++++++++++++----------
1 file changed, 91 insertions(+), 26 deletions(-)
diff --git a/drivers/input/rmi4/rmi_f12.c b/drivers/input/rmi4/rmi_f12.c
index 8246fe77114bb..1a103cc5f2235 100644
--- a/drivers/input/rmi4/rmi_f12.c
+++ b/drivers/input/rmi4/rmi_f12.c
@@ -218,6 +218,41 @@ static void rmi_f12_process_objects(struct f12_data *f12, u8 *data1, int size)
rmi_2d_sensor_abs_report(sensor, &sensor->objs[i], i);
}
+static void rmi_f12_set_hardcoded_desc(struct rmi_function *fn, struct f12_data *f12)
+{
+ struct rmi_2d_sensor *sensor = &f12->sensor;
+ struct rmi_register_desc_item *reg_desc;
+
+ /* We have no f12->data_reg_desc, so the pkt_size is 0, override it with
+ * a somewhat sensible default (this corresponds to 10 fingers).
+ */
+ sensor->pkt_size = 88;
+
+ /*
+ * There are no register descriptors to get these values from.
+ * We set them to high values to either be overwritten by the clip
+ * properties from devicetree, or to just not get in the way.
+ */
+ sensor->max_x = 65535;
+ sensor->max_y = 65535;
+
+ /*
+ * Create the Data1 register descriptor so that touch events
+ * can work properly.
+ */
+ reg_desc = devm_kcalloc(&fn->dev, 1,
+ sizeof(struct rmi_register_desc_item), GFP_KERNEL);
+ reg_desc->reg = 1;
+ reg_desc->reg_size = 80;
+ reg_desc->num_subpackets = 10;
+
+ f12->data1 = reg_desc;
+ f12->data1_offset = 0;
+ sensor->nbr_fingers = reg_desc->num_subpackets;
+ sensor->report_abs = 1;
+ sensor->attn_size += reg_desc->reg_size;
+}
+
static irqreturn_t rmi_f12_attention(int irq, void *ctx)
{
int retval;
@@ -338,6 +373,40 @@ static int rmi_f12_config(struct rmi_function *fn)
return 0;
}
+static int rmi_f12_sensor_init(struct rmi_function *fn, struct f12_data *f12)
+{
+ struct rmi_2d_sensor *sensor = &f12->sensor;
+
+ sensor->fn = fn;
+ f12->data_addr = fn->fd.data_base_addr;
+
+ /* On quirky devices that don't have a data_reg_desc we hardcode the packet
+ * in rmi_f12_set_hardcoded_desc(). Make sure not to set it to 0 here.
+ */
+ if (!sensor->pkt_size)
+ sensor->pkt_size = rmi_register_desc_calc_size(&f12->data_reg_desc);
+
+ sensor->axis_align =
+ f12->sensor_pdata.axis_align;
+
+ sensor->x_mm = f12->sensor_pdata.x_mm;
+ sensor->y_mm = f12->sensor_pdata.y_mm;
+ sensor->dribble = f12->sensor_pdata.dribble;
+
+ if (sensor->sensor_type == rmi_sensor_default)
+ sensor->sensor_type =
+ f12->sensor_pdata.sensor_type;
+
+ rmi_dbg(RMI_DEBUG_FN, &fn->dev, "%s: data packet size: %d\n", __func__,
+ sensor->pkt_size);
+
+ sensor->data_pkt = devm_kzalloc(&fn->dev, sensor->pkt_size, GFP_KERNEL);
+ if (!sensor->data_pkt)
+ return -ENOMEM;
+
+ return 0;
+}
+
static int rmi_f12_probe(struct rmi_function *fn)
{
struct f12_data *f12;
@@ -351,6 +420,7 @@ static int rmi_f12_probe(struct rmi_function *fn)
struct rmi_driver_data *drvdata = dev_get_drvdata(&rmi_dev->dev);
u16 data_offset = 0;
int mask_size;
+ bool hardcoded_desc_quirk = false;
rmi_dbg(RMI_DEBUG_FN, &fn->dev, "%s\n", __func__);
@@ -365,9 +435,9 @@ static int rmi_f12_probe(struct rmi_function *fn)
++query_addr;
if (!(buf & BIT(0))) {
- dev_err(&fn->dev,
- "Behavior of F12 without register descriptors is undefined.\n");
- return -ENODEV;
+ rmi_dbg(RMI_DEBUG_FN, &fn->dev,
+ "No register descriptors defined for F12, using fallback\n");
+ hardcoded_desc_quirk = true;
}
f12 = devm_kzalloc(&fn->dev, sizeof(struct f12_data) + mask_size * 2,
@@ -375,6 +445,8 @@ static int rmi_f12_probe(struct rmi_function *fn)
if (!f12)
return -ENOMEM;
+ dev_set_drvdata(&fn->dev, f12);
+
f12->abs_mask = (unsigned long *)((char *)f12
+ sizeof(struct f12_data));
f12->rel_mask = (unsigned long *)((char *)f12
@@ -393,6 +465,18 @@ static int rmi_f12_probe(struct rmi_function *fn)
f12->sensor_pdata = pdata->sensor_pdata;
}
+ sensor = &f12->sensor;
+
+ if (hardcoded_desc_quirk) {
+ rmi_f12_set_hardcoded_desc(fn, f12);
+
+ ret = rmi_f12_sensor_init(fn, f12);
+ if (ret)
+ return ret;
+
+ goto skip_register_desc;
+ }
+
ret = rmi_read_register_desc(rmi_dev, query_addr,
&f12->query_reg_desc);
if (ret) {
@@ -423,29 +507,9 @@ static int rmi_f12_probe(struct rmi_function *fn)
}
query_addr += 3;
- sensor = &f12->sensor;
- sensor->fn = fn;
- f12->data_addr = fn->fd.data_base_addr;
- sensor->pkt_size = rmi_register_desc_calc_size(&f12->data_reg_desc);
-
- sensor->axis_align =
- f12->sensor_pdata.axis_align;
-
- sensor->x_mm = f12->sensor_pdata.x_mm;
- sensor->y_mm = f12->sensor_pdata.y_mm;
- sensor->dribble = f12->sensor_pdata.dribble;
-
- if (sensor->sensor_type == rmi_sensor_default)
- sensor->sensor_type =
- f12->sensor_pdata.sensor_type;
-
- rmi_dbg(RMI_DEBUG_FN, &fn->dev, "%s: data packet size: %d\n", __func__,
- sensor->pkt_size);
- sensor->data_pkt = devm_kzalloc(&fn->dev, sensor->pkt_size, GFP_KERNEL);
- if (!sensor->data_pkt)
- return -ENOMEM;
-
- dev_set_drvdata(&fn->dev, f12);
+ ret = rmi_f12_sensor_init(fn, f12);
+ if (ret)
+ return ret;
ret = rmi_f12_read_sensor_tuning(f12);
if (ret)
@@ -543,6 +607,7 @@ static int rmi_f12_probe(struct rmi_function *fn)
data_offset += item->reg_size;
}
+skip_register_desc:
/* allocate the in-kernel tracking buffers */
sensor->tracking_pos = devm_kcalloc(&fn->dev,
sensor->nbr_fingers, sizeof(struct input_mt_pos),
--
2.53.0
^ permalink raw reply related
* [PATCH v8 1/7] dt-bindings: input: syna,rmi4: Document syna,rmi4-s3706b
From: David Heidelberg via B4 Relay @ 2026-03-24 19:40 UTC (permalink / raw)
To: Kaustabh Chakraborty, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jason A. Donenfeld,
Matthias Schiffer, Vincent Huang, Casey Connolly
Cc: David Heidelberg, linux-input, devicetree, linux-kernel,
phone-devel, Krzysztof Kozlowski
In-Reply-To: <20260324-synaptics-rmi4-v8-0-2168d2df68f5@ixit.cz>
From: David Heidelberg <david@ixit.cz>
Mostly irrelevant for authentic Synaptics touchscreens, but very important
for applying workarounds to cheap TS knockoffs.
These knockoffs work well with the downstream driver, and since the user
has no way to distinguish them, later in this patch set, we introduce
workarounds to ensure they function as well as possible.
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
Documentation/devicetree/bindings/input/syna,rmi4.yaml | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/input/syna,rmi4.yaml b/Documentation/devicetree/bindings/input/syna,rmi4.yaml
index 8685ef4481f4a..fb4804ac3544d 100644
--- a/Documentation/devicetree/bindings/input/syna,rmi4.yaml
+++ b/Documentation/devicetree/bindings/input/syna,rmi4.yaml
@@ -18,9 +18,14 @@ description: |
properties:
compatible:
- enum:
- - syna,rmi4-i2c
- - syna,rmi4-spi
+ oneOf:
+ - enum:
+ - syna,rmi4-i2c
+ - syna,rmi4-spi
+ - items:
+ - enum:
+ - syna,rmi4-s3706b # OnePlus 6/6T
+ - const: syna,rmi4-i2c
reg:
maxItems: 1
--
2.53.0
^ permalink raw reply related
* [PATCH v8 2/7] Input: synaptics-rmi4 - handle duplicate/unknown PDT entries
From: David Heidelberg via B4 Relay @ 2026-03-24 19:40 UTC (permalink / raw)
To: Kaustabh Chakraborty, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jason A. Donenfeld,
Matthias Schiffer, Vincent Huang, Casey Connolly
Cc: David Heidelberg, linux-input, devicetree, linux-kernel,
phone-devel
In-Reply-To: <20260324-synaptics-rmi4-v8-0-2168d2df68f5@ixit.cz>
From: Casey Connolly <casey.connolly@linaro.org>
Some third party rmi4-compatible ICs don't expose their PDT entries
very well. Add a few checks to skip duplicate entries as well as entries
for unsupported functions.
This is required to support some phones with third party displays.
Validated on a stock OnePlus 6T (original parts):
manufacturer: Synaptics, product: S3706B, fw id: 2852315
Co-developed-by: Kaustabh Chakraborty <kauschluss@disroot.org>
Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
Co-developed-by: David Heidelberg <david@ixit.cz>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
drivers/input/rmi4/rmi_driver.c | 41 +++++++++++++++++++++++++++++++++++------
drivers/input/rmi4/rmi_driver.h | 7 +++++++
2 files changed, 42 insertions(+), 6 deletions(-)
diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index ccd9338a44dbe..dea04cf076eb4 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -494,12 +494,38 @@ static void rmi_driver_copy_pdt_to_fd(const struct pdt_entry *pdt,
fd->function_version = pdt->function_version;
}
+static bool rmi_pdt_entry_is_valid(struct rmi_device *rmi_dev,
+ struct pdt_scan_state *state, u8 fn)
+{
+ switch (fn) {
+ case 0x01:
+ case 0x03:
+ case 0x11:
+ case 0x12:
+ case 0x30:
+ case 0x34:
+ case 0x3a:
+ case 0x54:
+ case 0x55:
+ if (state->pdts[fn] == true)
+ return false;
+ break;
+ default:
+ rmi_dbg(RMI_DEBUG_CORE, &rmi_dev->dev,
+ "PDT has unknown function number %#02x\n", fn);
+ return false;
+ }
+
+ state->pdts[fn] = true;
+ return true;
+}
+
#define RMI_SCAN_CONTINUE 0
#define RMI_SCAN_DONE 1
static int rmi_scan_pdt_page(struct rmi_device *rmi_dev,
int page,
- int *empty_pages,
+ struct pdt_scan_state *state,
void *ctx,
int (*callback)(struct rmi_device *rmi_dev,
void *ctx,
@@ -522,6 +548,9 @@ static int rmi_scan_pdt_page(struct rmi_device *rmi_dev,
if (RMI4_END_OF_PDT(pdt_entry.function_number))
break;
+ if (!rmi_pdt_entry_is_valid(rmi_dev, state, pdt_entry.function_number))
+ continue;
+
retval = callback(rmi_dev, ctx, &pdt_entry);
if (retval != RMI_SCAN_CONTINUE)
return retval;
@@ -532,11 +561,11 @@ static int rmi_scan_pdt_page(struct rmi_device *rmi_dev,
* or more is found, stop scanning.
*/
if (addr == pdt_start)
- ++*empty_pages;
+ ++state->empty_pages;
else
- *empty_pages = 0;
+ state->empty_pages = 0;
- return (data->bootloader_mode || *empty_pages >= 2) ?
+ return (data->bootloader_mode || state->empty_pages >= 2) ?
RMI_SCAN_DONE : RMI_SCAN_CONTINUE;
}
@@ -545,11 +574,11 @@ int rmi_scan_pdt(struct rmi_device *rmi_dev, void *ctx,
void *ctx, const struct pdt_entry *entry))
{
int page;
- int empty_pages = 0;
+ struct pdt_scan_state state = {0, 0, {0}};
int retval = RMI_SCAN_DONE;
for (page = 0; page <= RMI4_MAX_PAGE; page++) {
- retval = rmi_scan_pdt_page(rmi_dev, page, &empty_pages,
+ retval = rmi_scan_pdt_page(rmi_dev, page, &state,
ctx, callback);
if (retval != RMI_SCAN_CONTINUE)
break;
diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
index e84495caab151..2c47373c3d177 100644
--- a/drivers/input/rmi4/rmi_driver.h
+++ b/drivers/input/rmi4/rmi_driver.h
@@ -46,6 +46,13 @@ struct pdt_entry {
u8 function_number;
};
+#define RMI_PDT_MAX 0x55
+
+struct pdt_scan_state {
+ u8 empty_pages;
+ bool pdts[RMI_PDT_MAX + 1];
+};
+
#define RMI_REG_DESC_PRESENSE_BITS (32 * BITS_PER_BYTE)
#define RMI_REG_DESC_SUBPACKET_BITS (37 * BITS_PER_BYTE)
--
2.53.0
^ permalink raw reply related
* [PATCH v8 0/7] Input: synaptics-rmi4 - add quirks for third party touchscreen controllers
From: David Heidelberg via B4 Relay @ 2026-03-24 19:40 UTC (permalink / raw)
To: Kaustabh Chakraborty, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jason A. Donenfeld,
Matthias Schiffer, Vincent Huang, Casey Connolly
Cc: David Heidelberg, linux-input, devicetree, linux-kernel,
phone-devel, Krzysztof Kozlowski
With the growing popularity of running upstream Linux on mobile devices,
we're beginning to run into more and more edgecases. The OnePlus 6 is a
fairly well supported 2018 era smartphone, selling over a million units
in it's first 22 days. With this level of popularity, it's almost
inevitable that we get third party replacement displays, and as a
result, replacement touchscreen controllers.
The OnePlus 6 shipped with an extremely usecase specific touchscreen
driver, it implemented only the bare minimum parts of the highly generic
rmi4 protocol, instead hardcoding most of the register addresses.
As a result, the third party touchscreen controllers that are often
found in replacement screens, implement only the registers that the
downstream driver reads from. They additionally have other restrictions
such as heavy penalties on unaligned reads.
This series attempts to implement the necessary workaround to support
some of these chips with the rmi4 driver. Although it's worth noting
that at the time of writing there are other unofficial controllers in
the wild that don't work even with these patches.
We have been shipping these patches in postmarketOS for the last several
years, and they are known to not cause any regressions on the OnePlus
6/6T (with the official Synaptics controller), however I don't own any
other rmi4 hardware to further validate this.
The series is also available (until merged) at
https://codeberg.org/sdm845/linux/commits/b4/synaptics-rmi4
---
Changes in v8:
- The pdt_scan_state->pdts array should actually be of size
(RMI_PDT_MAX+1). (Casey)
- Move the pdt_count introduction to the relevant patch. (Casey)
- Link to v7: https://lore.kernel.org/r/20260320-synaptics-rmi4-v7-0-379360de18d0@ixit.cz
Changes in v7:
- Rebased on top of next-20260320, no other changes.
- Link to v6: https://lore.kernel.org/r/20251113-synaptics-rmi4-v6-0-d9836afab801@ixit.cz
Changes in v6:
- Rebased on top of next-20251113.
- No other change since the Rob Herring comment.
- Link to v5: https://lore.kernel.org/r/20250410-synaptics-rmi4-v5-0-b41bb90f78b9@ixit.cz
Changes in v5:
- Removed -i2c suffix from rmi4-s3706b-i2c (Krzysztof).
- Link to v4: https://lore.kernel.org/r/20250402-synaptics-rmi4-v4-0-1bb95959e564@ixit.cz
Changes in v4:
- Replaced patch "dt-bindings: input: syna,rmi4: document syna,pdt-fallback-desc"
with patch documenting specific touchscreen model used in OnePlus 6 and 6T.
- Fixed zero electrode return code (Dmitry).
- Switched the duplicate detection algo to bitmap (Dmitry).
- Optimized rmi_device_platform_data struct to avoid unnecessary
padding.
- Changed fallback_size from int to unsigned int.
- Changed SoB from nickname and old address (methanal <baclofen@tuta.io>) to
Kaustabh Chakraborty <kauschluss@disroot.org>.
Verified ownership through the sdm845 chatroom on Matrix.
- Link to v3: https://lore.kernel.org/r/20250308-synaptics-rmi4-v3-0-215d3e7289a2@ixit.cz
Changes in v3:
- reworded dt-bindings property description
- fixed the rmi_driver_of_probe definition for non device-tree builds.
- fixed some indentation issues reported by checkpatch
- change rmi_pdt_entry_is_valid() variable to unsigned
- Link to v2: https://lore.kernel.org/all/20230929-caleb-rmi4-quirks-v2-0-b227ac498d88@linaro.org
Changes in v2:
- Improve dt-bindings patch (thanks Rob)
- Add missing cast in patch 5 to fix the pointer arithmetic
- Link to v1: https://lore.kernel.org/r/20230929-caleb-rmi4-quirks-v1-0-cc3c703f022d@linaro.org
---
Casey Connolly (1):
Input: synaptics-rmi4 - handle duplicate/unknown PDT entries
David Heidelberg (1):
dt-bindings: input: syna,rmi4: Document syna,rmi4-s3706b
Kaustabh Chakraborty (5):
Input: synaptics-rmi4 - f12: use hardcoded values for aftermarket touch ICs
Input: synaptics-rmi4 - f55: handle zero electrode count
Input: synaptics-rmi4 - don't do unaligned reads in IRQ context
Input: synaptics-rmi4 - read product ID on aftermarket touch ICs
Input: synaptics-rmi4 - support fallback values for PDT descriptor bytes
.../devicetree/bindings/input/syna,rmi4.yaml | 11 +-
drivers/input/rmi4/rmi_driver.c | 124 +++++++++++++++++----
drivers/input/rmi4/rmi_driver.h | 10 ++
drivers/input/rmi4/rmi_f01.c | 14 +++
drivers/input/rmi4/rmi_f12.c | 117 ++++++++++++++-----
drivers/input/rmi4/rmi_f55.c | 5 +
include/linux/rmi.h | 3 +
7 files changed, 234 insertions(+), 50 deletions(-)
---
base-commit: 85964cdcad0fac9a0eb7b87a0f9d88cc074b854c
change-id: 20250308-synaptics-rmi4-c832b2f73ceb
Best regards,
--
David Heidelberg <david@ixit.cz>
^ permalink raw reply
* [PATCH] HID: hid-lenovo-go: fix LEDS dependencies
From: Arnd Bergmann @ 2026-03-24 19:20 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Derek J. Clark, Mark Pearson,
Mario Limonciello, Ethan Tidmore
Cc: Arnd Bergmann, Aditya Garg, Jonathan Denose, Geert Uytterhoeven,
linux-input, linux-kernel
From: Arnd Bergmann <arnd@arndb.de>
The newly added hid-lenovo-go and hid-lenovo-go-s drivers attempt to
'select LEDS_CLASS', which has a dependency on NEW_LEDS, causing a build
failure when NEW_LEDS itself it disabled:
WARNING: unmet direct dependencies detected for LEDS_CLASS
Depends on [n]: NEW_LEDS [=n]
Selected by [m]:
- HID_LENOVO_GO [=m] && HID_SUPPORT [=y] && HID [=m] && USB_HID [=m]
- HID_LENOVO_GO_S [=m] && HID_SUPPORT [=y] && HID [=m] && USB_HID [=m]
WARNING: unmet direct dependencies detected for LEDS_CLASS_MULTICOLOR
Depends on [n]: NEW_LEDS [=n] && LEDS_CLASS [=m]
Selected by [m]:
- HID_LENOVO_GO [=m] && HID_SUPPORT [=y] && HID [=m] && USB_HID [=m]
- HID_LENOVO_GO_S [=m] && HID_SUPPORT [=y] && HID [=m] && USB_HID [=m]
ERROR: modpost: "led_set_brightness_nopm" [drivers/leds/led-class.ko] undefined!
ERROR: modpost: "led_set_brightness" [drivers/leds/led-class.ko] undefined!
ERROR: modpost: "led_get_color_name" [drivers/leds/led-class-multicolor.ko] undefined!
ERROR: modpost: "led_set_brightness" [drivers/leds/led-class-multicolor.ko] undefined!
Device drivers generally should not select other subsystems like this, as
that tends to cause dependency problems including loops in the dependency
graph.
Change these two and the older hid-lenovo driver to behave the same way as all
other HID drivers and use 'depends on LEDS_CLASS' or 'depends on LEDS_CLASS_MULTICOLOR'
instead, which itself has NEW_LEDS and LEDS_CLASS as dependencies.
Fixes: a23f3497bf20 ("HID: hid-lenovo-go-s: Add Lenovo Legion Go S Series HID Driver")
Fixes: d69ccfcbc955 ("HID: hid-lenovo-go: Add Lenovo Legion Go Series HID Driver")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/hid/Kconfig | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 10c12d8e6557..f658ed0168ea 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -610,8 +610,7 @@ config HID_LED
config HID_LENOVO
tristate "Lenovo / Thinkpad devices"
- select NEW_LEDS
- select LEDS_CLASS
+ depends on LEDS_CLASS
help
Support for IBM/Lenovo devices that are not fully compliant with HID standard.
@@ -626,8 +625,7 @@ config HID_LENOVO
config HID_LENOVO_GO
tristate "HID Driver for Lenovo Legion Go Series Controllers"
depends on USB_HID
- select LEDS_CLASS
- select LEDS_CLASS_MULTICOLOR
+ depends on LEDS_CLASS_MULTICOLOR
help
Support for Lenovo Legion Go devices with detachable controllers.
@@ -638,8 +636,7 @@ config HID_LENOVO_GO
config HID_LENOVO_GO_S
tristate "HID Driver for Lenovo Legion Go S Controller"
depends on USB_HID
- select LEDS_CLASS
- select LEDS_CLASS_MULTICOLOR
+ depends on LEDS_CLASS_MULTICOLOR
help
Support for Lenovo Legion Go S Handheld Console Controller.
--
2.39.5
^ permalink raw reply related
* [PATCH] HID: ft260: validate report size in raw_event handler
From: Sebastian Josue Alba Vives @ 2026-03-24 17:35 UTC (permalink / raw)
To: michael.zaidman, jikos, bentiss
Cc: linux-i2c, linux-input, linux-kernel, stable,
Sebastian Josue Alba Vives
ft260_raw_event() casts the raw data buffer to a
ft260_i2c_input_report struct and accesses its fields without
validating the size parameter. Since __hid_input_report() invokes
the driver's raw_event callback before hid_report_raw_event()
performs its own report-size validation, a device sending a
truncated HID report can cause out-of-bounds heap reads in the
kernel.
In the I2C response path, xfer->length (data[1]) is used as the
length for a memcpy into dev->read_buf. While xfer->length is
checked against dev->read_len, there is no check that size is large
enough to actually contain xfer->length bytes of data starting at
offset 2. A malicious USB device could therefore cause an OOB read
from the kernel heap, with the result accessible from userspace via
the I2C read interface.
FT260 devices use 64-byte HID reports. Add a check at the top of
the handler to reject any report shorter than expected, and log a
warning to aid debugging.
Cc: stable@vger.kernel.org
Signed-off-by: Sebastian Josue Alba Vives <sebasjosue84@gmail.com>
---
drivers/hid/hid-ft260.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 333341e80..7ca323992 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -1068,6 +1068,12 @@ static int ft260_raw_event(struct hid_device *hdev, struct hid_report *report,
struct ft260_device *dev = hid_get_drvdata(hdev);
struct ft260_i2c_input_report *xfer = (void *)data;
+ /* FT260 always sends 64-byte reports */
+ if (size < 64) {
+ hid_warn(hdev, "report too short: %d < 64\n", size);
+ return 0;
+ }
+
if (xfer->report >= FT260_I2C_REPORT_MIN &&
xfer->report <= FT260_I2C_REPORT_MAX) {
ft260_dbg("i2c resp: rep %#02x len %d\n", xfer->report,
--
2.43.0
^ permalink raw reply related
* [PATCH v2] HID: mcp2221: validate report size in raw_event handler
From: Sebastian Josue Alba Vives @ 2026-03-24 17:06 UTC (permalink / raw)
To: gupt21, jikos, bentiss
Cc: linux-i2c, linux-input, linux-kernel, stable,
Sebastian Josue Alba Vives
In-Reply-To: <20260324062403.341855-1-sebasjosue84@gmail.com>
mcp2221_raw_event() accesses the data buffer at offsets up to 55
without validating the size parameter. Since __hid_input_report()
invokes the driver's raw_event callback before
hid_report_raw_event() performs its own report-size validation, a
device sending a truncated HID report can cause out-of-bounds heap
reads in the kernel.
The most critical access is the memcpy from data[50] into
mcp->adc_values (6 bytes) when CONFIG_IIO is reachable. Other
unchecked accesses include data[20] and a memcpy at data[22].
Additionally, a memcpy with device-controlled length (data[3],
up to 60 bytes) from data[4] does not verify that size is large
enough to cover the copy.
MCP2221 devices use 64-byte HID reports. Add a check at the top of
the handler to reject any report shorter than expected, and log a
warning to aid debugging.
Cc: stable@vger.kernel.org
Signed-off-by: Sebastian Josue Alba Vives <sebasjosue84@gmail.com>
---
drivers/hid/hid-mcp2221.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/hid/hid-mcp2221.c b/drivers/hid/hid-mcp2221.c
index ef3b5c77c..770c305d8 100644
--- a/drivers/hid/hid-mcp2221.c
+++ b/drivers/hid/hid-mcp2221.c
@@ -850,6 +850,11 @@ static int mcp2221_raw_event(struct hid_device *hdev,
{
u8 *buf;
struct mcp2221 *mcp = hid_get_drvdata(hdev);
+ /* MCP2221 always sends 64-byte reports */
+ if (size < 64) {
+ hid_warn(hdev, "report too short: %d < 64\n", size);
+ return 0;
+ }
switch (data[0]) {
--
2.43.0
^ permalink raw reply related
* [PATCH v2 2/2] HID: logitech-dj: Prevent REPORT_ID_DJ_SHORT related user initiated OOB write
From: Lee Jones @ 2026-03-24 14:36 UTC (permalink / raw)
To: lee, Filipe Laíns, Jiri Kosina, Benjamin Tissoires,
linux-input, linux-kernel
In-Reply-To: <20260324143651.342273-1-lee@kernel.org>
logi_dj_recv_send_report() assumes that all incoming REPORT_ID_DJ_SHORT
reports are 14 Bytes (DJREPORT_SHORT_LENGTH - 1) long. It uses that
assumption to load the associated field's 'value' array with 14 Bytes of
data. However, if a malicious user only sends say 1 Byte of data,
'report_count' will be 1 and only 1 Byte of memory will be allocated to
the 'value' Byte array. When we come to populate 'value[1-13]' we will
experience an OOB write.
Signed-off-by: Lee Jones <lee@kernel.org>
---
v1 => v2: Move handling to .probe()
drivers/hid/hid-logitech-dj.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 32139b2561c0..a8082199d13d 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -1859,6 +1859,7 @@ static int logi_dj_probe(struct hid_device *hdev,
const struct hid_device_id *id)
{
struct hid_report_enum *input_report_enum;
+ struct hid_report_enum *output_report_enum;
struct hid_report *rep;
struct dj_receiver_dev *djrcv_dev;
struct usb_interface *intf;
@@ -1903,6 +1904,15 @@ static int logi_dj_probe(struct hid_device *hdev,
}
}
+ output_report_enum = &hdev->report_enum[HID_OUTPUT_REPORT];
+ rep = output_report_enum->report_id_hash[REPORT_ID_DJ_SHORT];
+
+ if (rep->maxfield < 1 || rep->field[0]->report_count != DJREPORT_SHORT_LENGTH - 1) {
+ hid_err(hdev, "Expected size of DJ short report is %d, but got %d",
+ DJREPORT_SHORT_LENGTH - 1, rep->field[0]->report_count);
+ return -EINVAL;
+ }
+
input_report_enum = &hdev->report_enum[HID_INPUT_REPORT];
/* no input reports, bail out */
--
2.53.0.983.g0bb29b3bc5-goog
^ permalink raw reply related
* [PATCH v2 1/2] HID: logitech-dj: Standardise hid_report_enum variable nomenclature
From: Lee Jones @ 2026-03-24 14:36 UTC (permalink / raw)
To: lee, Filipe Laíns, Jiri Kosina, Benjamin Tissoires,
linux-input, linux-kernel
Since we will need to differentiate between the two report_enum types
soon, let's unify the naming conventions now to save confusion and/or
unnecessary/unrelated changes in upcoming commits.
{input,output}_report_enum is used in other places to let's conform.
Signed-off-by: Lee Jones <lee@kernel.org>
---
v1 => v2: New patch
drivers/hid/hid-logitech-dj.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 44b716697510..32139b2561c0 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -1858,7 +1858,7 @@ static int logi_dj_raw_event(struct hid_device *hdev,
static int logi_dj_probe(struct hid_device *hdev,
const struct hid_device_id *id)
{
- struct hid_report_enum *rep_enum;
+ struct hid_report_enum *input_report_enum;
struct hid_report *rep;
struct dj_receiver_dev *djrcv_dev;
struct usb_interface *intf;
@@ -1903,10 +1903,10 @@ static int logi_dj_probe(struct hid_device *hdev,
}
}
- rep_enum = &hdev->report_enum[HID_INPUT_REPORT];
+ input_report_enum = &hdev->report_enum[HID_INPUT_REPORT];
/* no input reports, bail out */
- if (list_empty(&rep_enum->report_list))
+ if (list_empty(&input_report_enum->report_list))
return -ENODEV;
/*
@@ -1914,7 +1914,7 @@ static int logi_dj_probe(struct hid_device *hdev,
* Note: we should theoretically check for HID++ and DJ
* collections, but this will do.
*/
- list_for_each_entry(rep, &rep_enum->report_list, list) {
+ list_for_each_entry(rep, &input_report_enum->report_list, list) {
if (rep->application == 0xff000001)
has_hidpp = true;
}
@@ -1927,7 +1927,7 @@ static int logi_dj_probe(struct hid_device *hdev,
return -ENODEV;
/* get the current application attached to the node */
- rep = list_first_entry(&rep_enum->report_list, struct hid_report, list);
+ rep = list_first_entry(&input_report_enum->report_list, struct hid_report, list);
djrcv_dev = dj_get_receiver_dev(hdev, id->driver_data,
rep->application, has_hidpp);
if (!djrcv_dev) {
@@ -1935,7 +1935,7 @@ static int logi_dj_probe(struct hid_device *hdev,
return -ENOMEM;
}
- if (!rep_enum->numbered)
+ if (!input_report_enum->numbered)
djrcv_dev->unnumbered_application = rep->application;
/* Starts the usb device and connects to upper interfaces hiddev and
--
2.53.0.983.g0bb29b3bc5-goog
^ permalink raw reply related
* Re: [PATCH v2 00/19] tracepoint: Avoid double static_branch evaluation at guarded call sites
From: Steven Rostedt @ 2026-03-24 14:28 UTC (permalink / raw)
To: Vineeth Pillai (Google)
Cc: Peter Zijlstra, Dmitry Ilvokhin, Masami Hiramatsu,
Mathieu Desnoyers, Ingo Molnar, Jens Axboe, io-uring,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Alexei Starovoitov, Daniel Borkmann, Marcelo Ricardo Leitner,
Xin Long, Jon Maloy, Aaron Conole, Eelco Chaudron, Ilya Maximets,
netdev, bpf, linux-sctp, tipc-discussion, dev, Jiri Pirko,
Oded Gabbay, Koby Elbaz, dri-devel, Rafael J. Wysocki,
Viresh Kumar, Gautham R. Shenoy, Huang Rui, Mario Limonciello,
Len Brown, Srinivas Pandruvada, linux-pm, MyungJoo Ham,
Kyungmin Park, Chanwoo Choi, Christian König, Sumit Semwal,
linaro-mm-sig, Eddie James, Andrew Jeffery, Joel Stanley,
linux-fsi, David Airlie, Simona Vetter, Alex Deucher,
Danilo Krummrich, Matthew Brost, Philipp Stanner, Harry Wentland,
Leo Li, amd-gfx, Jiri Kosina, Benjamin Tissoires, linux-input,
Wolfram Sang, linux-i2c, Mark Brown, Michael Hennerich,
Nuno Sá, linux-spi, James E.J. Bottomley, Martin K. Petersen,
linux-scsi, Chris Mason, David Sterba, linux-btrfs,
Thomas Gleixner, Andrew Morton, SeongJae Park, linux-mm,
Borislav Petkov, Dave Hansen, x86, linux-trace-kernel,
linux-kernel
In-Reply-To: <20260323160052.17528-1-vineeth@bitbyteword.org>
On Mon, 23 Mar 2026 12:00:19 -0400
"Vineeth Pillai (Google)" <vineeth@bitbyteword.org> wrote:
> When a caller already guards a tracepoint with an explicit enabled check:
>
> if (trace_foo_enabled() && cond)
> trace_foo(args);
Thanks Vineeth!
I'm going to start pulling in this series. I'll take the first patch, and
then any patch that has an Acked-by or Reviewed-by from the maintainer.
For patches without acks, I'll leave alone and then after the first patch
gets merged into mainline, the maintainers could pull in their own patches
at their own convenience. Unless of course they speak up now if they want
me to take them ;-)
-- Steve
^ permalink raw reply
* [PATCH] hid: usbhid: fix deadlock in hid_post_reset()
From: Oliver Neukum @ 2026-03-24 14:24 UTC (permalink / raw)
To: bentiss, jikos, linux-input, linux-usb; +Cc: Oliver Neukum
You can build a USB device that includes a HID component
and a storage or UAS component. The components can be reset
only together. That means that hid_pre_reset() and hid_post_reset()
are in the block IO error handling. Hence no memory allocation
used in them may do block IO because the IO can deadlock
on the mutex held while resetting a device and calling the
interface drivers.
Use GFP_NOIO for all allocations in them.
Fixes: dc3c78e434690 ("HID: usbhid: Check HID report descriptor contents after device reset")
Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
drivers/hid/usbhid/hid-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index ddd5d77fb5a5..fd3e1aedc5cb 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -1552,7 +1552,7 @@ static int hid_post_reset(struct usb_interface *intf)
* configuration descriptors passed, we already know that
* the size of the HID report descriptor has not changed.
*/
- rdesc = kmalloc(hid->dev_rsize, GFP_KERNEL);
+ rdesc = kmalloc(hid->dev_rsize, GFP_NOIO);
if (!rdesc)
return -ENOMEM;
--
2.53.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox