* Re: [BUG][NEW DATA] Kmemleak, possibly hiddev_connect(), in 6.3.0+ torvalds tree commit gfc4354c6e5c2
From: Mirsad Goran Todorovac @ 2023-05-17 7:24 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Mirsad Goran Todorovac, linux-usb, linux-kernel, linux-input,
Benjamin Tissoires, Jiri Kosina
In-Reply-To: <2023051607-sturdy-jiffy-ca99@gregkh>
On 16.5.2023. 16:36, Greg Kroah-Hartman wrote:
> On Fri, May 12, 2023 at 11:33:31PM +0200, Mirsad Goran Todorovac wrote:
>> Hi,
>>
>> On 5/9/23 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.
>>
>> I have figured out that the leak occurs on keyboard unplugging only, one
>> or two leaks (maybe a race condition?).
>>
>> Please NOTE that the number of leaks is now odd:
>>
>> root@defiant:/home/marvin# cat /sys/kernel/debug/kmemleak | grep comm
>> comm "systemd-udevd", pid 330, jiffies 4294892588 (age 715.772s)
>> comm "systemd-udevd", pid 330, jiffies 4294892588 (age 715.772s)
>> comm "kworker/6:0", pid 54, jiffies 4294907989 (age 654.224s)
>> comm "kworker/6:0", pid 54, jiffies 4294907989 (age 654.272s)
>> comm "kworker/6:3", pid 3046, jiffies 4294935362 (age 544.780s)
>> comm "kworker/6:0", pid 54, jiffies 4294964122 (age 429.740s)
>> comm "kworker/6:0", pid 54, jiffies 4294964122 (age 429.784s)
>> root@defiant:/home/marvin#
>>
>> At one time unplugging keyboard generated only one leak, but only at one
>> time. As it requires manually unplugging keyboard, I didn't seem to find a
>> way to automate it, but it doesn't seem to require root access.
>>
>> BTW, I've seen in syzbot output that kmemleak output has debug source file
>> names and line numbers. I couldn't make that work with the dbg .deb.
>>
>> I will do some more homework, but this was a rough week.
>
> I made up a patch based on code inspection alone, as I couldn't
> reproduce this locally at all:
> https://lore.kernel.org/r/2023051628-thumb-boaster-5680@gregkh
> and it seemed to pass syzbot's tests.
>
> I've included it here below, can you test it as well?
>
> Hm, I only tested with a USB mouse unplug/plug cycle, maybe the issue is
> a keyboard?
>
> thanks,
>
> greg k-h
Hi, Greg,
Yes, the problem was with the keyboard, and with unplugging, mouse unplugged
w/o problems.
I will test the patch in the afternoon, as the issue did not appear on the
work computer. Probably it is for the best that the exact environment is
reproduced for the test.
Best regards,
Mirsad
> -------------
>
> diff --git a/drivers/base/class.c b/drivers/base/class.c
> index ac1808d1a2e8..9b44edc8416f 100644
> --- a/drivers/base/class.c
> +++ b/drivers/base/class.c
> @@ -320,6 +322,7 @@ void class_dev_iter_init(struct class_dev_iter *iter, const struct class *class,
> start_knode = &start->p->knode_class;
> klist_iter_init_node(&sp->klist_devices, &iter->ki, start_knode);
> iter->type = type;
> + iter->sp = sp;
> }
> EXPORT_SYMBOL_GPL(class_dev_iter_init);
>
> @@ -361,6 +364,7 @@ EXPORT_SYMBOL_GPL(class_dev_iter_next);
> void class_dev_iter_exit(struct class_dev_iter *iter)
> {
> klist_iter_exit(&iter->ki);
> + subsys_put(iter->sp);
> }
> EXPORT_SYMBOL_GPL(class_dev_iter_exit);
>
> diff --git a/include/linux/device/class.h b/include/linux/device/class.h
> index 9deeaeb457bb..abf3d3bfb6fe 100644
> --- a/include/linux/device/class.h
> +++ b/include/linux/device/class.h
> @@ -74,6 +74,7 @@ struct class {
> struct class_dev_iter {
> struct klist_iter ki;
> const struct device_type *type;
> + struct subsys_private *sp;
> };
>
> int __must_check class_register(const struct class *class);
--
Mirsad Todorovac
System engineer
Faculty of Graphic Arts | Academy of Fine Arts
University of Zagreb
Republic of Croatia, the European Union
Sistem inženjer
Grafički fakultet | Akademija likovnih umjetnosti
Sveučilište u Zagrebu
^ permalink raw reply
* Re: [PATCH v4 1/2] dt-bindings: gpio: Convert STMPE GPIO to YAML schema
From: Linus Walleij @ 2023-05-16 18:09 UTC (permalink / raw)
To: Bartosz Golaszewski, Lee Jones
Cc: Rob Herring, Krzysztof Kozlowski, Maxime Coquelin,
Alexandre Torgue, Dmitry Torokhov, Philippe Schenker,
Stefan Agner, Marek Vasut, Steffen Trumtrar, linux-gpio,
devicetree, linux-stm32, linux-arm-kernel, linux-kernel,
linux-input
In-Reply-To: <CAMRc=Mf+RsU6PT7fwm=r9OLbmxNjiv9Ru8HEfpMEAqDN5-0Qig@mail.gmail.com>
On Tue, May 16, 2023 at 5:34 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> On Thu, May 11, 2023 at 10:39 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> >
> > 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
>
> Sure:
>
> The following changes since commit ac9a78681b921877518763ba0e89202254349d1b:
>
> Linux 6.4-rc1 (2023-05-07 13:34:35 -0700)
>
> are available in the Git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git
> tags/v6.4-stmpe-gpio
>
> for you to fetch changes up to 6c19974d1e83fba2cca1cbea2fbf250f093eb5ed:
>
> dt-bindings: gpio: Convert STMPE GPIO to YAML schema (2023-05-11
> 16:58:04 +0200)
Hey cool :D
Lee if you pull this in you can apply 2/2 on top if it checks out.
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH] Input: tests: add test to cover all input_grab_device() function
From: Dana Elfassy @ 2023-05-16 16:24 UTC (permalink / raw)
To: eballetb, dmitry.torokhov, javierm, linux-input, linux-kernel
Cc: Dana Elfassy
Currently input_grab_device() isn't covered by any tests
Thus, adding a test to cover the cases:
1. The device is grabbed successfully
2. Trying to grab a device that is already grabbed by another input
handle
Signed-off-by: Dana Elfassy <dangel101@gmail.com>
---
drivers/input/tests/input_test.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/drivers/input/tests/input_test.c b/drivers/input/tests/input_test.c
index 25bbf51b5c87..cd4db365e9fa 100644
--- a/drivers/input/tests/input_test.c
+++ b/drivers/input/tests/input_test.c
@@ -124,10 +124,38 @@ static void input_test_match_device_id(struct kunit *test)
KUNIT_ASSERT_FALSE(test, input_match_device_id(input_dev, &id));
}
+
+static void input_test_grab(struct kunit *test)
+{
+ struct input_dev *input_dev = test->priv;
+ struct input_handle test_handle;
+ struct input_handler handler;
+ struct input_handle handle;
+ struct input_device_id id;
+ int res;
+
+ handler.name = "handler";
+ handler.id_table = &id;
+
+ handle.dev = input_get_device(input_dev);
+ handle.name = dev_name(&input_dev->dev);
+ handle.handler = &handler;
+ res = input_grab_device(&handle);
+ KUNIT_ASSERT_TRUE(test, input_grab_device(&handle));
+
+ test_handle.dev = input_get_device(input_dev);
+ test_handle.name = dev_name(&input_dev->dev);
+ test_handle.handler = &handler;
+
+ res = input_grab_device(&test_handle);
+ KUNIT_ASSERT_EQ(test, res, -EBUSY);
+}
+
static struct kunit_case input_tests[] = {
KUNIT_CASE(input_test_polling),
KUNIT_CASE(input_test_timestamp),
KUNIT_CASE(input_test_match_device_id),
+ KUNIT_CASE(input_test_grab),
{ /* sentinel */ }
};
--
2.40.1
^ permalink raw reply related
* Your urgent feedback is highly appreciated.
From: Eun-ji Ha @ 2023-05-16 12:06 UTC (permalink / raw)
To: linux-input
Good day,
My name is Mrs. Eun-ji Ha.
I would like to get in touch with you.
Did you receive my previous email for investment purposes?
Please send your telephone number to enhance communication.
Have a great day!
Best regards,
^ permalink raw reply
* Re: [PATCH v4 1/2] dt-bindings: gpio: Convert STMPE GPIO to YAML schema
From: Bartosz Golaszewski @ 2023-05-16 15:34 UTC (permalink / raw)
To: Linus Walleij, Lee Jones
Cc: Rob Herring, Krzysztof Kozlowski, Maxime Coquelin,
Alexandre Torgue, Dmitry Torokhov, Philippe Schenker,
Stefan Agner, Marek Vasut, Steffen Trumtrar, linux-gpio,
devicetree, linux-stm32, linux-arm-kernel, linux-kernel,
linux-input
In-Reply-To: <CACRpkdaJrB1f13LB4aHSWys63448a4NQZORgwdk8z=C8qe-BDA@mail.gmail.com>
On Thu, May 11, 2023 at 10:39 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> 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
Sure:
The following changes since commit ac9a78681b921877518763ba0e89202254349d1b:
Linux 6.4-rc1 (2023-05-07 13:34:35 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git
tags/v6.4-stmpe-gpio
for you to fetch changes up to 6c19974d1e83fba2cca1cbea2fbf250f093eb5ed:
dt-bindings: gpio: Convert STMPE GPIO to YAML schema (2023-05-11
16:58:04 +0200)
Bart
^ permalink raw reply
* Re: [BUG][NEW DATA] Kmemleak, possibly hiddev_connect(), in 6.3.0+ torvalds tree commit gfc4354c6e5c2
From: Greg Kroah-Hartman @ 2023-05-16 14:36 UTC (permalink / raw)
To: Mirsad Goran Todorovac
Cc: Mirsad Goran Todorovac, linux-usb, linux-kernel, linux-input,
Benjamin Tissoires, Jiri Kosina
In-Reply-To: <987f9008-7eac-e2a4-31f6-8479f0e4a626@alu.unizg.hr>
On Fri, May 12, 2023 at 11:33:31PM +0200, Mirsad Goran Todorovac wrote:
> Hi,
>
> On 5/9/23 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.
>
> I have figured out that the leak occurs on keyboard unplugging only, one
> or two leaks (maybe a race condition?).
>
> Please NOTE that the number of leaks is now odd:
>
> root@defiant:/home/marvin# cat /sys/kernel/debug/kmemleak | grep comm
> comm "systemd-udevd", pid 330, jiffies 4294892588 (age 715.772s)
> comm "systemd-udevd", pid 330, jiffies 4294892588 (age 715.772s)
> comm "kworker/6:0", pid 54, jiffies 4294907989 (age 654.224s)
> comm "kworker/6:0", pid 54, jiffies 4294907989 (age 654.272s)
> comm "kworker/6:3", pid 3046, jiffies 4294935362 (age 544.780s)
> comm "kworker/6:0", pid 54, jiffies 4294964122 (age 429.740s)
> comm "kworker/6:0", pid 54, jiffies 4294964122 (age 429.784s)
> root@defiant:/home/marvin#
>
> At one time unplugging keyboard generated only one leak, but only at one
> time. As it requires manually unplugging keyboard, I didn't seem to find a
> way to automate it, but it doesn't seem to require root access.
>
> BTW, I've seen in syzbot output that kmemleak output has debug source file
> names and line numbers. I couldn't make that work with the dbg .deb.
>
> I will do some more homework, but this was a rough week.
I made up a patch based on code inspection alone, as I couldn't
reproduce this locally at all:
https://lore.kernel.org/r/2023051628-thumb-boaster-5680@gregkh
and it seemed to pass syzbot's tests.
I've included it here below, can you test it as well?
Hm, I only tested with a USB mouse unplug/plug cycle, maybe the issue is
a keyboard?
thanks,
greg k-h
-------------
diff --git a/drivers/base/class.c b/drivers/base/class.c
index ac1808d1a2e8..9b44edc8416f 100644
--- a/drivers/base/class.c
+++ b/drivers/base/class.c
@@ -320,6 +322,7 @@ void class_dev_iter_init(struct class_dev_iter *iter, const struct class *class,
start_knode = &start->p->knode_class;
klist_iter_init_node(&sp->klist_devices, &iter->ki, start_knode);
iter->type = type;
+ iter->sp = sp;
}
EXPORT_SYMBOL_GPL(class_dev_iter_init);
@@ -361,6 +364,7 @@ EXPORT_SYMBOL_GPL(class_dev_iter_next);
void class_dev_iter_exit(struct class_dev_iter *iter)
{
klist_iter_exit(&iter->ki);
+ subsys_put(iter->sp);
}
EXPORT_SYMBOL_GPL(class_dev_iter_exit);
diff --git a/include/linux/device/class.h b/include/linux/device/class.h
index 9deeaeb457bb..abf3d3bfb6fe 100644
--- a/include/linux/device/class.h
+++ b/include/linux/device/class.h
@@ -74,6 +74,7 @@ struct class {
struct class_dev_iter {
struct klist_iter ki;
const struct device_type *type;
+ struct subsys_private *sp;
};
int __must_check class_register(const struct class *class);
^ permalink raw reply related
* Re: [regression] Since kernel 6.3.1 logitech unify receiver not working properly
From: Thorsten Leemhuis @ 2023-05-16 13:53 UTC (permalink / raw)
To: Bagas Sanjaya, Linux regressions mailing list, Filipe Laíns,
Bastien Nocera, Jiri Kosina, Benjamin Tissoires, Linus Torvalds
Cc: open list:HID CORE LAYER, LKML, guy.b
In-Reply-To: <b7717c43-74bf-b91d-d3ce-874493df602c@gmail.com>
On 16.05.23 15:24, Bagas Sanjaya wrote:
> On 5/11/23 18:58, Thorsten Leemhuis wrote:
>> Hi, Thorsten here, the Linux kernel's regression tracker.
>>
>> On 08.05.23 11:55, Linux regression tracking (Thorsten Leemhuis) wrote:
>>> Hi, Thorsten here, the Linux kernel's regression tracker.
>>>
>>> I noticed a regression report in bugzilla.kernel.org. As many (most?)
>>> kernel developers don't keep an eye on it, I decided to forward it by mail.
>>>
>>> Quoting from https://bugzilla.kernel.org/show_bug.cgi?id=217412 :
>>
>> TWIMC: a few other users (three or four iirc) showed up in that ticket
>> and reported problems with the receiver, albeit the symptoms are not
>> exactly the same for all of them, so there might be more than one problem.
>>
>> I'll try to motivate the affected users to perform a bisection. But
>> would be great if those with more knowledge about this code could
>> briefly look into the ticket, maybe the details the users shared allows
>> one of you to guess what causes this.
>
> Hmm,
>
> You noted in the similar report [1] that developers involved here
> ignore this regressions. I wonder if Linus has to be hired in
> this case, and if it is the case, let's take a look and hear closely what
> he will say.
>
> [1]: https://lore.kernel.org/regressions/8941c5f2-3861-da68-06ca-adc68a37e53b@leemhuis.info/
You CCed him so maybe we'll learn soon.
I expect he doesn't like the situation, but at the same time I guess
there is nothing much he will do (which is why I do not CC him in cases
like this, unless they are urgent/severe or something like that).
That's because as far as I know in the end it is the duty of the
reporter(s) to find the the culprit.
Because in the end developers and subsystem maintainers are volunteers,
too -- and making them bisect each and every report would make the job
way to hard. And the question "which developer/subsystem maintainer
needs to perform the bisection" would often become quickly complicated
as well, as an issue in one area of the kernel can be caused by a change
in a totally different area (for file-systems that is way more likely
than for input drivers, but still). Not to mention that developer and
subsystem maintainers might not even have the environment at hand to
reproduce the issue.
That being said: I think a quick "We looked into these three reports
that might be related, but we have no idea what might cause this;
somebody needs to bisect things" from one of the involved
developers/maintainers would have been nice (appropriate?).
Ciao, Thorsten
^ permalink raw reply
* Re: [regression] Since kernel 6.3.1 logitech unify receiver not working properly
From: Benjamin Tissoires @ 2023-05-16 13:34 UTC (permalink / raw)
To: Bagas Sanjaya
Cc: Linux regressions mailing list, Filipe Laíns, Bastien Nocera,
Jiri Kosina, Linus Torvalds, open list:HID CORE LAYER, LKML,
guy.b
In-Reply-To: <b7717c43-74bf-b91d-d3ce-874493df602c@gmail.com>
On Tue, May 16, 2023 at 3:25 PM Bagas Sanjaya <bagasdotme@gmail.com> wrote:
>
> On 5/11/23 18:58, Thorsten Leemhuis wrote:
> > Hi, Thorsten here, the Linux kernel's regression tracker.
> >
> > On 08.05.23 11:55, Linux regression tracking (Thorsten Leemhuis) wrote:
> >> Hi, Thorsten here, the Linux kernel's regression tracker.
> >>
> >> I noticed a regression report in bugzilla.kernel.org. As many (most?)
> >> kernel developers don't keep an eye on it, I decided to forward it by mail.
> >>
> >> Quoting from https://bugzilla.kernel.org/show_bug.cgi?id=217412 :
> >
> > TWIMC: a few other users (three or four iirc) showed up in that ticket
> > and reported problems with the receiver, albeit the symptoms are not
> > exactly the same for all of them, so there might be more than one problem.
> >
> > I'll try to motivate the affected users to perform a bisection. But
> > would be great if those with more knowledge about this code could
> > briefly look into the ticket, maybe the details the users shared allows
> > one of you to guess what causes this.
> >
>
> Hmm,
>
> You noted in the similar report [1] that developers involved here
> ignore this regressions. I wonder if Linus has to be hired in
> this case, and if it is the case, let's take a look and hear closely what
> he will say.
Sigh... Not answering an email is bad, but maybe you can also
understand that developers can take days off?
And it turns out that I was waiting for Bastien to chime in, but I can
access his calendar too and just found out that he was AFK for the
entire month, except for the first week, which I wasn't aware. May is
a time where people in France have a lot of public holidays and is
also the cut to use those time off or they expire.
For me, I'll also be taking time off the rest of this week, so I won't
be able to have a look at this until next week at the earliest.
Cheers,
Benjamin
>
> Thanks.
>
>
> [1]: https://lore.kernel.org/regressions/8941c5f2-3861-da68-06ca-adc68a37e53b@leemhuis.info/
>
> --
> An old man doll... just what I always wanted! - Clara
>
^ permalink raw reply
* Re: [regression] Since kernel 6.3.1 logitech unify receiver not working properly
From: Bagas Sanjaya @ 2023-05-16 13:24 UTC (permalink / raw)
To: Linux regressions mailing list, Filipe Laíns, Bastien Nocera,
Jiri Kosina, Benjamin Tissoires, Linus Torvalds
Cc: open list:HID CORE LAYER, LKML, guy.b
In-Reply-To: <bec024d5-4088-00ae-f7b5-7188868b1707@leemhuis.info>
On 5/11/23 18:58, Thorsten Leemhuis wrote:
> Hi, Thorsten here, the Linux kernel's regression tracker.
>
> On 08.05.23 11:55, Linux regression tracking (Thorsten Leemhuis) wrote:
>> Hi, Thorsten here, the Linux kernel's regression tracker.
>>
>> I noticed a regression report in bugzilla.kernel.org. As many (most?)
>> kernel developers don't keep an eye on it, I decided to forward it by mail.
>>
>> Quoting from https://bugzilla.kernel.org/show_bug.cgi?id=217412 :
>
> TWIMC: a few other users (three or four iirc) showed up in that ticket
> and reported problems with the receiver, albeit the symptoms are not
> exactly the same for all of them, so there might be more than one problem.
>
> I'll try to motivate the affected users to perform a bisection. But
> would be great if those with more knowledge about this code could
> briefly look into the ticket, maybe the details the users shared allows
> one of you to guess what causes this.
>
Hmm,
You noted in the similar report [1] that developers involved here
ignore this regressions. I wonder if Linus has to be hired in
this case, and if it is the case, let's take a look and hear closely what
he will say.
Thanks.
[1]: https://lore.kernel.org/regressions/8941c5f2-3861-da68-06ca-adc68a37e53b@leemhuis.info/
--
An old man doll... just what I always wanted! - Clara
^ permalink raw reply
* Re: [BUG: 6.3 kernel] Logitech Trackball M575 misidentified
From: Linux regression tracking (Thorsten Leemhuis) @ 2023-05-16 12:33 UTC (permalink / raw)
To: Xose Vazquez Perez, linux-input
Cc: Linux kernel regressions list, Bastien Nocera, Filipe Laíns,
Jiri Kosina, Benjamin Tissoires, LKML
In-Reply-To: <eeb19342-3499-a1fb-388f-d4670472b16c@gmail.com>
[CCing a few people and the the regression list, as it should be in the
loop for regressions:
https://docs.kernel.org/admin-guide/reporting-regressions.html]
On 11.05.23 23:22, Xose Vazquez Perez wrote:
>
> 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
I wonder if this if this is some related to this issue:
https://bugzilla.kernel.org/show_bug.cgi?id=217412
("Since kernel 6.3.1 logitech unify receiver not working properly")
That one so far seems to be ignored by the developers. Your report one
also didn't get any reply yet.
Could you maybe perform a bisection to get down to this?
Side note: there is also
https://bugzilla.kernel.org/show_bug.cgi?id=217330
("Broken Logitech unifying battery names in hid-next tree")
Anyway, for the rest of this mail:
[TLDR: I'm adding this report to the list of tracked Linux kernel
regressions; the text you find below is based on a few templates
paragraphs you might have encountered already in similar form.
See link in footer if these mails annoy you.]
Thanks for the report. To be sure the issue doesn't fall through the
cracks unnoticed, I'm adding it to regzbot, the Linux kernel regression
tracking bot:
#regzbot ^introduced v6.2..v6.3
#regzbot title input: Logitech Trackball M575 misidentified
#regzbot ignore-activity
This isn't a regression? This issue or a fix for it are already
discussed somewhere else? It was fixed already? You want to clarify when
the regression started to happen? Or point out I got the title or
something else totally wrong? Then just reply and tell me -- ideally
while also telling regzbot about it, as explained by the page listed in
the footer of this mail.
Developers: When fixing the issue, remember to add 'Link:' tags pointing
to the report (the parent of this mail). See page linked in footer for
details.
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
That page also explains what to do if mails like this annoy you.
^ permalink raw reply
* [PATCH v4 13/41] Input: add HAS_IOPORT dependencies
From: Niklas Schnelle @ 2023-05-16 11:00 UTC (permalink / raw)
To: Arnd Bergmann, Dmitry Torokhov
Cc: Greg Kroah-Hartman, Bjorn Helgaas, Uwe Kleine-König,
Mauro Carvalho Chehab, Alan Stern, Rafael J. Wysocki,
Geert Uytterhoeven, Paul Walmsley, Palmer Dabbelt, Albert Ou,
linux-kernel, linux-arch, linux-pci, Arnd Bergmann, linux-input
In-Reply-To: <20230516110038.2413224-1-schnelle@linux.ibm.com>
In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
not being declared. We thus need to add HAS_IOPORT as dependency for
those drivers using them.
Co-developed-by: Arnd Bergmann <arnd@kernel.org>
Signed-off-by: Arnd Bergmann <arnd@kernel.org>
Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
---
Note: The HAS_IOPORT Kconfig option was added in v6.4-rc1 so
per-subsystem patches may be applied independently
drivers/input/serio/Kconfig | 1 +
drivers/input/touchscreen/Kconfig | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index f39b7b3f7942..17edc1597446 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -148,6 +148,7 @@ config HIL_MLC
config SERIO_PCIPS2
tristate "PCI PS/2 keyboard and PS/2 mouse controller"
depends on PCI
+ depends on HAS_IOPORT
help
Say Y here if you have a Mobility Docking station with PS/2
keyboard and mice ports.
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 143ff43c67ae..c2cbd332af1d 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -700,6 +700,7 @@ config TOUCHSCREEN_INEXIO
config TOUCHSCREEN_MK712
tristate "ICS MicroClock MK712 touchscreen"
+ depends on ISA
help
Say Y here if you have the ICS MicroClock MK712 touchscreen
controller chip in your system.
--
2.39.2
^ permalink raw reply related
* [PATCH v4 14/41] Input: gameport: add ISA and HAS_IOPORT dependencies
From: Niklas Schnelle @ 2023-05-16 11:00 UTC (permalink / raw)
To: Arnd Bergmann, Dmitry Torokhov
Cc: Greg Kroah-Hartman, Bjorn Helgaas, Uwe Kleine-König,
Mauro Carvalho Chehab, Alan Stern, Rafael J. Wysocki,
Geert Uytterhoeven, Paul Walmsley, Palmer Dabbelt, Albert Ou,
linux-kernel, linux-arch, linux-pci, Arnd Bergmann, linux-input
In-Reply-To: <20230516110038.2413224-1-schnelle@linux.ibm.com>
In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
not being declared. As ISA already implies HAS_IOPORT we can simply add
this dependency and guard sections of code using inb()/outb() as
alternative access methods.
Co-developed-by: Arnd Bergmann <arnd@kernel.org>
Signed-off-by: Arnd Bergmann <arnd@kernel.org>
Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
---
Note: The HAS_IOPORT Kconfig option was added in v6.4-rc1 so
per-subsystem patches may be applied independently
drivers/input/gameport/Kconfig | 4 +++-
include/linux/gameport.h | 9 +++++++--
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/input/gameport/Kconfig b/drivers/input/gameport/Kconfig
index 5a2c2fb3217d..fe73b26e647a 100644
--- a/drivers/input/gameport/Kconfig
+++ b/drivers/input/gameport/Kconfig
@@ -25,6 +25,7 @@ if GAMEPORT
config GAMEPORT_NS558
tristate "Classic ISA and PnP gameport support"
+ depends on ISA
help
Say Y here if you have an ISA or PnP gameport.
@@ -35,6 +36,7 @@ config GAMEPORT_NS558
config GAMEPORT_L4
tristate "PDPI Lightning 4 gamecard support"
+ depends on ISA
help
Say Y here if you have a PDPI Lightning 4 gamecard.
@@ -53,7 +55,7 @@ config GAMEPORT_EMU10K1
config GAMEPORT_FM801
tristate "ForteMedia FM801 gameport support"
- depends on PCI
+ depends on PCI && HAS_IOPORT
help
Say Y here if you have ForteMedia FM801 PCI audio controller
(Abit AU10, Genius Sound Maker, HP Workstation zx2000,
diff --git a/include/linux/gameport.h b/include/linux/gameport.h
index 8c2f00018e89..4d5720022b63 100644
--- a/include/linux/gameport.h
+++ b/include/linux/gameport.h
@@ -167,16 +167,21 @@ static inline void gameport_trigger(struct gameport *gameport)
{
if (gameport->trigger)
gameport->trigger(gameport);
+#ifdef CONFIG_HAS_IOPORT
else
outb(0xff, gameport->io);
+#endif
}
static inline unsigned char gameport_read(struct gameport *gameport)
{
if (gameport->read)
return gameport->read(gameport);
- else
- return inb(gameport->io);
+#ifdef CONFIG_HAS_IOPORT
+ return inb(gameport->io);
+#else
+ return 0xff;
+#endif
}
static inline int gameport_cooked_read(struct gameport *gameport, int *axes, int *buttons)
--
2.39.2
^ permalink raw reply related
* Re: [PATCH v2 2/4] dt-bindings: touchscreen: add virtual-touchscreen and virtual-buttons properties
From: Javier Carrasco @ 2023-05-16 9:03 UTC (permalink / raw)
To: Krzysztof Kozlowski, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Henrik Rydberg, Bastian Hecht,
Michael Riesch
Cc: linux-kernel, linux-input, devicetree
In-Reply-To: <99e39fb6-26e0-eb9a-3c07-e07d1fa4122c@linaro.org>
On 16.05.23 10:13, Krzysztof Kozlowski wrote:
> On 16/05/2023 10:10, Krzysztof Kozlowski wrote:
>> On 15/05/2023 17:00, 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.
>>>
>>> Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
>>> ---
>>
>>
>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>
> Apologies, second thoughts - why calling all this binding and properties
> "virtual"? That's the word which immediately raises questions, because
> bindings are only for real things, not virtual.
>
> Touchscreen is just clipped, not virtual, so maybe "clipped-area"
> instead of virtual-touchscreen? Buttons are real, so maybe just "buttons"?
>
> Best regards,
> Krzysztof
>
I guess it is a matter of perspective. For a user the buttons and the
clipped area are 100% real, but for a driver developer they are virtual
in the sense that there is not an "active" hardware behind apart from
the original touchscreen.
I just wanted to avoid misunderstandings when implementing this feature
for other drivers. One might wonder if the touchscreen now has
mechanical keys attached to it. With the "virtual-" prefix it is clear
that the objects are not additional pieces of hardware or extensions of
the touchscreen functionality.
For the virtual-touchscreen your point is stronger because there is
indeed a real touchscreen hardware no matter the area you define, but my
approach was keeping homogeneous names for the different objects in case
some new ones might appear in the future: every object that gets on top
of the touchscreen area is virtual, so add a new object type and name it
virtual-xxx.
I have nothing against about doing some renaming and I will do it if it
is required, but with the documentation I think it is now more clear
what everything means and in the end it might make more sense for the
drivers so they can differentiate between real and virtual devices.
^ permalink raw reply
* Re: [PATCH v2 2/4] dt-bindings: touchscreen: add virtual-touchscreen and virtual-buttons properties
From: Krzysztof Kozlowski @ 2023-05-16 8:13 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: <58c8b822-8b47-3269-3b78-334b53c20bff@linaro.org>
On 16/05/2023 10:10, Krzysztof Kozlowski wrote:
> On 15/05/2023 17:00, 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.
>>
>> Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
>> ---
>
>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Apologies, second thoughts - why calling all this binding and properties
"virtual"? That's the word which immediately raises questions, because
bindings are only for real things, not virtual.
Touchscreen is just clipped, not virtual, so maybe "clipped-area"
instead of virtual-touchscreen? Buttons are real, so maybe just "buttons"?
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2 2/4] dt-bindings: touchscreen: add virtual-touchscreen and virtual-buttons properties
From: Krzysztof Kozlowski @ 2023-05-16 8:10 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: <20230510-feature-ts_virtobj_patch-v2-2-f68a6bfe7a0f@wolfvision.net>
On 15/05/2023 17:00, 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.
>
> Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
> ---
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 00/43] ep93xx device tree conversion
From: Nikita Shubin @ 2023-05-16 10:37 UTC (permalink / raw)
To: Florian Fainelli
Cc: Arnd Bergmann, Linus Walleij, Alexander Sverdlin, David S. Miller,
Jonathan Neuschäfer, Russell King (Oracle),
Uwe Kleine-König, Alessandro Zummo, Alexander Gordeev,
Alexandre Belloni, Andy Shevchenko, Arnd Bergmann,
Bartosz Golaszewski, Brian Norris, Chuanhong Guo, Conor Dooley,
Damien Le Moal, Daniel Lezcano, Dmitry Torokhov,
Emil Renner Berthing, Eric Dumazet, Guenter Roeck,
Hartley Sweeten, Heiko Stuebner, Hitomi Hasegawa, Jakub Kicinski,
Jaroslav Kysela, Jean Delvare, Joel Stanley, Jonathan Cameron,
Krzysztof Kozlowski, Le Moal, Liam Girdwood, Liang Yang,
Linus Walleij, Lukasz Majewski, Lv Ruyi, Mark Brown,
Masahiro Yamada, Michael Turquette, Miquel Raynal,
Nathan Chancellor, Nick Desaulniers, Nicolas Saenz Julienne,
Olof Johansson, Paolo Abeni, Qin Jian, Richard Weinberger,
Rob Herring, Robert Jarzmik, Russell King, Sebastian Reichel,
Sergey Shtylyov, Stephen Boyd, Sumanth Korikkar, Sven Peter,
Takashi Iwai, Thierry Reding, Thomas Gleixner, Ulf Hansson,
Vasily Gorbik, Vignesh Raghavendra, Vinod Koul, Walker Chen,
Wim Van Sebroeck, Yinbo Zhu, alsa-devel, devicetree, dmaengine,
linux-arm-kernel, linux-clk, linux-gpio, linux-ide, linux-input,
linux-kernel, linux-mtd, linux-pm, linux-pwm, linux-rtc,
linux-spi, linux-watchdog, netdev, soc
In-Reply-To: <1ff2333a-8f78-c066-0158-9c8a1a17684f@gmail.com>
Hello Florian!
On Mon, 2023-05-15 at 20:47 -0700, Florian Fainelli wrote:
>
>
> On 4/24/2023 5:34 AM, Nikita Shubin wrote:
> > This series aims to convert ep93xx from platform to full device
> > tree support.
> >
> > Tested on ts7250 64 RAM/128 MiB Nand flash, edb9302.
> >
> > Thank you Linus and Arnd for your support, review and comments,
> > sorry if i missed something -
> > these series are quite big for me.
> >
> > Big thanks to Alexander Sverdlin for his testing, support, review,
> > fixes and patches.
>
> If anyone is interested I still have a TS-7300 board [1] that is
> fully
> functional and could be sent out to a new home.
Thank you kindly, i'll keep this in mind !
>
> https://www.embeddedts.com/products/TS-7300
^ permalink raw reply
* Re: [PATCH 00/43] ep93xx device tree conversion
From: Florian Fainelli @ 2023-05-16 3:47 UTC (permalink / raw)
To: Nikita Shubin
Cc: Arnd Bergmann, Linus Walleij, Alexander Sverdlin, David S. Miller,
Jonathan Neuschäfer, Russell King (Oracle),
Uwe Kleine-König, Alessandro Zummo, Alexander Gordeev,
Alexandre Belloni, Andy Shevchenko, Arnd Bergmann,
Bartosz Golaszewski, Brian Norris, Chuanhong Guo, Conor Dooley,
Damien Le Moal, Daniel Lezcano, Dmitry Torokhov,
Emil Renner Berthing, Eric Dumazet, Guenter Roeck,
Hartley Sweeten, Heiko Stuebner, Hitomi Hasegawa, Jakub Kicinski,
Jaroslav Kysela, Jean Delvare, Joel Stanley, Jonathan Cameron,
Krzysztof Kozlowski, Le Moal, Liam Girdwood, Liang Yang,
Linus Walleij, Lukasz Majewski, Lv Ruyi, Mark Brown,
Masahiro Yamada, Michael Turquette, Miquel Raynal,
Nathan Chancellor, Nick Desaulniers, Nicolas Saenz Julienne,
Olof Johansson, Paolo Abeni, Qin Jian, Richard Weinberger,
Rob Herring, Robert Jarzmik, Russell King, Sebastian Reichel,
Sergey Shtylyov, Stephen Boyd, Sumanth Korikkar, Sven Peter,
Takashi Iwai, Thierry Reding, Thomas Gleixner, Ulf Hansson,
Vasily Gorbik, Vignesh Raghavendra, Vinod Koul, Walker Chen,
Wim Van Sebroeck, Yinbo Zhu, alsa-devel, devicetree, dmaengine,
linux-arm-kernel, linux-clk, linux-gpio, linux-ide, linux-input,
linux-kernel, linux-mtd, linux-pm, linux-pwm, linux-rtc,
linux-spi, linux-watchdog, netdev, soc
In-Reply-To: <20230424123522.18302-1-nikita.shubin@maquefel.me>
On 4/24/2023 5:34 AM, Nikita Shubin wrote:
> This series aims to convert ep93xx from platform to full device tree support.
>
> Tested on ts7250 64 RAM/128 MiB Nand flash, edb9302.
>
> Thank you Linus and Arnd for your support, review and comments, sorry if i missed something -
> these series are quite big for me.
>
> Big thanks to Alexander Sverdlin for his testing, support, review, fixes and patches.
If anyone is interested I still have a TS-7300 board [1] that is fully
functional and could be sent out to a new home.
https://www.embeddedts.com/products/TS-7300
--
Florian
^ permalink raw reply
* [PATCH v2 6/7] Input: libps2 - introduce common interrupt handler
From: Dmitry Torokhov @ 2023-05-15 23:14 UTC (permalink / raw)
To: linux-input; +Cc: Raul E Rangel, linux-kernel
In-Reply-To: <20230511185252.386941-7-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 | 81 +++++++------
drivers/input/mouse/psmouse-base.c | 53 ++++----
drivers/input/serio/libps2.c | 226 ++++++++++++++++++++++++++----------
include/linux/libps2.h | 61 ++++++----
4 files changed, 269 insertions(+), 152 deletions(-)
diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index 2fb2ad73e796..c92e544c792d 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -399,46 +399,60 @@ static unsigned int atkbd_compat_scancode(struct atkbd *atkbd, unsigned int code
}
/*
- * atkbd_interrupt(). Here takes place processing of data received from
- * the keyboard into events.
+ * Tries to handle frame or parity error by requesting the keyboard controller
+ * to resend the last byte. This historically not done on x86 as controllers
+ * there typically do not implement this command.
*/
-
-static irqreturn_t atkbd_interrupt(struct serio *serio, unsigned char data,
- unsigned int flags)
+static bool __maybe_unused atkbd_handle_frame_error(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 atkbd *atkbd = container_of(ps2dev, struct atkbd, ps2dev);
+ 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) {
+ 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;
+ return true;
}
if (!flags && data == ATKBD_RET_ACK)
atkbd->resend = false;
+
+ return false;
+}
+
+static enum ps2_disposition atkbd_pre_receive_byte(struct ps2dev *ps2dev,
+ u8 data, unsigned int flags)
+{
+ struct serio *serio = ps2dev->serio;
+
+ dev_dbg(&serio->dev, "Received %02x flags %02x\n", data, flags);
+
+#if !defined(__i386__) && !defined (__x86_64__)
+ if (atkbd_handle_frame_error(ps2dev, data, flags))
+ return PS2_IGNORE;
#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 +474,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 +491,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 +578,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 +1241,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 +1398,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 */
^ permalink raw reply related
* Re: AW: AW: [PATCH] Input: pwm-beeper - Support volume setting via sysfs
From: Marek Vasut @ 2023-05-15 17:28 UTC (permalink / raw)
To: Traut Manuel LCPF-CH, linux-input@vger.kernel.org
Cc: Uwe Kleine-König, Dmitry Torokhov, Frieder Schrempf,
Thierry Reding, linux-pwm@vger.kernel.org
In-Reply-To: <AS8PR03MB76216EADE882B038464E9E76FA789@AS8PR03MB7621.eurprd03.prod.outlook.com>
On 5/15/23 17:24, Traut Manuel LCPF-CH wrote:
> Hi Marek,
Hi,
>>> I tend to not change existing user-space interfaces. I would prefer to have an additional event or using sysfs.
>> I am increasingly concerned about the race condition between change of volume (via sysfs) and frequency (via SND_TONE) . So I would be banking toward additional event, like SND_TONE_WITH_VOLUME or something along those lines.
>
> SND_TONE_WITH_VOLUME is also ok from my side. But implementing some locking shall also be possible.
>
>>>> 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/
>>>
>>> This one is more complex, because the mapping between duty cycle and volume is not linear. Probably it depends also on the used beeper hardware which values are doing a significant change in volume. Therefore the patchset introduced a mapping between volume levels and duty cycle times in the device-tree to allow user-space applications to control the beeper volume hardware independently.
>
>> I wonder whether this mapping shouldn't be considered policy and left to userspace to deal with, instead of swamping the kernel or DT with it ?
> How could a Linux distribution detect which mapping is required to be installed?
> For me it seems to be easier to have the device-specific information in the device-tree.
The alternative might be to have volume in 0..65535 range (i.e. the top
16 MSbits) and be done with it, then the PWM subsystem is responsible
for mapping this to 0..50% of PWM duty cycle .
^ permalink raw reply
* Re: AW: EXTERNAL - [PATCH] Input: pwm-beeper - Support volume setting via sysfs
From: Marek Vasut @ 2023-05-15 17:27 UTC (permalink / raw)
To: Jeff LaBundy
Cc: Traut Manuel LCPF-CH, linux-input@vger.kernel.org,
Uwe Kleine-König, Dmitry Torokhov, Frieder Schrempf,
Thierry Reding, linux-pwm@vger.kernel.org
In-Reply-To: <ZGJA2M+V8ualidHH@nixie71>
On 5/15/23 16:25, Jeff LaBundy wrote:
> Hi Marek and Traut,
Hi,
> On Mon, May 15, 2023 at 03:36:02PM +0200, Marek Vasut wrote:
>> On 5/15/23 08:50, Traut Manuel LCPF-CH wrote:
>>> Hi Marek,
>>
>> Hi,
>>
>>>> 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 ?
>>>
>>> I tend to not change existing user-space interfaces. I would prefer to have an additional event or using sysfs.
>>
>> I am increasingly concerned about the race condition between change of
>> volume (via sysfs) and frequency (via SND_TONE) . So I would be banking
>> toward additional event, like SND_TONE_WITH_VOLUME or something along those
>> lines.
>
> This is just my $.02, but I don't see anything wrong with proposing an
> _additive_ change to the ABI such as this. My only concern is that this
> kind of change seems useful to any effect (e.g. SND_BEEP) and not limited
> to only tones. Unless volume adjustment is less useful for those?
I would say the volume should also apply to SND_BEEP, sure.
>>>> ---
>>>> 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/
>>>
>>> This one is more complex, because the mapping between duty cycle and volume is not linear. Probably it depends also on the used beeper hardware which values are doing a significant change in volume. Therefore the patchset introduced a mapping between volume levels and duty cycle times in the device-tree to allow user-space applications to control the beeper volume hardware independently.
>>
>> I wonder whether this mapping shouldn't be considered policy and left to
>> userspace to deal with, instead of swamping the kernel or DT with it ?
>
> I agree that the kernel need not try and linearize the values; in fact LEDs
> already have the same problem. I still feel however that imposing a unique
> maximum value (e.g. 50,000) is inappropriate because the range should remain
> the same regardless of the underlying HW implementation (PWM, class A/B, etc.).
We can easily just say 0..65536 if we agree the 16 MSbits are the volume
, that's really not a problem.
^ permalink raw reply
* Re: [PATCH] HID: uclogic: Modular KUnit tests should not depend on KUNIT=y
From: José Expósito @ 2023-05-15 16:31 UTC (permalink / raw)
To: David Gow
Cc: Geert Uytterhoeven, Jiri Kosina, Benjamin Tissoires,
Daniel Latypov, Brendan Higgins, linux-input, linux-kselftest,
kunit-dev, linux-kernel
In-Reply-To: <CABVgOSmYapFcpnrC60o4r5LznT92TpjteNb=1MQejvMDWC+2vw@mail.gmail.com>
On Thu, May 04, 2023 at 02:04:18PM +0800, David Gow wrote:
> On Tue, 2 May 2023 at 18:09, Geert Uytterhoeven <geert+renesas@glider.be> wrote:
> >
> > While KUnit tests that cannot be built as a loadable module must depend
> > on "KUNIT=y", this is not true for modular tests, where it adds an
> > unnecessary limitation.
> >
> > Fix this by relaxing the dependency to "KUNIT".
> >
> > Fixes: 08809e482a1c44d9 ("HID: uclogic: KUnit best practices and naming conventions")
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
>
> This works here, and I definitely agree we should get rid of these
> unnecessary constraints.
>
> Let me know if this should go in via the kselftest/kunit tree, or if
> it makes sense to take this via input?
>
> Reviewed-by: David Gow <davidgow@google.com>
>
> Cheers,
> -- David
>
> > drivers/hid/Kconfig | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> > index 4ce012f83253ec9f..b977450cac75265d 100644
> > --- a/drivers/hid/Kconfig
> > +++ b/drivers/hid/Kconfig
> > @@ -1285,7 +1285,7 @@ config HID_MCP2221
> >
> > config HID_KUNIT_TEST
> > tristate "KUnit tests for HID" if !KUNIT_ALL_TESTS
> > - depends on KUNIT=y
> > + depends on KUNIT
Tested here as well with different configurations for the UCLogic
driver and it is working.
Reviewed-by: José Expósito <jose.exposito89@gmail.com>
^ permalink raw reply
* AW: AW: [PATCH] Input: pwm-beeper - Support volume setting via sysfs
From: Traut Manuel LCPF-CH @ 2023-05-15 15:24 UTC (permalink / raw)
To: Marek Vasut, linux-input@vger.kernel.org
Cc: Uwe Kleine-König, Dmitry Torokhov, Frieder Schrempf,
Thierry Reding, linux-pwm@vger.kernel.org
In-Reply-To: <a5293af4-8d02-ed8f-52d1-722c71d47f37@denx.de>
Hi Marek,
>> I tend to not change existing user-space interfaces. I would prefer to have an additional event or using sysfs.
> I am increasingly concerned about the race condition between change of volume (via sysfs) and frequency (via SND_TONE) . So I would be banking toward additional event, like SND_TONE_WITH_VOLUME or something along those lines.
SND_TONE_WITH_VOLUME is also ok from my side. But implementing some locking shall also be possible.
>>> 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/
>>
>> This one is more complex, because the mapping between duty cycle and volume is not linear. Probably it depends also on the used beeper hardware which values are doing a significant change in volume. Therefore the patchset introduced a mapping between volume levels and duty cycle times in the device-tree to allow user-space applications to control the beeper volume hardware independently.
> I wonder whether this mapping shouldn't be considered policy and left to userspace to deal with, instead of swamping the kernel or DT with it ?
How could a Linux distribution detect which mapping is required to be installed?
For me it seems to be easier to have the device-specific information in the device-tree.
Regards
Manuel
^ permalink raw reply
* [PATCH v2 1/4] Input: ts-virtobj - Add touchscreen virtual object handling
From: Javier Carrasco @ 2023-05-15 15:00 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Henrik Rydberg, Bastian Hecht, Michael Riesch
Cc: linux-kernel, linux-input, devicetree, Javier Carrasco
In-Reply-To: <20230510-feature-ts_virtobj_patch-v2-0-f68a6bfe7a0f@wolfvision.net>
Some touchscreens provide mechanical overlays with different objects
like buttons or clipped touchscreen surfaces.
In order to support these objects, add a series of helper functions
to the input subsystem to transform them into virtual objects via
device tree nodes.
These virtual objects consume the raw touch events and report the
expected input events depending on the object properties.
Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
---
MAINTAINERS | 7 +
drivers/input/touchscreen/Kconfig | 9 +
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/ts-virtobj.c | 356 +++++++++++++++++++++++++++++++++
include/linux/input/ts-virtobj.h | 43 ++++
5 files changed, 416 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 7e0b87d5aa2e..296f71bcfe92 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21434,6 +21434,13 @@ W: https://github.com/srcres258/linux-doc
T: git git://github.com/srcres258/linux-doc.git doc-zh-tw
F: Documentation/translations/zh_TW/
+TOUCHSCREEN VIRTUAL OBJECTS
+M: Javier Carrasco <javier.carrasco@wolfvision.net>
+L: linux-input@vger.kernel.org
+S: Maintained
+F: drivers/input/touchscreen/ts-virtobj.c
+F: include/linux/input/ts-virtobj.h
+
TTY LAYER
M: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
M: Jiri Slaby <jirislaby@kernel.org>
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 143ff43c67ae..0bdbe0a77b34 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -1388,4 +1388,13 @@ config TOUCHSCREEN_HIMAX_HX83112B
To compile this driver as a module, choose M here: the
module will be called himax_hx83112b.
+config TOUCHSCREEN_TS_VIRTOBJ
+ tristate "Touchscreen Virtual Objects"
+ help
+ Say Y here if you are using a touchscreen driver that supports
+ printed overlays with keys or a clipped touchscreen area.
+
+ Should be selected by the touchscren drivers that support
+ this feature.
+
endif
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index 159cd5136fdb..dc315d58a03b 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -117,3 +117,4 @@ obj-$(CONFIG_TOUCHSCREEN_RASPBERRYPI_FW) += raspberrypi-ts.o
obj-$(CONFIG_TOUCHSCREEN_IQS5XX) += iqs5xx.o
obj-$(CONFIG_TOUCHSCREEN_ZINITIX) += zinitix.o
obj-$(CONFIG_TOUCHSCREEN_HIMAX_HX83112B) += himax_hx83112b.o
+obj-$(CONFIG_TOUCHSCREEN_TS_VIRTOBJ) += ts-virtobj.o
diff --git a/drivers/input/touchscreen/ts-virtobj.c b/drivers/input/touchscreen/ts-virtobj.c
new file mode 100644
index 000000000000..07e5611b7b3d
--- /dev/null
+++ b/drivers/input/touchscreen/ts-virtobj.c
@@ -0,0 +1,356 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Helper functions for virtual objects on touchscreens
+ *
+ * Copyright (c) 2023 Javier Carrasco <javier.carrasco@wolfvision.net>
+ */
+
+#include <linux/property.h>
+#include <linux/input.h>
+#include <linux/input/mt.h>
+#include <linux/module.h>
+#include <linux/input/ts-virtobj.h>
+
+enum ts_virtobj_valid_objects {
+ TOUCHSCREEN,
+ BUTTON,
+};
+
+static const char *const ts_virtobj_names[] = {
+ [TOUCHSCREEN] = "virtual-touchscreen",
+ [BUTTON] = "virtual-buttons",
+};
+
+struct ts_virtobj_shape {
+ u32 x_origin;
+ u32 y_origin;
+ u32 x_size;
+ u32 y_size;
+};
+
+struct ts_virtobj_button {
+ struct ts_virtobj_shape shape;
+ u32 key;
+ bool pressed;
+ int slot;
+};
+
+static int ts_virtobj_get_shape_properties(struct fwnode_handle *child_node,
+ struct ts_virtobj_shape *shape)
+{
+ int rc;
+
+ rc = fwnode_property_read_u32(child_node, "x-origin", &shape->x_origin);
+ if (rc < 0)
+ return rc;
+
+ rc = fwnode_property_read_u32(child_node, "y-origin", &shape->y_origin);
+ if (rc < 0)
+ return rc;
+
+ rc = fwnode_property_read_u32(child_node, "x-size", &shape->x_size);
+ if (rc < 0)
+ return rc;
+
+ rc = fwnode_property_read_u32(child_node, "y-size", &shape->y_size);
+ if (rc < 0)
+ return rc;
+
+ return 0;
+}
+
+static int ts_virtobj_get_button_properties(struct device *dev,
+ struct fwnode_handle *child_node,
+ struct ts_virtobj_button *btn)
+{
+ struct fwnode_handle *child_btn;
+ int rc;
+ int j = 0;
+
+ fwnode_for_each_child_node(child_node, child_btn) {
+ rc = ts_virtobj_get_shape_properties(child_btn, &btn[j].shape);
+ if (rc < 0)
+ goto button_prop_cleanup;
+
+ rc = fwnode_property_read_u32(child_btn, "linux,code",
+ &btn[j].key);
+ if (rc < 0)
+ goto button_prop_cleanup;
+
+ dev_info(dev, "Added button at (%u, %u), size %ux%u, code=%u\n",
+ btn[j].shape.x_origin, btn[j].shape.y_origin,
+ btn[j].shape.x_size, btn[j].shape.y_size, btn[j].key);
+ j++;
+ }
+
+ return 0;
+
+button_prop_cleanup:
+ fwnode_handle_put(child_btn);
+ return rc;
+}
+
+void ts_virtobj_set_button_caps(struct ts_virtobj_map *map,
+ struct input_dev *dev)
+{
+ int i;
+
+ for (i = 0; i < map->button_count; i++)
+ input_set_capability(dev, EV_KEY, map->buttons[i].key);
+}
+EXPORT_SYMBOL(ts_virtobj_set_button_caps);
+
+static int ts_virtobj_count_buttons(struct device *dev)
+{
+ struct fwnode_handle *child_node;
+ struct fwnode_handle *child_button;
+ int count = 0;
+
+ child_node = device_get_named_child_node(dev, ts_virtobj_names[BUTTON]);
+ if (!child_node)
+ return 0;
+
+ fwnode_for_each_child_node(child_node, child_button)
+ count++;
+ fwnode_handle_put(child_node);
+
+ return count;
+}
+
+static int ts_virtobj_map_touchscreen(struct device *dev,
+ struct ts_virtobj_map *map)
+{
+ struct fwnode_handle *child;
+ int rc = 0;
+
+ child = device_get_named_child_node(dev, ts_virtobj_names[TOUCHSCREEN]);
+ if (!child)
+ goto touchscreen_ret;
+
+ map->touchscreen =
+ devm_kzalloc(dev, sizeof(*map->touchscreen), GFP_KERNEL);
+ if (!map->touchscreen) {
+ rc = -ENOMEM;
+ goto touchscreen_handle;
+ }
+ rc = ts_virtobj_get_shape_properties(child, map->touchscreen);
+ if (rc < 0)
+ goto touchscreen_free;
+
+ map->virtual_touchscreen = true;
+ dev_info(dev, "Added virtual touchscreen at (%u, %u), size %u x %u\n",
+ map->touchscreen->x_origin, map->touchscreen->y_origin,
+ map->touchscreen->x_size, map->touchscreen->y_size);
+
+ rc = 0;
+ goto touchscreen_handle;
+
+touchscreen_free:
+ devm_kfree(dev, map->touchscreen);
+touchscreen_handle:
+ fwnode_handle_put(child);
+touchscreen_ret:
+ return rc;
+}
+
+static int ts_virtobj_map_buttons(struct device *dev,
+ struct ts_virtobj_map *map,
+ struct input_dev *input)
+{
+ struct fwnode_handle *child;
+ u32 button_count;
+ int rc = 0;
+
+ button_count = ts_virtobj_count_buttons(dev);
+ if (button_count) {
+ map->buttons = devm_kcalloc(dev, button_count,
+ sizeof(*map->buttons), GFP_KERNEL);
+ if (!map->buttons) {
+ rc = -ENOMEM;
+ goto map_buttons_ret;
+ }
+ child = device_get_named_child_node(dev,
+ ts_virtobj_names[BUTTON]);
+ if (unlikely(!child))
+ goto map_buttons_free;
+
+ rc = ts_virtobj_get_button_properties(dev, child, map->buttons);
+ if (rc < 0)
+ goto map_buttons_free;
+
+ map->button_count = button_count;
+ }
+
+ return 0;
+
+map_buttons_free:
+ devm_kfree(dev, map->buttons);
+map_buttons_ret:
+ return rc;
+}
+
+static bool ts_virtobj_defined_objects(struct device *dev)
+{
+ struct fwnode_handle *child;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(ts_virtobj_names); i++) {
+ child = device_get_named_child_node(dev, ts_virtobj_names[i]);
+ if (child) {
+ fwnode_handle_put(child);
+ return true;
+ }
+ fwnode_handle_put(child);
+ }
+
+ return false;
+}
+
+struct ts_virtobj_map *ts_virtobj_map_objects(struct device *dev,
+ struct input_dev *input)
+{
+ struct ts_virtobj_map *map = NULL;
+ int rc;
+
+ if (!ts_virtobj_defined_objects(dev))
+ return NULL;
+
+ map = devm_kzalloc(dev, sizeof(*map), GFP_KERNEL);
+ if (!map) {
+ rc = -ENOMEM;
+ goto objects_err;
+ }
+ rc = ts_virtobj_map_touchscreen(dev, map);
+ if (rc < 0)
+ goto objects_free;
+
+ rc = ts_virtobj_map_buttons(dev, map, input);
+ if (rc < 0)
+ goto objects_free;
+
+ return map;
+
+objects_free:
+ devm_kfree(dev, map);
+objects_err:
+ return ERR_PTR(rc);
+}
+EXPORT_SYMBOL(ts_virtobj_map_objects);
+
+void ts_virtobj_get_touchscreen_abs(struct ts_virtobj_map *map, u16 *x, u16 *y)
+{
+ *x = map->touchscreen->x_size - 1;
+ *y = map->touchscreen->y_size - 1;
+}
+EXPORT_SYMBOL(ts_virtobj_get_touchscreen_abs);
+
+static bool ts_virtobj_shape_event(struct ts_virtobj_shape *shape, u32 x, u32 y)
+{
+ if (!shape)
+ return false;
+
+ if (x >= shape->x_origin && x < (shape->x_origin + shape->x_size) &&
+ y >= shape->y_origin && y < (shape->y_origin + shape->y_size))
+ return true;
+
+ return false;
+}
+
+static bool ts_virtobj_touchscreen_event(struct ts_virtobj_shape *touchscreen,
+ u32 *x, u32 *y)
+{
+ if (ts_virtobj_shape_event(touchscreen, *x, *y)) {
+ *x -= touchscreen->x_origin;
+ *y -= touchscreen->y_origin;
+ return true;
+ }
+
+ return false;
+}
+
+bool ts_virtobj_mapped_touchscreen(struct ts_virtobj_map *map)
+{
+ if (!map || !map->virtual_touchscreen)
+ return false;
+
+ return true;
+}
+EXPORT_SYMBOL(ts_virtobj_mapped_touchscreen);
+
+bool ts_virtobj_mapped_buttons(struct ts_virtobj_map *map)
+{
+ if (!map || !map->button_count)
+ return false;
+
+ return true;
+}
+EXPORT_SYMBOL(ts_virtobj_mapped_buttons);
+
+bool ts_virtobj_mt_on_touchscreen(struct ts_virtobj_map *map, u32 *x, u32 *y)
+{
+ if (!ts_virtobj_mapped_touchscreen(map))
+ return true;
+
+ if (!ts_virtobj_touchscreen_event(map->touchscreen, x, y))
+ return false;
+
+ return true;
+}
+EXPORT_SYMBOL(ts_virtobj_mt_on_touchscreen);
+
+bool ts_virtobj_button_press(struct ts_virtobj_map *map,
+ struct input_dev *input, u32 x, u32 y, u32 slot)
+{
+ int i;
+
+ if (!ts_virtobj_mapped_buttons(map))
+ return false;
+
+ for (i = 0; i < map->button_count; i++) {
+ if (ts_virtobj_shape_event(&map->buttons[i].shape, x, y)) {
+ input_report_key(input, map->buttons[i].key, 1);
+ map->buttons[i].pressed = true;
+ map->buttons[i].slot = slot;
+ return true;
+ }
+ }
+
+ return false;
+}
+EXPORT_SYMBOL(ts_virtobj_button_press);
+
+bool ts_virtobj_is_button_slot(struct ts_virtobj_map *map, int slot)
+{
+ int i;
+
+ if (!map || !map->button_count)
+ return false;
+
+ for (i = 0; i < map->button_count; i++) {
+ if (map->buttons[i].pressed && map->buttons[i].slot == slot)
+ return true;
+ }
+
+ return false;
+}
+EXPORT_SYMBOL(ts_virtobj_is_button_slot);
+
+void ts_virtobj_button_release(struct ts_virtobj_map *map,
+ struct input_dev *input, u32 slot)
+{
+ int i;
+
+ if (!map || !map->button_count)
+ return;
+
+ for (i = 0; i < map->button_count; i++) {
+ if (map->buttons[i].pressed && map->buttons[i].slot == slot) {
+ input_report_key(input, map->buttons[i].key, 0);
+ map->buttons[i].pressed = false;
+ }
+ }
+}
+EXPORT_SYMBOL(ts_virtobj_button_release);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Helper functions for virtual objects on touchscreens");
diff --git a/include/linux/input/ts-virtobj.h b/include/linux/input/ts-virtobj.h
new file mode 100644
index 000000000000..f221b46c4d09
--- /dev/null
+++ b/include/linux/input/ts-virtobj.h
@@ -0,0 +1,43 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2023 Javier Carrasco <javier.carrasco@wolfvision.net>
+ */
+
+#ifndef _TS_VIRTOBJ
+#define _TS_VIRTOBJ
+
+#include <linux/types.h>
+
+struct input_dev;
+struct device;
+
+struct ts_virtobj_map {
+ struct ts_virtobj_shape *touchscreen;
+ bool virtual_touchscreen;
+ struct ts_virtobj_button *buttons;
+ u32 button_count;
+};
+
+struct ts_virtobj_map *ts_virtobj_map_objects(struct device *dev,
+ struct input_dev *input);
+
+void ts_virtobj_get_touchscreen_abs(struct ts_virtobj_map *map, u16 *x, u16 *y);
+
+bool ts_virtobj_mapped_touchscreen(struct ts_virtobj_map *map);
+
+bool ts_virtobj_mapped_buttons(struct ts_virtobj_map *map);
+
+bool ts_virtobj_mt_on_touchscreen(struct ts_virtobj_map *map, u32 *x, u32 *y);
+
+bool ts_virtobj_button_press(struct ts_virtobj_map *map,
+ struct input_dev *input, u32 x, u32 y, u32 slot);
+
+bool ts_virtobj_is_button_slot(struct ts_virtobj_map *map, int slot);
+
+void ts_virtobj_button_release(struct ts_virtobj_map *map,
+ struct input_dev *input, u32 slot);
+
+void ts_virtobj_set_button_caps(struct ts_virtobj_map *map,
+ struct input_dev *dev);
+
+#endif
--
2.39.2
^ permalink raw reply related
* [PATCH v2 2/4] dt-bindings: touchscreen: add virtual-touchscreen and virtual-buttons properties
From: Javier Carrasco @ 2023-05-15 15:00 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Henrik Rydberg, Bastian Hecht, Michael Riesch
Cc: linux-kernel, linux-input, devicetree, Javier Carrasco
In-Reply-To: <20230510-feature-ts_virtobj_patch-v2-0-f68a6bfe7a0f@wolfvision.net>
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.
Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
---
.../bindings/input/touchscreen/touchscreen.yaml | 139 +++++++++++++++++++++
1 file changed, 139 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
index 895592da9626..866bfb45a8cf 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
@@ -80,6 +80,145 @@ properties:
touchscreen-y-plate-ohms:
description: Resistance of the Y-plate in Ohms
+ virtual-touchscreen:
+ description: Clipped touchscreen area
+
+ This object can be used to describe a frame that restricts the area
+ within touch events are reported, ignoring the events that occur outside
+ this area. This is of special interest if the touchscreen is shipped
+ with a physical overlay on top of it with a frame that hides some part
+ of the original touchscreen area.
+
+ The x-origin and y-origin properties of this object define the offset of
+ a new origin from where the touchscreen events are referenced.
+ This offset is applied to the events accordingly. The x-size and y-size
+ properties define the size of the virtual-touchscreen (effective area).
+
+ The following example shows the new touchscreen area and the new origin
+ (0',0') for the touch events generated by the device.
+
+ Touchscreen (full area)
+ ┌────────────────────────────────────────┐
+ │ ┌───────────────────────────────┐ │
+ │ │ │ │
+ │ ├ y-size │ │
+ │ │ │ │
+ │ │ virtual-touchscreen │ │
+ │ │ │ │
+ │ │ │ │
+ │ │ x-size │ │
+ │ ┌└──────────────┴────────────────┘ │
+ │(0',0') │
+ ┌└────────────────────────────────────────┘
+ (0,0)
+
+ where (0',0') = (0+x-origin,0+y-origin)
+
+ type: object
+
+ properties:
+ x-origin:
+ description: horizontal origin of the clipped area
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ y-origin:
+ description: vertical origin of the clipped area
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ x-size:
+ description: horizontal resolution of the clipped area
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ y-size:
+ description: vertical resolution of the clipped area
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ virtual-buttons:
+ description: list of nodes defining the buttons on the touchscreen
+
+ This object can be used to describe buttons on the touchscreen area,
+ reporting the touch events on their surface as key events instead of
+ the original touch events.
+
+ This is of special interest if the touchscreen is shipped with a
+ physical overlay on top of it where a number of buttons with some
+ predefined functionality are printed. In that case a specific behavior
+ is expected from those buttons instead of raw touch events.
+
+ The virtual-buttons properties define a per-button area as well as an
+ origin relative to the real touchscreen origin. Touch events within the
+ button area are reported as the key event defined in the linux,code
+ property. Given that the key events do not provide coordinates, the
+ button origin is only used to place the button area on the touchscreen
+ surface. Any event outside the virtual-buttons object is reported as a
+ touch event with no coordinate transformation.
+
+ The following example shows a touchscreen with a single button on it
+
+ Touchscreen (full area)
+ ┌───────────────────────────────────┐
+ │ │
+ │ │
+ │ ┌─────────┐ │
+ │ │button 0 │ │
+ │ │KEY_POWER│ │
+ │ └─────────┘ │
+ │ │
+ │ │
+ ┌└───────────────────────────────────┘
+ (0,0)
+
+ The virtual-buttons object can be combined with the virtual-touchscreen
+ object as shown in the following example. In that case only the events
+ within the virtual-touchscreen object are reported as touch events.
+
+ Touchscreen (full area)
+ ┌─────────┬──────────────────────────────┐
+ │ │ │
+ │ │ ┌───────────────────────┐ │
+ │ button 0│ │ │ │
+ │KEY_POWER│ │ │ │
+ │ │ │ │ │
+ ├─────────┤ │ virtual-touchscreen │ │
+ │ │ │ │ │
+ │ │ │ │ │
+ │ button 1│ │ │ │
+ │ KEY_INFO│ ┌└───────────────────────┘ │
+ │ │(0',0') │
+ ┌└─────────┴──────────────────────────────┘
+ (0,0)
+
+ type: object
+
+ patternProperties:
+ '^button-':
+ type: object
+ description:
+ Each button (key) is represented as a sub-node.
+
+ properties:
+ label:
+ $ref: /schemas/types.yaml#/definitions/string
+ description: descriptive name of the button
+
+ linux,code: true
+
+ x-origin:
+ description: horizontal origin of the button area
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ y-origin:
+ description: vertical origin of the button area
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ x-size:
+ description: horizontal resolution of the button area
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ y-size:
+ description: vertical resolution of the button area
+ $ref: /schemas/types.yaml#/definitions/uint32
+
dependencies:
touchscreen-size-x: [ touchscreen-size-y ]
touchscreen-size-y: [ touchscreen-size-x ]
--
2.39.2
^ permalink raw reply related
* [PATCH v2 3/4] Input: st1232 - add virtual touchscreen and buttons handling
From: Javier Carrasco @ 2023-05-15 15:01 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Henrik Rydberg, Bastian Hecht, Michael Riesch
Cc: linux-kernel, linux-input, devicetree, Javier Carrasco
In-Reply-To: <20230510-feature-ts_virtobj_patch-v2-0-f68a6bfe7a0f@wolfvision.net>
Use ts-virtobj to support overlay objects such as buttons and resized
frames defined in the device tree.
If a virtual-touchscreen is provided, ignore touch events outside of
the area defined by its properties. Otherwise, transform the event
coordinates to the virtual-touchscreen x and y-axis if required.
If buttons are provided, register an additional device to report key
events separately. A key event will be generated if the coordinates
of a touch event are within the area defined by the button properties.
Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
---
drivers/input/touchscreen/Kconfig | 1 +
drivers/input/touchscreen/st1232.c | 87 ++++++++++++++++++++++++++++++--------
2 files changed, 70 insertions(+), 18 deletions(-)
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 0bdbe0a77b34..d975ca308439 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -1215,6 +1215,7 @@ config TOUCHSCREEN_SIS_I2C
config TOUCHSCREEN_ST1232
tristate "Sitronix ST1232 or ST1633 touchscreen controllers"
depends on I2C
+ select TOUCHSCREEN_TS_VIRTOBJ
help
Say Y here if you want to support the Sitronix ST1232
or ST1633 touchscreen controller.
diff --git a/drivers/input/touchscreen/st1232.c b/drivers/input/touchscreen/st1232.c
index f49566dc96f8..b8139b7daa40 100644
--- a/drivers/input/touchscreen/st1232.c
+++ b/drivers/input/touchscreen/st1232.c
@@ -22,6 +22,7 @@
#include <linux/pm_qos.h>
#include <linux/slab.h>
#include <linux/types.h>
+#include <linux/input/ts-virtobj.h>
#define ST1232_TS_NAME "st1232-ts"
#define ST1633_TS_NAME "st1633-ts"
@@ -56,6 +57,8 @@ struct st1232_ts_data {
struct touchscreen_properties prop;
struct dev_pm_qos_request low_latency_req;
struct gpio_desc *reset_gpio;
+ struct input_dev *virtual_keypad;
+ struct ts_virtobj_map *map;
const struct st_chip_info *chip_info;
int read_buf_len;
u8 *read_buf;
@@ -129,10 +132,12 @@ static int st1232_ts_read_resolution(struct st1232_ts_data *ts, u16 *max_x,
static int st1232_ts_parse_and_report(struct st1232_ts_data *ts)
{
- struct input_dev *input = ts->input_dev;
+ struct input_dev *tscreen = ts->input_dev;
+ __maybe_unused struct input_dev *keypad = ts->virtual_keypad;
struct input_mt_pos pos[ST_TS_MAX_FINGERS];
u8 z[ST_TS_MAX_FINGERS];
int slots[ST_TS_MAX_FINGERS];
+ __maybe_unused bool button_pressed[ST_TS_MAX_FINGERS];
int n_contacts = 0;
int i;
@@ -143,6 +148,15 @@ static int st1232_ts_parse_and_report(struct st1232_ts_data *ts)
unsigned int x = ((buf[0] & 0x70) << 4) | buf[1];
unsigned int y = ((buf[0] & 0x07) << 8) | buf[2];
+ /* forward button presses to the keypad input device */
+ if (ts_virtobj_is_button_slot(ts->map, i) ||
+ ts_virtobj_button_press(ts->map, keypad, x, y, i)) {
+ button_pressed[i] = true;
+ continue;
+ }
+ /* Ignore events out of the virtual screen if defined */
+ if (!ts_virtobj_mt_on_touchscreen(ts->map, &x, &y))
+ continue;
touchscreen_set_mt_pos(&pos[n_contacts],
&ts->prop, x, y);
@@ -154,18 +168,25 @@ static int st1232_ts_parse_and_report(struct st1232_ts_data *ts)
}
}
- input_mt_assign_slots(input, slots, pos, n_contacts, 0);
+ input_mt_assign_slots(tscreen, slots, pos, n_contacts, 0);
for (i = 0; i < n_contacts; i++) {
- input_mt_slot(input, slots[i]);
- input_mt_report_slot_state(input, MT_TOOL_FINGER, true);
- input_report_abs(input, ABS_MT_POSITION_X, pos[i].x);
- input_report_abs(input, ABS_MT_POSITION_Y, pos[i].y);
+ input_mt_slot(tscreen, slots[i]);
+ input_mt_report_slot_state(tscreen, MT_TOOL_FINGER, true);
+ input_report_abs(tscreen, ABS_MT_POSITION_X, pos[i].x);
+ input_report_abs(tscreen, ABS_MT_POSITION_Y, pos[i].y);
if (ts->chip_info->have_z)
- input_report_abs(input, ABS_MT_TOUCH_MAJOR, z[i]);
+ input_report_abs(tscreen, ABS_MT_TOUCH_MAJOR, z[i]);
+ }
+ input_mt_sync_frame(tscreen);
+ input_sync(tscreen);
+
+ if (ts_virtobj_mapped_buttons(ts->map)) {
+ for (i = 0; i < ts->chip_info->max_fingers; i++)
+ if (ts_virtobj_is_button_slot(ts->map, i) &&
+ !button_pressed[i])
+ ts_virtobj_button_release(ts->map, keypad, i);
+ input_sync(keypad);
}
-
- input_mt_sync_frame(input);
- input_sync(input);
return n_contacts;
}
@@ -226,6 +247,7 @@ static int st1232_ts_probe(struct i2c_client *client)
const struct st_chip_info *match;
struct st1232_ts_data *ts;
struct input_dev *input_dev;
+ struct input_dev __maybe_unused *virtual_keypad;
u16 max_x, max_y;
int error;
@@ -292,18 +314,28 @@ static int st1232_ts_probe(struct i2c_client *client)
if (error)
return error;
- /* Read resolution from the chip */
- error = st1232_ts_read_resolution(ts, &max_x, &max_y);
- if (error) {
- dev_err(&client->dev,
- "Failed to read resolution: %d\n", error);
- return error;
- }
-
if (ts->chip_info->have_z)
input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0,
ts->chip_info->max_area, 0, 0);
+ /* map virtual objects if defined in the device tree */
+ ts->map = ts_virtobj_map_objects(&ts->client->dev, ts->input_dev);
+ if (IS_ERR(ts->map))
+ return PTR_ERR(ts->map);
+
+ if (ts_virtobj_mapped_touchscreen(ts->map)) {
+ /* Read resolution from the virtual touchscreen if defined*/
+ ts_virtobj_get_touchscreen_abs(ts->map, &max_x, &max_y);
+ } else {
+ /* Read resolution from the chip */
+ error = st1232_ts_read_resolution(ts, &max_x, &max_y);
+ if (error) {
+ dev_err(&client->dev,
+ "Failed to read resolution: %d\n", error);
+ return error;
+ }
+ }
+
input_set_abs_params(input_dev, ABS_MT_POSITION_X,
0, max_x, 0, 0);
input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
@@ -335,6 +367,25 @@ static int st1232_ts_probe(struct i2c_client *client)
return error;
}
+ /* Register keypad input device if virtual buttons were defined */
+ if (ts_virtobj_mapped_buttons(ts->map)) {
+ virtual_keypad = devm_input_allocate_device(&client->dev);
+ if (!virtual_keypad)
+ return -ENOMEM;
+
+ ts->virtual_keypad = virtual_keypad;
+ virtual_keypad->name = "st1232-keypad";
+ virtual_keypad->id.bustype = BUS_I2C;
+ ts_virtobj_set_button_caps(ts->map, virtual_keypad);
+ error = input_register_device(ts->virtual_keypad);
+ if (error) {
+ dev_err(&client->dev,
+ "Unable to register %s input device\n",
+ virtual_keypad->name);
+ return error;
+ }
+ }
+
i2c_set_clientdata(client, ts);
return 0;
--
2.39.2
^ 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