* Re: atkbd input regression
From: José Ramón Muñoz Pekkarinen @ 2023-08-30 14:25 UTC (permalink / raw)
To: Linux regressions mailing list; +Cc: linux-input, dmitry.torokhov, gregkh
In-Reply-To: <a0b8cbf4-a3db-5b05-14ba-297fe24efd52@leemhuis.info>
On Wed, 30 Aug 2023 at 14:06, Linux regression tracking (Thorsten
Leemhuis) <regressions@leemhuis.info> wrote:
> So strictly speaking all this sounds a lot like this is caused by kernel
> regression that thus should be fixed in the kernel once this was
> bisected (which iirc hasn't happened yet).
I'm afraid it may be a distro specific problem, if I'm not so mistaken
from the time Gentoo switched its default udev from eudev to systemd
udev, which also explain why I can't rebuild the good kernel nowadays,
test, and have the expected good results, because the new initramfs
will receive a copy of the new systemd udev, and not the former eudev,
and it acts as all my broken kernels in time.
There is still room to be a kernel issue, since rethinking the issue, the
kernel on early boot does what is expected, it configures the device and
with udev makes it work for the ttys, the problem comes when the wayland
seat comes available, where plasma or wayland needs to advertise the
kernel of the new seat, and the kernel then needs to generate the correct
event to makes udev populate the new information and tags that are
missing when the device doesn't work. This can be either a missing
implementation on Gentoo side because of using systemd udev without
the rest of systemd, or a kernel issue if the kernel is receiving the
information of the new seat and not generating an uevent to udev
to populate the new info. So in short, I may need to revisit this in the
following days.
> OTOH this afaics (please correct me if I'm wrong!) is the first such
> report, so the problem is likely pretty specific or might only occur on
> your system. In that case just looking for some solution for your system
> might be fine.
There is similar cases in the Gentoo forum that eventually gave up
on Gentoo[1] and made a switch to Fedora to be done with this. I just prefer
to look for a solution to the problem and fix it for everybody. So bear
with it while I dig into the code path that is propagating the seat info to
the kernel and udev.
[1] https://forums.gentoo.org/viewtopic-t-1161962-highlight-serial+keyboard.html
Thanks!
José.
^ permalink raw reply
* [PATCH -next] HID: hid-mf: Use list_for_each_entry() helper
From: Jinjie Ruan @ 2023-08-30 9:01 UTC (permalink / raw)
To: linux-input, Jiri Kosina, Benjamin Tissoires; +Cc: ruanjinjie
Convert list_for_each() to list_for_each_entry() so that the report_ptr
list_head pointer and list_entry() call are no longer needed, which
can reduce a few lines of code. No functional changed.
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
drivers/hid/hid-mf.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/hid/hid-mf.c b/drivers/hid/hid-mf.c
index 92d7ecd41a78..2d9a500cf364 100644
--- a/drivers/hid/hid-mf.c
+++ b/drivers/hid/hid-mf.c
@@ -61,7 +61,6 @@ static int mf_init(struct hid_device *hid)
struct list_head *report_list =
&hid->report_enum[HID_OUTPUT_REPORT].report_list;
- struct list_head *report_ptr;
struct hid_report *report;
struct list_head *input_ptr = &hid->inputs;
@@ -72,9 +71,7 @@ static int mf_init(struct hid_device *hid)
int error;
/* Setup each of the four inputs */
- list_for_each(report_ptr, report_list) {
- report = list_entry(report_ptr, struct hid_report, list);
-
+ list_for_each_entry(report, report_list, list) {
if (report->maxfield < 1 || report->field[0]->report_count < 2) {
hid_err(hid, "Invalid report, this should never happen!\n");
return -ENODEV;
--
2.34.1
^ permalink raw reply related
* Re: atkbd input regression
From: Linux regression tracking (Thorsten Leemhuis) @ 2023-08-30 11:06 UTC (permalink / raw)
To: José Ramón Muñoz Pekkarinen, linux-input
Cc: dmitry.torokhov, gregkh, Linux Regressions
In-Reply-To: <CANWZPgKe6GRUBVyi9Ua0Ns=eQDHT2riSsUZ7gr2nGfXT+jM00w@mail.gmail.com>
On 28.08.23 10:53, José Ramón Muñoz Pekkarinen wrote:
> On Sun, 27 Aug 2023 at 18:59, José Ramón Muñoz Pekkarinen
> <koalinux@gmail.com> wrote:
>>>
>>> Excuse the long wait, I've been investigating with the kde community
>>> and reading the code of libinput, systemd and the like, and I managed to
>>> find a difference that I'm not quite sure it is relevant to narrow the issue.
>>> While the keyboard is populated in the udev database as this:
>>>
>>> $ udevadm info -e
>>> P: /devices/platform/i8042/serio0/input/input0
>>> M: input0
>>> R: 0
>>> U: input
>>> E: DEVPATH=/devices/platform/i8042/serio0/input/input0
>>> E: SUBSYSTEM=input
>>> E: PRODUCT=11/1/1/ab54
>>> E: NAME="AT Translated Set 2 keyboard"
>>> E: PHYS="isa0060/serio0/input0"
>>> E: PROP=0
>>> E: EV=120013
>>> E: KEY=402000000 3803078f800d001 feffffdfffefffff fffffffffffffffe
>>> E: MSC=10
>>> E: LED=7
>>> E: MODALIAS=input:b0011v0001p0001eAB54-e0,1,4,11,14,k71,72,73,74,75,76,77,79,7A,7B,7C,7D,7E,7F,80,8C,8E,8F,9B,9C,9D,9E,9F,A3,A4,A5,A6,AC,AD,B7,B8,B9,D9,E2,ram4,l0,1,2,sfw
>>>
>>> There is no longer a corresponding entry under /run/udev/data,
>>> the following is the output of the ls -la /run/udev/data on 6.4.12:
>>>
>>> total 116
>>> [...]
>>>
>>> To what I read, having it in the db either populated
>>> by the file under /run/udev/data, or populated from sysfs
>>> should be enough, but for some reason, it seems systemd
>>> and libudev expects the file under this folder.
>
> I found that according to some early output on udev, the problem
> maybe that Gentoo is triggering the device discovery before the
> kernel is having multiple uevent files available. Retriggering the
> device discovery on a later stage makes the discovery of the
> devices work correctly and I can use the devices as in former
> kernels. The command to do so is:
>
> # udevadm trigger --type=devices --action=add
>
> Is this something that can be improved from kernel end or should
> I just look for some solution on Gentoo side to delay that trigger and
> be done with it?
So strictly speaking all this sounds a lot like this is caused by kernel
regression that thus should be fixed in the kernel once this was
bisected (which iirc hasn't happened yet).
OTOH this afaics (please correct me if I'm wrong!) is the first such
report, so the problem is likely pretty specific or might only occur on
your system. In that case just looking for some solution for your system
might be fine.
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
If I did something stupid, please tell me, as explained on that page.
^ permalink raw reply
* Re: [PATCH v4 2/4] dt-bindings: touchscreen: add overlay-touchscreen and overlay-buttons properties
From: Rob Herring @ 2023-08-31 11:48 UTC (permalink / raw)
To: Javier Carrasco
Cc: Dmitry Torokhov, Krzysztof Kozlowski, Conor Dooley,
Henrik Rydberg, Bastian Hecht, Michael Riesch, linux-kernel,
linux-input, devicetree
In-Reply-To: <20230510-feature-ts_virtobj_patch-v4-2-5c6c0fc1eed6@wolfvision.net>
On Thu, Aug 24, 2023 at 03:17:10PM +0200, Javier Carrasco wrote:
> The overlay-touchscreen object defines an area within the touchscreen
> where touch events are reported and their coordinates get converted to
> the overlay 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 overlay-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 | 152 +++++++++++++++++++++
> 1 file changed, 152 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
> index 895592da9626..d90cbb4932b5 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
> +++ b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
> @@ -80,6 +80,158 @@ properties:
> touchscreen-y-plate-ohms:
> description: Resistance of the Y-plate in Ohms
>
> + overlay-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 overlay-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 │ │
> + │ │ │ │
> + │ │ overlay-touchscreen │ │
> + │ │ │ │
> + │ │ │ │
> + │ │ x-size │ │
> + │ ┌└──────────────┴────────────────┘ │
> + │(0',0') │
> + ┌└────────────────────────────────────────┘
> + (0,0)
> +
> + where (0',0') = (0+x-origin,0+y-origin)
What happens if touchscreen-inverted-x/y are set?
Though the existing binding never defines what the non-inverted
orientation is.
> +
> + 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
> +
> + required:
> + - x-origin
> + - y-origin
> + - x-size
> + - y-size
> +
> + overlay-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 overlay-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 overlay-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 overlay-buttons object can be combined with the overlay-touchscreen
> + object as shown in the following example. In that case only the events
> + within the overlay-touchscreen object are reported as touch events.
> +
> + Touchscreen (full area)
> + ┌─────────┬──────────────────────────────┐
> + │ │ │
> + │ │ ┌───────────────────────┐ │
> + │ button 0│ │ │ │
> + │KEY_POWER│ │ │ │
> + │ │ │ │ │
> + ├─────────┤ │ overlay-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
> +
> + required:
> + - linux,code
> + - x-origin
> + - y-origin
> + - x-size
> + - y-size
We have the same properties defined twice. Move all the common ones to a
$def entry and then reference it here and in overlay-touchscreen.
$defs:
overlay-node:
type: object
properties:
x-origin:
...
And then here:
'^button-':
$ref: '#/$defs/overlay-node
unevaluatedProperties: false
properties:
label:
...
Rob
^ permalink raw reply
* [PATCH] Adding quirk for Betop Titanwolf Hotas Bundle
From: Anton @ 2023-08-31 13:09 UTC (permalink / raw)
To: benjamin.tissoires; +Cc: linux-input
Patch for enabling quirk for Betop Titanwolf Hotas Bundle joystick where missing one axis and second Hat
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 8a310f8ff20f..3d5abb9b2891 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -249,6 +249,7 @@
#define USB_VENDOR_ID_BETOP_2185PC 0x11c0
#define USB_VENDOR_ID_BETOP_2185V2PC 0x8380
#define USB_VENDOR_ID_BETOP_2185V2BFM 0x20bc
+#define USB_DEVICE_ID_BETOP_TWOLF_HOTAS_BUNDLE 0x5608
#define USB_VENDOR_ID_BIGBEN 0x146b
#define USB_DEVICE_ID_BIGBEN_PS3OFMINIPAD 0x0902
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index 3983b4f282f8..6d5e5e9f637a 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -206,6 +206,7 @@ static const struct hid_device_id hid_quirks[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_WISEGROUP, USB_DEVICE_ID_QUAD_USB_JOYPAD), HID_QUIRK_NOGET | HID_QUIRK_MULTI_INPUT },
{ HID_USB_DEVICE(USB_VENDOR_ID_XIN_MO, USB_DEVICE_ID_XIN_MO_DUAL_ARCADE), HID_QUIRK_MULTI_INPUT },
{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_GROUP_AUDIO), HID_QUIRK_NOGET },
+ { HID_USB_DEVICE(USB_VENDOR_ID_BETOP_2185PC, USB_DEVICE_ID_BETOP_TWOLF_HOTAS_BUNDLE), HID_QUIRK_INCREMENT_USAGE_ON_DUPLICATE },
{ 0 }
};
^ permalink raw reply related
* [dtor-input:for-linus] BUILD SUCCESS 1ac731c529cd4d6adbce134754b51ff7d822b145
From: kernel test robot @ 2023-08-31 13:39 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git for-linus
branch HEAD: 1ac731c529cd4d6adbce134754b51ff7d822b145 Merge branch 'next' into for-linus
elapsed time: 845m
configs tested: 161
configs skipped: 2
The following configs have been built successfully.
More configs may be tested in the coming days.
tested configs:
alpha allnoconfig gcc
alpha allyesconfig gcc
alpha defconfig gcc
alpha randconfig-r002-20230831 gcc
alpha randconfig-r015-20230831 gcc
alpha randconfig-r034-20230831 gcc
arc allmodconfig gcc
arc allnoconfig gcc
arc allyesconfig gcc
arc defconfig gcc
arc randconfig-001-20230831 gcc
arc randconfig-r024-20230831 gcc
arm allmodconfig gcc
arm allnoconfig gcc
arm allyesconfig gcc
arm defconfig gcc
arm randconfig-001-20230831 gcc
arm64 allmodconfig gcc
arm64 allnoconfig gcc
arm64 allyesconfig gcc
arm64 defconfig gcc
csky allmodconfig gcc
csky allnoconfig gcc
csky allyesconfig gcc
csky defconfig gcc
csky randconfig-r022-20230831 gcc
csky randconfig-r031-20230831 gcc
csky randconfig-r033-20230831 gcc
hexagon randconfig-001-20230831 clang
hexagon randconfig-002-20230831 clang
i386 allmodconfig gcc
i386 allnoconfig gcc
i386 allyesconfig gcc
i386 buildonly-randconfig-001-20230831 gcc
i386 buildonly-randconfig-002-20230831 gcc
i386 buildonly-randconfig-003-20230831 gcc
i386 buildonly-randconfig-004-20230831 gcc
i386 buildonly-randconfig-005-20230831 gcc
i386 buildonly-randconfig-006-20230831 gcc
i386 debian-10.3 gcc
i386 defconfig gcc
i386 randconfig-001-20230831 gcc
i386 randconfig-002-20230831 gcc
i386 randconfig-003-20230831 gcc
i386 randconfig-004-20230831 gcc
i386 randconfig-005-20230831 gcc
i386 randconfig-006-20230831 gcc
i386 randconfig-011-20230831 clang
i386 randconfig-012-20230831 clang
i386 randconfig-013-20230831 clang
i386 randconfig-014-20230831 clang
i386 randconfig-015-20230831 clang
i386 randconfig-016-20230831 clang
i386 randconfig-r013-20230831 clang
loongarch allmodconfig gcc
loongarch allnoconfig gcc
loongarch allyesconfig gcc
loongarch defconfig gcc
loongarch randconfig-001-20230831 gcc
m68k allmodconfig gcc
m68k allnoconfig gcc
m68k allyesconfig gcc
m68k defconfig gcc
microblaze allmodconfig gcc
microblaze allnoconfig gcc
microblaze allyesconfig gcc
microblaze defconfig gcc
mips allmodconfig gcc
mips allnoconfig gcc
mips allyesconfig gcc
mips randconfig-r016-20230831 gcc
mips randconfig-r023-20230831 gcc
nios2 allmodconfig gcc
nios2 allnoconfig gcc
nios2 allyesconfig gcc
nios2 defconfig gcc
openrisc allmodconfig gcc
openrisc allnoconfig gcc
openrisc allyesconfig gcc
openrisc defconfig gcc
openrisc randconfig-r001-20230831 gcc
openrisc randconfig-r006-20230831 gcc
openrisc randconfig-r032-20230831 gcc
parisc allmodconfig gcc
parisc allnoconfig gcc
parisc allyesconfig gcc
parisc defconfig gcc
parisc64 defconfig gcc
powerpc allmodconfig gcc
powerpc allnoconfig gcc
powerpc allyesconfig gcc
powerpc64 randconfig-r011-20230831 clang
powerpc64 randconfig-r026-20230831 clang
powerpc64 randconfig-r035-20230831 gcc
riscv allmodconfig gcc
riscv allnoconfig gcc
riscv allyesconfig gcc
riscv defconfig gcc
riscv randconfig-001-20230831 gcc
riscv randconfig-r004-20230831 gcc
riscv randconfig-r025-20230831 clang
riscv rv32_defconfig gcc
s390 allmodconfig gcc
s390 allnoconfig gcc
s390 allyesconfig gcc
s390 defconfig gcc
s390 randconfig-001-20230831 clang
s390 randconfig-r012-20230831 clang
s390 randconfig-r036-20230831 gcc
sh allmodconfig gcc
sh allnoconfig gcc
sh allyesconfig gcc
sh defconfig gcc
sparc allmodconfig gcc
sparc allnoconfig gcc
sparc allyesconfig gcc
sparc defconfig gcc
sparc randconfig-r005-20230831 gcc
sparc64 allmodconfig gcc
sparc64 allyesconfig gcc
sparc64 defconfig gcc
sparc64 randconfig-r014-20230831 gcc
um allmodconfig clang
um allnoconfig clang
um allyesconfig clang
um defconfig gcc
um i386_defconfig gcc
um randconfig-r021-20230831 gcc
um x86_64_defconfig gcc
x86_64 allnoconfig gcc
x86_64 allyesconfig gcc
x86_64 buildonly-randconfig-001-20230831 gcc
x86_64 buildonly-randconfig-002-20230831 gcc
x86_64 buildonly-randconfig-003-20230831 gcc
x86_64 buildonly-randconfig-004-20230831 gcc
x86_64 buildonly-randconfig-005-20230831 gcc
x86_64 buildonly-randconfig-006-20230831 gcc
x86_64 defconfig gcc
x86_64 randconfig-001-20230831 clang
x86_64 randconfig-002-20230831 clang
x86_64 randconfig-003-20230831 clang
x86_64 randconfig-004-20230831 clang
x86_64 randconfig-005-20230831 clang
x86_64 randconfig-006-20230831 clang
x86_64 randconfig-011-20230831 gcc
x86_64 randconfig-012-20230831 gcc
x86_64 randconfig-013-20230831 gcc
x86_64 randconfig-014-20230831 gcc
x86_64 randconfig-015-20230831 gcc
x86_64 randconfig-016-20230831 gcc
x86_64 randconfig-071-20230831 gcc
x86_64 randconfig-072-20230831 gcc
x86_64 randconfig-073-20230831 gcc
x86_64 randconfig-074-20230831 gcc
x86_64 randconfig-075-20230831 gcc
x86_64 randconfig-076-20230831 gcc
x86_64 rhel-8.3-rust clang
x86_64 rhel-8.3 gcc
xtensa allnoconfig gcc
xtensa allyesconfig gcc
xtensa randconfig-r003-20230831 gcc
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v4 4/4] dt-bindings: input: touchscreen: st1232: add example with touch-overlay
From: Rob Herring @ 2023-08-31 16:22 UTC (permalink / raw)
To: Javier Carrasco
Cc: Dmitry Torokhov, Krzysztof Kozlowski, Conor Dooley,
Henrik Rydberg, Bastian Hecht, Michael Riesch, linux-kernel,
linux-input, devicetree
In-Reply-To: <20230510-feature-ts_virtobj_patch-v4-4-5c6c0fc1eed6@wolfvision.net>
On Thu, Aug 24, 2023 at 03:17:12PM +0200, Javier Carrasco wrote:
> The st1232 driver supports the overlay-touchscreen and overlay-buttons
> objects defined in the generic touchscreen bindings and implemented in
> the touch-overlay module. Add an example where nodes for an overlay
> touchscreen and overlay buttons are defined.
>
> Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
> ---
> .../input/touchscreen/sitronix,st1232.yaml | 40 ++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml b/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml
> index 1d8ca19fd37a..857b611f84c2 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml
> +++ b/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml
> @@ -48,3 +48,43 @@ examples:
> gpios = <&gpio1 166 0>;
> };
> };
> + - |
> + #include <dt-bindings/input/linux-event-codes.h>
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + touchscreen@55 {
> + compatible = "sitronix,st1232";
> + reg = <0x55>;
> + interrupts = <2 0>;
> + gpios = <&gpio1 166 0>;
Just add this to the existing example. No value in 2 examples.
> +
> + overlay-touchscreen {
> + x-origin = <0>;
> + x-size = <240>;
> + y-origin = <40>;
> + y-size = <280>;
> + };
> +
> + overlay-buttons {
> + button-light {
> + label = "Camera light";
> + linux,code = <KEY_LIGHTS_TOGGLE>;
> + x-origin = <40>;
> + x-size = <40>;
> + y-origin = <0>;
> + y-size = <40>;
> + };
> +
> + button-power {
> + label = "Power";
> + linux,code = <KEY_POWER>;
> + x-origin = <160>;
> + x-size = <40>;
> + y-origin = <0>;
> + y-size = <40>;
> + };
> + };
> + };
> + };
>
> --
> 2.39.2
>
^ permalink raw reply
* Re: [PATCH v5] hid-mcp2200: added driver for GPIOs of MCP2200
From: Johannes Roith @ 2023-08-31 18:53 UTC (permalink / raw)
To: sergeantsagara
Cc: ak, andi.shyti, benjamin.tissoires, christophe.jaillet, jikos,
johannes, linux-input, linux-kernel, rdunlap
In-Reply-To: <87a5unt25h.fsf@protonmail.com>
Hi Rahul,
thanks for the feedback, I will add it to the driver.
> My personal recommendation is to just have a single DMA buffer allocated
> for the mcp2200 instance rather than having to call the allocator and
> release the memory per command.
I added an 16-byte Array hid_report to the mcp2000 struct. When I need the
report, I do the following:
struct mcp_set_clear_outputs *cmd;
mutex_lock(&mcp->lock);
cmd = (struct mcp_set_clear_outputs *) mcp->hid_report
Do you think this is a valid implementation or do I really have to add a
pointer to the mcp2200 struct instead of the 16 byte array and allocate
another 16 byte for it in the probe function?
> The reason you run into this is likely because of the action added to
> devm conflicting with hid_device_remove....
>
> I recommend not depending on devm for teardown rather than making a stub
> remove function to work around the issue.
I am not sure, if I have understand this correctly, but basically I already
have a stub remove function (which is empty). First the remove function gets
called, then the unregister function and everything is cleaned up correctly.
Did I get this right or do you have any other recommendation for me?
So, do I need any adaptions, or can we go with the empty remove function?
Best regards,
Johannes
^ permalink raw reply
* Re: [PATCH v10 2/2] Input: Add Novatek NT36xxx touchscreen driver
From: Konrad Dybcio @ 2023-09-01 0:37 UTC (permalink / raw)
To: Joel Selvaraj, AngeloGioacchino Del Regno, Dmitry Torokhov,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Henrik Rydberg
Cc: Marijn Suijten, linux-input, devicetree, linux-kernel, Dang Huynh,
Amit Pundir
In-Reply-To: <500b52ee-02e5-4809-b03b-21a9ad6d2b30@gmail.com>
On 18.08.2023 00:17, Joel Selvaraj wrote:
> Hi Konrad,
>
> On 8/16/23 12:36, Konrad Dybcio wrote:
>> Do you have your end outcome somewhere?
>
> Here is the driver changes on top of upstream "drivers/input/touchscreen/novatek-nvt-ts.c"
>
> Link: https://gitlab.com/sdm845-mainline/linux/-/commit/d2f7702a7f6a72eaf2655840036668398942c194
>
> and here is how I specified it in the Poco F1 dts:
>
> Link: https://gitlab.com/sdm845-mainline/linux/-/commit/4dd6e4578cc737d2584c7f9657f9f185effe9035
>
> Regards
> Joel Selvaraj
Hm, the one present in mainline is quite lackluster compared to this
one that Angelo made..
Konrad
^ permalink raw reply
* [PATCH v3] HID: i2c-hid: use print_hex_dump_debug to print report descriptor
From: Riwen Lu @ 2023-09-01 8:33 UTC (permalink / raw)
To: jikos, benjamin.tissoires, dmitry.torokhov, linux, hdegoede,
rrangel, u.kleine-koenig
Cc: linux-input, linux-kernel, sergeantsagara, Riwen Lu
In-Reply-To: <87bketu33z.fsf@protonmail.com>
From: Riwen Lu <luriwen@kylinos.cn>
The format '%*ph' prints up to 64 bytes long as a hex string with ' '
separator. Usually the size of report descriptor is larger than 64
bytes, so consider using print_hex_dump_debug to print out all of it for
better debugging.
Signed-off-by: Riwen Lu <luriwen@kylinos.cn>
---
v1->v2:
- Add a prefix for the hex dump.
v2->v3:
- Print the size of report descriptor.
---
drivers/hid/i2c-hid/i2c-hid-core.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index efbba0465eef..dd69abdd1f0d 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -772,7 +772,9 @@ static int i2c_hid_parse(struct hid_device *hid)
}
}
- i2c_hid_dbg(ihid, "Report Descriptor: %*ph\n", rsize, rdesc);
+ i2c_hid_dbg(ihid, "Report Descriptor size: %#x\n", rsize);
+ print_hex_dump_debug("Report Descriptor: ", DUMP_PREFIX_OFFSET, 16, 1,
+ rdesc, rsize, false);
ret = hid_parse_report(hid, rdesc, rsize);
if (!use_override)
--
2.25.1
^ permalink raw reply related
* Fwd: Betop Titanwolf Hotas Bundle missing axis and second Hat
From: Bagas Sanjaya @ 2023-09-01 12:34 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, llancelot7
Cc: Linux Kernel Mailing List, Linux Input Devices
Hi,
I notice a bug report on Bugzilla [1]. Quoting from it:
> I has USB Flashfire Cobra V5 Hotas wich recognized as:
> 11c0:5608 Betop Titanwolf Hotas Bundle
>
> Problem is in that bundle missing Throttle axis and second Hat, because it's mapped to Sync.Report:
>
> GenericDesktop.X ---> Absolute.X
> GenericDesktop.Y ---> Absolute.Y
> GenericDesktop.Slider ---> Absolute.Throttle
> Simulation.Rudder ---> Absolute.Rudder
> GenericDesktop.HatSwitch ---> Absolute.Hat0X
> GenericDesktop.HatSwitch ---> Sync.Report
> GenericDesktop.Rx ---> Absolute.Rx
> GenericDesktop.Ry ---> Absolute.Ry
> Simulation.Throttle ---> Sync.Report
>
>
> Kernel 6.4.10
>
> P.S. is it needs special "driver"(quirk)? Or hid joy really can recognize more than 1 Hat, because I saw only `map_abs(ABS_HAT0X);` for `HID_GD_HATSWITCH` in hid-input.c
See Bugzilla for the full thread and attached debug logs and a patch
that fixes the report.
Thanks.
--
An old man doll... just what I always wanted! - Clara
^ permalink raw reply
* Re: [PATCH v3] HID: i2c-hid: use print_hex_dump_debug to print report descriptor
From: Rahul Rameshbabu @ 2023-09-01 15:59 UTC (permalink / raw)
To: Riwen Lu
Cc: jikos, benjamin.tissoires, dmitry.torokhov, linux, hdegoede,
rrangel, u.kleine-koenig, linux-input, linux-kernel, Riwen Lu
In-Reply-To: <TYCP286MB26078CEC570EA9055D86D82DB1E4A@TYCP286MB2607.JPNP286.PROD.OUTLOOK.COM>
On Fri, 01 Sep, 2023 16:33:56 +0800 "Riwen Lu" <luriwen@hotmail.com> wrote:
> From: Riwen Lu <luriwen@kylinos.cn>
>
> The format '%*ph' prints up to 64 bytes long as a hex string with ' '
> separator. Usually the size of report descriptor is larger than 64
> bytes, so consider using print_hex_dump_debug to print out all of it for
> better debugging.
>
> Signed-off-by: Riwen Lu <luriwen@kylinos.cn>
>
> ---
> v1->v2:
> - Add a prefix for the hex dump.
>
> v2->v3:
> - Print the size of report descriptor.
> ---
> drivers/hid/i2c-hid/i2c-hid-core.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
> index efbba0465eef..dd69abdd1f0d 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
> @@ -772,7 +772,9 @@ static int i2c_hid_parse(struct hid_device *hid)
> }
> }
>
> - i2c_hid_dbg(ihid, "Report Descriptor: %*ph\n", rsize, rdesc);
> + i2c_hid_dbg(ihid, "Report Descriptor size: %#x\n", rsize);
> + print_hex_dump_debug("Report Descriptor: ", DUMP_PREFIX_OFFSET, 16, 1,
> + rdesc, rsize, false);
>
> ret = hid_parse_report(hid, rdesc, rsize);
> if (!use_override)
Thanks for the new version.
Reviewed-by: Rahul Rameshbabu <sergeantsagara@protonmail.com>
^ permalink raw reply
* Re: [PATCH v5] hid-mcp2200: added driver for GPIOs of MCP2200
From: Rahul Rameshbabu @ 2023-09-01 19:09 UTC (permalink / raw)
To: Johannes Roith
Cc: ak, andi.shyti, benjamin.tissoires, christophe.jaillet, jikos,
linux-input, linux-kernel, rdunlap
On Thu, 31 Aug, 2023 20:53:43 +0200 "Johannes Roith" <johannes@gnu-linux.rocks> wrote:
> Hi Rahul,
>
> thanks for the feedback, I will add it to the driver.
>
>> My personal recommendation is to just have a single DMA buffer allocated
>> for the mcp2200 instance rather than having to call the allocator and
>> release the memory per command.
>
> I added an 16-byte Array hid_report to the mcp2000 struct. When I need the
> report, I do the following:
>
> struct mcp_set_clear_outputs *cmd;
>
> mutex_lock(&mcp->lock);
> cmd = (struct mcp_set_clear_outputs *) mcp->hid_report
>
> Do you think this is a valid implementation or do I really have to add a
> pointer to the mcp2200 struct instead of the 16 byte array and allocate
> another 16 byte for it in the probe function?
This works fine since the mcp2000 struct will be dynamically allocated.
The reason I went with a separate allocation for the buffer was just to
make it explicitly clear that no matter how a thunderstrike instance is
set up, the buffer will need to be dynamically allocated for hid
requests.
>> The reason you run into this is likely because of the action added to
>> devm conflicting with hid_device_remove....
>>
>> I recommend not depending on devm for teardown rather than making a stub
>> remove function to work around the issue.
I have reinserted the relevant code from the core hid stack in my
previous email since it's important for this discussion.
static void hid_device_remove(struct device *dev)
{
struct hid_device *hdev = to_hid_device(dev);
struct hid_driver *hdrv;
down(&hdev->driver_input_lock);
hdev->io_started = false;
hdrv = hdev->driver;
if (hdrv) {
if (hdrv->remove)
hdrv->remove(hdev);
else /* default remove */
hid_hw_stop(hdev);
hid_device_remove will call hid_hw_stop and so will
mcp2200_hid_unregister because of the devm action you added.
>
> I am not sure, if I have understand this correctly, but basically I already
> have a stub remove function (which is empty). First the remove function gets
> called, then the unregister function and everything is cleaned up correctly.
> Did I get this right or do you have any other recommendation for me?
Let me try to break down the problem first.
1. You add mcp2200_hid_unregister to the devm actions for clean up the
device.
2. mcp2200_hid_unregister will call hid_hw_close and hid_hw_stop,
tearing down the device.
3. hid_device_remove is invoked when the device is removed, which
already calls hid_hw_stop when no remove function is registered (the
expectation is the device is simple when this is the case)
4. This leads to the device already being torn down, which leads to the
exception seen when the devm kicks in and mcp_hid_unregister is then
triggered.
5. Using an empty remove function resolves this but indicates the driver
has an inappropriate devm action in my opinion/has problematic
design.
I am saying that using an empty remove function to work
around the problem is not an upstream-able solution in my opinion.
Given this, I think its best to not use devm in this can and manually
handle cleanup, so you do not have a stub remove function and take
control of the teardown.
> So, do I need any adaptions, or can we go with the empty remove function?
That said, maybe someone else can chime in on this to see if this aligns
with others' preferences? At the very least though if others feel using
an empty remove function is ok, I think the comment in the remove
function needs to updated to add clear detail about the issue than what
is currently provided. That said, I am very much against using an empty
remove function to work around problematic devm practices.
/*
* With no remove function you sometimes get a segmentation fault when
* unloading the module or disconnecting the USB device
*/
--
Thanks,
Rahul Rameshbabu
^ permalink raw reply
* Re: [PATCH v2] HID: nintendo: reinitialize USB Pro Controller after resuming from suspend
From: Martino Fontana @ 2023-09-02 12:36 UTC (permalink / raw)
To: Silvan Jegen; +Cc: linux-input
In-Reply-To: <2XDNP3MKLE5L5.3MQT6EJ99UJBR@homearch.localdomain>
Hi
Is this patch (alongside "cleanup LED code") expected to land for Linux 6.6?
I'm asking because I can't find the patch in `for-next`.
https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git/log/?h=for-next
(Since it's my first patch here, I don't know if it's supposed to be seen
there.)
Cheers
On Sun, 23 Jul 2023 at 20:58, Silvan Jegen <s.jegen@gmail.com> wrote:
>
> Heyho
>
> Martino Fontana <tinozzo123@gmail.com> wrote:
> > It is my first patch on the Linux kernel, so I just did kinda what I
> > would do on GitHub (amend and force push).
> > What should I do here in case of trivial adjustments?
>
> I would leave it as is and only respin this one if either there are more
> comments on the old version or if the maintainer tells you to.
>
> BTW, just as a heads-up: posting an email reply at the top of the quoted
> email (aka "top-posting") is frowned upon in the Linux mailing lists. See
>
> https://people.kernel.org/tglx/
>
> for some pointers regarding netiquette.
>
> Cheers,
> Silvan
^ permalink raw reply
* Re: [PATCH (repost)] Input: MT - use __GFP_NOWARN allocation at input_mt_init_slots()
From: Tetsuo Handa @ 2023-09-03 13:55 UTC (permalink / raw)
To: Henrik Rydberg, Greg Kroah-Hartman
Cc: linux-input@vger.kernel.org, Dmitry Torokhov
In-Reply-To: <aa0d1e42-9a62-698b-fa89-000ce397316c@I-love.SAKURA.ne.jp>
Ping?
On 2023/03/28 20:25, Tetsuo Handa wrote:
> syzbot is reporting too large allocation at input_mt_init_slots(), for
> num_slots is supplied from userspace using ioctl(UI_DEV_CREATE).
>
> Use __GFP_NOWARN. Also, replace n2 with array_size(), for 32bits variable
> n2 will overflow if num_slots >= 65536 and will cause out of bounds
> read/write at input_mt_set_matrix()/input_mt_set_slots() on architectures
> where PAGE_SIZE > 4096 is available (e.g. PPC64).
>
> Reported-by: syzbot <syzbot+0122fa359a69694395d5@syzkaller.appspotmail.com>
> Link: https://syzkaller.appspot.com/bug?extid=0122fa359a69694395d5
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> ---
> Dmitry Torokhov proposed limiting max size to accept. But since it seems
> that nobody knows appropriate max size to accept, reposting as-is.
> https://lkml.kernel.org/r/03e8c3f0-bbbf-af37-6f52-67547cbd4cde@I-love.SAKURA.ne.jp
>
> drivers/input/input-mt.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/input/input-mt.c b/drivers/input/input-mt.c
> index 14b53dac1253..cf74579462ba 100644
> --- a/drivers/input/input-mt.c
> +++ b/drivers/input/input-mt.c
> @@ -47,7 +47,7 @@ int input_mt_init_slots(struct input_dev *dev, unsigned int num_slots,
> if (mt)
> return mt->num_slots != num_slots ? -EINVAL : 0;
>
> - mt = kzalloc(struct_size(mt, slots, num_slots), GFP_KERNEL);
> + mt = kzalloc(struct_size(mt, slots, num_slots), GFP_KERNEL | __GFP_NOWARN);
> if (!mt)
> goto err_mem;
>
> @@ -80,8 +80,8 @@ int input_mt_init_slots(struct input_dev *dev, unsigned int num_slots,
> if (flags & INPUT_MT_SEMI_MT)
> __set_bit(INPUT_PROP_SEMI_MT, dev->propbit);
> if (flags & INPUT_MT_TRACK) {
> - unsigned int n2 = num_slots * num_slots;
> - mt->red = kcalloc(n2, sizeof(*mt->red), GFP_KERNEL);
> + mt->red = kcalloc(array_size(num_slots, num_slots),
> + sizeof(*mt->red), GFP_KERNEL | __GFP_NOWARN);
> if (!mt->red)
> goto err_mem;
> }
^ permalink raw reply
* Re: [PATCH (repost)] Input: MT - use __GFP_NOWARN allocation at input_mt_init_slots()
From: Greg Kroah-Hartman @ 2023-09-03 14:03 UTC (permalink / raw)
To: Tetsuo Handa; +Cc: Henrik Rydberg, linux-input@vger.kernel.org, Dmitry Torokhov
In-Reply-To: <07350163-de52-a2bf-58bf-88c3d9d8d85b@I-love.SAKURA.ne.jp>
On Sun, Sep 03, 2023 at 10:55:45PM +0900, Tetsuo Handa wrote:
> Ping?
>
> On 2023/03/28 20:25, Tetsuo Handa wrote:
It's the middle of the merge window, no one can do anything until after
-rc1 is out, you know this...
greg k-h
^ permalink raw reply
* [PATCH] HID: sony: Fix a potential memory leak in sony_probe()
From: Christophe JAILLET @ 2023-09-03 16:04 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Pascal Giard
Cc: linux-kernel, kernel-janitors, Christophe JAILLET, Jiri Kosina,
linux-input
If an error occurs after a successful usb_alloc_urb() call, usb_free_urb()
should be called.
Fixes: fb1a79a6b6e1 ("HID: sony: fix freeze when inserting ghlive ps3/wii dongles")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
The NULL check is not needed, but I think that it is more informative
written this way.
---
drivers/hid/hid-sony.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index dd942061fd77..a02046a78b2d 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -2155,6 +2155,9 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
return ret;
err:
+ if (sc->ghl_urb)
+ usb_free_urb(sc->ghl_urb);
+
hid_hw_stop(hdev);
return ret;
}
--
2.34.1
^ permalink raw reply related
* [PATCH] input: gpio-keys - use spin_lock()
From: Lizhe @ 2023-09-03 16:03 UTC (permalink / raw)
To: dmitry.torokhov, geert+renesas, Jonathan.Cameron, soyer
Cc: linux-input, linux-kernel, Lizhe
Use the spin_lock() and spin_unlock() instead of spin_lock_irqsave()
and spin_unlock_restore()
Signed-off-by: Lizhe <sensor1010@163.com>
---
drivers/input/keyboard/gpio_keys.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index c928829a8b0c..a55d62e1ff6d 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -468,11 +468,10 @@ static irqreturn_t gpio_keys_irq_isr(int irq, void *dev_id)
{
struct gpio_button_data *bdata = dev_id;
struct input_dev *input = bdata->input;
- unsigned long flags;
BUG_ON(irq != bdata->irq);
- spin_lock_irqsave(&bdata->lock, flags);
+ spin_lock(&bdata->lock);
if (!bdata->key_pressed) {
if (bdata->button->wakeup)
@@ -495,7 +494,7 @@ static irqreturn_t gpio_keys_irq_isr(int irq, void *dev_id)
ms_to_ktime(bdata->release_delay),
HRTIMER_MODE_REL_HARD);
out:
- spin_unlock_irqrestore(&bdata->lock, flags);
+ spin_unlock(&bdata->lock);
return IRQ_HANDLED;
}
--
2.25.1
^ permalink raw reply related
* Re: [PATCH] input: gpio-keys - use spin_lock()
From: Geert Uytterhoeven @ 2023-09-04 7:46 UTC (permalink / raw)
To: Lizhe; +Cc: dmitry.torokhov, Jonathan.Cameron, soyer, linux-input,
linux-kernel
In-Reply-To: <20230903160349.3919-1-sensor1010@163.com>
Hi Lizhe,
Thanks for your patch!
On Sun, Sep 3, 2023 at 6:04 PM Lizhe <sensor1010@163.com> wrote:
> Use the spin_lock() and spin_unlock() instead of spin_lock_irqsave()
> and spin_unlock_restore()
Please explain why...
> Signed-off-by: Lizhe <sensor1010@163.com>
> --- a/drivers/input/keyboard/gpio_keys.c
> +++ b/drivers/input/keyboard/gpio_keys.c
> @@ -468,11 +468,10 @@ static irqreturn_t gpio_keys_irq_isr(int irq, void *dev_id)
> {
> struct gpio_button_data *bdata = dev_id;
> struct input_dev *input = bdata->input;
> - unsigned long flags;
>
> BUG_ON(irq != bdata->irq);
>
> - spin_lock_irqsave(&bdata->lock, flags);
> + spin_lock(&bdata->lock);
>
> if (!bdata->key_pressed) {
> if (bdata->button->wakeup)
> @@ -495,7 +494,7 @@ static irqreturn_t gpio_keys_irq_isr(int irq, void *dev_id)
> ms_to_ktime(bdata->release_delay),
> HRTIMER_MODE_REL_HARD);
> out:
> - spin_unlock_irqrestore(&bdata->lock, flags);
> + spin_unlock(&bdata->lock);
> return IRQ_HANDLED;
> }
Are you sure this is safe, given the interrupt is requested with
devm_request_any_context_irq(), and thus the handler may be registered
using either request_irq() or request_threaded_irq()?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH] HID: sony: Fix a potential memory leak in sony_probe()
From: Jiri Kosina @ 2023-09-04 9:14 UTC (permalink / raw)
To: Christophe JAILLET
Cc: Benjamin Tissoires, Pascal Giard, linux-kernel, kernel-janitors,
linux-input
In-Reply-To: <db06095c912d6bc56bed6b7e4663c7994072a2ce.1693757011.git.christophe.jaillet@wanadoo.fr>
On Sun, 3 Sep 2023, Christophe JAILLET wrote:
> If an error occurs after a successful usb_alloc_urb() call, usb_free_urb()
> should be called.
>
> Fixes: fb1a79a6b6e1 ("HID: sony: fix freeze when inserting ghlive ps3/wii dongles")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> The NULL check is not needed, but I think that it is more informative
> written this way.
> ---
> drivers/hid/hid-sony.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
> index dd942061fd77..a02046a78b2d 100644
> --- a/drivers/hid/hid-sony.c
> +++ b/drivers/hid/hid-sony.c
> @@ -2155,6 +2155,9 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
> return ret;
>
> err:
> + if (sc->ghl_urb)
> + usb_free_urb(sc->ghl_urb);
> +
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH 1/8] fbdev/smscufx: Use fb_ops helpers for deferred I/O
From: Javier Martinez Canillas @ 2023-09-04 12:59 UTC (permalink / raw)
To: Thomas Zimmermann, deller, daniel, sam
Cc: linux-fbdev, dri-devel, linux-staging, linux-hyperv, linux-input,
Thomas Zimmermann, Steve Glendinning
In-Reply-To: <20230828132131.29295-2-tzimmermann@suse.de>
Thomas Zimmermann <tzimmermann@suse.de> writes:
Hello Thomas,
> Generate callback functions for struct fb_ops with the fbdev macro
> FB_GEN_DEFAULT_DEFERRED_SYSMEM_OPS(). Initialize struct fb_ops to
> the generated functions with fbdev initializer macros.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Steve Glendinning <steve.glendinning@shawell.net>
> ---
The patch looks good to me, but I've a question below.
Acked-by: Javier Martinez Canillas <javierm@redhat.com>
> drivers/video/fbdev/smscufx.c | 85 +++++++++--------------------------
> 1 file changed, 22 insertions(+), 63 deletions(-)
>
> diff --git a/drivers/video/fbdev/smscufx.c b/drivers/video/fbdev/smscufx.c
[...]
> static const struct fb_ops ufx_ops = {
> .owner = THIS_MODULE,
> - .fb_read = fb_sys_read,
> - .fb_write = ufx_ops_write,
> + __FB_DEFAULT_DEFERRED_OPS_RDWR(ufx_ops),
> .fb_setcolreg = ufx_ops_setcolreg,
> - .fb_fillrect = ufx_ops_fillrect,
> - .fb_copyarea = ufx_ops_copyarea,
> - .fb_imageblit = ufx_ops_imageblit,
> + __FB_DEFAULT_DEFERRED_OPS_DRAW(ufx_ops),
> .fb_mmap = ufx_ops_mmap,
There are no generated functions for .fb_mmap, I wonder what's the value
of __FB_DEFAULT_DEFERRED_OPS_MMAP() ? Maybe just removing that macro and
setting .fb_mmap = fb_deferred_io_mmap instead if there's no custom mmap
handler would be easier to read ?
Alternatively, __FB_DEFAULT_DEFERRED_OPS_MMAP() could still be left but
not taking a __prefix argument since that is not used anyways ?
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
^ permalink raw reply
* Re: [PATCH 2/8] fbdev/udlfb: Use fb_ops helpers for deferred I/O
From: Javier Martinez Canillas @ 2023-09-04 13:05 UTC (permalink / raw)
To: Thomas Zimmermann, deller, daniel, sam
Cc: linux-fbdev, dri-devel, linux-staging, linux-hyperv, linux-input,
Thomas Zimmermann, Bernie Thompson
In-Reply-To: <20230828132131.29295-3-tzimmermann@suse.de>
Thomas Zimmermann <tzimmermann@suse.de> writes:
> Generate callback functions for struct fb_ops with the fbdev macro
> FB_GEN_DEFAULT_DEFERRED_SYSMEM_OPS(). Initialize struct fb_ops to
> the generated functions with fbdev initializer macros.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Bernie Thompson <bernie@plugable.com>
> ---
Acked-by: Javier Martinez Canillas <javierm@redhat.com>
[...]
> +static void dlfb_ops_damage_range(struct fb_info *info, off_t off, size_t len)
> +{
> + struct dlfb_data *dlfb = info->par;
> + int start = max((int)(off / info->fix.line_length), 0);
> + int lines = min((u32)((len / info->fix.line_length) + 1), (u32)info->var.yres);
> +
> + dlfb_handle_damage(dlfb, 0, start, info->var.xres, lines);
> +}
> +
> +static void dlfb_ops_damage_area(struct fb_info *info, u32 x, u32 y, u32 width, u32 height)
> +{
> + struct dlfb_data *dlfb = info->par;
> +
> + dlfb_offload_damage(dlfb, x, y, width, height);
> +}
> +
These two are very similar to the helpers you added for the smscufx driver
in patch #1. I guess there's room for further consolidation as follow-up ?
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
^ permalink raw reply
* Re: [PATCH 3/8] fbdev: Add Kconfig macro FB_IOMEM_HELPERS_DEFERRED
From: Javier Martinez Canillas @ 2023-09-04 13:10 UTC (permalink / raw)
To: Thomas Zimmermann, deller, daniel, sam
Cc: linux-fbdev, dri-devel, linux-staging, linux-hyperv, linux-input,
Thomas Zimmermann
In-Reply-To: <20230828132131.29295-4-tzimmermann@suse.de>
Thomas Zimmermann <tzimmermann@suse.de> writes:
> The new Kconfig macro FB_IOMEM_HELPERS_DEFERRED selects fbdev's
> helpers for device I/O memory and deferred I/O. Drivers should
> use it if they perform damage updates on device I/O memory.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
Acked-by: Javier Martinez Canillas <javierm@redhat.com>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
^ permalink raw reply
* Re: [PATCH 4/8] fbdev/hyperv_fb: Use fb_ops helpers for deferred I/O
From: Javier Martinez Canillas @ 2023-09-04 13:18 UTC (permalink / raw)
To: Thomas Zimmermann, deller, daniel, sam
Cc: linux-fbdev, dri-devel, linux-staging, linux-hyperv, linux-input,
Thomas Zimmermann, K. Y. Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui
In-Reply-To: <20230828132131.29295-5-tzimmermann@suse.de>
Thomas Zimmermann <tzimmermann@suse.de> writes:
> Generate callback functions for struct fb_ops with the fbdev macro
> FB_GEN_DEFAULT_DEFERRED_IOMEM_OPS(). Initialize struct fb_ops to
> the generated functions with fbdev initializer macros.
>
> The hyperv_fb driver is incomplete in its handling of deferred I/O
> and damage framebuffers. Write operations do no trigger damage handling.
> Fixing this is beyond the scope of this patch.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Javier Martinez Canillas <javierm@redhat.com>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
^ permalink raw reply
* Re: [PATCH 5/8] hid: Remove trailing whitespace
From: Javier Martinez Canillas @ 2023-09-04 13:24 UTC (permalink / raw)
To: Thomas Zimmermann, deller, daniel, sam
Cc: linux-fbdev, dri-devel, linux-staging, linux-hyperv, linux-input,
Thomas Zimmermann, Jiri Kosina, Benjamin Tissoires
In-Reply-To: <20230828132131.29295-6-tzimmermann@suse.de>
Thomas Zimmermann <tzimmermann@suse.de> writes:
> Fix coding style in Kconfig. No functional changes.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Jiri Kosina <jikos@kernel.org>
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
Acked-by: Javier Martinez Canillas <javierm@redhat.com>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox