* Re: [RFC v1 0/4] Input: support virtual objects on touchscreens
From: Peter Hutterer @ 2023-05-04 4:29 UTC (permalink / raw)
To: Jeff LaBundy
Cc: Javier Carrasco, Thomas Weißschuh, linux-input, devicetree,
linux-kernel, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Henrik Rydberg, Ulf Hansson, Hans Verkuil, Stephen Boyd,
Sebastian Reichel, Linus Walleij, Jonathan Cameron, Uwe Kleine-g,
Bastian Hecht, Michael Riesch
In-Reply-To: <ZEqvgnhzm7r4O7hz@nixie71>
On Thu, Apr 27, 2023 at 12:23:14PM -0500, Jeff LaBundy wrote:
> Hi Javier,
>
> On Thu, Apr 27, 2023 at 05:59:42PM +0200, Javier Carrasco wrote:
> > Hi,
> >
> > On 25.04.23 18:02, Jeff LaBundy wrote:
> > > Hi Thomas,
> > >
> > > On Tue, Apr 25, 2023 at 05:29:39PM +0200, Thomas Weißschuh wrote:
> > >> Hi Javier,
> > >>
> > >> On 2023-04-25 13:50:45+0200, Javier Carrasco wrote:
> > >>> Some touchscreens are shipped with a physical layer on top of them where
> > >>> a number of buttons and a resized touchscreen surface might be available.
> > >>>
> > >>> In order to generate proper key events by overlay buttons and adjust the
> > >>> touch events to a clipped surface, these patches offer a documented,
> > >>> device-tree-based solution by means of helper functions.
> > >>> An implementation for a specific touchscreen driver is also included.
> > >>>
> > >>> The functions in ts-virtobj provide a simple workflow to acquire
> > >>> physical objects from the device tree, map them into the device driver
> > >>> structures as virtual objects and generate events according to
> > >>> the object descriptions.
> > >>>
> > >>> This solution has been tested with a JT240MHQS-E3 display, which uses
> > >>> the st1624 as a touchscreen and provides two overly buttons and a frame
> > >>> that clips its effective surface.
> > >>
> > >> There are quite a few of notebooks from Asus that feature a printed
> > >> numpad on their touchpad [0]. The mapping from the touch events to the
> > >> numpad events needs to happen in software.
> > >
> > > That example seems a kind of fringe use-case in my opinion; I think the
> > > gap filled by this RFC is the case where a touchscreen has a printed
> > > overlay with a key that represents a fixed function.
> >
> > Exactly, this RFC addresses exactly such printed overlays.
> > >
> > > One problem I do see here is something like libinput or multitouch taking
> > > hold of the input device, and swallowing the key presses because it sees
> > > the device as a touchscreen and is not interested in these keys.
> >
> > Unfortunately I do not know libinput or multitouch and I might be
> > getting you wrong, but I guess the same would apply to any event
> > consumer that takes touchscreens as touch event producers and nothing else.
> >
> > Should they not check the supported events from the device instead of
> > making such assumptions? This RFC adds key events defined in the device
> > tree and they are therefore available and published as device
> > capabilities. That is for example what evtest does to report the
> > supported events and they are then notified accordingly. Is that not the
> > right way to do it?
>
> evtest is just that, a test tool. It's handy for ensuring the device emits
> the appropriate input events in response to hardware inputs, but it is not
> necessarily representative of how the input device may be used in practice.
ftr, I strongly recommend "libinput record" over evtest since it can be
replayed. And for libinput testing "libinput debug-events" to see what
comes out of libinput.
> I would encourage you to test this solution with a simple use-case such as
> Raspbian, and the virtual keys mapped to easily recognizable functions like
> volume up/down.
>
> Here, you will find that libinput will grab the device and declare it to be
> a touchscreen based on the input events it advertises. However, you will not
> see volume up/down keys are handled.
that would be a bug in libinput. libinput doesn't classify devices. It
uses *internal* backends but the backend for keyboard and touchscreen
devices is the same. So as long as your device advertises the various
EV_KEY and EV_ABS bit correctly, things should just work. If that's not
the case for a device please file a bug.
It's still "better" to split it up into different event nodes because
a lot of userspace may not be able to handle touchscreen+keyboard
devices but at least at the libinput level this shouldn't be a problem.
And the xf86-input-libinput driver splits up such devices at the X
level, so even where a device is touchscreen + keyboard you would end up
with two X devices with separate capabilities so they fit into the X
"everything is either a pointer or a keyboard" worldview.
Cheers,
Peter
^ permalink raw reply
* Re: [PATCH] Input: fix open count when closing inhibited device
From: Peter Hutterer @ 2023-05-04 4:18 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel
In-Reply-To: <ZFFz0xAdPNSL3PT7@google.com>
On Tue, May 02, 2023 at 01:34:27PM -0700, Dmitry Torokhov wrote:
> Because the kernel increments device's open count in input_open_device()
> even if device is inhibited, the counter should always be decremented in
> input_close_device() to keep it balanced.
>
> Fixes: a181616487db ("Input: Add "inhibited" property")
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Yep.
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Cheers,
Peter
> ---
> drivers/input/input.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/input/input.c b/drivers/input/input.c
> index f791d14ecf23..8c5fdb0f858a 100644
> --- a/drivers/input/input.c
> +++ b/drivers/input/input.c
> @@ -705,7 +705,7 @@ void input_close_device(struct input_handle *handle)
>
> __input_release_device(handle);
>
> - if (!dev->inhibited && !--dev->users) {
> + if (!--dev->users && !dev->inhibited) {
> if (dev->poller)
> input_dev_poller_stop(dev->poller);
> if (dev->close)
> --
> 2.40.1.495.gc816e09b53d-goog
>
>
> --
> Dmitry
^ permalink raw reply
* [PATCH V1] Input: synaptics-rmi4-add support for F21.
From: Marge Yang @ 2023-05-04 3:43 UTC (permalink / raw)
To: dmitry.torokhov, linux-input, linux-kernel, marge.yang
Cc: vincent.huang, david.chiu, derek.cheng, sam.tsai, Marge Yang
From: Marge Yang <marge.yang@synaptics.com>
RMI4 F21 supports the forcepad click feature.
Signed-off-by: Marge Yang <marge.yang@synaptics.com>
---
drivers/input/rmi4/Kconfig | 8 +++
drivers/input/rmi4/Makefile | 1 +
drivers/input/rmi4/rmi_bus.c | 3 ++
drivers/input/rmi4/rmi_driver.h | 5 +-
drivers/input/rmi4/rmi_f21.c | 110 ++++++++++++++++++++++++++++++++++++++++
5 files changed, 126 insertions(+), 1 deletion(-)
create mode 100644 drivers/input/rmi4/rmi_f21.c
diff --git a/drivers/input/rmi4/Kconfig b/drivers/input/rmi4/Kconfig
index c0163b9..37e2ba4 100644
--- a/drivers/input/rmi4/Kconfig
+++ b/drivers/input/rmi4/Kconfig
@@ -108,6 +108,14 @@ config RMI4_F3A
Function 3A provides GPIO support for RMI4 devices. This includes
support for buttons on TouchPads and ClickPads.
+config RMI4_F21
+ bool "RMI4 Function 21 (PRESSURE)"
+ help
+ Say Y here if you want to add support for RMI4 function 21.
+
+ Function 21 provides buttons/pressure for RMI4 devices. This includes
+ support for buttons/pressure on PressurePad.
+
config RMI4_F54
bool "RMI4 Function 54 (Analog diagnostics)"
depends on VIDEO_DEV=y || (RMI4_CORE=m && VIDEO_DEV=m)
diff --git a/drivers/input/rmi4/Makefile b/drivers/input/rmi4/Makefile
index 02f14c8..ec4f085 100644
--- a/drivers/input/rmi4/Makefile
+++ b/drivers/input/rmi4/Makefile
@@ -11,6 +11,7 @@ rmi_core-$(CONFIG_RMI4_F12) += rmi_f12.o
rmi_core-$(CONFIG_RMI4_F30) += rmi_f30.o
rmi_core-$(CONFIG_RMI4_F34) += rmi_f34.o rmi_f34v7.o
rmi_core-$(CONFIG_RMI4_F3A) += rmi_f3a.o
+rmi_core-$(CONFIG_RMI4_F21) += rmi_f21.o
rmi_core-$(CONFIG_RMI4_F54) += rmi_f54.o
rmi_core-$(CONFIG_RMI4_F55) += rmi_f55.o
diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c
index 50a0134..d42df5f 100644
--- a/drivers/input/rmi4/rmi_bus.c
+++ b/drivers/input/rmi4/rmi_bus.c
@@ -369,6 +369,9 @@ static struct rmi_function_handler *fn_handlers[] = {
#ifdef CONFIG_RMI4_F3A
&rmi_f3a_handler,
#endif
+#ifdef CONFIG_RMI4_F21
+ &rmi_f21_handler,
+#endif
#ifdef CONFIG_RMI4_F54
&rmi_f54_handler,
#endif
diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
index 1c6c608..57f2583 100644
--- a/drivers/input/rmi4/rmi_driver.h
+++ b/drivers/input/rmi4/rmi_driver.h
@@ -114,7 +114,9 @@ static inline int rmi_f03_overwrite_button(struct rmi_function *fn,
}
static inline void rmi_f03_commit_buttons(struct rmi_function *fn) {}
#endif
-
+#ifdef CONFIG_RMI4_F21
+int rmi_f21_report_pressure(struct rmi_function *fn, int i);
+#endif
#ifdef CONFIG_RMI4_F34
int rmi_f34_create_sysfs(struct rmi_device *rmi_dev);
void rmi_f34_remove_sysfs(struct rmi_device *rmi_dev);
@@ -136,6 +138,7 @@ extern struct rmi_function_handler rmi_f12_handler;
extern struct rmi_function_handler rmi_f30_handler;
extern struct rmi_function_handler rmi_f34_handler;
extern struct rmi_function_handler rmi_f3a_handler;
+extern struct rmi_function_handler rmi_f21_handler;
extern struct rmi_function_handler rmi_f54_handler;
extern struct rmi_function_handler rmi_f55_handler;
#endif
diff --git a/drivers/input/rmi4/rmi_f21.c b/drivers/input/rmi4/rmi_f21.c
new file mode 100644
index 0000000..5657bf4
--- /dev/null
+++ b/drivers/input/rmi4/rmi_f21.c
@@ -0,0 +1,110 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2012-2020 Synaptics Incorporated
+ */
+
+#include <linux/kernel.h>
+#include <linux/rmi.h>
+#include <linux/input.h>
+#include <linux/slab.h>
+#include "rmi_driver.h"
+
+#define RMI_F21_FORCE_CLICK_OFFSET 8
+#define RMI_f21_FORCE_CLICK 0x01
+#define RMI_f21_DATA_REGS_MAX_SIZE 1
+#define RMI_f21_FORCEPAD_BUTTON_COUNT 1
+
+struct f21_data {
+ /* Query Data */
+ u8 data_regs[RMI_f21_DATA_REGS_MAX_SIZE];
+ struct input_dev *input;
+ u16 key_code;
+};
+
+static irqreturn_t rmi_f21_attention(int irq, void *ctx)
+{
+ struct rmi_function *fn = ctx;
+ struct f21_data *f21 = dev_get_drvdata(&fn->dev);
+ int error;
+
+ error = rmi_read_block(fn->rmi_dev,
+ fn->fd.data_base_addr+RMI_F21_FORCE_CLICK_OFFSET,
+ f21->data_regs, 1);
+ if (error) {
+ dev_err(&fn->dev,
+ "%s: Failed to read f21 data registers: %d\n",
+ __func__, error);
+ return IRQ_RETVAL(error);
+ }
+
+ if (!!(f21->data_regs[0] & RMI_f21_FORCE_CLICK))
+ input_report_key(f21->input, f21->key_code, true);
+ else
+ input_report_key(f21->input, f21->key_code, false);
+ return IRQ_HANDLED;
+}
+
+static int rmi_f21_config(struct rmi_function *fn)
+{
+ struct f21_data *f21 = dev_get_drvdata(&fn->dev);
+ struct rmi_driver *drv = fn->rmi_dev->driver;
+
+ if (!f21)
+ return 0;
+
+ drv->set_irq_bits(fn->rmi_dev, fn->irq_mask);
+
+ return 0;
+}
+
+static int rmi_f21_initialize(struct rmi_function *fn, struct f21_data *f21)
+{
+ struct input_dev *input = f21->input;
+ unsigned int button = BTN_LEFT;
+
+ f21->key_code = button;
+ input_set_capability(input, EV_KEY, f21->key_code);
+ input->keycode = &(f21->key_code);
+ input->keycodesize = sizeof(f21->key_code);
+ input->keycodemax = RMI_f21_FORCEPAD_BUTTON_COUNT;
+
+ __set_bit(INPUT_PROP_BUTTONPAD, input->propbit);
+
+ return 0;
+}
+
+static int rmi_f21_probe(struct rmi_function *fn)
+{
+ struct rmi_device *rmi_dev = fn->rmi_dev;
+ struct rmi_driver_data *drv_data = dev_get_drvdata(&rmi_dev->dev);
+ struct f21_data *f21;
+ int error;
+
+ if (!drv_data->input) {
+ dev_info(&fn->dev, "f21: no input device found, ignoring\n");
+ return -ENXIO;
+ }
+
+ f21 = devm_kzalloc(&fn->dev, sizeof(*f21), GFP_KERNEL);
+ if (!f21)
+ return -ENOMEM;
+
+ f21->input = drv_data->input;
+
+ error = rmi_f21_initialize(fn, f21);
+ if (error)
+ return error;
+
+ dev_set_drvdata(&fn->dev, f21);
+ return 0;
+}
+
+struct rmi_function_handler rmi_f21_handler = {
+ .driver = {
+ .name = "rmi4_f21",
+ },
+ .func = 0x21,
+ .probe = rmi_f21_probe,
+ .config = rmi_f21_config,
+ .attention = rmi_f21_attention,
+};
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v2] Input: synaptics - disable intertouch for Lenovo L440
From: Jeffery Miller @ 2023-05-04 3:32 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Jonathan Denose, Andrew Duggan, Lyude Paul, Andrew Duggan,
amandhoot12@gmail.com, jdenose@google.com,
linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
markpearson@lenovo.com, wsa+renesas@sang-engineering.com,
benjamin.tissoires
In-Reply-To: <ZFB/f0cvwGlr7VeF@google.com>
[-- Attachment #1: Type: text/plain, Size: 4499 bytes --]
`
On Mon, May 1, 2023 at 10:12 PM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Link: https://lore.kernel.org/r/89456fcd-a113-4c82-4b10-a9bcaefac68f@google.com
>
> Which should have ensured that PS/2 device is resumed first, before
> trying to resume RMI interface. I wonder why it is not working for you.
>
> Can you enable logging and see if the order of resume is correct or not.
> If it is still wrong we need to figure out why setting the link between
> the devices does not have the desired effect.
I have a Lenovo T440p with a similar issue that fails on resume in
rmi_smb_resume's
call to rmi_smb_get_version.
I've patched rmi_smb_get_version adding a sleep and retry loop in an
attempt to see if
it could avoid some race condition.
I've attached a dmesg log from a 6.3.1 kernel, pm debug messages on and with my
retry patch. In this log it succeeds due to the retry loop.
I believe I'm seeing the suspend order of rmi_smbus then PS/2 first
and PS/2 then rmi_smbus on resume.
dmesg during suspend:
[ 96.357933] rmi4_smbus 0-002c: PM: calling rmi_smb_suspend+0x0/0x44
[rmi_smbus] @ 2375, parent: i2c-0
...
[ 96.358521] psmouse serio1: PM: calling serio_suspend+0x0/0x1c @
4517, parent: i8042
dmesg during resume with debugging showing the retry loop around
getting the smbus version number:
[ 97.048216] psmouse serio1: PM: calling serio_resume+0x0/0x8c @
4517, parent: i8042
[ 97.051545] psmouse serio1: PM: serio_resume+0x0/0x8c returned 0
after 3325 usecs
...
[ 97.051557] rmi4_smbus 0-002c: PM: calling rmi_smb_resume+0x0/0x67
[rmi_smbus] @ 4577, parent: i2c-0
...
[ 97.051725] [4577] i2c_i801:i801_check_post:414: i801_smbus
0000:00:1f.3: No response
[ 97.051730] rmi4_smbus 0-002c: failed to get SMBus version number!
[ 97.051732] rmi4_smbus 0-002c: sleeping to try again
[ 97.052780] [4577] i2c_i801:i801_check_post:414: i801_smbus
0000:00:1f.3: No response
[ 97.052785] rmi4_smbus 0-002c: failed to get SMBus version number!
[ 97.052786] rmi4_smbus 0-002c: sleeping to try again
[ 97.053531] snd_hda_intel 0000:00:03.0: PM: pci_pm_resume+0x0/0xdf
returned 0 after 2275 usecs
[ 97.053790] [4577] i2c_i801:i801_check_post:414: i801_smbus
0000:00:1f.3: No response
[ 97.053795] rmi4_smbus 0-002c: failed to get SMBus version number!
[ 97.053796] rmi4_smbus 0-002c: sleeping to try again
[ 97.054783] [4577] i2c_i801:i801_check_post:414: i801_smbus
0000:00:1f.3: No response
[ 97.054788] rmi4_smbus 0-002c: failed to get SMBus version number!
[ 97.054789] rmi4_smbus 0-002c: sleeping to try again
[ 97.055779] [4577] i2c_i801:i801_check_post:414: i801_smbus
0000:00:1f.3: No response
[ 97.055784] rmi4_smbus 0-002c: failed to get SMBus version number!
[ 97.055785] rmi4_smbus 0-002c: sleeping to try again
[ 97.057117] rmi4_smbus 0-002c: JAM: time to return 6ms
[ 97.057893] rmi4_smbus 0-002c: wrote 4 bytes at 0x86: 0 (4c 00 01
00)
...
[ 97.067335] rmi4_smbus 0-002c: wrote 1 bytes at 0x07: 0 (80)
[ 97.067340] rmi4_f11 rmi4-00.fn11: Resuming...
[ 97.067342] rmi4_smbus 0-002c: PM: rmi_smb_resume+0x0/0x67
[rmi_smbus] returned 0 after 15783 usecs
WIth this short timeout loop it seems to take between 6ms to 12ms
before the calls
to `i2c_smbus_read_byte_data` stop returning -ENXIO from "No response"
log lines from i2c_i801.
I have seen this on various versions such as 5.15, 5.10 (with the
https://lore.kernel.org/r/89456fcd-a113-4c82-4b10-a9bcaefac68f@google.com
patch applied), 6.1
and reproduced here on the 6.3.1 kernel.
I don't know what the delay is waiting on and if there is some other
way to deal with it.
In these logs it appears that there are a few calls occuring at the
same time as rmi_smb_resume.
None of them return just before the retry loop ends and I'm not sure
if they are related.
I'm not sure if there is more information during this time that would
be useful to look at
while it's resuming?
If I apply the patch to `device_disable_async_suspend(&client->dev);`
in rmi_smb_probe it
doesn't seem to need the delay. I could post dmesg output with the
disable in rmi_smb_probe applied
if the timing is possibly interesting to see the ordering and timing
in that case.
If there is anything else you would like me to try, please let me know.
Is it possible there's just some sort of delay occurring here for the
i801 bus to come
up that can be handled in a different way?
Thanks,
Jeff
[-- Attachment #2: dmesg-6-3-1-retry.out.xz --]
[-- Type: application/x-xz, Size: 23836 bytes --]
[-- Attachment #3: 0001-Input-synaptics-rmi4-retry-reading-SMBus-version-on-.patch --]
[-- Type: text/x-patch, Size: 2960 bytes --]
From fd4e87a0d1d8efa0f3a924a5e5cdd47f10b336dc Mon Sep 17 00:00:00 2001
From: Jeffery Miller <jefferymiller@google.com>
Date: Thu, 20 Apr 2023 11:35:59 -0500
Subject: [PATCH] Input: synaptics-rmi4 - retry reading SMBus version on resume
When resuming from suspend there is a delay where
getting the version data from the smbus returns
-ENXIO. This causes the rmi4 resume to fail and
the touchpad to stop working after resume.
In lieu of finding what this really needs to wait
on just retry a few times so it works.
The limit of 100ms is arbitrary.
My tests have shown this to take an extra 7-12ms
on resume to succeed on the Lenovo t440p machine.
Logs on resume now look something like:
```
[ 108.622951] rmi4_smbus 0-002c: calling rmi_smb_resume+0x0/0x63 [rmi_smbus] @ 3975, parent: i2c-0
...
[ 108.623132] rmi4_smbus 0-002c: failed to get SMBus version number!
[ 108.623134] rmi4_smbus 0-002c: sleeping to try again
[ 108.626003] rmi4_smbus 0-002c: failed to get SMBus version number!
[ 108.626005] rmi4_smbus 0-002c: sleeping to try again
[ 108.629025] rmi4_smbus 0-002c: failed to get SMBus version number!
[ 108.629028] rmi4_smbus 0-002c: sleeping to try again
[ 108.632017] rmi4_smbus 0-002c: failed to get SMBus version number!
[ 108.632019] rmi4_smbus 0-002c: sleeping to try again
[ 108.636172] rmi4_smbus 0-002c: wrote 4 bytes at 0x86: 0 (4c 00 01 00)
```
Signed-off-by: Jeffery Miller <jefferymiller@google.com>
---
drivers/input/rmi4/rmi_smbus.c | 24 +++++++++++++++++++-----
1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/drivers/input/rmi4/rmi_smbus.c b/drivers/input/rmi4/rmi_smbus.c
index 4bf0e1df6a4a..1fccad5811cf 100644
--- a/drivers/input/rmi4/rmi_smbus.c
+++ b/drivers/input/rmi4/rmi_smbus.c
@@ -9,6 +9,7 @@
#include <linux/delay.h>
#include <linux/i2c.h>
#include <linux/interrupt.h>
+#include <linux/jiffies.h>
#include <linux/kconfig.h>
#include <linux/lockdep.h>
#include <linux/module.h>
@@ -44,14 +45,27 @@ static int rmi_smb_get_version(struct rmi_smb_xport *rmi_smb)
struct i2c_client *client = rmi_smb->client;
int retval;
+ unsigned long deadline = jiffies + msecs_to_jiffies(100);
+ unsigned long before = jiffies;
+
/* Check if for SMBus new version device by reading version byte. */
- retval = i2c_smbus_read_byte_data(client, SMB_PROTOCOL_VERSION_ADDRESS);
- if (retval < 0) {
+ while (time_before(jiffies, deadline)) {
+
+ retval = i2c_smbus_read_byte_data(client,
+ SMB_PROTOCOL_VERSION_ADDRESS);
+ if (retval >= 0) {
+ retval++;
+ break;
+ }
+
dev_err(&client->dev, "failed to get SMBus version number!\n");
- return retval;
+ if (retval != -ENXIO)
+ break;
+ dev_warn(&client->dev, "sleeping to try again\n");
+ fsleep(500);
}
-
- return retval + 1;
+ dev_warn(&client->dev, "JAM: time to return %ums", jiffies_to_msecs(jiffies - before));
+ return retval;
}
/* SMB block write - wrapper over ic2_smb_write_block */
--
2.40.1.495.gc816e09b53d-goog
^ permalink raw reply related
* Re: [PATCH v5] Fix freeze in lm8333 i2c keyboard driver
From: Jeff LaBundy @ 2023-05-04 1:44 UTC (permalink / raw)
To: Tomas Mudrunka; +Cc: dmitry.torokhov, linux-input, linux-kernel
In-Reply-To: <20230503153231.1136114-1-tomas.mudrunka@gmail.com>
Hi Tomas,
On Wed, May 03, 2023 at 05:32:31PM +0200, Tomas Mudrunka wrote:
> LM8333 uses gpio interrupt line which is triggered by falling edge.
> When button is pressed before driver is loaded,
> driver will miss the edge and never respond again.
> To fix this we run the interrupt handler before registering IRQ
> to clear the interrupt via i2c command.
>
> Signed-off-by: Tomas Mudrunka <tomas.mudrunka@gmail.com>
> ---
Reviewed-by: Jeff LaBundy <jeff@labundy.com>
> drivers/input/keyboard/lm8333.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/input/keyboard/lm8333.c b/drivers/input/keyboard/lm8333.c
> index 7457c3220..52108c370 100644
> --- a/drivers/input/keyboard/lm8333.c
> +++ b/drivers/input/keyboard/lm8333.c
> @@ -178,6 +178,8 @@ static int lm8333_probe(struct i2c_client *client)
> dev_warn(&client->dev, "Unable to set active time\n");
> }
>
> + lm8333_irq_thread(client->irq, lm8333);
> +
> err = request_threaded_irq(client->irq, NULL, lm8333_irq_thread,
> IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> "lm8333", lm8333);
> --
> 2.40.1
Thank you for the productive discussion.
Kind regards,
Jeff LaBundy
^ permalink raw reply
* Re: [RFC PATCH 0/3] input: gpio-keys - fix pm ordering
From: Saravana Kannan @ 2023-05-03 22:31 UTC (permalink / raw)
To: Doug Berger
Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Dmitry Torokhov,
Lad Prabhakar, Gergo Koteles, Jonathan Cameron, Andy Shevchenko,
Dan Williams, Hans de Goede, Thomas Gleixner, Kees Cook,
Florian Fainelli, linux-kernel, linux-input, Android Kernel Team,
Linus Walleij
In-Reply-To: <597ba853-1fe4-9263-c448-422b0c2a91d8@gmail.com>
On Wed, May 3, 2023 at 3:21 PM Doug Berger <opendmb@gmail.com> wrote:
>
> On 5/2/2023 7:18 PM, Saravana Kannan wrote:
> > On Mon, May 1, 2023 at 1:40 PM Saravana Kannan <saravanak@google.com> wrote:
> >>
> >> On Thu, Apr 27, 2023 at 3:18 PM Doug Berger <opendmb@gmail.com> wrote:
> >>>
> >>> Commit 52cdbdd49853 ("driver core: correct device's shutdown
> >>> order") allowed for proper sequencing of the gpio-keys device
> >>> shutdown callbacks by moving the device to the end of the
> >>> devices_kset list at probe which was delayed by child
> >>> dependencies.
> >>>
> >>> However, commit 722e5f2b1eec ("driver core: Partially revert
> >>> "driver core: correct device's shutdown order"") removed this
> >>> portion of that commit causing a reversion in the gpio-keys
> >>> behavior which can prevent waking from shutdown.
> >>>
> >>> This RFC is an attempt to find a better solution for properly
> >>> creating gpio-keys device links to ensure its suspend/resume and
> >>> shutdown services are invoked before those of its suppliers.
> >>>
> >>> The first patch here is pretty brute force but simple and has
> >>> the advantage that it should be easily backportable to the
> >>> versions where the regression first occurred.
> >>
> >> We really shouldn't be calling device_pm_move_to_tail() in drivers
> >> because device link uses device_pm_move_to_tail() for ordering too.
> >> And it becomes a "race" between device links and when the driver calls
> >> device_pm_move_to_tail() and I'm not sure we'll get the same ordering
> >> every time.
> >>
> >>>
> >>> The second patch is perhaps better in spirit though still a bit
> >>> inelegant, but it can only be backported to versions of the
> >>> kernel that contain the commit in its 'Fixes:' tag. That isn't
> >>> really a valid 'Fixes:' tag since that commit did not cause the
> >>> regression, but it does represent how far the patch could be
> >>> backported.
> >>>
> >>> Both commits shouldn't really exist in the same kernel so the
> >>> third patch reverts the first in an attempt to make that clear
> >>> (though it may be a source of confusion for some).
> >>>
> >>> Hopefully someone with a better understanding of device links
> >>> will see a less intrusive way to automatically capture these
> >>> dependencies for parent device drivers that implement the
> >>> functions of child node devices.
> >>
> >> Can you give a summary of the issue on a real system? I took a look at
> >> the two commits you've referenced above and it's not clear what's
> >> still broken in the 6.3+
> >>
> >> But I'd think that just teaching fw_devlink about some property should
> >> be sufficient. If you are facing a real issue, have you made sure you
> >> have fw_devlink=on (this is the default unless you turned it off in
> >> the commandline when it had issues in the past).
> >>
> >
> > I took a closer look at how gpio-keys work and I can see why
> > fw_devlink doesn't pick up the GPIO dependencies. It's because the
> > gpio dependencies are listed under child "key-x" device nodes under
> > the main "gpio-keys" device tree node. fw_devlink doesn't consider
> > dependencies under child nodes as mandatory dependencies of the parent
> > node.
> >
> > The main reason for this was because of how fw_devlink used to work.
> > But I might be able to change fw_devlink to pick this up
> > automatically. I need to think a bit more about this because in some
> > cases, ignoring those dependencies is the right thing to do. Give me a
> > few weeks to think through and experiment with this on my end.
> Thank you for taking a deeper look at gpio-keys, and for getting through
> the gobblety-gook description in my cover-letter ;).
>
> Yes, the dependencies of children are not automatically inherited by
> their parents and it is not clear to me whether or not that should
> change, but it definitely creates a problem for the sequencing of
> gpio-keys device callbacks.
>
> I initially prepared the second patch as a way to explicitly create
> device links specifically for the gpio-keys device from these child
> dependencies as a work around for the fw_devlinks being dropped. I don't
> really consider this a viable patch which is why I made it an RFC, but I
> hoped it would highlight the issue.
Thanks. It definitely made it easier for me to get to the root of the
problem/omission.
> However, the regression actually occurs in v4.18 and fw_devlink isn't
> introduced until v5.7 so it is desirable to think about solutions that
> could be backported to older versions.
For older versions, if they have device link support, I'd recommend
creating device links and letting that take care of it.
Also, if you use the right GPIO APIs, at least on newer kernels Linus
W was looking into creating device links automatically from the GPIO
framework level.
So maybe you can backport some variant of that into the older kernels
and leave it to fw_devlink on the newer ones.
-Saravana
> This is why I provided the first
> patch for discussion. Again, it is not a desirable solution just an
> illustration what could be easily backported to restore the gpio-keys
> behavior prior to commit 722e5f2b1eec ("driver core: Partially revert
> "driver core: correct device's shutdown order"") without affecting other
> devices.
>
> Thanks again for your willingness to take the time to think this through,
> Doug
>
> >
> > -Saravana
>
^ permalink raw reply
* Re: [RFC PATCH 0/3] input: gpio-keys - fix pm ordering
From: Doug Berger @ 2023-05-03 22:20 UTC (permalink / raw)
To: Saravana Kannan
Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Dmitry Torokhov,
Lad Prabhakar, Gergo Koteles, Jonathan Cameron, Andy Shevchenko,
Dan Williams, Hans de Goede, Thomas Gleixner, Kees Cook,
Florian Fainelli, linux-kernel, linux-input, Android Kernel Team
In-Reply-To: <CAGETcx_xjNT1Tp0GeqoddFwGFpv3O33hZZpEiThNg1wwWcfEQw@mail.gmail.com>
On 5/2/2023 7:18 PM, Saravana Kannan wrote:
> On Mon, May 1, 2023 at 1:40 PM Saravana Kannan <saravanak@google.com> wrote:
>>
>> On Thu, Apr 27, 2023 at 3:18 PM Doug Berger <opendmb@gmail.com> wrote:
>>>
>>> Commit 52cdbdd49853 ("driver core: correct device's shutdown
>>> order") allowed for proper sequencing of the gpio-keys device
>>> shutdown callbacks by moving the device to the end of the
>>> devices_kset list at probe which was delayed by child
>>> dependencies.
>>>
>>> However, commit 722e5f2b1eec ("driver core: Partially revert
>>> "driver core: correct device's shutdown order"") removed this
>>> portion of that commit causing a reversion in the gpio-keys
>>> behavior which can prevent waking from shutdown.
>>>
>>> This RFC is an attempt to find a better solution for properly
>>> creating gpio-keys device links to ensure its suspend/resume and
>>> shutdown services are invoked before those of its suppliers.
>>>
>>> The first patch here is pretty brute force but simple and has
>>> the advantage that it should be easily backportable to the
>>> versions where the regression first occurred.
>>
>> We really shouldn't be calling device_pm_move_to_tail() in drivers
>> because device link uses device_pm_move_to_tail() for ordering too.
>> And it becomes a "race" between device links and when the driver calls
>> device_pm_move_to_tail() and I'm not sure we'll get the same ordering
>> every time.
>>
>>>
>>> The second patch is perhaps better in spirit though still a bit
>>> inelegant, but it can only be backported to versions of the
>>> kernel that contain the commit in its 'Fixes:' tag. That isn't
>>> really a valid 'Fixes:' tag since that commit did not cause the
>>> regression, but it does represent how far the patch could be
>>> backported.
>>>
>>> Both commits shouldn't really exist in the same kernel so the
>>> third patch reverts the first in an attempt to make that clear
>>> (though it may be a source of confusion for some).
>>>
>>> Hopefully someone with a better understanding of device links
>>> will see a less intrusive way to automatically capture these
>>> dependencies for parent device drivers that implement the
>>> functions of child node devices.
>>
>> Can you give a summary of the issue on a real system? I took a look at
>> the two commits you've referenced above and it's not clear what's
>> still broken in the 6.3+
>>
>> But I'd think that just teaching fw_devlink about some property should
>> be sufficient. If you are facing a real issue, have you made sure you
>> have fw_devlink=on (this is the default unless you turned it off in
>> the commandline when it had issues in the past).
>>
>
> I took a closer look at how gpio-keys work and I can see why
> fw_devlink doesn't pick up the GPIO dependencies. It's because the
> gpio dependencies are listed under child "key-x" device nodes under
> the main "gpio-keys" device tree node. fw_devlink doesn't consider
> dependencies under child nodes as mandatory dependencies of the parent
> node.
>
> The main reason for this was because of how fw_devlink used to work.
> But I might be able to change fw_devlink to pick this up
> automatically. I need to think a bit more about this because in some
> cases, ignoring those dependencies is the right thing to do. Give me a
> few weeks to think through and experiment with this on my end.
Thank you for taking a deeper look at gpio-keys, and for getting through
the gobblety-gook description in my cover-letter ;).
Yes, the dependencies of children are not automatically inherited by
their parents and it is not clear to me whether or not that should
change, but it definitely creates a problem for the sequencing of
gpio-keys device callbacks.
I initially prepared the second patch as a way to explicitly create
device links specifically for the gpio-keys device from these child
dependencies as a work around for the fw_devlinks being dropped. I don't
really consider this a viable patch which is why I made it an RFC, but I
hoped it would highlight the issue.
However, the regression actually occurs in v4.18 and fw_devlink isn't
introduced until v5.7 so it is desirable to think about solutions that
could be backported to older versions. This is why I provided the first
patch for discussion. Again, it is not a desirable solution just an
illustration what could be easily backported to restore the gpio-keys
behavior prior to commit 722e5f2b1eec ("driver core: Partially revert
"driver core: correct device's shutdown order"") without affecting other
devices.
Thanks again for your willingness to take the time to think this through,
Doug
>
> -Saravana
^ permalink raw reply
* Re: [PATCH] Input: tests - fix input_test_match_device_id test
From: Javier Martinez Canillas @ 2023-05-03 21:55 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input; +Cc: Geert Uytterhoeven, linux-kernel
In-Reply-To: <ZFLI7T2qZTGJ1UUK@google.com>
Dmitry Torokhov <dmitry.torokhov@gmail.com> writes:
Hello Dmitry,
> Properly initialize input_device_id structure in
> input_test_match_device_id test to make sure it contains no garbage
> causing the test to randomly fail.
>
> Fixes: fdefcbdd6f36 ("Input: Add KUnit tests for some of the input core helper functions")
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
Thanks a lot for fixing this!
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
^ permalink raw reply
* [PATCH] Input: tests - fix input_test_match_device_id test
From: Dmitry Torokhov @ 2023-05-03 20:49 UTC (permalink / raw)
To: linux-input; +Cc: Javier Martinez Canillas, Geert Uytterhoeven, linux-kernel
Properly initialize input_device_id structure in
input_test_match_device_id test to make sure it contains no garbage
causing the test to randomly fail.
Fixes: fdefcbdd6f36 ("Input: Add KUnit tests for some of the input core helper functions")
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/tests/input_test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/tests/input_test.c b/drivers/input/tests/input_test.c
index 8b8ac3412a70..0540225f0288 100644
--- a/drivers/input/tests/input_test.c
+++ b/drivers/input/tests/input_test.c
@@ -87,7 +87,7 @@ static void input_test_timestamp(struct kunit *test)
static void input_test_match_device_id(struct kunit *test)
{
struct input_dev *input_dev = test->priv;
- struct input_device_id id;
+ struct input_device_id id = { 0 };
/*
* Must match when the input device bus, vendor, product, version
--
2.40.1.495.gc816e09b53d-goog
--
Dmitry
^ permalink raw reply related
* Re: [regression] Bug 217394 - IRQ override skipping breaks the Aya Neo Air Plus 6800U keyboard buttons
From: Dmitry Torokhov @ 2023-05-03 19:00 UTC (permalink / raw)
To: Limonciello, Mario
Cc: Linux regressions mailing list, Chuanhong Guo, linux-input,
Mario Limonciello, Rafael J. Wysocki, ACPI Devel Maling List,
LKML
In-Reply-To: <68d017d9-d815-01d4-23c1-49c0aaf5f20b@amd.com>
On Wed, May 03, 2023 at 11:11:33AM -0500, Limonciello, Mario wrote:
> +linux-input
>
> On 5/3/2023 7:58 AM, Linux regression tracking (Thorsten Leemhuis) wrote:
> > Hi, Thorsten here, the Linux kernel's regression tracker.
> >
> > I noticed a regression report in bugzilla.kernel.org. As many (most?)
> > kernel developers don't keep an eye on it, I decided to forward it by mail.
> >
> > Chuanhong Guo, apparently it's cause by a change of yours.
> >
> > Note, you have to use bugzilla to reach the reporter, as I sadly[1] can
> > not CCed them in mails like this.
> >
> > Quoting from https://bugzilla.kernel.org/show_bug.cgi?id=217394 :
> >
> > > Matthew 2023-05-03 02:28:33 UTC
> > >
> > > Reverting the changes found in this patch fixes the issue:
> > > > https://lore.kernel.org/all/20220712020058.90374-1-gch981213@gmail.com/
> > > With that patch the AT Translated Set 2 Keyboard doesn't show up with the evtest and is not usable.
> > >
> > > Hardware:
> > >
> > > Aya Neo Air Plus
> > > AMD Ryzen 7 6800U
> > See the ticket for more details.
> >
> > BTW: there apparently is another IRQ override needed for a different
> > machine. See https://bugzilla.kernel.org/show_bug.cgi?id=216804#c8 for
> > details (ignore the comments before that, the quirk entry for that
> > machine was merged; comment 8 and all related to it really should have a
> > separate bug; that's also why this partly fall through the cracks here
> > :-/ ). The user is currently trying to create a patch.
> >
> Something I'm wondering about is if it's possible for i8042 to detect the
> polarity is incorrect when it probes and
> to try to correct it.
>
> If we could do that we can probably drop 9946e39fe8d0 ("ACPI: resource: skip
> IRQ override on AMD Zen platforms")
> to fix this issue along with all the other quirks that have collected over
> time on i8042 polarity issues.
>
8042 is shared between multiple platforms and is quite fragile as it is.
If there are issues in AMD firmware and you know the polarity that is
needed for 8042 on these platforms you should add a proper fixup for
override. Maybe you should only skip override for IRQ 1?
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 1/4] Input/ARM: ads7846: Get pendown IRQ from descriptors
From: Dmitry Torokhov @ 2023-05-03 18:41 UTC (permalink / raw)
To: Linus Walleij
Cc: Aaro Koskinen, Janusz Krzysztofik, Tony Lindgren, Russell King,
Daniel Mack, Haojian Zhuang, Robert Jarzmik, Thomas Bogendoerfer,
Mark Brown, Bartosz Golaszewski, Andreas Kemnade, Helge Deller,
Ulf Hansson, linux-omap, linux-arm-kernel, linux-kernel,
linux-mips, linux-input, linux-spi, linux-fbdev, dri-devel,
linux-mmc
In-Reply-To: <20230430-nokia770-regression-v1-1-97704e36b094@linaro.org>
On Sun, Apr 30, 2023 at 11:22:16AM +0200, Linus Walleij wrote:
> The ADS7846 has some limited support for using GPIO descriptors,
> let's convert it over completely and fix all users to provide
> GPIOs in descriptor tables.
>
> The Nokia 770 now has dynamic allocation of IRQ numbers, so this
> needs to be fixed for it to work.
>
> Fixes: 92bf78b33b0b ("gpio: omap: use dynamic allocation of base")
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> arch/arm/mach-omap1/board-nokia770.c | 12 +++++++++++-
> arch/arm/mach-pxa/spitz.c | 11 ++++++++++-
> arch/mips/alchemy/devboards/db1000.c | 11 ++++++++++-
> drivers/input/touchscreen/ads7846.c | 32 ++++++++------------------------
> include/linux/spi/ads7846.h | 2 --
> 5 files changed, 39 insertions(+), 29 deletions(-)
>
> diff --git a/arch/arm/mach-omap1/board-nokia770.c b/arch/arm/mach-omap1/board-nokia770.c
> index a501a473ffd6..eb7652670447 100644
> --- a/arch/arm/mach-omap1/board-nokia770.c
> +++ b/arch/arm/mach-omap1/board-nokia770.c
> @@ -118,7 +118,16 @@ static struct ads7846_platform_data nokia770_ads7846_platform_data __initdata =
> .debounce_max = 10,
> .debounce_tol = 3,
> .debounce_rep = 1,
> - .gpio_pendown = ADS7846_PENDOWN_GPIO,
> +};
> +
> +static struct gpiod_lookup_table nokia770_ads7846_gpio_table = {
> + /* SPI bus 2, device with chip select 0 */
> + .dev_id = "spi2.0",
> + .table = {
> + GPIO_LOOKUP("gpio-0-15", ADS7846_PENDOWN_GPIO,
> + "pendown", GPIO_ACTIVE_HIGH),
> + { }
> + },
> };
I would like to eventually get rid of GPIO_LOOKUP in favor of
PROPERTY_ENTRY_GPIO. Can we try something like the draft below (just
typed, not even compiled):
diff --git a/arch/arm/mach-omap1/board-nokia770.c b/arch/arm/mach-omap1/board-nokia770.c
index a501a473ffd6..34b8e392b917 100644
--- a/arch/arm/mach-omap1/board-nokia770.c
+++ b/arch/arm/mach-omap1/board-nokia770.c
@@ -12,6 +12,7 @@
#include <linux/init.h>
#include <linux/mutex.h>
#include <linux/platform_device.h>
+#include <linux/property.h>
#include <linux/input.h>
#include <linux/omapfb.h>
@@ -35,6 +36,24 @@
#include "clock.h"
#include "mmc.h"
+static const struct software_node nokia770_mpuio_gpiochip_node = {
+ .name = "mpuio",
+};
+
+static const struct software_node nokia770_gpiochip1_node = {
+ .name = "gpio-0-15",
+};
+
+static const struct software_node nokia770_gpiochip2_node = {
+ .name = "gpio-16-31",
+};
+
+static const struct software_node nokia770_gpiochip_nodes[] = {
+ &nokia770_mpuio_gpiochip_node
+ &nokia770_gpiochip1_node,
+ &nokia770_gpiochip2_node,
+};
+
#define ADS7846_PENDOWN_GPIO 15
static const unsigned int nokia770_keymap[] = {
@@ -102,6 +121,17 @@ static const struct omap_lcd_config nokia770_lcd_config __initconst = {
.ctrl_name = "hwa742",
};
+static const struct property_entry nokia770_mipid_props[] = {
+ PROPERTY_ENTRY_GPIO("reset-gpios", &nokia770_gpiochip1_node,
+ 13, GPIO_ACTIVE_LOW),
+ { }
+};
+
+static const struct software_node nokia770_mipid_swnode = {
+ .name = "lcd_mipid",
+ .properties = nokia770_mipid_props,
+};
+
static void __init mipid_dev_init(void)
{
nokia770_mipid_platform_data.nreset_gpio = 13;
@@ -110,15 +140,22 @@ static void __init mipid_dev_init(void)
omapfb_set_lcd_config(&nokia770_lcd_config);
}
-static struct ads7846_platform_data nokia770_ads7846_platform_data __initdata = {
- .x_max = 0x0fff,
- .y_max = 0x0fff,
- .x_plate_ohms = 180,
- .pressure_max = 255,
- .debounce_max = 10,
- .debounce_tol = 3,
- .debounce_rep = 1,
- .gpio_pendown = ADS7846_PENDOWN_GPIO,
+static const struct property_entry nokia770_ads7846_props[] = {
+ PROPERTY_ENTRY_U32("touchscreen-size-x", 4096),
+ PROPERTY_ENTRY_U32("touchscreen-size-y", 4096),
+ PROPERTY_ENTRY_U32("touchscreen-max-pressure", 256),
+ PROPERTY_ENTRY_U32("touchscreen-average-samples", 10),
+ PROPERTY_ENTRY_U16("ti,x-plate-ohms", 180),
+ PROPERTY_ENTRY_U16("ti,debounce-tol", 3),
+ PROPERTY_ENTRY_U16("ti,debounce-rep", 1),
+ PROPERTY_ENTRY_GPIO("pendown-gpios", &nokia770_gpiochip1_node,
+ ADS7846_PENDOWN_GPIO, GPIO_ACTIVE_HIGH),
+ { }
+};
+
+static const struct software_node nokia770_ads7846_swnode = {
+ .name = "ads7846",
+ .properties = nokia770_ads7846_props,
};
static struct spi_board_info nokia770_spi_board_info[] __initdata = {
@@ -128,13 +165,14 @@ static struct spi_board_info nokia770_spi_board_info[] __initdata = {
.chip_select = 3,
.max_speed_hz = 12000000,
.platform_data = &nokia770_mipid_platform_data,
+ .swnode = &nokia770_mipid_swnode,
},
[1] = {
.modalias = "ads7846",
.bus_num = 2,
.chip_select = 0,
.max_speed_hz = 2500000,
- .platform_data = &nokia770_ads7846_platform_data,
+ .swnode = &nokia770_ads7846_swnode,
},
};
@@ -212,14 +250,15 @@ static inline void nokia770_mmc_init(void)
#endif
#if IS_ENABLED(CONFIG_I2C_CBUS_GPIO)
-static struct gpiod_lookup_table nokia770_cbus_gpio_table = {
- .dev_id = "i2c-cbus-gpio.2",
- .table = {
- GPIO_LOOKUP_IDX("mpuio", 9, NULL, 0, 0), /* clk */
- GPIO_LOOKUP_IDX("mpuio", 10, NULL, 1, 0), /* dat */
- GPIO_LOOKUP_IDX("mpuio", 11, NULL, 2, 0), /* sel */
- { },
- },
+static const struct software_node_ref_args nokia770_cbus_gpio_refs[] = {
+ SOFTWARE_NODE_REFERENCE(&nokia770_mpuio_gpiochip_swnode, 9, 0),
+ SOFTWARE_NODE_REFERENCE(&nokia770_mpuio_gpiochip_swnode, 10, 0),
+ SOFTWARE_NODE_REFERENCE(&nokia770_mpuio_gpiochip_swnode, 11, 0),
+};
+
+static const struct property_entry nokia770_ads7846_props[] = {
+ PROPERTY_ENTRY_REF_ARRAY("gpios", nokia770_cbus_gpio_refs),
+ { }
};
static struct platform_device nokia770_cbus_device = {
@@ -253,7 +292,8 @@ static void __init nokia770_cbus_init(void)
nokia770_i2c_board_info_2[1].irq = gpio_to_irq(tahvo_irq_gpio);
i2c_register_board_info(2, nokia770_i2c_board_info_2,
ARRAY_SIZE(nokia770_i2c_board_info_2));
- gpiod_add_lookup_table(&nokia770_cbus_gpio_table);
+ device_create_managed_software_node(&nokia770_cbus_device.dev,
+ nokia770_cbus_props, NULL);
platform_device_register(&nokia770_cbus_device);
}
#else /* CONFIG_I2C_CBUS_GPIO */
@@ -273,6 +313,7 @@ static void __init omap_nokia770_init(void)
/* Unmask SleepX signal */
omap_writew((omap_readw(0xfffb5004) & ~2), 0xfffb5004);
+ software_node_register_node_group(nokia770_gpiochip_swnodes);
platform_add_devices(nokia770_devices, ARRAY_SIZE(nokia770_devices));
nokia770_spi_board_info[1].irq = gpio_to_irq(15);
spi_register_board_info(nokia770_spi_board_info,
This will need switching ads7846.c from using of_property* to
device_property* so that it can parse software nodes.
Thanks.
--
Dmitry
^ permalink raw reply related
* Re: [regression] Bug 217394 - IRQ override skipping breaks the Aya Neo Air Plus 6800U keyboard buttons
From: Limonciello, Mario @ 2023-05-03 16:11 UTC (permalink / raw)
To: Linux regressions mailing list, Chuanhong Guo, linux-input
Cc: Mario Limonciello, Rafael J. Wysocki, ACPI Devel Maling List,
LKML
In-Reply-To: <01ea5c8e-ed2f-7568-f6ed-896329e7b673@leemhuis.info>
+linux-input
On 5/3/2023 7:58 AM, Linux regression tracking (Thorsten Leemhuis) wrote:
> Hi, Thorsten here, the Linux kernel's regression tracker.
>
> I noticed a regression report in bugzilla.kernel.org. As many (most?)
> kernel developers don't keep an eye on it, I decided to forward it by mail.
>
> Chuanhong Guo, apparently it's cause by a change of yours.
>
> Note, you have to use bugzilla to reach the reporter, as I sadly[1] can
> not CCed them in mails like this.
>
> Quoting from https://bugzilla.kernel.org/show_bug.cgi?id=217394 :
>
>> Matthew 2023-05-03 02:28:33 UTC
>>
>> Reverting the changes found in this patch fixes the issue:
>>> https://lore.kernel.org/all/20220712020058.90374-1-gch981213@gmail.com/
>> With that patch the AT Translated Set 2 Keyboard doesn't show up with the evtest and is not usable.
>>
>> Hardware:
>>
>> Aya Neo Air Plus
>> AMD Ryzen 7 6800U
> See the ticket for more details.
>
> BTW: there apparently is another IRQ override needed for a different
> machine. See https://bugzilla.kernel.org/show_bug.cgi?id=216804#c8 for
> details (ignore the comments before that, the quirk entry for that
> machine was merged; comment 8 and all related to it really should have a
> separate bug; that's also why this partly fall through the cracks here
> :-/ ). The user is currently trying to create a patch.
>
Something I'm wondering about is if it's possible for i8042 to detect
the polarity is incorrect when it probes and
to try to correct it.
If we could do that we can probably drop 9946e39fe8d0 ("ACPI: resource:
skip IRQ override on AMD Zen platforms")
to fix this issue along with all the other quirks that have collected
over time on i8042 polarity issues.
^ permalink raw reply
* [PATCH v5] Fix freeze in lm8333 i2c keyboard driver
From: Tomas Mudrunka @ 2023-05-03 15:32 UTC (permalink / raw)
To: tomas.mudrunka; +Cc: dmitry.torokhov, jeff, linux-input, linux-kernel
In-Reply-To: <CAH2-hcJYVpBNwnMS6qUp4=MW8kSryDAz7G5cNA8R00QabC9bjg@mail.gmail.com>
LM8333 uses gpio interrupt line which is triggered by falling edge.
When button is pressed before driver is loaded,
driver will miss the edge and never respond again.
To fix this we run the interrupt handler before registering IRQ
to clear the interrupt via i2c command.
Signed-off-by: Tomas Mudrunka <tomas.mudrunka@gmail.com>
---
drivers/input/keyboard/lm8333.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/input/keyboard/lm8333.c b/drivers/input/keyboard/lm8333.c
index 7457c3220..52108c370 100644
--- a/drivers/input/keyboard/lm8333.c
+++ b/drivers/input/keyboard/lm8333.c
@@ -178,6 +178,8 @@ static int lm8333_probe(struct i2c_client *client)
dev_warn(&client->dev, "Unable to set active time\n");
}
+ lm8333_irq_thread(client->irq, lm8333);
+
err = request_threaded_irq(client->irq, NULL, lm8333_irq_thread,
IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
"lm8333", lm8333);
--
2.40.1
^ permalink raw reply related
* Re: [PATCH 3/4] ARM/mmc: Convert old mmci-omap to GPIO descriptors
From: Linus Walleij @ 2023-05-03 12:30 UTC (permalink / raw)
To: Ulf Hansson
Cc: Aaro Koskinen, Janusz Krzysztofik, Tony Lindgren, Russell King,
Daniel Mack, Haojian Zhuang, Robert Jarzmik, Thomas Bogendoerfer,
Dmitry Torokhov, Mark Brown, Bartosz Golaszewski, Andreas Kemnade,
Helge Deller, linux-omap, linux-arm-kernel, linux-kernel,
linux-mips, linux-input, linux-spi, linux-fbdev, dri-devel,
linux-mmc
In-Reply-To: <CAPDyKFoPa3PZGFBO4njSfEd6H0rdc6KKwPFvE1x6Xx13thKv8g@mail.gmail.com>
On Tue, May 2, 2023 at 4:26 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On Sun, 30 Apr 2023 at 11:22, Linus Walleij <linus.walleij@linaro.org> wrote:
> > Fixes: 92bf78b33b0b ("gpio: omap: use dynamic allocation of base")
> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>
> This looks like it's best funneled through the soc maintainer's tree(s), right?
>
> Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
Thanks, yeah the plan is to wait and see if I get some testing from
the OMAP1/2/3
guys and then collect the lot and put on a branch to SoC unless Tony wants
the job :D
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v4] Fix freeze in lm8333 i2c keyboard driver
From: Tomáš Mudruňka @ 2023-05-03 8:54 UTC (permalink / raw)
To: Jeff LaBundy; +Cc: dmitry.torokhov, linux-input, linux-kernel
In-Reply-To: <ZFHOwEu6tKDjoPA4@nixie71>
> Just to clarify, my stance is that this call should go _before_ the handler
> is registered.
Ok, i will fix the patch later today.
> Any events that come before then are off the table, and this is OK because
> user space isn't going to start consuming key events until well after this
> driver has probed anyway.
Well, that was never my point. I don't care about capturing events
that happen before driver was properly loaded.
My only concern was to limit possibility of deadlock which happened previously.
Because that makes device unusable till the IC is power cycled.
Which might be especially annoying on devices that have power button
implemented using this exact IC :-)
> The reason behind my assertion is that as a matter of best practice, you
> should not have two asynchronous threads that can in theory access the same
> register.
Yeah, this makes bit more sense now. Didn't realized IRQ might
interrupt that lm8333_irq_thread() call immediately.
While not very likely to cause problems like deadlock of the driver, i
think it's a valid point.
After all this is what happens with IRQF_ONESHOT anyway right? Each
time the IRQ is triggered it's disabled, lm8333_irq_thread() is run
and then it's enabled immediately after that. So i guess the behaviour
on each keypress is very similar to calling lm8333_irq_thread() before
registering the IRQ handler, which gives me some confidence there
might not be huge chance for deadlock under normal circumstances.
Though i wonder what would happen if some EMI burst would create rapid
train of randomly timed keypresses, that might just hit the
unfortunate sweetspot after while... Might test that later in the lab,
since i really need the software to remain operational after such
condition had passed.
^ permalink raw reply
* [dtor-input:next] BUILD SUCCESS 3516fa162a01f6611c3c129ce9529bdc720d36b7
From: kernel test robot @ 2023-05-03 8:38 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
branch HEAD: 3516fa162a01f6611c3c129ce9529bdc720d36b7 Input: avoid calling input_set_abs_val() in the event handling core
elapsed time: 725m
configs tested: 217
configs skipped: 24
The following configs have been built successfully.
More configs may be tested in the coming days.
tested configs:
alpha allyesconfig gcc
alpha buildonly-randconfig-r001-20230501 gcc
alpha defconfig gcc
alpha randconfig-r034-20230430 gcc
arc allyesconfig gcc
arc buildonly-randconfig-r001-20230430 gcc
arc buildonly-randconfig-r002-20230430 gcc
arc buildonly-randconfig-r003-20230430 gcc
arc buildonly-randconfig-r004-20230430 gcc
arc defconfig gcc
arc randconfig-r003-20230502 gcc
arc randconfig-r004-20230501 gcc
arc randconfig-r011-20230430 gcc
arc randconfig-r022-20230502 gcc
arc randconfig-r032-20230501 gcc
arc randconfig-r043-20230430 gcc
arc randconfig-r043-20230501 gcc
arc randconfig-r043-20230502 gcc
arm allmodconfig gcc
arm allyesconfig gcc
arm buildonly-randconfig-r002-20230430 gcc
arm defconfig gcc
arm randconfig-r016-20230501 gcc
arm randconfig-r023-20230430 gcc
arm randconfig-r031-20230501 clang
arm randconfig-r033-20230430 clang
arm randconfig-r035-20230502 gcc
arm randconfig-r046-20230430 gcc
arm randconfig-r046-20230501 gcc
arm randconfig-r046-20230502 clang
arm64 allyesconfig gcc
arm64 buildonly-randconfig-r003-20230430 gcc
arm64 buildonly-randconfig-r004-20230430 gcc
arm64 defconfig gcc
arm64 randconfig-r005-20230430 gcc
arm64 randconfig-r011-20230501 clang
arm64 randconfig-r032-20230430 gcc
csky buildonly-randconfig-r004-20230502 gcc
csky defconfig gcc
csky randconfig-r003-20230501 gcc
csky randconfig-r006-20230502 gcc
csky randconfig-r016-20230430 gcc
csky randconfig-r034-20230502 gcc
hexagon buildonly-randconfig-r006-20230430 clang
hexagon randconfig-r023-20230502 clang
hexagon randconfig-r031-20230502 clang
hexagon randconfig-r032-20230502 clang
hexagon randconfig-r041-20230430 clang
hexagon randconfig-r041-20230501 clang
hexagon randconfig-r041-20230502 clang
hexagon randconfig-r045-20230430 clang
hexagon randconfig-r045-20230501 clang
hexagon randconfig-r045-20230502 clang
i386 allyesconfig gcc
i386 debian-10.3 gcc
i386 defconfig gcc
i386 randconfig-a001-20230501 gcc
i386 randconfig-a002-20230501 gcc
i386 randconfig-a003-20230501 gcc
i386 randconfig-a004-20230501 gcc
i386 randconfig-a005-20230501 gcc
i386 randconfig-a006-20230501 gcc
i386 randconfig-a011-20230501 clang
i386 randconfig-a012-20230501 clang
i386 randconfig-a013-20230501 clang
i386 randconfig-a014-20230501 clang
i386 randconfig-a015-20230501 clang
i386 randconfig-a016-20230501 clang
i386 randconfig-r034-20230501 gcc
ia64 allmodconfig gcc
ia64 buildonly-randconfig-r001-20230501 gcc
ia64 buildonly-randconfig-r003-20230501 gcc
ia64 defconfig gcc
ia64 randconfig-r015-20230430 gcc
ia64 randconfig-r021-20230501 gcc
ia64 randconfig-r023-20230430 gcc
ia64 randconfig-r025-20230430 gcc
ia64 randconfig-r036-20230501 gcc
loongarch allmodconfig gcc
loongarch allnoconfig gcc
loongarch buildonly-randconfig-r002-20230501 gcc
loongarch buildonly-randconfig-r006-20230501 gcc
loongarch defconfig gcc
loongarch randconfig-r013-20230430 gcc
loongarch randconfig-r014-20230430 gcc
loongarch randconfig-r016-20230502 gcc
loongarch randconfig-r022-20230501 gcc
loongarch randconfig-r024-20230430 gcc
loongarch randconfig-r025-20230430 gcc
loongarch randconfig-r025-20230501 gcc
loongarch randconfig-r032-20230501 gcc
loongarch randconfig-r036-20230502 gcc
m68k allmodconfig gcc
m68k buildonly-randconfig-r005-20230502 gcc
m68k defconfig gcc
m68k randconfig-r001-20230501 gcc
m68k randconfig-r001-20230502 gcc
m68k randconfig-r006-20230502 gcc
m68k randconfig-r026-20230430 gcc
m68k randconfig-r031-20230501 gcc
m68k randconfig-r032-20230430 gcc
m68k randconfig-r033-20230501 gcc
m68k randconfig-r033-20230502 gcc
m68k randconfig-r036-20230430 gcc
microblaze buildonly-randconfig-r005-20230501 gcc
microblaze buildonly-randconfig-r006-20230501 gcc
microblaze buildonly-randconfig-r006-20230502 gcc
microblaze randconfig-r022-20230430 gcc
microblaze randconfig-r036-20230502 gcc
mips allmodconfig gcc
mips allyesconfig gcc
mips buildonly-randconfig-r002-20230502 gcc
mips randconfig-r006-20230501 clang
mips randconfig-r023-20230501 gcc
mips randconfig-r036-20230430 clang
nios2 buildonly-randconfig-r005-20230430 gcc
nios2 buildonly-randconfig-r006-20230430 gcc
nios2 defconfig gcc
nios2 randconfig-r015-20230501 gcc
nios2 randconfig-r021-20230430 gcc
nios2 randconfig-r022-20230501 gcc
nios2 randconfig-r031-20230430 gcc
openrisc buildonly-randconfig-r005-20230430 gcc
openrisc randconfig-r004-20230502 gcc
openrisc randconfig-r013-20230501 gcc
openrisc randconfig-r014-20230502 gcc
openrisc randconfig-r024-20230502 gcc
openrisc randconfig-r035-20230502 gcc
parisc defconfig gcc
parisc randconfig-r003-20230430 gcc
parisc randconfig-r003-20230502 gcc
parisc randconfig-r015-20230430 gcc
parisc randconfig-r015-20230502 gcc
parisc randconfig-r024-20230501 gcc
parisc randconfig-r035-20230430 gcc
parisc64 defconfig gcc
powerpc allmodconfig gcc
powerpc allnoconfig gcc
powerpc buildonly-randconfig-r004-20230501 clang
powerpc randconfig-r012-20230502 gcc
powerpc randconfig-r014-20230501 clang
powerpc randconfig-r024-20230501 clang
powerpc randconfig-r026-20230501 clang
riscv allmodconfig gcc
riscv allnoconfig gcc
riscv buildonly-randconfig-r001-20230502 gcc
riscv defconfig gcc
riscv randconfig-r002-20230430 gcc
riscv randconfig-r002-20230502 clang
riscv randconfig-r004-20230430 gcc
riscv randconfig-r004-20230502 clang
riscv randconfig-r011-20230502 gcc
riscv randconfig-r025-20230501 clang
riscv randconfig-r042-20230430 clang
riscv randconfig-r042-20230501 clang
riscv randconfig-r042-20230502 gcc
riscv rv32_defconfig gcc
s390 allmodconfig gcc
s390 allyesconfig gcc
s390 buildonly-randconfig-r004-20230501 clang
s390 defconfig gcc
s390 randconfig-r011-20230430 clang
s390 randconfig-r015-20230502 gcc
s390 randconfig-r016-20230430 clang
s390 randconfig-r044-20230430 clang
s390 randconfig-r044-20230501 clang
s390 randconfig-r044-20230502 gcc
sh allmodconfig gcc
sh randconfig-r005-20230502 gcc
sh randconfig-r013-20230502 gcc
sh randconfig-r014-20230502 gcc
sh randconfig-r022-20230430 gcc
sh randconfig-r026-20230501 gcc
sh randconfig-r035-20230501 gcc
sparc defconfig gcc
sparc randconfig-r012-20230430 gcc
sparc randconfig-r012-20230502 gcc
sparc randconfig-r016-20230501 gcc
sparc randconfig-r032-20230502 gcc
sparc randconfig-r034-20230501 gcc
sparc64 randconfig-r001-20230430 gcc
sparc64 randconfig-r002-20230501 gcc
sparc64 randconfig-r005-20230502 gcc
sparc64 randconfig-r011-20230501 gcc
sparc64 randconfig-r013-20230501 gcc
sparc64 randconfig-r014-20230430 gcc
sparc64 randconfig-r014-20230501 gcc
sparc64 randconfig-r021-20230430 gcc
sparc64 randconfig-r021-20230501 gcc
um i386_defconfig gcc
um x86_64_defconfig gcc
x86_64 allnoconfig gcc
x86_64 allyesconfig gcc
x86_64 defconfig gcc
x86_64 kexec gcc
x86_64 randconfig-a001-20230501 gcc
x86_64 randconfig-a002-20230501 gcc
x86_64 randconfig-a003-20230501 gcc
x86_64 randconfig-a004-20230501 gcc
x86_64 randconfig-a005-20230501 gcc
x86_64 randconfig-a006-20230501 gcc
x86_64 randconfig-a011-20230501 clang
x86_64 randconfig-a012-20230501 clang
x86_64 randconfig-a013-20230501 clang
x86_64 randconfig-a014-20230501 clang
x86_64 randconfig-a015-20230501 clang
x86_64 randconfig-a016-20230501 clang
x86_64 randconfig-r005-20230501 gcc
x86_64 randconfig-r012-20230501 clang
x86_64 rhel-8.3 gcc
xtensa randconfig-r011-20230502 gcc
xtensa randconfig-r012-20230501 gcc
xtensa randconfig-r021-20230502 gcc
xtensa randconfig-r033-20230430 gcc
xtensa randconfig-r033-20230502 gcc
xtensa randconfig-r034-20230430 gcc
xtensa randconfig-r035-20230501 gcc
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply
* Re: [PATCH 0/2] Input: tests - miscellaneous fixes
From: Javier Martinez Canillas @ 2023-05-03 7:05 UTC (permalink / raw)
To: Geert Uytterhoeven, Dmitry Torokhov
Cc: Brendan Higgins, David Gow, linux-input, linux-kselftest,
kunit-dev, linux-kernel
In-Reply-To: <CAMuHMdW_K9R7L9M_=M+HyWKivK6S2_Bhb5jvwdGv_oqZ06-NxA@mail.gmail.com>
Geert Uytterhoeven <geert@linux-m68k.org> writes:
> Hi Dmitry,
>
> On Tue, May 2, 2023 at 7:05 PM Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
>> On Tue, May 02, 2023 at 06:31:51PM +0200, Javier Martinez Canillas wrote:
>> > Geert Uytterhoeven <geert@linux-m68k.org> writes:
>> > > On Tue, May 2, 2023 at 12:17 PM Geert Uytterhoeven
>> > > <geert+renesas@glider.be> wrote:
>> > >> This patch series fixes a crash in the new input selftest, and makes the
>> > >> test available when the KUnit framework is modular.
>> > >>
>> > >> Unfortunately test 3 still fails for me (tested on Koelsch (R-Car M2-W)
>> > >> and ARAnyM):
>> > >>
>> > >> KTAP version 1
>> > >> # Subtest: input_core
>> > >> 1..3
>> > >> input: Test input device as /devices/virtual/input/input1
>> > >> ok 1 input_test_polling
>> > >> input: Test input device as /devices/virtual/input/input2
>> > >> ok 2 input_test_timestamp
>> > >> input: Test input device as /devices/virtual/input/input3
>> > >> # input_test_match_device_id: ASSERTION FAILED at # drivers/input/tests/input_test.c:99
>> > >> Expected input_match_device_id(input_dev, &id) to be true, but is false
>> > >> not ok 3 input_test_match_device_id
>> > >> # input_core: pass:2 fail:1 skip:0 total:3
>> > >> # Totals: pass:2 fail:1 skip:0 total:3
>> > >> not ok 1 input_core
>> > >
>> > > Adding more debug code shows that it's the test on evbit [1] in
>> > > input_match_device_id() that fails.
>> > > Looking at your input_test_match_device_id(), I think you expect
>> > > the checks for the various bitmaps to be gated by
>> > > "if (id->flags & INPUT_DEVICE_ID_MATCH_EVBIT)", like is done for the
>> > > other checks?
>> > >
>> > > [1] https://elixir.bootlin.com/linux/latest/source/drivers/input/input.c#L1021
>> > >
>> >
>> > That's correct. In input_test_init(), the input dev is marked as capable
>> > of emitting EV_KEY BTN_LEFT and BTN_RIGHT. The goal of that test was to
>> > check this.
>> >
>> > That is, check if matches by the input dev capabilities in which case the
>> > __set_bit(EV_KEY, ...) would make the match true and __set_bit(EV_ABS, ..)
>> > would make the condition false.
>> >
>> > But maybe I misunderstood how the input_set_capability() and __set_bit()
>> > functions work ?
>> >
>> > I'll take a look to this tomorrow, thanks a lot for your report!
>>
>> Unfortunately (?) INPUT_DEVICE_ID_MATCH_*BIT have never had any effect,
>> the kernel always used bitmaps when matching (with the assumption that
>> if one does not care about given bitmap they can simply pass empty one),
>> so I think what we need to change is:
>>
>> diff --git a/drivers/input/tests/input_test.c b/drivers/input/tests/input_test.c
>> index 8b8ac3412a70..0540225f0288 100644
>> --- a/drivers/input/tests/input_test.c
>> +++ b/drivers/input/tests/input_test.c
>> @@ -87,7 +87,7 @@ static void input_test_timestamp(struct kunit *test)
>> static void input_test_match_device_id(struct kunit *test)
>> {
>> struct input_dev *input_dev = test->priv;
>> - struct input_device_id id;
>> + struct input_device_id id = { 0 };
>>
>> /*
>> * Must match when the input device bus, vendor, product, version
>>
>> to avoid having garbage in the match data.
>
> Thanks, that did the trick! 3/3 tests pass.
>
Oh, silly me. Are you going to post that as a proper patch ?
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
^ permalink raw reply
* Re: [PATCH 0/2] Input: tests - miscellaneous fixes
From: Geert Uytterhoeven @ 2023-05-03 6:53 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Javier Martinez Canillas, Brendan Higgins, David Gow, linux-input,
linux-kselftest, kunit-dev, linux-kernel
In-Reply-To: <ZFFCzHwJqyeXB52w@google.com>
Hi Dmitry,
On Tue, May 2, 2023 at 7:05 PM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Tue, May 02, 2023 at 06:31:51PM +0200, Javier Martinez Canillas wrote:
> > Geert Uytterhoeven <geert@linux-m68k.org> writes:
> > > On Tue, May 2, 2023 at 12:17 PM Geert Uytterhoeven
> > > <geert+renesas@glider.be> wrote:
> > >> This patch series fixes a crash in the new input selftest, and makes the
> > >> test available when the KUnit framework is modular.
> > >>
> > >> Unfortunately test 3 still fails for me (tested on Koelsch (R-Car M2-W)
> > >> and ARAnyM):
> > >>
> > >> KTAP version 1
> > >> # Subtest: input_core
> > >> 1..3
> > >> input: Test input device as /devices/virtual/input/input1
> > >> ok 1 input_test_polling
> > >> input: Test input device as /devices/virtual/input/input2
> > >> ok 2 input_test_timestamp
> > >> input: Test input device as /devices/virtual/input/input3
> > >> # input_test_match_device_id: ASSERTION FAILED at # drivers/input/tests/input_test.c:99
> > >> Expected input_match_device_id(input_dev, &id) to be true, but is false
> > >> not ok 3 input_test_match_device_id
> > >> # input_core: pass:2 fail:1 skip:0 total:3
> > >> # Totals: pass:2 fail:1 skip:0 total:3
> > >> not ok 1 input_core
> > >
> > > Adding more debug code shows that it's the test on evbit [1] in
> > > input_match_device_id() that fails.
> > > Looking at your input_test_match_device_id(), I think you expect
> > > the checks for the various bitmaps to be gated by
> > > "if (id->flags & INPUT_DEVICE_ID_MATCH_EVBIT)", like is done for the
> > > other checks?
> > >
> > > [1] https://elixir.bootlin.com/linux/latest/source/drivers/input/input.c#L1021
> > >
> >
> > That's correct. In input_test_init(), the input dev is marked as capable
> > of emitting EV_KEY BTN_LEFT and BTN_RIGHT. The goal of that test was to
> > check this.
> >
> > That is, check if matches by the input dev capabilities in which case the
> > __set_bit(EV_KEY, ...) would make the match true and __set_bit(EV_ABS, ..)
> > would make the condition false.
> >
> > But maybe I misunderstood how the input_set_capability() and __set_bit()
> > functions work ?
> >
> > I'll take a look to this tomorrow, thanks a lot for your report!
>
> Unfortunately (?) INPUT_DEVICE_ID_MATCH_*BIT have never had any effect,
> the kernel always used bitmaps when matching (with the assumption that
> if one does not care about given bitmap they can simply pass empty one),
> so I think what we need to change is:
>
> diff --git a/drivers/input/tests/input_test.c b/drivers/input/tests/input_test.c
> index 8b8ac3412a70..0540225f0288 100644
> --- a/drivers/input/tests/input_test.c
> +++ b/drivers/input/tests/input_test.c
> @@ -87,7 +87,7 @@ static void input_test_timestamp(struct kunit *test)
> static void input_test_match_device_id(struct kunit *test)
> {
> struct input_dev *input_dev = test->priv;
> - struct input_device_id id;
> + struct input_device_id id = { 0 };
>
> /*
> * Must match when the input device bus, vendor, product, version
>
> to avoid having garbage in the match data.
Thanks, that did the trick! 3/3 tests pass.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* [PATCH 3/3] HID: asus: reformat the hotkey mapping block
From: Luke D. Jones @ 2023-05-03 3:47 UTC (permalink / raw)
To: linux-input
Cc: jikos, benjamin.tissoires, rydberg, linux-kernel, Luke D. Jones
In-Reply-To: <20230503034711.253439-1-luke@ljones.dev>
Older formatting of this block was beginning to get somewhat cluttered.
Condensing the block and putting comments to the side makes it easier
to read and scan the scancodes plus keycodes.
Signed-off-by: Luke D. Jones <luke@ljones.dev>
---
drivers/hid/hid-asus.c | 44 ++++++++++++------------------------------
1 file changed, 12 insertions(+), 32 deletions(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 918d0d05ca88..2bc14e076739 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -885,38 +885,18 @@ static int asus_input_mapping(struct hid_device *hdev,
case 0xc5: asus_map_key_clear(KEY_KBDILLUMDOWN); break;
case 0xc7: asus_map_key_clear(KEY_KBDILLUMTOGGLE); break;
- /* ASUS touchpad toggle */
- case 0x6b: asus_map_key_clear(KEY_F21); break;
+ case 0x6b: asus_map_key_clear(KEY_F21); break; /* ASUS touchpad toggle */
+ case 0x38: asus_map_key_clear(KEY_PROG1); break; /* ROG key */
+ case 0xba: asus_map_key_clear(KEY_PROG2); break; /* Fn+C ASUS Splendid */
+ case 0x5c: asus_map_key_clear(KEY_PROG3); break; /* Fn+Space Power4Gear */
+ case 0x99: asus_map_key_clear(KEY_PROG4); break; /* Fn+F5 "fan" symbol */
+ case 0xae: asus_map_key_clear(KEY_PROG4); break; /* Fn+F5 "fan" symbol */
+ case 0x92: asus_map_key_clear(KEY_CALC); break; /* Fn+Ret "Calc" symbol */
+ case 0xb2: asus_map_key_clear(KEY_PROG2); break; /* Fn+Left previous aura */
+ case 0xb3: asus_map_key_clear(KEY_PROG3); break; /* Fn+Left next aura */
+ case 0x6a: asus_map_key_clear(KEY_F13); break; /* Screenpad toggle */
+ case 0x4b: asus_map_key_clear(KEY_F14); break; /* Arrows/Pg-Up/Dn toggle */
- /* ROG key */
- case 0x38: asus_map_key_clear(KEY_PROG1); break;
-
- /* Fn+C ASUS Splendid */
- case 0xba: asus_map_key_clear(KEY_PROG2); break;
-
- /* Fn+Space Power4Gear Hybrid */
- case 0x5c: asus_map_key_clear(KEY_PROG3); break;
-
- /* Fn+F5 "fan" symbol on FX503VD */
- case 0x99: asus_map_key_clear(KEY_PROG4); break;
-
- /* Fn+F5 "fan" symbol on N-Key keyboard */
- case 0xae: asus_map_key_clear(KEY_PROG4); break;
-
- /* Fn+Ret "Calc" symbol on N-Key keyboard */
- case 0x92: asus_map_key_clear(KEY_CALC); break;
-
- /* Fn+Left Aura mode previous on N-Key keyboard */
- case 0xb2: asus_map_key_clear(KEY_PROG2); break;
-
- /* Fn+Right Aura mode next on N-Key keyboard */
- case 0xb3: asus_map_key_clear(KEY_PROG3); break;
-
- /* Screenpad toggle on N-Key keyboard */
- case 0x6a: asus_map_key_clear(KEY_F13); break;
-
- /* Arrows/Page-up/Down toggle on N-Key keyboard */
- case 0x4b: asus_map_key_clear(KEY_F14); break;
default:
/* ASUS lazily declares 256 usages, ignore the rest,
@@ -1319,4 +1299,4 @@ static struct hid_driver asus_driver = {
};
module_hid_driver(asus_driver);
-MODULE_LICENSE("GPL");
+MODULE_LICENSE("GPL");
\ No newline at end of file
--
2.40.0
^ permalink raw reply related
* [PATCH 2/3] HID: asus: add keycodes for 0x6a, 0x4b, and 0xc7
From: Luke D. Jones @ 2023-05-03 3:47 UTC (permalink / raw)
To: linux-input
Cc: jikos, benjamin.tissoires, rydberg, linux-kernel, Luke D. Jones
In-Reply-To: <20230503034711.253439-1-luke@ljones.dev>
These two keys are found on some models with dual display.
- 0x6a is intended for controlling the secondary screen brightness.
- 0x4b is intended for toggling the arrow keys between arrows and
page up / page down.
This key is found on a slightly modified keyboard layout.
- 0xc7 is intended to cycle through keybvoard brightnesses (upwards) but
there is not suitable existing code for this behaviour. Using
`KEY_KBDILLUMTOGGLE` is different behaviour to Windows but at least
is picked up by many desktops already.
Signed-off-by: Luke D. Jones <luke@ljones.dev>
---
drivers/hid/hid-asus.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index ac93f987d822..918d0d05ca88 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -883,6 +883,7 @@ static int asus_input_mapping(struct hid_device *hdev,
case 0xb5: asus_map_key_clear(KEY_CALC); break;
case 0xc4: asus_map_key_clear(KEY_KBDILLUMUP); break;
case 0xc5: asus_map_key_clear(KEY_KBDILLUMDOWN); break;
+ case 0xc7: asus_map_key_clear(KEY_KBDILLUMTOGGLE); break;
/* ASUS touchpad toggle */
case 0x6b: asus_map_key_clear(KEY_F21); break;
@@ -911,6 +912,12 @@ static int asus_input_mapping(struct hid_device *hdev,
/* Fn+Right Aura mode next on N-Key keyboard */
case 0xb3: asus_map_key_clear(KEY_PROG3); break;
+ /* Screenpad toggle on N-Key keyboard */
+ case 0x6a: asus_map_key_clear(KEY_F13); break;
+
+ /* Arrows/Page-up/Down toggle on N-Key keyboard */
+ case 0x4b: asus_map_key_clear(KEY_F14); break;
+
default:
/* ASUS lazily declares 256 usages, ignore the rest,
* as some make the keyboard appear as a pointer device. */
--
2.40.0
^ permalink raw reply related
* [PATCH 1/3] HID: asus: Add support for ASUS ROG Z13 keyboard
From: Luke D. Jones @ 2023-05-03 3:47 UTC (permalink / raw)
To: linux-input
Cc: jikos, benjamin.tissoires, rydberg, linux-kernel, Luke D. Jones
Add support for the ROG Z13 keyboard. This is a variant of the last
few ASUS ROG keyboards and has much of the same functionality.
Signed-off-by: Luke D. Jones <luke@ljones.dev>
---
drivers/hid/hid-asus.c | 3 +++
drivers/hid/hid-ids.h | 1 +
2 files changed, 4 insertions(+)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index d1094bb1aa42..ac93f987d822 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -1267,6 +1267,9 @@ static const struct hid_device_id asus_devices[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD2),
QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
+ { HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
+ USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD3),
+ QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
USB_DEVICE_ID_ASUSTEK_ROG_CLAYMORE_II_KEYBOARD),
QUIRK_ROG_CLAYMORE_II_KEYBOARD },
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index c2e9b6d1fd7d..513290a2e91c 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -207,6 +207,7 @@
#define USB_DEVICE_ID_ASUSTEK_ROG_KEYBOARD3 0x1822
#define USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD 0x1866
#define USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD2 0x19b6
+#define USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD3 0x1a30
#define USB_DEVICE_ID_ASUSTEK_ROG_CLAYMORE_II_KEYBOARD 0x196b
#define USB_DEVICE_ID_ASUSTEK_FX503VD_KEYBOARD 0x1869
--
2.40.0
^ permalink raw reply related
* Re: [PATCH v4] Fix freeze in lm8333 i2c keyboard driver
From: Jeff LaBundy @ 2023-05-03 3:02 UTC (permalink / raw)
To: Tomas Mudrunka; +Cc: dmitry.torokhov, linux-input, linux-kernel
In-Reply-To: <20230428102015.810686-1-tomas.mudrunka@gmail.com>
Hi Tomas,
On Fri, Apr 28, 2023 at 12:20:15PM +0200, Tomas Mudrunka wrote:
> LM8333 uses gpio interrupt line which is triggered by falling edge.
> When button is pressed before driver is loaded,
> driver will miss the edge and never respond again.
> To fix this we run the interrupt handler after registering IRQ
> to clear the interrupt via i2c command.
>
> Signed-off-by: Tomas Mudrunka <tomas.mudrunka@gmail.com>
> ---
> drivers/input/keyboard/lm8333.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/input/keyboard/lm8333.c b/drivers/input/keyboard/lm8333.c
> index 7457c3220..9a810ca00 100644
> --- a/drivers/input/keyboard/lm8333.c
> +++ b/drivers/input/keyboard/lm8333.c
> @@ -184,6 +184,8 @@ static int lm8333_probe(struct i2c_client *client)
> if (err)
> goto free_mem;
>
> + lm8333_irq_thread(client->irq, lm8333);
Just to clarify, my stance is that this call should go _before_ the handler
is registered. Your earlier statement that doing so would steal any pending
status from the handler is correct; however, it is a moot point because the
handler cannot do anything with that status until the input device has been
registered anyway.
Any events that come before then are off the table, and this is OK because
user space isn't going to start consuming key events until well after this
driver has probed anyway.
The reason behind my assertion is that as a matter of best practice, you
should not have two asynchronous threads that can in theory access the same
register. You are correct that the handler would simply return IRQ_NONE in
such a race, but it sets a bad precedent and opens room for bugs in case
this driver is modified in the future. It also creates one unnecessary I2C
read.
This is why it is much more common to register the handler _after_ manually
accessing read-to-clear registers; the register access remains synchronous.
In case you feel I have misunderstood, please let me know.
> +
> err = input_register_device(input);
> if (err)
> goto free_irq;
> --
> 2.40.0
Kind regards,
Jeff LaBundy
^ permalink raw reply
* Re: [RFC PATCH 0/3] input: gpio-keys - fix pm ordering
From: Saravana Kannan @ 2023-05-03 2:18 UTC (permalink / raw)
To: Doug Berger
Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Dmitry Torokhov,
Lad Prabhakar, Gergo Koteles, Jonathan Cameron, Andy Shevchenko,
Dan Williams, Hans de Goede, Thomas Gleixner, Kees Cook,
Kishon Vijay Abraham I, Florian Fainelli, linux-kernel,
linux-input, Android Kernel Team
In-Reply-To: <CAGETcx-gDcqY7-_Ud0_rOtgvk7NbzevSa4UCV=NcqiV9zjAv7w@mail.gmail.com>
On Mon, May 1, 2023 at 1:40 PM Saravana Kannan <saravanak@google.com> wrote:
>
> On Thu, Apr 27, 2023 at 3:18 PM Doug Berger <opendmb@gmail.com> wrote:
> >
> > Commit 52cdbdd49853 ("driver core: correct device's shutdown
> > order") allowed for proper sequencing of the gpio-keys device
> > shutdown callbacks by moving the device to the end of the
> > devices_kset list at probe which was delayed by child
> > dependencies.
> >
> > However, commit 722e5f2b1eec ("driver core: Partially revert
> > "driver core: correct device's shutdown order"") removed this
> > portion of that commit causing a reversion in the gpio-keys
> > behavior which can prevent waking from shutdown.
> >
> > This RFC is an attempt to find a better solution for properly
> > creating gpio-keys device links to ensure its suspend/resume and
> > shutdown services are invoked before those of its suppliers.
> >
> > The first patch here is pretty brute force but simple and has
> > the advantage that it should be easily backportable to the
> > versions where the regression first occurred.
>
> We really shouldn't be calling device_pm_move_to_tail() in drivers
> because device link uses device_pm_move_to_tail() for ordering too.
> And it becomes a "race" between device links and when the driver calls
> device_pm_move_to_tail() and I'm not sure we'll get the same ordering
> every time.
>
> >
> > The second patch is perhaps better in spirit though still a bit
> > inelegant, but it can only be backported to versions of the
> > kernel that contain the commit in its 'Fixes:' tag. That isn't
> > really a valid 'Fixes:' tag since that commit did not cause the
> > regression, but it does represent how far the patch could be
> > backported.
> >
> > Both commits shouldn't really exist in the same kernel so the
> > third patch reverts the first in an attempt to make that clear
> > (though it may be a source of confusion for some).
> >
> > Hopefully someone with a better understanding of device links
> > will see a less intrusive way to automatically capture these
> > dependencies for parent device drivers that implement the
> > functions of child node devices.
>
> Can you give a summary of the issue on a real system? I took a look at
> the two commits you've referenced above and it's not clear what's
> still broken in the 6.3+
>
> But I'd think that just teaching fw_devlink about some property should
> be sufficient. If you are facing a real issue, have you made sure you
> have fw_devlink=on (this is the default unless you turned it off in
> the commandline when it had issues in the past).
>
I took a closer look at how gpio-keys work and I can see why
fw_devlink doesn't pick up the GPIO dependencies. It's because the
gpio dependencies are listed under child "key-x" device nodes under
the main "gpio-keys" device tree node. fw_devlink doesn't consider
dependencies under child nodes as mandatory dependencies of the parent
node.
The main reason for this was because of how fw_devlink used to work.
But I might be able to change fw_devlink to pick this up
automatically. I need to think a bit more about this because in some
cases, ignoring those dependencies is the right thing to do. Give me a
few weeks to think through and experiment with this on my end.
-Saravana
^ permalink raw reply
* Re: [PATCH v2 3/6] dt-bindings: input: cypress,tt21000 - fix interrupt type in dts example
From: Dmitry Torokhov @ 2023-05-02 20:51 UTC (permalink / raw)
To: Maximilian Weigand
Cc: Linus Walleij, Rob Herring, Krzysztof Kozlowski, linux-input,
linux-kernel, devicetree, Alistair Francis
In-Reply-To: <5aaba4d1-f7e2-9d30-5f15-0713f9fc6a8c@gmail.com>
On Tue, May 02, 2023 at 04:23:54PM +0200, Maximilian Weigand wrote:
> Hi,
>
> On 02.05.23 02:24, Dmitry Torokhov wrote:
> > On Mon, May 01, 2023 at 01:30:07PM +0200, Maximilian Weigand wrote:
> >> Triggering the interrupt of the IRQ_TYPE_LEVEL_LOW type can lead to
> >> probing issues with the device for the current driver (encountered on
> >> the Pine64 PineNote). Basically the interrupt would be triggered before
> >> certain commands were sent to the device, leading to a race between the
> >> device responding fast enough and the irq handler fetching a data frame
> >> from it. Actually all devices currently using the driver already use a
> >> falling edge trigger.
> >
> > I'd prefer we adjusted the driver to handle level interrupts properly.
>
> Ok, I will have a look at that. Just to be clear: The driver should work
> only with level interrupts, or should it optimally support both level
> and falling edge triggers?
Optimally a driver would work well with both.
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH] Input: fix open count when closing inhibited device
From: Dmitry Torokhov @ 2023-05-02 20:34 UTC (permalink / raw)
To: linux-input; +Cc: linux-kernel, Peter Hutterer
Because the kernel increments device's open count in input_open_device()
even if device is inhibited, the counter should always be decremented in
input_close_device() to keep it balanced.
Fixes: a181616487db ("Input: Add "inhibited" property")
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/input.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/input.c b/drivers/input/input.c
index f791d14ecf23..8c5fdb0f858a 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -705,7 +705,7 @@ void input_close_device(struct input_handle *handle)
__input_release_device(handle);
- if (!dev->inhibited && !--dev->users) {
+ if (!--dev->users && !dev->inhibited) {
if (dev->poller)
input_dev_poller_stop(dev->poller);
if (dev->close)
--
2.40.1.495.gc816e09b53d-goog
--
Dmitry
^ 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