On Mon, 21 Sep 2020 18:34:29 GMT, John Neffenger <github.com+1413266+jgn...@openjdk.org> wrote:
>> Based on my notes below, I believe this change is safe for any Linux input >> device driver because the driver shouldn't >> receive the intermediate *open* and *close* calls at all. >> Nonetheless, it would be reassuring if someone could try [this >> change](https://github.com/openjdk/jfx/pull/258/files) >> just once on a mainstream device, such as the Raspberry Pi with a >> touchscreen LCD panel. I only have six obscure ARM >> devices and a headless QEMU ARM virtual machine for testing. @johanvos or >> @dellgreen - Is that something you could >> test? If you think it's overkill and not worth it, that's fine, too. #### >> Notes >> The Linux kernel documentation about [opening and >> closing](https://www.kernel.org/doc/html/latest/input/input-programming.html#dev-open-and-dev-close) >> input devices >> states: >>> Note that input core keeps track of number of users for the device and >>> makes sure that dev->open() is called only when >>> the first user connects to the device and that dev->close() is called when >>> the very last user disconnects. Calls to >>> both callbacks are serialized. >> >> Also, the Linux Programmer's Manual for the *close* system call (`man 2 >> close`) states: >> >>> If *fd* is the last file descriptor referring to the underlying open file >>> description (see **open**(2)), the resources >>> associated with the open file description are freed. >> >> Running a JavaFX program with `strace -f -e trace=open,close -o strace.log` >> shows the one *open* for the *event0* >> keyboard, and the *open, open, close* for the *event1* touchscreen: >> 5847 open("/dev/input/event0", O_RDONLY) = 11 >> ... >> 5847 open("/dev/input/event1", O_RDONLY) = 12 >> 5847 open("/dev/input/event1", O_RDONLY) = 13 >> 5847 close(13) = 0 >> >> Both devices are actually closed by the kernel when the JavaFX program ends. > >> Nonetheless, it would be reassuring if someone could try [this >> change](https://github.com/openjdk/jfx/pull/258/files) >> just once on a mainstream device, such as the Raspberry Pi with a >> touchscreen LCD panel. > > I just placed an order for a Raspberry Pi 2 Model B -- the older model with > the 32-bit ARMv7-A architecture. It's the > oldest, slowest, and coolest-running Raspberry Pi supported by Ubuntu, and > it's also the closest match to the > processors found in devices with e-paper displays. The Raspberry Pi should > finally let me test changes like this to the > Linux Monocle platform on which the EPD platform is based. I may eventually > add a touchscreen, but I'm hoping to get by > with just the mouse for now. @johanvos, would you mind approving this pull request again? It looks as if your prior approval was removed when I merged the master branch. All of the tests I ran this week worked as expected and are described below. ### Tests I ran additional tests on the following devices after merging the master branch: * Kobo Touch Model N905C, * Kobo Glo HD Model N437, and * Raspberry Pi 2 Model B. I ran the tests: * without this fix and without the workaround, * with this fix but without the workaround, * with this fix and with the workaround (representing this pull request). The workaround is the code in `EPDInputDeviceRegistry.createDevice` that lets the current JavaFX 15 release avoid the problem even without this fix. The goal of this pull request is to be able eventually to remove the `EPDInputDeviceRegistry` subclass entirely. The test results are shown below. The system call trace was captured with the following command using the [epd-javafx][1] application. $ sudo strace -f -e trace=open,openat,close -o strace.log \ bin/run.sh --pattern=3 --loops=1 **Note:** In the messages below from the kernel ring buffer, the final two lines with `zforce_i2c_close` and `Deactivate` are printed when the program terminates. That call made by the kernel on behalf of the program is not recorded in the program's system call trace. ### Kobo Touch Model N905C This device has the "command overrun" bug. #### Without fix, without workaround The system call trace shows: 3576 open("/dev/input/event1", O_RDONLY) = 12 3576 close(12) = 0 3576 open("/dev/input/event1", O_RDONLY) = 12 The error occurs, and the touchscreen is disabled: $ dmesg | grep -i zforce [drivers/input/touchscreen/zforce_i2c.c-425] zforce_i2c_open() [zForce_ir_touch_recv_data-145] command Activate (0) ... [zForce_ir_touch_recv_data-154] command Resolution (0) ... [zForce_ir_touch_recv_data-179] command Frequency (0) ... [drivers/input/touchscreen/zforce_i2c.c-440] zforce_i2c_close() [drivers/input/touchscreen/zforce_i2c.c-425] zforce_i2c_open() [zForce_ir_touch_recv_data-142] command Deactivate ... [zForce_ir_touch_recv_data-198] command overrun (8) ... [drivers/input/touchscreen/zforce_i2c.c-440] zforce_i2c_close() [zForce_ir_touch_recv_data-142] command Deactivate ... #### With fix, without workaround The system call trace shows: 3997 open("/dev/input/event1", O_RDONLY) = 12 3997 open("/dev/input/event1", O_RDONLY) = 13 3997 close(13) = 0 The error does not occur, and the touchscreen works as expected: $ dmesg | grep -i zforce [drivers/input/touchscreen/zforce_i2c.c-425] zforce_i2c_open() [zForce_ir_touch_recv_data-145] command Activate (0) ... [zForce_ir_touch_recv_data-154] command Resolution (0) ... [zForce_ir_touch_recv_data-179] command Frequency (0) ... [drivers/input/touchscreen/zforce_i2c.c-440] zforce_i2c_close() [zForce_ir_touch_recv_data-142] command Deactivate ... #### With fix, with workaround The system call trace shows the additional *open* call due to the workaround: 4123 open("/dev/input/event1", O_RDONLY) = 13 4123 open("/dev/input/event1", O_RDONLY) = 14 4123 open("/dev/input/event1", O_RDONLY) = 15 4123 close(15) = 0 The error does not occur, and the touchscreen works as expected: $ dmesg | grep -i zforce [drivers/input/touchscreen/zforce_i2c.c-425] zforce_i2c_open() [zForce_ir_touch_recv_data-145] command Activate (0) ... [zForce_ir_touch_recv_data-154] command Resolution (0) ... [zForce_ir_touch_recv_data-179] command Frequency (0) ... [drivers/input/touchscreen/zforce_i2c.c-440] zforce_i2c_close() [zForce_ir_touch_recv_data-142] command Deactivate ... ### Kobo Glo HD Model N437 This device does not have the "command overrun" bug. #### Without fix, without workaround The system call trace shows: 3396 open("/dev/input/event1", O_RDONLY) = 11 3396 close(11) = 0 3396 open("/dev/input/event1", O_RDONLY) = 11 The error does not occur, and the touchscreen works as expected: $ dmesg | grep -i zforce [drivers/input/touchscreen/zforce_i2c.c-740] zforce_i2c_open() [zForce_ir_touch_recv_data-233] command Activate (0) ... ... [zForce_ir_touch_recv_data-250] command Resolution (0) ... [zForce_ir_touch_recv_data-278] command Frequency (0) ... [zForce_ir_touch_recv_data-260] command Configuration ... [drivers/input/touchscreen/zforce_i2c.c-756] zforce_i2c_close() [zForce_ir_touch_recv_data-230] command Deactivate ... #### With fix, without workaround The system call trace shows: 3620 open("/dev/input/event1", O_RDONLY) = 15 3620 open("/dev/input/event1", O_RDONLY) = 16 3620 close(16) = 0 The error does not occur, and the touchscreen works as expected: $ dmesg | grep -i zforce [drivers/input/touchscreen/zforce_i2c.c-740] zforce_i2c_open() [zForce_ir_touch_recv_data-233] command Activate (0) ... ... [zForce_ir_touch_recv_data-250] command Resolution (0) ... [zForce_ir_touch_recv_data-278] command Frequency (0) ... [zForce_ir_touch_recv_data-260] command Configuration ... [drivers/input/touchscreen/zforce_i2c.c-756] zforce_i2c_close() [zForce_ir_touch_recv_data-230] command Deactivate ... #### With fix, with workaround The system call trace shows the additional *open* call due to the workaround: 3913 open("/dev/input/event1", O_RDONLY) = 18 3913 open("/dev/input/event1", O_RDONLY) = 19 3913 open("/dev/input/event1", O_RDONLY) = 20 3913 close(20) = 0 The error does not occur, and the touchscreen works as expected: $ dmesg | grep -i zforce [drivers/input/touchscreen/zforce_i2c.c-740] zforce_i2c_open() [zForce_ir_touch_recv_data-233] command Activate (0) ... ... [zForce_ir_touch_recv_data-250] command Resolution (0) ... [zForce_ir_touch_recv_data-278] command Frequency (0) ... [zForce_ir_touch_recv_data-260] command Configuration ... [drivers/input/touchscreen/zforce_i2c.c-756] zforce_i2c_close() [zForce_ir_touch_recv_data-230] command Deactivate ... ### Raspberry Pi 2 Model B This device has a normal mouse rather than a touchscreen. #### Without fix, without workaround The system call trace shows: 19407 openat(AT_FDCWD, "/dev/input/event0", O_RDONLY) = 18 There is no output to the kernel ring buffer, and the mouse (event0) works as expected. #### With fix, without workaround The system call trace shows: 19548 openat(AT_FDCWD, "/dev/input/event0", O_RDONLY) = 18 There is no output to the kernel ring buffer, and the mouse (event0) works as expected. #### With fix, with workaround The system call trace shows: 19802 openat(AT_FDCWD, "/dev/input/event0", O_RDONLY) = 18 There is no output to the kernel ring buffer, and the mouse (event0) works as expected. [1]: https://github.com/jgneff/epd-javafx ------------- PR: https://git.openjdk.java.net/jfx/pull/258
