* Re: [PATCH v5 4/4] Input: Add TouchNetix aXiom I2C Touchscreen support
From: Marco Felsch @ 2026-02-25 20:50 UTC (permalink / raw)
To: Andrew Thomas
Cc: Luis Chamberlain, Russ Weight, Greg Kroah-Hartman,
Rafael J. Wysocki, Andrew Morton, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Dmitry Torokhov, Kamel Bouhara,
Marco Felsch, Henrik Rydberg, Danilo Krummrich, linux-kernel,
devicetree, linux-input
In-Reply-To: <qoelgb5k77a4c4jodn622a6wauotzkeygy5fj54cjjnobb5g6c@ysxkou6nhkop>
Hi Andrew,
thanks for the your reply, please see below.
On 26-02-25, Andrew Thomas wrote:
> On Sun, Jan 11, 2026 at 04:05:47PM +0100, Marco Felsch wrote:
...
> > +struct axiom_u33_rev3 {
> > + __le32 runtime_crc;
> > + __le32 runtime_nvm_crc;
> > + __le32 bootloader_crc;
> > + __le32 nvltlusageconfig_crc;
> > + __le32 vltusageconfig_crc;
> > + __le32 u22_sequencedata_crc;
> > + __le32 u43_hotspots_crc;
> > + __le32 u77_dod_data_crc;
> > + __le32 u93_profiles_crc;
> > + __le32 u94_deltascalemap_crc;
> > + __le32 runtimehash_crc;
> > +};
> > +
>
> I think revision handling should be kept in unpacking where possible.
> Currently there are 10 revisions of u33, so adding support for many
> revisions and usages would add alot of code.
This isn't very complex nor very large code. Each u33 rev will add
44-byte of code, so in the end there will be 440-byte. I've also seen
that some revisions reduce the size because some fields aren't required.
E.g. u93_crc is at offset-5.
Furthermore describing the complete layout of u33 allows us using it to
query the u33 in one i2c-bulk-transfer.
> > +#define AXIOM_U34 0x34
> > +#define AXIOM_U34_REV1_OVERFLOW_MASK BIT(7)
> > +#define AXIOM_U34_REV1_REPORTLENGTH_MASK GENMASK(6, 0)
> > +#define AXIOM_U34_REV1_PREAMBLE_BYTES 2
> > +#define AXIOM_U34_REV1_POSTAMBLE_BYTES 4
...
> > +enum axiom_runmode {
> > + AXIOM_DISCOVERY_MODE,
> > + AXIOM_TCP_MODE,
> > + AXIOM_TCP_CFG_UPDATE_MODE,
> > + AXIOM_BLP_PRE_MODE,
> > + AXIOM_BLP_MODE,
> > +};
>
> There are only two actual axiom states, bootloader and runtime (TCP).
> This is more of a driver state rather than an axiom state.
> Could you label it as such?
Yes I know and the AXIOM_BLP_PRE_MODE will be dropped with the next
version, which I'm going to send this week!. Not sure why this would be
required.
> > +struct axiom_data {
> > + struct input_dev *input;
> > + struct device *dev;
> > +
> > + struct gpio_desc *reset_gpio;
> > + struct regulator_bulk_data supplies[2];
> > + unsigned int num_supplies;
> > +
> > + struct regmap *regmap;
> > + struct touchscreen_properties prop;
> > + bool irq_setup_done;
> > + u32 poll_interval;
> > +
> > + struct drm_panel_follower panel_follower;
> > + bool is_panel_follower;
> > +
> > + enum axiom_runmode mode;
> > + /*
> > + * Two completion types to support firmware updates
> > + * in irq and poll mode.
> > + */
> > + struct axiom_completion {
> > + struct completion completion;
> > + bool poll_done;
> > + } nvm_write, boot_complete;
> > +
> > + /* Lock to protect both firmware interfaces */
> > + struct mutex fwupdate_lock;
> > + struct axiom_firmware {
> > + /* Lock to protect cancel */
> > + struct mutex lock;
> > + bool cancel;
> > + struct fw_upload *fwl;
> > + } fw[AXIOM_FW_NUM];
> > +
> > + unsigned int fw_major;
> > + unsigned int fw_minor;
> > + unsigned int fw_rc;
> > + unsigned int fw_status;
> > + unsigned int fw_variant;
> > + u16 device_id;
> > + u16 jedec_id;
> > + u8 silicon_rev;
> > +
> > + /* CRCs we need to check during a config update */
> > + struct axiom_crc {
> > + u32 runtime;
> > + u32 vltusageconfig;
> > + u32 nvltlusageconfig;
> > + u32 u22_sequencedata;
> > + u32 u43_hotspots;
> > + u32 u77_dod_data;
> > + u32 u93_profiles;
> > + u32 u94_deltascalemap;
> > + } crc[AXIOM_CRC_NUM];
>
> I think this structure should hold all possible u33 CRCs and then
> invalid ones can be ignored for the given u33 revision.
Why should be the bootloader CRC interessting? The bootloader can't be
updated/flashed, at least not according my documentation. Therefore I
didn't listed the bootloader CRC here.
> > + bool cds_enabled;
> > + unsigned long enabled_slots;
> > + unsigned int num_slots;
> > +
> > + unsigned int max_report_byte_len;
> > + struct axiom_usage_table_entry {
> > + bool populated;
> > + unsigned int baseaddr;
> > + unsigned int size_bytes;
> > + const struct axiom_usage_info *info;
> > + } usage_table[AXIOM_MAX_USAGES];
> > +};
....
> > +static int axiom_u02_swreset(struct axiom_data *ts)
> > +{
> > + struct axiom_u02_rev1_system_manager_msg msg = { };
> > + int ret;
> > +
> > + if (!axiom_driver_supports_usage(ts, AXIOM_U02))
> > + return -EINVAL;
> > +
> > + msg.command = cpu_to_le16(AXIOM_U02_REV1_CMD_SOFTRESET);
> > + ret = axiom_u02_send_msg(ts, &msg, false);
> > + if (ret)
> > + return ret;
> > +
> > + /*
> > + * Downstream http://axcfg.py waits for 1sec without checking U01 hello. Tests
> > + * showed that waiting for the hello message isn't enough therefore we
> > + * need both to make it robuster.
> > + */
> > + ret = axiom_wait_for_completion_timeout(ts, &ts->boot_complete,
> > + msecs_to_jiffies(1 * MSEC_PER_SEC));
>
> Boot can take up to 2s with all selftests enabled.
Thanks for this information :) I will add it.
> > + if (!ret)
> > + dev_err(ts->dev, "Error swreset timedout\n");
> > +
> > + fsleep(USEC_PER_SEC);
> > +
> > + return ret ? 0 : -ETIMEDOUT;
> > +}
...
> > +static int axiom_u02_enter_bootloader(struct axiom_data *ts)
> > +{
> > + struct axiom_u02_rev1_system_manager_msg msg = { };
> > + struct device *dev = ts->dev;
> > + unsigned int val;
> > + int ret;
> > +
> > + if (!axiom_driver_supports_usage(ts, AXIOM_U02))
> > + return -EINVAL;
> > +
> > + /*
> > + * Enter the bootloader mode requires 3 consecutive messages so we can't
> > + * check for the response.
> > + */
> > + msg.command = cpu_to_le16(AXIOM_U02_REV1_CMD_ENTERBOOTLOADER);
> > + msg.parameters[0] = cpu_to_le16(AXIOM_U02_REV1_PARAM0_ENTERBOOLOADER_KEY1);
> > + ret = axiom_u02_send_msg(ts, &msg, false);
> > + if (ret) {
> > + dev_err(dev, "Failed to send bootloader-key1: %d\n", ret);
> > + return ret;
> > + }
>
> A delay is required between commands. 10ms is fine.
Can I make use of the axiom_u02_wait_idle() logic which checks the
AXIOM_U02_REV1_RESP_SUCCESS? Arbitrary delays are always a source of
trouble.
> > + msg.parameters[0] = cpu_to_le16(AXIOM_U02_REV1_PARAM0_ENTERBOOLOADER_KEY2);
> > + ret = axiom_u02_send_msg(ts, &msg, false);
> > + if (ret) {
> > + dev_err(dev, "Failed to send bootloader-key2: %d\n", ret);
> > + return ret;
> > + }
>
> And here.
>
> > +
> > + msg.parameters[0] = cpu_to_le16(AXIOM_U02_REV1_PARAM0_ENTERBOOLOADER_KEY3);
> > + ret = axiom_u02_send_msg(ts, &msg, false);
> > + if (ret) {
> > + dev_err(dev, "Failed to send bootloader-key3: %d\n", ret);
> > + return ret;
> > + }
> > +
> > + /* Sleep before the first read to give the device time */
> > + fsleep(250 * USEC_PER_MSEC);
> > +
> > + /* Wait till the device reports it is in bootloader mode */
> > + return regmap_read_poll_timeout(ts->regmap,
> > + AXIOM_U31_REV1_DEVICE_ID_HIGH_REG, val,
> > + FIELD_GET(AXIOM_U31_REV1_MODE_MASK, val) ==
> > + AXIOM_U31_REV1_MODE_BLP, 250 * USEC_PER_MSEC,
> > + USEC_PER_SEC);
> > +}
>
> Just to note if we cannot enter bootloader with u02 due to a corrupted firmware,
> you can enter bootloader if the nRESET line is toggled 5 times without comms.
This could be added later on by $dev (maybe you :)) since I can't test
this. Our system has the reset line not connected :/
...
> > +static int axiom_u33_read(struct axiom_data *ts, struct axiom_crc *crc)
> > +{
> > + struct device *dev = ts->dev;
> > + unsigned int reg;
> > + int ret;
> > +
> > + if (!axiom_driver_supports_usage(ts, AXIOM_U33))
> > + return -EINVAL;
> > +
> > + if (axiom_usage_rev(ts, AXIOM_U33) == 2) {
> > + struct axiom_u33_rev2 val;
> > +
> > + reg = axiom_usage_baseaddr(ts, AXIOM_U33);
> > + ret = regmap_raw_read(ts->regmap, reg, &val, sizeof(val));
>
> Could we read into a raw buffer to save having to define a little endian
> version of the CRCs?
I don't see the benefit.
...
> > +/* Custom regmap read/write handling is required due to the aXiom protocol */
> > +static int axiom_regmap_read(void *context, const void *reg_buf, size_t reg_size,
> > + void *val_buf, size_t val_size)
> > +{
> > + struct device *dev = context;
> > + struct i2c_client *i2c = to_i2c_client(dev);
> > + struct axiom_data *ts = i2c_get_clientdata(i2c);
> > + struct axiom_cmd_header hdr;
> > + u16 xferlen, addr, baseaddr;
> > + struct i2c_msg xfer[2];
> > + int ret;
> > +
> > + if (val_size > AXIOM_MAX_XFERLEN) {
> > + dev_err(ts->dev, "Exceed max xferlen: %zu > %u\n",
> > + val_size, AXIOM_MAX_XFERLEN);
> > + return -EINVAL;
> > + }
> > +
> > + addr = *((u16 *)reg_buf);
> > + hdr.target_address = cpu_to_le16(addr);
> > + xferlen = FIELD_PREP(AXIOM_CMD_HDR_DIR_MASK, AXIOM_CMD_HDR_READ) |
> > + FIELD_PREP(AXIOM_CMD_HDR_LEN_MASK, val_size);
> > + hdr.xferlen = cpu_to_le16(xferlen);
> > +
> > + /* Verify that usage including the usage rev is supported */
> > + baseaddr = addr & AXIOM_USAGE_BASEADDR_MASK;
> > + if (!axiom_usage_supported(ts, baseaddr))
> > + return -EINVAL;
> > +
> > + xfer[0].addr = i2c->addr;
> > + xfer[0].flags = 0;
> > + xfer[0].len = sizeof(hdr);
> > + xfer[0].buf = (u8 *)&hdr;
> > +
> > + xfer[1].addr = i2c->addr;
> > + xfer[1].flags = I2C_M_RD;
> > + xfer[1].len = val_size;
> > + xfer[1].buf = val_buf;
> > +
> > + ret = i2c_transfer(i2c->adapter, xfer, 2);
> > + if (ret == 2)
> > + return 0;
> > + else if (ret < 0)
> > + return ret;
> > + else
> > + return -EIO;
> > +}
>
> There needs to be atleast 40us holdoff between axiom bus transfers.
> I am not sure that has been considered here.
Is this written somewhere within the datasheet/programming-guide?
...
> > +static enum fw_upload_err
> > +axiom_cfg_fw_prepare(struct fw_upload *fw_upload, const u8 *data, u32 size)
> > +{
...
> > + cur_runtime_crc = ts->crc[AXIOM_CRC_CUR].runtime;
> > + fw_runtime_crc = ts->crc[AXIOM_CRC_NEW].runtime;
> > + if (cur_runtime_crc != fw_runtime_crc) {
> > + dev_err(dev, "TH2CFG and device runtime CRC doesn't match: %#x != %#x\n",
> > + fw_runtime_crc, cur_runtime_crc);
> > + ret = FW_UPLOAD_ERR_FW_INVALID;
> > + goto out;
> > + }
>
> The firmware CRCs dont need to match for a config load, only the usage revision/length.
What difference does it make? The firmware CRC implicit includes the
usage revision and the length (register layout). So we can ensure that
the configuration was made for the correct register layout without
checking each register and revision.
...
> > +static enum fw_upload_err
> > +axiom_cfg_fw_write(struct fw_upload *fw_upload, const u8 *data, u32 offset,
> > + u32 size, u32 *written)
> > +{
....
> > + /* Ensure that the chunks are written correctly */
> > + ret = axiom_verify_volatile_mem(ts);
> > + if (ret) {
> > + dev_err(dev, "Failed to verify written config, abort\n");
> > + goto err_swreset;
> > + }
> > +
> > + ret = axiom_u02_save_config(ts);
> > + if (ret)
> > + goto err_swreset;
> > +
> > + /*
> > + * TODO: Check if u02 start would be sufficient to load the new config
> > + * values
> > + */
>
> It is not necessarily needed.
What do you mean by this? Do we need the axiom_u02_swreset() or can we
just start the system via u02 (without swreset)?
>
> > + ret = axiom_u02_swreset(ts);
> > + if (ret) {
> > + dev_err(dev, "Soft reset failed\n");
> > + goto err_unlock;
> > + }
....
> > +static ssize_t fw_variant_show(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + struct i2c_client *i2c = to_i2c_client(dev);
> > + struct axiom_data *ts = i2c_get_clientdata(i2c);
> > + const char *val;
> > +
> > + switch (ts->fw_variant) {
> > + case 0:
> > + val = "3d";
> > + break;
> > + case 1:
> > + val = "2d";
> > + break;
> > + case 3:
> > + val = "force";
> > + break;
> > + default:
> > + val = "unknown";
> > + break;
> > + }
>
> The following are all the variants we currently support in order:
> FW_VARIANTS = ["3D", "2D", "FORCE", "0D", "XL"]
Means:
0 == 3d
1 == 2d
3 == force
4 == 0d
5 == xl
?
This is also something I can test on my site. Patches are welcome once
this is mainline of course :)
Regards,
Marco
--
#gernperDu
#CallMeByMyFirstName
Pengutronix e.K. | |
Steuerwalder Str. 21 | https://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
^ permalink raw reply
* Re: [PATCH v6 4/7] iio: Rename 'sign' field to `format` in struct iio_scan_type
From: David Lechner @ 2026-02-25 21:27 UTC (permalink / raw)
To: Francesco Lavra, Jonathan Corbet, Shuah Khan, Lars-Peter Clausen,
Michael Hennerich, Lucas Stankus, Jonathan Cameron, Nuno Sá,
Andy Shevchenko, Puranjay Mohan, Cosmin Tanislav,
Ramona Gradinariu, Antoniu Miclaus, Petre Rodan, Dan Robertson,
Benson Leung, Guenter Roeck, Jiri Kosina, Srinivas Pandruvada,
Matti Vaittinen, Marcelo Schmitt, Esteban Blanc, Jorge Marques,
Sergiu Cuciurean, Dragos Bogdan, Alisa-Dariana Roman,
Trevor Gamblin, Renato Lui Geh, Eugen Hristev, Nicolas Ferre,
Alexandre Belloni, Claudiu Beznea, Andreas Klinger, Paul Cercueil,
Ramona Bolboaca, Marcus Folkesson, Kent Gustavsson,
Matthias Brugger, AngeloGioacchino Del Regno, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Matteo Martelli, Marius Cristea, Heiko Stuebner, Maxime Coquelin,
Alexandre Torgue, Kurt Borja, Francesco Dolcini,
João Paulo Gonçalves, Leonard Göhrs,
Oleksij Rempel, Haibo Chen, Salih Erim, Conall O'Griofa,
Michal Simek, Gustavo Silva, Tomasz Duszynski, Roan van Dijk,
Jyoti Bhayana, Mariel Tinaco, Nishant Malpani, Rui Miguel Silva,
Linus Walleij, Lorenzo Bianconi, Alex Lanzano, Jagath Jog J,
Jean-Baptiste Maneyrol, Remi Buisson, Christian Eggers,
Mudit Sharma, Javier Carrasco, Ondřej Jirman, Song Qiang,
Dixit Parmar, Gerald Loacker, linux-doc, linux-kernel, linux-iio,
chrome-platform, linux-input, linux-arm-kernel, linux-mips,
linux-mediatek, imx, linux-rockchip, linux-stm32
In-Reply-To: <20260225101735.2368252-1-flavra@baylibre.com>
On 2/25/26 4:17 AM, Francesco Lavra wrote:
> This field is used to differentiate between signed and unsigned integers.
> A following commit will extend its use to in order to add support for non-
> integer scan elements; therefore, change its name from 'sign' to a more
> generic 'format'.
>
Maybe Jonathan is OK with doing this all at once, but another alternative
could be to introduce a union to allow both names at the same time, then
we could make the change more gradually.
^ permalink raw reply
* Re: [PATCH V3 RESEND 2/2] HID: i2c-hid: elan: Add parade-tc3408 timing
From: Dmitry Torokhov @ 2026-02-26 4:59 UTC (permalink / raw)
To: Benjamin Tissoires, Jiri Kosina
Cc: Doug Anderson, Langyan Ye, robh, krzk+dt, conor+dt, linux-input,
devicetree, linux-kernel
In-Reply-To: <CAD=FV=XKqFTNYopxV9H8rnjEYAgDebW42Ot92aZYiex5MP1wnQ@mail.gmail.com>
On Thu, Jan 29, 2026 at 08:50:57AM -0800, Doug Anderson wrote:
> Hi,
>
> On Wed, Jan 21, 2026 at 2:40 PM Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> >
> > On Thu, Jan 08, 2026 at 02:35:24PM +0800, Langyan Ye wrote:
> > > Parade-tc3408 requires reset to pull down time greater than 10ms,
> > > so the configuration post_power_delay_ms is 10, and the chipset
> > > initial time is required to be greater than 300ms,
> > > so the post_gpio_reset_on_delay_ms is set to 300.
> > >
> > > Signed-off-by: Langyan Ye <yelangyan@huaqin.corp-partner.google.com>
> > > Reviewed-by: Douglas Anderson <dianders@chromium.org>
> >
> > Jiri, Benjamin, another I2C hid with bindings...
>
> I guess maybe this could go through the input tree, but it needs
> either Jiri's or Benjamin's ack? Is that the plan?
Jiri, Benjamin,
I am going to pick up the DT binding changes, do you want me to pick up
the driver changes as well or do you want to merge them through your
tree?
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH RESEND v2] HID: winwing: Enable rumble effects
From: Ivan Gorinov @ 2026-02-26 5:49 UTC (permalink / raw)
To: Jiri Kosina; +Cc: linux-input, linux-kernel
Enable rumble motor control on TGRIP-15E and TGRIP-15EX throttle grips
by sending haptic feedback commands (EV_FF events) to the input device.
Signed-off-by: Ivan Gorinov <linux-kernel@altimeter.info>
---
Changes since v1:
- Fix possible NULL pointer dereference
drivers/hid/hid-winwing.c | 184 +++++++++++++++++++++++++++++++++++---
1 file changed, 170 insertions(+), 14 deletions(-)
diff --git a/drivers/hid/hid-winwing.c b/drivers/hid/hid-winwing.c
index ab65dc12d1e0..b5d0978d81e5 100644
--- a/drivers/hid/hid-winwing.c
+++ b/drivers/hid/hid-winwing.c
@@ -12,6 +12,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/mutex.h>
+#include <linux/workqueue.h>
#define MAX_REPORT 16
@@ -35,10 +36,14 @@ static const struct winwing_led_info led_info[3] = {
struct winwing_drv_data {
struct hid_device *hdev;
- __u8 *report_buf;
- struct mutex lock;
- int map_more_buttons;
- unsigned int num_leds;
+ struct mutex lights_lock;
+ __u8 *report_lights;
+ __u8 *report_rumble;
+ struct work_struct rumble_work;
+ struct ff_rumble_effect rumble;
+ int rumble_left;
+ int rumble_right;
+ int has_grip15;
struct winwing_led leds[];
};
@@ -47,10 +52,10 @@ static int winwing_led_write(struct led_classdev *cdev,
{
struct winwing_led *led = (struct winwing_led *) cdev;
struct winwing_drv_data *data = hid_get_drvdata(led->hdev);
- __u8 *buf = data->report_buf;
+ __u8 *buf = data->report_lights;
int ret;
- mutex_lock(&data->lock);
+ mutex_lock(&data->lights_lock);
buf[0] = 0x02;
buf[1] = 0x60;
@@ -69,7 +74,7 @@ static int winwing_led_write(struct led_classdev *cdev,
ret = hid_hw_output_report(led->hdev, buf, 14);
- mutex_unlock(&data->lock);
+ mutex_unlock(&data->lights_lock);
return ret;
}
@@ -87,9 +92,9 @@ static int winwing_init_led(struct hid_device *hdev,
if (!data)
return -EINVAL;
- data->report_buf = devm_kmalloc(&hdev->dev, MAX_REPORT, GFP_KERNEL);
+ data->report_lights = devm_kzalloc(&hdev->dev, MAX_REPORT, GFP_KERNEL);
- if (!data->report_buf)
+ if (!data->report_lights)
return -ENOMEM;
for (i = 0; i < 3; i += 1) {
@@ -117,7 +122,7 @@ static int winwing_init_led(struct hid_device *hdev,
return ret;
}
-static int winwing_map_button(int button, int map_more_buttons)
+static int winwing_map_button(int button, int has_grip15)
{
if (button < 1)
return KEY_RESERVED;
@@ -141,7 +146,7 @@ static int winwing_map_button(int button, int map_more_buttons)
return (button - 65) + BTN_TRIGGER_HAPPY17;
}
- if (!map_more_buttons) {
+ if (!has_grip15) {
/*
* Not mapping numbers [33 .. 64] which
* are not assigned to any real buttons
@@ -194,13 +199,141 @@ static int winwing_input_mapping(struct hid_device *hdev,
/* Button numbers start with 1 */
button = usage->hid & HID_USAGE;
- code = winwing_map_button(button, data->map_more_buttons);
+ code = winwing_map_button(button, data->has_grip15);
hid_map_usage(hi, usage, bit, max, EV_KEY, code);
return 1;
}
+/*
+ * If x ≤ 0, return 0;
+ * if x is in [1 .. 65535], return a value in [1 .. 255]
+ */
+static inline int convert_magnitude(int x)
+{
+ if (x < 1)
+ return 0;
+
+ return ((x * 255) >> 16) + 1;
+}
+
+static int winwing_haptic_rumble(struct winwing_drv_data *data)
+{
+ __u8 *buf;
+ __u8 m;
+
+ if (!data)
+ return -EINVAL;
+
+ if (!data->hdev)
+ return -EINVAL;
+
+ buf = data->report_rumble;
+
+ if (!buf)
+ return -EINVAL;
+
+ m = convert_magnitude(data->rumble.strong_magnitude);
+ if (m != data->rumble_left) {
+ int ret;
+
+ buf[0] = 0x02;
+ buf[1] = 0x01;
+ buf[2] = 0xbf;
+ buf[3] = 0x00;
+ buf[4] = 0x00;
+ buf[5] = 0x03;
+ buf[6] = 0x49;
+ buf[7] = 0x00;
+ buf[8] = m;
+ buf[9] = 0x00;
+ buf[10] = 0;
+ buf[11] = 0;
+ buf[12] = 0;
+ buf[13] = 0;
+
+ ret = hid_hw_output_report(data->hdev, buf, 14);
+ if (ret < 0) {
+ hid_err(data->hdev, "error %d (%*ph)\n", ret, 14, buf);
+ return ret;
+ }
+ data->rumble_left = m;
+ }
+
+ m = convert_magnitude(data->rumble.weak_magnitude);
+ if (m != data->rumble_right) {
+ int ret;
+
+ buf[0] = 0x02;
+ buf[1] = 0x03;
+ buf[2] = 0xbf;
+ buf[3] = 0x00;
+ buf[4] = 0x00;
+ buf[5] = 0x03;
+ buf[6] = 0x49;
+ buf[7] = 0x00;
+ buf[8] = m;
+ buf[9] = 0x00;
+ buf[10] = 0;
+ buf[11] = 0;
+ buf[12] = 0;
+ buf[13] = 0;
+
+ ret = hid_hw_output_report(data->hdev, buf, 14);
+ if (ret < 0) {
+ hid_err(data->hdev, "error %d (%*ph)\n", ret, 14, buf);
+ return ret;
+ }
+ data->rumble_right = m;
+ }
+
+ return 0;
+}
+
+
+static void winwing_haptic_rumble_cb(struct work_struct *work)
+{
+ struct winwing_drv_data *data;
+
+ data = container_of(work, struct winwing_drv_data, rumble_work);
+ winwing_haptic_rumble(data);
+}
+
+static int winwing_play_effect(struct input_dev *dev, void *context,
+ struct ff_effect *effect)
+{
+ struct winwing_drv_data *data = (struct winwing_drv_data *) context;
+
+ if (effect->type != FF_RUMBLE)
+ return 0;
+
+ if (!data)
+ return -EINVAL;
+
+ data->rumble = effect->u.rumble;
+
+ return schedule_work(&data->rumble_work);
+}
+
+static int winwing_init_ff(struct hid_device *hdev, struct hid_input *hidinput)
+{
+ struct winwing_drv_data *data;
+
+ data = (struct winwing_drv_data *) hid_get_drvdata(hdev);
+ if (!data)
+ return -EINVAL;
+
+ data->report_rumble = devm_kzalloc(&hdev->dev, MAX_REPORT, GFP_KERNEL);
+ data->rumble_left = -1;
+ data->rumble_right = -1;
+
+ input_set_capability(hidinput->input, EV_FF, FF_RUMBLE);
+
+ return input_ff_create_memless(hidinput->input, data,
+ winwing_play_effect);
+}
+
static int winwing_probe(struct hid_device *hdev,
const struct hid_device_id *id)
{
@@ -219,10 +352,12 @@ static int winwing_probe(struct hid_device *hdev,
if (!data)
return -ENOMEM;
- data->map_more_buttons = id->driver_data;
-
+ data->hdev = hdev;
+ data->has_grip15 = id->driver_data;
hid_set_drvdata(hdev, data);
+ INIT_WORK(&data->rumble_work, winwing_haptic_rumble_cb);
+
ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
if (ret) {
hid_err(hdev, "hw start failed\n");
@@ -232,19 +367,39 @@ static int winwing_probe(struct hid_device *hdev,
return 0;
}
+static void winwing_remove(struct hid_device *hdev)
+{
+ struct winwing_drv_data *data;
+
+ data = (struct winwing_drv_data *) hid_get_drvdata(hdev);
+
+ if (data)
+ cancel_work_sync(&data->rumble_work);
+
+ hid_hw_close(hdev);
+ hid_hw_stop(hdev);
+}
+
static int winwing_input_configured(struct hid_device *hdev,
struct hid_input *hidinput)
{
+ struct winwing_drv_data *data;
int ret;
+ data = (struct winwing_drv_data *) hid_get_drvdata(hdev);
+
ret = winwing_init_led(hdev, hidinput->input);
if (ret)
hid_err(hdev, "led init failed\n");
+ if (data->has_grip15)
+ winwing_init_ff(hdev, hidinput);
+
return ret;
}
+/* Set driver_data to 1 for grips with rumble motor and more than 32 buttons */
static const struct hid_device_id winwing_devices[] = {
{ HID_USB_DEVICE(0x4098, 0xbd65), .driver_data = 1 }, /* TGRIP-15E */
{ HID_USB_DEVICE(0x4098, 0xbd64), .driver_data = 1 }, /* TGRIP-15EX */
@@ -261,6 +416,7 @@ static struct hid_driver winwing_driver = {
.input_configured = winwing_input_configured,
.input_mapping = winwing_input_mapping,
.probe = winwing_probe,
+ .remove = winwing_remove,
};
module_hid_driver(winwing_driver);
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v3 3/4] dt-bindings: input: add GPIO charlieplex keypad
From: Geert Uytterhoeven @ 2026-02-26 9:32 UTC (permalink / raw)
To: Hugo Villeneuve
Cc: robin, andy, robh, krzk+dt, conor+dt, dmitry.torokhov,
hvilleneuve, mkorpershoek, matthias.bgg,
angelogioacchino.delregno, lee, alexander.sverdlin, marek.vasut,
akurz, devicetree, linux-kernel, linux-input, linux-arm-kernel,
linux-mediatek
In-Reply-To: <20260225155409.612478-4-hugo@hugovil.com>
Hi Hugo,
On Wed, 25 Feb 2026 at 16:54, Hugo Villeneuve <hugo@hugovil.com> wrote:
> From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
>
> Add DT bindings for GPIO charlieplex keypad.
>
> Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
Thanks for your patch!
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml
> @@ -0,0 +1,106 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +
> +$id: http://devicetree.org/schemas/input/gpio-charlieplex-keypad.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: GPIO charlieplex keypad
> +
> +maintainers:
> + - Hugo Villeneuve <hvilleneuve@dimonoff.com>
> +
> +description: |
> + The charlieplex keypad supports N^2)-N different key combinations (where N is
> + the number of lines). Key presses and releases are detected by configuring
> + only one line as output at a time, and reading other line states. This process
> + is repeated for each line. Diodes are required to ensure current flows in only
> + one direction between any pair of pins.
> + This mechanism doesn't allow to detect simultaneous key presses.
Indeed, e.g. pressing S1 and S2 simultaneously will show a ghost
S5 keypress.
> +
> + Wiring example for 3 lines keyboard with 6 switches and 3 diodes:
> +
> + L0 --+---------------------+----------------------+
> + | | |
> + L1 -------+-----------+---------------------+ |
> + | | | | | |
> + L2 -------------+----------------+-----+ | |
> + | | | | | | | | |
> + | | | | | | | | |
> + | S1 \ S2 \ | S3 \ S4 \ | S5 \ S6 \
> + | | | | | | | | |
> + | +--+--+ | +--+--+ | +--+--+
> + | | | | | |
> + | D1 v | D2 v | D3 v
> + | - (k) | - (k) | - (k)
> + | | | | | |
> + +-------+ +-------+ +-------+
Don't you need pull-down resistors on L[0-2], and/or a way to specify
in DT to enable internal poll-down on GPIO controllers that support it?
Some controllers may support internal pull-up only, but I guess that
can be handled using GPIO_ACTIVE_LOW?
> +
> + L: GPIO line
> + S: switch
> + D: diode (k indicates cathode)
> +
> +allOf:
> + - $ref: input.yaml#
> + - $ref: /schemas/input/matrix-keymap.yaml#
> +
> +properties:
> + compatible:
> + const: gpio-charlieplex-keypad
> +
> + autorepeat: true
> +
> + debounce-delay-ms:
> + default: 5
> +
> + line-gpios:
> + description:
> + List of GPIOs used as lines. The gpio specifier for this property
> + depends on the gpio controller to which these lines are connected.
> +
> + linux,keymap: true
> +
> + poll-interval: true
> +
> + settling-time-us: true
> +
> + wakeup-source: true
> +
> +required:
> + - compatible
> + - line-gpios
> + - linux,keymap
> + - poll-interval
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> + #include <dt-bindings/input/input.h>
> +
> + charlieplex-keypad {
"keyboard", as per Devicetree Specification Generic Names
Recommendation.
> + compatible = "gpio-charlieplex-keypad";
> + debounce-delay-ms = <20>;
> + poll-interval = <5>;
> + settling-time-us = <2>;
> +
> + line-gpios = <&gpio2 25 GPIO_ACTIVE_HIGH
> + &gpio2 26 GPIO_ACTIVE_HIGH
> + &gpio2 27 GPIO_ACTIVE_HIGH>;
> +
> + /* MATRIX_KEY(output, input, key-code) */
> + linux,keymap = <
> + /*
> + * According to wiring diagram above, if L1 is configured as
> + * output and HIGH, and we detect a HIGH level on input L0,
> + * then it means S1 is pressed: MATRIX_KEY(L1, L0, KEY...)
> + */
> + MATRIX_KEY(1, 0, KEY_F1) /* S1 */
> + MATRIX_KEY(2, 0, KEY_F2) /* S2 */
> + MATRIX_KEY(0, 1, KEY_F3) /* S3 */
> + MATRIX_KEY(2, 1, KEY_F4) /* S4 */
> + MATRIX_KEY(1, 2, KEY_F5) /* S5 */
> + MATRIX_KEY(0, 2, KEY_F6) /* S6 */
> + >;
> + };
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v2 4/4] HID: steelseries: Add support for Arctis headset lineup
From: Bastien Nocera @ 2026-02-26 10:01 UTC (permalink / raw)
To: Sriman Achanta; +Cc: linux-kernel, linux-input
In-Reply-To: <CABMjph9TzmCfks0OEtCx4gFFe0pKOccBwVutq6dVH2DhzSC5vA@mail.gmail.com>
Hey,
On Thu, 2026-02-26 at 04:33 -0500, Sriman Achanta wrote:
> I agree these should be proper ALSA controls rather than sysfs files.
> I've been investigating the best way to accomplish this, since the
> challenge is that these values (chatmix, sidetone, mic volume, etc.)
> are reported/controlled via the HID vendor interface, while the USB
> audio card is owned by snd-usb-audio.
>
> There are a few approaches I've found precedent for: (1) adding
> vendor quirks in sound/usb/mixer_quirks.c that bridge back to the HID
> interface, similar to the DualSense jack detection code which uses an
> input_handler to receive events from hid-playstation; (2) creating a
> separate snd_card from the HID driver itself with custom mixer
> controls, similar to how hid-prodikeys creates its own ALSA card; or
> (3) having the HID driver find the sibling USB audio card and add
> controls to it directly (though this has no existing precedent and
> raises lifetime/ordering concerns).
>
> My inclination is toward option 2 — a dedicated snd_card owned by the
> HID driver — since the chatmix dial and sidetone are logically
> properties of the HID device rather than the audio stream, and it
> avoids fragile cross-driver coupling. However, this does mean
> userspace sees a second ALSA card for what is physically one device.
> I'd appreciate guidance on which approach the maintainers would
> prefer before I implement it.
Option 2 is definitely the way to go. I would expect widely deployed
front-ends to ALSA like Pipewire and PulseAudio to be able to make it
look like the 2 sets of mixing controls come from the same physical
device.
Cheers
>
> Best,
> Sriman Achanta
>
> On Wed, Jan 14, 2026 at 4:51 AM Bastien Nocera <hadess@hadess.net>
> wrote:
> > On Tue, 2026-01-13 at 21:57 -0500, Sriman Achanta wrote:
> > > > As mentioned in the earlier patch, sidetone control, chatmix
> > > > level, mic
> > > > level, volume limiter and bluetooth call volume all should be
> > > > implemented as ALSA mixers/switches so they can be toggled with
> > > > stock
> > > > tools and presented in a uniform way up the stack
> > > > (Pipewire/Pulseaudio
> > > > and desktop environments).
> > >
> > > My only concern with this is that there is no ability to read the
> > > setting state from the headset so the only way to implement ALSA
> > > mixers would be to return a guessed value / default and then
> > > write-
> > > through caching for subsequently set values, creating a desync
> > > between the device and the reported state of the mixer.
> >
> > How is that different from a sysfs value that you can't read when
> > the
> > headset is off?
> >
> > > Additionally, inactive_timeout and bt_power_on should remain as
> > > Sysfs handles as they modify the power system, not audio.
> >
> > They should however be separate patches, so the maintainers have a
> > choice to take them or not, especially since adding new sysfs files
> > is
> > frowned upon, and maintainers will likely want a more generic way
> > to
> > implement that functionality for other devices as well.
> >
> > Note that you emailed me in private, you should definitely CC: the
> > mailing-list when you answer, otherwise nobody but me will see your
> > response.
> >
> > Cheers
> >
> > >
> > > On Mon, Jan 12, 2026 at 8:09 AM Bastien Nocera
> > > <hadess@hadess.net>
> > > wrote:
> > > > On Sun, 2026-01-11 at 23:19 -0500, Sriman Achanta wrote:
> > > > > Add full support for the SteelSeries Arctis wireless gaming
> > > > > headset
> > > > > lineup, extending the driver from basic support for 2 models
> > > > > (Arctis
> > > > > 1
> > > > > and 9) to comprehensive support for 25+ models across all
> > > > > Arctis
> > > > > generations.
> > > > >
> > > > > This is a major restructure of the hid-steelseries driver
> > > > > that
> > > > > replaces
> > > > > the previous minimal implementation with a unified,
> > > > > capability-
> > > > > based
> > > > > architecture.
> > > >
> > > > This patch needs to be split up, at the very least it needs new
> > > > features to be split up from any other refactoring that might
> > > > be
> > > > needed
> > > > to support features with each new feature getting its own
> > > > commit.
> > > >
> > > > As mentioned in the earlier patch, sidetone control, chatmix
> > > > level,
> > > > mic
> > > > level, volume limiter and bluetooth call volume all should be
> > > > implemented as ALSA mixers/switches so they can be toggled with
> > > > stock
> > > > tools and presented in a uniform way up the stack
> > > > (Pipewire/Pulseaudio
> > > > and desktop environments).
> > > >
> > > > An additional comments inline.
> > > >
> > > > > <snip>
> > > > > -/* Fixed report descriptor for Steelseries SRW-S1 wheel
> > > > > controller
> > > > > - *
> > > > > - * The original descriptor hides the sensitivity and assists
> > > > > dials
> > > > > - * a custom vendor usage page. This inserts a patch to make
> > > > > them
> > > > > - * appear in the 'Generic Desktop' usage.
> > > > > - */
> > > > > -
> > > > > +/* Fixed report descriptor for Steelseries SRW-S1 wheel
> > > > > controller
> > > > > */
> > > >
> > > > There's really no need to reindent this array.
> > > >
> > > > > static const __u8 steelseries_srws1_rdesc_fixed[] = {
> > > > > -0x05, 0x01, /* Usage Page (Desktop)
> > > > > */
> > > > > -0x09, 0x08, /* Usage (MultiAxis), Changed
> > > > > */
> > > > > -0xA1, 0x01, /* Collection (Application),
> > > > > */
> > > > > -0xA1, 0x02, /* Collection (Logical),
> > > > > */
> > > > > -0x95, 0x01, /* Report Count (1),
> > > > > */
> > > > > -0x05, 0x01, /* Changed Usage Page (Desktop),
> > > > > */
> > > > > -0x09, 0x30, /* Changed Usage (X),
> > > > > */
> > > > > -0x16, 0xF8, 0xF8, /* Logical Minimum (-1800),
> > > > > */
> > > > > -0x26, 0x08, 0x07, /* Logical Maximum (1800),
> > > > > */
> > > > > -0x65, 0x14, /* Unit (Degrees),
> > > > > */
> > > > > -0x55, 0x0F, /* Unit Exponent (15),
> > > > > */
> > > > > -0x75, 0x10, /* Report Size (16),
> > > > > */
> > > > > -0x81, 0x02, /* Input (Variable),
> > > > > */
> > > > > -0x09, 0x31, /* Changed Usage (Y),
> > > > > */
> > > > > -0x15, 0x00, /* Logical Minimum (0),
> > > > > */
> > > > > -0x26, 0xFF, 0x03, /* Logical Maximum (1023),
> > > > > */
> > > > > -0x75, 0x0C, /* Report Size (12),
> > > > > */
> > > > > -0x81, 0x02, /* Input (Variable),
> > > > > */
> > > > > -0x09, 0x32, /* Changed Usage (Z),
> > > > > */
> > > > > -0x15, 0x00, /* Logical Minimum (0),
> > > > > */
> > > > > -0x26, 0xFF, 0x03, /* Logical Maximum (1023),
> > > > > */
> > > > > -0x75, 0x0C, /* Report Size (12),
> > > > > */
> > > > > -0x81, 0x02, /* Input (Variable),
> > > > > */
> > > > > -0x05, 0x01, /* Usage Page (Desktop),
> > > > > */
> > > > > -0x09, 0x39, /* Usage (Hat Switch),
> > > > > */
> > > > > -0x25, 0x07, /* Logical Maximum (7),
> > > > > */
> > > > > -0x35, 0x00, /* Physical Minimum (0),
> > > > > */
> > > > > -0x46, 0x3B, 0x01, /* Physical Maximum (315),
> > > > > */
> > > > > -0x65, 0x14, /* Unit (Degrees),
> > > > > */
> > > > > -0x75, 0x04, /* Report Size (4),
> > > > > */
> > > > > -0x95, 0x01, /* Report Count (1),
> > > > > */
> > > > > -0x81, 0x02, /* Input (Variable),
> > > > > */
> > > > > -0x25, 0x01, /* Logical Maximum (1),
> > > > > */
> > > > > -0x45, 0x01, /* Physical Maximum (1),
> > > > > */
> > > > > -0x65, 0x00, /* Unit,
> > > > > */
> > > > > -0x75, 0x01, /* Report Size (1),
> > > > > */
> > > > > -0x95, 0x03, /* Report Count (3),
> > > > > */
> > > > > -0x81, 0x01, /* Input (Constant),
> > > > > */
> > > > > -0x05, 0x09, /* Usage Page (Button),
> > > > > */
> > > > > -0x19, 0x01, /* Usage Minimum (01h),
> > > > > */
> > > > > -0x29, 0x11, /* Usage Maximum (11h),
> > > > > */
> > > > > -0x95, 0x11, /* Report Count (17),
> > > > > */
> > > > > -0x81, 0x02, /* Input (Variable),
> > > > > */
> > > > > - /* ---- Dial patch starts here ----
> > > > > */
> > > > > -0x05, 0x01, /* Usage Page (Desktop),
> > > > > */
> > > > > -0x09, 0x33, /* Usage (RX),
> > > > > */
> > > > > -0x75, 0x04, /* Report Size (4),
> > > > > */
> > > > > -0x95, 0x02, /* Report Count (2),
> > > > > */
> > > > > -0x15, 0x00, /* Logical Minimum (0),
> > > > > */
> > > > > -0x25, 0x0b, /* Logical Maximum (b),
> > > > > */
> > > > > -0x81, 0x02, /* Input (Variable),
> > > > > */
> > > > > -0x09, 0x35, /* Usage (RZ),
> > > > > */
> > > > > -0x75, 0x04, /* Report Size (4),
> > > > > */
> > > > > -0x95, 0x01, /* Report Count (1),
> > > > > */
> > > > > -0x25, 0x03, /* Logical Maximum (3),
> > > > > */
> > > > > -0x81, 0x02, /* Input (Variable),
> > > > > */
> > > > > - /* ---- Dial patch ends here ----
> > > > > */
> > > > > -0x06, 0x00, 0xFF, /* Usage Page (FF00h),
> > > > > */
> > > > > -0x09, 0x01, /* Usage (01h),
> > > > > */
> > > > > -0x75, 0x04, /* Changed Report Size (4),
> > > > > */
> > > > > -0x95, 0x0D, /* Changed Report Count (13),
> > > > > */
> > > > > -0x81, 0x02, /* Input (Variable),
> > > > > */
> > > > > -0xC0, /* End Collection,
> > > > > */
> > > > > -0xA1, 0x02, /* Collection (Logical),
> > > > > */
> > > > > -0x09, 0x02, /* Usage (02h),
> > > > > */
> > > > > -0x75, 0x08, /* Report Size (8),
> > > > > */
> > > > > -0x95, 0x10, /* Report Count (16),
> > > > > */
> > > > > -0x91, 0x02, /* Output (Variable),
> > > > > */
> > > > > -0xC0, /* End Collection,
> > > > > */
> > > > > -0xC0 /* End Collection
> > > > > */
> > > > > + 0x05, 0x01, /* Usage Page (Desktop) */
> > > > > + 0x09, 0x08, /* Usage (MultiAxis), Changed */
> > > > > + 0xA1, 0x01, /* Collection (Application), */
> > > > > + 0xA1, 0x02, /* Collection (Logical), */
> > > > > + 0x95, 0x01, /* Report Count (1), */
> > > > > + 0x05, 0x01, /* Changed Usage Page (Desktop), */
> > > > > + 0x09, 0x30, /* Changed Usage (X), */
> > > > > + 0x16, 0xF8, 0xF8, /* Logical Minimum (-
> > > > > 1800),
> > > > > */
> > > > > + 0x26, 0x08, 0x07, /* Logical Maximum
> > > > > (1800),
> > > > > */
> > > > > + 0x65, 0x14, /* Unit (Degrees), */
> > > > > + 0x55, 0x0F, /* Unit Exponent (15), */
> > > > > + 0x75, 0x10, /* Report Size (16), */
> > > > > + 0x81, 0x02, /* Input (Variable), */
> > > > > + 0x09, 0x31, /* Changed Usage (Y), */
> > > > > + 0x15, 0x00, /* Logical Minimum (0), */
> > > > > + 0x26, 0xFF, 0x03, /* Logical Maximum
> > > > > (1023),
> > > > > */
> > > > > + 0x75, 0x0C, /* Report Size (12), */
> > > > > + 0x81, 0x02, /* Input (Variable), */
> > > > > + 0x09, 0x32, /* Changed Usage (Z), */
> > > > > + 0x15, 0x00, /* Logical Minimum (0), */
> > > > > + 0x26, 0xFF, 0x03, /* Logical Maximum
> > > > > (1023),
> > > > > */
> > > > > + 0x75, 0x0C, /* Report Size (12), */
> > > > > + 0x81, 0x02, /* Input (Variable), */
> > > > > + 0x05, 0x01, /* Usage Page (Desktop), */
> > > > > + 0x09, 0x39, /* Usage (Hat Switch), */
> > > > > + 0x25, 0x07, /* Logical Maximum (7), */
> > > > > + 0x35, 0x00, /* Physical Minimum (0), */
> > > > > + 0x46, 0x3B, 0x01, /* Physical Maximum
> > > > > (315),
> > > > > */
> > > > > + 0x65, 0x14, /* Unit (Degrees), */
> > > > > + 0x75, 0x04, /* Report Size (4), */
> > > > > + 0x95, 0x01, /* Report Count (1), */
> > > > > + 0x81, 0x02, /* Input (Variable), */
> > > > > + 0x25, 0x01, /* Logical Maximum (1), */
> > > > > + 0x45, 0x01, /* Physical Maximum (1), */
> > > > > + 0x65, 0x00, /* Unit, */
> > > > > + 0x75, 0x01, /* Report Size (1), */
> > > > > + 0x95, 0x03, /* Report Count (3), */
> > > > > + 0x81, 0x01, /* Input (Constant), */
> > > > > + 0x05, 0x09, /* Usage Page (Button), */
> > > > > + 0x19, 0x01, /* Usage Minimum (01h), */
> > > > > + 0x29, 0x11, /* Usage Maximum (11h), */
> > > > > + 0x95, 0x11, /* Report Count (17), */
> > > > > + 0x81, 0x02, /* Input (Variable), */
> > > > > + /* ---- Dial patch starts here ---- */
> > > > > + 0x05, 0x01, /* Usage Page (Desktop), */
> > > > > + 0x09, 0x33, /* Usage (RX), */
> > > > > + 0x75, 0x04, /* Report Size (4), */
> > > > > + 0x95, 0x02, /* Report Count (2), */
> > > > > + 0x15, 0x00, /* Logical Minimum (0), */
> > > > > + 0x25, 0x0b, /* Logical Maximum (b), */
> > > > > + 0x81, 0x02, /* Input (Variable), */
> > > > > + 0x09, 0x35, /* Usage (RZ), */
> > > > > + 0x75, 0x04, /* Report Size (4), */
> > > > > + 0x95, 0x01, /* Report Count (1), */
> > > > > + 0x25, 0x03, /* Logical Maximum (3), */
> > > > > + 0x81, 0x02, /* Input (Variable), */
> > > > > + /* ---- Dial patch ends here ---- */
> > > > > + 0x06, 0x00, 0xFF, /* Usage Page
> > > > > (FF00h),
> > > > > */
> > > > > + 0x09, 0x01, /* Usage (01h), */
> > > > > + 0x75, 0x04, /* Changed Report Size (4), */
> > > > > + 0x95, 0x0D, /* Changed Report Count (13), */
> > > > > + 0x81, 0x02, /* Input (Variable), */
> > > > > + 0xC0, /* End Collection, */
> > > > > + 0xA1, 0x02, /* Collection (Logical), */
> > > > > + 0x09, 0x02, /* Usage (02h), */
> > > > > + 0x75, 0x08, /* Report Size (8), */
> > > > > + 0x95, 0x10, /* Report Count (16), */
> > > > > + 0x91, 0x02, /* Output (Variable), */
> > > > > + 0xC0, /* End Collection, */
> > > > > + 0xC0 /* End Collection */
> > > > > };
^ permalink raw reply
* Re: [PATCH v5 4/4] Input: Add TouchNetix aXiom I2C Touchscreen support
From: Andrew Thomas @ 2026-02-26 11:08 UTC (permalink / raw)
To: Marco Felsch
Cc: Luis Chamberlain, Russ Weight, Greg Kroah-Hartman,
Rafael J. Wysocki, Andrew Morton, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Dmitry Torokhov, Kamel Bouhara,
Marco Felsch, Henrik Rydberg, Danilo Krummrich, linux-kernel,
devicetree, linux-input
In-Reply-To: <a7hajq5edw3w2pm5l3ytn65kmudjckvaj5relydayoua5i7oha@wsattyisot4s>
On Wed, Feb 25, 2026 at 09:50:11PM +0100, Marco Felsch wrote:
>Hi Andrew,
>
>thanks for the your reply, please see below.
>
>On 26-02-25, Andrew Thomas wrote:
>> On Sun, Jan 11, 2026 at 04:05:47PM +0100, Marco Felsch wrote:
>
>...
>
>> > +struct axiom_u33_rev3 {
>> > + __le32 runtime_crc;
>> > + __le32 runtime_nvm_crc;
>> > + __le32 bootloader_crc;
>> > + __le32 nvltlusageconfig_crc;
>> > + __le32 vltusageconfig_crc;
>> > + __le32 u22_sequencedata_crc;
>> > + __le32 u43_hotspots_crc;
>> > + __le32 u77_dod_data_crc;
>> > + __le32 u93_profiles_crc;
>> > + __le32 u94_deltascalemap_crc;
>> > + __le32 runtimehash_crc;
>> > +};
>> > +
>>
>> I think revision handling should be kept in unpacking where possible.
>> Currently there are 10 revisions of u33, so adding support for many
>> revisions and usages would add alot of code.
>
>This isn't very complex nor very large code. Each u33 rev will add
>44-byte of code, so in the end there will be 440-byte. I've also seen
>that some revisions reduce the size because some fields aren't required.
>E.g. u93_crc is at offset-5.
>
>Furthermore describing the complete layout of u33 allows us using it to
>query the u33 in one i2c-bulk-transfer.
>
Fair enough.
>> > +#define AXIOM_U34 0x34
>> > +#define AXIOM_U34_REV1_OVERFLOW_MASK BIT(7)
>> > +#define AXIOM_U34_REV1_REPORTLENGTH_MASK GENMASK(6, 0)
>> > +#define AXIOM_U34_REV1_PREAMBLE_BYTES 2
>> > +#define AXIOM_U34_REV1_POSTAMBLE_BYTES 4
>
>...
>
>> > +enum axiom_runmode {
>> > + AXIOM_DISCOVERY_MODE,
>> > + AXIOM_TCP_MODE,
>> > + AXIOM_TCP_CFG_UPDATE_MODE,
>> > + AXIOM_BLP_PRE_MODE,
>> > + AXIOM_BLP_MODE,
>> > +};
>>
>> There are only two actual axiom states, bootloader and runtime (TCP).
>> This is more of a driver state rather than an axiom state.
>> Could you label it as such?
>
>Yes I know and the AXIOM_BLP_PRE_MODE will be dropped with the next
>version, which I'm going to send this week!. Not sure why this would be
>required.
>
>> > +struct axiom_data {
>> > + struct input_dev *input;
>> > + struct device *dev;
>> > +
>> > + struct gpio_desc *reset_gpio;
>> > + struct regulator_bulk_data supplies[2];
>> > + unsigned int num_supplies;
>> > +
>> > + struct regmap *regmap;
>> > + struct touchscreen_properties prop;
>> > + bool irq_setup_done;
>> > + u32 poll_interval;
>> > +
>> > + struct drm_panel_follower panel_follower;
>> > + bool is_panel_follower;
>> > +
>> > + enum axiom_runmode mode;
>> > + /*
>> > + * Two completion types to support firmware updates
>> > + * in irq and poll mode.
>> > + */
>> > + struct axiom_completion {
>> > + struct completion completion;
>> > + bool poll_done;
>> > + } nvm_write, boot_complete;
>> > +
>> > + /* Lock to protect both firmware interfaces */
>> > + struct mutex fwupdate_lock;
>> > + struct axiom_firmware {
>> > + /* Lock to protect cancel */
>> > + struct mutex lock;
>> > + bool cancel;
>> > + struct fw_upload *fwl;
>> > + } fw[AXIOM_FW_NUM];
>> > +
>> > + unsigned int fw_major;
>> > + unsigned int fw_minor;
>> > + unsigned int fw_rc;
>> > + unsigned int fw_status;
>> > + unsigned int fw_variant;
>> > + u16 device_id;
>> > + u16 jedec_id;
>> > + u8 silicon_rev;
>> > +
>> > + /* CRCs we need to check during a config update */
>> > + struct axiom_crc {
>> > + u32 runtime;
>> > + u32 vltusageconfig;
>> > + u32 nvltlusageconfig;
>> > + u32 u22_sequencedata;
>> > + u32 u43_hotspots;
>> > + u32 u77_dod_data;
>> > + u32 u93_profiles;
>> > + u32 u94_deltascalemap;
>> > + } crc[AXIOM_CRC_NUM];
>>
>> I think this structure should hold all possible u33 CRCs and then
>> invalid ones can be ignored for the given u33 revision.
>
>Why should be the bootloader CRC interessting? The bootloader can't be
>updated/flashed, at least not according my documentation. Therefore I
>didn't listed the bootloader CRC here.
That is true, there are newer CDUs I did not consider are not available.
I shall hopefully soon update the python so that most usage revisions
can be seen more easily.
However this change could be added later.
>
>> > + bool cds_enabled;
>> > + unsigned long enabled_slots;
>> > + unsigned int num_slots;
>> > +
>> > + unsigned int max_report_byte_len;
>> > + struct axiom_usage_table_entry {
>> > + bool populated;
>> > + unsigned int baseaddr;
>> > + unsigned int size_bytes;
>> > + const struct axiom_usage_info *info;
>> > + } usage_table[AXIOM_MAX_USAGES];
>> > +};
>
>....
>
>> > +static int axiom_u02_swreset(struct axiom_data *ts)
>> > +{
>> > + struct axiom_u02_rev1_system_manager_msg msg = { };
>> > + int ret;
>> > +
>> > + if (!axiom_driver_supports_usage(ts, AXIOM_U02))
>> > + return -EINVAL;
>> > +
>> > + msg.command = cpu_to_le16(AXIOM_U02_REV1_CMD_SOFTRESET);
>> > + ret = axiom_u02_send_msg(ts, &msg, false);
>> > + if (ret)
>> > + return ret;
>> > +
>> > + /*
>> > + * Downstream http://axcfg.py waits for 1sec without checking U01 hello. Tests
>> > + * showed that waiting for the hello message isn't enough therefore we
>> > + * need both to make it robuster.
>> > + */
>> > + ret = axiom_wait_for_completion_timeout(ts, &ts->boot_complete,
>> > + msecs_to_jiffies(1 * MSEC_PER_SEC));
>>
>> Boot can take up to 2s with all selftests enabled.
>
>Thanks for this information :) I will add it.
>
>> > + if (!ret)
>> > + dev_err(ts->dev, "Error swreset timedout\n");
>> > +
>> > + fsleep(USEC_PER_SEC);
>> > +
>> > + return ret ? 0 : -ETIMEDOUT;
>> > +}
>
>...
>
>> > +static int axiom_u02_enter_bootloader(struct axiom_data *ts)
>> > +{
>> > + struct axiom_u02_rev1_system_manager_msg msg = { };
>> > + struct device *dev = ts->dev;
>> > + unsigned int val;
>> > + int ret;
>> > +
>> > + if (!axiom_driver_supports_usage(ts, AXIOM_U02))
>> > + return -EINVAL;
>> > +
>> > + /*
>> > + * Enter the bootloader mode requires 3 consecutive messages so we can't
>> > + * check for the response.
>> > + */
>> > + msg.command = cpu_to_le16(AXIOM_U02_REV1_CMD_ENTERBOOTLOADER);
>> > + msg.parameters[0] = cpu_to_le16(AXIOM_U02_REV1_PARAM0_ENTERBOOLOADER_KEY1);
>> > + ret = axiom_u02_send_msg(ts, &msg, false);
>> > + if (ret) {
>> > + dev_err(dev, "Failed to send bootloader-key1: %d\n", ret);
>> > + return ret;
>> > + }
>>
>> A delay is required between commands. 10ms is fine.
>
>Can I make use of the axiom_u02_wait_idle() logic which checks the
>AXIOM_U02_REV1_RESP_SUCCESS? Arbitrary delays are always a source of
>trouble.
Yes, I tested with axiom_u02_wait_idle() which is OK.
I am slightly worried about too short a delay to axiom causing instability,
however this works fine.
It can unfortunately be an unstable device..
>
>> > + msg.parameters[0] = cpu_to_le16(AXIOM_U02_REV1_PARAM0_ENTERBOOLOADER_KEY2);
>> > + ret = axiom_u02_send_msg(ts, &msg, false);
>> > + if (ret) {
>> > + dev_err(dev, "Failed to send bootloader-key2: %d\n", ret);
>> > + return ret;
>> > + }
>>
>> And here.
>>
>> > +
>> > + msg.parameters[0] = cpu_to_le16(AXIOM_U02_REV1_PARAM0_ENTERBOOLOADER_KEY3);
>> > + ret = axiom_u02_send_msg(ts, &msg, false);
>> > + if (ret) {
>> > + dev_err(dev, "Failed to send bootloader-key3: %d\n", ret);
>> > + return ret;
>> > + }
>> > +
>> > + /* Sleep before the first read to give the device time */
>> > + fsleep(250 * USEC_PER_MSEC);
>> > +
>> > + /* Wait till the device reports it is in bootloader mode */
>> > + return regmap_read_poll_timeout(ts->regmap,
>> > + AXIOM_U31_REV1_DEVICE_ID_HIGH_REG, val,
>> > + FIELD_GET(AXIOM_U31_REV1_MODE_MASK, val) ==
>> > + AXIOM_U31_REV1_MODE_BLP, 250 * USEC_PER_MSEC,
>> > + USEC_PER_SEC);
>> > +}
>>
>> Just to note if we cannot enter bootloader with u02 due to a corrupted firmware,
>> you can enter bootloader if the nRESET line is toggled 5 times without comms.
>
>This could be added later on by $dev (maybe you :)) since I can't test
>this. Our system has the reset line not connected :/
Sounds good.
>
>...
>
>> > +static int axiom_u33_read(struct axiom_data *ts, struct axiom_crc *crc)
>> > +{
>> > + struct device *dev = ts->dev;
>> > + unsigned int reg;
>> > + int ret;
>> > +
>> > + if (!axiom_driver_supports_usage(ts, AXIOM_U33))
>> > + return -EINVAL;
>> > +
>> > + if (axiom_usage_rev(ts, AXIOM_U33) == 2) {
>> > + struct axiom_u33_rev2 val;
>> > +
>> > + reg = axiom_usage_baseaddr(ts, AXIOM_U33);
>> > + ret = regmap_raw_read(ts->regmap, reg, &val, sizeof(val));
>>
>> Could we read into a raw buffer to save having to define a little endian
>> version of the CRCs?
>
>I don't see the benefit.
OK.
>
>...
>
>> > +/* Custom regmap read/write handling is required due to the aXiom protocol */
>> > +static int axiom_regmap_read(void *context, const void *reg_buf, size_t reg_size,
>> > + void *val_buf, size_t val_size)
>> > +{
>> > + struct device *dev = context;
>> > + struct i2c_client *i2c = to_i2c_client(dev);
>> > + struct axiom_data *ts = i2c_get_clientdata(i2c);
>> > + struct axiom_cmd_header hdr;
>> > + u16 xferlen, addr, baseaddr;
>> > + struct i2c_msg xfer[2];
>> > + int ret;
>> > +
>> > + if (val_size > AXIOM_MAX_XFERLEN) {
>> > + dev_err(ts->dev, "Exceed max xferlen: %zu > %u\n",
>> > + val_size, AXIOM_MAX_XFERLEN);
>> > + return -EINVAL;
>> > + }
>> > +
>> > + addr = *((u16 *)reg_buf);
>> > + hdr.target_address = cpu_to_le16(addr);
>> > + xferlen = FIELD_PREP(AXIOM_CMD_HDR_DIR_MASK, AXIOM_CMD_HDR_READ) |
>> > + FIELD_PREP(AXIOM_CMD_HDR_LEN_MASK, val_size);
>> > + hdr.xferlen = cpu_to_le16(xferlen);
>> > +
>> > + /* Verify that usage including the usage rev is supported */
>> > + baseaddr = addr & AXIOM_USAGE_BASEADDR_MASK;
>> > + if (!axiom_usage_supported(ts, baseaddr))
>> > + return -EINVAL;
>> > +
>> > + xfer[0].addr = i2c->addr;
>> > + xfer[0].flags = 0;
>> > + xfer[0].len = sizeof(hdr);
>> > + xfer[0].buf = (u8 *)&hdr;
>> > +
>> > + xfer[1].addr = i2c->addr;
>> > + xfer[1].flags = I2C_M_RD;
>> > + xfer[1].len = val_size;
>> > + xfer[1].buf = val_buf;
>> > +
>> > + ret = i2c_transfer(i2c->adapter, xfer, 2);
>> > + if (ret == 2)
>> > + return 0;
>> > + else if (ret < 0)
>> > + return ret;
>> > + else
>> > + return -EIO;
>> > +}
>>
>> There needs to be atleast 40us holdoff between axiom bus transfers.
>> I am not sure that has been considered here.
>
>Is this written somewhere within the datasheet/programming-guide?
In aXiom Comms Protocol in v4.8.9 if you have access to the webportal
it says to use 40us holdoff for report reading. Although this may apply
to all transactions.
Doing comms while axiom is changing the DMA causes issues (NAKs), for the
driver I posted atleast, the holdoff was required otherwise I would receive
0-length reports frequently.
It looks to be fine currently, however if there is unstability for users
we should consider adding this or something similar.
>
>...
>
>> > +static enum fw_upload_err
>> > +axiom_cfg_fw_prepare(struct fw_upload *fw_upload, const u8 *data, u32 size)
>> > +{
>
>...
>
>> > + cur_runtime_crc = ts->crc[AXIOM_CRC_CUR].runtime;
>> > + fw_runtime_crc = ts->crc[AXIOM_CRC_NEW].runtime;
>> > + if (cur_runtime_crc != fw_runtime_crc) {
>> > + dev_err(dev, "TH2CFG and device runtime CRC doesn't match: %#x != %#x\n",
>> > + fw_runtime_crc, cur_runtime_crc);
>> > + ret = FW_UPLOAD_ERR_FW_INVALID;
>> > + goto out;
>> > + }
>>
>> The firmware CRCs dont need to match for a config load, only the usage revision/length.
>
>What difference does it make? The firmware CRC implicit includes the
>usage revision and the length (register layout). So we can ensure that
>the configuration was made for the correct register layout without
>checking each register and revision.
Different firmware revisions/CRCs can have compatible usages.
Aslong as the usage revisions match to u31 the usages will be compatible.
For us atleast we have different CRCs for small firmware changes, therefore
if testing such firmware here we would always have to uncomment this section.
In the updated python I changed the check to the following:
# Compare the firmware runtime CRC from the file with the CRC from the device.
# Only proceed if the CRCs match.
if not force:
if u33_from_file.fld_runtime_crc != ax.u33.fld_runtime_crc:
logging.error("Cannot load config file as it was saved from a different revision of firmware:")
logging.error("Firmware info from device : 0x{0:08X}, {1}".format(ax.u33.fld_runtime_crc, ax.u31.get_device_info_short()))
logging.error("Firmware info from config file : 0x{0:08X}, {1}".format(u33_from_file.fld_runtime_crc, u31_from_file.get_device_info_short()))
return ERROR_CFG_FILE_NOT_COMPATIBLE
else:
if u33_from_file.fld_runtime_crc != ax.u33.fld_runtime_crc:
logging.warning("The config file was saved from a different revision of firmware therefore it may not be compatible:")
logging.warning("Firmware info from device : 0x{0:08X}, {1}".format(ax.u33.fld_runtime_crc, ax.u31.get_device_info_short()))
logging.warning("Firmware info from config file : 0x{0:08X}, {1}".format(u33_from_file.fld_runtime_crc, u31_from_file.get_device_info_short()))
# Now ensure the config is compatible with the device
valid_cfg = False
file_usage_table = u31_from_file.get_usage_table()
for usage in usages.keys():
if usage not in ax.u31.get_usages():
logging.error(f"Usage u{usage:02x} unsupported on this device.")
break
usage_entry = ax.u31.get_usage_entry(usage)
if usage_entry.start_page != file_usage_table[usage].start_page:
logging.error(f"Incompatible config address for u{usage:02x}:")
logging.error(f" Device: 0x{usage_entry.start_page:02x}00 File: 0x{file_usage_table[usage].start_page:02x}00")
break
if ax.get_usage_length(usage) != len(usages[usage][2]):
logging.error(f"Incompatible config length for u{usage:02x}:")
logging.error(f" Device: {ax.get_usage_length(usage)} File: {len(usages[usage][2])}")
break
else:
valid_cfg = True
if not valid_cfg:
logging.error("Cannot load config as the usages are incompatible with the device.")
return ERROR_CFG_FILE_NOT_COMPATIBLE
Possibly we could add a force parameter to the sysfs like above?
>
>...
>
>> > +static enum fw_upload_err
>> > +axiom_cfg_fw_write(struct fw_upload *fw_upload, const u8 *data, u32 offset,
>> > + u32 size, u32 *written)
>> > +{
>
>....
>
>> > + /* Ensure that the chunks are written correctly */
>> > + ret = axiom_verify_volatile_mem(ts);
>> > + if (ret) {
>> > + dev_err(dev, "Failed to verify written config, abort\n");
>> > + goto err_swreset;
>> > + }
>> > +
>> > + ret = axiom_u02_save_config(ts);
>> > + if (ret)
>> > + goto err_swreset;
>> > +
>> > + /*
>> > + * TODO: Check if u02 start would be sufficient to load the new config
>> > + * values
>> > + */
>>
>> It is not necessarily needed.
>
>What do you mean by this? Do we need the axiom_u02_swreset() or can we
>just start the system via u02 (without swreset)?
There is no need to do a reset after a config load you can just start the AE
with CMD_START, but we can keep it as is since it does the same thing.
>
>>
>> > + ret = axiom_u02_swreset(ts);
>> > + if (ret) {
>> > + dev_err(dev, "Soft reset failed\n");
>> > + goto err_unlock;
>> > + }
>
>....
>
>> > +static ssize_t fw_variant_show(struct device *dev,
>> > + struct device_attribute *attr, char *buf)
>> > +{
>> > + struct i2c_client *i2c = to_i2c_client(dev);
>> > + struct axiom_data *ts = i2c_get_clientdata(i2c);
>> > + const char *val;
>> > +
>> > + switch (ts->fw_variant) {
>> > + case 0:
>> > + val = "3d";
>> > + break;
>> > + case 1:
>> > + val = "2d";
>> > + break;
>> > + case 3:
>> > + val = "force";
>> > + break;
>> > + default:
>> > + val = "unknown";
>> > + break;
>> > + }
>>
>> The following are all the variants we currently support in order:
>> FW_VARIANTS = ["3D", "2D", "FORCE", "0D", "XL"]
>
>Means:
>
>0 == 3d
>1 == 2d
>3 == force
>4 == 0d
>5 == xl
>
>?
>
>This is also something I can test on my site. Patches are welcome once
>this is mainline of course :)
It is like so:
#define DEVICE_BUILD_VARIANT_3D (0U)
#define DEVICE_BUILD_VARIANT_2D (1U)
#define DEVICE_BUILD_VARIANT_FORCE (2U)
#define DEVICE_BUILD_VARIANT_0D (3U)
#define DEVICE_BUILD_VARIANT_XL (4U)
>
>Regards,
> Marco
>
>--
>#gernperDu
>#CallMeByMyFirstName
>
>Pengutronix e.K. | |
>Steuerwalder Str. 21 | https://www.pengutronix.de |
>31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
>Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
I shall try to give a more prompt review once you have the new version up.
Many Thanks,
Andrew
^ permalink raw reply
* Re: [PATCH 1/1] HID: uhid: Fix out-of-bounds write caused by raw events mismanagement
From: Lee Jones @ 2026-02-26 11:18 UTC (permalink / raw)
To: Jiri Kosina
Cc: Benjamin Tissoires, David Rheinsberg, linux-input, linux-kernel,
stable
In-Reply-To: <r6574n79-563r-9rrp-0n92-784532r67o63@xreary.bet>
On Tue, 24 Feb 2026, Jiri Kosina wrote:
> On Tue, 24 Feb 2026, Benjamin Tissoires wrote:
>
> > Long story short: that patch is too intrusive as it makes assumption on
> > the behavior of the device. We need to understand where/if the bug was
> > spotted and fix the caller of hid_hw_raw_request, not the uhid
> > implementation.
>
> Thanks a lot for the analysis, Benjamin!
>
> I asked about that here:
>
> https://lore.kernel.org/all/172q4775-616s-p7s4-7n80-p8579n0r3516@xreary.bet/
>
> So let's wait for Lee to clarify. Until that, the patch stays out of the
> branch.
Thanks to both of you for looking into this. I appreciate your efforts.
This is very much real world.
Is there a way to add an errata for the PS3 controller?
--
Lee Jones [李琼斯]
^ permalink raw reply
* Re: [PATCH v3 0/4] input: add GPIO-based charlieplex keypad
From: Geert Uytterhoeven @ 2026-02-26 9:20 UTC (permalink / raw)
To: Hugo Villeneuve
Cc: robin, andy, robh, krzk+dt, conor+dt, dmitry.torokhov,
hvilleneuve, mkorpershoek, matthias.bgg,
angelogioacchino.delregno, lee, alexander.sverdlin, marek.vasut,
akurz, devicetree, linux-kernel, linux-input, linux-arm-kernel,
linux-mediatek
In-Reply-To: <20260225155409.612478-1-hugo@hugovil.com>
Hi Hugo,
Thanks for your series!
On Wed, 25 Feb 2026 at 16:54, Hugo Villeneuve <hugo@hugovil.com> wrote:
> From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
>
> Hello,
> this patch series add a new GPIO charlieplex keypad driver.
>
> The first two patches simply commonize two properties that are present in
> a few bindings, so that the actual patches for the charlieplex keypad driver
> can reuse them instead of also redefining them.
>
> I have tested the driver on a custom board with a Solidrun RZ/G2LC SOM
> with three charlieplex keyboards, all connected thru a single PCA9416 I2C GPIO
PCA9416? The closest I could find is TCA9416, which is something
different.
> expander.
>
> Link: [v1] https://lore.kernel.org/all/20260203155023.536103-1-hugo@hugovil.com/
> Link: [v2] https://lore.kernel.org/all/20260213171431.2228814-1-hugo@hugovil.com/
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH 1/1] HID: uhid: Fix out-of-bounds write caused by raw events mismanagement
From: Benjamin Tissoires @ 2026-02-26 12:22 UTC (permalink / raw)
To: Lee Jones
Cc: Jiri Kosina, David Rheinsberg, linux-input, linux-kernel, stable
In-Reply-To: <20260226111816.GA8023@google.com>
On Feb 26 2026, Lee Jones wrote:
> On Tue, 24 Feb 2026, Jiri Kosina wrote:
>
> > On Tue, 24 Feb 2026, Benjamin Tissoires wrote:
> >
> > > Long story short: that patch is too intrusive as it makes assumption on
> > > the behavior of the device. We need to understand where/if the bug was
> > > spotted and fix the caller of hid_hw_raw_request, not the uhid
> > > implementation.
> >
> > Thanks a lot for the analysis, Benjamin!
> >
> > I asked about that here:
> >
> > https://lore.kernel.org/all/172q4775-616s-p7s4-7n80-p8579n0r3516@xreary.bet/
> >
> > So let's wait for Lee to clarify. Until that, the patch stays out of the
> > branch.
>
> Thanks to both of you for looking into this. I appreciate your efforts.
>
> This is very much real world.
>
> Is there a way to add an errata for the PS3 controller?
>
Unfortunatelly no. uhid merely emulates what a device can do, and HID is
a convention. So if we were to have a special case to PS3 controllers,
we would then start having to maintain an endless list of quirks when
the issue is *not* in uhid, but in the processing of the device after
(maybe in hid-core?).
Cheers,
Benjamin
^ permalink raw reply
* Re: [PATCH 1/1] HID: uhid: Fix out-of-bounds write caused by raw events mismanagement
From: Lee Jones @ 2026-02-26 14:08 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Jiri Kosina, David Rheinsberg, linux-input, linux-kernel, stable
In-Reply-To: <aaA6fioiB9_aiBrA@plouf>
On Thu, 26 Feb 2026, Benjamin Tissoires wrote:
> On Feb 26 2026, Lee Jones wrote:
> > On Tue, 24 Feb 2026, Jiri Kosina wrote:
> >
> > > On Tue, 24 Feb 2026, Benjamin Tissoires wrote:
> > >
> > > > Long story short: that patch is too intrusive as it makes assumption on
> > > > the behavior of the device. We need to understand where/if the bug was
> > > > spotted and fix the caller of hid_hw_raw_request, not the uhid
> > > > implementation.
> > >
> > > Thanks a lot for the analysis, Benjamin!
> > >
> > > I asked about that here:
> > >
> > > https://lore.kernel.org/all/172q4775-616s-p7s4-7n80-p8579n0r3516@xreary.bet/
> > >
> > > So let's wait for Lee to clarify. Until that, the patch stays out of the
> > > branch.
> >
> > Thanks to both of you for looking into this. I appreciate your efforts.
> >
> > This is very much real world.
> >
> > Is there a way to add an errata for the PS3 controller?
> >
>
> Unfortunatelly no. uhid merely emulates what a device can do, and HID is
> a convention. So if we were to have a special case to PS3 controllers,
> we would then start having to maintain an endless list of quirks when
> the issue is *not* in uhid, but in the processing of the device after
> (maybe in hid-core?).
Actually I think the issue is in UHID. At least the way I read it.
Are there legitimate use-cases for devices overwriting the Report ID
contained in the first index of the data buffer? From my very limited
knowledge of the subsystem, this sounds like an oversight.
--
Lee Jones [李琼斯]
^ permalink raw reply
* Re: [PATCH v3 0/4] input: add GPIO-based charlieplex keypad
From: Hugo Villeneuve @ 2026-02-26 14:27 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: robin, andy, robh, krzk+dt, conor+dt, dmitry.torokhov,
hvilleneuve, mkorpershoek, matthias.bgg,
angelogioacchino.delregno, lee, alexander.sverdlin, marek.vasut,
akurz, devicetree, linux-kernel, linux-input, linux-arm-kernel,
linux-mediatek
In-Reply-To: <CAMuHMdVd0Ds5vHQa+MG2E+E36qAu5HQE3b+Vzpbv84MM=2DFWw@mail.gmail.com>
Hi Geert,
On Thu, 26 Feb 2026 10:20:33 +0100
Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Hi Hugo,
>
> Thanks for your series!
>
> On Wed, 25 Feb 2026 at 16:54, Hugo Villeneuve <hugo@hugovil.com> wrote:
> > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> >
> > Hello,
> > this patch series add a new GPIO charlieplex keypad driver.
> >
> > The first two patches simply commonize two properties that are present in
> > a few bindings, so that the actual patches for the charlieplex keypad driver
> > can reuse them instead of also redefining them.
> >
> > I have tested the driver on a custom board with a Solidrun RZ/G2LC SOM
> > with three charlieplex keyboards, all connected thru a single PCA9416 I2C GPIO
>
> PCA9416? The closest I could find is TCA9416, which is something
> different.
Its a typo, it is PCA6416, thanks for catching that!
Hugo.
>
> > expander.
> >
> > Link: [v1] https://lore.kernel.org/all/20260203155023.536103-1-hugo@hugovil.com/
> > Link: [v2] https://lore.kernel.org/all/20260213171431.2228814-1-hugo@hugovil.com/
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
>
--
Hugo Villeneuve
^ permalink raw reply
* Re: [PATCH 0/5] hid-pidff set_condition compatibility fixes
From: Jiri Kosina @ 2026-02-26 14:46 UTC (permalink / raw)
To: Tomasz Pakuła; +Cc: bentiss, oleg, linux-input, linux-kernel
In-Reply-To: <20260203174531.2866644-1-tomasz.pakula.oficjalny@gmail.com>
On Tue, 3 Feb 2026, Tomasz Pakuła wrote:
> This patch series adds a few more quirks to the hid-pidff driver to allow for
> greater compatibility with HID PID devices in the wild. Some omit selected
> fields based on the permissiveness of the Windows driver/DirectInput.
>
> All the combined quirks allow for Conditional effects playback on affected
> devices simply by following DirectInput and skipping missing fields. This brings
> force feedback feeling in line with other platforms for affected devices.
>
> Last patch updates MAINTAINERS to officially take ownership of hid-pidff driver.
>
> Tomasz Pakuła (5):
> HID: pidff: Refactor field quirks detection
> HID: pidff: Add MISSING_NEG_COEFFICIENT quirk
> HID: pidff: Add MISSING_NEG_SATURATION quirk
> HID: pidff: Add MISSING_DEADBAND quirk
> HID: Update MAINTAINERS for USB HID PID
>
> MAINTAINERS | 17 +++++-----
> drivers/hid/usbhid/hid-pidff.c | 57 ++++++++++++++++++++++------------
> drivers/hid/usbhid/hid-pidff.h | 9 ++++++
> 3 files changed, 56 insertions(+), 27 deletions(-)
Now in hid.git#for-7.1/pidff. Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v3 3/4] dt-bindings: input: add GPIO charlieplex keypad
From: Hugo Villeneuve @ 2026-02-26 14:48 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: robin, andy, robh, krzk+dt, conor+dt, dmitry.torokhov,
hvilleneuve, mkorpershoek, matthias.bgg,
angelogioacchino.delregno, lee, alexander.sverdlin, marek.vasut,
akurz, devicetree, linux-kernel, linux-input, linux-arm-kernel,
linux-mediatek
In-Reply-To: <CAMuHMdVdYX9p9DfDoyMv8qEm52kY51QULkEnuxRBH2OyWyYf6g@mail.gmail.com>
Hi Geert,
On Thu, 26 Feb 2026 10:32:30 +0100
Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Hi Hugo,
>
> On Wed, 25 Feb 2026 at 16:54, Hugo Villeneuve <hugo@hugovil.com> wrote:
> > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> >
> > Add DT bindings for GPIO charlieplex keypad.
> >
> > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
>
> Thanks for your patch!
>
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml
> > @@ -0,0 +1,106 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +
> > +$id: http://devicetree.org/schemas/input/gpio-charlieplex-keypad.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: GPIO charlieplex keypad
> > +
> > +maintainers:
> > + - Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > +
> > +description: |
> > + The charlieplex keypad supports N^2)-N different key combinations (where N is
> > + the number of lines). Key presses and releases are detected by configuring
> > + only one line as output at a time, and reading other line states. This process
> > + is repeated for each line. Diodes are required to ensure current flows in only
> > + one direction between any pair of pins.
> > + This mechanism doesn't allow to detect simultaneous key presses.
>
> Indeed, e.g. pressing S1 and S2 simultaneously will show a ghost
> S5 keypress.
>
> > +
> > + Wiring example for 3 lines keyboard with 6 switches and 3 diodes:
> > +
> > + L0 --+---------------------+----------------------+
> > + | | |
> > + L1 -------+-----------+---------------------+ |
> > + | | | | | |
> > + L2 -------------+----------------+-----+ | |
> > + | | | | | | | | |
> > + | | | | | | | | |
> > + | S1 \ S2 \ | S3 \ S4 \ | S5 \ S6 \
> > + | | | | | | | | |
> > + | +--+--+ | +--+--+ | +--+--+
> > + | | | | | |
> > + | D1 v | D2 v | D3 v
> > + | - (k) | - (k) | - (k)
> > + | | | | | |
> > + +-------+ +-------+ +-------+
>
> Don't you need pull-down resistors on L[0-2], and/or a way to specify
> in DT to enable internal poll-down on GPIO controllers that support it?
> Some controllers may support internal pull-up only, but I guess that
> can be handled using GPIO_ACTIVE_LOW?
Yes, I did not put any resistors in the diagram in order to keep it
simple and clear.
You need pull-down resistors either on the board, or specified in the
DT like we do for our board (we use the PCAL6416 with pull-up/down
support):
line-gpios = <&gpio20 0 (GPIO_ACTIVE_HIGH | GPIO_PULL_DOWN)
...
I will add this to the example, and add a few lines explaining this in
the description.
> > +
> > + L: GPIO line
> > + S: switch
> > + D: diode (k indicates cathode)
> > +
> > +allOf:
> > + - $ref: input.yaml#
> > + - $ref: /schemas/input/matrix-keymap.yaml#
> > +
> > +properties:
> > + compatible:
> > + const: gpio-charlieplex-keypad
> > +
> > + autorepeat: true
> > +
> > + debounce-delay-ms:
> > + default: 5
> > +
> > + line-gpios:
> > + description:
> > + List of GPIOs used as lines. The gpio specifier for this property
> > + depends on the gpio controller to which these lines are connected.
> > +
> > + linux,keymap: true
> > +
> > + poll-interval: true
> > +
> > + settling-time-us: true
> > +
> > + wakeup-source: true
> > +
> > +required:
> > + - compatible
> > + - line-gpios
> > + - linux,keymap
> > + - poll-interval
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/gpio/gpio.h>
> > + #include <dt-bindings/input/input.h>
> > +
> > + charlieplex-keypad {
>
> "keyboard", as per Devicetree Specification Generic Names
> Recommendation.
Ok.
Thank you,
Hugo.
>
> > + compatible = "gpio-charlieplex-keypad";
> > + debounce-delay-ms = <20>;
> > + poll-interval = <5>;
> > + settling-time-us = <2>;
> > +
> > + line-gpios = <&gpio2 25 GPIO_ACTIVE_HIGH
> > + &gpio2 26 GPIO_ACTIVE_HIGH
> > + &gpio2 27 GPIO_ACTIVE_HIGH>;
> > +
> > + /* MATRIX_KEY(output, input, key-code) */
> > + linux,keymap = <
> > + /*
> > + * According to wiring diagram above, if L1 is configured as
> > + * output and HIGH, and we detect a HIGH level on input L0,
> > + * then it means S1 is pressed: MATRIX_KEY(L1, L0, KEY...)
> > + */
> > + MATRIX_KEY(1, 0, KEY_F1) /* S1 */
> > + MATRIX_KEY(2, 0, KEY_F2) /* S2 */
> > + MATRIX_KEY(0, 1, KEY_F3) /* S3 */
> > + MATRIX_KEY(2, 1, KEY_F4) /* S4 */
> > + MATRIX_KEY(1, 2, KEY_F5) /* S5 */
> > + MATRIX_KEY(0, 2, KEY_F6) /* S6 */
> > + >;
> > + };
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
>
--
Hugo Villeneuve
^ permalink raw reply
* Re: [PATCH V3 RESEND 2/2] HID: i2c-hid: elan: Add parade-tc3408 timing
From: Jiri Kosina @ 2026-02-26 14:52 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Benjamin Tissoires, Doug Anderson, Langyan Ye, robh, krzk+dt,
conor+dt, linux-input, devicetree, linux-kernel
In-Reply-To: <aZ_SqXA8ad3SAD3J@google.com>
On Wed, 25 Feb 2026, Dmitry Torokhov wrote:
> > > > Parade-tc3408 requires reset to pull down time greater than 10ms,
> > > > so the configuration post_power_delay_ms is 10, and the chipset
> > > > initial time is required to be greater than 300ms,
> > > > so the post_gpio_reset_on_delay_ms is set to 300.
> > > >
> > > > Signed-off-by: Langyan Ye <yelangyan@huaqin.corp-partner.google.com>
> > > > Reviewed-by: Douglas Anderson <dianders@chromium.org>
> > >
> > > Jiri, Benjamin, another I2C hid with bindings...
> >
> > I guess maybe this could go through the input tree, but it needs
> > either Jiri's or Benjamin's ack? Is that the plan?
>
> Jiri, Benjamin,
>
> I am going to pick up the DT binding changes, do you want me to pick up
> the driver changes as well or do you want to merge them through your
> tree?
I think it's easier if it goes all together to avoid cross-tree
dependencies.
Please feel free to add
Acked-by: Jiri Kosina <jkosina@suse.com>
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: intel-ish-hid: ipc: Add Nova Lake-H/S PCI device IDs
From: Jiri Kosina @ 2026-02-26 14:54 UTC (permalink / raw)
To: Zhang Lixu
Cc: linux-input, srinivas.pandruvada, benjamin.tissoires,
andriy.shevchenko, selina.wang
In-Reply-To: <20260203005507.44430-1-lixu.zhang@intel.com>
On Tue, 3 Feb 2026, Zhang Lixu wrote:
> Add device IDs of Nova Lake-H and Nova Lake-S into ishtp support list.
>
> Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Now in hid.git#for-7.0/upstream-fixes, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH RESEND v2] HID: winwing: Enable rumble effects
From: Jiri Kosina @ 2026-02-26 14:58 UTC (permalink / raw)
To: Ivan Gorinov; +Cc: linux-input, linux-kernel
In-Reply-To: <20260226054913.GA8668@altimeter-info>
On Thu, 26 Feb 2026, Ivan Gorinov wrote:
> Enable rumble motor control on TGRIP-15E and TGRIP-15EX throttle grips
> by sending haptic feedback commands (EV_FF events) to the input device.
>
> Signed-off-by: Ivan Gorinov <linux-kernel@altimeter.info>
[ ... snip ... ]
> +static int winwing_haptic_rumble(struct winwing_drv_data *data)
> +{
> + __u8 *buf;
> + __u8 m;
> +
> + if (!data)
> + return -EINVAL;
> +
> + if (!data->hdev)
> + return -EINVAL;
> +
> + buf = data->report_rumble;
> +
> + if (!buf)
> + return -EINVAL;
> +
> + m = convert_magnitude(data->rumble.strong_magnitude);
> + if (m != data->rumble_left) {
> + int ret;
> +
> + buf[0] = 0x02;
> + buf[1] = 0x01;
> + buf[2] = 0xbf;
> + buf[3] = 0x00;
> + buf[4] = 0x00;
> + buf[5] = 0x03;
> + buf[6] = 0x49;
> + buf[7] = 0x00;
> + buf[8] = m;
> + buf[9] = 0x00;
> + buf[10] = 0;
> + buf[11] = 0;
> + buf[12] = 0;
> + buf[13] = 0;
> +
> + ret = hid_hw_output_report(data->hdev, buf, 14);
> + if (ret < 0) {
> + hid_err(data->hdev, "error %d (%*ph)\n", ret, 14, buf);
> + return ret;
> + }
> + data->rumble_left = m;
> + }
> +
> + m = convert_magnitude(data->rumble.weak_magnitude);
> + if (m != data->rumble_right) {
> + int ret;
> +
> + buf[0] = 0x02;
> + buf[1] = 0x03;
> + buf[2] = 0xbf;
> + buf[3] = 0x00;
> + buf[4] = 0x00;
> + buf[5] = 0x03;
> + buf[6] = 0x49;
> + buf[7] = 0x00;
> + buf[8] = m;
> + buf[9] = 0x00;
> + buf[10] = 0;
> + buf[11] = 0;
> + buf[12] = 0;
> + buf[13] = 0;
Do these magic numbers have any real meaning, or is it just mimicking
observed binary stream?
It'd be nice to have at least short comment explaining it.
Thanks!
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: apple: Add EPOMAKER TH87 to the non-apple keyboards list
From: Jiri Kosina @ 2026-02-26 15:01 UTC (permalink / raw)
To: Takashi Iwai; +Cc: linux-input, Benjamin Tissoires, linux-kernel
In-Reply-To: <20260224090004.73980-1-tiwai@suse.de>
On Tue, 24 Feb 2026, Takashi Iwai wrote:
> EPOMAKER TH87 has the very same ID as Apple Aluminum keyboard
> (05ac:024f) although it doesn't work as expected in compatible way.
>
> Put three entries to the non-apple keyboards list to exclude this
> device: one for BT ("TH87"), one for USB ("HFD Epomaker TH87") and one
> for dongle ("2.4G Wireless Receiver").
Applied to hid.git#for-7.0/upstream-fixes, thanks Takashi.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCHv2 1/2] hid: hid-pl: handle probe errors
From: Jiri Kosina @ 2026-02-26 15:06 UTC (permalink / raw)
To: Oliver Neukum; +Cc: bentiss, linux-input, stable
In-Reply-To: <20260216134958.260648-2-oneukum@suse.com>
On Mon, 16 Feb 2026, Oliver Neukum wrote:
> Errors in init must be reported back or we'll
> follow a NULL pointer the first time FF is used,
> because plff_init() initializes the private member.
>
> V2: resend full series
This one is in Linus' tree as 3756a272d2cf3 already.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH 2/2] hid: hid-pl: eliminate private debug macro
From: Jiri Kosina @ 2026-02-26 15:07 UTC (permalink / raw)
To: Oliver Neukum; +Cc: bentiss, linux-input
In-Reply-To: <20260216134958.260648-3-oneukum@suse.com>
On Mon, 16 Feb 2026, Oliver Neukum wrote:
> Use proper dynamic debugging.
>
> V2: resend full series
>
> Signed-off-by: Oliver Neukum <oneukum@suse.com>
> ---
> drivers/hid/hid-pl.c | 12 ++++--------
> 1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/hid/hid-pl.c b/drivers/hid/hid-pl.c
> index dc11d5322fc0..4b6b33597d11 100644
> --- a/drivers/hid/hid-pl.c
> +++ b/drivers/hid/hid-pl.c
> @@ -24,10 +24,6 @@
> */
>
>
> -/* #define DEBUG */
> -
> -#define debug(format, arg...) pr_debug("hid-plff: " format "\n" , ## arg)
> -
> #include <linux/input.h>
> #include <linux/slab.h>
> #include <linux/module.h>
> @@ -53,14 +49,14 @@ static int hid_plff_play(struct input_dev *dev, void *data,
>
> left = effect->u.rumble.strong_magnitude;
> right = effect->u.rumble.weak_magnitude;
> - debug("called with 0x%04x 0x%04x", left, right);
> + dev_dbg(&dev->dev, "called with 0x%04x 0x%04x", left, right);
Can you please use hid_dbg() instead?
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v5] hid/hid-multitouch: Keep latency normal on deactivate for reactivation gesture
From: Jiri Kosina @ 2026-02-26 15:10 UTC (permalink / raw)
To: Werner Sembach; +Cc: Benjamin Tissoires, linux-input, linux-kernel
In-Reply-To: <20260108161139.32681-1-wse@tuxedocomputers.com>
On Thu, 8 Jan 2026, Werner Sembach wrote:
> Uniwill devices have a built in gesture in the touchpad to de- and
> reactivate it by double taping the upper left corner. This gesture stops
> working when latency is set to high, so this patch keeps the latency on
> normal.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
> ---
> V1->V2: Use a quirk to narrow down the devices this is applied to.
> V2->V3: Fix this patch breaking touchpads on some devices.
> Add another device ID.
> V3->V4: Readd quirks formerly applied to the devices via the default class.
> V4->V5: Fix whitespace error.
Applied to hid.git#for-7.0/upstream-fixes. Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] Input: evdev: fix data race in evdev_read() and evdev_poll()
From: Osama Abdelkader @ 2026-02-26 15:17 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input, linux-kernel; +Cc: syzbot+1b327485934adf39955b
In-Reply-To: <20260208194516.172227-1-osama.abdelkader@gmail.com>
On Sun, Feb 08, 2026 at 08:45:15PM +0100, Osama Abdelkader wrote:
> Protect all reads of client->packet_head with buffer_lock to fix a
> KCSAN-reported data race. The race occurs between:
>
> - evdev_pass_values() writing to packet_head (protected by buffer_lock)
> - evdev_read() reading packet_head without lock protection
> - evdev_poll() reading packet_head without lock protection
>
> The fix ensures all accesses to packet_head are protected by buffer_lock,
> matching the existing write-side protection pattern used in
> evdev_pass_values() and evdev_fetch_next_event().
>
> Changes:
> - evdev_read(): Protect packet_head read in O_NONBLOCK check
> - evdev_read(): Protect packet_head read in wait loop condition
> - evdev_poll(): Protect packet_head read in poll check
>
> KCSAN report:
> BUG: KCSAN: data-race in evdev_pass_values / evdev_read
>
> write to 0xffff888104842008 of 4 bytes by task 8439 on cpu 1:
> __pass_event drivers/input/evdev.c:239 [inline]
> evdev_pass_values+0x387/0x4e0 drivers/input/evdev.c:278
> evdev_events+0x8e/0xd0 drivers/input/evdev.c:306
> input_pass_values+0x123/0x390 drivers/input/input.c:128
> input_event_dispose+0x248/0x320 drivers/input/input.c:342
> input_handle_event+0x9e8/0xa20 drivers/input/input.c:370
> input_inject_event+0xbc/0x120 drivers/input/input.c:424
> evdev_write+0x224/0x2b0 drivers/input/evdev.c:528
> vfs_write+0x269/0x9f0 fs/read_write.c:684
> ksys_write+0xdc/0x1a0 fs/read_write.c:738
> __do_sys_write fs/read_write.c:749 [inline]
> __se_sys_write fs/read_write.c:746 [inline]
> __x64_sys_write+0x40/0x50 fs/read_write.c:746
> x64_sys_call+0x2847/0x3000 arch/x86/include/generated/asm/syscalls_64.h:2
> do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
> do_syscall_64+0xc0/0x2a0 arch/x86/entry/syscall_64.c:94
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> read to 0xffff888104842008 of 4 bytes by task 2991 on cpu 0:
> evdev_read+0x157/0x5e0 drivers/input/evdev.c:572
> vfs_read+0x1ab/0x7f0 fs/read_write.c:570
> ksys_read+0xdc/0x1a0 fs/read_write.c:715
> __do_sys_read fs/read_write.c:724 [inline]
> __se_sys_read fs/read_write.c:722 [inline]
> __x64_sys_read+0x40/0x50 fs/read_write.c:722
> x64_sys_call+0x2889/0x3000 arch/x86/include/generated/asm/syscalls_64.h:1
> do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
> do_syscall_64+0xc0/0x2a0 arch/x86/entry/syscall_64.c:94
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> value changed: 0x00000002 -> 0x00000004
>
> Reported-by: syzbot+1b327485934adf39955b@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=1b327485934adf39955b
> Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
> ---
> drivers/input/evdev.c | 20 ++++++++++++++++----
> 1 file changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> index 90ff6be85cf4..eebd59d190f5 100644
> --- a/drivers/input/evdev.c
> +++ b/drivers/input/evdev.c
> @@ -569,9 +569,13 @@ static ssize_t evdev_read(struct file *file, char __user *buffer,
> if (!evdev->exist || client->revoked)
> return -ENODEV;
>
> + spin_lock_irq(&client->buffer_lock);
> if (client->packet_head == client->tail &&
> - (file->f_flags & O_NONBLOCK))
> + (file->f_flags & O_NONBLOCK)) {
> + spin_unlock_irq(&client->buffer_lock);
> return -EAGAIN;
> + }
> + spin_unlock_irq(&client->buffer_lock);
>
> /*
> * count == 0 is special - no IO is done but we check
> @@ -593,9 +597,12 @@ static ssize_t evdev_read(struct file *file, char __user *buffer,
> break;
>
> if (!(file->f_flags & O_NONBLOCK)) {
> - error = wait_event_interruptible(client->wait,
> + spin_lock_irq(&client->buffer_lock);
> + error = wait_event_interruptible_lock_irq(client->wait,
> client->packet_head != client->tail ||
> - !evdev->exist || client->revoked);
> + !evdev->exist || client->revoked,
> + client->buffer_lock);
> + spin_unlock_irq(&client->buffer_lock);
> if (error)
> return error;
> }
> @@ -610,6 +617,7 @@ static __poll_t evdev_poll(struct file *file, poll_table *wait)
> struct evdev_client *client = file->private_data;
> struct evdev *evdev = client->evdev;
> __poll_t mask;
> + bool have_data;
>
> poll_wait(file, &client->wait, wait);
>
> @@ -618,7 +626,11 @@ static __poll_t evdev_poll(struct file *file, poll_table *wait)
> else
> mask = EPOLLHUP | EPOLLERR;
>
> - if (client->packet_head != client->tail)
> + spin_lock_irq(&client->buffer_lock);
> + have_data = client->packet_head != client->tail;
> + spin_unlock_irq(&client->buffer_lock);
> +
> + if (have_data)
> mask |= EPOLLIN | EPOLLRDNORM;
>
> return mask;
> --
> 2.43.0
>
ping
^ permalink raw reply
* Re: [PATCH v3 3/4] dt-bindings: input: add GPIO charlieplex keypad
From: Hugo Villeneuve @ 2026-02-26 15:35 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: robin, andy, robh, krzk+dt, conor+dt, dmitry.torokhov,
hvilleneuve, mkorpershoek, matthias.bgg,
angelogioacchino.delregno, lee, alexander.sverdlin, marek.vasut,
akurz, devicetree, linux-kernel, linux-input, linux-arm-kernel,
linux-mediatek
In-Reply-To: <CAMuHMdVdYX9p9DfDoyMv8qEm52kY51QULkEnuxRBH2OyWyYf6g@mail.gmail.com>
Hi Geert,
On Thu, 26 Feb 2026 10:32:30 +0100
Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Hi Hugo,
>
> On Wed, 25 Feb 2026 at 16:54, Hugo Villeneuve <hugo@hugovil.com> wrote:
> > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> >
> > Add DT bindings for GPIO charlieplex keypad.
> >
> > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
>
> Thanks for your patch!
>
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml
> > @@ -0,0 +1,106 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +
> > +$id: http://devicetree.org/schemas/input/gpio-charlieplex-keypad.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: GPIO charlieplex keypad
> > +
> > +maintainers:
> > + - Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > +
> > +description: |
> > + The charlieplex keypad supports N^2)-N different key combinations (where N is
> > + the number of lines). Key presses and releases are detected by configuring
> > + only one line as output at a time, and reading other line states. This process
> > + is repeated for each line. Diodes are required to ensure current flows in only
> > + one direction between any pair of pins.
> > + This mechanism doesn't allow to detect simultaneous key presses.
>
> Indeed, e.g. pressing S1 and S2 simultaneously will show a ghost
> S5 keypress.
>
> > +
> > + Wiring example for 3 lines keyboard with 6 switches and 3 diodes:
> > +
> > + L0 --+---------------------+----------------------+
> > + | | |
> > + L1 -------+-----------+---------------------+ |
> > + | | | | | |
> > + L2 -------------+----------------+-----+ | |
> > + | | | | | | | | |
> > + | | | | | | | | |
> > + | S1 \ S2 \ | S3 \ S4 \ | S5 \ S6 \
> > + | | | | | | | | |
> > + | +--+--+ | +--+--+ | +--+--+
> > + | | | | | |
> > + | D1 v | D2 v | D3 v
> > + | - (k) | - (k) | - (k)
> > + | | | | | |
> > + +-------+ +-------+ +-------+
>
> Don't you need pull-down resistors on L[0-2], and/or a way to specify
> in DT to enable internal poll-down on GPIO controllers that support it?
> Some controllers may support internal pull-up only, but I guess that
> can be handled using GPIO_ACTIVE_LOW?
Yes, using something like this:
line-gpios = <&gpio2 25 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)
should work I think, although with my hardware unfortunately I cannot
test it.
Hugo.
>
> > +
> > + L: GPIO line
> > + S: switch
> > + D: diode (k indicates cathode)
> > +
> > +allOf:
> > + - $ref: input.yaml#
> > + - $ref: /schemas/input/matrix-keymap.yaml#
> > +
> > +properties:
> > + compatible:
> > + const: gpio-charlieplex-keypad
> > +
> > + autorepeat: true
> > +
> > + debounce-delay-ms:
> > + default: 5
> > +
> > + line-gpios:
> > + description:
> > + List of GPIOs used as lines. The gpio specifier for this property
> > + depends on the gpio controller to which these lines are connected.
> > +
> > + linux,keymap: true
> > +
> > + poll-interval: true
> > +
> > + settling-time-us: true
> > +
> > + wakeup-source: true
> > +
> > +required:
> > + - compatible
> > + - line-gpios
> > + - linux,keymap
> > + - poll-interval
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/gpio/gpio.h>
> > + #include <dt-bindings/input/input.h>
> > +
> > + charlieplex-keypad {
>
> "keyboard", as per Devicetree Specification Generic Names
> Recommendation.
>
> > + compatible = "gpio-charlieplex-keypad";
> > + debounce-delay-ms = <20>;
> > + poll-interval = <5>;
> > + settling-time-us = <2>;
> > +
> > + line-gpios = <&gpio2 25 GPIO_ACTIVE_HIGH
> > + &gpio2 26 GPIO_ACTIVE_HIGH
> > + &gpio2 27 GPIO_ACTIVE_HIGH>;
> > +
> > + /* MATRIX_KEY(output, input, key-code) */
> > + linux,keymap = <
> > + /*
> > + * According to wiring diagram above, if L1 is configured as
> > + * output and HIGH, and we detect a HIGH level on input L0,
> > + * then it means S1 is pressed: MATRIX_KEY(L1, L0, KEY...)
> > + */
> > + MATRIX_KEY(1, 0, KEY_F1) /* S1 */
> > + MATRIX_KEY(2, 0, KEY_F2) /* S2 */
> > + MATRIX_KEY(0, 1, KEY_F3) /* S3 */
> > + MATRIX_KEY(2, 1, KEY_F4) /* S4 */
> > + MATRIX_KEY(1, 2, KEY_F5) /* S5 */
> > + MATRIX_KEY(0, 2, KEY_F6) /* S6 */
> > + >;
> > + };
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
>
--
Hugo Villeneuve
^ permalink raw reply
* Re: [PATCH 1/1] HID: uhid: Fix out-of-bounds write caused by raw events mismanagement
From: Benjamin Tissoires @ 2026-02-26 15:51 UTC (permalink / raw)
To: Lee Jones
Cc: Jiri Kosina, David Rheinsberg, linux-input, linux-kernel, stable
In-Reply-To: <20260226140810.GD8023@google.com>
On Feb 26 2026, Lee Jones wrote:
> On Thu, 26 Feb 2026, Benjamin Tissoires wrote:
>
> > On Feb 26 2026, Lee Jones wrote:
> > > On Tue, 24 Feb 2026, Jiri Kosina wrote:
> > >
> > > > On Tue, 24 Feb 2026, Benjamin Tissoires wrote:
> > > >
> > > > > Long story short: that patch is too intrusive as it makes assumption on
> > > > > the behavior of the device. We need to understand where/if the bug was
> > > > > spotted and fix the caller of hid_hw_raw_request, not the uhid
> > > > > implementation.
> > > >
> > > > Thanks a lot for the analysis, Benjamin!
> > > >
> > > > I asked about that here:
> > > >
> > > > https://lore.kernel.org/all/172q4775-616s-p7s4-7n80-p8579n0r3516@xreary.bet/
> > > >
> > > > So let's wait for Lee to clarify. Until that, the patch stays out of the
> > > > branch.
> > >
> > > Thanks to both of you for looking into this. I appreciate your efforts.
> > >
> > > This is very much real world.
> > >
> > > Is there a way to add an errata for the PS3 controller?
> > >
> >
> > Unfortunatelly no. uhid merely emulates what a device can do, and HID is
> > a convention. So if we were to have a special case to PS3 controllers,
> > we would then start having to maintain an endless list of quirks when
> > the issue is *not* in uhid, but in the processing of the device after
> > (maybe in hid-core?).
>
> Actually I think the issue is in UHID. At least the way I read it.
And I disagree :)
>
> Are there legitimate use-cases for devices overwriting the Report ID
> contained in the first index of the data buffer? From my very limited
> knowledge of the subsystem, this sounds like an oversight.
>
Legitimate, probably no, but we are talking about physical devices
here. uhid is a mere replacement of a transport layer, and there is
nothing that prevents a device to reply with a buffer starting with 1
when requested about feature 2 (because it's firmware and they just
don't care).
This happens a lot with proprietary features on devices, when there is
no spec, so ODM provide their own driver and they can do whatever they
want.
If uhid or any transport layer solely takes the decision that a reply to
a request is wrong, we have no chance of fixing it after the fact. This
is what happens with the PS3 controller: an undocumented feature is
used, but that's what the Playstation does, so we need to tag along.
I hope it makes more sense now.
FTR, Lee shared the logs of the issue privately, and I already told him
where we should fix the issue.
Cheers,
Benjamin
^ permalink raw reply
* Re: [PATCH V3 RESEND 0/2] Add tc3408 bindings and timing
From: Dmitry Torokhov @ 2026-02-26 16:06 UTC (permalink / raw)
To: Langyan Ye
Cc: robh, krzk+dt, conor+dt, jikos, bentiss, dianders, linux-input,
devicetree, linux-kernel
In-Reply-To: <20260108063524.742464-1-yelangyan@huaqin.corp-partner.google.com>
On Thu, Jan 08, 2026 at 02:35:22PM +0800, Langyan Ye wrote:
> The tc3408 touch screen chip same as Elan eKTH6915 controller
> has a reset gpio. The difference is that they have different
> post_power_delay_ms.
>
> According to the Parade TC3408 datasheet, the reset pin requires a
> pull-down duration longer than 10 ms, therefore post_power_delay_ms
> is set to 10. In addition, the chipset requires an initialization
> time greater than 300 ms after reset, so post_gpio_reset_on_delay_ms
> is configured as 300.
>
> Changes in v3:
> - PATCH 2/2: Corrected post_gpio_reset_on_delay_ms: 100 -> 300
> - Link to v2: https://lore.kernel.org/all/20250820122520.3356738-1-yelangyan@huaqin.corp-partner.google.com/
>
> Changes in v2:
> - PATCH 1/2: Drop redundant "bindings for" from subject
> - PATCH 1/2: Improve description (describe hardware instead of bindings)
> - PATCH 1/2: Drop "panel: true" property
> - PATCH 1/2: Drop redundant description for reset-gpios
> - PATCH 1/2: Use unevaluatedProperties: false instead of additionalProperties
> - Link to v1: https://lore.kernel.org/all/20250819034852.1230264-1-yelangyan@huaqin.corp-partner.google.com/
>
> Langyan Ye (2):
> dt-bindings: input: Add Parade TC3408 touchscreen controller
> HID: i2c-hid: elan: Add parade-tc3408 timing
>
> .../bindings/input/parade,tc3408.yaml | 68 +++++++++++++++++++
> drivers/hid/i2c-hid/i2c-hid-of-elan.c | 8 +++
> 2 files changed, 76 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/parade,tc3408.yaml
Applied the lot, thank you.
--
Dmitry
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox