* 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: [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 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 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 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 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 12:01 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: <2026020346-ashamed-campfire-b483@gregkh>
вт, 3 лют. 2026 р. о 14:00 Greg Kroah-Hartman <gregkh@linuxfoundation.org> пише:
>
> 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 :)
>
Noted
> > > > +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.
>
Noted. Thank you.
> thanks,
>
> greg k-h
^ permalink raw reply
* Re: [PATCH] HID: asus: Add check for cancelling fn_lock_sync_work
From: Sahil Chandna @ 2026-02-03 12:07 UTC (permalink / raw)
To: Alan Stern
Cc: Jiri Kosina, bentiss, connorbelli2003, linux-input, linux-kernel
In-Reply-To: <273d8509-3d2e-4686-b56f-a12c5c203291@rowland.harvard.edu>
On Mon, Feb 02, 2026 at 10:27:33PM -0500, Alan Stern wrote:
>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
Thanks, I will test with mutex between INIT_WORK and cancel_work
and share v2.
Regards,
Sahil
^ permalink raw reply
* Re: [syzbot] [input?] [usb?] WARNING in asus_remove
From: syzbot @ 2026-02-03 13:35 UTC (permalink / raw)
To: bentiss, chandna.sahil, jikos, linux-input, linux-kernel,
linux-usb, syzkaller-bugs
In-Reply-To: <aYHJUJzHlkBD_Nza@chandna.localdomain>
Hello,
syzbot tried to test the proposed patch but the build/boot failed:
failed to apply patch:
checking file drivers/hid/hid-asus.c
Hunk #1 FAILED at 147.
Hunk #4 succeeded at 1346 with fuzz 1.
1 out of 4 hunks FAILED
Tested on:
commit: 193579fe Add linux-next specific files for 20260202
git tree: linux-next
kernel config: https://syzkaller.appspot.com/x/.config?x=750532df2c47a03
dashboard link: https://syzkaller.appspot.com/bug?extid=13f8286fa2de04a7cd48
compiler:
patch: https://syzkaller.appspot.com/x/patch.diff?x=1129c53a580000
^ permalink raw reply
* Re: [PATCH v1 07/10] dt-bindings: input: cpcap-pwrbutton: convert to schema
From: Rob Herring @ 2026-02-03 15:01 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: David Lechner, Jonathan Cameron, Nuno Sá, Andy Shevchenko,
Krzysztof Kozlowski, Conor Dooley, Dmitry Torokhov, Lee Jones,
Pavel Machek, Liam Girdwood, Mark Brown, Alexandre Belloni,
Dixit Parmar, Tony Lindgren, linux-iio, devicetree, linux-kernel,
linux-input, linux-leds, linux-rtc
In-Reply-To: <CAPVz0n25ukBJ6=hmmR9nd4MBoPkHaHQ+ZHMXYxghYZdkB28_sg@mail.gmail.com>
On Sun, Feb 01, 2026 at 09:07:07AM +0200, Svyatoslav Ryhel wrote:
> сб, 31 січ. 2026 р. о 22:02 David Lechner <dlechner@baylibre.com> пише:
> >
> > On 1/25/26 7:42 AM, Svyatoslav Ryhel wrote:
> > > Convert power button devicetree bindings for the Motorola CPCAP MFD from
> > > TXT to YAML format. This patch does not change any functionality; the
> > > bindings remain the same.
> > >
> > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > ---
> > > .../bindings/input/cpcap-pwrbutton.txt | 20 ------------
> > > .../input/motorola,cpcap-pwrbutton.yaml | 32 +++++++++++++++++++
> > > 2 files changed, 32 insertions(+), 20 deletions(-)
> > > delete mode 100644 Documentation/devicetree/bindings/input/cpcap-pwrbutton.txt
> > > create mode 100644 Documentation/devicetree/bindings/input/motorola,cpcap-pwrbutton.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/input/cpcap-pwrbutton.txt b/Documentation/devicetree/bindings/input/cpcap-pwrbutton.txt
> > > deleted file mode 100644
> > > index 0dd0076daf71..000000000000
> > > --- a/Documentation/devicetree/bindings/input/cpcap-pwrbutton.txt
> > > +++ /dev/null
> > > @@ -1,20 +0,0 @@
> > > -Motorola CPCAP on key
> > > -
> > > -This module is part of the CPCAP. For more details about the whole
> > > -chip see Documentation/devicetree/bindings/mfd/motorola-cpcap.txt.
> > > -
> > > -This module provides a simple power button event via an Interrupt.
> > > -
> > > -Required properties:
> > > -- compatible: should be one of the following
> > > - - "motorola,cpcap-pwrbutton"
> > > -- interrupts: irq specifier for CPCAP's ON IRQ
> > > -
> > > -Example:
> > > -
> > > -&cpcap {
> > > - cpcap_pwrbutton: pwrbutton {
> > > - compatible = "motorola,cpcap-pwrbutton";
> > > - interrupts = <23 IRQ_TYPE_NONE>;
> > > - };
> > > -};
> > > diff --git a/Documentation/devicetree/bindings/input/motorola,cpcap-pwrbutton.yaml b/Documentation/devicetree/bindings/input/motorola,cpcap-pwrbutton.yaml
> > > new file mode 100644
> > > index 000000000000..643f6b2b1f13
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/input/motorola,cpcap-pwrbutton.yaml
> > > @@ -0,0 +1,32 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/input/motorola,cpcap-pwrbutton.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Motorola CPCAP PMIC power key
> > > +
> > > +maintainers:
> > > + - Svyatoslav Ryhel <clamor95@gmail.com>
> > > +
> > > +description:
> > > + This module is part of the Motorola CPCAP MFD device. For more details
> > > + see Documentation/devicetree/bindings/mfd/motorola,cpcap.yaml. The
> > > + power key is represented as a sub-node of the PMIC node on the device
> > > + tree.
> > > +
> > > +properties:
> > > + compatible:
> > > + const: motorola,cpcap-pwrbutton
> > > +
> > > + interrupts:
> > > + minItems: 1
> >
> > Should this be maxItems: 1?
> >
> > > + description: CPCAP's ON interrupt
> >
> > Or I suppose:
> >
> > items:
> > - description: ...
> >
>
> Both options are perfectly fine for me, and I lean towards using
> "items: desc" but I would like to hear what schema maintainers would
> say, which layout is preferred in this case.
Either is fine. 'description' is fine if you have something specific
about the interrupt to say. Saying what the interrupt is for is
specific. So 'description' is good in this case.
Rob
^ permalink raw reply
* Re: [PATCH v1 07/10] dt-bindings: input: cpcap-pwrbutton: convert to schema
From: Svyatoslav Ryhel @ 2026-02-03 15:03 UTC (permalink / raw)
To: Rob Herring
Cc: David Lechner, Jonathan Cameron, Nuno Sá, Andy Shevchenko,
Krzysztof Kozlowski, Conor Dooley, Dmitry Torokhov, Lee Jones,
Pavel Machek, Liam Girdwood, Mark Brown, Alexandre Belloni,
Dixit Parmar, Tony Lindgren, linux-iio, devicetree, linux-kernel,
linux-input, linux-leds, linux-rtc
In-Reply-To: <20260203150136.GA2294714-robh@kernel.org>
вт, 3 лют. 2026 р. о 17:01 Rob Herring <robh@kernel.org> пише:
>
> On Sun, Feb 01, 2026 at 09:07:07AM +0200, Svyatoslav Ryhel wrote:
> > сб, 31 січ. 2026 р. о 22:02 David Lechner <dlechner@baylibre.com> пише:
> > >
> > > On 1/25/26 7:42 AM, Svyatoslav Ryhel wrote:
> > > > Convert power button devicetree bindings for the Motorola CPCAP MFD from
> > > > TXT to YAML format. This patch does not change any functionality; the
> > > > bindings remain the same.
> > > >
> > > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > > ---
> > > > .../bindings/input/cpcap-pwrbutton.txt | 20 ------------
> > > > .../input/motorola,cpcap-pwrbutton.yaml | 32 +++++++++++++++++++
> > > > 2 files changed, 32 insertions(+), 20 deletions(-)
> > > > delete mode 100644 Documentation/devicetree/bindings/input/cpcap-pwrbutton.txt
> > > > create mode 100644 Documentation/devicetree/bindings/input/motorola,cpcap-pwrbutton.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/input/cpcap-pwrbutton.txt b/Documentation/devicetree/bindings/input/cpcap-pwrbutton.txt
> > > > deleted file mode 100644
> > > > index 0dd0076daf71..000000000000
> > > > --- a/Documentation/devicetree/bindings/input/cpcap-pwrbutton.txt
> > > > +++ /dev/null
> > > > @@ -1,20 +0,0 @@
> > > > -Motorola CPCAP on key
> > > > -
> > > > -This module is part of the CPCAP. For more details about the whole
> > > > -chip see Documentation/devicetree/bindings/mfd/motorola-cpcap.txt.
> > > > -
> > > > -This module provides a simple power button event via an Interrupt.
> > > > -
> > > > -Required properties:
> > > > -- compatible: should be one of the following
> > > > - - "motorola,cpcap-pwrbutton"
> > > > -- interrupts: irq specifier for CPCAP's ON IRQ
> > > > -
> > > > -Example:
> > > > -
> > > > -&cpcap {
> > > > - cpcap_pwrbutton: pwrbutton {
> > > > - compatible = "motorola,cpcap-pwrbutton";
> > > > - interrupts = <23 IRQ_TYPE_NONE>;
> > > > - };
> > > > -};
> > > > diff --git a/Documentation/devicetree/bindings/input/motorola,cpcap-pwrbutton.yaml b/Documentation/devicetree/bindings/input/motorola,cpcap-pwrbutton.yaml
> > > > new file mode 100644
> > > > index 000000000000..643f6b2b1f13
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/input/motorola,cpcap-pwrbutton.yaml
> > > > @@ -0,0 +1,32 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/input/motorola,cpcap-pwrbutton.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Motorola CPCAP PMIC power key
> > > > +
> > > > +maintainers:
> > > > + - Svyatoslav Ryhel <clamor95@gmail.com>
> > > > +
> > > > +description:
> > > > + This module is part of the Motorola CPCAP MFD device. For more details
> > > > + see Documentation/devicetree/bindings/mfd/motorola,cpcap.yaml. The
> > > > + power key is represented as a sub-node of the PMIC node on the device
> > > > + tree.
> > > > +
> > > > +properties:
> > > > + compatible:
> > > > + const: motorola,cpcap-pwrbutton
> > > > +
> > > > + interrupts:
> > > > + minItems: 1
> > >
> > > Should this be maxItems: 1?
> > >
> > > > + description: CPCAP's ON interrupt
> > >
> > > Or I suppose:
> > >
> > > items:
> > > - description: ...
> > >
> >
> > Both options are perfectly fine for me, and I lean towards using
> > "items: desc" but I would like to hear what schema maintainers would
> > say, which layout is preferred in this case.
>
> Either is fine. 'description' is fine if you have something specific
> about the interrupt to say. Saying what the interrupt is for is
> specific. So 'description' is good in this case.
>
Noted, thank you.
> Rob
^ permalink raw reply
* [PATCH 1/2] dt-bindings: input: add GPIO charlieplex keypad
From: Hugo Villeneuve @ 2026-02-03 15:49 UTC (permalink / raw)
To: hvilleneuve, dmitry.torokhov, robh, krzk+dt, conor+dt
Cc: linux-input, devicetree, linux-kernel, hugo
In-Reply-To: <20260203155023.536103-1-hugo@hugovil.com>
From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
Add DT bindings for GPIO charlieplex keypad.
Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
.../input/gpio-charlieplex-keypad.yaml | 88 +++++++++++++++++++
1 file changed, 88 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml
diff --git a/Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml b/Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml
new file mode 100644
index 0000000000000..b382c8caa096d
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml
@@ -0,0 +1,88 @@
+# 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.
+ This mechanism doesn't allow to detect simultaneous key presses.
+
+allOf:
+ - $ref: input.yaml#
+ - $ref: /schemas/input/matrix-keymap.yaml#
+
+properties:
+ compatible:
+ const: gpio-charlieplex-keypad
+
+ line-scan-delay-us:
+ description:
+ Delay, measured in microseconds, that is needed
+ before we can scan keypad after activating one line.
+ default: 0
+
+ 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
+
+ linux,no-autorepeat:
+ type: boolean
+ description: Do not enable autorepeat feature.
+
+ gpio-activelow:
+ type: boolean
+ description:
+ Force GPIO polarity to active low.
+ In the absence of this property GPIOs are treated as active high.
+
+ debounce-delay-ms:
+ description: Debounce interval in milliseconds.
+ default: 5
+
+ poll-interval: true
+
+ wakeup-source: true
+
+required:
+ - compatible
+ - line-gpios
+ - linux,keymap
+ - poll-interval
+
+additionalProperties: false
+
+examples:
+ - |
+ charlieplex-keypad {
+ compatible = "gpio-charlieplex-keypad";
+ debounce-delay-ms = <20>;
+ poll-interval = <5>;
+ line-scan-delay-us = <2>;
+
+ line-gpios = <&gpio2 25 0
+ &gpio2 26 0
+ &gpio2 27 0>;
+
+ /* MATRIX_KEY(output, input, key-code) */
+ linux,keymap = <
+ MATRIX_KEY(0, 1, KEY_F1)
+ MATRIX_KEY(0, 2, KEY_F2)
+ MATRIX_KEY(1, 0, KEY_F3)
+ MATRIX_KEY(1, 2, KEY_F4)
+ MATRIX_KEY(2, 0, KEY_F5)
+ MATRIX_KEY(2, 1, KEY_F6)
+ >;
+ };
--
2.47.3
^ permalink raw reply related
* [PATCH 2/2] Input: charlieplex_keypad: add GPIO charlieplex keypad
From: Hugo Villeneuve @ 2026-02-03 15:49 UTC (permalink / raw)
To: hvilleneuve, dmitry.torokhov, robh, krzk+dt, conor+dt
Cc: linux-input, devicetree, linux-kernel, hugo
In-Reply-To: <20260203155023.536103-1-hugo@hugovil.com>
From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
Add support for GPIO-based charlieplex keypad, allowing to control
N^2-N keys using N GPIO lines.
Reuse matrix keypad keymap to simplify, even if there is no concept
of rows and columns in this type of keyboard.
Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
MAINTAINERS | 7 +
drivers/input/keyboard/Kconfig | 14 ++
drivers/input/keyboard/Makefile | 1 +
drivers/input/keyboard/charlieplex_keypad.c | 223 ++++++++++++++++++++
4 files changed, 245 insertions(+)
create mode 100644 drivers/input/keyboard/charlieplex_keypad.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 9ed6d11a77466..0b2d71f32b400 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5765,6 +5765,13 @@ S: Maintained
F: Documentation/hwmon/powerz.rst
F: drivers/hwmon/powerz.c
+CHARLIEPLEX KEYPAD DRIVER
+M: Hugo Villeneuve <hvilleneuve@dimonoff.com>
+S: Supported
+W: http://www.mosaic-industries.com/embedded-systems/microcontroller-projects/electronic-circuits/matrix-keypad-scan-decode
+F: Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml
+F: drivers/input/keyboard/charlieplex_keypad.c
+
CHECKPATCH
M: Andy Whitcroft <apw@canonical.com>
M: Joe Perches <joe@perches.com>
diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index 2ff4fef322c24..ae54b4b7e2d8a 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -289,6 +289,20 @@ config KEYBOARD_MATRIX
To compile this driver as a module, choose M here: the
module will be called matrix_keypad.
+config KEYBOARD_CHARLIEPLEX
+ tristate "GPIO driven chearlieplex keypad support"
+ depends on GPIOLIB || COMPILE_TEST
+ select INPUT_MATRIXKMAP
+ help
+ Enable support for GPIO driven charlieplex keypad. A charlieplex
+ keypad allows to use fewer GPIO lines to interface to key switches.
+ For example, an N lines charlieplex keypad can be used to interface
+ to N^2-N different key switches. However, this type of keypad
+ cannot detect more than one key press at a time.
+
+ To compile this driver as a module, choose M here: the
+ module will be called charlieplex_keypad.
+
config KEYBOARD_HIL_OLD
tristate "HP HIL keyboard support (simple driver)"
depends on GSC || HP300
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index 2d906e14f3e27..40b5cf5d374d2 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -40,6 +40,7 @@ obj-$(CONFIG_KEYBOARD_LOCOMO) += locomokbd.o
obj-$(CONFIG_KEYBOARD_LPC32XX) += lpc32xx-keys.o
obj-$(CONFIG_KEYBOARD_MAPLE) += maple_keyb.o
obj-$(CONFIG_KEYBOARD_MATRIX) += matrix_keypad.o
+obj-$(CONFIG_KEYBOARD_CHARLIEPLEX) += charlieplex_keypad.o
obj-$(CONFIG_KEYBOARD_MAX7359) += max7359_keypad.o
obj-$(CONFIG_KEYBOARD_MAX7360) += max7360-keypad.o
obj-$(CONFIG_KEYBOARD_MPR121) += mpr121_touchkey.o
diff --git a/drivers/input/keyboard/charlieplex_keypad.c b/drivers/input/keyboard/charlieplex_keypad.c
new file mode 100644
index 0000000000000..f9d6f5ecfa8dc
--- /dev/null
+++ b/drivers/input/keyboard/charlieplex_keypad.c
@@ -0,0 +1,223 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * GPIO driven charlieplex keypad driver
+ *
+ * Copyright (c) 2025 Hugo Villeneuve <hvilleneuve@dimonoff.com>
+ *
+ * Based on matrix_keyboard.c
+ */
+
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/input.h>
+#include <linux/input/matrix_keypad.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/types.h>
+
+struct charlieplex_keypad {
+ struct input_dev *input_dev;
+ struct gpio_descs *line_gpios;
+ unsigned int nlines;
+ unsigned int line_scan_delay_us;
+ unsigned int debounce_threshold;
+ unsigned int debounce_count;
+ int debounce_code;
+ int current_code;
+};
+
+static void charlieplex_keypad_report_key(struct input_dev *input)
+{
+ struct charlieplex_keypad *keypad = input_get_drvdata(input);
+ const unsigned short *keycodes = input->keycode;
+
+ if (keypad->current_code > 0) {
+ input_event(input, EV_MSC, MSC_SCAN, keypad->current_code);
+ input_report_key(input, keycodes[keypad->current_code], 0);
+ }
+
+ if (keypad->debounce_code) {
+ input_event(input, EV_MSC, MSC_SCAN, keypad->debounce_code);
+ input_report_key(input, keycodes[keypad->debounce_code], 1);
+ }
+
+ input_sync(input);
+ keypad->current_code = keypad->debounce_code;
+}
+
+static void charlieplex_keypad_check_switch_change(struct input_dev *input,
+ int code)
+{
+ struct charlieplex_keypad *keypad = input_get_drvdata(input);
+
+ if (code != keypad->debounce_code) {
+ keypad->debounce_count = 0;
+ keypad->debounce_code = code;
+ } else if (keypad->debounce_count < keypad->debounce_threshold) {
+ keypad->debounce_count++;
+
+ if (keypad->debounce_count >= keypad->debounce_threshold &&
+ keypad->debounce_code != keypad->current_code)
+ charlieplex_keypad_report_key(input);
+ }
+}
+
+static void charlieplex_keypad_poll(struct input_dev *input)
+{
+ struct charlieplex_keypad *keypad = input_get_drvdata(input);
+ int oline;
+ int code;
+
+ for (code = 0, oline = 0; oline < keypad->nlines; oline++) {
+ DECLARE_BITMAP(values, MATRIX_MAX_ROWS);
+ int iline;
+ int rc;
+
+ /* Activate only one line as output at a time. */
+ gpiod_direction_output(keypad->line_gpios->desc[oline], 1);
+
+ if (keypad->line_scan_delay_us)
+ fsleep(keypad->line_scan_delay_us);
+
+ /* Read input on all other lines. */
+ rc = gpiod_get_array_value_cansleep(keypad->line_gpios->ndescs,
+ keypad->line_gpios->desc,
+ keypad->line_gpios->info, values);
+ if (rc)
+ return;
+
+ for (iline = 0; iline < keypad->nlines; iline++) {
+ if (iline == oline)
+ continue; /* Do not read active output line. */
+
+ /* Check if GPIO is asserted. */
+ if (test_bit(iline, values)) {
+ code = MATRIX_SCAN_CODE(oline, iline,
+ get_count_order(keypad->nlines));
+ /*
+ * Exit loop immediately since we cannot detect
+ * more than one key press at a time.
+ */
+ break;
+ }
+ }
+
+ gpiod_direction_input(keypad->line_gpios->desc[oline]);
+
+ if (code)
+ break;
+ }
+
+ charlieplex_keypad_check_switch_change(input, code);
+}
+
+static int charlieplex_keypad_init_gpio(struct platform_device *pdev,
+ struct charlieplex_keypad *keypad)
+{
+ bool active_low;
+ int nkeys;
+ int i;
+
+ keypad->line_gpios = devm_gpiod_get_array(&pdev->dev, "line", GPIOD_IN);
+ if (IS_ERR(keypad->line_gpios))
+ return PTR_ERR(keypad->line_gpios);
+
+ keypad->nlines = keypad->line_gpios->ndescs;
+
+ if (keypad->nlines > MATRIX_MAX_ROWS)
+ return -EINVAL;
+
+ nkeys = (keypad->nlines * keypad->nlines) - keypad->nlines;
+
+ active_low = device_property_read_bool(&pdev->dev, "gpio-activelow");
+
+ for (i = 0; i < keypad->nlines; i++) {
+ gpiod_set_consumer_name(keypad->line_gpios->desc[i], "charlieplex_kbd_line");
+
+ if (active_low ^ gpiod_is_active_low(keypad->line_gpios->desc[i]))
+ gpiod_toggle_active_low(keypad->line_gpios->desc[i]);
+ }
+
+ return 0;
+}
+
+static int charlieplex_keypad_probe(struct platform_device *pdev)
+{
+ struct charlieplex_keypad *keypad;
+ unsigned int debounce_interval_ms;
+ unsigned int poll_interval_ms;
+ struct input_dev *input_dev;
+ int err;
+
+ keypad = devm_kzalloc(&pdev->dev, sizeof(*keypad), GFP_KERNEL);
+ if (!keypad)
+ return -ENOMEM;
+
+ input_dev = devm_input_allocate_device(&pdev->dev);
+ if (!input_dev)
+ return -ENOMEM;
+
+ keypad->input_dev = input_dev;
+
+ device_property_read_u32(&pdev->dev, "poll-interval", &poll_interval_ms);
+ device_property_read_u32(&pdev->dev, "debounce-delay-ms", &debounce_interval_ms);
+ device_property_read_u32(&pdev->dev, "line-scan-delay-us", &keypad->line_scan_delay_us);
+
+ keypad->current_code = -1;
+ keypad->debounce_code = -1;
+ keypad->debounce_threshold = DIV_ROUND_UP(debounce_interval_ms, poll_interval_ms);
+
+ err = charlieplex_keypad_init_gpio(pdev, keypad);
+ if (err)
+ return err;
+
+ input_dev->name = pdev->name;
+ input_dev->id.bustype = BUS_HOST;
+
+ err = matrix_keypad_build_keymap(NULL, NULL, keypad->nlines,
+ keypad->nlines, NULL, input_dev);
+ if (err)
+ dev_err_probe(&pdev->dev, -ENOMEM, "failed to build keymap\n");
+
+ if (!device_property_read_bool(&pdev->dev, "linux,no-autorepeat"))
+ __set_bit(EV_REP, input_dev->evbit);
+
+ input_set_capability(input_dev, EV_MSC, MSC_SCAN);
+
+ err = input_setup_polling(input_dev, charlieplex_keypad_poll);
+ if (err)
+ dev_err_probe(&pdev->dev, err, "unable to set up polling\n");
+
+ input_set_poll_interval(input_dev, poll_interval_ms);
+
+ input_set_drvdata(input_dev, keypad);
+
+ err = input_register_device(keypad->input_dev);
+ if (err)
+ return err;
+
+ platform_set_drvdata(pdev, keypad);
+
+ return 0;
+}
+
+static const struct of_device_id charlieplex_keypad_dt_match[] = {
+ { .compatible = "gpio-charlieplex-keypad" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, charlieplex_keypad_dt_match);
+
+static struct platform_driver charlieplex_keypad_driver = {
+ .probe = charlieplex_keypad_probe,
+ .driver = {
+ .name = "charlieplex-keypad",
+ .of_match_table = of_match_ptr(charlieplex_keypad_dt_match),
+ },
+};
+module_platform_driver(charlieplex_keypad_driver);
+
+MODULE_AUTHOR("Hugo Villeneuve <hvilleneuve@dimonoff.com>");
+MODULE_DESCRIPTION("GPIO driven charlieplex keypad driver");
+MODULE_LICENSE("GPL");
--
2.47.3
^ permalink raw reply related
* [PATCH 0/2] input: add GPIO-based charlieplex keypad
From: Hugo Villeneuve @ 2026-02-03 15:49 UTC (permalink / raw)
To: hvilleneuve, dmitry.torokhov, robh, krzk+dt, conor+dt
Cc: linux-input, devicetree, linux-kernel, hugo
From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
Hello,
this patch series add a new GPIO charlieplex keypad driver.
I have tested the driver on a custom board with a Solidrun RZ/G2LC SOM
with three charlieplex keyboards, all connected thru a a PCA9416 I2C GPIO
expander.
Thank you.
Hugo Villeneuve (2):
dt-bindings: input: add GPIO charlieplex keypad
Input: charlieplex_keypad: add GPIO charlieplex keypad
.../input/gpio-charlieplex-keypad.yaml | 88 +++++++
MAINTAINERS | 7 +
drivers/input/keyboard/Kconfig | 14 ++
drivers/input/keyboard/Makefile | 1 +
drivers/input/keyboard/charlieplex_keypad.c | 223 ++++++++++++++++++
5 files changed, 333 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml
create mode 100644 drivers/input/keyboard/charlieplex_keypad.c
base-commit: ed8a4ef29da3821ee3155d3b1925fa67fc92aae2
--
2.47.3
^ permalink raw reply
* Re: [PATCH v1 2/9] misc: Support Asus Transformer's EC access device
From: Svyatoslav Ryhel @ 2026-02-03 16:28 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: <2026020346-ashamed-campfire-b483@gregkh>
вт, 3 лют. 2026 р. о 14:00 Greg Kroah-Hartman <gregkh@linuxfoundation.org> пише:
>
> 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.
>
There is no similar way to handle BIN_ATTR_RW in the debugfs (), may I
preserve dockram_read/write with __maybe_unused instead of removing
them? I will add comment with explanation
> 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 16:45 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: <CAPVz0n2HmLwdif5ry+y56LB8Gpwh2o9_gJ7K2jhcZVR=rPgfPA@mail.gmail.com>
On Tue, Feb 03, 2026 at 06:28:11PM +0200, Svyatoslav Ryhel wrote:
> вт, 3 лют. 2026 р. о 14:00 Greg Kroah-Hartman <gregkh@linuxfoundation.org> пише:
> >
> > 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.
> >
>
> There is no similar way to handle BIN_ATTR_RW in the debugfs (), may I
> preserve dockram_read/write with __maybe_unused instead of removing
> them? I will add comment with explanation
debugfs allows you to do much much more than simple stuff like
BIN_ATTR_RW(). Go wild there, but don't put debugging stuff in sysfs,
that is NOT what it is there for at all, but rather, that is exactly
what debugfs is for.
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 16:50 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: <2026020314-humbling-mobility-c24a@gregkh>
вт, 3 лют. 2026 р. о 18:45 Greg Kroah-Hartman <gregkh@linuxfoundation.org> пише:
>
> On Tue, Feb 03, 2026 at 06:28:11PM +0200, Svyatoslav Ryhel wrote:
> > вт, 3 лют. 2026 р. о 14:00 Greg Kroah-Hartman <gregkh@linuxfoundation.org> пише:
> > >
> > > 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.
> > >
> >
> > There is no similar way to handle BIN_ATTR_RW in the debugfs (), may I
> > preserve dockram_read/write with __maybe_unused instead of removing
> > them? I will add comment with explanation
>
> debugfs allows you to do much much more than simple stuff like
> BIN_ATTR_RW(). Go wild there, but don't put debugging stuff in sysfs,
> that is NOT what it is there for at all, but rather, that is exactly
> what debugfs is for.
>
I am removing said stuff from sysfs, that is not what I am asking.
Debugs does not allow to upload register values in a form of binary
block. It allows only dumping via debugfs_create_blob or
debugfs_create_regset32 but not writing. If you know examples of
reading and writing register sets as binary data, please point me to
it.
I am asking if it is possible only to preserve dockram_read/write
functions in the code, without exposing it to sysfs.
> 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 16:58 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: <CAPVz0n0TMOCYnMiVUZ7xx-1SqrXuaVCOY-o4-x9L=f-xSMDj8g@mail.gmail.com>
On Tue, Feb 03, 2026 at 06:50:01PM +0200, Svyatoslav Ryhel wrote:
> > debugfs allows you to do much much more than simple stuff like
> > BIN_ATTR_RW(). Go wild there, but don't put debugging stuff in sysfs,
> > that is NOT what it is there for at all, but rather, that is exactly
> > what debugfs is for.
> >
>
> I am removing said stuff from sysfs, that is not what I am asking.
> Debugs does not allow to upload register values in a form of binary
> block. It allows only dumping via debugfs_create_blob or
> debugfs_create_regset32 but not writing. If you know examples of
> reading and writing register sets as binary data, please point me to
> it.
You can easily write your own given that debugfs allows you to use what
ever file operations you want to use for a file. Why not just use that?
> I am asking if it is possible only to preserve dockram_read/write
> functions in the code, without exposing it to sysfs.
Why would you want to do that?
confused,
greg k-h
^ permalink raw reply
* [PATCH] HID: pidff: Fix condition effect bit clearing
From: Tomasz Pakuła @ 2026-02-03 17:42 UTC (permalink / raw)
To: jikos, bentiss, sashal
Cc: oleg, linux-input, linux-kernel, stable, tomasz.pakula.oficjalny
As reported by MPDarkGuy on discord, NULL pointer dereferences were
happening because not all the conditional effects bits were cleared.
Properly clear all conditional effect bits from ffbit
Signed-off-by: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>
---
Urgent for 6.19 rc period and backports for 6.18
drivers/hid/usbhid/hid-pidff.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
index a4e700b40ba9..56d6af39ba81 100644
--- a/drivers/hid/usbhid/hid-pidff.c
+++ b/drivers/hid/usbhid/hid-pidff.c
@@ -1452,10 +1452,13 @@ static int pidff_init_fields(struct pidff_device *pidff, struct input_dev *dev)
hid_warn(pidff->hid, "unknown ramp effect layout\n");
if (PIDFF_FIND_FIELDS(set_condition, PID_SET_CONDITION, 1)) {
- if (test_and_clear_bit(FF_SPRING, dev->ffbit) ||
- test_and_clear_bit(FF_DAMPER, dev->ffbit) ||
- test_and_clear_bit(FF_FRICTION, dev->ffbit) ||
- test_and_clear_bit(FF_INERTIA, dev->ffbit))
+ bool test = false;
+
+ test |= test_and_clear_bit(FF_SPRING, dev->ffbit);
+ test |= test_and_clear_bit(FF_DAMPER, dev->ffbit);
+ test |= test_and_clear_bit(FF_FRICTION, dev->ffbit);
+ test |= test_and_clear_bit(FF_INERTIA, dev->ffbit);
+ if (test)
hid_warn(pidff->hid, "unknown condition effect layout\n");
}
--
2.52.0
^ permalink raw reply related
* [PATCH 0/5] hid-pidff set_condition compatibility fixes
From: Tomasz Pakuła @ 2026-02-03 17:45 UTC (permalink / raw)
To: jikos, bentiss; +Cc: oleg, linux-input, linux-kernel, tomasz.pakula.oficjalny
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(-)
--
2.52.0
^ permalink raw reply
* [PATCH 1/5] HID: pidff: Refactor field quirks detection
From: Tomasz Pakuła @ 2026-02-03 17:45 UTC (permalink / raw)
To: jikos, bentiss; +Cc: oleg, linux-input, linux-kernel, tomasz.pakula.oficjalny
In-Reply-To: <20260203174531.2866644-1-tomasz.pakula.oficjalny@gmail.com>
In preparation for additional quirks
Co-developed-by: Oleg Makarenko <oleg@makarenk.ooo>
Signed-off-by: Oleg Makarenko <oleg@makarenk.ooo>
Signed-off-by: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>
---
drivers/hid/usbhid/hid-pidff.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
index a4e700b40ba9..68049d5d76b3 100644
--- a/drivers/hid/usbhid/hid-pidff.c
+++ b/drivers/hid/usbhid/hid-pidff.c
@@ -13,6 +13,7 @@
#include <linux/input.h>
#include <linux/minmax.h>
#include <linux/slab.h>
+#include <linux/stringify.h>
#include <linux/usb.h>
#define PID_EFFECTS_MAX 64
@@ -1053,6 +1054,11 @@ static int pidff_find_field_with_usage(int *usage_index,
return -1;
}
+#define PIDFF_MISSING_FIELD(name, quirks) \
+ ({ pr_debug("%s field not found, but that's OK\n", __stringify(name)); \
+ pr_debug("Setting MISSING_%s quirk\n", __stringify(name)); \
+ *quirks |= HID_PIDFF_QUIRK_MISSING_ ## name; })
+
/*
* Find fields from a report and fill a pidff_usage
*/
@@ -1060,9 +1066,6 @@ static int pidff_find_fields(struct pidff_usage *usage, const u8 *table,
struct hid_report *report, int count, int strict,
u32 *quirks)
{
- const u8 block_offset = pidff_set_condition[PID_PARAM_BLOCK_OFFSET];
- const u8 delay = pidff_set_effect[PID_START_DELAY];
-
if (!report) {
pr_debug("%s, null report\n", __func__);
return -1;
@@ -1080,17 +1083,14 @@ static int pidff_find_fields(struct pidff_usage *usage, const u8 *table,
continue;
}
- if (table[i] == delay) {
- pr_debug("Delay field not found, but that's OK\n");
- pr_debug("Setting MISSING_DELAY quirk\n");
- *quirks |= HID_PIDFF_QUIRK_MISSING_DELAY;
+ /* Field quirks auto-detection */
+ if (table[i] == pidff_set_effect[PID_START_DELAY])
+ PIDFF_MISSING_FIELD(DELAY, quirks);
- } else if (table[i] == block_offset) {
- pr_debug("PBO field not found, but that's OK\n");
- pr_debug("Setting MISSING_PBO quirk\n");
- *quirks |= HID_PIDFF_QUIRK_MISSING_PBO;
+ else if (table[i] == pidff_set_condition[PID_PARAM_BLOCK_OFFSET])
+ PIDFF_MISSING_FIELD(PBO, quirks);
- } else if (strict) {
+ else if (strict) {
pr_debug("failed to locate %d\n", i);
return -1;
}
--
2.52.0
^ permalink raw reply related
* [PATCH 3/5] HID: pidff: Add MISSING_NEG_SATURATION quirk
From: Tomasz Pakuła @ 2026-02-03 17:45 UTC (permalink / raw)
To: jikos, bentiss; +Cc: oleg, linux-input, linux-kernel, tomasz.pakula.oficjalny
In-Reply-To: <20260203174531.2866644-1-tomasz.pakula.oficjalny@gmail.com>
This is the same case as the previous MISSING_NEG_COEFFICIENT quirk
Co-developed-by: Oleg Makarenko <oleg@makarenk.ooo>
Signed-off-by: Oleg Makarenko <oleg@makarenk.ooo>
Signed-off-by: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>
---
drivers/hid/usbhid/hid-pidff.c | 14 ++++++++++----
drivers/hid/usbhid/hid-pidff.h | 3 +++
2 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
index aebf6c89643f..17bdc36d1908 100644
--- a/drivers/hid/usbhid/hid-pidff.c
+++ b/drivers/hid/usbhid/hid-pidff.c
@@ -619,16 +619,19 @@ static void pidff_set_condition_report(struct pidff_device *pidff,
effect->u.condition[i].center);
pidff_set_signed(&pidff->set_condition[PID_POS_COEFFICIENT],
effect->u.condition[i].right_coeff);
+ pidff_set(&pidff->set_condition[PID_POS_SATURATION],
+ effect->u.condition[i].right_saturation);
/* Omit Negative Coefficient if missing */
if (!(pidff->quirks & HID_PIDFF_QUIRK_MISSING_NEG_COEFFICIENT))
pidff_set_signed(&pidff->set_condition[PID_NEG_COEFFICIENT],
effect->u.condition[i].left_coeff);
- pidff_set(&pidff->set_condition[PID_POS_SATURATION],
- effect->u.condition[i].right_saturation);
- pidff_set(&pidff->set_condition[PID_NEG_SATURATION],
- effect->u.condition[i].left_saturation);
+ /* Omit Negative Saturation if missing */
+ if (!(pidff->quirks & HID_PIDFF_QUIRK_MISSING_NEG_SATURATION))
+ pidff_set_signed(&pidff->set_condition[PID_NEG_SATURATION],
+ effect->u.condition[i].left_saturation);
+
pidff_set(&pidff->set_condition[PID_DEAD_BAND],
effect->u.condition[i].deadband);
hid_hw_request(pidff->hid, pidff->reports[PID_SET_CONDITION],
@@ -1097,6 +1100,9 @@ static int pidff_find_fields(struct pidff_usage *usage, const u8 *table,
else if (table[i] == pidff_set_condition[PID_NEG_COEFFICIENT])
PIDFF_MISSING_FIELD(NEG_COEFFICIENT, quirks);
+ else if (table[i] == pidff_set_condition[PID_NEG_SATURATION])
+ PIDFF_MISSING_FIELD(NEG_SATURATION, quirks);
+
else if (strict) {
pr_debug("failed to locate %d\n", i);
return -1;
diff --git a/drivers/hid/usbhid/hid-pidff.h b/drivers/hid/usbhid/hid-pidff.h
index 5bf54e981543..8d879067718f 100644
--- a/drivers/hid/usbhid/hid-pidff.h
+++ b/drivers/hid/usbhid/hid-pidff.h
@@ -24,6 +24,9 @@
/* Allow devices with missing negative coefficient in the set condition usage */
#define HID_PIDFF_QUIRK_MISSING_NEG_COEFFICIENT BIT(5)
+/* Allow devices with missing negative saturation in the set condition usage */
+#define HID_PIDFF_QUIRK_MISSING_NEG_SATURATION BIT(6)
+
#ifdef CONFIG_HID_PID
int hid_pidff_init(struct hid_device *hid);
int hid_pidff_init_with_quirks(struct hid_device *hid, u32 initial_quirks);
--
2.52.0
^ permalink raw reply related
* [PATCH 2/5] HID: pidff: Add MISSING_NEG_COEFFICIENT quirk
From: Tomasz Pakuła @ 2026-02-03 17:45 UTC (permalink / raw)
To: jikos, bentiss; +Cc: oleg, linux-input, linux-kernel, tomasz.pakula.oficjalny
In-Reply-To: <20260203174531.2866644-1-tomasz.pakula.oficjalny@gmail.com>
Windows/Directinput allows devices with missing negative coefficient for
conditional effects. Negative coefficient is ignored in such cases.
Donot fail set_condition usage search if negative coefficient is missing.
Fixes conditional effect playback on Asetek wheelbases.
https://learn.microsoft.com/en-us/previous-versions/windows/desktop/ee416601(v=vs.85)
Co-developed-by: Oleg Makarenko <oleg@makarenk.ooo>
Signed-off-by: Oleg Makarenko <oleg@makarenk.ooo>
Signed-off-by: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>
---
drivers/hid/usbhid/hid-pidff.c | 11 +++++++++--
drivers/hid/usbhid/hid-pidff.h | 3 +++
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
index 68049d5d76b3..aebf6c89643f 100644
--- a/drivers/hid/usbhid/hid-pidff.c
+++ b/drivers/hid/usbhid/hid-pidff.c
@@ -619,8 +619,12 @@ static void pidff_set_condition_report(struct pidff_device *pidff,
effect->u.condition[i].center);
pidff_set_signed(&pidff->set_condition[PID_POS_COEFFICIENT],
effect->u.condition[i].right_coeff);
- pidff_set_signed(&pidff->set_condition[PID_NEG_COEFFICIENT],
- effect->u.condition[i].left_coeff);
+
+ /* Omit Negative Coefficient if missing */
+ if (!(pidff->quirks & HID_PIDFF_QUIRK_MISSING_NEG_COEFFICIENT))
+ pidff_set_signed(&pidff->set_condition[PID_NEG_COEFFICIENT],
+ effect->u.condition[i].left_coeff);
+
pidff_set(&pidff->set_condition[PID_POS_SATURATION],
effect->u.condition[i].right_saturation);
pidff_set(&pidff->set_condition[PID_NEG_SATURATION],
@@ -1090,6 +1094,9 @@ static int pidff_find_fields(struct pidff_usage *usage, const u8 *table,
else if (table[i] == pidff_set_condition[PID_PARAM_BLOCK_OFFSET])
PIDFF_MISSING_FIELD(PBO, quirks);
+ else if (table[i] == pidff_set_condition[PID_NEG_COEFFICIENT])
+ PIDFF_MISSING_FIELD(NEG_COEFFICIENT, quirks);
+
else if (strict) {
pr_debug("failed to locate %d\n", i);
return -1;
diff --git a/drivers/hid/usbhid/hid-pidff.h b/drivers/hid/usbhid/hid-pidff.h
index f321f675e131..5bf54e981543 100644
--- a/drivers/hid/usbhid/hid-pidff.h
+++ b/drivers/hid/usbhid/hid-pidff.h
@@ -21,6 +21,9 @@
/* Force all periodic effects to be uploaded as SINE */
#define HID_PIDFF_QUIRK_PERIODIC_SINE_ONLY BIT(4)
+/* Allow devices with missing negative coefficient in the set condition usage */
+#define HID_PIDFF_QUIRK_MISSING_NEG_COEFFICIENT BIT(5)
+
#ifdef CONFIG_HID_PID
int hid_pidff_init(struct hid_device *hid);
int hid_pidff_init_with_quirks(struct hid_device *hid, u32 initial_quirks);
--
2.52.0
^ permalink raw reply related
* [PATCH 4/5] HID: pidff: Add MISSING_DEADBAND quirk
From: Tomasz Pakuła @ 2026-02-03 17:45 UTC (permalink / raw)
To: jikos, bentiss; +Cc: oleg, linux-input, linux-kernel, tomasz.pakula.oficjalny
In-Reply-To: <20260203174531.2866644-1-tomasz.pakula.oficjalny@gmail.com>
Some devices (mainly Asetek) do not have deadband field in set
conditional usage. Do not fail set conditional usage search if it's
missing. Allows conditional effect playback on Asetek wheelbases.
Deadband is practically never used in simracing anyway.
Align property name in the whole driver to use 'deadband' without space.
Co-developed-by: Oleg Makarenko <oleg@makarenk.ooo>
Signed-off-by: Oleg Makarenko <oleg@makarenk.ooo>
Signed-off-by: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>
---
drivers/hid/usbhid/hid-pidff.c | 12 +++++++++---
drivers/hid/usbhid/hid-pidff.h | 3 +++
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
index 17bdc36d1908..8106b045a8f7 100644
--- a/drivers/hid/usbhid/hid-pidff.c
+++ b/drivers/hid/usbhid/hid-pidff.c
@@ -82,7 +82,7 @@ static const u8 pidff_set_envelope[] = { 0x22, 0x5b, 0x5c, 0x5d, 0x5e };
#define PID_NEG_COEFFICIENT 4
#define PID_POS_SATURATION 5
#define PID_NEG_SATURATION 6
-#define PID_DEAD_BAND 7
+#define PID_DEADBAND 7
static const u8 pidff_set_condition[] = {
0x22, 0x23, 0x60, 0x61, 0x62, 0x63, 0x64, 0x65
};
@@ -632,8 +632,11 @@ static void pidff_set_condition_report(struct pidff_device *pidff,
pidff_set_signed(&pidff->set_condition[PID_NEG_SATURATION],
effect->u.condition[i].left_saturation);
- pidff_set(&pidff->set_condition[PID_DEAD_BAND],
- effect->u.condition[i].deadband);
+ /* Omit Deadband field if missing */
+ if (!(pidff->quirks & HID_PIDFF_QUIRK_MISSING_DEADBAND))
+ pidff_set(&pidff->set_condition[PID_DEADBAND],
+ effect->u.condition[i].deadband);
+
hid_hw_request(pidff->hid, pidff->reports[PID_SET_CONDITION],
HID_REQ_SET_REPORT);
}
@@ -1103,6 +1106,9 @@ static int pidff_find_fields(struct pidff_usage *usage, const u8 *table,
else if (table[i] == pidff_set_condition[PID_NEG_SATURATION])
PIDFF_MISSING_FIELD(NEG_SATURATION, quirks);
+ else if (table[i] == pidff_set_condition[PID_DEADBAND])
+ PIDFF_MISSING_FIELD(DEADBAND, quirks);
+
else if (strict) {
pr_debug("failed to locate %d\n", i);
return -1;
diff --git a/drivers/hid/usbhid/hid-pidff.h b/drivers/hid/usbhid/hid-pidff.h
index 8d879067718f..c413aa732842 100644
--- a/drivers/hid/usbhid/hid-pidff.h
+++ b/drivers/hid/usbhid/hid-pidff.h
@@ -27,6 +27,9 @@
/* Allow devices with missing negative saturation in the set condition usage */
#define HID_PIDFF_QUIRK_MISSING_NEG_SATURATION BIT(6)
+/* Allow devices with missing deadband in the set condition usage */
+#define HID_PIDFF_QUIRK_MISSING_DEADBAND BIT(7)
+
#ifdef CONFIG_HID_PID
int hid_pidff_init(struct hid_device *hid);
int hid_pidff_init_with_quirks(struct hid_device *hid, u32 initial_quirks);
--
2.52.0
^ permalink raw reply related
* [PATCH 5/5] HID: Update MAINTAINERS for USB HID PID
From: Tomasz Pakuła @ 2026-02-03 17:45 UTC (permalink / raw)
To: jikos, bentiss; +Cc: oleg, linux-input, linux-kernel, tomasz.pakula.oficjalny
In-Reply-To: <20260203174531.2866644-1-tomasz.pakula.oficjalny@gmail.com>
Take de-facto ownership of core hid-pidff driver
Signed-off-by: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>
---
MAINTAINERS | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index a0dd762f5648..b6c16e134070 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11205,14 +11205,6 @@ F: drivers/hid/hid-sensor-*
F: drivers/iio/*/hid-*
F: include/linux/hid-sensor-*
-HID UNIVERSAL PIDFF DRIVER
-M: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>
-M: Oleg Makarenko <oleg@makarenk.ooo>
-L: linux-input@vger.kernel.org
-S: Maintained
-B: https://github.com/JacKeTUs/universal-pidff/issues
-F: drivers/hid/hid-universal-pidff.c
-
HID VRC-2 CAR CONTROLLER DRIVER
M: Marcus Folkesson <marcus.folkesson@gmail.com>
L: linux-input@vger.kernel.org
@@ -26927,6 +26919,15 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git
F: Documentation/hid/hiddev.rst
F: drivers/hid/usbhid/
+USB HID PID DRIVERS (USB WHEELBASES, JOYSTICKS, RUDDERS, ...)
+M: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>
+M: Oleg Makarenko <oleg@makarenk.ooo>
+L: linux-input@vger.kernel.org
+S: Maintained
+B: https://github.com/JacKeTUs/universal-pidff/issues
+F: drivers/hid/usbhid/hid-pidff*
+F: drivers/hid/hid-universal-pidff.c
+
USB INTEL XHCI ROLE MUX DRIVER
M: Hans de Goede <hansg@kernel.org>
L: linux-usb@vger.kernel.org
--
2.52.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox