* Re: [BUG] Kmemleak, possibly hiddev_connect(), in 6.3.0+ torvalds tree commit gfc4354c6e5c2
From: Mirsad Goran Todorovac @ 2023-05-12 19:08 UTC (permalink / raw)
To: Greg Kroah-Hartman, Mirsad Goran Todorovac
Cc: linux-usb, linux-kernel, linux-input, Benjamin Tissoires,
Jiri Kosina
In-Reply-To: <2023050958-precut-vividly-94bf@gregkh>
Hi, Greg,
On 09. 05. 2023. 04:59, Greg Kroah-Hartman wrote:
> On Tue, May 09, 2023 at 01:51:35AM +0200, Mirsad Goran Todorovac wrote:
>>
>>
>> On 08. 05. 2023. 16:01, Greg Kroah-Hartman wrote:
>>> On Mon, May 08, 2023 at 08:51:55AM +0200, Greg Kroah-Hartman wrote:
>>>> On Mon, May 08, 2023 at 08:30:07AM +0200, Mirsad Goran Todorovac wrote:
>>>>> Hi,
>>>>>
>>>>> There seems to be a kernel memory leak in the USB keyboard driver.
>>>>>
>>>>> The leaked memory allocs are 96 and 512 bytes.
>>>>>
>>>>> The platform is Ubuntu 22.04 LTS on a assembled AMD Ryzen 9 with X670E PG
>>>>> Lightning mobo,
>>>>> and Genius SlimStar i220 GK-080012 keyboard.
>>>>>
>>>>> (Logitech M100 HID mouse is not affected by the bug.)
>>>>>
>>>>> BIOS is:
>>>>>
>>>>> *-firmware
>>>>> description: BIOS
>>>>> vendor: American Megatrends International, LLC.
>>>>> physical id: 0
>>>>> version: 1.21
>>>>> date: 04/26/2023
>>>>> size: 64KiB
>>>>>
>>>>> The kernel is 6.3.0-torvalds-<id>-13466-gfc4354c6e5c2.
>>>>>
>>>>> The keyboard is recognised as Chicony:
>>>>>
>>>>> *-usb
>>>>> description: Keyboard
>>>>> product: CHICONY USB Keyboard
>>>>> vendor: CHICONY
>>>>> physical id: 2
>>>>> bus info: usb@5:2
>>>>> logical name: input35
>>>>> logical name: /dev/input/event4
>>>>> logical name: input35::capslock
>>>>> logical name: input35::numlock
>>>>> logical name: input35::scrolllock
>>>>> logical name: input36
>>>>> logical name: /dev/input/event5
>>>>> logical name: input37
>>>>> logical name: /dev/input/event6
>>>>> logical name: input38
>>>>> logical name: /dev/input/event8
>>>>> version: 2.30
>>>>> capabilities: usb-2.00 usb
>>>>> configuration: driver=usbhid maxpower=100mA
>>>>> speed=1Mbit/s
>>>>>
>>>>> The bug is easily reproduced by unplugging the USB keyboard, waiting about a
>>>>> couple of seconds,
>>>>> and then reconnect and scan for memory leaks twice.
>>>>>
>>>>> The kmemleak log is as follows [edited privacy info]:
>>>>>
>>>>> root@hostname:/home/username# cat /sys/kernel/debug/kmemleak
>>>>> unreferenced object 0xffff8dd020037c00 (size 96):
>>>>> comm "systemd-udevd", pid 435, jiffies 4294892550 (age 8909.356s)
>>>>> hex dump (first 32 bytes):
>>>>> 5d 8e 4e b9 ff ff ff ff 00 00 00 00 00 00 00 00 ].N.............
>>>>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
>>>>> backtrace:
>>>>> [<ffffffffb81a74be>] __kmem_cache_alloc_node+0x22e/0x2b0
>>>>> [<ffffffffb8127b6e>] kmalloc_trace+0x2e/0xa0
>>>>> [<ffffffffb87543d9>] class_create+0x29/0x80
>>>>> [<ffffffffb8880d24>] usb_register_dev+0x1d4/0x2e0
>>>>
>>>> As the call to class_create() in this path is now gone in 6.4-rc1, can
>>>> you retry that release to see if this is still there or not?
>>>
>>> No, wait, it's still there, I was looking at a development branch of
>>> mine that isn't sent upstream yet. And syzbot just reported the same
>>> thing:
>>> https://lore.kernel.org/r/00000000000058d15f05fb264013@google.com
>>>
>>> So something's wrong here, let me dig into it tomorrow when I get a
>>> chance...
>>
>> If this could help, here is the bisect of the bug (I could not discern what
>> could possibly be wrong):
>>
>> user@host:~/linux/kernel/linux_torvalds$ git bisect log
>> git bisect start
>> # bad: [ac9a78681b921877518763ba0e89202254349d1b] Linux 6.4-rc1
>> git bisect bad ac9a78681b921877518763ba0e89202254349d1b
>> # good: [c9c3395d5e3dcc6daee66c6908354d47bf98cb0c] Linux 6.2
>> git bisect good c9c3395d5e3dcc6daee66c6908354d47bf98cb0c
>> # good: [85496c9b3bf8dbe15e2433d3a0197954d323cadc] Merge branch
>> 'net-remove-some-rcu_bh-cruft'
>> git bisect good 85496c9b3bf8dbe15e2433d3a0197954d323cadc
>> # good: [b68ee1c6131c540a62ecd443be89c406401df091] Merge tag 'scsi-misc' of
>> git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi
>> git bisect good b68ee1c6131c540a62ecd443be89c406401df091
>> # bad: [888d3c9f7f3ae44101a3fd76528d3dd6f96e9fd0] Merge tag 'sysctl-6.4-rc1'
>> of git://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux
>> git bisect bad 888d3c9f7f3ae44101a3fd76528d3dd6f96e9fd0
>> # good: [34b62f186db9614e55d021f8c58d22fc44c57911] Merge tag
>> 'pci-v6.4-changes' of git://git.kernel.org/pub/scm/linux/kernel/git/pci/pci
>> git bisect good 34b62f186db9614e55d021f8c58d22fc44c57911
>> # good: [34da76dca4673ab1819830b4924bb5b436325b26] Merge tag
>> 'for-linus-2023042601' of
>> git://git.kernel.org/pub/scm/linux/kernel/git/hid/hid
>> git bisect good 34da76dca4673ab1819830b4924bb5b436325b26
>> # good: [97b2ff294381d05e59294a931c4db55276470cb5] Merge tag
>> 'staging-6.4-rc1' of
>> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging
>> git bisect good 97b2ff294381d05e59294a931c4db55276470cb5
>> # good: [2025b2ca8004c04861903d076c67a73a0ec6dfca] mcb-lpc: Reallocate
>> memory region to avoid memory overlapping
>> git bisect good 2025b2ca8004c04861903d076c67a73a0ec6dfca
>> # bad: [d06f5a3f7140921ada47d49574ae6fa4de5e2a89] cdx: fix build failure due
>> to sysfs 'bus_type' argument needing to be const
>> git bisect bad d06f5a3f7140921ada47d49574ae6fa4de5e2a89
>> # good: [dcfbb67e48a2becfce7990386e985b9c45098ee5] driver core: class: use
>> lock_class_key already present in struct subsys_private
>> git bisect good dcfbb67e48a2becfce7990386e985b9c45098ee5
>> # bad: [6f14c02220c791d5c46b0f965b9340c58f3d503d] driver core: create
>> class_is_registered()
>> git bisect bad 6f14c02220c791d5c46b0f965b9340c58f3d503d
>> # good: [2f9e87f5a2941b259336c7ea6c5a1499ede4554a] driver core: Add a
>> comment to set_primary_fwnode() on nullifying
>> git bisect good 2f9e87f5a2941b259336c7ea6c5a1499ede4554a
>> # bad: [02fe26f25325b547b7a31a65deb0326c04bb5174] firmware_loader: Add debug
>> message with checksum for FW file
>> git bisect bad 02fe26f25325b547b7a31a65deb0326c04bb5174
>> # good: [884f8ce42ccec9d0bf11d8bf9f111e5961ca1c82] driver core: class:
>> implement class_get/put without the private pointer.
>> git bisect good 884f8ce42ccec9d0bf11d8bf9f111e5961ca1c82
>> # bad: [3f84aa5ec052dba960baca4ab8a352d43d47028e] base: soc: populate
>> machine name in soc_device_register if empty
>> git bisect bad 3f84aa5ec052dba960baca4ab8a352d43d47028e
>> # bad: [7b884b7f24b42fa25e92ed724ad82f137610afaf] driver core: class.c:
>> convert to only use class_to_subsys
>> git bisect bad 7b884b7f24b42fa25e92ed724ad82f137610afaf
>> # first bad commit: [7b884b7f24b42fa25e92ed724ad82f137610afaf] driver core:
>> class.c: convert to only use class_to_subsys
>> user@host:~/linux/kernel/linux_torvalds$
>
> This helps a lot, thanks. I got the reference counting wrong somewhere
> in here, I thought I tested this better, odd it shows up now...
>
> I'll try to work on it this week.
>
> thanks,
>
> greg k-h
Not at all!
I hope you had better luck because this part of code still looks to me
like hieroglyphs.
Linux kernel rose to 10.9M lines, and it would take me thirty years to
just read it once, 1000 lines a day ... 6.7M lines are "just drivers".
# find . -name '*.c' -o -name '*.h' -print0 | wc --files0-from -
10913623 35587483 631377958 total
# find drivers -name '*.c' -o -name '*.h' -print0 | wc --files0-from -
6705084 19985060 495162001 total
Best regards,
Mirsad
^ permalink raw reply
* [PATCH] Input: pwm-beeper - Support volume setting via sysfs
From: Marek Vasut @ 2023-05-12 18:55 UTC (permalink / raw)
To: linux-input
Cc: Marek Vasut, Uwe Kleine-König, Dmitry Torokhov,
Frieder Schrempf, Manuel Traut, Thierry Reding, linux-pwm
The PWM beeper volume can be controlled by adjusting the PWM duty cycle,
expose volume setting via sysfs, so users can make the beeper quieter.
This patch adds sysfs attribute 'volume' in range 0..50000, i.e. from 0
to 50% in 1/1000th of percent steps, this resolution should be sufficient.
The reason for 50000 cap on volume or PWM duty cycle is because duty cycle
above 50% again reduces the loudness, the PWM wave form is inverted wave
form of the one for duty cycle below 50% and the beeper gets quieter the
closer the setting is to 100% . Hence, 50% cap where the wave form yields
the loudest result.
Signed-off-by: Marek Vasut <marex@denx.de>
---
An alternative option would be to extend the userspace input ABI, e.g. by
using SND_TONE top 16bits to encode the duty cycle in 0..50000 range, and
bottom 16bit to encode the existing frequency in Hz . Since frequency in
Hz is likely to be below some 25 kHz for audible bell, this fits in 16bits
just fine. Thoughts ?
---
NOTE: This uses approach similar to [1], except it is much simpler.
[1] https://patchwork.kernel.org/project/linux-input/cover/20230201152128.614439-1-manuel.traut@mt.com/
---
Cc: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Frieder Schrempf <frieder.schrempf@kontron.de>
Cc: Manuel Traut <manuel.traut@mt.com>
Cc: Marek Vasut <marex@denx.de>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: linux-input@vger.kernel.org
Cc: linux-pwm@vger.kernel.org
---
drivers/input/misc/pwm-beeper.c | 58 ++++++++++++++++++++++++++++++++-
1 file changed, 57 insertions(+), 1 deletion(-)
diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
index 3cf1812384e6a..f63d0ebbaf573 100644
--- a/drivers/input/misc/pwm-beeper.c
+++ b/drivers/input/misc/pwm-beeper.c
@@ -21,6 +21,7 @@ struct pwm_beeper {
struct regulator *amplifier;
struct work_struct work;
unsigned long period;
+ unsigned long duty_cycle;
unsigned int bell_frequency;
bool suspended;
bool amplifier_on;
@@ -37,7 +38,7 @@ static int pwm_beeper_on(struct pwm_beeper *beeper, unsigned long period)
state.enabled = true;
state.period = period;
- pwm_set_relative_duty_cycle(&state, 50, 100);
+ pwm_set_relative_duty_cycle(&state, beeper->duty_cycle, 100000);
error = pwm_apply_state(beeper->pwm, &state);
if (error)
@@ -119,6 +120,53 @@ static void pwm_beeper_close(struct input_dev *input)
pwm_beeper_stop(beeper);
}
+static ssize_t volume_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct pwm_beeper *beeper = dev_get_drvdata(dev);
+
+ return sysfs_emit(buf, "%ld\n", beeper->duty_cycle);
+}
+
+static ssize_t volume_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct pwm_beeper *beeper = dev_get_drvdata(dev);
+ unsigned long val;
+
+ if (kstrtoul(buf, 0, &val) < 0)
+ return -EINVAL;
+
+ /*
+ * Volume is really PWM duty cycle in pcm (per cent mille, 1/1000th
+ * of percent). This value therefore ranges from 0 to 50000 . Duty
+ * cycle of 50% = 50000pcm is the maximum volume .
+ */
+ val = clamp(val, 0UL, 50000UL);
+ /* No change in current settings, do nothing. */
+ if (val == beeper->duty_cycle)
+ return count;
+
+ /* Update current settings and apply to PWM chip. */
+ beeper->duty_cycle = val;
+ if (!beeper->suspended)
+ schedule_work(&beeper->work);
+
+ return count;
+}
+static DEVICE_ATTR_RW(volume);
+
+static struct attribute *pwm_beeper_dev_attrs[] = {
+ &dev_attr_volume.attr,
+ NULL
+};
+
+static const struct attribute_group pwm_beeper_attr_group = {
+ .attrs = pwm_beeper_dev_attrs,
+};
+
static int pwm_beeper_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -192,6 +240,14 @@ static int pwm_beeper_probe(struct platform_device *pdev)
input_set_drvdata(beeper->input, beeper);
+ beeper->duty_cycle = 50000;
+ error = devm_device_add_group(dev, &pwm_beeper_attr_group);
+ if (error) {
+ dev_err(dev, "Unable to create sysfs attributes: %d\n",
+ error);
+ return error;
+ }
+
error = input_register_device(beeper->input);
if (error) {
dev_err(dev, "Failed to register input device: %d\n", error);
--
2.39.2
^ permalink raw reply related
* Re: [PATCH v5] Fix freeze in lm8333 i2c keyboard driver
From: Jeff LaBundy @ 2023-05-12 17:28 UTC (permalink / raw)
To: Tomáš Mudruňka; +Cc: Dmitry Torokhov, linux-input, linux-kernel
In-Reply-To: <CAH2-hcJ8XxK060O-QCCBz+=cKk1HbT-aOKQH2EOY=D=xttvz7A@mail.gmail.com>
Hi Tomas,
On Fri, May 12, 2023 at 06:55:08PM +0200, Tomáš Mudruňka wrote:
> > So this is still racy, isn't it? The interrupt may come after read is
> > done, but before we register the handler.
>
> Well. It is. But please see the rest of the thread, where we've
> already discussed this.
>
> Every time the interrupt handler runs, the interrupt is disabled and
> then reenabled after i2c communication is done. Which means this exact
> thing happens on each keypress anyway. So i don't think it's a
> necessarily huge deal. It might not be perfect solution, but it makes
> things much better. window in which the deadlock condition can happen
> is now in range of few ms (or us), instead of ~10 seconds (previously
> it included bootloader and basicaly any moment from power up to driver
> load)
Right, but the point is that there are some alternatives to reduce the
range to zero. You posted one already, but I mistakenly advised against
it due to my own oversight :)
>
> Another solution would be to trigger on LOW instead of FALLING as
> proposed in initial version of the patch. That would be safer in terms
> of lm8333 deadlock, but Jeff was concerned about possibility of
> interrupt storm taking down whole system in case the IRQ line gets
> stuck in LOW for some reason...
Just to clarify, this is not my concern; all bets are off in case of
gross hardware failure such as this. Rather, my recommendations are:
1. Level (or edge) sensitivity should be specified in dts, not hard-coded
in the driver.
2. If you open support for level-triggered interrupts, you should verify
on a scope whether there is any chance that the IRQ line may still be in
the process of rising at the moment the read is completed. The datasheet
is ambiguous here.
>
> Tom
>
> pá 12. 5. 2023 v 1:44 odesílatel Dmitry Torokhov
> <dmitry.torokhov@gmail.com> napsal:
> >
> > On Wed, May 03, 2023 at 08:44:06PM -0500, Jeff LaBundy wrote:
> > > 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);
> >
> > So this is still racy, isn't it? The interrupt may come after read is
> > done, but before we register the handler.
> >
> > > > +
> > > > err = request_threaded_irq(client->irq, NULL, lm8333_irq_thread,
> > > > IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> > > > "lm8333", lm8333);
> > > > --
> > > > 2.40.1
> > >
> >
> > Thanks.
> >
> > --
> > Dmitry
Kind regards,
Jeff LaBundy
^ permalink raw reply
* Re: [PATCH v3 2/5] Input: add driver for Focaltech FTS touchscreen
From: Jeff LaBundy @ 2023-05-12 17:00 UTC (permalink / raw)
To: Joel Selvaraj
Cc: Caleb Connolly, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Andy Gross, Bjorn Andersson, Konrad Dybcio, Henrik Rydberg,
Arnd Bergmann, Robert Jarzmik, Markuss Broks, Jean Delvare,
Max Krummenacher, Chris Morgan, Job Noorman, Alistair Francis,
Hans de Goede, Maxime Ripard, linux-input, devicetree,
linux-kernel, linux-arm-msm, ~postmarketos/upstreaming,
phone-devel
In-Reply-To: <e25468f4-e3a3-bcf7-c2c0-826edb0600c2@gmail.com>
Hi Joel,
On Thu, May 11, 2023 at 07:43:28PM -0500, Joel Selvaraj wrote:
> Hi Jeff LaBundy,
>
> On 4/23/23 21:39, Jeff LaBundy wrote:
> > Hi Joel,
> >
> > Great work so far! It's coming along nicely. Please find my latest
> > feedback below.
>
> Sorry for the late reply, university semester end got me hooked up.
> I have a sad kind of good news... As pointed out by Hans de Goede, the
> edt-ft5x06 driver works out of the box for the fts8719 touchscreen in my
> device. So I think this patch series is no longer needed. I did have a look
> at the driver once when working on this patch series, but it looked
> different/not compatible at that time. After mentioned by Hans de Goede, I
> had a more closer look and the main touch buffer handling seems to be the
> same.
>
> I am sorry as we put some considerable time in this patch series. I should
> have noted more carefully. Thank you though as I learnt things working on
> this patch series.
There is absolutely no need to apologize; the review process is just as
informative for the reviewer as it is the reviewee. The important thing
is that the most optimal solution lands. "Less code is the best code" :)
>
> I guess I will send a different patch to add the compatible data to the
> existing edt-ft5x06 driver and dts changes to include that driver to my
> device.
Sounds great.
>
> > On Fri, Apr 14, 2023 at 09:02:19PM -0500, Joel Selvaraj wrote:
> > > The Focaltech FTS driver supports several variants of focaltech
> > > touchscreens found in ~2018 era smartphones including variants found on
> > > the PocoPhone F1 and the SHIFT6mq which are already present in mainline.
> > > This driver is loosely based on the original driver from Focaltech
> > > but has been simplified and largely reworked.
> > >
> > > Co-developed-by: Caleb Connolly <caleb@connolly.tech>
> > > Signed-off-by: Caleb Connolly <caleb@connolly.tech>
> > > Signed-off-by: Joel Selvaraj <joelselvaraj.oss@gmail.com>
> > > ---
> > > MAINTAINERS | 8 +
> > > drivers/input/touchscreen/Kconfig | 12 +
> > > drivers/input/touchscreen/Makefile | 1 +
> > > drivers/input/touchscreen/focaltech_fts5452.c | 432 ++++++++++++++++++
> > > 4 files changed, 453 insertions(+)
> > > create mode 100644 drivers/input/touchscreen/focaltech_fts5452.c
> > >
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index 7ec4ce64f66d..1a3ea61e1f52 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -8028,6 +8028,14 @@ L: linux-input@vger.kernel.org
> > > S: Maintained
> > > F: drivers/input/joystick/fsia6b.c
> > > +FOCALTECH FTS5452 TOUCHSCREEN DRIVER
> > > +M: Joel Selvaraj <joelselvaraj.oss@gmail.com>
> > > +M: Caleb Connolly <caleb@connolly.tech>
> > > +L: linux-input@vger.kernel.org
> > > +S: Maintained
> > > +F: Documentation/devicetree/bindings/input/touchscreen/focaltech,fts5452.yaml
> > > +F: drivers/input/touchscreen/focaltech_fts5452.c
> > > +
> > > FOCUSRITE SCARLETT GEN 2/3 MIXER DRIVER
> > > M: Geoffrey D. Bennett <g@b4.vu>
> > > L: alsa-devel@alsa-project.org (moderated for non-subscribers)
> > > diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> > > index 1feecd7ed3cb..11af91504969 100644
> > > --- a/drivers/input/touchscreen/Kconfig
> > > +++ b/drivers/input/touchscreen/Kconfig
> > > @@ -388,6 +388,18 @@ config TOUCHSCREEN_EXC3000
> > > To compile this driver as a module, choose M here: the
> > > module will be called exc3000.
> > > +config TOUCHSCREEN_FOCALTECH_FTS5452
> > > + tristate "Focaltech FTS Touchscreen"
> > > + depends on I2C
> > > + help
> > > + Say Y here to enable support for I2C connected Focaltech FTS
> > > + based touch panels, including the 5452 and 8917 panels.
> >
> > This language is a bit misleading, as it seems to suggest three or more
> > models are supported. It seems the title should simply be "FocalTech
> > FTS5452 touchscreen controller" with the description as "...FocalTech
> > FTS5452 and compatible touchscreen controllers."
> >
> > As more are found to be compatible (e.g. FTS8917), the compatible strings
> > can simply be appended.
> >
> > > +
> > > + If unsure, say N.
> > > +
> > > + To compile this driver as a module, choose M here: the
> > > + module will be called focaltech_fts.
> > > +
> > > config TOUCHSCREEN_FUJITSU
> > > tristate "Fujitsu serial touchscreen"
> > > select SERIO
> > > diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
> > > index 159cd5136fdb..47d78c9cff21 100644
> > > --- a/drivers/input/touchscreen/Makefile
> > > +++ b/drivers/input/touchscreen/Makefile
> > > @@ -45,6 +45,7 @@ obj-$(CONFIG_TOUCHSCREEN_ELO) += elo.o
> > > obj-$(CONFIG_TOUCHSCREEN_EGALAX) += egalax_ts.o
> > > obj-$(CONFIG_TOUCHSCREEN_EGALAX_SERIAL) += egalax_ts_serial.o
> > > obj-$(CONFIG_TOUCHSCREEN_EXC3000) += exc3000.o
> > > +obj-$(CONFIG_TOUCHSCREEN_FOCALTECH_FTS5452) += focaltech_fts5452.o
> > > obj-$(CONFIG_TOUCHSCREEN_FUJITSU) += fujitsu_ts.o
> > > obj-$(CONFIG_TOUCHSCREEN_GOODIX) += goodix_ts.o
> > > obj-$(CONFIG_TOUCHSCREEN_HIDEEP) += hideep.o
> > > diff --git a/drivers/input/touchscreen/focaltech_fts5452.c b/drivers/input/touchscreen/focaltech_fts5452.c
> > > new file mode 100644
> > > index 000000000000..abf8a2f271ca
> > > --- /dev/null
> > > +++ b/drivers/input/touchscreen/focaltech_fts5452.c
> > > @@ -0,0 +1,432 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * FocalTech touchscreen driver.
> > > + *
> > > + * Copyright (c) 2010-2017, FocalTech Systems, Ltd., all rights reserved.
> > > + * Copyright (C) 2018 XiaoMi, Inc.
> > > + * Copyright (c) 2021 Caleb Connolly <caleb@connolly.tech>
> > > + * Copyright (c) 2023 Joel Selvaraj <joelselvaraj.oss@gmail.com>
> > > + */
>
> [skip]
>
> > > +static const struct of_device_id fts_of_match[] = {
> > > + { .compatible = "focaltech,fts5452", .data = &fts5452_chip_data },
> > > + { .compatible = "focaltech,fts8719", .data = &fts8719_chip_data },
> > > + { /* sentinel */ }
> > > +};
> > > +
> > > +MODULE_DEVICE_TABLE(of, fts_of_match);
> > > +
> > > +static struct i2c_driver fts_ts_driver = {
> > > + .probe_new = fts_ts_probe,
> > > + .id_table = fts_i2c_id,
> > > + .driver = {
> > > + .name = FTS_DRIVER_NAME,
> > > + .pm = pm_sleep_ptr(&fts_dev_pm_ops),
> > > + .of_match_table = fts_of_match,
> > > + },
> > > +};
> > > +module_i2c_driver(fts_ts_driver);
> > > +
> > > +MODULE_AUTHOR("Joel Selvaraj <joelselvaraj.oss@gmail.com>");
> > > +MODULE_AUTHOR("Caleb Connolly <caleb@connolly.tech>");
> > > +MODULE_DESCRIPTION("Focaltech Touchscreen Driver");
> >
> > Nit: mixing 'FocalTech' and 'Focaltech' throughout.
> >
> > > +MODULE_LICENSE("GPL");
> > > --
> > > 2.40.0
> > >
> >
> > Kind regards,
> > Jeff LaBundy
>
> Thank you,
> Joel Selvaraj
Kind regards,
Jeff LaBundy
^ permalink raw reply
* Re: [PATCH v5] Fix freeze in lm8333 i2c keyboard driver
From: Tomáš Mudruňka @ 2023-05-12 16:55 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Jeff LaBundy, linux-input, linux-kernel
In-Reply-To: <ZF19yOojiaSbo3vS@google.com>
> So this is still racy, isn't it? The interrupt may come after read is
> done, but before we register the handler.
Well. It is. But please see the rest of the thread, where we've
already discussed this.
Every time the interrupt handler runs, the interrupt is disabled and
then reenabled after i2c communication is done. Which means this exact
thing happens on each keypress anyway. So i don't think it's a
necessarily huge deal. It might not be perfect solution, but it makes
things much better. window in which the deadlock condition can happen
is now in range of few ms (or us), instead of ~10 seconds (previously
it included bootloader and basicaly any moment from power up to driver
load)
Another solution would be to trigger on LOW instead of FALLING as
proposed in initial version of the patch. That would be safer in terms
of lm8333 deadlock, but Jeff was concerned about possibility of
interrupt storm taking down whole system in case the IRQ line gets
stuck in LOW for some reason...
Tom
pá 12. 5. 2023 v 1:44 odesílatel Dmitry Torokhov
<dmitry.torokhov@gmail.com> napsal:
>
> On Wed, May 03, 2023 at 08:44:06PM -0500, Jeff LaBundy wrote:
> > 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);
>
> So this is still racy, isn't it? The interrupt may come after read is
> done, but before we register the handler.
>
> > > +
> > > err = request_threaded_irq(client->irq, NULL, lm8333_irq_thread,
> > > IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> > > "lm8333", lm8333);
> > > --
> > > 2.40.1
> >
>
> Thanks.
>
> --
> Dmitry
^ permalink raw reply
* Re: [PATCH v5] Fix freeze in lm8333 i2c keyboard driver
From: Jeff LaBundy @ 2023-05-12 16:54 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Tomas Mudrunka, linux-input, linux-kernel
In-Reply-To: <ZF19yOojiaSbo3vS@google.com>
Hi Dmitry,
On Thu, May 11, 2023 at 04:44:08PM -0700, Dmitry Torokhov wrote:
> On Wed, May 03, 2023 at 08:44:06PM -0500, Jeff LaBundy wrote:
> > 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);
>
> So this is still racy, isn't it? The interrupt may come after read is
> done, but before we register the handler.
You're absolutely correct; I had not considered this corner case. Apologies
for the churn Tomas.
In that case, it seems the solution is to either move the dummy read after
the handler is registered as in v4, or remove the hard-coded flag and allow
dts to specify level sensitivity.
>
> > > +
> > > err = request_threaded_irq(client->irq, NULL, lm8333_irq_thread,
> > > IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> > > "lm8333", lm8333);
> > > --
> > > 2.40.1
> >
>
> Thanks.
>
> --
> Dmitry
Kind regards,
Jeff LaBundy
^ permalink raw reply
* [dtor-input:next] BUILD SUCCESS 17caa38a988e8f73e392f1f5ec2afb854552edcc
From: kernel test robot @ 2023-05-12 8:53 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: 17caa38a988e8f73e392f1f5ec2afb854552edcc dt-bindings: input: cypress,cyapa: convert to dtschema
elapsed time: 785m
configs tested: 181
configs skipped: 18
The following configs have been built successfully.
More configs may be tested in the coming days.
tested configs:
alpha allyesconfig gcc
alpha buildonly-randconfig-r004-20230511 gcc
alpha buildonly-randconfig-r005-20230509 gcc
alpha buildonly-randconfig-r006-20230511 gcc
alpha defconfig gcc
alpha randconfig-r003-20230511 gcc
alpha randconfig-r005-20230509 gcc
alpha randconfig-r006-20230511 gcc
alpha randconfig-r026-20230511 gcc
arc allyesconfig gcc
arc defconfig gcc
arc randconfig-r005-20230511 gcc
arc randconfig-r011-20230509 gcc
arc randconfig-r014-20230509 gcc
arc randconfig-r016-20230511 gcc
arc randconfig-r024-20230511 gcc
arc randconfig-r026-20230512 gcc
arc randconfig-r031-20230509 gcc
arc randconfig-r043-20230509 gcc
arc randconfig-r043-20230511 gcc
arm allmodconfig gcc
arm allyesconfig gcc
arm aspeed_g4_defconfig clang
arm defconfig gcc
arm exynos_defconfig gcc
arm orion5x_defconfig clang
arm randconfig-r002-20230509 clang
arm randconfig-r024-20230509 gcc
arm randconfig-r033-20230509 clang
arm randconfig-r046-20230509 gcc
arm randconfig-r046-20230511 clang
arm64 allyesconfig gcc
arm64 buildonly-randconfig-r002-20230511 clang
arm64 defconfig gcc
arm64 randconfig-r003-20230509 gcc
arm64 randconfig-r004-20230509 gcc
arm64 randconfig-r014-20230511 gcc
arm64 randconfig-r024-20230509 clang
arm64 randconfig-r032-20230511 clang
arm64 randconfig-r035-20230511 clang
csky defconfig gcc
csky randconfig-r004-20230511 gcc
csky randconfig-r022-20230511 gcc
csky randconfig-r025-20230511 gcc
csky randconfig-r032-20230509 gcc
hexagon randconfig-r001-20230509 clang
hexagon randconfig-r001-20230511 clang
hexagon randconfig-r002-20230509 clang
hexagon randconfig-r011-20230511 clang
hexagon randconfig-r015-20230511 clang
hexagon randconfig-r022-20230512 clang
hexagon randconfig-r031-20230509 clang
hexagon randconfig-r041-20230509 clang
hexagon randconfig-r041-20230511 clang
hexagon randconfig-r045-20230509 clang
hexagon randconfig-r045-20230511 clang
i386 allyesconfig gcc
i386 debian-10.3 gcc
i386 defconfig gcc
ia64 allmodconfig gcc
ia64 defconfig gcc
ia64 randconfig-r002-20230511 gcc
ia64 randconfig-r021-20230512 gcc
ia64 randconfig-r023-20230511 gcc
ia64 randconfig-r035-20230509 gcc
loongarch allmodconfig gcc
loongarch allnoconfig gcc
loongarch defconfig gcc
loongarch randconfig-r006-20230511 gcc
loongarch randconfig-r013-20230509 gcc
loongarch randconfig-r014-20230511 gcc
loongarch randconfig-r021-20230509 gcc
loongarch randconfig-r021-20230511 gcc
loongarch randconfig-r034-20230511 gcc
m68k allmodconfig gcc
m68k buildonly-randconfig-r003-20230509 gcc
m68k defconfig gcc
m68k m5275evb_defconfig gcc
m68k randconfig-r012-20230511 gcc
m68k randconfig-r014-20230509 gcc
m68k randconfig-r025-20230512 gcc
microblaze buildonly-randconfig-r002-20230509 gcc
microblaze randconfig-r001-20230509 gcc
microblaze randconfig-r005-20230511 gcc
microblaze randconfig-r022-20230511 gcc
microblaze randconfig-r036-20230509 gcc
mips allmodconfig gcc
mips allyesconfig gcc
mips buildonly-randconfig-r001-20230509 clang
mips buildonly-randconfig-r004-20230511 gcc
mips decstation_defconfig gcc
mips ip22_defconfig clang
mips pic32mzda_defconfig clang
mips randconfig-r006-20230509 clang
mips randconfig-r011-20230511 clang
mips randconfig-r021-20230511 clang
mips randconfig-r023-20230511 clang
nios2 10m50_defconfig gcc
nios2 defconfig gcc
nios2 randconfig-r003-20230509 gcc
nios2 randconfig-r023-20230512 gcc
nios2 randconfig-r024-20230511 gcc
nios2 randconfig-r024-20230512 gcc
nios2 randconfig-r026-20230509 gcc
nios2 randconfig-r036-20230509 gcc
openrisc buildonly-randconfig-r002-20230511 gcc
openrisc buildonly-randconfig-r006-20230511 gcc
openrisc randconfig-r002-20230511 gcc
openrisc randconfig-r025-20230509 gcc
openrisc randconfig-r026-20230509 gcc
openrisc randconfig-r031-20230511 gcc
openrisc randconfig-r033-20230511 gcc
openrisc randconfig-r035-20230511 gcc
openrisc randconfig-r036-20230511 gcc
parisc defconfig gcc
parisc randconfig-r013-20230511 gcc
parisc randconfig-r022-20230509 gcc
parisc64 defconfig gcc
powerpc allmodconfig gcc
powerpc allnoconfig gcc
powerpc buildonly-randconfig-r001-20230511 gcc
powerpc buildonly-randconfig-r004-20230509 clang
powerpc cm5200_defconfig gcc
powerpc ge_imp3a_defconfig clang
powerpc powernv_defconfig clang
powerpc randconfig-r005-20230509 gcc
powerpc xes_mpc85xx_defconfig clang
riscv alldefconfig clang
riscv allmodconfig gcc
riscv allnoconfig gcc
riscv buildonly-randconfig-r002-20230509 clang
riscv buildonly-randconfig-r004-20230509 clang
riscv buildonly-randconfig-r005-20230511 gcc
riscv defconfig gcc
riscv randconfig-r021-20230509 clang
riscv randconfig-r022-20230509 clang
riscv randconfig-r025-20230511 gcc
riscv randconfig-r034-20230509 gcc
riscv randconfig-r034-20230511 clang
riscv randconfig-r042-20230509 clang
riscv randconfig-r042-20230511 gcc
riscv rv32_defconfig gcc
s390 allmodconfig gcc
s390 allyesconfig gcc
s390 buildonly-randconfig-r001-20230511 gcc
s390 defconfig gcc
s390 randconfig-r013-20230509 clang
s390 randconfig-r013-20230511 gcc
s390 randconfig-r023-20230509 clang
s390 randconfig-r033-20230509 gcc
s390 randconfig-r033-20230511 clang
s390 randconfig-r034-20230509 gcc
s390 randconfig-r044-20230509 clang
s390 randconfig-r044-20230511 gcc
sh allmodconfig gcc
sh apsh4ad0a_defconfig gcc
sh randconfig-r003-20230511 gcc
sh randconfig-r004-20230511 gcc
sh randconfig-r015-20230511 gcc
sh sh7724_generic_defconfig gcc
sparc buildonly-randconfig-r003-20230511 gcc
sparc defconfig gcc
sparc randconfig-r015-20230509 gcc
sparc randconfig-r016-20230509 gcc
sparc64 buildonly-randconfig-r005-20230511 gcc
sparc64 randconfig-r006-20230509 gcc
sparc64 randconfig-r016-20230511 gcc
sparc64 randconfig-r032-20230509 gcc
sparc64 randconfig-r032-20230511 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 rhel-8.3 gcc
xtensa buildonly-randconfig-r001-20230509 gcc
xtensa buildonly-randconfig-r006-20230509 gcc
xtensa randconfig-r001-20230511 gcc
xtensa randconfig-r004-20230509 gcc
xtensa randconfig-r035-20230509 gcc
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply
* Re: [PATCH 2/4] dt-bindings: touchscreen: add virtual-touchscreen and virtual-buttons properties
From: Michael Riesch @ 2023-05-12 8:13 UTC (permalink / raw)
To: Krzysztof Kozlowski, Javier Carrasco, Dmitry Torokhov,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Henrik Rydberg,
Bastian Hecht
Cc: linux-kernel, linux-input, devicetree
In-Reply-To: <cdcd5656-2c7f-23bf-d016-fff79a279ebb@linaro.org>
Hi Krzysztof,
On 5/12/23 09:20, Krzysztof Kozlowski wrote:
> On 12/05/2023 09:08, Michael Riesch wrote:
>> Hi Krzysztof,
>>
>>>> These buttons are actually physical i.e. printed and with a given
>>>> functionality, but still part of the touchscreen as the physical device
>>>> is not aware of them. Therefore they only make sense in the touchscreen
>>>> context.
>>>
>>> So basically you still have the same touchscreen under the buttons and
>>> these are not separate devices. Whether someone put a sticker on
>>> touchscreen, does not really change hardware behavior and it's up to
>>> system to handle this. What you are trying to do here is to create
>>> virtual buttons through Devicetree and DT is not for that.
>>
>> I have already addressed a similar statement here:
>> https://lore.kernel.org/lkml/20230504042927.GA1129520@quokka/t/#m1a93595c36535312df0061459a1da5e062de6c44
>> but let me extend this comment a bit.
>>
>> The notion of "someone putting a sticker on touchscreen" does not really
>> reflect the use case we have in mind. We are talking about devices that
>> are shipped from the factory in a particular configuration, e.g.,
>>
>> +-----------------------+---------+
>> | | power |
>> | | button |
>> | touchscreen +---------+
>> | (behind: display) | return |
>> | | button |
>> +-----------------------+---------+
>>
>> Here, the real touchscreen is larger than the display. The display is
>> behind the "touchscreen" part. Behind the buttons there are symbols
>> engraved in metal or printed foils or what not. I just would like to
>> make it clear that these symbols are not going to change.
>>
>> We believe that the engraved or printed symbols actually define the
>> (expected) hardware behavior. Of course there is a virtual notion to
>> that, and of course it would be possible to let the power button work as
>> return button and vice versa in software. However, the user sees the
>> engraved or printed symbols (which are not going to change) and expects
>> the device to react appropriately.
>
> OK, I actually missed the concept that display is not equal to the
> touchscreen ("screen" actually suggests display :) ). In your case here
> it sounds good, but please put some parts of this description into this
> common binding. The sketch above is nice, especially if you can point
> where the virtual origin x/y are. Picture is thousands words.
I think this can be arranged :-)
>> Now if you suggest that the system (that is user space, right?) should
>> handle this, then the question would be which component is supposed to
>> handle the touchscreen events and react accordingly. I don't have an
>> answer to that and hope I don't need to find one. But independent of
>> that, a configuration file is required that defines the area of the
>> virtual buttons etc. Wouldn't this be similar to the (mostly) beloved
>> xorg.conf containing the definitions of input devices?
>
> If the case was a bit different - e.g. display is everywhere - I could
> easily imagine that on the device rotation you want to move
> (re-position) the buttons. Or if user has some accessibility option
> enabled, you want bigger buttons. Then it would be a prove that it must
> be configured and managed from user-space. How, I don't know.
I fully agree here. If user space is able to draw the symbols, then
naturally it is also responsible for handling the events appropriately.
This could happen in an application with a GUI or on display
manager/compositor level (e.g., weston controller or similar).
But I take it that for the situation sketched above the device tree
approach is the correct one. Documentation should be improved, though.
Best regards,
Michael
^ permalink raw reply
* [dtor-input:for-linus] BUILD SUCCESS 7b63a88bb62ba2ddf5fcd956be85fe46624628b9
From: kernel test robot @ 2023-05-12 7:55 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git for-linus
branch HEAD: 7b63a88bb62ba2ddf5fcd956be85fe46624628b9 Input: psmouse - fix OOB access in Elantech protocol
elapsed time: 729m
configs tested: 132
configs skipped: 7
The following configs have been built successfully.
More configs may be tested in the coming days.
tested configs:
alpha allyesconfig gcc
alpha defconfig gcc
alpha randconfig-r002-20230511 gcc
alpha randconfig-r006-20230511 gcc
alpha randconfig-r011-20230509 gcc
arc allyesconfig gcc
arc defconfig gcc
arc randconfig-r003-20230509 gcc
arc randconfig-r023-20230509 gcc
arc randconfig-r043-20230509 gcc
arc randconfig-r043-20230511 gcc
arm allmodconfig gcc
arm allyesconfig gcc
arm defconfig gcc
arm randconfig-r016-20230509 gcc
arm randconfig-r031-20230511 gcc
arm randconfig-r033-20230509 clang
arm randconfig-r046-20230509 gcc
arm randconfig-r046-20230511 clang
arm64 allyesconfig gcc
arm64 buildonly-randconfig-r005-20230511 clang
arm64 buildonly-randconfig-r006-20230511 clang
arm64 defconfig gcc
csky defconfig gcc
csky randconfig-r005-20230511 gcc
hexagon randconfig-r004-20230509 clang
hexagon randconfig-r024-20230509 clang
hexagon randconfig-r024-20230511 clang
hexagon randconfig-r034-20230511 clang
hexagon randconfig-r036-20230509 clang
hexagon randconfig-r041-20230509 clang
hexagon randconfig-r041-20230511 clang
hexagon randconfig-r045-20230509 clang
hexagon randconfig-r045-20230511 clang
i386 allyesconfig gcc
i386 debian-10.3 gcc
i386 defconfig gcc
i386 randconfig-a001 gcc
i386 randconfig-a002 clang
i386 randconfig-a003 gcc
i386 randconfig-a004 clang
i386 randconfig-a005 gcc
i386 randconfig-a006 clang
i386 randconfig-a011 clang
i386 randconfig-a012 gcc
i386 randconfig-a013 clang
i386 randconfig-a014 gcc
i386 randconfig-a015 clang
i386 randconfig-a016 gcc
ia64 allmodconfig gcc
ia64 buildonly-randconfig-r005-20230509 gcc
ia64 defconfig gcc
ia64 randconfig-r006-20230509 gcc
ia64 randconfig-r035-20230511 gcc
loongarch allmodconfig gcc
loongarch allnoconfig gcc
loongarch defconfig gcc
loongarch randconfig-r021-20230509 gcc
loongarch randconfig-r021-20230511 gcc
m68k allmodconfig gcc
m68k buildonly-randconfig-r003-20230511 gcc
m68k defconfig gcc
m68k randconfig-r001-20230509 gcc
m68k randconfig-r012-20230509 gcc
m68k randconfig-r025-20230509 gcc
m68k randconfig-r032-20230509 gcc
microblaze buildonly-randconfig-r001-20230511 gcc
microblaze randconfig-r013-20230509 gcc
mips allmodconfig gcc
mips allyesconfig gcc
mips randconfig-r004-20230511 gcc
mips randconfig-r011-20230511 clang
mips randconfig-r015-20230509 gcc
nios2 defconfig gcc
nios2 randconfig-r022-20230511 gcc
nios2 randconfig-r033-20230511 gcc
nios2 randconfig-r035-20230509 gcc
parisc buildonly-randconfig-r002-20230509 gcc
parisc buildonly-randconfig-r003-20230509 gcc
parisc defconfig gcc
parisc randconfig-r025-20230511 gcc
parisc randconfig-r026-20230511 gcc
parisc64 defconfig gcc
powerpc allmodconfig gcc
powerpc allnoconfig gcc
powerpc randconfig-r003-20230511 clang
powerpc randconfig-r005-20230509 gcc
powerpc randconfig-r034-20230509 gcc
riscv allmodconfig gcc
riscv allnoconfig gcc
riscv defconfig gcc
riscv randconfig-r026-20230509 clang
riscv randconfig-r042-20230509 clang
riscv randconfig-r042-20230511 gcc
riscv rv32_defconfig gcc
s390 allmodconfig gcc
s390 allyesconfig gcc
s390 defconfig gcc
s390 randconfig-r013-20230509 clang
s390 randconfig-r032-20230511 clang
s390 randconfig-r044-20230509 clang
s390 randconfig-r044-20230511 gcc
sh allmodconfig gcc
sh randconfig-r014-20230509 gcc
sh randconfig-r022-20230509 gcc
sh randconfig-r023-20230511 gcc
sparc buildonly-randconfig-r004-20230509 gcc
sparc buildonly-randconfig-r006-20230509 gcc
sparc defconfig gcc
sparc randconfig-r002-20230509 gcc
sparc randconfig-r031-20230509 gcc
sparc64 buildonly-randconfig-r002-20230511 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 clang
x86_64 randconfig-a002 gcc
x86_64 randconfig-a003 clang
x86_64 randconfig-a004 gcc
x86_64 randconfig-a005 clang
x86_64 randconfig-a006 gcc
x86_64 randconfig-a011 gcc
x86_64 randconfig-a012 clang
x86_64 randconfig-a013 gcc
x86_64 randconfig-a014 clang
x86_64 randconfig-a015 gcc
x86_64 randconfig-a016 clang
x86_64 rhel-8.3 gcc
xtensa buildonly-randconfig-r004-20230511 gcc
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply
* Re: [PATCH 2/4] dt-bindings: touchscreen: add virtual-touchscreen and virtual-buttons properties
From: Krzysztof Kozlowski @ 2023-05-12 7:20 UTC (permalink / raw)
To: Michael Riesch, Javier Carrasco, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Henrik Rydberg, Bastian Hecht
Cc: linux-kernel, linux-input, devicetree
In-Reply-To: <eb3f40e6-a604-39f2-eb49-8b41590a65d4@wolfvision.net>
On 12/05/2023 09:08, Michael Riesch wrote:
> Hi Krzysztof,
>
>>> These buttons are actually physical i.e. printed and with a given
>>> functionality, but still part of the touchscreen as the physical device
>>> is not aware of them. Therefore they only make sense in the touchscreen
>>> context.
>>
>> So basically you still have the same touchscreen under the buttons and
>> these are not separate devices. Whether someone put a sticker on
>> touchscreen, does not really change hardware behavior and it's up to
>> system to handle this. What you are trying to do here is to create
>> virtual buttons through Devicetree and DT is not for that.
>
> I have already addressed a similar statement here:
> https://lore.kernel.org/lkml/20230504042927.GA1129520@quokka/t/#m1a93595c36535312df0061459a1da5e062de6c44
> but let me extend this comment a bit.
>
> The notion of "someone putting a sticker on touchscreen" does not really
> reflect the use case we have in mind. We are talking about devices that
> are shipped from the factory in a particular configuration, e.g.,
>
> +-----------------------+---------+
> | | power |
> | | button |
> | touchscreen +---------+
> | (behind: display) | return |
> | | button |
> +-----------------------+---------+
>
> Here, the real touchscreen is larger than the display. The display is
> behind the "touchscreen" part. Behind the buttons there are symbols
> engraved in metal or printed foils or what not. I just would like to
> make it clear that these symbols are not going to change.
>
> We believe that the engraved or printed symbols actually define the
> (expected) hardware behavior. Of course there is a virtual notion to
> that, and of course it would be possible to let the power button work as
> return button and vice versa in software. However, the user sees the
> engraved or printed symbols (which are not going to change) and expects
> the device to react appropriately.
OK, I actually missed the concept that display is not equal to the
touchscreen ("screen" actually suggests display :) ). In your case here
it sounds good, but please put some parts of this description into this
common binding. The sketch above is nice, especially if you can point
where the virtual origin x/y are. Picture is thousands words.
>
> Now if you suggest that the system (that is user space, right?) should
> handle this, then the question would be which component is supposed to
> handle the touchscreen events and react accordingly. I don't have an
> answer to that and hope I don't need to find one. But independent of
> that, a configuration file is required that defines the area of the
> virtual buttons etc. Wouldn't this be similar to the (mostly) beloved
> xorg.conf containing the definitions of input devices?
If the case was a bit different - e.g. display is everywhere - I could
easily imagine that on the device rotation you want to move
(re-position) the buttons. Or if user has some accessibility option
enabled, you want bigger buttons. Then it would be a prove that it must
be configured and managed from user-space. How, I don't know.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 2/4] dt-bindings: touchscreen: add virtual-touchscreen and virtual-buttons properties
From: Michael Riesch @ 2023-05-12 7:08 UTC (permalink / raw)
To: Krzysztof Kozlowski, Javier Carrasco, Dmitry Torokhov,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Henrik Rydberg,
Bastian Hecht
Cc: linux-kernel, linux-input, devicetree
In-Reply-To: <a7261bc1-902d-99f9-aa3e-2c90dd264c8d@linaro.org>
Hi Krzysztof,
On 5/12/23 08:25, Krzysztof Kozlowski wrote:
> On 11/05/2023 12:28, Javier Carrasco wrote:
>> On 11.05.23 11:45, Krzysztof Kozlowski wrote:
>>> On 10/05/2023 15:50, Javier Carrasco wrote:
>>>> The virtual-touchscreen object defines an area within the touchscreen
>>>> where touch events are reported and their coordinates get converted to
>>>> the virtual origin. This object avoids getting events from areas that
>>>> are physically hidden by overlay frames.
>>>>
>>>> For touchscreens where overlay buttons on the touchscreen surface are
>>>> provided, the virtual-buttons object contains a node for every button
>>>> and the key event that should be reported when pressed.
>>>
>>> Hm, I don't understand - are these separate physical buttons? If so, why
>>> would they be part of touchscreen? Where do the wires go?
>> Those buttons are actually printed on some overlays which are mounted on
>> top of the touchscreen and they are used to provide a predefined
>> functionality to that touchscreen region. Any touchscreen with such a
>> physical overlay with buttons printed on them or clipped touchscreen
>> areas might use this functionality.
>>
>> These buttons are actually physical i.e. printed and with a given
>> functionality, but still part of the touchscreen as the physical device
>> is not aware of them. Therefore they only make sense in the touchscreen
>> context.
>
> So basically you still have the same touchscreen under the buttons and
> these are not separate devices. Whether someone put a sticker on
> touchscreen, does not really change hardware behavior and it's up to
> system to handle this. What you are trying to do here is to create
> virtual buttons through Devicetree and DT is not for that.
I have already addressed a similar statement here:
https://lore.kernel.org/lkml/20230504042927.GA1129520@quokka/t/#m1a93595c36535312df0061459a1da5e062de6c44
but let me extend this comment a bit.
The notion of "someone putting a sticker on touchscreen" does not really
reflect the use case we have in mind. We are talking about devices that
are shipped from the factory in a particular configuration, e.g.,
+-----------------------+---------+
| | power |
| | button |
| touchscreen +---------+
| (behind: display) | return |
| | button |
+-----------------------+---------+
Here, the real touchscreen is larger than the display. The display is
behind the "touchscreen" part. Behind the buttons there are symbols
engraved in metal or printed foils or what not. I just would like to
make it clear that these symbols are not going to change.
We believe that the engraved or printed symbols actually define the
(expected) hardware behavior. Of course there is a virtual notion to
that, and of course it would be possible to let the power button work as
return button and vice versa in software. However, the user sees the
engraved or printed symbols (which are not going to change) and expects
the device to react appropriately.
Now if you suggest that the system (that is user space, right?) should
handle this, then the question would be which component is supposed to
handle the touchscreen events and react accordingly. I don't have an
answer to that and hope I don't need to find one. But independent of
that, a configuration file is required that defines the area of the
virtual buttons etc. Wouldn't this be similar to the (mostly) beloved
xorg.conf containing the definitions of input devices?
Best regards,
Michael
^ permalink raw reply
* Re: [PATCH 2/4] dt-bindings: touchscreen: add virtual-touchscreen and virtual-buttons properties
From: Krzysztof Kozlowski @ 2023-05-12 6:25 UTC (permalink / raw)
To: Javier Carrasco, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Henrik Rydberg, Bastian Hecht,
Michael Riesch
Cc: linux-kernel, linux-input, devicetree
In-Reply-To: <18526d2a-ac5f-2b26-9ed3-5a82f20cac86@wolfvision.net>
On 11/05/2023 12:28, Javier Carrasco wrote:
> On 11.05.23 11:45, Krzysztof Kozlowski wrote:
>> On 10/05/2023 15:50, Javier Carrasco wrote:
>>> The virtual-touchscreen object defines an area within the touchscreen
>>> where touch events are reported and their coordinates get converted to
>>> the virtual origin. This object avoids getting events from areas that
>>> are physically hidden by overlay frames.
>>>
>>> For touchscreens where overlay buttons on the touchscreen surface are
>>> provided, the virtual-buttons object contains a node for every button
>>> and the key event that should be reported when pressed.
>>
>> Hm, I don't understand - are these separate physical buttons? If so, why
>> would they be part of touchscreen? Where do the wires go?
> Those buttons are actually printed on some overlays which are mounted on
> top of the touchscreen and they are used to provide a predefined
> functionality to that touchscreen region. Any touchscreen with such a
> physical overlay with buttons printed on them or clipped touchscreen
> areas might use this functionality.
>
> These buttons are actually physical i.e. printed and with a given
> functionality, but still part of the touchscreen as the physical device
> is not aware of them. Therefore they only make sense in the touchscreen
> context.
So basically you still have the same touchscreen under the buttons and
these are not separate devices. Whether someone put a sticker on
touchscreen, does not really change hardware behavior and it's up to
system to handle this. What you are trying to do here is to create
virtual buttons through Devicetree and DT is not for that.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 6/7] Input: libps2 - introduce common interrupt handler
From: kernel test robot @ 2023-05-12 1:01 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input
Cc: llvm, oe-kbuild-all, Raul E Rangel, linux-kernel
In-Reply-To: <20230511185252.386941-7-dmitry.torokhov@gmail.com>
Hi Dmitry,
kernel test robot noticed the following build errors:
[auto build test ERROR on dtor-input/next]
[also build test ERROR on dtor-input/for-linus hid/for-next linus/master v6.4-rc1 next-20230511]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Dmitry-Torokhov/Input-libps2-attach-ps2dev-instances-as-serio-port-s-drvdata/20230512-025431
base: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
patch link: https://lore.kernel.org/r/20230511185252.386941-7-dmitry.torokhov%40gmail.com
patch subject: [PATCH 6/7] Input: libps2 - introduce common interrupt handler
config: mips-randconfig-r031-20230510 (https://download.01.org/0day-ci/archive/20230512/202305120818.unCn8fQ9-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project b0fb98227c90adf2536c9ad644a74d5e92961111)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install mips cross compiling tool for clang build
# apt-get install binutils-mipsel-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/1f2ba3a941e6f6a3ad745fa780825ac56570616e
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Dmitry-Torokhov/Input-libps2-attach-ps2dev-instances-as-serio-port-s-drvdata/20230512-025431
git checkout 1f2ba3a941e6f6a3ad745fa780825ac56570616e
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=mips olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash drivers/input/keyboard/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202305120818.unCn8fQ9-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/input/keyboard/atkbd.c:424:3: error: must use 'struct' tag to refer to type 'atkbd'
atkbd->resend = false;
^
struct
>> drivers/input/keyboard/atkbd.c:424:3: error: expected expression
2 errors generated.
vim +424 drivers/input/keyboard/atkbd.c
^1da177e4c3f41 Linus Torvalds 2005-04-16 400
1f2ba3a941e6f6 Dmitry Torokhov 2023-05-11 401 static enum ps2_disposition atkbd_pre_receive_byte(struct ps2dev *ps2dev,
1f2ba3a941e6f6 Dmitry Torokhov 2023-05-11 402 u8 data, unsigned int flags)
^1da177e4c3f41 Linus Torvalds 2005-04-16 403 {
1f2ba3a941e6f6 Dmitry Torokhov 2023-05-11 404 struct serio *serio = ps2dev->serio;
^1da177e4c3f41 Linus Torvalds 2005-04-16 405
a9a1f9c315c27f Dmitry Torokhov 2010-01-06 406 dev_dbg(&serio->dev, "Received %02x flags %02x\n", data, flags);
^1da177e4c3f41 Linus Torvalds 2005-04-16 407
^1da177e4c3f41 Linus Torvalds 2005-04-16 408 #if !defined(__i386__) && !defined (__x86_64__)
1f2ba3a941e6f6 Dmitry Torokhov 2023-05-11 409 if ((flags & (SERIO_FRAME | SERIO_PARITY)) &&
1f2ba3a941e6f6 Dmitry Torokhov 2023-05-11 410 (~flags & SERIO_TIMEOUT)) {
1f2ba3a941e6f6 Dmitry Torokhov 2023-05-11 411 struct atkbd *atkbd = container_of(ps2dev, struct atkbd,
1f2ba3a941e6f6 Dmitry Torokhov 2023-05-11 412 ps2dev);
1f2ba3a941e6f6 Dmitry Torokhov 2023-05-11 413
1f2ba3a941e6f6 Dmitry Torokhov 2023-05-11 414 if (!atkbd->resend && atkbd->write) {
1f2ba3a941e6f6 Dmitry Torokhov 2023-05-11 415 dev_warn(&serio->dev,
1f2ba3a941e6f6 Dmitry Torokhov 2023-05-11 416 "Frame/parity error: %02x\n", flags);
^1da177e4c3f41 Linus Torvalds 2005-04-16 417 serio_write(serio, ATKBD_CMD_RESEND);
a9a1f9c315c27f Dmitry Torokhov 2010-01-06 418 atkbd->resend = true;
1f2ba3a941e6f6 Dmitry Torokhov 2023-05-11 419 return PS2_IGNORE;
1f2ba3a941e6f6 Dmitry Torokhov 2023-05-11 420 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 421 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 422
^1da177e4c3f41 Linus Torvalds 2005-04-16 423 if (!flags && data == ATKBD_RET_ACK)
a9a1f9c315c27f Dmitry Torokhov 2010-01-06 @424 atkbd->resend = false;
^1da177e4c3f41 Linus Torvalds 2005-04-16 425 #endif
^1da177e4c3f41 Linus Torvalds 2005-04-16 426
1f2ba3a941e6f6 Dmitry Torokhov 2023-05-11 427 return PS2_PROCESS;
1f2ba3a941e6f6 Dmitry Torokhov 2023-05-11 428 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 429
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply
* Re: [PATCH v3 2/5] Input: add driver for Focaltech FTS touchscreen
From: Joel Selvaraj @ 2023-05-12 0:43 UTC (permalink / raw)
To: Jeff LaBundy
Cc: Caleb Connolly, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Andy Gross, Bjorn Andersson, Konrad Dybcio, Henrik Rydberg,
Arnd Bergmann, Robert Jarzmik, Markuss Broks, Jean Delvare,
Max Krummenacher, Chris Morgan, Job Noorman, Alistair Francis,
Hans de Goede, Maxime Ripard, linux-input, devicetree,
linux-kernel, linux-arm-msm, ~postmarketos/upstreaming,
phone-devel
In-Reply-To: <ZEXr1hC+Q5Bo/3Tc@nixie71>
Hi Jeff LaBundy,
On 4/23/23 21:39, Jeff LaBundy wrote:
> Hi Joel,
>
> Great work so far! It's coming along nicely. Please find my latest
> feedback below.
Sorry for the late reply, university semester end got me hooked up.
I have a sad kind of good news... As pointed out by Hans de Goede, the
edt-ft5x06 driver works out of the box for the fts8719 touchscreen in my
device. So I think this patch series is no longer needed. I did have a
look at the driver once when working on this patch series, but it looked
different/not compatible at that time. After mentioned by Hans de Goede,
I had a more closer look and the main touch buffer handling seems to be
the same.
I am sorry as we put some considerable time in this patch series. I
should have noted more carefully. Thank you though as I learnt things
working on this patch series.
I guess I will send a different patch to add the compatible data to the
existing edt-ft5x06 driver and dts changes to include that driver to my
device.
> On Fri, Apr 14, 2023 at 09:02:19PM -0500, Joel Selvaraj wrote:
>> The Focaltech FTS driver supports several variants of focaltech
>> touchscreens found in ~2018 era smartphones including variants found on
>> the PocoPhone F1 and the SHIFT6mq which are already present in mainline.
>> This driver is loosely based on the original driver from Focaltech
>> but has been simplified and largely reworked.
>>
>> Co-developed-by: Caleb Connolly <caleb@connolly.tech>
>> Signed-off-by: Caleb Connolly <caleb@connolly.tech>
>> Signed-off-by: Joel Selvaraj <joelselvaraj.oss@gmail.com>
>> ---
>> MAINTAINERS | 8 +
>> drivers/input/touchscreen/Kconfig | 12 +
>> drivers/input/touchscreen/Makefile | 1 +
>> drivers/input/touchscreen/focaltech_fts5452.c | 432 ++++++++++++++++++
>> 4 files changed, 453 insertions(+)
>> create mode 100644 drivers/input/touchscreen/focaltech_fts5452.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 7ec4ce64f66d..1a3ea61e1f52 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -8028,6 +8028,14 @@ L: linux-input@vger.kernel.org
>> S: Maintained
>> F: drivers/input/joystick/fsia6b.c
>>
>> +FOCALTECH FTS5452 TOUCHSCREEN DRIVER
>> +M: Joel Selvaraj <joelselvaraj.oss@gmail.com>
>> +M: Caleb Connolly <caleb@connolly.tech>
>> +L: linux-input@vger.kernel.org
>> +S: Maintained
>> +F: Documentation/devicetree/bindings/input/touchscreen/focaltech,fts5452.yaml
>> +F: drivers/input/touchscreen/focaltech_fts5452.c
>> +
>> FOCUSRITE SCARLETT GEN 2/3 MIXER DRIVER
>> M: Geoffrey D. Bennett <g@b4.vu>
>> L: alsa-devel@alsa-project.org (moderated for non-subscribers)
>> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
>> index 1feecd7ed3cb..11af91504969 100644
>> --- a/drivers/input/touchscreen/Kconfig
>> +++ b/drivers/input/touchscreen/Kconfig
>> @@ -388,6 +388,18 @@ config TOUCHSCREEN_EXC3000
>> To compile this driver as a module, choose M here: the
>> module will be called exc3000.
>>
>> +config TOUCHSCREEN_FOCALTECH_FTS5452
>> + tristate "Focaltech FTS Touchscreen"
>> + depends on I2C
>> + help
>> + Say Y here to enable support for I2C connected Focaltech FTS
>> + based touch panels, including the 5452 and 8917 panels.
>
> This language is a bit misleading, as it seems to suggest three or more
> models are supported. It seems the title should simply be "FocalTech
> FTS5452 touchscreen controller" with the description as "...FocalTech
> FTS5452 and compatible touchscreen controllers."
>
> As more are found to be compatible (e.g. FTS8917), the compatible strings
> can simply be appended.
>
>> +
>> + If unsure, say N.
>> +
>> + To compile this driver as a module, choose M here: the
>> + module will be called focaltech_fts.
>> +
>> config TOUCHSCREEN_FUJITSU
>> tristate "Fujitsu serial touchscreen"
>> select SERIO
>> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
>> index 159cd5136fdb..47d78c9cff21 100644
>> --- a/drivers/input/touchscreen/Makefile
>> +++ b/drivers/input/touchscreen/Makefile
>> @@ -45,6 +45,7 @@ obj-$(CONFIG_TOUCHSCREEN_ELO) += elo.o
>> obj-$(CONFIG_TOUCHSCREEN_EGALAX) += egalax_ts.o
>> obj-$(CONFIG_TOUCHSCREEN_EGALAX_SERIAL) += egalax_ts_serial.o
>> obj-$(CONFIG_TOUCHSCREEN_EXC3000) += exc3000.o
>> +obj-$(CONFIG_TOUCHSCREEN_FOCALTECH_FTS5452) += focaltech_fts5452.o
>> obj-$(CONFIG_TOUCHSCREEN_FUJITSU) += fujitsu_ts.o
>> obj-$(CONFIG_TOUCHSCREEN_GOODIX) += goodix_ts.o
>> obj-$(CONFIG_TOUCHSCREEN_HIDEEP) += hideep.o
>> diff --git a/drivers/input/touchscreen/focaltech_fts5452.c b/drivers/input/touchscreen/focaltech_fts5452.c
>> new file mode 100644
>> index 000000000000..abf8a2f271ca
>> --- /dev/null
>> +++ b/drivers/input/touchscreen/focaltech_fts5452.c
>> @@ -0,0 +1,432 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * FocalTech touchscreen driver.
>> + *
>> + * Copyright (c) 2010-2017, FocalTech Systems, Ltd., all rights reserved.
>> + * Copyright (C) 2018 XiaoMi, Inc.
>> + * Copyright (c) 2021 Caleb Connolly <caleb@connolly.tech>
>> + * Copyright (c) 2023 Joel Selvaraj <joelselvaraj.oss@gmail.com>
>> + */
[skip]
>> +static const struct of_device_id fts_of_match[] = {
>> + { .compatible = "focaltech,fts5452", .data = &fts5452_chip_data },
>> + { .compatible = "focaltech,fts8719", .data = &fts8719_chip_data },
>> + { /* sentinel */ }
>> +};
>> +
>> +MODULE_DEVICE_TABLE(of, fts_of_match);
>> +
>> +static struct i2c_driver fts_ts_driver = {
>> + .probe_new = fts_ts_probe,
>> + .id_table = fts_i2c_id,
>> + .driver = {
>> + .name = FTS_DRIVER_NAME,
>> + .pm = pm_sleep_ptr(&fts_dev_pm_ops),
>> + .of_match_table = fts_of_match,
>> + },
>> +};
>> +module_i2c_driver(fts_ts_driver);
>> +
>> +MODULE_AUTHOR("Joel Selvaraj <joelselvaraj.oss@gmail.com>");
>> +MODULE_AUTHOR("Caleb Connolly <caleb@connolly.tech>");
>> +MODULE_DESCRIPTION("Focaltech Touchscreen Driver");
>
> Nit: mixing 'FocalTech' and 'Focaltech' throughout.
>
>> +MODULE_LICENSE("GPL");
>> --
>> 2.40.0
>>
>
> Kind regards,
> Jeff LaBundy
Thank you,
Joel Selvaraj
^ permalink raw reply
* Re: [PATCH] Input: elan_i2c - Implement inhibit/uninhibit functions.
From: 'Dmitry Torokhov' @ 2023-05-12 0:07 UTC (permalink / raw)
To: Jingle.Wu; +Cc: linux-kernel, linux-input, phoenix, josh.chen, dave.wang
In-Reply-To: <000b01d97978$32775ca0$976615e0$@emc.com.tw>
Hi Jingle,
On Fri, Apr 28, 2023 at 10:21:53AM +0800, Jingle.Wu wrote:
> Hi Dmitry:
> During the initial process and when the users open/close device,
> having the elan uninhibit/inhibit commands (low power mode) was not what
> Elan expects to happen. Due to that touchpad would do the calibration in
> uninhibit moment , we don't want the calibration to be affected by fingers
> on the touchpad.
> However, the LID inhibit/uninhibit functions in the Linux kernel
> driver calls open/close(), so we need to separate the inhibit/uninhibit
> behavior from open/close() function
>
> https://elixir.bootlin.com/linux/latest/source/drivers/input/input.c#L1783
>
> https://elixir.bootlin.com/linux/latest/source/drivers/input/input.c#L1813
You quite intentionally can not separate inhibit/uninhibit from open and
close. As with open/close nothing stops inhibit/uninhibit to be called
rapidly multiple times in a row. You probably rely on particular
operation of ChromeOS devices during normal user interaction.
As far as I remember, you need to perform recalibration on certain
devices when LID gets open. I recommend you implement something similar
to https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/3578852
where we monitor LID events and recalibrate when needed, instead of
trying to make assumptions on when inhibit and uninhibit functions are
called.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 2/2] Input: cap11xx - add advanced sensitivity settings
From: Dmitry Torokhov @ 2023-05-11 23:57 UTC (permalink / raw)
To: Jiri Valek - 2N
Cc: krzysztof.kozlowski+dt, devicetree, linux-input, linux-kernel,
robh+dt, u.kleine-koenig
In-Reply-To: <20230414233815.4004526-3-jiriv@axis.com>
Hi Jiri,
On Sat, Apr 15, 2023 at 01:38:15AM +0200, Jiri Valek - 2N wrote:
> @@ -474,7 +645,7 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
> if (error)
> return error;
>
> - irq = irq_of_parse_and_map(node, 0);
> + irq = irq_of_parse_and_map(dev->of_node, 0);
Do you know if this is actually needed or we can rely on I2C core to
figure out the interrupt for us?
Also, could I ask you to move the driver from of_property_*() to
device_property_*() API?
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v5] Fix freeze in lm8333 i2c keyboard driver
From: Dmitry Torokhov @ 2023-05-11 23:44 UTC (permalink / raw)
To: Jeff LaBundy; +Cc: Tomas Mudrunka, linux-input, linux-kernel
In-Reply-To: <ZFMN5nmqLAX170SE@nixie71>
On Wed, May 03, 2023 at 08:44:06PM -0500, Jeff LaBundy wrote:
> 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);
So this is still racy, isn't it? The interrupt may come after read is
done, but before we register the handler.
> > +
> > err = request_threaded_irq(client->irq, NULL, lm8333_irq_thread,
> > IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> > "lm8333", lm8333);
> > --
> > 2.40.1
>
Thanks.
--
Dmitry
^ permalink raw reply
* [BUG: 6.3 kernel] Logitech Trackball M575 misidentified
From: Xose Vazquez Perez @ 2023-05-11 21:22 UTC (permalink / raw)
To: linux-input
Hi,
6.3.2 kernel identifies "Logitech" "ERGO M575" as "Logitech" "(\xc9_O\x04)",
6.2.15 works fine.
6.2.15 boot log:
input: Logitech ERGO M575 as /devices/pci0000:00/0000:00:1a.0/usb3/3-1/3-1.3/3-1.3:1.2/0003:046D:C52B.0003/0003:046D:4096.0005/input/input15
logitech-hidpp-device 0003:046D:4096.0005: input,hidraw1: USB HID v1.11 Mouse [Logitech ERGO M575] on usb-0000:00:1a.0-1.3/input2:1
6.3.2 boot log:
input: Logitech \xc9_O\x04 as /devices/pci0000:00/0000:00:1a.0/usb3/3-1/3-1.3/3-1.3:1.2/0003:046D:C52B.0003/0003:046D:4096.0005/input/input15
logitech-hidpp-device 0003:046D:4096.0005: input,hidraw2: USB HID v1.11 Mouse [Logitech \xc9_O\x04] on usb-0000:00:1a.0-1.3/input2:1
Thanks.
^ permalink raw reply
* Re: [PATCH v4 1/2] dt-bindings: gpio: Convert STMPE GPIO to YAML schema
From: Linus Walleij @ 2023-05-11 20:39 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Rob Herring, Krzysztof Kozlowski, Maxime Coquelin,
Alexandre Torgue, Dmitry Torokhov, Lee Jones, Philippe Schenker,
Stefan Agner, Marek Vasut, Steffen Trumtrar, linux-gpio,
devicetree, linux-stm32, linux-arm-kernel, linux-kernel,
linux-input
In-Reply-To: <CAMRc=MdsBiV3AvzSPtCuR58w0N9z7o+hUrBDtXUC4a++pECb8w@mail.gmail.com>
On Thu, May 11, 2023 at 4:58 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> Applied, thanks!
That works ... but patch 2/2 depends on this one. (uses $ref).
You'd have to give Lee an immutable branch that he can pull
before applying patch 2/2 so he has the dependency, or let him
apply both.
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH] selftests: hid: Add touch tests for Wacom devices
From: Joshua Dickens @ 2023-05-11 19:47 UTC (permalink / raw)
To: linux-input, Jiri Kosina, Benjamin Tissoires
Cc: Ping Cheng, Aaron Armstrong Skomra, Jason Gerecke, Joshua Dickens,
Joshua Dickens, Jason Gerecke
Adding a wacom touch device to use the test_multitouch tests.
Adding a 2 additional tests.
- A test to check if a touch event is sent when the contact_id of the event is 0.
- A test to check if a touch event is not sent when confidence is set to 0.
Signed-off-by: Joshua Dickens <joshua.dickens@wacom.com>
Reviewed-by: Jason Gerecke <jason.gerecke@wacom.com>
---
.../selftests/hid/tests/test_wacom_generic.py | 84 ++++++++++++++++++-
1 file changed, 81 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/hid/tests/test_wacom_generic.py b/tools/testing/selftests/hid/tests/test_wacom_generic.py
index b1eb2bc787fc..f92fe8e02c1b 100644
--- a/tools/testing/selftests/hid/tests/test_wacom_generic.py
+++ b/tools/testing/selftests/hid/tests/test_wacom_generic.py
@@ -31,6 +31,7 @@ from enum import Enum
from hidtools.hut import HUT
from hidtools.hid import HidUnit
from . import base
+from . import test_multitouch
import libevdev
import pytest
@@ -517,7 +518,7 @@ class BaseTest:
for usage in get_report_usages(report):
yield usage
- def assertName(self, uhdev):
+ def assertName(self, uhdev, type):
"""
Assert that the name is as we expect.
@@ -526,7 +527,7 @@ class BaseTest:
this assertion from the base class to work properly.
"""
evdev = uhdev.get_evdev()
- expected_name = uhdev.name + " Pen"
+ expected_name = uhdev.name + type
if "wacom" not in expected_name.lower():
expected_name = "Wacom " + expected_name
assert evdev.name == expected_name
@@ -549,6 +550,12 @@ class BaseTest:
usage_id("Generic Desktop", "Y"): PhysRange(
PhysRange.CENTIMETER, 5, 150
),
+ usage_id("Digitizers", "Width"): PhysRange(
+ PhysRange.CENTIMETER, 5, 150
+ ),
+ usage_id("Digitizers", "Height"): PhysRange(
+ PhysRange.CENTIMETER, 5, 150
+ ),
usage_id("Digitizers", "X Tilt"): PhysRange(PhysRange.DEGREE, 90, 180),
usage_id("Digitizers", "Y Tilt"): PhysRange(PhysRange.DEGREE, 90, 180),
usage_id("Digitizers", "Twist"): PhysRange(PhysRange.DEGREE, 358, 360),
@@ -603,7 +610,17 @@ class BaseTest:
pass
-class TestOpaqueTablet(BaseTest.TestTablet):
+class PenTabletTest(BaseTest.TestTablet):
+ def assertName(self, uhdev):
+ super().assertName(uhdev, " Pen")
+
+
+class TouchTabletTest(BaseTest.TestTablet):
+ def assertName(self, uhdev):
+ super().assertName(uhdev, " Finger")
+
+
+class TestOpaqueTablet(PenTabletTest):
def create_device(self):
return OpaqueTablet()
@@ -842,3 +859,64 @@ class TestPTHX60_Pen(TestOpaqueCTLTablet):
libevdev.InputEvent(libevdev.EV_KEY.BTN_0, 0),
],
)
+
+
+class TestDTH2452Tablet(test_multitouch.BaseTest.TestMultitouch, TouchTabletTest):
+ def create_device(self):
+ return test_multitouch.Digitizer(
+ "DTH 2452",
+ rdesc="05 0d 09 04 a1 01 85 0c 95 01 75 08 15 00 26 ff 00 81 03 09 54 81 02 09 22 a1 02 05 0d 95 01 75 01 25 01 09 42 81 02 81 03 09 47 81 02 95 05 81 03 09 51 26 ff 00 75 10 95 01 81 02 35 00 65 11 55 0e 05 01 09 30 26 a0 44 46 96 14 81 42 09 31 26 9a 26 46 95 0b 81 42 05 0d 75 08 95 01 15 00 09 48 26 5f 00 46 7c 14 81 02 09 49 25 35 46 7d 0b 81 02 45 00 65 00 55 00 c0 05 0d 09 22 a1 02 05 0d 95 01 75 01 25 01 09 42 81 02 81 03 09 47 81 02 95 05 81 03 09 51 26 ff 00 75 10 95 01 81 02 35 00 65 11 55 0e 05 01 09 30 26 a0 44 46 96 14 81 42 09 31 26 9a 26 46 95 0b 81 42 05 0d 75 08 95 01 15 00 09 48 26 5f 00 46 7c 14 81 02 09 49 25 35 46 7d 0b 81 02 45 00 65 00 55 00 c0 05 0d 09 22 a1 02 05 0d 95 01 75 01 25 01 09 42 81 02 81 03 09 47 81 02 95 05 81 03 09 51 26 ff 00 75 10 95 01 81 02 35 00 65 11 55 0e 05 01 09 30 26 a0 44 46 96 14 81 42 09 31 26 9a 26 46 95 0b 81 42 05 0d 75 08 95 01 15 00 09 48 26 5f 00 46 7c 14 81 02 09 49 25 35 46 7d 0b 81 02 45 00 65 00 55 00 c0 05 0d 09 22 a1 02 05 0d 95 01 75 01 25 01 09 42 81 02 81 03 09 47 81 02 95 05 81 03 09 51 26 ff 00 75 10 95 01 81 02 35 00 65 11 55 0e 05 01 09 30 26 a0 44 46 96 14 81 42 09 31 26 9a 26 46 95 0b 81 42 05 0d 75 08 95 01 15 00 09 48 26 5f 00 46 7c 14 81 02 09 49 25 35 46 7d 0b 81 02 45 00 65 00 55 00 c0 05 0d 09 22 a1 02 05 0d 95 01 75 01 25 01 09 42 81 02 81 03 09 47 81 02 95 05 81 03 09 51 26 ff 00 75 10 95 01 81 02 35 00 65 11 55 0e 05 01 09 30 26 a0 44 46 96 14 81 42 09 31 26 9a 26 46 95 0b 81 42 05 0d 75 08 95 01 15 00 09 48 26 5f 00 46 7c 14 81 02 09 49 25 35 46 7d 0b 81 02 45 00 65 00 55 00 c0 05 0d 27 ff ff 00 00 75 10 95 01 09 56 81 02 75 08 95 0e 81 03 09 55 26 ff 00 75 08 b1 02 85 0a 06 00 ff 09 c5 96 00 01 b1 02 c0 06 00 ff 09 01 a1 01 09 01 85 13 15 00 26 ff 00 75 08 95 3f 81 02 06 00 ff 09 01 15 00 26 ff 00 75 08 95 3f 91 02 c0",
+ input_info=(0x3, 0x056A, 0x0383),
+ )
+
+ def test_contact_id_0(self):
+ """
+ Bring a finger in contact with the tablet, then hold it down and remove it.
+
+ Ensure that even with contact ID = 0 which is usually given as an invalid
+ touch event by most tablets with the exception of a few, that given the
+ confidence bit is set to 1 it should process it as a valid touch to cover
+ the few tablets using contact ID = 0 as a valid touch value.
+ """
+ uhdev = self.uhdev
+ evdev = uhdev.get_evdev()
+
+ t0 = test_multitouch.Touch(0, 50, 100)
+ r = uhdev.event([t0])
+ events = uhdev.next_sync_events()
+ self.debug_reports(r, uhdev, events)
+
+ slot = self.get_slot(uhdev, t0, 0)
+
+ assert libevdev.InputEvent(libevdev.EV_KEY.BTN_TOUCH, 1) in events
+ assert evdev.slots[slot][libevdev.EV_ABS.ABS_MT_TRACKING_ID] == 0
+ assert evdev.slots[slot][libevdev.EV_ABS.ABS_MT_POSITION_X] == 50
+ assert evdev.slots[slot][libevdev.EV_ABS.ABS_MT_POSITION_Y] == 100
+
+ t0.tipswitch = False
+ if uhdev.quirks is None or "VALID_IS_INRANGE" not in uhdev.quirks:
+ t0.inrange = False
+ r = uhdev.event([t0])
+ events = uhdev.next_sync_events()
+ self.debug_reports(r, uhdev, events)
+ assert libevdev.InputEvent(libevdev.EV_KEY.BTN_TOUCH, 0) in events
+ assert evdev.slots[slot][libevdev.EV_ABS.ABS_MT_TRACKING_ID] == -1
+
+ def test_confidence_false(self):
+ """
+ Bring a finger in contact with the tablet with confidence set to false.
+
+ Ensure that the confidence bit being set to false should not result in a touch event.
+ """
+ uhdev = self.uhdev
+ evdev = uhdev.get_evdev()
+
+ t0 = test_multitouch.Touch(1, 50, 100)
+ t0.confidence = False
+ r = uhdev.event([t0])
+ events = uhdev.next_sync_events()
+ self.debug_reports(r, uhdev, events)
+
+ slot = self.get_slot(uhdev, t0, 0)
+
+ assert not events
\ No newline at end of file
--
2.40.0
^ permalink raw reply related
* [PATCH 6/7] Input: libps2 - introduce common interrupt handler
From: Dmitry Torokhov @ 2023-05-11 18:52 UTC (permalink / raw)
To: linux-input; +Cc: Raul E Rangel, linux-kernel
In-Reply-To: <20230511185252.386941-1-dmitry.torokhov@gmail.com>
Instead of exposing inner workings of libps2 to drivers such as atkbd and
psmouse, have them define pre-receive and receive callbacks, and provide a
common handler that can be used with underlying serio port.
While at this add kerneldoc to the module.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/keyboard/atkbd.c | 73 +++++-----
drivers/input/mouse/psmouse-base.c | 53 +++----
drivers/input/serio/libps2.c | 226 ++++++++++++++++++++---------
include/linux/libps2.h | 61 +++++---
4 files changed, 259 insertions(+), 154 deletions(-)
diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index 2fb2ad73e796..8ef663a589b3 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -398,47 +398,49 @@ static unsigned int atkbd_compat_scancode(struct atkbd *atkbd, unsigned int code
return code;
}
-/*
- * atkbd_interrupt(). Here takes place processing of data received from
- * the keyboard into events.
- */
-
-static irqreturn_t atkbd_interrupt(struct serio *serio, unsigned char data,
- unsigned int flags)
+static enum ps2_disposition atkbd_pre_receive_byte(struct ps2dev *ps2dev,
+ u8 data, unsigned int flags)
{
- struct atkbd *atkbd = atkbd_from_serio(serio);
- struct input_dev *dev = atkbd->dev;
- unsigned int code = data;
- int scroll = 0, hscroll = 0, click = -1;
- int value;
- unsigned short keycode;
+ struct serio *serio = ps2dev->serio;
dev_dbg(&serio->dev, "Received %02x flags %02x\n", data, flags);
#if !defined(__i386__) && !defined (__x86_64__)
- if ((flags & (SERIO_FRAME | SERIO_PARITY)) && (~flags & SERIO_TIMEOUT) && !atkbd->resend && atkbd->write) {
- dev_warn(&serio->dev, "Frame/parity error: %02x\n", flags);
- serio_write(serio, ATKBD_CMD_RESEND);
- atkbd->resend = true;
- goto out;
+ if ((flags & (SERIO_FRAME | SERIO_PARITY)) &&
+ (~flags & SERIO_TIMEOUT)) {
+ struct atkbd *atkbd = container_of(ps2dev, struct atkbd,
+ ps2dev);
+
+ if (!atkbd->resend && atkbd->write) {
+ dev_warn(&serio->dev,
+ "Frame/parity error: %02x\n", flags);
+ serio_write(serio, ATKBD_CMD_RESEND);
+ atkbd->resend = true;
+ return PS2_IGNORE;
+ }
}
if (!flags && data == ATKBD_RET_ACK)
atkbd->resend = false;
#endif
- if (unlikely(atkbd->ps2dev.flags & PS2_FLAG_ACK))
- if (ps2_handle_ack(&atkbd->ps2dev, data))
- goto out;
+ return PS2_PROCESS;
+}
- if (unlikely(atkbd->ps2dev.flags & PS2_FLAG_CMD))
- if (ps2_handle_response(&atkbd->ps2dev, data))
- goto out;
+static void atkbd_receive_byte(struct ps2dev *ps2dev, u8 data)
+{
+ struct serio *serio = ps2dev->serio;
+ struct atkbd *atkbd = container_of(ps2dev, struct atkbd, ps2dev);
+ struct input_dev *dev = atkbd->dev;
+ unsigned int code = data;
+ int scroll = 0, hscroll = 0, click = -1;
+ int value;
+ unsigned short keycode;
pm_wakeup_event(&serio->dev, 0);
if (!atkbd->enabled)
- goto out;
+ return;
input_event(dev, EV_MSC, MSC_RAW, code);
@@ -460,16 +462,16 @@ static irqreturn_t atkbd_interrupt(struct serio *serio, unsigned char data,
case ATKBD_RET_BAT:
atkbd->enabled = false;
serio_reconnect(atkbd->ps2dev.serio);
- goto out;
+ return;
case ATKBD_RET_EMUL0:
atkbd->emul = 1;
- goto out;
+ return;
case ATKBD_RET_EMUL1:
atkbd->emul = 2;
- goto out;
+ return;
case ATKBD_RET_RELEASE:
atkbd->release = true;
- goto out;
+ return;
case ATKBD_RET_ACK:
case ATKBD_RET_NAK:
if (printk_ratelimit())
@@ -477,18 +479,18 @@ static irqreturn_t atkbd_interrupt(struct serio *serio, unsigned char data,
"Spurious %s on %s. "
"Some program might be trying to access hardware directly.\n",
data == ATKBD_RET_ACK ? "ACK" : "NAK", serio->phys);
- goto out;
+ return;
case ATKBD_RET_ERR:
atkbd->err_count++;
dev_dbg(&serio->dev, "Keyboard on %s reports too many keys pressed.\n",
serio->phys);
- goto out;
+ return;
}
code = atkbd_compat_scancode(atkbd, code);
if (atkbd->emul && --atkbd->emul)
- goto out;
+ return;
keycode = atkbd->keycode[code];
@@ -564,8 +566,6 @@ static irqreturn_t atkbd_interrupt(struct serio *serio, unsigned char data,
}
atkbd->release = false;
-out:
- return IRQ_HANDLED;
}
static int atkbd_set_repeat_rate(struct atkbd *atkbd)
@@ -1229,7 +1229,8 @@ static int atkbd_connect(struct serio *serio, struct serio_driver *drv)
goto fail1;
atkbd->dev = dev;
- ps2_init(&atkbd->ps2dev, serio);
+ ps2_init(&atkbd->ps2dev, serio,
+ atkbd_pre_receive_byte, atkbd_receive_byte);
INIT_DELAYED_WORK(&atkbd->event_work, atkbd_event_work);
mutex_init(&atkbd->mutex);
@@ -1385,7 +1386,7 @@ static struct serio_driver atkbd_drv = {
},
.description = DRIVER_DESC,
.id_table = atkbd_serio_ids,
- .interrupt = atkbd_interrupt,
+ .interrupt = ps2_interrupt,
.connect = atkbd_connect,
.reconnect = atkbd_reconnect,
.disconnect = atkbd_disconnect,
diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
index ed5376099fba..a0aac76b1e41 100644
--- a/drivers/input/mouse/psmouse-base.c
+++ b/drivers/input/mouse/psmouse-base.c
@@ -336,17 +336,14 @@ static void psmouse_handle_oob_data(struct psmouse *psmouse, u8 data)
}
}
-/*
- * psmouse_interrupt() handles incoming characters, either passing them
- * for normal processing or gathering them as command response.
- */
-static irqreturn_t psmouse_interrupt(struct serio *serio,
- u8 data, unsigned int flags)
+static enum ps2_disposition psmouse_pre_receive_byte(struct ps2dev *ps2dev,
+ u8 data,
+ unsigned int flags)
{
- struct psmouse *psmouse = psmouse_from_serio(serio);
+ struct psmouse *psmouse = container_of(ps2dev, struct psmouse, ps2dev);
if (psmouse->state == PSMOUSE_IGNORE)
- goto out;
+ return PS2_IGNORE;
if (unlikely((flags & SERIO_TIMEOUT) ||
((flags & SERIO_PARITY) &&
@@ -357,27 +354,25 @@ static irqreturn_t psmouse_interrupt(struct serio *serio,
"bad data from KBC -%s%s\n",
flags & SERIO_TIMEOUT ? " timeout" : "",
flags & SERIO_PARITY ? " bad parity" : "");
- ps2_cmd_aborted(&psmouse->ps2dev);
- goto out;
+ return PS2_ERROR;
}
if (flags & SERIO_OOB_DATA) {
psmouse_handle_oob_data(psmouse, data);
- goto out;
+ return PS2_IGNORE;
}
- if (unlikely(psmouse->ps2dev.flags & PS2_FLAG_ACK))
- if (ps2_handle_ack(&psmouse->ps2dev, data))
- goto out;
+ return PS2_PROCESS;
+}
- if (unlikely(psmouse->ps2dev.flags & PS2_FLAG_CMD))
- if (ps2_handle_response(&psmouse->ps2dev, data))
- goto out;
+static void psmouse_receive_byte(struct ps2dev *ps2dev, u8 data)
+{
+ struct psmouse *psmouse = container_of(ps2dev, struct psmouse, ps2dev);
- pm_wakeup_event(&serio->dev, 0);
+ pm_wakeup_event(&ps2dev->serio->dev, 0);
if (psmouse->state <= PSMOUSE_RESYNCING)
- goto out;
+ return;
if (psmouse->state == PSMOUSE_ACTIVATED &&
psmouse->pktcnt && time_after(jiffies, psmouse->last + HZ/2)) {
@@ -386,7 +381,7 @@ static irqreturn_t psmouse_interrupt(struct serio *serio,
psmouse->badbyte = psmouse->packet[0];
__psmouse_set_state(psmouse, PSMOUSE_RESYNCING);
psmouse_queue_work(psmouse, &psmouse->resync_work, 0);
- goto out;
+ return;
}
psmouse->packet[psmouse->pktcnt++] = data;
@@ -395,21 +390,21 @@ static irqreturn_t psmouse_interrupt(struct serio *serio,
if (unlikely(psmouse->packet[0] == PSMOUSE_RET_BAT && psmouse->pktcnt <= 2)) {
if (psmouse->pktcnt == 1) {
psmouse->last = jiffies;
- goto out;
+ return;
}
if (psmouse->packet[1] == PSMOUSE_RET_ID ||
(psmouse->protocol->type == PSMOUSE_HGPK &&
psmouse->packet[1] == PSMOUSE_RET_BAT)) {
__psmouse_set_state(psmouse, PSMOUSE_IGNORE);
- serio_reconnect(serio);
- goto out;
+ serio_reconnect(ps2dev->serio);
+ return;
}
/* Not a new device, try processing first byte normally */
psmouse->pktcnt = 1;
if (psmouse_handle_byte(psmouse))
- goto out;
+ return;
psmouse->packet[psmouse->pktcnt++] = data;
}
@@ -424,14 +419,11 @@ static irqreturn_t psmouse_interrupt(struct serio *serio,
psmouse->badbyte = psmouse->packet[0];
__psmouse_set_state(psmouse, PSMOUSE_RESYNCING);
psmouse_queue_work(psmouse, &psmouse->resync_work, 0);
- goto out;
+ return;
}
psmouse->last = jiffies;
psmouse_handle_byte(psmouse);
-
- out:
- return IRQ_HANDLED;
}
/*
@@ -1604,7 +1596,8 @@ static int psmouse_connect(struct serio *serio, struct serio_driver *drv)
if (!psmouse || !input_dev)
goto err_free;
- ps2_init(&psmouse->ps2dev, serio);
+ ps2_init(&psmouse->ps2dev, serio,
+ psmouse_pre_receive_byte, psmouse_receive_byte);
INIT_DELAYED_WORK(&psmouse->resync_work, psmouse_resync);
psmouse->dev = input_dev;
snprintf(psmouse->phys, sizeof(psmouse->phys), "%s/input0", serio->phys);
@@ -1786,7 +1779,7 @@ static struct serio_driver psmouse_drv = {
},
.description = DRIVER_DESC,
.id_table = psmouse_serio_ids,
- .interrupt = psmouse_interrupt,
+ .interrupt = ps2_interrupt,
.connect = psmouse_connect,
.reconnect = psmouse_reconnect,
.fast_reconnect = psmouse_fast_reconnect,
diff --git a/drivers/input/serio/libps2.c b/drivers/input/serio/libps2.c
index 09eb605364bb..7c5fc853072a 100644
--- a/drivers/input/serio/libps2.c
+++ b/drivers/input/serio/libps2.c
@@ -19,9 +19,22 @@
#define DRIVER_DESC "PS/2 driver library"
-MODULE_AUTHOR("Dmitry Torokhov <dtor@mail.ru>");
-MODULE_DESCRIPTION("PS/2 driver library");
-MODULE_LICENSE("GPL");
+#define PS2_CMD_SETSCALE11 0x00e6
+#define PS2_CMD_SETRES 0x10e8
+#define PS2_CMD_GETID 0x02f2
+#define PS2_CMD_RESET_BAT 0x02ff
+
+#define PS2_RET_BAT 0xaa
+#define PS2_RET_ID 0x00
+#define PS2_RET_ACK 0xfa
+#define PS2_RET_NAK 0xfe
+#define PS2_RET_ERR 0xfc
+
+#define PS2_FLAG_ACK BIT(0) /* Waiting for ACK/NAK */
+#define PS2_FLAG_CMD BIT(1) /* Waiting for a command to finish */
+#define PS2_FLAG_CMD1 BIT(2) /* Waiting for the first byte of command response */
+#define PS2_FLAG_WAITID BIT(3) /* Command executing is GET ID */
+#define PS2_FLAG_NAK BIT(4) /* Last transmission was NAKed */
static int ps2_do_sendbyte(struct ps2dev *ps2dev, u8 byte,
unsigned int timeout, unsigned int max_attempts)
@@ -76,14 +89,17 @@ static int ps2_do_sendbyte(struct ps2dev *ps2dev, u8 byte,
return error;
}
-/*
- * ps2_sendbyte() sends a byte to the device and waits for acknowledge.
- * It doesn't handle retransmission, the caller is expected to handle
+/**
+ * ps2_sendbyte - sends a byte to the device and wait for acknowledgement
+ * @ps2dev: a PS/2 device to send the data to
+ * @byte: data to be sent to the device
+ * @timeout: timeout for sending the data and receiving an acknowledge
+ *
+ * The function doesn't handle retransmission, the caller is expected to handle
* it when needed.
*
* ps2_sendbyte() can only be called from a process context.
*/
-
int ps2_sendbyte(struct ps2dev *ps2dev, u8 byte, unsigned int timeout)
{
int retval;
@@ -99,6 +115,13 @@ int ps2_sendbyte(struct ps2dev *ps2dev, u8 byte, unsigned int timeout)
}
EXPORT_SYMBOL(ps2_sendbyte);
+/**
+ * ps2_begin_command - mark beginning of execution of a complex command
+ * @ps2dev: a PS/2 device executing the command
+ *
+ * Serializes a complex/compound command. Once command is finished
+ * ps2_end_command() should be called.
+ */
void ps2_begin_command(struct ps2dev *ps2dev)
{
struct mutex *m = ps2dev->serio->ps2_cmd_mutex ?: &ps2dev->cmd_mutex;
@@ -107,6 +130,10 @@ void ps2_begin_command(struct ps2dev *ps2dev)
}
EXPORT_SYMBOL(ps2_begin_command);
+/**
+ * ps2_end_command - mark end of execution of a complex command
+ * @ps2dev: a PS/2 device executing the command
+ */
void ps2_end_command(struct ps2dev *ps2dev)
{
struct mutex *m = ps2dev->serio->ps2_cmd_mutex ?: &ps2dev->cmd_mutex;
@@ -115,11 +142,13 @@ void ps2_end_command(struct ps2dev *ps2dev)
}
EXPORT_SYMBOL(ps2_end_command);
-/*
- * ps2_drain() waits for device to transmit requested number of bytes
- * and discards them.
+/**
+ * ps2_drain - waits for device to transmit requested number of bytes
+ * and discards them
+ * @ps2dev: the PS/2 device that should be drained
+ * @maxbytes: maximum number of bytes to be drained
+ * @timeout: time to drain the device
*/
-
void ps2_drain(struct ps2dev *ps2dev, size_t maxbytes, unsigned int timeout)
{
if (maxbytes > sizeof(ps2dev->cmdbuf)) {
@@ -142,11 +171,11 @@ void ps2_drain(struct ps2dev *ps2dev, size_t maxbytes, unsigned int timeout)
}
EXPORT_SYMBOL(ps2_drain);
-/*
- * ps2_is_keyboard_id() checks received ID byte against the list of
- * known keyboard IDs.
+/**
+ * ps2_is_keyboard_id - checks received ID byte against the list of
+ * known keyboard IDs
+ * @id_byte: data byte that should be checked
*/
-
bool ps2_is_keyboard_id(u8 id_byte)
{
static const u8 keyboard_ids[] = {
@@ -167,7 +196,6 @@ EXPORT_SYMBOL(ps2_is_keyboard_id);
* response and tries to reduce remaining timeout to speed up command
* completion.
*/
-
static int ps2_adjust_timeout(struct ps2dev *ps2dev,
unsigned int command, unsigned int timeout)
{
@@ -217,13 +245,19 @@ static int ps2_adjust_timeout(struct ps2dev *ps2dev,
return timeout;
}
-/*
- * ps2_command() sends a command and its parameters to the mouse,
- * then waits for the response and puts it in the param array.
+/**
+ * __ps2_command - send a command to PS/2 device
+ * @ps2dev: the PS/2 device that should execute the command
+ * @param: a buffer containing parameters to be sent along with the command,
+ * or place where the results of the command execution will be deposited,
+ * or both
+ * @command: command word that encodes the command itself, as well as number of
+ * additional parameter bytes that should be sent to the device and expected
+ * length of the command response
*
- * ps2_command() can only be called from a process context
+ * Not serialized. Callers should use ps2_begin_command() and ps2_end_command()
+ * to ensure proper serialization for complex commands.
*/
-
int __ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command)
{
unsigned int timeout;
@@ -327,6 +361,20 @@ int __ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command)
}
EXPORT_SYMBOL(__ps2_command);
+/**
+ * ps2_command - send a command to PS/2 device
+ * @ps2dev: the PS/2 device that should execute the command
+ * @param: a buffer containing parameters to be sent along with the command,
+ * or place where the results of the command execution will be deposited,
+ * or both
+ * @command: command word that encodes the command itself, as well as number of
+ * additional parameter bytes that should be sent to the device and expected
+ * length of the command response
+ *
+ * Note: ps2_command() serializes the command execution so that only one
+ * command can be executed at a time for either individual port or the entire
+ * 8042 controller.
+ */
int ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command)
{
int rc;
@@ -339,14 +387,16 @@ int ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command)
}
EXPORT_SYMBOL(ps2_command);
-/*
- * ps2_sliced_command() sends an extended PS/2 command to the mouse
- * using sliced syntax, understood by advanced devices, such as Logitech
- * or Synaptics touchpads. The command is encoded as:
+/**
+ * ps2_sliced_command - sends an extended PS/2 command to a mouse
+ * @ps2dev: the PS/2 device that should execute the command
+ * @command: command byte
+ *
+ * The command is sent using "sliced" syntax understood by advanced devices,
+ * such as Logitech or Synaptics touchpads. The command is encoded as:
* 0xE6 0xE8 rr 0xE8 ss 0xE8 tt 0xE8 uu where (rr*64)+(ss*16)+(tt*4)+uu
* is the command.
*/
-
int ps2_sliced_command(struct ps2dev *ps2dev, u8 command)
{
int i;
@@ -372,12 +422,22 @@ int ps2_sliced_command(struct ps2dev *ps2dev, u8 command)
}
EXPORT_SYMBOL(ps2_sliced_command);
-/*
- * ps2_init() initializes ps2dev structure
+/**
+ * ps2_init - initializes ps2dev structure
+ * @ps2dev: structure to be initialized
+ * @serio: serio port associated with the PS/2 device
+ * @pre_receive_handler: validation handler to check basic communication state
+ * @receive_handler: main protocol handler
+ *
+ * Prepares ps2dev structure for use in drivers for PS/2 devices.
*/
-
-void ps2_init(struct ps2dev *ps2dev, struct serio *serio)
+void ps2_init(struct ps2dev *ps2dev, struct serio *serio,
+ ps2_pre_receive_handler_t pre_receive_handler,
+ ps2_receive_handler_t receive_handler)
{
+ ps2dev->pre_receive_handler = pre_receive_handler;
+ ps2dev->receive_handler = receive_handler;
+
mutex_init(&ps2dev->cmd_mutex);
lockdep_set_subclass(&ps2dev->cmd_mutex, serio->depth);
init_waitqueue_head(&ps2dev->wait);
@@ -387,11 +447,35 @@ void ps2_init(struct ps2dev *ps2dev, struct serio *serio)
EXPORT_SYMBOL(ps2_init);
/*
- * ps2_handle_ack() is supposed to be used in interrupt handler
- * to properly process ACK/NAK of a command from a PS/2 device.
+ * ps2_handle_response() stores device's response to a command and notifies
+ * the process waiting for completion of the command. Note that there is a
+ * distinction between waiting for the first byte of the response, and
+ * waiting for subsequent bytes. It is done so that callers could shorten
+ * timeouts once first byte of response is received.
*/
+static void ps2_handle_response(struct ps2dev *ps2dev, u8 data)
+{
+ if (ps2dev->cmdcnt)
+ ps2dev->cmdbuf[--ps2dev->cmdcnt] = data;
-bool ps2_handle_ack(struct ps2dev *ps2dev, u8 data)
+ if (ps2dev->flags & PS2_FLAG_CMD1) {
+ ps2dev->flags &= ~PS2_FLAG_CMD1;
+ if (ps2dev->cmdcnt)
+ wake_up(&ps2dev->wait);
+ }
+
+ if (!ps2dev->cmdcnt) {
+ ps2dev->flags &= ~PS2_FLAG_CMD;
+ wake_up(&ps2dev->wait);
+ }
+}
+
+/*
+ * ps2_handle_ack() processes ACK/NAK of a command from a PS/2 device,
+ * possibly applying workarounds for mice not acknowledging the "get ID"
+ * command.
+ */
+static void ps2_handle_ack(struct ps2dev *ps2dev, u8 data)
{
switch (data) {
case PS2_RET_ACK:
@@ -436,53 +520,25 @@ bool ps2_handle_ack(struct ps2dev *ps2dev, u8 data)
*/
dev_dbg(&ps2dev->serio->dev, "unexpected %#02x\n", data);
ps2dev->flags &= ~PS2_FLAG_WAITID;
- return true;
+ return;
}
if (!ps2dev->nak)
ps2dev->flags &= ~PS2_FLAG_NAK;
ps2dev->flags &= ~PS2_FLAG_ACK;
- wake_up(&ps2dev->wait);
if (!ps2dev->nak && data != PS2_RET_ACK)
ps2_handle_response(ps2dev, data);
-
- return true;
-}
-EXPORT_SYMBOL(ps2_handle_ack);
-
-/*
- * ps2_handle_response() is supposed to be used in interrupt handler
- * to properly store device's response to a command and notify process
- * waiting for completion of the command.
- */
-
-bool ps2_handle_response(struct ps2dev *ps2dev, u8 data)
-{
- if (ps2dev->cmdcnt)
- ps2dev->cmdbuf[--ps2dev->cmdcnt] = data;
-
- if (ps2dev->flags & PS2_FLAG_CMD1) {
- ps2dev->flags &= ~PS2_FLAG_CMD1;
- if (ps2dev->cmdcnt)
- wake_up(&ps2dev->wait);
- }
-
- if (!ps2dev->cmdcnt) {
- ps2dev->flags &= ~PS2_FLAG_CMD;
+ else
wake_up(&ps2dev->wait);
- }
-
- return true;
}
-EXPORT_SYMBOL(ps2_handle_response);
/*
* Clears state of PS/2 device after communication error by resetting majority
* of flags and waking up waiters, if any.
*/
-void ps2_cmd_aborted(struct ps2dev *ps2dev)
+static void ps2_cleanup(struct ps2dev *ps2dev)
{
unsigned long old_flags = ps2dev->flags;
@@ -494,6 +550,46 @@ void ps2_cmd_aborted(struct ps2dev *ps2dev)
if (old_flags & (PS2_FLAG_ACK | PS2_FLAG_CMD))
wake_up(&ps2dev->wait);
+}
+/**
+ * ps2_interrupt - common interrupt handler for PS/2 devices
+ * @serio: serio port for the device
+ * @data: a data byte received from the device
+ * @flags: flags such as %SERIO_PARITY or %SERIO_TIMEOUT indicating state of
+ * the data transfer
+ *
+ * ps2_interrupt() invokes pre-receive handler, optionally handles command
+ * acknowledgement and response from the device, and finally passes the data
+ * to the main protocol handler for future processing.
+ */
+irqreturn_t ps2_interrupt(struct serio *serio, u8 data, unsigned int flags) {
+ struct ps2dev *ps2dev = serio_get_drvdata(serio);
+ enum ps2_disposition rc;
+
+ rc = ps2dev->pre_receive_handler(ps2dev, data, flags);
+ switch (rc) {
+ case PS2_ERROR:
+ ps2_cleanup(ps2dev);
+ break;
+
+ case PS2_IGNORE:
+ break;
+
+ case PS2_PROCESS:
+ if (ps2dev->flags & PS2_FLAG_ACK)
+ ps2_handle_ack(ps2dev, data);
+ else if (ps2dev->flags & PS2_FLAG_CMD)
+ ps2_handle_response(ps2dev, data);
+ else
+ ps2dev->receive_handler(ps2dev, data);
+ break;
+ }
+
+ return IRQ_HANDLED;
}
-EXPORT_SYMBOL(ps2_cmd_aborted);
+EXPORT_SYMBOL(ps2_interrupt);
+
+MODULE_AUTHOR("Dmitry Torokhov <dtor@mail.ru>");
+MODULE_DESCRIPTION("PS/2 driver library");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/libps2.h b/include/linux/libps2.h
index 193dd53ad18b..9ca9ce4e6e64 100644
--- a/include/linux/libps2.h
+++ b/include/linux/libps2.h
@@ -8,43 +8,59 @@
*/
#include <linux/bitops.h>
+#include <linux/interrupt.h>
#include <linux/mutex.h>
#include <linux/types.h>
#include <linux/wait.h>
-#define PS2_CMD_SETSCALE11 0x00e6
-#define PS2_CMD_SETRES 0x10e8
-#define PS2_CMD_GETID 0x02f2
-#define PS2_CMD_RESET_BAT 0x02ff
+struct ps2dev;
-#define PS2_RET_BAT 0xaa
-#define PS2_RET_ID 0x00
-#define PS2_RET_ACK 0xfa
-#define PS2_RET_NAK 0xfe
-#define PS2_RET_ERR 0xfc
+/**
+ * enum ps2_disposition - indicates how received byte should be handled
+ * @PS2_PROCESS: pass to the main protocol handler, process normally
+ * @PS2_IGNORE: skip the byte
+ * @PS2_ERROR: do not process the byte, abort command in progress
+ */
+enum ps2_disposition {
+ PS2_PROCESS,
+ PS2_IGNORE,
+ PS2_ERROR,
+};
-#define PS2_FLAG_ACK BIT(0) /* Waiting for ACK/NAK */
-#define PS2_FLAG_CMD BIT(1) /* Waiting for a command to finish */
-#define PS2_FLAG_CMD1 BIT(2) /* Waiting for the first byte of command response */
-#define PS2_FLAG_WAITID BIT(3) /* Command executing is GET ID */
-#define PS2_FLAG_NAK BIT(4) /* Last transmission was NAKed */
+typedef enum ps2_disposition (*ps2_pre_receive_handler_t)(struct ps2dev *, u8,
+ unsigned int);
+typedef void (*ps2_receive_handler_t)(struct ps2dev *, u8);
+/**
+ * struct ps2dev - represents a device using PS/2 protocol
+ * @serio: a serio port used by the PS/2 device
+ * @cmd_mutex: a mutex ensuring that only one command is executing at a time
+ * @wait: a waitqueue used to signal completion from the serio interrupt handler
+ * @flags: various internal flags indicating stages of PS/2 command execution
+ * @cmdbuf: buffer holding command response
+ * @cmdcnt: outstanding number of bytes of the command response
+ * @nak: a byte transmitted by the device when it refuses command
+ * @pre_receive_handler: checks communication errors and returns disposition
+ * (&enum ps2_disposition) of the received data byte
+ * @receive_handler: main handler of particular PS/2 protocol, such as keyboard
+ * or mouse protocol
+ */
struct ps2dev {
struct serio *serio;
-
- /* Ensures that only one command is executing at a time */
struct mutex cmd_mutex;
-
- /* Used to signal completion from interrupt handler */
wait_queue_head_t wait;
-
unsigned long flags;
u8 cmdbuf[8];
u8 cmdcnt;
u8 nak;
+
+ ps2_pre_receive_handler_t pre_receive_handler;
+ ps2_receive_handler_t receive_handler;
};
-void ps2_init(struct ps2dev *ps2dev, struct serio *serio);
+void ps2_init(struct ps2dev *ps2dev, struct serio *serio,
+ ps2_pre_receive_handler_t pre_receive_handler,
+ ps2_receive_handler_t receive_handler);
int ps2_sendbyte(struct ps2dev *ps2dev, u8 byte, unsigned int timeout);
void ps2_drain(struct ps2dev *ps2dev, size_t maxbytes, unsigned int timeout);
void ps2_begin_command(struct ps2dev *ps2dev);
@@ -52,9 +68,8 @@ void ps2_end_command(struct ps2dev *ps2dev);
int __ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command);
int ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command);
int ps2_sliced_command(struct ps2dev *ps2dev, u8 command);
-bool ps2_handle_ack(struct ps2dev *ps2dev, u8 data);
-bool ps2_handle_response(struct ps2dev *ps2dev, u8 data);
-void ps2_cmd_aborted(struct ps2dev *ps2dev);
bool ps2_is_keyboard_id(u8 id);
+irqreturn_t ps2_interrupt(struct serio *serio, u8 data, unsigned int flags);
+
#endif /* _LIBPS2_H */
--
2.40.1.606.ga4b1b128d6-goog
^ permalink raw reply related
* [PATCH 7/7] Input: libps2 - do not discard non-ack bytes when controlling LEDs
From: Dmitry Torokhov @ 2023-05-11 18:52 UTC (permalink / raw)
To: linux-input; +Cc: Raul E Rangel, linux-kernel
In-Reply-To: <20230511185252.386941-1-dmitry.torokhov@gmail.com>
Upon receiving a PS/2 command the device and controller are supposed to
stop sending normal data (scancodes or movement packets) and instead
immediately start delivering ACK/NAK and command response. Unfortunately
often EC has an output buffer which may contain latched data by the time
the EC receives a command from the host. The kernel used to ignore such
data, but that may cause "stuck" keys if the data dropped happens to be a
break code or a part of a break code. This occasionally happens, for
example, on Chromebooks when the kernel tries to toggle CapsLock LED on
a keyboard while user releases Alt+Search keyboard shortcut.
Fix this by passing the first non-ACK byte to the normal handler for a
handful of PS/2 commands that are expected to be used during normal device
operation (as opposed to probe/configuration time).
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/serio/libps2.c | 36 ++++++++++++++++++++++++++++++++----
1 file changed, 32 insertions(+), 4 deletions(-)
diff --git a/drivers/input/serio/libps2.c b/drivers/input/serio/libps2.c
index 7c5fc853072a..6d78a1fe00c1 100644
--- a/drivers/input/serio/libps2.c
+++ b/drivers/input/serio/libps2.c
@@ -21,7 +21,10 @@
#define PS2_CMD_SETSCALE11 0x00e6
#define PS2_CMD_SETRES 0x10e8
+#define PS2_CMD_EX_SETLEDS 0x20eb
+#define PS2_CMD_SETLEDS 0x10ed
#define PS2_CMD_GETID 0x02f2
+#define PS2_CMD_SETREP 0x10f3 /* Set repeat rate/set report rate */
#define PS2_CMD_RESET_BAT 0x02ff
#define PS2_RET_BAT 0xaa
@@ -35,6 +38,7 @@
#define PS2_FLAG_CMD1 BIT(2) /* Waiting for the first byte of command response */
#define PS2_FLAG_WAITID BIT(3) /* Command executing is GET ID */
#define PS2_FLAG_NAK BIT(4) /* Last transmission was NAKed */
+#define PS2_FLAG_PASS_NOACK BIT(5) /* Pass non-ACK byte to receive handler */
static int ps2_do_sendbyte(struct ps2dev *ps2dev, u8 byte,
unsigned int timeout, unsigned int max_attempts)
@@ -281,9 +285,28 @@ int __ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command)
serio_pause_rx(ps2dev->serio);
- /* Some mice do not ACK the "get ID" command, prepare to handle this. */
- ps2dev->flags = command == PS2_CMD_GETID ? PS2_FLAG_WAITID : 0;
ps2dev->cmdcnt = receive;
+
+ switch (command) {
+ case PS2_CMD_GETID:
+ /*
+ * Some mice do not ACK the "get ID" command, prepare to
+ * handle this.
+ */
+ ps2dev->flags = PS2_FLAG_WAITID;
+ break;
+
+ case PS2_CMD_SETLEDS:
+ case PS2_CMD_EX_SETLEDS:
+ case PS2_CMD_SETREP:
+ ps2dev->flags = PS2_FLAG_PASS_NOACK;
+ break;
+
+ default:
+ ps2dev->flags = 0;
+ break;
+ }
+
if (receive) {
/* Indicate that we expect response to the command. */
ps2dev->flags |= PS2_FLAG_CMD | PS2_FLAG_CMD1;
@@ -512,14 +535,19 @@ static void ps2_handle_ack(struct ps2dev *ps2dev, u8 data)
* Do not signal errors if we get unexpected reply while
* waiting for an ACK to the initial (first) command byte:
* the device might not be quiesced yet and continue
- * delivering data.
+ * delivering data. For certain commands (such as set leds and
+ * set repeat rate) that can be used during normal device
+ * operation, we even pass this data byte to the normal receive
+ * handler.
* Note that we reset PS2_FLAG_WAITID flag, so the workaround
* for mice not acknowledging the Get ID command only triggers
* on the 1st byte; if device spews data we really want to see
* a real ACK from it.
*/
dev_dbg(&ps2dev->serio->dev, "unexpected %#02x\n", data);
- ps2dev->flags &= ~PS2_FLAG_WAITID;
+ if (ps2dev->flags & PS2_FLAG_PASS_NOACK)
+ ps2dev->receive_handler(ps2dev, data);
+ ps2dev->flags &= ~(PS2_FLAG_WAITID | PS2_FLAG_PASS_NOACK);
return;
}
--
2.40.1.606.ga4b1b128d6-goog
^ permalink raw reply related
* [PATCH 4/7] Input: libps2 - fix NAK handling
From: Dmitry Torokhov @ 2023-05-11 18:52 UTC (permalink / raw)
To: linux-input; +Cc: Raul E Rangel, linux-kernel
In-Reply-To: <20230511185252.386941-1-dmitry.torokhov@gmail.com>
Do not try to process "resend" or "reject" responses from the device
as normal response data for a command.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/serio/libps2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/serio/libps2.c b/drivers/input/serio/libps2.c
index d09450eca9a7..14b70a78875d 100644
--- a/drivers/input/serio/libps2.c
+++ b/drivers/input/serio/libps2.c
@@ -445,7 +445,7 @@ bool ps2_handle_ack(struct ps2dev *ps2dev, u8 data)
ps2dev->flags &= ~PS2_FLAG_ACK;
wake_up(&ps2dev->wait);
- if (data != PS2_RET_ACK)
+ if (!ps2dev->nak && data != PS2_RET_ACK)
ps2_handle_response(ps2dev, data);
return true;
--
2.40.1.606.ga4b1b128d6-goog
^ permalink raw reply related
* [PATCH 5/7] Input: libps2 - fix aborting PS/2 commands
From: Dmitry Torokhov @ 2023-05-11 18:52 UTC (permalink / raw)
To: linux-input; +Cc: Raul E Rangel, linux-kernel
In-Reply-To: <20230511185252.386941-1-dmitry.torokhov@gmail.com>
When aborting PS/2 command the kernel should [re]set all flags before
waking up waiters, otherwise waiting thread may read obsolete values
of flags.
Reported-by: Raul Rangel <rrangel@chromium.org>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/serio/libps2.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/input/serio/libps2.c b/drivers/input/serio/libps2.c
index 14b70a78875d..09eb605364bb 100644
--- a/drivers/input/serio/libps2.c
+++ b/drivers/input/serio/libps2.c
@@ -478,15 +478,22 @@ bool ps2_handle_response(struct ps2dev *ps2dev, u8 data)
}
EXPORT_SYMBOL(ps2_handle_response);
+/*
+ * Clears state of PS/2 device after communication error by resetting majority
+ * of flags and waking up waiters, if any.
+ */
void ps2_cmd_aborted(struct ps2dev *ps2dev)
{
- if (ps2dev->flags & PS2_FLAG_ACK)
+ unsigned long old_flags = ps2dev->flags;
+
+ /* reset all flags except last nak */
+ ps2dev->flags &= PS2_FLAG_NAK;
+
+ if (old_flags & PS2_FLAG_ACK)
ps2dev->nak = 1;
- if (ps2dev->flags & (PS2_FLAG_ACK | PS2_FLAG_CMD))
+ if (old_flags & (PS2_FLAG_ACK | PS2_FLAG_CMD))
wake_up(&ps2dev->wait);
- /* reset all flags except last nack */
- ps2dev->flags &= PS2_FLAG_NAK;
}
EXPORT_SYMBOL(ps2_cmd_aborted);
--
2.40.1.606.ga4b1b128d6-goog
^ permalink raw reply related
* [PATCH 3/7] Input: libps2 - rework handling of command response
From: Dmitry Torokhov @ 2023-05-11 18:52 UTC (permalink / raw)
To: linux-input; +Cc: Raul E Rangel, linux-kernel
In-Reply-To: <20230511185252.386941-1-dmitry.torokhov@gmail.com>
It is not entirely correct that libps2 sets PS2_FLAG_CMD1 after
the device acknowledges each byte sent to the device by the host.
Rework the code so that PS2_FLAG_CMD1 and PS2_FLAG_CMD are set only once,
at the beginning of PS/2 command execution.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/serio/libps2.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/input/serio/libps2.c b/drivers/input/serio/libps2.c
index 399cda0d34f5..d09450eca9a7 100644
--- a/drivers/input/serio/libps2.c
+++ b/drivers/input/serio/libps2.c
@@ -247,14 +247,19 @@ int __ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command)
serio_pause_rx(ps2dev->serio);
+ /* Some mice do not ACK the "get ID" command, prepare to handle this. */
ps2dev->flags = command == PS2_CMD_GETID ? PS2_FLAG_WAITID : 0;
ps2dev->cmdcnt = receive;
- if (receive && param)
- for (i = 0; i < receive; i++)
- ps2dev->cmdbuf[(receive - 1) - i] = param[i];
+ if (receive) {
+ /* Indicate that we expect response to the command. */
+ ps2dev->flags |= PS2_FLAG_CMD | PS2_FLAG_CMD1;
+ if (param)
+ for (i = 0; i < receive; i++)
+ ps2dev->cmdbuf[(receive - 1) - i] = param[i];
+ }
/*
- * Some devices (Synaptics) peform the reset before
+ * Some devices (Synaptics) perform the reset before
* ACKing the reset command, and so it can take a long
* time before the ACK arrives.
*/
@@ -434,11 +439,8 @@ bool ps2_handle_ack(struct ps2dev *ps2dev, u8 data)
return true;
}
- if (!ps2dev->nak) {
+ if (!ps2dev->nak)
ps2dev->flags &= ~PS2_FLAG_NAK;
- if (ps2dev->cmdcnt)
- ps2dev->flags |= PS2_FLAG_CMD | PS2_FLAG_CMD1;
- }
ps2dev->flags &= ~PS2_FLAG_ACK;
wake_up(&ps2dev->wait);
--
2.40.1.606.ga4b1b128d6-goog
^ 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