* [PATCH] HID: mcp2221: fix slab out-of-bounds in mcp2221_raw_event
From: Atharv Dubey @ 2025-11-03 18:25 UTC (permalink / raw)
To: jikos
Cc: bentiss, linux-input, linux-kernel, Atharv Dubey,
syzbot+1018672fe70298606e5f
Fixes an out-of-bounds read triggered by malformed HID input reports.
Fixes: 3a8660878839 ("HID: mcp2221: add support for MCP2221 HID adapter")
Reported-by: syzbot+1018672fe70298606e5f@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=1018672fe70298606e5f
Signed-off-by: Atharv Dubey <atharvd440@gmail.com>
---
drivers/hid/hid-mcp2221.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/hid-mcp2221.c b/drivers/hid/hid-mcp2221.c
index a9fd7648515d..c97f0acbac8b 100644
--- a/drivers/hid/hid-mcp2221.c
+++ b/drivers/hid/hid-mcp2221.c
@@ -945,7 +945,7 @@ static int mcp2221_raw_event(struct hid_device *hdev,
switch (data[1]) {
case MCP2221_SUCCESS:
if ((data[mcp->gp_idx] == MCP2221_ALT_F_NOT_GPIOV) ||
- (mcp->gp_idx > 0 &&data[mcp->gp_idx - 1] == MCP2221_ALT_F_NOT_GPIOV)) {
+ (mcp->gp_idx > 0 && data[mcp->gp_idx - 1] == MCP2221_ALT_F_NOT_GPIOV)) {
mcp->status = -ENOENT;
} else {
mcp->status = 0;
--
2.43.0
^ permalink raw reply related
* [PATCH] HID: pidff: Fix needs_playback check
From: Tomasz Pakuła @ 2025-11-03 20:02 UTC (permalink / raw)
To: jikos, bentiss; +Cc: oleg, linux-input
Urgent for 6.18 rc period
A small bug made it's way here when rewriting code to Linux quality.
Currently, if an effect is not infinite and a program requests it's
playback with the same number of loops, the play command won't be fired
and if an effect is infinite, the spam will continue.
We want every playback update for non-infinite effects and only some
for infinite (detecting when a program requests stop with 0 which will
be different than previous value which is usually 1 or 255).
Signed-off-by: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>
---
drivers/hid/usbhid/hid-pidff.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
index edd61ef50e16..95377c5f6335 100644
--- a/drivers/hid/usbhid/hid-pidff.c
+++ b/drivers/hid/usbhid/hid-pidff.c
@@ -806,8 +806,8 @@ static int pidff_request_effect_upload(struct pidff_device *pidff, int efnum)
static int pidff_needs_playback(struct pidff_device *pidff, int effect_id, int n)
{
- return pidff->effect[effect_id].is_infinite ||
- pidff->effect[effect_id].loop_count != n;
+ return !pidff->effect[effect_id].is_infinite ||
+ pidff->effect[effect_id].loop_count != n;
}
/*
--
2.51.2
^ permalink raw reply related
* Re: [PATCH] Input: imx_sc_key - Fix memory corruption on unload
From: Peng Fan @ 2025-11-04 2:51 UTC (permalink / raw)
To: Dan Carpenter
Cc: Anson Huang, Jiri Kosina, Benjamin Tissoires, Dmitry Torokhov,
Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
linux-input, linux-kernel, imx, linux-arm-kernel, kernel-janitors
In-Reply-To: <aQYKR75r2VMFJutT@stanley.mountain>
On Sat, Nov 01, 2025 at 04:25:27PM +0300, Dan Carpenter wrote:
>This is supposed to be "priv" but we accidentally pass "&priv" which is
>an address in the stack and so it will lead to memory corruption when
>the imx_sc_key_action() function is called. Remove the &.
>
>Fixes: 768062fd1284 ("Input: imx_sc_key - use devm_add_action_or_reset() to handle all cleanups")
>Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
Thanks for the fix.
Reviewed-by: Peng Fan <peng.fan@nxp.com>
^ permalink raw reply
* [PATCH] Input: cros_ec_keyb - Fix an invalid memory access
From: Tzung-Bi Shih @ 2025-11-04 6:43 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Benson Leung, linux-input, chrome-platform, tzungbi
If cros_ec_keyb_register_matrix() isn't called (due to
`buttons_switches_only`) in cros_ec_keyb_probe(), `ckdev->idev` remains
NULL. An invalid memory access is observed in cros_ec_keyb_process()
when receiving an EC_MKBP_EVENT_KEY_MATRIX event in cros_ec_keyb_work()
in such case.
Unable to handle kernel read from unreadable memory at virtual address 0000000000000028
...
x3 : 0000000000000000 x2 : 0000000000000000
x1 : 0000000000000000 x0 : 0000000000000000
Call trace:
input_event
cros_ec_keyb_work
blocking_notifier_call_chain
ec_irq_thread
It's still unknown about why the kernel receives such malformed event,
in any cases, the kernel shouldn't access `ckdev->idev` and friends if
the driver doesn't intend to initialize them.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
drivers/input/keyboard/cros_ec_keyb.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index f7209c8ebbcc..635ee99fe0bd 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -261,6 +261,11 @@ static int cros_ec_keyb_work(struct notifier_block *nb,
case EC_MKBP_EVENT_KEY_MATRIX:
pm_wakeup_event(ckdev->dev, 0);
+ if (!ckdev->idev) {
+ dev_info(ckdev->dev, "No key matrix\n");
+ return NOTIFY_OK;
+ }
+
if (ckdev->ec->event_size != ckdev->cols) {
dev_err(ckdev->dev,
"Discarded incomplete key matrix event.\n");
--
2.51.2.997.g839fc31de9-goog
^ permalink raw reply related
* Re: [PATCH] Input: cros_ec_keyb - Fix an invalid memory access
From: Dmitry Torokhov @ 2025-11-04 6:52 UTC (permalink / raw)
To: Tzung-Bi Shih; +Cc: Benson Leung, linux-input, chrome-platform
In-Reply-To: <20251104064353.3072727-1-tzungbi@kernel.org>
Hi Tzung-Bi,
On Tue, Nov 04, 2025 at 06:43:53AM +0000, Tzung-Bi Shih wrote:
> If cros_ec_keyb_register_matrix() isn't called (due to
> `buttons_switches_only`) in cros_ec_keyb_probe(), `ckdev->idev` remains
> NULL. An invalid memory access is observed in cros_ec_keyb_process()
> when receiving an EC_MKBP_EVENT_KEY_MATRIX event in cros_ec_keyb_work()
> in such case.
>
> Unable to handle kernel read from unreadable memory at virtual address 0000000000000028
> ...
> x3 : 0000000000000000 x2 : 0000000000000000
> x1 : 0000000000000000 x0 : 0000000000000000
> Call trace:
> input_event
> cros_ec_keyb_work
> blocking_notifier_call_chain
> ec_irq_thread
>
> It's still unknown about why the kernel receives such malformed event,
> in any cases, the kernel shouldn't access `ckdev->idev` and friends if
> the driver doesn't intend to initialize them.
>
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> ---
> drivers/input/keyboard/cros_ec_keyb.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
> index f7209c8ebbcc..635ee99fe0bd 100644
> --- a/drivers/input/keyboard/cros_ec_keyb.c
> +++ b/drivers/input/keyboard/cros_ec_keyb.c
> @@ -261,6 +261,11 @@ static int cros_ec_keyb_work(struct notifier_block *nb,
> case EC_MKBP_EVENT_KEY_MATRIX:
> pm_wakeup_event(ckdev->dev, 0);
>
> + if (!ckdev->idev) {
> + dev_info(ckdev->dev, "No key matrix\n");
I think this should be dev_warn_once().
> + return NOTIFY_OK;
> + }
> +
> if (ckdev->ec->event_size != ckdev->cols) {
> dev_err(ckdev->dev,
> "Discarded incomplete key matrix event.\n");
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH v2] Input: cros_ec_keyb - Fix an invalid memory access
From: Tzung-Bi Shih @ 2025-11-04 7:03 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Benson Leung, linux-input, chrome-platform, tzungbi
If cros_ec_keyb_register_matrix() isn't called (due to
`buttons_switches_only`) in cros_ec_keyb_probe(), `ckdev->idev` remains
NULL. An invalid memory access is observed in cros_ec_keyb_process()
when receiving an EC_MKBP_EVENT_KEY_MATRIX event in cros_ec_keyb_work()
in such case.
Unable to handle kernel read from unreadable memory at virtual address 0000000000000028
...
x3 : 0000000000000000 x2 : 0000000000000000
x1 : 0000000000000000 x0 : 0000000000000000
Call trace:
input_event
cros_ec_keyb_work
blocking_notifier_call_chain
ec_irq_thread
It's still unknown about why the kernel receives such malformed event,
in any cases, the kernel shouldn't access `ckdev->idev` and friends if
the driver doesn't intend to initialize them.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
v2:
- dev_info() -> dev_warn_once().
v1: https://lore.kernel.org/chrome-platform/20251104064353.3072727-1-tzungbi@kernel.org
drivers/input/keyboard/cros_ec_keyb.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index f7209c8ebbcc..324c98b1cb52 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -261,6 +261,11 @@ static int cros_ec_keyb_work(struct notifier_block *nb,
case EC_MKBP_EVENT_KEY_MATRIX:
pm_wakeup_event(ckdev->dev, 0);
+ if (!ckdev->idev) {
+ dev_warn_once(ckdev->dev, "No key matrix\n");
+ return NOTIFY_OK;
+ }
+
if (ckdev->ec->event_size != ckdev->cols) {
dev_err(ckdev->dev,
"Discarded incomplete key matrix event.\n");
--
2.51.2.997.g839fc31de9-goog
^ permalink raw reply related
* Re: [PATCH 08/10] arm64: dts: add description for solidrun imx8mp hummingboard-iiot
From: Alexander Dahl @ 2025-11-04 10:24 UTC (permalink / raw)
To: Josua Mayer
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Neil Armstrong, Jessica Zhang, David Airlie, Simona Vetter,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Dmitry Torokhov, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Jon Nettleton, Mikhail Anikin, Yazan Shhady,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
dri-devel@lists.freedesktop.org, linux-input@vger.kernel.org,
imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
linux-leds@vger.kernel.org
In-Reply-To: <054eecb5-1296-4c41-ae86-1779abe0360c@solid-run.com>
Hello Josua,
Am Thu, Oct 30, 2025 at 04:44:40PM +0000 schrieb Josua Mayer:
> Hi Alex,
>
> Am 29.10.25 um 10:23 schrieb Alexander Dahl:
> > Hello Josua,
> >
> > Am Tue, Oct 28, 2025 at 12:24:36PM +0000 schrieb Josua Mayer:
> >> Am 27.10.25 um 18:48 schrieb Josua Mayer:
> >>
> >>> Add description for the SolidRun i.MX8MP HummingBoard IIoT.
> >>> The board is a new design around the i.MX8MP System on Module, not
> >>> sharing much with previous HummingBoards.
> >>>
> >>> It comes with some common features:
> >>> - 3x USB-3.0 Type A connector
> >>> - 2x 1Gbps RJ45 Ethernet
> >>> - USB Type-C Console Port
> >>> - microSD connector
> >>> - RTC with backup battery
> >>> - RGB Status LED
> >>> - 1x M.2 M-Key connector with PCI-E Gen. 3 x1
> >>> - 1x M.2 B-Key connector with USB-2.0/3.0 + SIM card holder
> >>> - 1x LVDS Display Connector
> >>> - 1x DSI Display Connector
> >>> - GPIO header
> >>> - 2x RS232/RS485 ports (configurable)
> >>> - 2x CAN
> >>>
> >>> In addition there is a board-to-board expansion connector to support
> >>> custom daughter boards with access to SPI, a range of GPIOs and -
> >>> notably - CAN and UART. Both 2x CAN and 2x UART can be muxed either
> >>> to this b2b connector, or a termianl block connector on the base board.
> >>>
> >>> The routing choice for UART and CAN is expressed through gpio
> >>> mux-controllers in DT and can be changed by applying dtb addons.
> >>>
> >>> Four dtb addons are provided:
> >>>
> >>> - dsi panel Winstar WJ70N3TYJHMNG0
> >>> - lvds panel Winstar WF70A8SYJHLNGA
> >>> - RS485 on UART port "A" (default rs232)
> >>> - RS485 on UART port "B" (default rs232)
> >>>
> >>> Signed-off-by: Josua Mayer <josua@solid-run.com>
> >>> ---
> >>> arch/arm64/boot/dts/freescale/Makefile | 6 +
> >>> ...hummingboard-iiot-panel-dsi-WJ70N3TYJHMNG0.dtso | 70 ++
> >>> ...ummingboard-iiot-panel-lvds-WF70A8SYJHLNGA.dtso | 105 +++
> >>> .../imx8mp-hummingboard-iiot-rs485-a.dtso | 18 +
> >>> .../imx8mp-hummingboard-iiot-rs485-b.dtso | 18 +
> >>> .../dts/freescale/imx8mp-hummingboard-iiot.dts | 710 +++++++++++++++++++++
> >>> 6 files changed, 927 insertions(+)
> >> cut
> >>> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-hummingboard-iiot.dts b/arch/arm64/boot/dts/freescale/imx8mp-hummingboard-iiot.dts
> >>> new file mode 100644
> >>> index 0000000000000..2e4cb676bc9da
> >>> --- /dev/null
> >>> +++ b/arch/arm64/boot/dts/freescale/imx8mp-hummingboard-iiot.dts
> >> cut
> >>> + led-controller@30 {
> >>> + compatible = "ti,lp5562";
> >>> + reg = <0x30>;
> >>> + /* use internal clock, could use external generated by rtc */
> >>> + clock-mode = /bits/ 8 <1>;
> >>> + #address-cells = <1>;
> >>> + #size-cells = <0>;
> >>> +
> >>> + multi-led@0 {
> >>> + reg = <0x0>;
> >>> + color = <LED_COLOR_ID_RGB>;
> >>> + #address-cells = <1>;
> >>> + #size-cells = <0>;
> >>> +
> >>> + led@0 {
> >>> + reg = <0x0>;
> >>> + color = <LED_COLOR_ID_RED>;
> >>> + led-cur = /bits/ 8 <0x32>;
> >>> + max-cur = /bits/ 8 <0x64>;
> >>> + };
> >>> +
> >>> + led@1 {
> >>> + reg = <0x1>;
> >>> + color = <LED_COLOR_ID_GREEN>;
> >>> + led-cur = /bits/ 8 <0x19>;
> >>> + max-cur = /bits/ 8 <0x32>;
> >>> + };
> >>> +
> >>> + led@2 {
> >>> + reg = <0x2>;
> >>> + color = <LED_COLOR_ID_BLUE>;
> >>> + led-cur = /bits/ 8 <0x19>;
> >>> + max-cur = /bits/ 8 <0x32>;
> >>> + };
> >>> + };
> >>> +
> >>> + led@3 {
> >>> + reg = <3>;
> >>> + chan-name = "D8";
> >> chan-name gives the led the name D6 in sysfs.
> >>
> >> The bindings do not allow however setting chan-name on
> >> the multi-led, and it has an auto-generated name in sysfs.
> >>
> >> Am I missing something? Can multi-leds have a custom name?
> > The sysfs names are auto-generated based on the attributes "color",
> > "function", and "label" with the last being discouraged for new
> > designs.
> Thank you for reminding me of this one!
> > If the "ti,lp5562" driver does nothing special,you could
> > add "function" to the multi-led node and see if that fits your needs.
> The board is not a complete product by itself so we have not chosen
> a specific function for each led.
> Therefore only color and label are actually applicable.
In such cases I use a generic function and the enumerator,
something like this (in this case for a different led driver):
led_11_red: led-0 {
function = LED_FUNCTION_INDICATOR;
function-enumerator = <0>;
color = <LED_COLOR_ID_RED>;
gpios = <&pioC 18 GPIO_ACTIVE_HIGH>;
};
…
led_13_red: led-2 {
function = LED_FUNCTION_INDICATOR;
function-enumerator = <1>;
color = <LED_COLOR_ID_RED>;
gpios = <&pioC 20 GPIO_ACTIVE_HIGH>;
};
This gives stable sysfs paths like this:
/sys/class/leds/red:indicator-0
/sys/class/leds/red:indicator-1
Of course only with a LED driver supporting that automatic naming
scheme.
> After testing on v6.18-rc1 I can state that leds-lp5562 driver does
> something special - function and label properties do not have any
> impact on the names in sysfs.
>
> However I could set label on both LEDs regardless?
When using the 'label' attribute, color and function attributes are
more or less informational only, because label determines the sysfs
path. Someone could change the leds-lp5562 driver to behave like the
other drivers, but then I would advise to explicitly set label in this
case, so you won't end up with changed sysfs paths after the driver is
changed.
Not sure if this helps you. Sorry.
Greets
Alex
>
> >
> > Adding linux-leds to Cc, because this is a LED related question.
> >
> > Greets
> > Alex
> >
> >> In v6.6 leds-lp5562 driver if I set in each multi-led led@[0-2] sub-node
> >> chan-name to the same string "D7" - then the sysfs name becomes D7.
> >>
> >>> + color = <LED_COLOR_ID_GREEN>;
> >>> + led-cur = /bits/ 8 <0x19>;
> >>> + max-cur = /bits/ 8 <0x64>;
> >>> + };
> >>> + };
^ permalink raw reply
* ioctl handler of the hidraw driver should return ENOIOCTLCMD for numbers it does not want to handle | tcgetattr() ⇔ ioctl(, TCGETS2, …) sets errno to undocumented EINVAL/22
From: Дилян Палаузов @ 2025-11-04 12:48 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires; +Cc: linux-input
Hello,
With kernel 6.17.6 on x86_64 and glibc 2.42 the below program substitutes tcgetattr() with ioctl(…, TCGETS2, …) and sets errno to 22 / Invalid argument / EINVAL. The only difference for kernel 6.6.60 on armv7l 2.26 is that the substitution has no 2, tcgetattr() translates to ioctl(…, TCGETS, …).
For tcgetattr() only errno EBADF and ENOTTY are documented at https://sourceware.org/glibc/manual/latest/html_mono/libc.html#index-tcgetattr , at https://man7.org/linux/man-pages/man3/tcgetattr.3p.html and at https://pubs.opengroup.org/onlinepubs/9799919799/functions/tcgetattr.html (Open Group Base Specifications Issue 8/year 2024).
In this concrete case changes to the man7.org documentation must be triggered by the Austin Group - https://lore.kernel.org/linux-man/dsb6oiv7q7ra3gbu4bovy3gah522lgsf3d6h3wxwe4ieuka6fh@g6u5qzlyoxf5/ and but https://sourceware.org/bugzilla/show_bug.cgi?id=33597 suggests this is a kernel bug.
For hidraw devices the possible ioctl commands are mentioned at https://docs.kernel.org/hid/hidraw.html#ioctl, TCGETS and TCGETS2 are not among them, so I was expecting a ENOTTY error. This problem might also happen for to tcsetattr() - I have not checked it.
#include <errno.h>
#include <fcntl.h>
#include <stdio.h>
#include <termios.h>
void main() {
int fd = open("/dev/hidraw0", O_RDWR | O_NONBLOCK |O_NOCTTY);
struct termios s;
errno = 0;
int ret = tcgetattr(fd, &s);
printf("Returned fd is %i ret is %i errno is %i %m\n", fd, ret, errno);
}
The above produces:
Returned fd is 3 ret is -1 errno is 22 Invalid argument
Kind regards // Дилян
^ permalink raw reply
* Re: [PATCH] Input: imx_sc_key - Fix memory corruption on unload
From: Dmitry Torokhov @ 2025-11-04 18:58 UTC (permalink / raw)
To: Dan Carpenter
Cc: Anson Huang, Jiri Kosina, Benjamin Tissoires, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, linux-input,
linux-kernel, imx, linux-arm-kernel, kernel-janitors
In-Reply-To: <aQYKR75r2VMFJutT@stanley.mountain>
On Sat, Nov 01, 2025 at 04:25:27PM +0300, Dan Carpenter wrote:
> This is supposed to be "priv" but we accidentally pass "&priv" which is
> an address in the stack and so it will lead to memory corruption when
> the imx_sc_key_action() function is called. Remove the &.
>
> Fixes: 768062fd1284 ("Input: imx_sc_key - use devm_add_action_or_reset() to handle all cleanups")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] input: elan_i2c: Add newline to sysfs attribute outputs
From: Dmitry Torokhov @ 2025-11-05 4:55 UTC (permalink / raw)
To: Zhongqiu Han; +Cc: krzk, linux-input, linux-kernel
In-Reply-To: <20251030123458.1760524-1-zhongqiu.han@oss.qualcomm.com>
Hi Zhongqiu,
On Thu, Oct 30, 2025 at 08:34:58PM +0800, Zhongqiu Han wrote:
> Append newline characters to sysfs_emit() outputs in func min_show() and
> max_show(). This aligns with common kernel conventions and improves
> readability for userspace tools that expect newline-terminated values.
This format for this driver was in use for 11 years, and there might be
tools that do not expect newlines. I would rather leave it as is.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: xpad - Change buttons the D-Pad gets mapped as to BTN_DPAD_*
From: Dmitry Torokhov @ 2025-11-05 5:04 UTC (permalink / raw)
To: Cameron Gutman; +Cc: Vicki Pfau, linux-input
In-Reply-To: <CAAfxzZ1xJC2QRms4RMS54JCbieupt7XmxO6bwZc0vnx0thT4Yg@mail.gmail.com>
On Sat, Nov 01, 2025 at 11:23:51AM -0500, Cameron Gutman wrote:
> On Sat, Nov 1, 2025 at 12:52 AM Vicki Pfau <vi@endrift.com> wrote:
> >
> >
> >
> > On 10/31/25 8:18 PM, Cameron Gutman wrote:
> > > On Sun, Jul 27, 2025 at 3:23 AM Dmitry Torokhov
> > > <dmitry.torokhov@gmail.com> wrote:
> > >>
> > >> On Tue, Jul 01, 2025 at 08:47:40PM -0700, Vicki Pfau wrote:
> > >>> Since dance pads can have both up/down or left/right pressed at the same time,
> > >>> by design, they are not suitable for mapping the buttons to axes. Historically,
> > >>> this driver mapped the D-pad to BTN_TRIGGER_HAPPY1-4 in these cases, and before
> > >>> that as mouse buttons. However, BTN_DPAD_* exists for this and makes far more
> > >>> sense than the arbitrary mapping it was before.
> > >>>
> > >>> Signed-off-by: Vicki Pfau <vi@endrift.com>
> > >>
> > >> This unfortunately changes existing mappings, but I guess new events are
> > >> better than old ones...
> > >>
> > >> Applied, thank you.
> > >
> > > Unfortunately this mapping change caused major userspace breakages.
> >
> > This is unfortunate, however,>
> > > I think it needs to be reverted.
> > >
> > > [0]: https://blog.dwickham.me.uk/why-your-xbox-360-wireless-controllers-d-pad-is-now-wrong-and-how-to-fix-it
> > > [1]: https://github.com/libsdl-org/SDL/issues/14324
> > > [2]: https://github.com/libsdl-org/SDL/pull/14339
> >
> > None of these pages suggest reverting the changes. The blog post suggests that these mappings are correct, and that the libraries need to be fixed. Reverting it is mentioned as an absolute last case scenario. The SDL issues say they need more testing, not an outright reversion.
>
> Breaking userspace is the #1 thing we cannot do as Linux kernel developers.
>
> The fact that it broke enough people to get a blog post is reason
> enough to revert.
>
> >
> > Reverting this is solely reverting to an old, uncomfortable status quo instead of actually fixing the things that broke. If this gets reverted, presumably more stuff will ship this broken code that will break if we ever fix this issue again, instead of things getting progressively fixed.
> >
>
> The "broken" 360 wireless mapping has been used since wireless adapter
> support was first implemented in 2008 [0].
>
> There's decades of software out there that already has workarounds for
> these devices that we must not break.
>
> We can send the proper button codes for new controllers, but I think
> we have to leave existing devices alone.
>
> [0]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=99de0912be6f384fc31c8e8e7ba0850d0d670385
>
> > The abandonware being broken is unfortunate, but things like Steam Input exist specifically to work around issues like that.
> >
>
> This isn't just breaking abandonware. It's breaking the *latest*
> releases of SDL2 and SDL3.
I agree that keeping the old mapping is not great idea, especially if we
start having newer devices using new mapping while other devices use the
old one. However we can't really be breaking existing user setup and now
that we are aware of such breakages we have to react. I am open to
suggestions.
From my side I see a few options:
1. Introduce a module option, and allow controlling the behavior. Hope
distributions will flip it when updating to new version of SDL that can
hopefully handle this. Eventually flip the default to be the new
mapping.
2. Allow overriding mapping from userspace. Allow [main] user (SDL or
not) load an updated keymap if they desire to do so.
3. Maybe we can send both old and new events at the same time and let
consumers decide which ones to act upon? Eventually we can drop the old
mapping (maybe again having an option to use old one or implement option
2 and allow loading old mapping via udev or a utility).
4. ???
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2] Documentation: input: expand INPUT_PROP_HAPTIC_TOUCHPAD to all pressure pads
From: Dmitry Torokhov @ 2025-11-05 5:09 UTC (permalink / raw)
To: Peter Hutterer
Cc: Jonathan Denose, Jiri Kosina, Benjamin Tissoires, Jonathan Corbet,
Henrik Rydberg, linux-input, linux-kernel, linux-doc,
Angela Czubak, Sean O'Brien, Randy Dunlap
In-Reply-To: <20251031041245.GA1316325@quokka>
Hi Peter,
On Fri, Oct 31, 2025 at 02:12:45PM +1000, Peter Hutterer wrote:
> Definition: "pressure pad" used here as all touchpads that use physical
> pressure to convert to click without physical hinges. Also called haptic
> touchpads in general parlance, Synaptics calls them ForcePads.
>
> Most (all?) pressure pads are currently advertised as
> INPUT_PROP_BUTTONPAD. The suggestion to identify them as pressure pads
> by defining the resolution on ABS_MT_PRESSURE has been in the docs since
> commit 20ccc8dd38a3 ("Documentation: input: define
> ABS_PRESSURE/ABS_MT_PRESSURE resolution as grams") but few devices
> provide this information.
>
> In userspace it's thus impossible to determine whether a device is a
> true pressure pad (pressure equals pressure) or a normal clickpad with
> (pressure equals finger size).
>
> Commit 7075ae4ac9db ("Input: add INPUT_PROP_HAPTIC_TOUCHPAD") introduces
> INPUT_PROP_HAPTIC_TOUCHPAD but restricted it to those touchpads that
> have support for userspace-controlled effects. Let's expand that
> definition to include all haptic touchpads (pressure pads) since those
> that do support FF effects can be identified by the presence of the
> FF_HAPTIC bit.
>
> This means:
> - clickpad: INPUT_PROP_BUTTONPAD
> - pressurepad: INPUT_PROP_BUTTONPAD + INPUT_PROP_HAPTIC_TOUCHPAD
> - pressurepad with haptics:
> INPUT_PROP_BUTTONPAD + INPUT_PROP_HAPTIC_TOUCHPAD + FF_HAPTIC
Should we maybe rename it to INPUT_PROP_PRESSURE_TOUCHPAD? We are within
a release of introducing it, so we should be able to rename it without
much fallout.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2] Documentation: input: expand INPUT_PROP_HAPTIC_TOUCHPAD to all pressure pads
From: Peter Hutterer @ 2025-11-05 6:03 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Jonathan Denose, Jiri Kosina, Benjamin Tissoires, Jonathan Corbet,
Henrik Rydberg, linux-input, linux-kernel, linux-doc,
Angela Czubak, Sean O'Brien, Randy Dunlap
In-Reply-To: <ueksimssfw7eqxa5mfmxruj2ghtuncznhbgdxvoorjf6jhjfhk@hsuapbn3kie3>
On Tue, Nov 04, 2025 at 09:09:09PM -0800, Dmitry Torokhov wrote:
> Hi Peter,
>
> On Fri, Oct 31, 2025 at 02:12:45PM +1000, Peter Hutterer wrote:
> > Definition: "pressure pad" used here as all touchpads that use physical
> > pressure to convert to click without physical hinges. Also called haptic
> > touchpads in general parlance, Synaptics calls them ForcePads.
> >
> > Most (all?) pressure pads are currently advertised as
> > INPUT_PROP_BUTTONPAD. The suggestion to identify them as pressure pads
> > by defining the resolution on ABS_MT_PRESSURE has been in the docs since
> > commit 20ccc8dd38a3 ("Documentation: input: define
> > ABS_PRESSURE/ABS_MT_PRESSURE resolution as grams") but few devices
> > provide this information.
> >
> > In userspace it's thus impossible to determine whether a device is a
> > true pressure pad (pressure equals pressure) or a normal clickpad with
> > (pressure equals finger size).
> >
> > Commit 7075ae4ac9db ("Input: add INPUT_PROP_HAPTIC_TOUCHPAD") introduces
> > INPUT_PROP_HAPTIC_TOUCHPAD but restricted it to those touchpads that
> > have support for userspace-controlled effects. Let's expand that
> > definition to include all haptic touchpads (pressure pads) since those
> > that do support FF effects can be identified by the presence of the
> > FF_HAPTIC bit.
> >
> > This means:
> > - clickpad: INPUT_PROP_BUTTONPAD
> > - pressurepad: INPUT_PROP_BUTTONPAD + INPUT_PROP_HAPTIC_TOUCHPAD
> > - pressurepad with haptics:
> > INPUT_PROP_BUTTONPAD + INPUT_PROP_HAPTIC_TOUCHPAD + FF_HAPTIC
>
> Should we maybe rename it to INPUT_PROP_PRESSURE_TOUCHPAD? We are within
> a release of introducing it, so we should be able to rename it without
> much fallout.
I'd be happy with a rename too. Want me to send a patch for that or do
you want to rename it locally during applying?
Cheers,
Peter
^ permalink raw reply
* Re: [PATCH] input: elan_i2c: Add newline to sysfs attribute outputs
From: Zhongqiu Han @ 2025-11-05 6:28 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: krzk, linux-input, linux-kernel, zhongqiu.han
In-Reply-To: <swn2pwumb7ffpcyer6n5nn6zbdjvxtwxprddymhxzdjzx4lbr5@hkwusbsr5uks>
On 11/5/2025 12:55 PM, Dmitry Torokhov wrote:
> Hi Zhongqiu,
>
> On Thu, Oct 30, 2025 at 08:34:58PM +0800, Zhongqiu Han wrote:
>> Append newline characters to sysfs_emit() outputs in func min_show() and
>> max_show(). This aligns with common kernel conventions and improves
>> readability for userspace tools that expect newline-terminated values.
>
> This format for this driver was in use for 11 years, and there might be
> tools that do not expect newlines. I would rather leave it as is.
>
> Thanks.
>
Hi Dmitry,
Thanks for your review~
Sure, I agree that we should avoid breaking unknown userspace tools.
--
Thx and BRs,
Zhongqiu Han
^ permalink raw reply
* Re: [PATCH] HID: playstation: Remap joystick axes to be centered at 0
From: Siarhei Vishniakou @ 2025-11-05 19:41 UTC (permalink / raw)
To: Jiri Kosina
Cc: Benjamin Tissoires, Roderick Colenbrander, linux-input,
linux-kernel
In-Reply-To: <no3r172s-899p-7s78-o199-8q8oq1r2nr71@xreary.bet>
пт, 31 окт. 2025 г. в 02:00, Jiri Kosina <jikos@kernel.org>:
>
> On Fri, 10 Oct 2025, Siarhei Vishniakou wrote:
>
> > The joystick axes (ABS_X, ABS_Y, ABS_RX, ABS_RY) for PlayStation
> > gamepads report a neutral state of 128 over HID, with a full range of
> > [0, 255]. The driver previously mapped this directly, resulting in an
> > evdev range of [0, 255] with a resting point of 128.
> >
> > This approach is unconventional for Linux gamepad drivers and has several
> > drawbacks: it requires userspace applications to be aware of the
> > non-zero resting state, and it is incompatible with the input
> > subsystem's 'flat' (deadzone) logic, which assumes a resting point of 0.
> >
> > This patch remaps the four joystick axes to the conventional signed
> > 8-bit range of [-128, 127], with 0 as the neutral state. This is
> > accomplished by changing their evdev range in ps_gamepad_create() and
> > translating the incoming hardware value in the report parsing functions
> > by subtracting 128.
> >
> > The analog trigger axes (ABS_Z, ABS_RZ) are handled separately. Their
> > resting state is 0 (un-pressed), and their hardware range of [0, 255]
> > is already the conventional representation. They are left unmodified by
> > this patch.
> >
> > This makes the joystick behavior consistent with other gamepad drivers
> > while preserving the standard behavior for the triggers.
> >
> > Signed-off-by: Siarhei Vishniakou <svv@google.com>
>
> Hmmm ... as I am rather unaware of the existing playstation userspace
> ecosystem -- is there any way how we could now be breaking existing
> playstation-specific userspace assumptions that have already developed
> based on asumptions that we're centered at 128?
>
> Thanks,
>
> --
> Jiri Kosina
> SUSE Labs
>
Thanks Jiri for looking! I chatted with Roderick, and he brought up
similar concerns. Generally, most of the joysticks have the "normal"
ranges with the 0 value at the center. However, I understand that some
user-space apps might have hardcoded some of these Sony-specific
assumptions.
Instead, I will change this patch to provide correct initial
(centered) values of 0x80 when the driver first starts. This is needed
in order to fix our Sony gamepad tests on Android (and will help
everyone else, because now there won't be a stray "bring to center"
event emitted from the gamepad in the beginning). I will follow up
later this week.
^ permalink raw reply
* [PATCH v3 0/3] mfd: twl603x: add power button
From: akemnade @ 2025-11-05 19:52 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Andreas Kemnade, Dmitry Torokhov, Tony Lindgren, Kevin Hilman
Cc: devicetree, linux-kernel, linux-input, linux-omap,
Andreas Kemnade
Like the TWL4030, the TWL603x also has a power button feature,
so extend the TWL4030 power button driver to support TWL603x.
Signed-off-by: Andreas Kemnade <akemnade@kernel.org>
---
Changes in v3:
- static definition of chipdata
- stricter rules in common part of binding
- Link to v2: https://lore.kernel.org/r/20251030-twl6030-button-v2-0-09653d05a2b1@kernel.org
Changes in v2:
- integrate driver into existing TWL4030 driver
- improve commit messages
- Link to v1: https://lore.kernel.org/r/20251020-twl6030-button-v1-0-93e4644ac974@kernel.org
---
Andreas Kemnade (3):
dt-bindings: mfd: twl: enable power button also for twl603x
Input: twl4030 - add TWL603x power button
ARM: dts: ti/omap: omap4-epson-embt2ws: add powerbutton
Documentation/devicetree/bindings/mfd/ti,twl.yaml | 40 ++++++++++++---
arch/arm/boot/dts/ti/omap/omap4-epson-embt2ws.dts | 5 ++
drivers/input/misc/twl4030-pwrbutton.c | 61 +++++++++++++++++++++--
3 files changed, 96 insertions(+), 10 deletions(-)
---
base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
change-id: 20251020-twl6030-button-83d759b060e6
Best regards,
--
Andreas Kemnade <akemnade@kernel.org>
^ permalink raw reply
* [PATCH v3 1/3] dt-bindings: mfd: twl: enable power button also for twl603x
From: akemnade @ 2025-11-05 19:52 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Andreas Kemnade, Dmitry Torokhov, Tony Lindgren, Kevin Hilman
Cc: devicetree, linux-kernel, linux-input, linux-omap,
Andreas Kemnade
In-Reply-To: <20251105-twl6030-button-v3-0-9b37eb2b0989@kernel.org>
From: Andreas Kemnade <andreas@kemnade.info>
TWL603x has also a power button function, so add the corresponding subnode.
As not in all cases there is a power button connected to the corresponding
pad of the TWL603x, the functionality can be disabled by
status = "disabled" or simply not adding the subnode.
To keep things simple, follow the established design pattern of using const
interrupts as used also by the other subdevices.
Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
Documentation/devicetree/bindings/mfd/ti,twl.yaml | 40 +++++++++++++++++++----
1 file changed, 34 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/mfd/ti,twl.yaml b/Documentation/devicetree/bindings/mfd/ti,twl.yaml
index 776b04e182cb..045fd07f476e 100644
--- a/Documentation/devicetree/bindings/mfd/ti,twl.yaml
+++ b/Documentation/devicetree/bindings/mfd/ti,twl.yaml
@@ -55,6 +55,15 @@ allOf:
gpadc: false
+ pwrbutton:
+ properties:
+ compatible:
+ const: ti,twl4030-pwrbutton
+ interrupts:
+ items:
+ - items:
+ const: 8
+
usb-comparator: false
- if:
@@ -95,7 +104,14 @@ allOf:
compatible:
const: ti,twl6030-gpadc
- pwrbutton: false
+ pwrbutton:
+ properties:
+ compatible:
+ const: ti,twl6030-pwrbutton
+ interrupts:
+ items:
+ - items:
+ const: 0
madc: false
@@ -146,7 +162,14 @@ allOf:
compatible:
const: ti,twl6032-gpadc
- pwrbutton: false
+ pwrbutton:
+ properties:
+ compatible:
+ const: ti,twl6030-pwrbutton
+ interrupts:
+ items:
+ - items:
+ const: 0
madc: false
@@ -226,11 +249,11 @@ properties:
properties:
compatible:
- const: ti,twl4030-pwrbutton
+ enum:
+ - ti,twl4030-pwrbutton
+ - ti,twl6030-pwrbutton
interrupts:
- items:
- - items:
- const: 8
+ maxItems: 1
watchdog:
type: object
@@ -459,6 +482,11 @@ examples:
#io-channel-cells = <1>;
};
+ pwrbutton {
+ compatible = "ti,twl6030-pwrbutton";
+ interrupts = <0>;
+ };
+
rtc {
compatible = "ti,twl4030-rtc";
interrupts = <8>;
--
2.47.3
^ permalink raw reply related
* [PATCH v3 2/3] Input: twl4030 - add TWL603x power button
From: akemnade @ 2025-11-05 19:52 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Andreas Kemnade, Dmitry Torokhov, Tony Lindgren, Kevin Hilman
Cc: devicetree, linux-kernel, linux-input, linux-omap,
Andreas Kemnade
In-Reply-To: <20251105-twl6030-button-v3-0-9b37eb2b0989@kernel.org>
From: Andreas Kemnade <andreas@kemnade.info>
Like the TWL4030, these PMICs also have a power button feature, so extend
the TWL4030 power button driver. As the irqchip of the TWL6030 mfd driver
does not provide mask, unmask finctions, do it manually.
Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
drivers/input/misc/twl4030-pwrbutton.c | 61 +++++++++++++++++++++++++++++++---
1 file changed, 57 insertions(+), 4 deletions(-)
diff --git a/drivers/input/misc/twl4030-pwrbutton.c b/drivers/input/misc/twl4030-pwrbutton.c
index f85cc289c053..b72fba9a1b2c 100644
--- a/drivers/input/misc/twl4030-pwrbutton.c
+++ b/drivers/input/misc/twl4030-pwrbutton.c
@@ -25,22 +25,40 @@
#include <linux/kernel.h>
#include <linux/errno.h>
#include <linux/input.h>
+#include <linux/bits.h>
#include <linux/interrupt.h>
#include <linux/of.h>
#include <linux/platform_device.h>
#include <linux/mfd/twl.h>
-#define PWR_PWRON_IRQ (1 << 0)
+#define PWR_PWRON_IRQ BIT(0)
-#define STS_HW_CONDITIONS 0xf
+#define STS_HW_CONDITIONS_4030 0xf
+#define STS_HW_CONDITIONS_6030 0x2
+
+struct twl_pwrbutton_chipdata {
+ u8 status_reg;
+ bool need_manual_irq;
+};
+
+static const struct twl_pwrbutton_chipdata twl4030_chipdata = {
+ STS_HW_CONDITIONS_4030,
+ false,
+};
+
+static const struct twl_pwrbutton_chipdata twl6030_chipdata = {
+ STS_HW_CONDITIONS_6030,
+ true,
+};
static irqreturn_t powerbutton_irq(int irq, void *_pwr)
{
struct input_dev *pwr = _pwr;
+ const struct twl_pwrbutton_chipdata *pdata = dev_get_drvdata(pwr->dev.parent);
int err;
u8 value;
- err = twl_i2c_read_u8(TWL_MODULE_PM_MASTER, &value, STS_HW_CONDITIONS);
+ err = twl_i2c_read_u8(TWL_MODULE_PM_MASTER, &value, pdata->status_reg);
if (!err) {
pm_wakeup_event(pwr->dev.parent, 0);
input_report_key(pwr, KEY_POWER, value & PWR_PWRON_IRQ);
@@ -55,10 +73,17 @@ static irqreturn_t powerbutton_irq(int irq, void *_pwr)
static int twl4030_pwrbutton_probe(struct platform_device *pdev)
{
+ const struct twl_pwrbutton_chipdata *pdata;
struct input_dev *pwr;
int irq = platform_get_irq(pdev, 0);
int err;
+ pdata = device_get_match_data(&pdev->dev);
+ if (!pdata)
+ return -EINVAL;
+
+ platform_set_drvdata(pdev, (void *)pdata);
+
pwr = devm_input_allocate_device(&pdev->dev);
if (!pwr) {
dev_err(&pdev->dev, "Can't allocate power button\n");
@@ -85,14 +110,41 @@ static int twl4030_pwrbutton_probe(struct platform_device *pdev)
return err;
}
+ if (pdata->need_manual_irq) {
+ err = twl6030_interrupt_unmask(0x01, REG_INT_MSK_LINE_A);
+ if (err < 0)
+ return err;
+
+ err = twl6030_interrupt_unmask(0x01, REG_INT_MSK_STS_A);
+ if (err < 0)
+ return err;
+ }
+
device_init_wakeup(&pdev->dev, true);
return 0;
}
+static void twl4030_pwrbutton_remove(struct platform_device *pdev)
+{
+ const struct twl_pwrbutton_chipdata *pdata = platform_get_drvdata(pdev);
+
+ if (pdata->need_manual_irq) {
+ twl6030_interrupt_mask(0x01, REG_INT_MSK_LINE_A);
+ twl6030_interrupt_mask(0x01, REG_INT_MSK_STS_A);
+ }
+}
+
#ifdef CONFIG_OF
static const struct of_device_id twl4030_pwrbutton_dt_match_table[] = {
- { .compatible = "ti,twl4030-pwrbutton" },
+ {
+ .compatible = "ti,twl4030-pwrbutton",
+ .data = &twl4030_chipdata,
+ },
+ {
+ .compatible = "ti,twl6030-pwrbutton",
+ .data = &twl6030_chipdata,
+ },
{},
};
MODULE_DEVICE_TABLE(of, twl4030_pwrbutton_dt_match_table);
@@ -100,6 +152,7 @@ MODULE_DEVICE_TABLE(of, twl4030_pwrbutton_dt_match_table);
static struct platform_driver twl4030_pwrbutton_driver = {
.probe = twl4030_pwrbutton_probe,
+ .remove = twl4030_pwrbutton_remove,
.driver = {
.name = "twl4030_pwrbutton",
.of_match_table = of_match_ptr(twl4030_pwrbutton_dt_match_table),
--
2.47.3
^ permalink raw reply related
* [PATCH v3 3/3] ARM: dts: ti/omap: omap4-epson-embt2ws: add powerbutton
From: akemnade @ 2025-11-05 19:52 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Andreas Kemnade, Dmitry Torokhov, Tony Lindgren, Kevin Hilman
Cc: devicetree, linux-kernel, linux-input, linux-omap,
Andreas Kemnade
In-Reply-To: <20251105-twl6030-button-v3-0-9b37eb2b0989@kernel.org>
From: Andreas Kemnade <andreas@kemnade.info>
There is a power button connected to the PMIC, so describe it to be able
to power off the device in a convenient manner.
Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
arch/arm/boot/dts/ti/omap/omap4-epson-embt2ws.dts | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/arm/boot/dts/ti/omap/omap4-epson-embt2ws.dts b/arch/arm/boot/dts/ti/omap/omap4-epson-embt2ws.dts
index c90f43cc2fae..673df1b693f2 100644
--- a/arch/arm/boot/dts/ti/omap/omap4-epson-embt2ws.dts
+++ b/arch/arm/boot/dts/ti/omap/omap4-epson-embt2ws.dts
@@ -229,6 +229,11 @@ rtc {
interrupts = <11>;
};
+ pwrbutton {
+ compatible = "ti,twl6030-pwrbutton";
+ interrupts = <0>;
+ };
+
ldo2: regulator-ldo2 {
compatible = "ti,twl6032-ldo2";
regulator-min-microvolt = <1000000>;
--
2.47.3
^ permalink raw reply related
* Re: [PATCH v2] Input: cros_ec_keyb - Fix an invalid memory access
From: Dmitry Torokhov @ 2025-11-05 21:49 UTC (permalink / raw)
To: Tzung-Bi Shih; +Cc: Benson Leung, linux-input, chrome-platform
In-Reply-To: <20251104070310.3212712-1-tzungbi@kernel.org>
Hi Tzung-Bi,
On Tue, Nov 04, 2025 at 07:03:10AM +0000, Tzung-Bi Shih wrote:
> If cros_ec_keyb_register_matrix() isn't called (due to
> `buttons_switches_only`) in cros_ec_keyb_probe(), `ckdev->idev` remains
> NULL. An invalid memory access is observed in cros_ec_keyb_process()
> when receiving an EC_MKBP_EVENT_KEY_MATRIX event in cros_ec_keyb_work()
> in such case.
>
> Unable to handle kernel read from unreadable memory at virtual address 0000000000000028
> ...
> x3 : 0000000000000000 x2 : 0000000000000000
> x1 : 0000000000000000 x0 : 0000000000000000
> Call trace:
> input_event
> cros_ec_keyb_work
> blocking_notifier_call_chain
> ec_irq_thread
>
> It's still unknown about why the kernel receives such malformed event,
> in any cases, the kernel shouldn't access `ckdev->idev` and friends if
> the driver doesn't intend to initialize them.
>
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> ---
> v2:
> - dev_info() -> dev_warn_once().
>
> v1: https://lore.kernel.org/chrome-platform/20251104064353.3072727-1-tzungbi@kernel.org
>
> drivers/input/keyboard/cros_ec_keyb.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
> index f7209c8ebbcc..324c98b1cb52 100644
> --- a/drivers/input/keyboard/cros_ec_keyb.c
> +++ b/drivers/input/keyboard/cros_ec_keyb.c
> @@ -261,6 +261,11 @@ static int cros_ec_keyb_work(struct notifier_block *nb,
> case EC_MKBP_EVENT_KEY_MATRIX:
> pm_wakeup_event(ckdev->dev, 0);
>
> + if (!ckdev->idev) {
> + dev_warn_once(ckdev->dev, "No key matrix\n");
Adjusted the message to "Unexpected key matrix event" and applied, thank
you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2] Documentation: input: expand INPUT_PROP_HAPTIC_TOUCHPAD to all pressure pads
From: Dmitry Torokhov @ 2025-11-05 21:50 UTC (permalink / raw)
To: Peter Hutterer
Cc: Jonathan Denose, Jiri Kosina, Benjamin Tissoires, Jonathan Corbet,
Henrik Rydberg, linux-input, linux-kernel, linux-doc,
Angela Czubak, Sean O'Brien, Randy Dunlap
In-Reply-To: <20251105060333.GA2615904@quokka>
On Wed, Nov 05, 2025 at 04:03:33PM +1000, Peter Hutterer wrote:
> On Tue, Nov 04, 2025 at 09:09:09PM -0800, Dmitry Torokhov wrote:
> > Hi Peter,
> >
> > On Fri, Oct 31, 2025 at 02:12:45PM +1000, Peter Hutterer wrote:
> > > Definition: "pressure pad" used here as all touchpads that use physical
> > > pressure to convert to click without physical hinges. Also called haptic
> > > touchpads in general parlance, Synaptics calls them ForcePads.
> > >
> > > Most (all?) pressure pads are currently advertised as
> > > INPUT_PROP_BUTTONPAD. The suggestion to identify them as pressure pads
> > > by defining the resolution on ABS_MT_PRESSURE has been in the docs since
> > > commit 20ccc8dd38a3 ("Documentation: input: define
> > > ABS_PRESSURE/ABS_MT_PRESSURE resolution as grams") but few devices
> > > provide this information.
> > >
> > > In userspace it's thus impossible to determine whether a device is a
> > > true pressure pad (pressure equals pressure) or a normal clickpad with
> > > (pressure equals finger size).
> > >
> > > Commit 7075ae4ac9db ("Input: add INPUT_PROP_HAPTIC_TOUCHPAD") introduces
> > > INPUT_PROP_HAPTIC_TOUCHPAD but restricted it to those touchpads that
> > > have support for userspace-controlled effects. Let's expand that
> > > definition to include all haptic touchpads (pressure pads) since those
> > > that do support FF effects can be identified by the presence of the
> > > FF_HAPTIC bit.
> > >
> > > This means:
> > > - clickpad: INPUT_PROP_BUTTONPAD
> > > - pressurepad: INPUT_PROP_BUTTONPAD + INPUT_PROP_HAPTIC_TOUCHPAD
> > > - pressurepad with haptics:
> > > INPUT_PROP_BUTTONPAD + INPUT_PROP_HAPTIC_TOUCHPAD + FF_HAPTIC
> >
> > Should we maybe rename it to INPUT_PROP_PRESSURE_TOUCHPAD? We are within
> > a release of introducing it, so we should be able to rename it without
> > much fallout.
>
> I'd be happy with a rename too. Want me to send a patch for that or do
> you want to rename it locally during applying?
Please send the updated patch for input.h along with updated
documentation.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 1/2] Input: Create input notifier chain in input.c
From: Dmitry Torokhov @ 2025-11-05 21:55 UTC (permalink / raw)
To: Jonathan Denose
Cc: Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel
In-Reply-To: <20251030-lid-switch-notifier-v1-1-c58dc9b1439d@google.com>
Hi Jonathan,
On Thu, Oct 30, 2025 at 02:10:40PM +0000, Jonathan Denose wrote:
> To expose input events to other kernel modules, add a blocking notifier
> chain. Publish LID_SWITCH_OPEN/LID_SWITCH_CLOSE events through this
> notifier chain when input_handle_event detects events signaling the lid
> switch has opened or closed.
>
> Additionally, export a function which allows other kernel modules to
> register notifier_block structs against this notifier chain.
>
> Signed-off-by: Jonathan Denose <jdenose@google.com>
> ---
> drivers/input/input.c | 13 +++++++++++++
> include/linux/input.h | 7 +++++++
> 2 files changed, 20 insertions(+)
>
> diff --git a/drivers/input/input.c b/drivers/input/input.c
> index a500e1e276c211d1146dbfea421a3402084007f8..b342b1ff138ccc58d4623edcf1152bd85d7054bf 100644
> --- a/drivers/input/input.c
> +++ b/drivers/input/input.c
> @@ -26,6 +26,7 @@
> #include <linux/kstrtox.h>
> #include <linux/mutex.h>
> #include <linux/rcupdate.h>
> +#include <linux/notifier.h>
> #include "input-compat.h"
> #include "input-core-private.h"
> #include "input-poller.h"
> @@ -62,6 +63,8 @@ static const unsigned int input_max_code[EV_CNT] = {
> [EV_FF] = FF_MAX,
> };
>
> +static struct blocking_notifier_head input_notifier_head;
> +
> static inline int is_event_supported(unsigned int code,
> unsigned long *bm, unsigned int max)
> {
> @@ -367,10 +370,20 @@ void input_handle_event(struct input_dev *dev,
> if (type != EV_SYN)
> add_input_randomness(type, code, value);
>
> + if (type == EV_SW && code == SW_LID && !value)
> + blocking_notifier_call_chain(&input_notifier_head, value ?
> + LID_SWITCH_CLOSE : LID_SWITCH_OPEN, dev);
I would prefer not having this directly in the input core but rather
have a lid handler that can then use notifier chain to forward the
events further.
Also, here you are running in atomic context, so you need atomic
notifier, not blocking (or you need to involve a workqueue).
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v3 2/3] Input: twl4030 - add TWL603x power button
From: Dmitry Torokhov @ 2025-11-05 22:18 UTC (permalink / raw)
To: akemnade
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Andreas Kemnade, Tony Lindgren, Kevin Hilman, devicetree,
linux-kernel, linux-input, linux-omap
In-Reply-To: <20251105-twl6030-button-v3-2-9b37eb2b0989@kernel.org>
Hi Andreas,
On Wed, Nov 05, 2025 at 08:52:36PM +0100, akemnade@kernel.org wrote:
> From: Andreas Kemnade <andreas@kemnade.info>
>
> Like the TWL4030, these PMICs also have a power button feature, so extend
> the TWL4030 power button driver. As the irqchip of the TWL6030 mfd driver
> does not provide mask, unmask finctions, do it manually.
>
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> ---
> drivers/input/misc/twl4030-pwrbutton.c | 61 +++++++++++++++++++++++++++++++---
> 1 file changed, 57 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/input/misc/twl4030-pwrbutton.c b/drivers/input/misc/twl4030-pwrbutton.c
> index f85cc289c053..b72fba9a1b2c 100644
> --- a/drivers/input/misc/twl4030-pwrbutton.c
> +++ b/drivers/input/misc/twl4030-pwrbutton.c
> @@ -25,22 +25,40 @@
> #include <linux/kernel.h>
> #include <linux/errno.h>
> #include <linux/input.h>
> +#include <linux/bits.h>
Move to the top of includes please.
> #include <linux/interrupt.h>
> #include <linux/of.h>
> #include <linux/platform_device.h>
> #include <linux/mfd/twl.h>
>
> -#define PWR_PWRON_IRQ (1 << 0)
> +#define PWR_PWRON_IRQ BIT(0)
>
> -#define STS_HW_CONDITIONS 0xf
> +#define STS_HW_CONDITIONS_4030 0xf
> +#define STS_HW_CONDITIONS_6030 0x2
Probably no need for these defines, just use numbers in structure
instances.
> +
> +struct twl_pwrbutton_chipdata {
> + u8 status_reg;
> + bool need_manual_irq;
> +};
> +
> +static const struct twl_pwrbutton_chipdata twl4030_chipdata = {
> + STS_HW_CONDITIONS_4030,
> + false,
I am a big fan of named initializers, so maybe
.status_reg = 0x0f,
.need_manual_irq = false,
?
Otherwise looks good.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 1/2] Input: Create input notifier chain in input.c
From: Jonathan Denose @ 2025-11-05 22:40 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel
In-Reply-To: <a4zd7uzo3aigyrhturbpgtcsm2slmtqefivky2bfhqiupcc5aj@iorbkwz6ief4>
Hi Dmitry,
Thanks for your feedback.
On Wed, Nov 5, 2025 at 3:55 PM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> Hi Jonathan,
>
> On Thu, Oct 30, 2025 at 02:10:40PM +0000, Jonathan Denose wrote:
> > To expose input events to other kernel modules, add a blocking notifier
> > chain. Publish LID_SWITCH_OPEN/LID_SWITCH_CLOSE events through this
> > notifier chain when input_handle_event detects events signaling the lid
> > switch has opened or closed.
> >
> > Additionally, export a function which allows other kernel modules to
> > register notifier_block structs against this notifier chain.
> >
> > Signed-off-by: Jonathan Denose <jdenose@google.com>
> > ---
> > drivers/input/input.c | 13 +++++++++++++
> > include/linux/input.h | 7 +++++++
> > 2 files changed, 20 insertions(+)
> >
> > diff --git a/drivers/input/input.c b/drivers/input/input.c
> > index a500e1e276c211d1146dbfea421a3402084007f8..b342b1ff138ccc58d4623edcf1152bd85d7054bf 100644
> > --- a/drivers/input/input.c
> > +++ b/drivers/input/input.c
> > @@ -26,6 +26,7 @@
> > #include <linux/kstrtox.h>
> > #include <linux/mutex.h>
> > #include <linux/rcupdate.h>
> > +#include <linux/notifier.h>
> > #include "input-compat.h"
> > #include "input-core-private.h"
> > #include "input-poller.h"
> > @@ -62,6 +63,8 @@ static const unsigned int input_max_code[EV_CNT] = {
> > [EV_FF] = FF_MAX,
> > };
> >
> > +static struct blocking_notifier_head input_notifier_head;
> > +
> > static inline int is_event_supported(unsigned int code,
> > unsigned long *bm, unsigned int max)
> > {
> > @@ -367,10 +370,20 @@ void input_handle_event(struct input_dev *dev,
> > if (type != EV_SYN)
> > add_input_randomness(type, code, value);
> >
> > + if (type == EV_SW && code == SW_LID && !value)
> > + blocking_notifier_call_chain(&input_notifier_head, value ?
> > + LID_SWITCH_CLOSE : LID_SWITCH_OPEN, dev);
>
> I would prefer not having this directly in the input core but rather
> have a lid handler that can then use notifier chain to forward the
> events further.
Ok, that makes sense to me. In that case, do you have a recommendation
for where the lid handler should go?
It looks like drivers/acpi/button.c initializes and handles the lid switch, so
would it make sense for it to go there?
> Also, here you are running in atomic context, so you need atomic
> notifier, not blocking (or you need to involve a workqueue).
I'll use an atomic notifier in the next version.
> Thanks.
>
> --
> Dmitry
--
Jonathan
^ permalink raw reply
* Re: [PATCH 1/2] Input: Create input notifier chain in input.c
From: Dmitry Torokhov @ 2025-11-05 22:59 UTC (permalink / raw)
To: Jonathan Denose
Cc: Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel
In-Reply-To: <CAMCVhVP+LW27iLXttyFegRj_HMHheYrZtj4uuERLN0uqUjkR6Q@mail.gmail.com>
On Wed, Nov 05, 2025 at 04:40:29PM -0600, Jonathan Denose wrote:
> Hi Dmitry,
>
> Thanks for your feedback.
>
> On Wed, Nov 5, 2025 at 3:55 PM Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> >
> > Hi Jonathan,
> >
> > On Thu, Oct 30, 2025 at 02:10:40PM +0000, Jonathan Denose wrote:
> > > To expose input events to other kernel modules, add a blocking notifier
> > > chain. Publish LID_SWITCH_OPEN/LID_SWITCH_CLOSE events through this
> > > notifier chain when input_handle_event detects events signaling the lid
> > > switch has opened or closed.
> > >
> > > Additionally, export a function which allows other kernel modules to
> > > register notifier_block structs against this notifier chain.
> > >
> > > Signed-off-by: Jonathan Denose <jdenose@google.com>
> > > ---
> > > drivers/input/input.c | 13 +++++++++++++
> > > include/linux/input.h | 7 +++++++
> > > 2 files changed, 20 insertions(+)
> > >
> > > diff --git a/drivers/input/input.c b/drivers/input/input.c
> > > index a500e1e276c211d1146dbfea421a3402084007f8..b342b1ff138ccc58d4623edcf1152bd85d7054bf 100644
> > > --- a/drivers/input/input.c
> > > +++ b/drivers/input/input.c
> > > @@ -26,6 +26,7 @@
> > > #include <linux/kstrtox.h>
> > > #include <linux/mutex.h>
> > > #include <linux/rcupdate.h>
> > > +#include <linux/notifier.h>
> > > #include "input-compat.h"
> > > #include "input-core-private.h"
> > > #include "input-poller.h"
> > > @@ -62,6 +63,8 @@ static const unsigned int input_max_code[EV_CNT] = {
> > > [EV_FF] = FF_MAX,
> > > };
> > >
> > > +static struct blocking_notifier_head input_notifier_head;
> > > +
> > > static inline int is_event_supported(unsigned int code,
> > > unsigned long *bm, unsigned int max)
> > > {
> > > @@ -367,10 +370,20 @@ void input_handle_event(struct input_dev *dev,
> > > if (type != EV_SYN)
> > > add_input_randomness(type, code, value);
> > >
> > > + if (type == EV_SW && code == SW_LID && !value)
> > > + blocking_notifier_call_chain(&input_notifier_head, value ?
> > > + LID_SWITCH_CLOSE : LID_SWITCH_OPEN, dev);
> >
> > I would prefer not having this directly in the input core but rather
> > have a lid handler that can then use notifier chain to forward the
> > events further.
>
> Ok, that makes sense to me. In that case, do you have a recommendation
> for where the lid handler should go?
>
> It looks like drivers/acpi/button.c initializes and handles the lid switch, so
> would it make sense for it to go there?
drivers/acpi/button.c is not the only source of SW_LID events (we also
have cros-ec-keyb.c and others), so I'd recommend putting it into
drivers/input, maybe as lid-notifier.c
>
> > Also, here you are running in atomic context, so you need atomic
> > notifier, not blocking (or you need to involve a workqueue).
>
> I'll use an atomic notifier in the next version.
Another option would be to schedule work and then use blocking notifier.
Thanks.
--
Dmitry
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox