* Re: [PATCH v1 2/9] misc: Support Asus Transformer's EC access device
From: Greg Kroah-Hartman @ 2026-02-03 12:00 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Dmitry Torokhov, Pavel Machek, Arnd Bergmann, Sebastian Reichel,
Michał Mirosław, Ion Agorria, devicetree, linux-kernel,
linux-input, linux-leds, linux-pm
In-Reply-To: <CAPVz0n35NkEXjur-oJhW6Yxwme_KMLdYCnRAtjHEWSPEVrSUXQ@mail.gmail.com>
On Tue, Feb 03, 2026 at 01:54:58PM +0200, Svyatoslav Ryhel wrote:
> вт, 3 лют. 2026 р. о 13:41 Greg Kroah-Hartman <gregkh@linuxfoundation.org> пише:
> >
> > On Sun, Feb 01, 2026 at 12:43:36PM +0200, Svyatoslav Ryhel wrote:
> > > --- /dev/null
> > > +++ b/drivers/misc/asus-dockram.c
> > > @@ -0,0 +1,327 @@
> > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > +/*
> > > + * ASUS EC: DockRAM
> > > + */
> > > +
> > > +#include <linux/device.h>
> > > +#include <linux/err.h>
> > > +#include <linux/i2c.h>
> > > +#include <linux/mfd/asus-ec.h>
> > > +#include <linux/mod_devicetable.h>
> > > +#include <linux/module.h>
> > > +#include <linux/mutex.h>
> > > +#include <linux/slab.h>
> > > +#include <linux/string.h>
> > > +#include <linux/sysfs.h>
> > > +#include <linux/types.h>
> > > +#include <linux/unaligned.h>
> > > +
> > > +struct dockram_ec_data {
> > > + struct mutex ctl_lock; /* prevent simultaneous access */
> > > + char ctl_data[DOCKRAM_ENTRY_BUFSIZE];
> > > +};
> > > +
> > > +int asus_dockram_read(struct i2c_client *client, int reg, char *buf)
> > > +{
> > > + int rc;
> > > +
> > > + memset(buf, 0, DOCKRAM_ENTRY_BUFSIZE);
> > > + rc = i2c_smbus_read_i2c_block_data(client, reg, DOCKRAM_ENTRY_BUFSIZE, buf);
> > > + if (rc < 0)
> > > + return rc;
> > > +
> > > + if (buf[0] > DOCKRAM_ENTRY_SIZE) {
> > > + dev_err(&client->dev, "bad data len; buffer: %*ph; rc: %d\n",
> > > + DOCKRAM_ENTRY_BUFSIZE, buf, rc);
> > > + return -EPROTO;
> > > + }
> > > +
> > > + dev_dbg(&client->dev, "got data; buffer: %*ph; rc: %d\n",
> > > + DOCKRAM_ENTRY_BUFSIZE, buf, rc);
> > > +
> > > + return 0;
> > > +}
> > > +EXPORT_SYMBOL_GPL(asus_dockram_read);
> >
> > No documentation for these new public symbols?
> >
>
> These functions are mainly used in communication between the dockram
> device, asus-ec and its subdevices. Export is used here because all
> mentioned devices can be built as modules. I can add descriptions of
> functions into header if needed, but they should never be used outside
> of dockram-EC complex. Same applies to 2 export functions in the EC
> MFD.
Then you should properly document this :)
> > > +static BIN_ATTR_RW(dockram, DOCKRAM_ENTRIES * DOCKRAM_ENTRY_SIZE);
> > > +static DEVICE_ATTR_RW(control_reg);
> >
> > You did not document your new sysfs files in Documentation/ABI/ which is
> > required.
> >
> > Also, why do you need a brand new user/kernel api at all? Who is going
> > to use this and for what?
> >
>
> These api were used mainly for debugging/logging purposes and descend
> from original downstream EC driver. I can both add documentation into
> ABI or remove them if that is absolutely necessary.
Debugging should not be in sysfs, please put this type of stuff into
debugfs instead if you really need it.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v1 2/9] misc: Support Asus Transformer's EC access device
From: Svyatoslav Ryhel @ 2026-02-03 11:54 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Dmitry Torokhov, Pavel Machek, Arnd Bergmann, Sebastian Reichel,
Michał Mirosław, Ion Agorria, devicetree, linux-kernel,
linux-input, linux-leds, linux-pm
In-Reply-To: <2026020350-unrevised-humming-7a42@gregkh>
вт, 3 лют. 2026 р. о 13:41 Greg Kroah-Hartman <gregkh@linuxfoundation.org> пише:
>
> On Sun, Feb 01, 2026 at 12:43:36PM +0200, Svyatoslav Ryhel wrote:
> > --- /dev/null
> > +++ b/drivers/misc/asus-dockram.c
> > @@ -0,0 +1,327 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * ASUS EC: DockRAM
> > + */
> > +
> > +#include <linux/device.h>
> > +#include <linux/err.h>
> > +#include <linux/i2c.h>
> > +#include <linux/mfd/asus-ec.h>
> > +#include <linux/mod_devicetable.h>
> > +#include <linux/module.h>
> > +#include <linux/mutex.h>
> > +#include <linux/slab.h>
> > +#include <linux/string.h>
> > +#include <linux/sysfs.h>
> > +#include <linux/types.h>
> > +#include <linux/unaligned.h>
> > +
> > +struct dockram_ec_data {
> > + struct mutex ctl_lock; /* prevent simultaneous access */
> > + char ctl_data[DOCKRAM_ENTRY_BUFSIZE];
> > +};
> > +
> > +int asus_dockram_read(struct i2c_client *client, int reg, char *buf)
> > +{
> > + int rc;
> > +
> > + memset(buf, 0, DOCKRAM_ENTRY_BUFSIZE);
> > + rc = i2c_smbus_read_i2c_block_data(client, reg, DOCKRAM_ENTRY_BUFSIZE, buf);
> > + if (rc < 0)
> > + return rc;
> > +
> > + if (buf[0] > DOCKRAM_ENTRY_SIZE) {
> > + dev_err(&client->dev, "bad data len; buffer: %*ph; rc: %d\n",
> > + DOCKRAM_ENTRY_BUFSIZE, buf, rc);
> > + return -EPROTO;
> > + }
> > +
> > + dev_dbg(&client->dev, "got data; buffer: %*ph; rc: %d\n",
> > + DOCKRAM_ENTRY_BUFSIZE, buf, rc);
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(asus_dockram_read);
>
> No documentation for these new public symbols?
>
These functions are mainly used in communication between the dockram
device, asus-ec and its subdevices. Export is used here because all
mentioned devices can be built as modules. I can add descriptions of
functions into header if needed, but they should never be used outside
of dockram-EC complex. Same applies to 2 export functions in the EC
MFD.
>
> > +static BIN_ATTR_RW(dockram, DOCKRAM_ENTRIES * DOCKRAM_ENTRY_SIZE);
> > +static DEVICE_ATTR_RW(control_reg);
>
> You did not document your new sysfs files in Documentation/ABI/ which is
> required.
>
> Also, why do you need a brand new user/kernel api at all? Who is going
> to use this and for what?
>
These api were used mainly for debugging/logging purposes and descend
from original downstream EC driver. I can both add documentation into
ABI or remove them if that is absolutely necessary.
> thanks,
>
> greg k-h
^ permalink raw reply
* Re: [PATCH v1 2/9] misc: Support Asus Transformer's EC access device
From: Greg Kroah-Hartman @ 2026-02-03 11:41 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Dmitry Torokhov, Pavel Machek, Arnd Bergmann, Sebastian Reichel,
Michał Mirosław, Ion Agorria, devicetree, linux-kernel,
linux-input, linux-leds, linux-pm
In-Reply-To: <20260201104343.79231-3-clamor95@gmail.com>
On Sun, Feb 01, 2026 at 12:43:36PM +0200, Svyatoslav Ryhel wrote:
> --- /dev/null
> +++ b/drivers/misc/asus-dockram.c
> @@ -0,0 +1,327 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * ASUS EC: DockRAM
> + */
> +
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/mfd/asus-ec.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/slab.h>
> +#include <linux/string.h>
> +#include <linux/sysfs.h>
> +#include <linux/types.h>
> +#include <linux/unaligned.h>
> +
> +struct dockram_ec_data {
> + struct mutex ctl_lock; /* prevent simultaneous access */
> + char ctl_data[DOCKRAM_ENTRY_BUFSIZE];
> +};
> +
> +int asus_dockram_read(struct i2c_client *client, int reg, char *buf)
> +{
> + int rc;
> +
> + memset(buf, 0, DOCKRAM_ENTRY_BUFSIZE);
> + rc = i2c_smbus_read_i2c_block_data(client, reg, DOCKRAM_ENTRY_BUFSIZE, buf);
> + if (rc < 0)
> + return rc;
> +
> + if (buf[0] > DOCKRAM_ENTRY_SIZE) {
> + dev_err(&client->dev, "bad data len; buffer: %*ph; rc: %d\n",
> + DOCKRAM_ENTRY_BUFSIZE, buf, rc);
> + return -EPROTO;
> + }
> +
> + dev_dbg(&client->dev, "got data; buffer: %*ph; rc: %d\n",
> + DOCKRAM_ENTRY_BUFSIZE, buf, rc);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(asus_dockram_read);
No documentation for these new public symbols?
> +static BIN_ATTR_RW(dockram, DOCKRAM_ENTRIES * DOCKRAM_ENTRY_SIZE);
> +static DEVICE_ATTR_RW(control_reg);
You did not document your new sysfs files in Documentation/ABI/ which is
required.
Also, why do you need a brand new user/kernel api at all? Who is going
to use this and for what?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v2 2/3] Input: aw86938 - add driver for Awinic AW86938
From: Konrad Dybcio @ 2026-02-03 11:39 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Luca Weiss, Griffin Kroah-Hartman, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
linux-input, devicetree, linux-kernel, linux-arm-msm
In-Reply-To: <aYHBgLyIttd4lkn6@google.com>
On 2/3/26 10:49 AM, Dmitry Torokhov wrote:
> On Mon, Feb 02, 2026 at 04:11:51PM +0100, Konrad Dybcio wrote:
>> On 2/2/26 12:04 PM, Dmitry Torokhov wrote:
>>> On Mon, Feb 02, 2026 at 11:19:36AM +0100, Konrad Dybcio wrote:
>>>> On 2/2/26 11:14 AM, Luca Weiss wrote:
>>>>> Hi Konrad,
>>>>>
>>>>> On Mon Feb 2, 2026 at 11:12 AM CET, Konrad Dybcio wrote:
>>>>>> On 2/1/26 2:49 AM, Dmitry Torokhov wrote:
>>>>>>> Hi Griffin,
>>>>>>>
>>>>>>> On Wed, Jan 28, 2026 at 04:51:14PM +0100, Griffin Kroah-Hartman wrote:
>>>>>>>> @@ -717,9 +746,19 @@ static int aw86927_detect(struct aw86927_data *haptics)
>>>>>>>>
>>>>>>>> chip_id = be16_to_cpu(read_buf);
>>>>>>>>
>>>>>>>> - if (chip_id != AW86927_CHIPID) {
>>>>>>>> - dev_err(haptics->dev, "Unexpected CHIPID value 0x%x\n", chip_id);
>>>>>>>> - return -ENODEV;
>>>>>>>> + switch (haptics->model) {
>>>>>>>> + case AW86927:
>>>>>>>> + if (chip_id != AW86927_CHIPID) {
>>>>>>>> + dev_err(haptics->dev, "Unexpected CHIPID value 0x%x\n", chip_id);
>>>>>>>> + return -ENODEV;
>>>>>>>> + }
>>>>>>>
>>>>>>> If we are able to query chip ID why do we need to have separate
>>>>>>> compatibles? I would define chip data structure with differences between
>>>>>>> variants and assign and use it instead of having separate compatible.
>>>>>>
>>>>>> dt-bindings guidelines explicitly call for this, a chipid comparison
>>>>>> then works as a safety net
>>>>>
>>>>> Are you saying, that
>>>>>
>>>>> 1. we should enforce dt-bindings == CHIP_ID (what's currently done)
>>>>
>>>> This
>>>
>>> No. If there is a compatible chip with different ID (for whatever reason
>>> - maybe there is additional functionality that either board does not
>>> need or the driver does not implement) we absolutely should not refuse
>>> to bind the driver.
>>>
>>> Hint: this thing is called _compatible_ for a reason.
>>
>> Right, the reason you have in mind is fulfilled by fallback compatibles
>>
>> (i.e. "vendor,actualchipname", "vendor,similarchipname" where the driver
>> only considers the latter becuase the software interface hasn't changed)
>
> And having chip_id checks will break this...
Depends on how they're implemented and how different the chips are.
If the software interface is exactly 1:1, with the only difference in the
ID register, something like
if (chipid == SIMILARCHIPNAME_ID || chipid == ACTUALCHIPNAME_ID)
...
would be fitting.
However, more often than not, like in this case, there's actual differences
that need to be taken into account, meaning we already need to act upon
the "actual" compatible
>
>>
>>>
>>>>
>>>>>
>>>>> or
>>>>>
>>>>> 2. we should have both compatibles with no handling based on compatible,
>>>>> but only use CHIP_ID at runtime to change behavior
>>>>
>>>> This is spaghetti
>>>
>>> I really do not understand the aversion of DT maintainers to generic
>>> compatibles. We see this in I2C HID where we keep adding compatibles
>>> for what could be described via device properties.
>>
>> This is because it's the only way to allow for retroactive changes that
>> do not require changing firmware. That's why ACPI carries new identifiers
>> for even very slightly different devices too. Once the firmware containing
>> (ACPI tables / DTB) is put on a production device, it is generally not
>> going to ever change.
>
> They are actually solving slightly different problem. In ACPI world they
> allocate a new ID to represent a peripheral in a given design, down to
> it's firmware behavior. It encodes much more than chip ID that DT
> maintainers want to key off of.
DT sort of does this to. In the Qualcomm world, how you get to interact
with the platform changes dramatically depending on the firmware flavor
it's flashed with (which I'd hope to see go away one day..) - if you have
a Chrome firmware, you're basically free/required to configure everything
from Linux. With Android firmware, much of that heavy lifting must be done
by the hypervisor or the secure world. With Windows firmware, you get the
Android experience + UEFI services. And at the tail end of the scale,
there's the automotive firmware where you don't even get to toggle clocks,
but instead all peripherals' power and performance states are exposed
through dozens of SCMI servers..
Somehow we can try and be smart, deducing the behavior based on the
properties present in DT, but often times, a separate compatible for
"this SoC except with this firmware" needs to exist, as the OS-accessible
software interface is simply different, as if this wasn't the same SoC
anymore.
>
>>
>> CHIP_ID registers are a good tool to validate that the author of the
>> firmware table is doing the right thing, but solely relying on them
>> encourages creating a "vendor,haptic" compatible, which I'm sure you'll
>> agree is totally meaningless.
>
> Is it? If a piece of hardware speaks i2c-hid protocol why do I need to
> know the exact chip that is being used? Depending on the chassis and the
> size of the sensing element and the version of the firmware that is
> loaded into it the behavior and timings of the same chip may be very
> different.
I agree this argument gets overused at times
>>
>> That's especially if the naming scheme makes no sense and you can't
>> even factor out a common wildcard-name (which also happens to be the case
>> quite often)
>>
>> Plus a compatible is used to restrict/modify the set of allowed/required
>> properties, so having an "actual" compatible is required for schema
>> validation to work
>
> Yes, in cases where there is not a common set of properties having
> different compatibles makes sense. But in cases when the device is
> supposed to have vendor-agnostic behavior insisting on myriad
> compatibles makes little sense.
Some people may be thrown off by the golden rule of implementing standards,
which is to break or bend them immediately, claiming full compatibility.
But for cases like hid-over-i2c, I symphatize with the "no one needs
to know if it's a synaptics a00001 or a synaptics a00002" sentiment
Konrad
^ permalink raw reply
* Re: [PATCH v1 6/9] input: keyboard: Add driver for Asus Transformer dock multimedia keys
From: Svyatoslav Ryhel @ 2026-02-03 11:18 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Pavel Machek, Arnd Bergmann, Greg Kroah-Hartman,
Sebastian Reichel, Michał Mirosław, Ion Agorria,
devicetree, linux-kernel, linux-input, linux-leds, linux-pm
In-Reply-To: <aYHU5g5iOVjrHrE_@google.com>
вт, 3 лют. 2026 р. о 13:00 Dmitry Torokhov <dmitry.torokhov@gmail.com> пише:
>
> Hi Svyatoslav,
>
> On Sun, Feb 01, 2026 at 12:43:40PM +0200, Svyatoslav Ryhel wrote:
> > +static void asus_ec_input_event(struct input_handle *handle,
> > + unsigned int event_type,
> > + unsigned int event_code, int value)
> > +{
> > + struct asus_ec_keys_data *priv = handle->handler->private;
> > +
> > + /* Store special key state */
> > + if (event_type == EV_KEY && event_code == KEY_RIGHTALT)
> > + priv->special_key_pressed = !!value;
>
> Is this functionality supposed to be triggered by any keyboard or only
> the dock one?
>
Any keyboard. Dock keyboard is basically a regular keyboard fused with
a multimedia top row.
> Thanks.
>
> --
> Dmitry
^ permalink raw reply
* Re: [PATCH v1 6/9] input: keyboard: Add driver for Asus Transformer dock multimedia keys
From: Dmitry Torokhov @ 2026-02-03 11:00 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Pavel Machek, Arnd Bergmann, Greg Kroah-Hartman,
Sebastian Reichel, Michał Mirosław, Ion Agorria,
devicetree, linux-kernel, linux-input, linux-leds, linux-pm
In-Reply-To: <20260201104343.79231-7-clamor95@gmail.com>
Hi Svyatoslav,
On Sun, Feb 01, 2026 at 12:43:40PM +0200, Svyatoslav Ryhel wrote:
> +static void asus_ec_input_event(struct input_handle *handle,
> + unsigned int event_type,
> + unsigned int event_code, int value)
> +{
> + struct asus_ec_keys_data *priv = handle->handler->private;
> +
> + /* Store special key state */
> + if (event_type == EV_KEY && event_code == KEY_RIGHTALT)
> + priv->special_key_pressed = !!value;
Is this functionality supposed to be triggered by any keyboard or only
the dock one?
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [syzbot] [input?] [usb?] WARNING in asus_remove
From: Sahil Chandna @ 2026-02-03 10:09 UTC (permalink / raw)
To: syzbot; +Cc: bentiss, jikos, linux-input, linux-kernel, linux-usb,
syzkaller-bugs
In-Reply-To: <697baa8f.a70a0220.9914.001f.GAE@google.com>
#syz test
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -147,6 +147,7 @@ struct asus_drvdata {
unsigned long battery_next_query;
struct work_struct fn_lock_sync_work;
bool fn_lock;
+ struct mutex lock;
};
static int asus_report_battery(struct asus_drvdata *, u8 *, int);
@@ -960,8 +961,10 @@ static int asus_input_configured(struct hid_device *hdev, struct hid_input *hi)
}
if (drvdata->quirks & QUIRK_HID_FN_LOCK) {
- drvdata->fn_lock = true;
+ mutex_lock(&drvdata->lock);
INIT_WORK(&drvdata->fn_lock_sync_work, asus_sync_fn_lock);
+ drvdata->fn_lock = true;
+ mutex_unlock(&drvdata->lock);
asus_kbd_set_fn_lock(hdev, true);
}
@@ -1258,6 +1261,7 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
hdev->quirks |= HID_QUIRK_NO_INIT_REPORTS;
drvdata->hdev = hdev;
+ mutex_init(&drvdata->lock);
if (drvdata->quirks & (QUIRK_T100CHI | QUIRK_T90CHI)) {
ret = asus_battery_probe(hdev);
@@ -1343,8 +1347,10 @@ static void asus_remove(struct hid_device *hdev)
cancel_work_sync(&drvdata->kbd_backlight->work);
}
- if (drvdata->quirks & QUIRK_HID_FN_LOCK)
+ mutex_lock(&drvdata->lock);
+ if ((drvdata->quirks & QUIRK_HID_FN_LOCK) && drvdata->fn_lock)
cancel_work_sync(&drvdata->fn_lock_sync_work);
+ mutex_unlock(&drvdata->lock);
hid_hw_stop(hdev);
}
--
^ permalink raw reply
* Re: [PATCH] Input: cyapa - fix missing input->mutex when calling input_device_enabled()
From: Dmitry Torokhov @ 2026-02-03 9:52 UTC (permalink / raw)
To: Ziyi Guo; +Cc: linux-input, linux-kernel
In-Reply-To: <20260202220223.2471730-1-n7l8m4@u.northwestern.edu>
Hi Ziyi,
On Mon, Feb 02, 2026 at 10:02:23PM +0000, Ziyi Guo wrote:
> cyapa_enable_irq_for_cmd()/cyapa_disable_irq_for_cmd() and
> cyapa_reinitialize call input_device_enabled() without holding
> input->mutex. However, input_device_enabled() has
> lockdep_assert_held(&dev->mutex).
>
> Add mutex_lock()/mutex_unlock() around the input_device_enabled()
> call to properly protect access to the input device state.
>
> Signed-off-by: Ziyi Guo <n7l8m4@u.northwestern.edu>
> ---
> drivers/input/mouse/cyapa.c | 27 ++++++++++++++++++++++++---
> 1 file changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/input/mouse/cyapa.c b/drivers/input/mouse/cyapa.c
> index 00c87c0532a6..e5fe3b8b6548 100644
> --- a/drivers/input/mouse/cyapa.c
> +++ b/drivers/input/mouse/cyapa.c
> @@ -525,8 +525,15 @@ static int cyapa_create_input_dev(struct cyapa *cyapa)
> static void cyapa_enable_irq_for_cmd(struct cyapa *cyapa)
> {
> struct input_dev *input = cyapa->input;
> + bool input_enabled = false;
>
> - if (!input || !input_device_enabled(input)) {
> + if (input) {
> + mutex_lock(&input->mutex);
> + input_enabled = input_device_enabled(input);
> + mutex_unlock(&input->mutex);
This solves absolutely nothing. The value of input_enabled becomes stale
the very moment you release the mutex.
The driver requires much more through conversion to handle potential
races with opening and closing the input device.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2 2/3] Input: aw86938 - add driver for Awinic AW86938
From: Dmitry Torokhov @ 2026-02-03 9:49 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Luca Weiss, Griffin Kroah-Hartman, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
linux-input, devicetree, linux-kernel, linux-arm-msm
In-Reply-To: <6fa17bda-4d4a-4b31-99a2-1d2b606b663b@oss.qualcomm.com>
On Mon, Feb 02, 2026 at 04:11:51PM +0100, Konrad Dybcio wrote:
> On 2/2/26 12:04 PM, Dmitry Torokhov wrote:
> > On Mon, Feb 02, 2026 at 11:19:36AM +0100, Konrad Dybcio wrote:
> >> On 2/2/26 11:14 AM, Luca Weiss wrote:
> >>> Hi Konrad,
> >>>
> >>> On Mon Feb 2, 2026 at 11:12 AM CET, Konrad Dybcio wrote:
> >>>> On 2/1/26 2:49 AM, Dmitry Torokhov wrote:
> >>>>> Hi Griffin,
> >>>>>
> >>>>> On Wed, Jan 28, 2026 at 04:51:14PM +0100, Griffin Kroah-Hartman wrote:
> >>>>>> @@ -717,9 +746,19 @@ static int aw86927_detect(struct aw86927_data *haptics)
> >>>>>>
> >>>>>> chip_id = be16_to_cpu(read_buf);
> >>>>>>
> >>>>>> - if (chip_id != AW86927_CHIPID) {
> >>>>>> - dev_err(haptics->dev, "Unexpected CHIPID value 0x%x\n", chip_id);
> >>>>>> - return -ENODEV;
> >>>>>> + switch (haptics->model) {
> >>>>>> + case AW86927:
> >>>>>> + if (chip_id != AW86927_CHIPID) {
> >>>>>> + dev_err(haptics->dev, "Unexpected CHIPID value 0x%x\n", chip_id);
> >>>>>> + return -ENODEV;
> >>>>>> + }
> >>>>>
> >>>>> If we are able to query chip ID why do we need to have separate
> >>>>> compatibles? I would define chip data structure with differences between
> >>>>> variants and assign and use it instead of having separate compatible.
> >>>>
> >>>> dt-bindings guidelines explicitly call for this, a chipid comparison
> >>>> then works as a safety net
> >>>
> >>> Are you saying, that
> >>>
> >>> 1. we should enforce dt-bindings == CHIP_ID (what's currently done)
> >>
> >> This
> >
> > No. If there is a compatible chip with different ID (for whatever reason
> > - maybe there is additional functionality that either board does not
> > need or the driver does not implement) we absolutely should not refuse
> > to bind the driver.
> >
> > Hint: this thing is called _compatible_ for a reason.
>
> Right, the reason you have in mind is fulfilled by fallback compatibles
>
> (i.e. "vendor,actualchipname", "vendor,similarchipname" where the driver
> only considers the latter becuase the software interface hasn't changed)
And having chip_id checks will break this...
>
> >
> >>
> >>>
> >>> or
> >>>
> >>> 2. we should have both compatibles with no handling based on compatible,
> >>> but only use CHIP_ID at runtime to change behavior
> >>
> >> This is spaghetti
> >
> > I really do not understand the aversion of DT maintainers to generic
> > compatibles. We see this in I2C HID where we keep adding compatibles
> > for what could be described via device properties.
>
> This is because it's the only way to allow for retroactive changes that
> do not require changing firmware. That's why ACPI carries new identifiers
> for even very slightly different devices too. Once the firmware containing
> (ACPI tables / DTB) is put on a production device, it is generally not
> going to ever change.
They are actually solving slightly different problem. In ACPI world they
allocate a new ID to represent a peripheral in a given design, down to
it's firmware behavior. It encodes much more than chip ID that DT
maintainers want to key off of.
>
> CHIP_ID registers are a good tool to validate that the author of the
> firmware table is doing the right thing, but solely relying on them
> encourages creating a "vendor,haptic" compatible, which I'm sure you'll
> agree is totally meaningless.
Is it? If a piece of hardware speaks i2c-hid protocol why do I need to
know the exact chip that is being used? Depending on the chassis and the
size of the sensing element and the version of the firmware that is
loaded into it the behavior and timings of the same chip may be very
different.
>
> That's especially if the naming scheme makes no sense and you can't
> even factor out a common wildcard-name (which also happens to be the case
> quite often)
>
> Plus a compatible is used to restrict/modify the set of allowed/required
> properties, so having an "actual" compatible is required for schema
> validation to work
Yes, in cases where there is not a common set of properties having
different compatibles makes sense. But in cases when the device is
supposed to have vendor-agnostic behavior insisting on myriad
compatibles makes little sense.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] HID: asus: Add check for cancelling fn_lock_sync_work
From: Alan Stern @ 2026-02-03 3:27 UTC (permalink / raw)
To: Sahil Chandna, Jiri Kosina
Cc: bentiss, connorbelli2003, linux-input, linux-kernel
In-Reply-To: <20260130155204.96831-1-chandna.sahil@gmail.com>
I just noticed this because of a related message in a different thread.
On Fri, Jan 26, 2026 at 09:22:04PM +0530, Sahil Chandna wrote:
> syzbot reported a workqueue warning where fn_lock_sync_work is cancelled
> during device removal before the work has been initialized. This can
> happen when the device is disconnected while initialization is still
> in progress.
> Fix this by initializing fn_lock_sync_work before marking fn_lock as
> enabled, and by using fn_lock as a flag in the remove path. This
> ensures cancel_work_sync() is only called for initialized work.
>
> Fixes: f631011e36b8 ("HID: hid-asus: Implement fn lock for Asus ProArt P16")
> Reported-by: syzbot+13f8286fa2de04a7cd48@syzkaller.appspotmail.com
> Signed-off-by: Sahil Chandna <chandna.sahil@gmail.com>
> ---
> drivers/hid/hid-asus.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 1b9793f7c07e..bb85a36de14f 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -960,8 +960,8 @@ static int asus_input_configured(struct hid_device *hdev, struct hid_input *hi)
> }
>
> if (drvdata->quirks & QUIRK_HID_FN_LOCK) {
> - drvdata->fn_lock = true;
> INIT_WORK(&drvdata->fn_lock_sync_work, asus_sync_fn_lock);
> + drvdata->fn_lock = true;
> asus_kbd_set_fn_lock(hdev, true);
> }
>
> @@ -1343,7 +1343,7 @@ static void asus_remove(struct hid_device *hdev)
> cancel_work_sync(&drvdata->kbd_backlight->work);
> }
>
> - if (drvdata->quirks & QUIRK_HID_FN_LOCK)
> + if ((drvdata->quirks & QUIRK_HID_FN_LOCK) && drvdata->fn_lock)
> cancel_work_sync(&drvdata->fn_lock_sync_work);
>
> hid_hw_stop(hdev);
With no synchronization between the two routines, this patch cannot
possibly be correct. There's nothing to prevent the CPU running
asus_input_configured() from executing the assignment to
drvdata->fn_lock before doing the INIT_WORK() (unless the INIT_WORK()
call itself contains some synchronization -- but obviously the code
shouldn't depend on that).
At the very least there should be a pair of memory barriers. A more
palatable substitute would be to protect both regions of code with a
mutex.
Alan Stern
^ permalink raw reply
* Re: [Poke?] Re: [PATCH v2 0/1] HID: switch2: Add preliminary Switch 2 controller driver
From: Vicki Pfau @ 2026-02-03 1:42 UTC (permalink / raw)
To: Jiri Kosina, lyude
Cc: Dmitry Torokhov, Benjamin Tissoires, linux-input,
Luiz Augusto von Dentz, Bastien Nocera
In-Reply-To: <or594565-p59s-784r-494s-82o6qrn9p496@xreary.bet>
Hi Jiri,
On 2/2/26 3:41 AM, Jiri Kosina wrote:
> On Fri, 30 Jan 2026, lyude@redhat.com wrote:
>
>> Bentiss/Jiri, any chance this patch is on your radar?
>
> I've reviewed the driver already, and I was sure I sent out an e-mail
> asking for a minor nit -- better naming of the driver (we usuall name
> drivers by vendors, not specific devices. But I can't find any traces of
> such e-mail, so I probably hallucinated it, sorry.
>
> Vicki, is there any reason not to follow the standard naming, and either
> fold the whole thing into hid-nintendo, or built it separately under e.g.
> hid-nintendo-switch2.c (which might or not might be then linked into
> hid-nintendo proper)?
I was not aware of such a convention. There were plenty of drivers that seem to defy this, like hid-wiimote, so I figured I'd just name it something shorter. I didn't want to fold it directly into hid-nintendo since there's no real opportunity for shared code between the two of them, so it'd just mean (effectively) dead code getting loaded along with the rest of the module if you're only using a Switch 1 controller. That said, hid-playstation doesn't seem to care about this, so I can fold it into hid-nintendo if that's not a concern.
I also have a v3 that I was working on last week that flattens out the larger TODO that I had left over. It still needs testing, but otherwise it's done. I can rename the driver to hid-nintendo-switch2 or fold it into hid-nintendo for that one, whichever you'd prefer.
>
> Thanks,
>
Thanks,
Vicki
^ permalink raw reply
* [PATCH] HID: intel-ish-hid: ipc: Add Nova Lake-H/S PCI device IDs
From: Zhang Lixu @ 2026-02-03 0:55 UTC (permalink / raw)
To: linux-input, srinivas.pandruvada, jikos, benjamin.tissoires
Cc: lixu.zhang, andriy.shevchenko, selina.wang
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>
---
drivers/hid/intel-ish-hid/ipc/hw-ish.h | 2 ++
drivers/hid/intel-ish-hid/ipc/pci-ish.c | 12 ++++++++++++
2 files changed, 14 insertions(+)
diff --git a/drivers/hid/intel-ish-hid/ipc/hw-ish.h b/drivers/hid/intel-ish-hid/ipc/hw-ish.h
index fa5d68c363134..27389971b96cc 100644
--- a/drivers/hid/intel-ish-hid/ipc/hw-ish.h
+++ b/drivers/hid/intel-ish-hid/ipc/hw-ish.h
@@ -39,6 +39,8 @@
#define PCI_DEVICE_ID_INTEL_ISH_PTL_H 0xE345
#define PCI_DEVICE_ID_INTEL_ISH_PTL_P 0xE445
#define PCI_DEVICE_ID_INTEL_ISH_WCL 0x4D45
+#define PCI_DEVICE_ID_INTEL_ISH_NVL_H 0xD354
+#define PCI_DEVICE_ID_INTEL_ISH_NVL_S 0x6E78
#define REVISION_ID_CHT_A0 0x6
#define REVISION_ID_CHT_Ax_SI 0x0
diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
index 1612e8cb23f0c..ed3405c05e73c 100644
--- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
+++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
@@ -28,11 +28,15 @@ enum ishtp_driver_data_index {
ISHTP_DRIVER_DATA_LNL_M,
ISHTP_DRIVER_DATA_PTL,
ISHTP_DRIVER_DATA_WCL,
+ ISHTP_DRIVER_DATA_NVL_H,
+ ISHTP_DRIVER_DATA_NVL_S,
};
#define ISH_FW_GEN_LNL_M "lnlm"
#define ISH_FW_GEN_PTL "ptl"
#define ISH_FW_GEN_WCL "wcl"
+#define ISH_FW_GEN_NVL_H "nvlh"
+#define ISH_FW_GEN_NVL_S "nvls"
#define ISH_FIRMWARE_PATH(gen) "intel/ish/ish_" gen ".bin"
#define ISH_FIRMWARE_PATH_ALL "intel/ish/ish_*.bin"
@@ -47,6 +51,12 @@ static struct ishtp_driver_data ishtp_driver_data[] = {
[ISHTP_DRIVER_DATA_WCL] = {
.fw_generation = ISH_FW_GEN_WCL,
},
+ [ISHTP_DRIVER_DATA_NVL_H] = {
+ .fw_generation = ISH_FW_GEN_NVL_H,
+ },
+ [ISHTP_DRIVER_DATA_NVL_S] = {
+ .fw_generation = ISH_FW_GEN_NVL_S,
+ },
};
static const struct pci_device_id ish_pci_tbl[] = {
@@ -76,6 +86,8 @@ static const struct pci_device_id ish_pci_tbl[] = {
{PCI_DEVICE_DATA(INTEL, ISH_PTL_H, ISHTP_DRIVER_DATA_PTL)},
{PCI_DEVICE_DATA(INTEL, ISH_PTL_P, ISHTP_DRIVER_DATA_PTL)},
{PCI_DEVICE_DATA(INTEL, ISH_WCL, ISHTP_DRIVER_DATA_WCL)},
+ {PCI_DEVICE_DATA(INTEL, ISH_NVL_H, ISHTP_DRIVER_DATA_NVL_H)},
+ {PCI_DEVICE_DATA(INTEL, ISH_NVL_S, ISHTP_DRIVER_DATA_NVL_S)},
{}
};
MODULE_DEVICE_TABLE(pci, ish_pci_tbl);
base-commit: 193579fe01389bc21aff0051d13f24e8ea95b47d
--
2.43.0
^ permalink raw reply related
* RE: [PATCH] HID: Intel-thc-hid: Intel-thc: Fix wrong register fields updating
From: Xu, Even @ 2026-02-03 0:34 UTC (permalink / raw)
To: Jiri Kosina
Cc: bentiss@kernel.org, srinivas.pandruvada@linux.intel.com,
linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
Zhang, Rui1
In-Reply-To: <2rs0n4rs-8821-69qo-7485-50nr37on2ns4@xreary.bet>
Thanks Jiri!
Best Regards,
Even Xu
> -----Original Message-----
> From: Jiri Kosina <jikos@kernel.org>
> Sent: Monday, February 2, 2026 9:40 PM
> To: Xu, Even <even.xu@intel.com>
> Cc: bentiss@kernel.org; srinivas.pandruvada@linux.intel.com; linux-
> input@vger.kernel.org; linux-kernel@vger.kernel.org; Zhang, Rui1
> <rui1.zhang@intel.com>
> Subject: Re: [PATCH] HID: Intel-thc-hid: Intel-thc: Fix wrong register fields
> updating
>
> On Mon, 2 Feb 2026, Even Xu wrote:
>
> > Clear the target bit fields in register before setting new values.
> > This ensures proper field updates by removing any existing bits that
> > might interfere with the new configuration.
> >
> > Fixes: 22da60f0304b ("HID: Intel-thc-hid: Intel-thc: Introduce
> > interrupt delay control")
> > Fixes: 45e92a093099 ("HID: Intel-thc-hid: Intel-thc: Introduce max
> > input size control")
> > Signed-off-by: Even Xu <even.xu@intel.com>
> > Tested-by: Rui Zhang <rui1.zhang@intel.com>
>
> Applied, thanks.
>
> --
> Jiri Kosina
> SUSE Labs
^ permalink raw reply
* [PATCH] Input: cyapa - fix missing input->mutex when calling input_device_enabled()
From: Ziyi Guo @ 2026-02-02 22:02 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, Ziyi Guo
cyapa_enable_irq_for_cmd()/cyapa_disable_irq_for_cmd() and
cyapa_reinitialize call input_device_enabled() without holding
input->mutex. However, input_device_enabled() has
lockdep_assert_held(&dev->mutex).
Add mutex_lock()/mutex_unlock() around the input_device_enabled()
call to properly protect access to the input device state.
Signed-off-by: Ziyi Guo <n7l8m4@u.northwestern.edu>
---
drivers/input/mouse/cyapa.c | 27 ++++++++++++++++++++++++---
1 file changed, 24 insertions(+), 3 deletions(-)
diff --git a/drivers/input/mouse/cyapa.c b/drivers/input/mouse/cyapa.c
index 00c87c0532a6..e5fe3b8b6548 100644
--- a/drivers/input/mouse/cyapa.c
+++ b/drivers/input/mouse/cyapa.c
@@ -525,8 +525,15 @@ static int cyapa_create_input_dev(struct cyapa *cyapa)
static void cyapa_enable_irq_for_cmd(struct cyapa *cyapa)
{
struct input_dev *input = cyapa->input;
+ bool input_enabled = false;
- if (!input || !input_device_enabled(input)) {
+ if (input) {
+ mutex_lock(&input->mutex);
+ input_enabled = input_device_enabled(input);
+ mutex_unlock(&input->mutex);
+ }
+
+ if (!input || !input_enabled) {
/*
* When input is NULL, TP must be in deep sleep mode.
* In this mode, later non-power I2C command will always failed
@@ -545,8 +552,15 @@ static void cyapa_enable_irq_for_cmd(struct cyapa *cyapa)
static void cyapa_disable_irq_for_cmd(struct cyapa *cyapa)
{
struct input_dev *input = cyapa->input;
+ bool input_enabled = false;
+
+ if (input) {
+ mutex_lock(&input->mutex);
+ input_enabled = input_device_enabled(input);
+ mutex_unlock(&input->mutex);
+ }
- if (!input || !input_device_enabled(input)) {
+ if (!input || !input_enabled) {
if (cyapa->gen >= CYAPA_GEN5)
disable_irq(cyapa->client->irq);
if (!input || cyapa->operational)
@@ -628,6 +642,7 @@ static int cyapa_reinitialize(struct cyapa *cyapa)
{
struct device *dev = &cyapa->client->dev;
struct input_dev *input = cyapa->input;
+ bool input_enabled = false;
int error;
if (pm_runtime_enabled(dev))
@@ -652,7 +667,13 @@ static int cyapa_reinitialize(struct cyapa *cyapa)
}
out:
- if (!input || !input_device_enabled(input)) {
+ if (input) {
+ mutex_lock(&input->mutex);
+ input_enabled = input_device_enabled(input);
+ mutex_unlock(&input->mutex);
+ }
+
+ if (!input || !input_enabled) {
/* Reset to power OFF state to save power when no user open. */
if (cyapa->operational)
cyapa->ops->set_power_mode(cyapa,
--
2.34.1
^ permalink raw reply related
* Re: [PATCH] HID: asus: Add check for cancelling fn_lock_sync_work
From: Jiri Kosina @ 2026-02-02 20:51 UTC (permalink / raw)
To: Sahil Chandna
Cc: bentiss, connorbelli2003, linux-input, linux-kernel,
syzbot+13f8286fa2de04a7cd48
In-Reply-To: <20260130155204.96831-1-chandna.sahil@gmail.com>
On Fri, 30 Jan 2026, Sahil Chandna wrote:
> syzbot reported a workqueue warning where fn_lock_sync_work is cancelled
> during device removal before the work has been initialized. This can
> happen when the device is disconnected while initialization is still
> in progress.
> Fix this by initializing fn_lock_sync_work before marking fn_lock as
> enabled, and by using fn_lock as a flag in the remove path. This
> ensures cancel_work_sync() is only called for initialized work.
>
> Fixes: f631011e36b8 ("HID: hid-asus: Implement fn lock for Asus ProArt P16")
> Reported-by: syzbot+13f8286fa2de04a7cd48@syzkaller.appspotmail.com
> Signed-off-by: Sahil Chandna <chandna.sahil@gmail.com>
Applied to hid.git#for-6.20/asus, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [syzbot] [input?] [usb?] WARNING in asus_remove
From: Jiri Kosina @ 2026-02-02 20:47 UTC (permalink / raw)
To: Sahil Chandna
Cc: syzbot, bentiss, linux-input, linux-kernel, linux-usb,
syzkaller-bugs
In-Reply-To: <r7845ps7-3qqp-o377-73r9-410s541452qo@xreary.bet>
On Mon, 2 Feb 2026, Jiri Kosina wrote:
> > #syz test
> > --- a/drivers/hid/hid-asus.c
> > +++ b/drivers/hid/hid-asus.c
> > @@ -960,8 +960,8 @@ static int asus_input_configured(struct hid_device *hdev,
> > struct hid_input *hi)
> > }
> >
> > if (drvdata->quirks & QUIRK_HID_FN_LOCK) {
> > - drvdata->fn_lock = true;
> > INIT_WORK(&drvdata->fn_lock_sync_work, asus_sync_fn_lock);
> > + drvdata->fn_lock = true;
> > asus_kbd_set_fn_lock(hdev, true);
> > }
> > @@ -1343,7 +1343,7 @@ static void asus_remove(struct hid_device *hdev)
> > cancel_work_sync(&drvdata->kbd_backlight->work);
> > }
> > - if (drvdata->quirks & QUIRK_HID_FN_LOCK)
> > + if ((drvdata->quirks & QUIRK_HID_FN_LOCK) && drvdata->fn_lock)
> > cancel_work_sync(&drvdata->fn_lock_sync_work);
> >
> > hid_hw_stop(hdev);
>
> Thanks! Could you please submit this with proper changelog, signoff, etc?
Nevermind, it's 20260130155204.96831-1-chandna.sahil@gmail.com, I'll
followup there.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [syzbot] [input?] [usb?] WARNING in asus_remove
From: Jiri Kosina @ 2026-02-02 20:28 UTC (permalink / raw)
To: Sahil Chandna
Cc: syzbot, bentiss, linux-input, linux-kernel, linux-usb,
syzkaller-bugs
In-Reply-To: <aXxOhV79TnuYaw0-@chandna.localdomain>
On Fri, 30 Jan 2026, Sahil Chandna wrote:
> #syz test
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -960,8 +960,8 @@ static int asus_input_configured(struct hid_device *hdev,
> struct hid_input *hi)
> }
>
> if (drvdata->quirks & QUIRK_HID_FN_LOCK) {
> - drvdata->fn_lock = true;
> INIT_WORK(&drvdata->fn_lock_sync_work, asus_sync_fn_lock);
> + drvdata->fn_lock = true;
> asus_kbd_set_fn_lock(hdev, true);
> }
> @@ -1343,7 +1343,7 @@ static void asus_remove(struct hid_device *hdev)
> cancel_work_sync(&drvdata->kbd_backlight->work);
> }
> - if (drvdata->quirks & QUIRK_HID_FN_LOCK)
> + if ((drvdata->quirks & QUIRK_HID_FN_LOCK) && drvdata->fn_lock)
> cancel_work_sync(&drvdata->fn_lock_sync_work);
>
> hid_hw_stop(hdev);
Thanks! Could you please submit this with proper changelog, signoff, etc?
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: usbhid: Intermittent EPROTO errors trigger USB reset and interrupt user input
From: Alan Stern @ 2026-02-02 16:09 UTC (permalink / raw)
To: Oliver Neukum
Cc: Liam Mitchell, Jiri Kosina, Benjamin Tissoires, linux-usb,
linux-input, linux-kernel
In-Reply-To: <1c317994-2932-4e2e-8e80-1c90606e63c9@suse.com>
On Mon, Feb 02, 2026 at 04:48:49PM +0100, Oliver Neukum wrote:
>
>
> On 01.02.26 21:40, Alan Stern wrote:
> > On Sun, Feb 01, 2026 at 06:57:06PM +0100, Liam Mitchell wrote:
> > > Hi,
> > >
> > > I'm trying to understand and fix intermittent keyboard/trackpad issues
> > > on my 2013 MacBook Pro running Linux v6.18.4:
> > > - missed/repeated/sticky keys while typing (this thread)
>
> Alan,
>
> this raises a question. What happens to KEY_UP events generated in
> between the last EPROTO and the reset? It seems to me like we need
> to assume that a reset implies that all keys have been released.
That question is specialized to keyboards. You could ask a similar
question about mouse buttons, or other input (or output!) devices.
But it's a good point. There should be some sort of callback to inform
HID drivers that their device has been reset, so they could reset
whatever internal state they are maintaining.
This sounds like something going way beyond usbhid, though. And it
probably would not crop up very often, so it wouldn't get much testing.
Liam's computer seems to be pretty unusual.
Alan Stern
^ permalink raw reply
* Re: usbhid: Intermittent EPROTO errors trigger USB reset and interrupt user input
From: Oliver Neukum @ 2026-02-02 15:48 UTC (permalink / raw)
To: Alan Stern, Liam Mitchell
Cc: Jiri Kosina, Benjamin Tissoires, linux-usb, linux-input,
linux-kernel
In-Reply-To: <1ebf9d19-7293-4902-857b-164fd4d21f25@rowland.harvard.edu>
On 01.02.26 21:40, Alan Stern wrote:
> On Sun, Feb 01, 2026 at 06:57:06PM +0100, Liam Mitchell wrote:
>> Hi,
>>
>> I'm trying to understand and fix intermittent keyboard/trackpad issues
>> on my 2013 MacBook Pro running Linux v6.18.4:
>> - missed/repeated/sticky keys while typing (this thread)
Alan,
this raises a question. What happens to KEY_UP events generated in
between the last EPROTO and the reset? It seems to me like we need
to assume that a reset implies that all keys have been released.
Regards
Oliver
^ permalink raw reply
* Re: [PATCH v2 2/3] Input: aw86938 - add driver for Awinic AW86938
From: Konrad Dybcio @ 2026-02-02 15:11 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Luca Weiss, Griffin Kroah-Hartman, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
linux-input, devicetree, linux-kernel, linux-arm-msm
In-Reply-To: <aYCCv6nI2QkvD8rb@google.com>
On 2/2/26 12:04 PM, Dmitry Torokhov wrote:
> On Mon, Feb 02, 2026 at 11:19:36AM +0100, Konrad Dybcio wrote:
>> On 2/2/26 11:14 AM, Luca Weiss wrote:
>>> Hi Konrad,
>>>
>>> On Mon Feb 2, 2026 at 11:12 AM CET, Konrad Dybcio wrote:
>>>> On 2/1/26 2:49 AM, Dmitry Torokhov wrote:
>>>>> Hi Griffin,
>>>>>
>>>>> On Wed, Jan 28, 2026 at 04:51:14PM +0100, Griffin Kroah-Hartman wrote:
>>>>>> @@ -717,9 +746,19 @@ static int aw86927_detect(struct aw86927_data *haptics)
>>>>>>
>>>>>> chip_id = be16_to_cpu(read_buf);
>>>>>>
>>>>>> - if (chip_id != AW86927_CHIPID) {
>>>>>> - dev_err(haptics->dev, "Unexpected CHIPID value 0x%x\n", chip_id);
>>>>>> - return -ENODEV;
>>>>>> + switch (haptics->model) {
>>>>>> + case AW86927:
>>>>>> + if (chip_id != AW86927_CHIPID) {
>>>>>> + dev_err(haptics->dev, "Unexpected CHIPID value 0x%x\n", chip_id);
>>>>>> + return -ENODEV;
>>>>>> + }
>>>>>
>>>>> If we are able to query chip ID why do we need to have separate
>>>>> compatibles? I would define chip data structure with differences between
>>>>> variants and assign and use it instead of having separate compatible.
>>>>
>>>> dt-bindings guidelines explicitly call for this, a chipid comparison
>>>> then works as a safety net
>>>
>>> Are you saying, that
>>>
>>> 1. we should enforce dt-bindings == CHIP_ID (what's currently done)
>>
>> This
>
> No. If there is a compatible chip with different ID (for whatever reason
> - maybe there is additional functionality that either board does not
> need or the driver does not implement) we absolutely should not refuse
> to bind the driver.
>
> Hint: this thing is called _compatible_ for a reason.
Right, the reason you have in mind is fulfilled by fallback compatibles
(i.e. "vendor,actualchipname", "vendor,similarchipname" where the driver
only considers the latter becuase the software interface hasn't changed)
>
>>
>>>
>>> or
>>>
>>> 2. we should have both compatibles with no handling based on compatible,
>>> but only use CHIP_ID at runtime to change behavior
>>
>> This is spaghetti
>
> I really do not understand the aversion of DT maintainers to generic
> compatibles. We see this in I2C HID where we keep adding compatibles
> for what could be described via device properties.
This is because it's the only way to allow for retroactive changes that
do not require changing firmware. That's why ACPI carries new identifiers
for even very slightly different devices too. Once the firmware containing
(ACPI tables / DTB) is put on a production device, it is generally not
going to ever change.
CHIP_ID registers are a good tool to validate that the author of the
firmware table is doing the right thing, but solely relying on them
encourages creating a "vendor,haptic" compatible, which I'm sure you'll
agree is totally meaningless.
That's especially if the naming scheme makes no sense and you can't
even factor out a common wildcard-name (which also happens to be the case
quite often)
Plus a compatible is used to restrict/modify the set of allowed/required
properties, so having an "actual" compatible is required for schema
validation to work
Konrad
^ permalink raw reply
* Re: [PATCH] HID: Intel-thc-hid: Intel-thc: Fix wrong register fields updating
From: Jiri Kosina @ 2026-02-02 13:40 UTC (permalink / raw)
To: Even Xu; +Cc: bentiss, srinivas.pandruvada, linux-input, linux-kernel,
Rui Zhang
In-Reply-To: <20260202030144.967964-1-even.xu@intel.com>
On Mon, 2 Feb 2026, Even Xu wrote:
> Clear the target bit fields in register before setting new values. This
> ensures proper field updates by removing any existing bits that might
> interfere with the new configuration.
>
> Fixes: 22da60f0304b ("HID: Intel-thc-hid: Intel-thc: Introduce interrupt delay control")
> Fixes: 45e92a093099 ("HID: Intel-thc-hid: Intel-thc: Introduce max input size control")
> Signed-off-by: Even Xu <even.xu@intel.com>
> Tested-by: Rui Zhang <rui1.zhang@intel.com>
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* RE: [PATCH v2] HID: intel-ish-hid: fix NULL-ptr-deref in ishtp_bus_remove_all_clients
From: Jiri Kosina @ 2026-02-02 13:38 UTC (permalink / raw)
To: Lin, Ryan
Cc: linux-kernel@vger.kernel.org, linux-input@vger.kernel.org,
srinivas.pandruvada@linux.intel.com, Zhang, Lixu,
bentiss@kernel.org
In-Reply-To: <PH0PR11MB749313441EEE2608C86E43818E9CA@PH0PR11MB7493.namprd11.prod.outlook.com>
On Sat, 31 Jan 2026, Lin, Ryan wrote:
> Hi Jiri and Benjamin,
>
> Looping you in for this v2. Apologies for the oversight in the To list.
Thanks for fixing that up. I've now queued the patch.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: sony: add second device IDs to CRKD Gibson SG
From: Jiri Kosina @ 2026-02-02 13:35 UTC (permalink / raw)
To: Rosalie Wanders; +Cc: Benjamin Tissoires, linux-input, linux-kernel
In-Reply-To: <20260129071227.5533-1-rosalie@mailbox.org>
On Thu, 29 Jan 2026, Rosalie Wanders wrote:
> CRKD changed the product IDs in a firmware revision, yet the dongle
> doesn't have these ID changes included yet but will have them included
> in a future firmware revision.
>
> CRKD also doesn't allow the firmware to be updated on Linux so having
> both the old and new product IDs is necessary.
>
> Signed-off-by: Rosalie Wanders <rosalie@mailbox.org>
I've added
Fixes: bbf992775faa66d6c9 ("HID: sony: add support for Rock Band 4 PS4 and PS5 guitars")
and applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH 0/2] HID: logitech-hidpp: Add support for Logitech K980
From: Jiri Kosina @ 2026-02-02 12:48 UTC (permalink / raw)
To: Bastien Nocera
Cc: linux-input, linux-kernel, Benjamin Tissoires,
Peter F . Patel-Schneider, Filipe Laíns, Nestor Lopez Casado
In-Reply-To: <20260125121257.26508-1-hadess@hadess.net>
On Sun, 25 Jan 2026, Bastien Nocera wrote:
> Tested over Bluetooth.
>
> Bastien Nocera (2):
> HID: logitech-dj: Differentiate "invalid device index" error
> HID: logitech-hidpp: Add support for Logitech K980
Applied, thanks a lot Bastien.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [Poke?] Re: [PATCH v2 0/1] HID: switch2: Add preliminary Switch 2 controller driver
From: Jiri Kosina @ 2026-02-02 11:41 UTC (permalink / raw)
To: lyude
Cc: Vicki Pfau, Dmitry Torokhov, Benjamin Tissoires, linux-input,
Luiz Augusto von Dentz, Bastien Nocera
In-Reply-To: <721a51815d1cc21aa22e255ce31598035b687592.camel@redhat.com>
On Fri, 30 Jan 2026, lyude@redhat.com wrote:
> Bentiss/Jiri, any chance this patch is on your radar?
I've reviewed the driver already, and I was sure I sent out an e-mail
asking for a minor nit -- better naming of the driver (we usuall name
drivers by vendors, not specific devices. But I can't find any traces of
such e-mail, so I probably hallucinated it, sorry.
Vicki, is there any reason not to follow the standard naming, and either
fold the whole thing into hid-nintendo, or built it separately under e.g.
hid-nintendo-switch2.c (which might or not might be then linked into
hid-nintendo proper)?
Thanks,
--
Jiri Kosina
SUSE Labs
^ 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