* [HID Patchsets for Samsung driver v3 0/6]
From: Sandeep C S @ 2024-01-24 16:10 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: gaudium.lee, ih0923.kim, suhyun_.kim, jitender.s21, junwan.cho,
sandeep.cs, linux-input, linux-kernel
In-Reply-To: <CGME20240124161058epcas5p40b641fe9049630c2a9ce69cab17c40b9@epcas5p4.samsung.com>
Dear Kernel Team Members,
I hope this email finds you well.
As per the review comments given by Mr. Joe Perches in our last converstaion over mail.
We have incorporated the feedback on our driver. Please check this set of series and help us to improve the samsung driver.
As of today, Opensource kernel Samsung driver only supports USB HID devices and do not have support for Bluetooth HID devices.
Samsung would like to improve the samsung driver and extend it's support for bluetooth devices as well.
Summary of changes in Samsung driver:
1. Add support for below bluetooth devices
Samsung wireless Keyboard
Samsung wireless GamePad
Samsung Wireless Action Mouse
Samsung Wireless Book Cover
Samsung Wireless Universal Keyboard
Samsung Wireless HOGP Keyboard
2. Add support for Special key processing on each of the above devices.
Patch Series Overview:
--------------------------------------
[Patch 1/6]
HID Samsung : Broaden device compatibility in samsung driver.
hid_is_usb() check being moved.
[Patch 2/6]
HID: Samsung : Fix the checkpatch complain.
Warning found by checkpatch.pl script.
[Patch 3/6]
HID: Samsung : Add Samsung wireless keyboard support.
[Patch 4/6]
HID: Samsung : Add Samsung wireless gamepad support.
[Patch 5/6]
HID: Samsung : Add Samsung wireless action mouse support.
[Patch 6/6]
HID: Samsung : Add Samsung wireless bookcover and universal keyboard support.
All these changes have been verified and tested thoroughly in android devices.
Please accept our changes.
Thanks for your time and consideration.
Best regards
Sandeep C S
Sandeep C S (6):
HID Samsung : Broaden device compatibility in samsung driver.
HID: Samsung : Fix the checkpatch complain. Rewritten code using
memcmp where applicable.
HID: Samsung : Add Samsung wireless keyboard support.
HID: Samsung : Add Samsung wireless gamepad support.
HID: Samsung : Add Samsung wireless action mouse support.
HID: Samsung : Add Samsung wireless bookcover and universal keyboard
support.
drivers/hid/hid-ids.h | 7 +
drivers/hid/hid-samsung.c | 459 +++++++++++++++++++++++++++++++++++---
2 files changed, 430 insertions(+), 36 deletions(-)
--
2.34.1
^ permalink raw reply
* [PATCH v2 3/3] HID: bpf: use __bpf_kfunc instead of noinline
From: Benjamin Tissoires @ 2024-01-24 11:26 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Dan Carpenter, Daniel Borkmann,
Andrii Nakryiko
Cc: linux-input, linux-kernel, bpf, Benjamin Tissoires
In-Reply-To: <20240124-b4-hid-bpf-fixes-v2-0-052520b1e5e6@kernel.org>
Follow the docs at Documentation/bpf/kfuncs.rst:
- declare the function with `__bpf_kfunc`
- disables missing prototype warnings, which allows to remove them from
include/linux/hid-bpf.h
Removing the prototypes is not an issue because we currently have to
redeclare them when writing the BPF program. They will eventually be
generated by bpftool directly AFAIU.
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
changes in v2:
* make use of __bpf_kfunc_start/end_defs() instead of manual push/pop
---
drivers/hid/bpf/hid_bpf_dispatch.c | 18 +++++++++++++-----
include/linux/hid_bpf.h | 11 -----------
2 files changed, 13 insertions(+), 16 deletions(-)
diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c
index 7903c8638e81..470ae2c29c94 100644
--- a/drivers/hid/bpf/hid_bpf_dispatch.c
+++ b/drivers/hid/bpf/hid_bpf_dispatch.c
@@ -143,6 +143,9 @@ u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, u8 *rdesc, unsigned int *s
}
EXPORT_SYMBOL_GPL(call_hid_bpf_rdesc_fixup);
+/* Disables missing prototype warnings */
+__bpf_kfunc_start_defs();
+
/**
* hid_bpf_get_data - Get the kernel memory pointer associated with the context @ctx
*
@@ -152,7 +155,7 @@ EXPORT_SYMBOL_GPL(call_hid_bpf_rdesc_fixup);
*
* @returns %NULL on error, an %__u8 memory pointer on success
*/
-noinline __u8 *
+__bpf_kfunc __u8 *
hid_bpf_get_data(struct hid_bpf_ctx *ctx, unsigned int offset, const size_t rdwr_buf_size)
{
struct hid_bpf_ctx_kern *ctx_kern;
@@ -167,6 +170,7 @@ hid_bpf_get_data(struct hid_bpf_ctx *ctx, unsigned int offset, const size_t rdwr
return ctx_kern->data + offset;
}
+__bpf_kfunc_end_defs();
/*
* The following set contains all functions we agree BPF programs
@@ -274,6 +278,9 @@ static int do_hid_bpf_attach_prog(struct hid_device *hdev, int prog_fd, struct b
return fd;
}
+/* Disables missing prototype warnings */
+__bpf_kfunc_start_defs();
+
/**
* hid_bpf_attach_prog - Attach the given @prog_fd to the given HID device
*
@@ -286,7 +293,7 @@ static int do_hid_bpf_attach_prog(struct hid_device *hdev, int prog_fd, struct b
* is pinned to the BPF file system).
*/
/* called from syscall */
-noinline int
+__bpf_kfunc int
hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags)
{
struct hid_device *hdev;
@@ -338,7 +345,7 @@ hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags)
*
* @returns A pointer to &struct hid_bpf_ctx on success, %NULL on error.
*/
-noinline struct hid_bpf_ctx *
+__bpf_kfunc struct hid_bpf_ctx *
hid_bpf_allocate_context(unsigned int hid_id)
{
struct hid_device *hdev;
@@ -371,7 +378,7 @@ hid_bpf_allocate_context(unsigned int hid_id)
* @ctx: the HID-BPF context to release
*
*/
-noinline void
+__bpf_kfunc void
hid_bpf_release_context(struct hid_bpf_ctx *ctx)
{
struct hid_bpf_ctx_kern *ctx_kern;
@@ -397,7 +404,7 @@ hid_bpf_release_context(struct hid_bpf_ctx *ctx)
*
* @returns %0 on success, a negative error code otherwise.
*/
-noinline int
+__bpf_kfunc int
hid_bpf_hw_request(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz,
enum hid_report_type rtype, enum hid_class_request reqtype)
{
@@ -465,6 +472,7 @@ hid_bpf_hw_request(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz,
kfree(dma_data);
return ret;
}
+__bpf_kfunc_end_defs();
/* our HID-BPF entrypoints */
BTF_SET8_START(hid_bpf_fmodret_ids)
diff --git a/include/linux/hid_bpf.h b/include/linux/hid_bpf.h
index 840cd254172d..7118ac28d468 100644
--- a/include/linux/hid_bpf.h
+++ b/include/linux/hid_bpf.h
@@ -77,17 +77,6 @@ enum hid_bpf_attach_flags {
int hid_bpf_device_event(struct hid_bpf_ctx *ctx);
int hid_bpf_rdesc_fixup(struct hid_bpf_ctx *ctx);
-/* Following functions are kfunc that we export to BPF programs */
-/* available everywhere in HID-BPF */
-__u8 *hid_bpf_get_data(struct hid_bpf_ctx *ctx, unsigned int offset, const size_t __sz);
-
-/* only available in syscall */
-int hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags);
-int hid_bpf_hw_request(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz,
- enum hid_report_type rtype, enum hid_class_request reqtype);
-struct hid_bpf_ctx *hid_bpf_allocate_context(unsigned int hid_id);
-void hid_bpf_release_context(struct hid_bpf_ctx *ctx);
-
/*
* Below is HID internal
*/
--
2.43.0
^ permalink raw reply related
* [PATCH v2 2/3] HID: bpf: actually free hdev memory after attaching a HID-BPF program
From: Benjamin Tissoires @ 2024-01-24 11:26 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Dan Carpenter, Daniel Borkmann,
Andrii Nakryiko
Cc: linux-input, linux-kernel, bpf, Benjamin Tissoires, stable
In-Reply-To: <20240124-b4-hid-bpf-fixes-v2-0-052520b1e5e6@kernel.org>
Turns out that I got my reference counts wrong and each successful
bus_find_device() actually calls get_device(), and we need to manually
call put_device().
Ensure each bus_find_device() gets a matching put_device() when releasing
the bpf programs and fix all the error paths.
Cc: stable@vger.kernel.org
Fixes: f5c27da4e3c8 ("HID: initial BPF implementation")
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
new in v2
---
drivers/hid/bpf/hid_bpf_dispatch.c | 29 +++++++++++++++++++++++------
drivers/hid/bpf/hid_bpf_jmp_table.c | 19 ++++++++++++++++---
2 files changed, 39 insertions(+), 9 deletions(-)
diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c
index 5111d1fef0d3..7903c8638e81 100644
--- a/drivers/hid/bpf/hid_bpf_dispatch.c
+++ b/drivers/hid/bpf/hid_bpf_dispatch.c
@@ -292,7 +292,7 @@ hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags)
struct hid_device *hdev;
struct bpf_prog *prog;
struct device *dev;
- int fd;
+ int err, fd;
if (!hid_bpf_ops)
return -EINVAL;
@@ -311,14 +311,24 @@ hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags)
* on errors or when it'll be detached
*/
prog = bpf_prog_get(prog_fd);
- if (IS_ERR(prog))
- return PTR_ERR(prog);
+ if (IS_ERR(prog)) {
+ err = PTR_ERR(prog);
+ goto out_dev_put;
+ }
fd = do_hid_bpf_attach_prog(hdev, prog_fd, prog, flags);
- if (fd < 0)
- bpf_prog_put(prog);
+ if (fd < 0) {
+ err = fd;
+ goto out_prog_put;
+ }
return fd;
+
+ out_prog_put:
+ bpf_prog_put(prog);
+ out_dev_put:
+ put_device(dev);
+ return err;
}
/**
@@ -345,8 +355,10 @@ hid_bpf_allocate_context(unsigned int hid_id)
hdev = to_hid_device(dev);
ctx_kern = kzalloc(sizeof(*ctx_kern), GFP_KERNEL);
- if (!ctx_kern)
+ if (!ctx_kern) {
+ put_device(dev);
return NULL;
+ }
ctx_kern->ctx.hid = hdev;
@@ -363,10 +375,15 @@ noinline void
hid_bpf_release_context(struct hid_bpf_ctx *ctx)
{
struct hid_bpf_ctx_kern *ctx_kern;
+ struct hid_device *hid;
ctx_kern = container_of(ctx, struct hid_bpf_ctx_kern, ctx);
+ hid = (struct hid_device *)ctx_kern->ctx.hid; /* ignore const */
kfree(ctx_kern);
+
+ /* get_device() is called by bus_find_device() */
+ put_device(&hid->dev);
}
/**
diff --git a/drivers/hid/bpf/hid_bpf_jmp_table.c b/drivers/hid/bpf/hid_bpf_jmp_table.c
index 12f7cebddd73..85a24bc0ea25 100644
--- a/drivers/hid/bpf/hid_bpf_jmp_table.c
+++ b/drivers/hid/bpf/hid_bpf_jmp_table.c
@@ -196,6 +196,7 @@ static void __hid_bpf_do_release_prog(int map_fd, unsigned int idx)
static void hid_bpf_release_progs(struct work_struct *work)
{
int i, j, n, map_fd = -1;
+ bool hdev_destroyed;
if (!jmp_table.map)
return;
@@ -220,6 +221,12 @@ static void hid_bpf_release_progs(struct work_struct *work)
if (entry->hdev) {
hdev = entry->hdev;
type = entry->type;
+ /*
+ * hdev is still valid, even if we are called after hid_destroy_device():
+ * when hid_bpf_attach() gets called, it takes a ref on the dev through
+ * bus_find_device()
+ */
+ hdev_destroyed = hdev->bpf.destroyed;
hid_bpf_populate_hdev(hdev, type);
@@ -232,12 +239,18 @@ static void hid_bpf_release_progs(struct work_struct *work)
if (test_bit(next->idx, jmp_table.enabled))
continue;
- if (next->hdev == hdev && next->type == type)
+ if (next->hdev == hdev && next->type == type) {
+ /*
+ * clear the hdev reference and decrement the device ref
+ * that was taken during bus_find_device() while calling
+ * hid_bpf_attach()
+ */
next->hdev = NULL;
+ put_device(&hdev->dev);
}
- /* if type was rdesc fixup, reconnect device */
- if (type == HID_BPF_PROG_TYPE_RDESC_FIXUP)
+ /* if type was rdesc fixup and the device is not gone, reconnect device */
+ if (type == HID_BPF_PROG_TYPE_RDESC_FIXUP && !hdev_destroyed)
hid_bpf_reconnect(hdev);
}
}
--
2.43.0
^ permalink raw reply related
* [PATCH v2 1/3] HID: bpf: remove double fdget()
From: Benjamin Tissoires @ 2024-01-24 11:26 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Dan Carpenter, Daniel Borkmann,
Andrii Nakryiko
Cc: linux-input, linux-kernel, bpf, Benjamin Tissoires, stable
In-Reply-To: <20240124-b4-hid-bpf-fixes-v2-0-052520b1e5e6@kernel.org>
When the kfunc hid_bpf_attach_prog() is called, we called twice fdget():
one for fetching the type of the bpf program, and one for actually
attaching the program to the device.
The problem is that between those two calls, we have no guarantees that
the prog_fd is still the same file descriptor for the given program.
Solve this by calling bpf_prog_get() earlier, and use this to fetch the
program type.
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Link: https://lore.kernel.org/bpf/CAO-hwJJ8vh8JD3-P43L-_CLNmPx0hWj44aom0O838vfP4=_1CA@mail.gmail.com/T/#t
Cc: stable@vger.kernel.org
Fixes: f5c27da4e3c8 ("HID: initial BPF implementation")
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
changes in v2:
* add missing Fixes tag in the commit description
---
drivers/hid/bpf/hid_bpf_dispatch.c | 66 ++++++++++++++++++++++++-------------
drivers/hid/bpf/hid_bpf_dispatch.h | 4 +--
drivers/hid/bpf/hid_bpf_jmp_table.c | 20 ++---------
3 files changed, 49 insertions(+), 41 deletions(-)
diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c
index d9ef45fcaeab..5111d1fef0d3 100644
--- a/drivers/hid/bpf/hid_bpf_dispatch.c
+++ b/drivers/hid/bpf/hid_bpf_dispatch.c
@@ -241,6 +241,39 @@ int hid_bpf_reconnect(struct hid_device *hdev)
return 0;
}
+static int do_hid_bpf_attach_prog(struct hid_device *hdev, int prog_fd, struct bpf_prog *prog,
+ __u32 flags)
+{
+ int fd, err, prog_type;
+
+ prog_type = hid_bpf_get_prog_attach_type(prog);
+ if (prog_type < 0)
+ return prog_type;
+
+ if (prog_type >= HID_BPF_PROG_TYPE_MAX)
+ return -EINVAL;
+
+ if (prog_type == HID_BPF_PROG_TYPE_DEVICE_EVENT) {
+ err = hid_bpf_allocate_event_data(hdev);
+ if (err)
+ return err;
+ }
+
+ fd = __hid_bpf_attach_prog(hdev, prog_type, prog_fd, prog, flags);
+ if (fd < 0)
+ return fd;
+
+ if (prog_type == HID_BPF_PROG_TYPE_RDESC_FIXUP) {
+ err = hid_bpf_reconnect(hdev);
+ if (err) {
+ close_fd(fd);
+ return err;
+ }
+ }
+
+ return fd;
+}
+
/**
* hid_bpf_attach_prog - Attach the given @prog_fd to the given HID device
*
@@ -257,18 +290,13 @@ noinline int
hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags)
{
struct hid_device *hdev;
+ struct bpf_prog *prog;
struct device *dev;
- int fd, err, prog_type = hid_bpf_get_prog_attach_type(prog_fd);
+ int fd;
if (!hid_bpf_ops)
return -EINVAL;
- if (prog_type < 0)
- return prog_type;
-
- if (prog_type >= HID_BPF_PROG_TYPE_MAX)
- return -EINVAL;
-
if ((flags & ~HID_BPF_FLAG_MASK))
return -EINVAL;
@@ -278,23 +306,17 @@ hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags)
hdev = to_hid_device(dev);
- if (prog_type == HID_BPF_PROG_TYPE_DEVICE_EVENT) {
- err = hid_bpf_allocate_event_data(hdev);
- if (err)
- return err;
- }
+ /*
+ * take a ref on the prog itself, it will be released
+ * on errors or when it'll be detached
+ */
+ prog = bpf_prog_get(prog_fd);
+ if (IS_ERR(prog))
+ return PTR_ERR(prog);
- fd = __hid_bpf_attach_prog(hdev, prog_type, prog_fd, flags);
+ fd = do_hid_bpf_attach_prog(hdev, prog_fd, prog, flags);
if (fd < 0)
- return fd;
-
- if (prog_type == HID_BPF_PROG_TYPE_RDESC_FIXUP) {
- err = hid_bpf_reconnect(hdev);
- if (err) {
- close_fd(fd);
- return err;
- }
- }
+ bpf_prog_put(prog);
return fd;
}
diff --git a/drivers/hid/bpf/hid_bpf_dispatch.h b/drivers/hid/bpf/hid_bpf_dispatch.h
index 63dfc8605cd2..fbe0639d09f2 100644
--- a/drivers/hid/bpf/hid_bpf_dispatch.h
+++ b/drivers/hid/bpf/hid_bpf_dispatch.h
@@ -12,9 +12,9 @@ struct hid_bpf_ctx_kern {
int hid_bpf_preload_skel(void);
void hid_bpf_free_links_and_skel(void);
-int hid_bpf_get_prog_attach_type(int prog_fd);
+int hid_bpf_get_prog_attach_type(struct bpf_prog *prog);
int __hid_bpf_attach_prog(struct hid_device *hdev, enum hid_bpf_prog_type prog_type, int prog_fd,
- __u32 flags);
+ struct bpf_prog *prog, __u32 flags);
void __hid_bpf_destroy_device(struct hid_device *hdev);
int hid_bpf_prog_run(struct hid_device *hdev, enum hid_bpf_prog_type type,
struct hid_bpf_ctx_kern *ctx_kern);
diff --git a/drivers/hid/bpf/hid_bpf_jmp_table.c b/drivers/hid/bpf/hid_bpf_jmp_table.c
index eca34b7372f9..12f7cebddd73 100644
--- a/drivers/hid/bpf/hid_bpf_jmp_table.c
+++ b/drivers/hid/bpf/hid_bpf_jmp_table.c
@@ -333,15 +333,10 @@ static int hid_bpf_insert_prog(int prog_fd, struct bpf_prog *prog)
return err;
}
-int hid_bpf_get_prog_attach_type(int prog_fd)
+int hid_bpf_get_prog_attach_type(struct bpf_prog *prog)
{
- struct bpf_prog *prog = NULL;
- int i;
int prog_type = HID_BPF_PROG_TYPE_UNDEF;
-
- prog = bpf_prog_get(prog_fd);
- if (IS_ERR(prog))
- return PTR_ERR(prog);
+ int i;
for (i = 0; i < HID_BPF_PROG_TYPE_MAX; i++) {
if (hid_bpf_btf_ids[i] == prog->aux->attach_btf_id) {
@@ -350,8 +345,6 @@ int hid_bpf_get_prog_attach_type(int prog_fd)
}
}
- bpf_prog_put(prog);
-
return prog_type;
}
@@ -388,19 +381,13 @@ static const struct bpf_link_ops hid_bpf_link_lops = {
/* called from syscall */
noinline int
__hid_bpf_attach_prog(struct hid_device *hdev, enum hid_bpf_prog_type prog_type,
- int prog_fd, __u32 flags)
+ int prog_fd, struct bpf_prog *prog, __u32 flags)
{
struct bpf_link_primer link_primer;
struct hid_bpf_link *link;
- struct bpf_prog *prog = NULL;
struct hid_bpf_prog_entry *prog_entry;
int cnt, err = -EINVAL, prog_table_idx = -1;
- /* take a ref on the prog itself */
- prog = bpf_prog_get(prog_fd);
- if (IS_ERR(prog))
- return PTR_ERR(prog);
-
mutex_lock(&hid_bpf_attach_lock);
link = kzalloc(sizeof(*link), GFP_USER);
@@ -467,7 +454,6 @@ __hid_bpf_attach_prog(struct hid_device *hdev, enum hid_bpf_prog_type prog_type,
err_unlock:
mutex_unlock(&hid_bpf_attach_lock);
- bpf_prog_put(prog);
kfree(link);
return err;
--
2.43.0
^ permalink raw reply related
* [PATCH v2 0/3] HID: bpf: couple of upstream fixes
From: Benjamin Tissoires @ 2024-01-24 11:26 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Dan Carpenter, Daniel Borkmann,
Andrii Nakryiko
Cc: linux-input, linux-kernel, bpf, Benjamin Tissoires, stable
Hi,
This is the v2 of this series of HID-BPF fixes.
I have forgotten to include a Fixes tag in the first patch
and got a review from Andrii on patch 2.
And this first patch made me realize that something was fishy
in the refcount of the hid devices. I was not crashing the system
even if I accessed the struct hid_device after hid_destroy_device()
was called, which was suspicious to say the least. So after some
debugging I found the culprit and realized that I had a pretty
nice memleak as soon as one HID-BPF program was attached to a HID
device.
The good thing though is that this ref count prevents a crash in
case a HID-BPF program attempts to access a removed HID device when
hid_bpf_allocate_context() has been called but not yet released.
Anyway, for reference, the cover letter of v1:
---
Hi,
these are a couple of fixes for hid-bpf. The first one should
probably go in ASAP, after the reviews, and the second one is nice
to have and doesn't hurt much.
Thanks Dan for finding out the issue with bpf_prog_get()
Cheers,
Benjamin
To: Jiri Kosina <jikos@kernel.org>
To: Benjamin Tissoires <benjamin.tissoires@redhat.com>
To: Dan Carpenter <dan.carpenter@linaro.org>
To: Daniel Borkmann <daniel@iogearbox.net>
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: <linux-input@vger.kernel.org>
Cc: <linux-kernel@vger.kernel.org>
Cc: <bpf@vger.kernel.org>
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
Changes in v2:
- add Fixes tags
- handled Andrii review (use of __bpf_kfunc_start/end_defs())
- new patch to fetch ref counting of struct hid_device
- Link to v1: https://lore.kernel.org/r/20240123-b4-hid-bpf-fixes-v1-0-aa1fac734377@kernel.org
---
Benjamin Tissoires (3):
HID: bpf: remove double fdget()
HID: bpf: actually free hdev memory after attaching a HID-BPF program
HID: bpf: use __bpf_kfunc instead of noinline
drivers/hid/bpf/hid_bpf_dispatch.c | 101 ++++++++++++++++++++++++++----------
drivers/hid/bpf/hid_bpf_dispatch.h | 4 +-
drivers/hid/bpf/hid_bpf_jmp_table.c | 39 +++++++-------
include/linux/hid_bpf.h | 11 ----
4 files changed, 95 insertions(+), 60 deletions(-)
---
base-commit: fef018d8199661962b5fc0f0d1501caa54b2b533
change-id: 20240123-b4-hid-bpf-fixes-662908fe2234
Best regards,
--
Benjamin Tissoires <bentiss@kernel.org>
^ permalink raw reply
* Re: [PATCH 2/2] HID: bpf: use __bpf_kfunc instead of noinline
From: Benjamin Tissoires @ 2024-01-24 9:58 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Benjamin Tissoires, Jiri Kosina, Dan Carpenter, Daniel Borkmann,
linux-input, linux-kernel, bpf
In-Reply-To: <CAEf4BzbE-YHR1FuXfG7ryi_77H=nzF0XBoppqrbG4Uh1uPz8Lg@mail.gmail.com>
On Tue, Jan 23, 2024 at 8:57 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Tue, Jan 23, 2024 at 8:46 AM Benjamin Tissoires <bentiss@kernel.org> wrote:
> >
> > Follow the docs at Documentation/bpf/kfuncs.rst:
> > - declare the function with `__bpf_kfunc`
> > - disables missing prototype warnings, which allows to remove them from
> > include/linux/hid-bpf.h
> >
> > Removing the prototypes is not an issue because we currently have to
> > redeclare them when writing the BPF program. They will eventually be
> > generated by bpftool directly AFAIU.
> >
> > Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
> > ---
> > drivers/hid/bpf/hid_bpf_dispatch.c | 22 +++++++++++++++++-----
> > include/linux/hid_bpf.h | 11 -----------
> > 2 files changed, 17 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c
> > index 5111d1fef0d3..119e4f03df55 100644
> > --- a/drivers/hid/bpf/hid_bpf_dispatch.c
> > +++ b/drivers/hid/bpf/hid_bpf_dispatch.c
> > @@ -143,6 +143,11 @@ u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, u8 *rdesc, unsigned int *s
> > }
> > EXPORT_SYMBOL_GPL(call_hid_bpf_rdesc_fixup);
> >
> > +/* Disables missing prototype warnings */
> > +__diag_push();
> > +__diag_ignore_all("-Wmissing-prototypes",
> > + "Global kfuncs as their definitions will be in BTF");
> > +
>
> would it be possible to use __bpf_kfunc_start_defs() and
> __bpf_kfunc_end_defs() macros instead of explicit __diag push/pop
> pairs? It's defined in include/linux/btf.h
Sure, I'll use them in v2.
I also realized that I had some memory leaks because I never called
put_device() after bus_find_device(), so I need to add another fix to
this series.
Cheers,
Benjamin
>
> > /**
> > * hid_bpf_get_data - Get the kernel memory pointer associated with the context @ctx
> > *
> > @@ -152,7 +157,7 @@ EXPORT_SYMBOL_GPL(call_hid_bpf_rdesc_fixup);
> > *
> > * @returns %NULL on error, an %__u8 memory pointer on success
> > */
> > -noinline __u8 *
> > +__bpf_kfunc __u8 *
> > hid_bpf_get_data(struct hid_bpf_ctx *ctx, unsigned int offset, const size_t rdwr_buf_size)
> > {
> > struct hid_bpf_ctx_kern *ctx_kern;
> > @@ -167,6 +172,7 @@ hid_bpf_get_data(struct hid_bpf_ctx *ctx, unsigned int offset, const size_t rdwr
> >
> > return ctx_kern->data + offset;
> > }
> > +__diag_pop(); /* missing prototype warnings */
> >
> > /*
> > * The following set contains all functions we agree BPF programs
> > @@ -274,6 +280,11 @@ static int do_hid_bpf_attach_prog(struct hid_device *hdev, int prog_fd, struct b
> > return fd;
> > }
> >
> > +/* Disables missing prototype warnings */
> > +__diag_push();
> > +__diag_ignore_all("-Wmissing-prototypes",
> > + "Global kfuncs as their definitions will be in BTF");
> > +
> > /**
> > * hid_bpf_attach_prog - Attach the given @prog_fd to the given HID device
> > *
> > @@ -286,7 +297,7 @@ static int do_hid_bpf_attach_prog(struct hid_device *hdev, int prog_fd, struct b
> > * is pinned to the BPF file system).
> > */
> > /* called from syscall */
> > -noinline int
> > +__bpf_kfunc int
> > hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags)
> > {
> > struct hid_device *hdev;
> > @@ -328,7 +339,7 @@ hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags)
> > *
> > * @returns A pointer to &struct hid_bpf_ctx on success, %NULL on error.
> > */
> > -noinline struct hid_bpf_ctx *
> > +__bpf_kfunc struct hid_bpf_ctx *
> > hid_bpf_allocate_context(unsigned int hid_id)
> > {
> > struct hid_device *hdev;
> > @@ -359,7 +370,7 @@ hid_bpf_allocate_context(unsigned int hid_id)
> > * @ctx: the HID-BPF context to release
> > *
> > */
> > -noinline void
> > +__bpf_kfunc void
> > hid_bpf_release_context(struct hid_bpf_ctx *ctx)
> > {
> > struct hid_bpf_ctx_kern *ctx_kern;
> > @@ -380,7 +391,7 @@ hid_bpf_release_context(struct hid_bpf_ctx *ctx)
> > *
> > * @returns %0 on success, a negative error code otherwise.
> > */
> > -noinline int
> > +__bpf_kfunc int
> > hid_bpf_hw_request(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz,
> > enum hid_report_type rtype, enum hid_class_request reqtype)
> > {
> > @@ -448,6 +459,7 @@ hid_bpf_hw_request(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz,
> > kfree(dma_data);
> > return ret;
> > }
> > +__diag_pop(); /* missing prototype warnings */
> >
> > /* our HID-BPF entrypoints */
> > BTF_SET8_START(hid_bpf_fmodret_ids)
> > diff --git a/include/linux/hid_bpf.h b/include/linux/hid_bpf.h
> > index 840cd254172d..7118ac28d468 100644
> > --- a/include/linux/hid_bpf.h
> > +++ b/include/linux/hid_bpf.h
> > @@ -77,17 +77,6 @@ enum hid_bpf_attach_flags {
> > int hid_bpf_device_event(struct hid_bpf_ctx *ctx);
> > int hid_bpf_rdesc_fixup(struct hid_bpf_ctx *ctx);
> >
> > -/* Following functions are kfunc that we export to BPF programs */
> > -/* available everywhere in HID-BPF */
> > -__u8 *hid_bpf_get_data(struct hid_bpf_ctx *ctx, unsigned int offset, const size_t __sz);
> > -
> > -/* only available in syscall */
> > -int hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags);
> > -int hid_bpf_hw_request(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz,
> > - enum hid_report_type rtype, enum hid_class_request reqtype);
> > -struct hid_bpf_ctx *hid_bpf_allocate_context(unsigned int hid_id);
> > -void hid_bpf_release_context(struct hid_bpf_ctx *ctx);
> > -
> > /*
> > * Below is HID internal
> > */
> >
> > --
> > 2.43.0
> >
> >
>
^ permalink raw reply
* Re: [dtor-input:next 134/135] drivers/input/joystick/adc-joystick.c:194:10: error: implicit declaration of function 'min_array'
From: Dmitry Torokhov @ 2024-01-23 21:04 UTC (permalink / raw)
To: Chris Morgan; +Cc: kernel test robot, oe-kbuild-all, linux-input
In-Reply-To: <DM6PR06MB533707F89AA767C845DD3D68A5742@DM6PR06MB5337.namprd06.prod.outlook.com>
On Tue, Jan 23, 2024 at 10:17:25AM -0600, Chris Morgan wrote:
> On Sat, Jan 20, 2024 at 06:55:32AM +0800, kernel test robot wrote:
> > tree: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
> > head: 3af6e24a456437d323d1080bd254053f7af05234
> > commit: 6380a59c534ecab1462608a1f76490289a45a377 [134/135] Input: adc-joystick - handle inverted axes
> > config: i386-randconfig-011-20240120 (https://download.01.org/0day-ci/archive/20240120/202401200614.B4PnBzhk-lkp@intel.com/config)
> > compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240120/202401200614.B4PnBzhk-lkp@intel.com/reproduce)
> >
> > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > the same patch/commit), kindly add following tags
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Closes: https://lore.kernel.org/oe-kbuild-all/202401200614.B4PnBzhk-lkp@intel.com/
> >
>
> Assume this means we need to explicitly `#include <linux/minmax.h>`, right?
>
> Should I submit a new patch or a bugfix for this patch?
As I mentioned in another email I think usage of min/max_array() is too
clever here. How about we simply "swap(range[0], range[1])" when axis is
inverted?
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 2/2] HID: bpf: use __bpf_kfunc instead of noinline
From: Andrii Nakryiko @ 2024-01-23 19:57 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Jiri Kosina, Benjamin Tissoires, Dan Carpenter, Daniel Borkmann,
linux-input, linux-kernel, bpf
In-Reply-To: <20240123-b4-hid-bpf-fixes-v1-2-aa1fac734377@kernel.org>
On Tue, Jan 23, 2024 at 8:46 AM Benjamin Tissoires <bentiss@kernel.org> wrote:
>
> Follow the docs at Documentation/bpf/kfuncs.rst:
> - declare the function with `__bpf_kfunc`
> - disables missing prototype warnings, which allows to remove them from
> include/linux/hid-bpf.h
>
> Removing the prototypes is not an issue because we currently have to
> redeclare them when writing the BPF program. They will eventually be
> generated by bpftool directly AFAIU.
>
> Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
> ---
> drivers/hid/bpf/hid_bpf_dispatch.c | 22 +++++++++++++++++-----
> include/linux/hid_bpf.h | 11 -----------
> 2 files changed, 17 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c
> index 5111d1fef0d3..119e4f03df55 100644
> --- a/drivers/hid/bpf/hid_bpf_dispatch.c
> +++ b/drivers/hid/bpf/hid_bpf_dispatch.c
> @@ -143,6 +143,11 @@ u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, u8 *rdesc, unsigned int *s
> }
> EXPORT_SYMBOL_GPL(call_hid_bpf_rdesc_fixup);
>
> +/* Disables missing prototype warnings */
> +__diag_push();
> +__diag_ignore_all("-Wmissing-prototypes",
> + "Global kfuncs as their definitions will be in BTF");
> +
would it be possible to use __bpf_kfunc_start_defs() and
__bpf_kfunc_end_defs() macros instead of explicit __diag push/pop
pairs? It's defined in include/linux/btf.h
> /**
> * hid_bpf_get_data - Get the kernel memory pointer associated with the context @ctx
> *
> @@ -152,7 +157,7 @@ EXPORT_SYMBOL_GPL(call_hid_bpf_rdesc_fixup);
> *
> * @returns %NULL on error, an %__u8 memory pointer on success
> */
> -noinline __u8 *
> +__bpf_kfunc __u8 *
> hid_bpf_get_data(struct hid_bpf_ctx *ctx, unsigned int offset, const size_t rdwr_buf_size)
> {
> struct hid_bpf_ctx_kern *ctx_kern;
> @@ -167,6 +172,7 @@ hid_bpf_get_data(struct hid_bpf_ctx *ctx, unsigned int offset, const size_t rdwr
>
> return ctx_kern->data + offset;
> }
> +__diag_pop(); /* missing prototype warnings */
>
> /*
> * The following set contains all functions we agree BPF programs
> @@ -274,6 +280,11 @@ static int do_hid_bpf_attach_prog(struct hid_device *hdev, int prog_fd, struct b
> return fd;
> }
>
> +/* Disables missing prototype warnings */
> +__diag_push();
> +__diag_ignore_all("-Wmissing-prototypes",
> + "Global kfuncs as their definitions will be in BTF");
> +
> /**
> * hid_bpf_attach_prog - Attach the given @prog_fd to the given HID device
> *
> @@ -286,7 +297,7 @@ static int do_hid_bpf_attach_prog(struct hid_device *hdev, int prog_fd, struct b
> * is pinned to the BPF file system).
> */
> /* called from syscall */
> -noinline int
> +__bpf_kfunc int
> hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags)
> {
> struct hid_device *hdev;
> @@ -328,7 +339,7 @@ hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags)
> *
> * @returns A pointer to &struct hid_bpf_ctx on success, %NULL on error.
> */
> -noinline struct hid_bpf_ctx *
> +__bpf_kfunc struct hid_bpf_ctx *
> hid_bpf_allocate_context(unsigned int hid_id)
> {
> struct hid_device *hdev;
> @@ -359,7 +370,7 @@ hid_bpf_allocate_context(unsigned int hid_id)
> * @ctx: the HID-BPF context to release
> *
> */
> -noinline void
> +__bpf_kfunc void
> hid_bpf_release_context(struct hid_bpf_ctx *ctx)
> {
> struct hid_bpf_ctx_kern *ctx_kern;
> @@ -380,7 +391,7 @@ hid_bpf_release_context(struct hid_bpf_ctx *ctx)
> *
> * @returns %0 on success, a negative error code otherwise.
> */
> -noinline int
> +__bpf_kfunc int
> hid_bpf_hw_request(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz,
> enum hid_report_type rtype, enum hid_class_request reqtype)
> {
> @@ -448,6 +459,7 @@ hid_bpf_hw_request(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz,
> kfree(dma_data);
> return ret;
> }
> +__diag_pop(); /* missing prototype warnings */
>
> /* our HID-BPF entrypoints */
> BTF_SET8_START(hid_bpf_fmodret_ids)
> diff --git a/include/linux/hid_bpf.h b/include/linux/hid_bpf.h
> index 840cd254172d..7118ac28d468 100644
> --- a/include/linux/hid_bpf.h
> +++ b/include/linux/hid_bpf.h
> @@ -77,17 +77,6 @@ enum hid_bpf_attach_flags {
> int hid_bpf_device_event(struct hid_bpf_ctx *ctx);
> int hid_bpf_rdesc_fixup(struct hid_bpf_ctx *ctx);
>
> -/* Following functions are kfunc that we export to BPF programs */
> -/* available everywhere in HID-BPF */
> -__u8 *hid_bpf_get_data(struct hid_bpf_ctx *ctx, unsigned int offset, const size_t __sz);
> -
> -/* only available in syscall */
> -int hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags);
> -int hid_bpf_hw_request(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz,
> - enum hid_report_type rtype, enum hid_class_request reqtype);
> -struct hid_bpf_ctx *hid_bpf_allocate_context(unsigned int hid_id);
> -void hid_bpf_release_context(struct hid_bpf_ctx *ctx);
> -
> /*
> * Below is HID internal
> */
>
> --
> 2.43.0
>
>
^ permalink raw reply
* [dtor-input:next] BUILD SUCCESS ab30e1a93c07bab9374fac83fdde9bf5794da2af
From: kernel test robot @ 2024-01-23 19:41 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
branch HEAD: ab30e1a93c07bab9374fac83fdde9bf5794da2af Input: 88pm80x_onkey - add SPDX and drop GPL boilerplate
elapsed time: 1457m
configs tested: 176
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
arc allmodconfig gcc
arc allnoconfig gcc
arc allyesconfig gcc
arc defconfig gcc
arc randconfig-001-20240123 gcc
arc randconfig-002-20240123 gcc
arc vdk_hs38_defconfig gcc
arm allmodconfig gcc
arm allnoconfig gcc
arm allyesconfig gcc
arm defconfig clang
arm dove_defconfig clang
arm exynos_defconfig gcc
arm randconfig-001-20240123 gcc
arm randconfig-002-20240123 gcc
arm randconfig-003-20240123 gcc
arm randconfig-004-20240123 gcc
arm shmobile_defconfig gcc
arm64 allmodconfig clang
arm64 allnoconfig gcc
arm64 defconfig gcc
arm64 randconfig-001-20240123 gcc
arm64 randconfig-002-20240123 gcc
arm64 randconfig-003-20240123 gcc
arm64 randconfig-004-20240123 gcc
csky allmodconfig gcc
csky allnoconfig gcc
csky allyesconfig gcc
csky defconfig gcc
csky randconfig-001-20240123 gcc
csky randconfig-002-20240123 gcc
hexagon allmodconfig clang
hexagon allnoconfig clang
hexagon allyesconfig clang
hexagon defconfig clang
hexagon randconfig-001-20240123 clang
hexagon randconfig-002-20240123 clang
i386 allmodconfig clang
i386 allnoconfig clang
i386 allyesconfig clang
i386 buildonly-randconfig-001-20240123 gcc
i386 buildonly-randconfig-002-20240123 gcc
i386 buildonly-randconfig-003-20240123 gcc
i386 buildonly-randconfig-004-20240123 gcc
i386 buildonly-randconfig-005-20240123 gcc
i386 buildonly-randconfig-006-20240123 gcc
i386 defconfig gcc
i386 randconfig-001-20240123 gcc
i386 randconfig-002-20240123 gcc
i386 randconfig-003-20240123 gcc
i386 randconfig-004-20240123 gcc
i386 randconfig-005-20240123 gcc
i386 randconfig-006-20240123 gcc
i386 randconfig-011-20240123 clang
i386 randconfig-012-20240123 clang
i386 randconfig-013-20240123 clang
i386 randconfig-014-20240123 clang
i386 randconfig-015-20240123 clang
i386 randconfig-016-20240123 clang
loongarch allmodconfig gcc
loongarch allnoconfig gcc
loongarch defconfig gcc
loongarch loongson3_defconfig gcc
loongarch randconfig-001-20240123 gcc
loongarch randconfig-002-20240123 gcc
m68k allmodconfig gcc
m68k allnoconfig gcc
m68k allyesconfig gcc
m68k amiga_defconfig gcc
m68k atari_defconfig gcc
m68k defconfig gcc
m68k hp300_defconfig gcc
m68k m5475evb_defconfig gcc
m68k mvme147_defconfig gcc
m68k mvme16x_defconfig gcc
m68k sun3x_defconfig gcc
microblaze allmodconfig gcc
microblaze allnoconfig gcc
microblaze allyesconfig gcc
microblaze defconfig gcc
mips allnoconfig clang
mips allyesconfig gcc
mips bmips_be_defconfig gcc
mips cu1000-neo_defconfig clang
mips loongson2k_defconfig gcc
mips rt305x_defconfig gcc
nios2 allmodconfig gcc
nios2 allnoconfig gcc
nios2 allyesconfig gcc
nios2 defconfig gcc
nios2 randconfig-001-20240123 gcc
nios2 randconfig-002-20240123 gcc
openrisc allnoconfig gcc
openrisc allyesconfig gcc
openrisc defconfig gcc
parisc allmodconfig gcc
parisc allnoconfig gcc
parisc allyesconfig gcc
parisc defconfig gcc
parisc randconfig-001-20240123 gcc
parisc randconfig-002-20240123 gcc
parisc64 defconfig gcc
powerpc allmodconfig clang
powerpc allnoconfig gcc
powerpc allyesconfig clang
powerpc canyonlands_defconfig gcc
powerpc mpc5200_defconfig clang
powerpc mpc885_ads_defconfig clang
powerpc pasemi_defconfig gcc
powerpc randconfig-001-20240123 gcc
powerpc randconfig-002-20240123 gcc
powerpc randconfig-003-20240123 gcc
powerpc64 randconfig-001-20240123 gcc
powerpc64 randconfig-002-20240123 gcc
powerpc64 randconfig-003-20240123 gcc
riscv allmodconfig gcc
riscv allnoconfig clang
riscv allyesconfig gcc
riscv defconfig gcc
riscv randconfig-001-20240123 gcc
riscv randconfig-002-20240123 gcc
riscv rv32_defconfig clang
s390 allmodconfig gcc
s390 allnoconfig gcc
s390 allyesconfig gcc
s390 defconfig gcc
s390 randconfig-001-20240123 clang
s390 randconfig-002-20240123 clang
sh allmodconfig gcc
sh allnoconfig gcc
sh allyesconfig gcc
sh defconfig gcc
sh dreamcast_defconfig gcc
sh kfr2r09_defconfig gcc
sh r7780mp_defconfig gcc
sh randconfig-001-20240123 gcc
sh randconfig-002-20240123 gcc
sh se7724_defconfig gcc
sh sh7770_generic_defconfig gcc
sparc allmodconfig gcc
sparc64 allmodconfig gcc
sparc64 allyesconfig gcc
sparc64 defconfig gcc
sparc64 randconfig-001-20240123 gcc
sparc64 randconfig-002-20240123 gcc
um allmodconfig clang
um allnoconfig clang
um allyesconfig clang
um defconfig gcc
um i386_defconfig gcc
um randconfig-001-20240123 gcc
um randconfig-002-20240123 gcc
um x86_64_defconfig gcc
x86_64 allnoconfig gcc
x86_64 allyesconfig clang
x86_64 buildonly-randconfig-001-20240124 clang
x86_64 buildonly-randconfig-002-20240124 clang
x86_64 buildonly-randconfig-003-20240124 clang
x86_64 buildonly-randconfig-004-20240124 clang
x86_64 buildonly-randconfig-005-20240124 clang
x86_64 buildonly-randconfig-006-20240124 clang
x86_64 defconfig gcc
x86_64 randconfig-001-20240124 gcc
x86_64 randconfig-002-20240124 gcc
x86_64 randconfig-003-20240124 gcc
x86_64 randconfig-004-20240124 gcc
x86_64 randconfig-005-20240124 gcc
x86_64 randconfig-006-20240124 gcc
x86_64 randconfig-011-20240124 clang
x86_64 rhel-8.3-rust clang
xtensa allnoconfig gcc
xtensa randconfig-001-20240123 gcc
xtensa randconfig-002-20240123 gcc
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH 1/2] HID: bpf: remove double fdget()
From: Benjamin Tissoires @ 2024-01-23 17:19 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Jiri Kosina, Dan Carpenter, Daniel Borkmann, linux-input,
linux-kernel, bpf, stable
In-Reply-To: <20240123-b4-hid-bpf-fixes-v1-1-aa1fac734377@kernel.org>
On Tue, Jan 23, 2024 at 5:41 PM Benjamin Tissoires <bentiss@kernel.org> wrote:
>
> When the kfunc hid_bpf_attach_prog() is called, we called twice fdget():
> one for fetching the type of the bpf program, and one for actually
> attaching the program to the device.
>
> The problem is that between those two calls, we have no guarantees that
> the prog_fd is still the same file descriptor for the given program.
>
> Solve this by calling bpf_prog_get() earlier, and use this to fetch the
> program type.
>
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Link: https://lore.kernel.org/bpf/CAO-hwJJ8vh8JD3-P43L-_CLNmPx0hWj44aom0O838vfP4=_1CA@mail.gmail.com/T/#t
> Cc: stable@vger.kernel.org
Sigh, I forgot:
Fixes: f5c27da4e3c8 ("HID: initial BPF implementation")
Cheers,
Benjamin
> Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
> ---
> drivers/hid/bpf/hid_bpf_dispatch.c | 66 ++++++++++++++++++++++++-------------
> drivers/hid/bpf/hid_bpf_dispatch.h | 4 +--
> drivers/hid/bpf/hid_bpf_jmp_table.c | 20 ++---------
> 3 files changed, 49 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c
> index d9ef45fcaeab..5111d1fef0d3 100644
> --- a/drivers/hid/bpf/hid_bpf_dispatch.c
> +++ b/drivers/hid/bpf/hid_bpf_dispatch.c
> @@ -241,6 +241,39 @@ int hid_bpf_reconnect(struct hid_device *hdev)
> return 0;
> }
>
> +static int do_hid_bpf_attach_prog(struct hid_device *hdev, int prog_fd, struct bpf_prog *prog,
> + __u32 flags)
> +{
> + int fd, err, prog_type;
> +
> + prog_type = hid_bpf_get_prog_attach_type(prog);
> + if (prog_type < 0)
> + return prog_type;
> +
> + if (prog_type >= HID_BPF_PROG_TYPE_MAX)
> + return -EINVAL;
> +
> + if (prog_type == HID_BPF_PROG_TYPE_DEVICE_EVENT) {
> + err = hid_bpf_allocate_event_data(hdev);
> + if (err)
> + return err;
> + }
> +
> + fd = __hid_bpf_attach_prog(hdev, prog_type, prog_fd, prog, flags);
> + if (fd < 0)
> + return fd;
> +
> + if (prog_type == HID_BPF_PROG_TYPE_RDESC_FIXUP) {
> + err = hid_bpf_reconnect(hdev);
> + if (err) {
> + close_fd(fd);
> + return err;
> + }
> + }
> +
> + return fd;
> +}
> +
> /**
> * hid_bpf_attach_prog - Attach the given @prog_fd to the given HID device
> *
> @@ -257,18 +290,13 @@ noinline int
> hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags)
> {
> struct hid_device *hdev;
> + struct bpf_prog *prog;
> struct device *dev;
> - int fd, err, prog_type = hid_bpf_get_prog_attach_type(prog_fd);
> + int fd;
>
> if (!hid_bpf_ops)
> return -EINVAL;
>
> - if (prog_type < 0)
> - return prog_type;
> -
> - if (prog_type >= HID_BPF_PROG_TYPE_MAX)
> - return -EINVAL;
> -
> if ((flags & ~HID_BPF_FLAG_MASK))
> return -EINVAL;
>
> @@ -278,23 +306,17 @@ hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags)
>
> hdev = to_hid_device(dev);
>
> - if (prog_type == HID_BPF_PROG_TYPE_DEVICE_EVENT) {
> - err = hid_bpf_allocate_event_data(hdev);
> - if (err)
> - return err;
> - }
> + /*
> + * take a ref on the prog itself, it will be released
> + * on errors or when it'll be detached
> + */
> + prog = bpf_prog_get(prog_fd);
> + if (IS_ERR(prog))
> + return PTR_ERR(prog);
>
> - fd = __hid_bpf_attach_prog(hdev, prog_type, prog_fd, flags);
> + fd = do_hid_bpf_attach_prog(hdev, prog_fd, prog, flags);
> if (fd < 0)
> - return fd;
> -
> - if (prog_type == HID_BPF_PROG_TYPE_RDESC_FIXUP) {
> - err = hid_bpf_reconnect(hdev);
> - if (err) {
> - close_fd(fd);
> - return err;
> - }
> - }
> + bpf_prog_put(prog);
>
> return fd;
> }
> diff --git a/drivers/hid/bpf/hid_bpf_dispatch.h b/drivers/hid/bpf/hid_bpf_dispatch.h
> index 63dfc8605cd2..fbe0639d09f2 100644
> --- a/drivers/hid/bpf/hid_bpf_dispatch.h
> +++ b/drivers/hid/bpf/hid_bpf_dispatch.h
> @@ -12,9 +12,9 @@ struct hid_bpf_ctx_kern {
>
> int hid_bpf_preload_skel(void);
> void hid_bpf_free_links_and_skel(void);
> -int hid_bpf_get_prog_attach_type(int prog_fd);
> +int hid_bpf_get_prog_attach_type(struct bpf_prog *prog);
> int __hid_bpf_attach_prog(struct hid_device *hdev, enum hid_bpf_prog_type prog_type, int prog_fd,
> - __u32 flags);
> + struct bpf_prog *prog, __u32 flags);
> void __hid_bpf_destroy_device(struct hid_device *hdev);
> int hid_bpf_prog_run(struct hid_device *hdev, enum hid_bpf_prog_type type,
> struct hid_bpf_ctx_kern *ctx_kern);
> diff --git a/drivers/hid/bpf/hid_bpf_jmp_table.c b/drivers/hid/bpf/hid_bpf_jmp_table.c
> index eca34b7372f9..12f7cebddd73 100644
> --- a/drivers/hid/bpf/hid_bpf_jmp_table.c
> +++ b/drivers/hid/bpf/hid_bpf_jmp_table.c
> @@ -333,15 +333,10 @@ static int hid_bpf_insert_prog(int prog_fd, struct bpf_prog *prog)
> return err;
> }
>
> -int hid_bpf_get_prog_attach_type(int prog_fd)
> +int hid_bpf_get_prog_attach_type(struct bpf_prog *prog)
> {
> - struct bpf_prog *prog = NULL;
> - int i;
> int prog_type = HID_BPF_PROG_TYPE_UNDEF;
> -
> - prog = bpf_prog_get(prog_fd);
> - if (IS_ERR(prog))
> - return PTR_ERR(prog);
> + int i;
>
> for (i = 0; i < HID_BPF_PROG_TYPE_MAX; i++) {
> if (hid_bpf_btf_ids[i] == prog->aux->attach_btf_id) {
> @@ -350,8 +345,6 @@ int hid_bpf_get_prog_attach_type(int prog_fd)
> }
> }
>
> - bpf_prog_put(prog);
> -
> return prog_type;
> }
>
> @@ -388,19 +381,13 @@ static const struct bpf_link_ops hid_bpf_link_lops = {
> /* called from syscall */
> noinline int
> __hid_bpf_attach_prog(struct hid_device *hdev, enum hid_bpf_prog_type prog_type,
> - int prog_fd, __u32 flags)
> + int prog_fd, struct bpf_prog *prog, __u32 flags)
> {
> struct bpf_link_primer link_primer;
> struct hid_bpf_link *link;
> - struct bpf_prog *prog = NULL;
> struct hid_bpf_prog_entry *prog_entry;
> int cnt, err = -EINVAL, prog_table_idx = -1;
>
> - /* take a ref on the prog itself */
> - prog = bpf_prog_get(prog_fd);
> - if (IS_ERR(prog))
> - return PTR_ERR(prog);
> -
> mutex_lock(&hid_bpf_attach_lock);
>
> link = kzalloc(sizeof(*link), GFP_USER);
> @@ -467,7 +454,6 @@ __hid_bpf_attach_prog(struct hid_device *hdev, enum hid_bpf_prog_type prog_type,
> err_unlock:
> mutex_unlock(&hid_bpf_attach_lock);
>
> - bpf_prog_put(prog);
> kfree(link);
>
> return err;
>
> --
> 2.43.0
>
^ permalink raw reply
* Re: Implement per-key keyboard backlight as auxdisplay?
From: Werner Sembach @ 2024-01-23 16:51 UTC (permalink / raw)
To: Pavel Machek, Dmitry Torokhov
Cc: Jani Nikula, Hans de Goede, jikos, Jelle van der Waa,
Miguel Ojeda, Lee Jones, linux-kernel,
dri-devel@lists.freedesktop.org, linux-input, ojeda, linux-leds,
Benjamin Tissoires
In-Reply-To: <Zar0MFelV4gY50La@duo.ucw.cz>
Am 19.01.24 um 23:14 schrieb Pavel Machek:
> Hi!
>
>>> And while I would personally hate it, you can imagine a use case where
>>> you'd like a keypress to have a visual effect around the key you
>>> pressed. A kind of force feedback, if you will. I don't actually know,
>>> and correct me if I'm wrong, but feels like implementing that outside of
>>> the input subsystem would be non-trivial.
>> Actually I think it does not belong to the input subsystem as it is,
>> where the goal is to deliver keystrokes and gestures to userspace. The
>> "force feedback" kind of fits, but not really practical, again because
>> of lack of geometry info. It is also not really essential to be fully
>> and automatically handled by the kernel. So I think the best way is
>>> to
> So that's actually big question.
>
> If the common usage is "run bad apple demo on keyboard" than pretty
> clearly it should be display.
>
> If the common usage is "computer is asking yes/no question, so
> highlight yes and no buttons", then there are good arguments why input
> should handle that (as it does capslock led, for example).
The common usage is "make keyboard look flashy", for some a fixed color scheme
is enough, other ones might probably enable one of the built in modes. Most
people I think will be satisfied with these 2 options, albeit both of your
suggestions sound cool.
>
> Actually I could imagine "real" use when shift / control /alt
> backlight would indicate sticky-shift keys for handicapped.
>
> It seems they are making mice with backlit buttons. If the main use is
> highlight this key whereever it is, then it should be input.
>
> But I suspect may use is just fancy colors and it should be display.
>
> Best regards,
> Pavel
^ permalink raw reply
* [PATCH 2/2] HID: bpf: use __bpf_kfunc instead of noinline
From: Benjamin Tissoires @ 2024-01-23 16:40 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Dan Carpenter, Daniel Borkmann
Cc: linux-input, linux-kernel, bpf, Benjamin Tissoires
In-Reply-To: <20240123-b4-hid-bpf-fixes-v1-0-aa1fac734377@kernel.org>
Follow the docs at Documentation/bpf/kfuncs.rst:
- declare the function with `__bpf_kfunc`
- disables missing prototype warnings, which allows to remove them from
include/linux/hid-bpf.h
Removing the prototypes is not an issue because we currently have to
redeclare them when writing the BPF program. They will eventually be
generated by bpftool directly AFAIU.
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
drivers/hid/bpf/hid_bpf_dispatch.c | 22 +++++++++++++++++-----
include/linux/hid_bpf.h | 11 -----------
2 files changed, 17 insertions(+), 16 deletions(-)
diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c
index 5111d1fef0d3..119e4f03df55 100644
--- a/drivers/hid/bpf/hid_bpf_dispatch.c
+++ b/drivers/hid/bpf/hid_bpf_dispatch.c
@@ -143,6 +143,11 @@ u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, u8 *rdesc, unsigned int *s
}
EXPORT_SYMBOL_GPL(call_hid_bpf_rdesc_fixup);
+/* Disables missing prototype warnings */
+__diag_push();
+__diag_ignore_all("-Wmissing-prototypes",
+ "Global kfuncs as their definitions will be in BTF");
+
/**
* hid_bpf_get_data - Get the kernel memory pointer associated with the context @ctx
*
@@ -152,7 +157,7 @@ EXPORT_SYMBOL_GPL(call_hid_bpf_rdesc_fixup);
*
* @returns %NULL on error, an %__u8 memory pointer on success
*/
-noinline __u8 *
+__bpf_kfunc __u8 *
hid_bpf_get_data(struct hid_bpf_ctx *ctx, unsigned int offset, const size_t rdwr_buf_size)
{
struct hid_bpf_ctx_kern *ctx_kern;
@@ -167,6 +172,7 @@ hid_bpf_get_data(struct hid_bpf_ctx *ctx, unsigned int offset, const size_t rdwr
return ctx_kern->data + offset;
}
+__diag_pop(); /* missing prototype warnings */
/*
* The following set contains all functions we agree BPF programs
@@ -274,6 +280,11 @@ static int do_hid_bpf_attach_prog(struct hid_device *hdev, int prog_fd, struct b
return fd;
}
+/* Disables missing prototype warnings */
+__diag_push();
+__diag_ignore_all("-Wmissing-prototypes",
+ "Global kfuncs as their definitions will be in BTF");
+
/**
* hid_bpf_attach_prog - Attach the given @prog_fd to the given HID device
*
@@ -286,7 +297,7 @@ static int do_hid_bpf_attach_prog(struct hid_device *hdev, int prog_fd, struct b
* is pinned to the BPF file system).
*/
/* called from syscall */
-noinline int
+__bpf_kfunc int
hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags)
{
struct hid_device *hdev;
@@ -328,7 +339,7 @@ hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags)
*
* @returns A pointer to &struct hid_bpf_ctx on success, %NULL on error.
*/
-noinline struct hid_bpf_ctx *
+__bpf_kfunc struct hid_bpf_ctx *
hid_bpf_allocate_context(unsigned int hid_id)
{
struct hid_device *hdev;
@@ -359,7 +370,7 @@ hid_bpf_allocate_context(unsigned int hid_id)
* @ctx: the HID-BPF context to release
*
*/
-noinline void
+__bpf_kfunc void
hid_bpf_release_context(struct hid_bpf_ctx *ctx)
{
struct hid_bpf_ctx_kern *ctx_kern;
@@ -380,7 +391,7 @@ hid_bpf_release_context(struct hid_bpf_ctx *ctx)
*
* @returns %0 on success, a negative error code otherwise.
*/
-noinline int
+__bpf_kfunc int
hid_bpf_hw_request(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz,
enum hid_report_type rtype, enum hid_class_request reqtype)
{
@@ -448,6 +459,7 @@ hid_bpf_hw_request(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz,
kfree(dma_data);
return ret;
}
+__diag_pop(); /* missing prototype warnings */
/* our HID-BPF entrypoints */
BTF_SET8_START(hid_bpf_fmodret_ids)
diff --git a/include/linux/hid_bpf.h b/include/linux/hid_bpf.h
index 840cd254172d..7118ac28d468 100644
--- a/include/linux/hid_bpf.h
+++ b/include/linux/hid_bpf.h
@@ -77,17 +77,6 @@ enum hid_bpf_attach_flags {
int hid_bpf_device_event(struct hid_bpf_ctx *ctx);
int hid_bpf_rdesc_fixup(struct hid_bpf_ctx *ctx);
-/* Following functions are kfunc that we export to BPF programs */
-/* available everywhere in HID-BPF */
-__u8 *hid_bpf_get_data(struct hid_bpf_ctx *ctx, unsigned int offset, const size_t __sz);
-
-/* only available in syscall */
-int hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags);
-int hid_bpf_hw_request(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz,
- enum hid_report_type rtype, enum hid_class_request reqtype);
-struct hid_bpf_ctx *hid_bpf_allocate_context(unsigned int hid_id);
-void hid_bpf_release_context(struct hid_bpf_ctx *ctx);
-
/*
* Below is HID internal
*/
--
2.43.0
^ permalink raw reply related
* [PATCH 1/2] HID: bpf: remove double fdget()
From: Benjamin Tissoires @ 2024-01-23 16:40 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Dan Carpenter, Daniel Borkmann
Cc: linux-input, linux-kernel, bpf, Benjamin Tissoires, stable
In-Reply-To: <20240123-b4-hid-bpf-fixes-v1-0-aa1fac734377@kernel.org>
When the kfunc hid_bpf_attach_prog() is called, we called twice fdget():
one for fetching the type of the bpf program, and one for actually
attaching the program to the device.
The problem is that between those two calls, we have no guarantees that
the prog_fd is still the same file descriptor for the given program.
Solve this by calling bpf_prog_get() earlier, and use this to fetch the
program type.
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Link: https://lore.kernel.org/bpf/CAO-hwJJ8vh8JD3-P43L-_CLNmPx0hWj44aom0O838vfP4=_1CA@mail.gmail.com/T/#t
Cc: stable@vger.kernel.org
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
drivers/hid/bpf/hid_bpf_dispatch.c | 66 ++++++++++++++++++++++++-------------
drivers/hid/bpf/hid_bpf_dispatch.h | 4 +--
drivers/hid/bpf/hid_bpf_jmp_table.c | 20 ++---------
3 files changed, 49 insertions(+), 41 deletions(-)
diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c
index d9ef45fcaeab..5111d1fef0d3 100644
--- a/drivers/hid/bpf/hid_bpf_dispatch.c
+++ b/drivers/hid/bpf/hid_bpf_dispatch.c
@@ -241,6 +241,39 @@ int hid_bpf_reconnect(struct hid_device *hdev)
return 0;
}
+static int do_hid_bpf_attach_prog(struct hid_device *hdev, int prog_fd, struct bpf_prog *prog,
+ __u32 flags)
+{
+ int fd, err, prog_type;
+
+ prog_type = hid_bpf_get_prog_attach_type(prog);
+ if (prog_type < 0)
+ return prog_type;
+
+ if (prog_type >= HID_BPF_PROG_TYPE_MAX)
+ return -EINVAL;
+
+ if (prog_type == HID_BPF_PROG_TYPE_DEVICE_EVENT) {
+ err = hid_bpf_allocate_event_data(hdev);
+ if (err)
+ return err;
+ }
+
+ fd = __hid_bpf_attach_prog(hdev, prog_type, prog_fd, prog, flags);
+ if (fd < 0)
+ return fd;
+
+ if (prog_type == HID_BPF_PROG_TYPE_RDESC_FIXUP) {
+ err = hid_bpf_reconnect(hdev);
+ if (err) {
+ close_fd(fd);
+ return err;
+ }
+ }
+
+ return fd;
+}
+
/**
* hid_bpf_attach_prog - Attach the given @prog_fd to the given HID device
*
@@ -257,18 +290,13 @@ noinline int
hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags)
{
struct hid_device *hdev;
+ struct bpf_prog *prog;
struct device *dev;
- int fd, err, prog_type = hid_bpf_get_prog_attach_type(prog_fd);
+ int fd;
if (!hid_bpf_ops)
return -EINVAL;
- if (prog_type < 0)
- return prog_type;
-
- if (prog_type >= HID_BPF_PROG_TYPE_MAX)
- return -EINVAL;
-
if ((flags & ~HID_BPF_FLAG_MASK))
return -EINVAL;
@@ -278,23 +306,17 @@ hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags)
hdev = to_hid_device(dev);
- if (prog_type == HID_BPF_PROG_TYPE_DEVICE_EVENT) {
- err = hid_bpf_allocate_event_data(hdev);
- if (err)
- return err;
- }
+ /*
+ * take a ref on the prog itself, it will be released
+ * on errors or when it'll be detached
+ */
+ prog = bpf_prog_get(prog_fd);
+ if (IS_ERR(prog))
+ return PTR_ERR(prog);
- fd = __hid_bpf_attach_prog(hdev, prog_type, prog_fd, flags);
+ fd = do_hid_bpf_attach_prog(hdev, prog_fd, prog, flags);
if (fd < 0)
- return fd;
-
- if (prog_type == HID_BPF_PROG_TYPE_RDESC_FIXUP) {
- err = hid_bpf_reconnect(hdev);
- if (err) {
- close_fd(fd);
- return err;
- }
- }
+ bpf_prog_put(prog);
return fd;
}
diff --git a/drivers/hid/bpf/hid_bpf_dispatch.h b/drivers/hid/bpf/hid_bpf_dispatch.h
index 63dfc8605cd2..fbe0639d09f2 100644
--- a/drivers/hid/bpf/hid_bpf_dispatch.h
+++ b/drivers/hid/bpf/hid_bpf_dispatch.h
@@ -12,9 +12,9 @@ struct hid_bpf_ctx_kern {
int hid_bpf_preload_skel(void);
void hid_bpf_free_links_and_skel(void);
-int hid_bpf_get_prog_attach_type(int prog_fd);
+int hid_bpf_get_prog_attach_type(struct bpf_prog *prog);
int __hid_bpf_attach_prog(struct hid_device *hdev, enum hid_bpf_prog_type prog_type, int prog_fd,
- __u32 flags);
+ struct bpf_prog *prog, __u32 flags);
void __hid_bpf_destroy_device(struct hid_device *hdev);
int hid_bpf_prog_run(struct hid_device *hdev, enum hid_bpf_prog_type type,
struct hid_bpf_ctx_kern *ctx_kern);
diff --git a/drivers/hid/bpf/hid_bpf_jmp_table.c b/drivers/hid/bpf/hid_bpf_jmp_table.c
index eca34b7372f9..12f7cebddd73 100644
--- a/drivers/hid/bpf/hid_bpf_jmp_table.c
+++ b/drivers/hid/bpf/hid_bpf_jmp_table.c
@@ -333,15 +333,10 @@ static int hid_bpf_insert_prog(int prog_fd, struct bpf_prog *prog)
return err;
}
-int hid_bpf_get_prog_attach_type(int prog_fd)
+int hid_bpf_get_prog_attach_type(struct bpf_prog *prog)
{
- struct bpf_prog *prog = NULL;
- int i;
int prog_type = HID_BPF_PROG_TYPE_UNDEF;
-
- prog = bpf_prog_get(prog_fd);
- if (IS_ERR(prog))
- return PTR_ERR(prog);
+ int i;
for (i = 0; i < HID_BPF_PROG_TYPE_MAX; i++) {
if (hid_bpf_btf_ids[i] == prog->aux->attach_btf_id) {
@@ -350,8 +345,6 @@ int hid_bpf_get_prog_attach_type(int prog_fd)
}
}
- bpf_prog_put(prog);
-
return prog_type;
}
@@ -388,19 +381,13 @@ static const struct bpf_link_ops hid_bpf_link_lops = {
/* called from syscall */
noinline int
__hid_bpf_attach_prog(struct hid_device *hdev, enum hid_bpf_prog_type prog_type,
- int prog_fd, __u32 flags)
+ int prog_fd, struct bpf_prog *prog, __u32 flags)
{
struct bpf_link_primer link_primer;
struct hid_bpf_link *link;
- struct bpf_prog *prog = NULL;
struct hid_bpf_prog_entry *prog_entry;
int cnt, err = -EINVAL, prog_table_idx = -1;
- /* take a ref on the prog itself */
- prog = bpf_prog_get(prog_fd);
- if (IS_ERR(prog))
- return PTR_ERR(prog);
-
mutex_lock(&hid_bpf_attach_lock);
link = kzalloc(sizeof(*link), GFP_USER);
@@ -467,7 +454,6 @@ __hid_bpf_attach_prog(struct hid_device *hdev, enum hid_bpf_prog_type prog_type,
err_unlock:
mutex_unlock(&hid_bpf_attach_lock);
- bpf_prog_put(prog);
kfree(link);
return err;
--
2.43.0
^ permalink raw reply related
* [PATCH 0/2] HID: bpf: couple of upstream fixes
From: Benjamin Tissoires @ 2024-01-23 16:40 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Dan Carpenter, Daniel Borkmann
Cc: linux-input, linux-kernel, bpf, Benjamin Tissoires, stable
Hi,
these are a couple of fixes for hid-bpf. The first one should
probably go in ASAP, after the reviews, and the second one is nice
to have and doesn't hurt much.
Thanks Dan for finding out the issue with bpf_prog_get()
Cheers,
Benjamin
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
Benjamin Tissoires (2):
HID: bpf: remove double fdget()
HID: bpf: use __bpf_kfunc instead of noinline
drivers/hid/bpf/hid_bpf_dispatch.c | 88 +++++++++++++++++++++++++------------
drivers/hid/bpf/hid_bpf_dispatch.h | 4 +-
drivers/hid/bpf/hid_bpf_jmp_table.c | 20 ++-------
include/linux/hid_bpf.h | 11 -----
4 files changed, 66 insertions(+), 57 deletions(-)
---
base-commit: fef018d8199661962b5fc0f0d1501caa54b2b533
change-id: 20240123-b4-hid-bpf-fixes-662908fe2234
Best regards,
--
Benjamin Tissoires <bentiss@kernel.org>
^ permalink raw reply
* Re: [dtor-input:next 134/135] drivers/input/joystick/adc-joystick.c:194:10: error: implicit declaration of function 'min_array'
From: Chris Morgan @ 2024-01-23 16:17 UTC (permalink / raw)
To: kernel test robot; +Cc: oe-kbuild-all, linux-input, Dmitry Torokhov
In-Reply-To: <202401200614.B4PnBzhk-lkp@intel.com>
On Sat, Jan 20, 2024 at 06:55:32AM +0800, kernel test robot wrote:
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
> head: 3af6e24a456437d323d1080bd254053f7af05234
> commit: 6380a59c534ecab1462608a1f76490289a45a377 [134/135] Input: adc-joystick - handle inverted axes
> config: i386-randconfig-011-20240120 (https://download.01.org/0day-ci/archive/20240120/202401200614.B4PnBzhk-lkp@intel.com/config)
> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240120/202401200614.B4PnBzhk-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202401200614.B4PnBzhk-lkp@intel.com/
>
Assume this means we need to explicitly `#include <linux/minmax.h>`, right?
Should I submit a new patch or a bugfix for this patch?
Thank you.
> All errors (new ones prefixed by >>):
>
> drivers/input/joystick/adc-joystick.c: In function 'adc_joystick_set_axes':
> >> drivers/input/joystick/adc-joystick.c:194:10: error: implicit declaration of function 'min_array' [-Werror=implicit-function-declaration]
> 194 | min_array(axes[i].range, 2),
> | ^~~~~~~~~
> >> drivers/input/joystick/adc-joystick.c:195:10: error: implicit declaration of function 'max_array'; did you mean 'kmalloc_array'? [-Werror=implicit-function-declaration]
> 195 | max_array(axes[i].range, 2),
> | ^~~~~~~~~
> | kmalloc_array
> cc1: some warnings being treated as errors
>
>
> vim +/min_array +194 drivers/input/joystick/adc-joystick.c
>
> 135
> 136 static int adc_joystick_set_axes(struct device *dev, struct adc_joystick *joy)
> 137 {
> 138 struct adc_joystick_axis *axes;
> 139 struct fwnode_handle *child;
> 140 int num_axes, error, i;
> 141
> 142 num_axes = device_get_child_node_count(dev);
> 143 if (!num_axes) {
> 144 dev_err(dev, "Unable to find child nodes\n");
> 145 return -EINVAL;
> 146 }
> 147
> 148 if (num_axes != joy->num_chans) {
> 149 dev_err(dev, "Got %d child nodes for %d channels\n",
> 150 num_axes, joy->num_chans);
> 151 return -EINVAL;
> 152 }
> 153
> 154 axes = devm_kmalloc_array(dev, num_axes, sizeof(*axes), GFP_KERNEL);
> 155 if (!axes)
> 156 return -ENOMEM;
> 157
> 158 device_for_each_child_node(dev, child) {
> 159 error = fwnode_property_read_u32(child, "reg", &i);
> 160 if (error) {
> 161 dev_err(dev, "reg invalid or missing\n");
> 162 goto err_fwnode_put;
> 163 }
> 164
> 165 if (i >= num_axes) {
> 166 error = -EINVAL;
> 167 dev_err(dev, "No matching axis for reg %d\n", i);
> 168 goto err_fwnode_put;
> 169 }
> 170
> 171 error = fwnode_property_read_u32(child, "linux,code",
> 172 &axes[i].code);
> 173 if (error) {
> 174 dev_err(dev, "linux,code invalid or missing\n");
> 175 goto err_fwnode_put;
> 176 }
> 177
> 178 error = fwnode_property_read_u32_array(child, "abs-range",
> 179 axes[i].range, 2);
> 180 if (error) {
> 181 dev_err(dev, "abs-range invalid or missing\n");
> 182 goto err_fwnode_put;
> 183 }
> 184
> 185 if (axes[i].range[0] > axes[i].range[1]) {
> 186 dev_dbg(dev, "abs-axis %d inverted\n", i);
> 187 axes[i].inverted = true;
> 188 }
> 189
> 190 fwnode_property_read_u32(child, "abs-fuzz", &axes[i].fuzz);
> 191 fwnode_property_read_u32(child, "abs-flat", &axes[i].flat);
> 192
> 193 input_set_abs_params(joy->input, axes[i].code,
> > 194 min_array(axes[i].range, 2),
> > 195 max_array(axes[i].range, 2),
> 196 axes[i].fuzz, axes[i].flat);
> 197 input_set_capability(joy->input, EV_ABS, axes[i].code);
> 198 }
> 199
> 200 joy->axes = axes;
> 201
> 202 return 0;
> 203
> 204 err_fwnode_put:
> 205 fwnode_handle_put(child);
> 206 return error;
> 207 }
> 208
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Input: psmouse - enable Synaptics InterTouch for ThinkPad T490
From: Virgil @ 2024-01-23 14:09 UTC (permalink / raw)
To: linux-input
[-- Attachment #1: Type: text/plain, Size: 103 bytes --]
Please consider adding "LEN205B" to smbus_pnp_ids[].
Attached log with psmouse.synaptics_intertouch=1
[-- Attachment #2: t490.txt --]
[-- Type: text/plain, Size: 1079 bytes --]
janv. 21 18:31:27 arch kernel: thinkpad_acpi: Lenovo ThinkPad T490, model 20N2000TMX
janv. 21 18:31:27 arch kernel: psmouse serio1: synaptics: queried max coordinates: x [..5678], y [..4694]
janv. 21 18:31:27 arch kernel: psmouse serio1: synaptics: queried min coordinates: x [1266..], y [1162..]
janv. 21 18:31:27 arch kernel: psmouse serio1: synaptics: Trying to set up SMBus access
janv. 21 18:31:27 arch kernel: rmi4_smbus 7-002c: registering SMbus-connected sensor
janv. 21 18:31:28 arch kernel: rmi4_f01 rmi4-00.fn01: found RMI device, manufacturer: Synaptics, product: TM3471-020, fw id: 3972349
janv. 21 18:31:28 arch kernel: input: Synaptics TM3471-020 as /devices/pci0000:00/0000:00:1f.4/i2c-7/7-002c/rmi4-00/input/input14
janv. 21 18:31:28 arch kernel: serio: RMI4 PS/2 pass-through port at rmi4-00.fn03
janv. 21 18:31:28 arch kernel: psmouse serio2: trackpoint: Elan TrackPoint firmware: 0x11, buttons: 3/3
janv. 21 18:31:28 arch kernel: input: TPPS/2 Elan TrackPoint as /devices/pci0000:00/0000:00:1f.4/i2c-7/7-002c/rmi4-00/rmi4-00.fn03/serio2/input/input15
^ permalink raw reply
* Multitouch HID issue in Android 14
From: Lyuboslav Angelov @ 2024-01-23 12:12 UTC (permalink / raw)
To: linux-input
Dear kernel developers,
We are reaching out to you to discuss and hopefully resolve an
important issue we’ve encountered in the recent versions of the
Android kernel (for example 5.10.177-Android13, as reported on a Pixel
7 device), which affects millions of automotive multimedia devices
already in production.
The issue we’ve detected is related to the handling of the HID
descriptor by the kernel. According to our investigation the issue is
caused by a conflicting interpretation of the “Usage Page (Button)”
and the “Usage (Touch Screen)” feature items in the used HID
descriptor. The issue is noticed when a registered HID descriptor
contains the “Usage Page (Button)” feature item, which when parsed
later on in the kernel, leads to interpreting the device as a
mouse-like or trackpad device, ignoring the value of the “Usage (Touch
Screen)” field, and manifesting a mouse cursor. Previous versions of
the kernel correctly handle the value of the “Usage (Touch Screen)”
field and identify properly the attached touch screen device, hence
our request for fixing the issue in the future versions of the kernel
and for the affected devices already in the field.
This issue does not reproduce with а Samsung device with Android 14,
which reports kernel version 5.10.177-Android12.
HID Descriptor overview:
static const uint8_t c_descriptorArray[] = {
0x06, 0x0D, 0x00, // Usage Page (Digitizer)
0x0A, 0x04, 0x00, // Usage (Touch Screen)
0xA1, 0x01, // Collection (Application)
0x06, 0x0D, 0x00, // Usage Page (Digitizer)
0x0A, 0x22, 0x00, // Usage (Finger)
0xA1, 0x02, // Collection (Logical)
0x06, 0x01, 0x00, // Usage Page (Generic Desktop Ctrls)
0x0A, 0x01, 0x00, // Usage (Pointer)
0xA1, 0x00, // Collection (Physical)
0x06, 0x0D, 0x00, // Usage Page (Digitizer)
0x0A, 0x51, 0x00, // Usage (Contact ID)
0x16, 0x00, 0x00, // Logical Minimum (0)
0x26, 0x00, 0x09, // Logical Maximum (9)
0x95, 0x01, // Report Count (1)
0x75, 0x08, // Report Size (8)
0x82, 0x02, 0x00, // Input (Data,Var,Abs,No Wrap,Linear,Preferred
State,No Null Position,Bit Field)
0x06, 0x0D, 0x00, // Usage Page (Digitizer)
0x0A, 0x42, 0x00, // Usage (Tip Switch)
0x0A, 0x32, 0x00, // Usage (In Range)
0x16, 0x00, 0x00, // Logical Minimum (0)
0x26, 0x01, 0x00, // Logical Maximum (1)
0x95, 0x02, // Report Count (2)
0x75, 0x01, // Report Size (1)
0x82, 0x02, 0x00, // Input (Data,Var,Abs,No Wrap,Linear,Preferred
State,No Null Position,Bit Field)
0x95, 0x01, // Report Count (1)
0x75, 0x06, // Report Size (6)
0x82, 0x01, 0x00, // Input (Const,Array,Abs,No
Wrap,Linear,Preferred State,No Null Position,Bit Field)
0x06, 0x0D, 0x00, // Usage Page (Digitizer)
0x0A, 0x30, 0x00, // Usage (Tip Pressure)
0x16, 0x00, 0x00, // Logical Minimum (0)
0x26, 0xFF, 0x00, // Logical Maximum (255)
0x95, 0x01, // Report Count (1)
0x75, 0x08, // Report Size (8)
0x82, 0x02, 0x00, // Input (Data,Var,Abs,No Wrap,Linear,Preferred
State,No Null Position,Bit Field)
0x06, 0x01, 0x00, // Usage Page (Generic Desktop Ctrls)
0x0A, 0x30, 0x00, // Usage (X)
0x0A, 0x31, 0x00, // Usage (Y)
0x16, 0x00, 0x00, // Logical Minimum (0)
0x26, 0x00, 0x40, // Logical Maximum (16384)
0x95, 0x02, // Report Count (2)
0x75, 0x10, // Report Size (16)
0x82, 0x02, 0x00, // Input (Data,Var,Abs,No Wrap,Linear,Preferred
State,No Null Position,Bit Field)
0x06, 0x09, 0x00, // Usage Page (Button)
0x1A, 0x01, 0x00, // Usage Minimum (0x01)
0x2A, 0x08, 0x00, // Usage Maximum (0x08)
0x16, 0x00, 0x00, // Logical Minimum (0)
0x26, 0x01, 0x00, // Logical Maximum (1)
0x95, 0x08, // Report Count (8)
0x75, 0x01, // Report Size (1)
0x82, 0x02, 0x00, // Input (Data,Var,Abs,No Wrap,Linear,Preferred
State,No Null Position,Bit Field)
0xC0, // End Collection
0xC0, // End Collection
0xC0, // End Collection
};
struct SHIDReport_MT
{
uint8_t m_touchPointId;
uint8_t m_tipSwitch;
uint8_t m_pressure;
uint16_t m_posX;
uint16_t m_posY;
uint8_t m_button;
} SHIDReport_MT_PACKED;
Thank you for your time!
Best regards,
Lyuboslav Angelov
Software developer
Abalta Technologies, Inc.
langelov@abaltatech.com
^ permalink raw reply
* Re: [PATCH] HID: lenovo: Add middleclick_workaround sysfs knob for cptkbd
From: Jiri Kosina @ 2024-01-23 10:55 UTC (permalink / raw)
To: Mikhail Khvainitski
Cc: Benjamin Tissoires, linux-input, linux-kernel, iam, jekhor, u-v
In-Reply-To: <20231223191401.70727-1-me@khvoinitsky.org>
On Sat, 23 Dec 2023, Mikhail Khvainitski wrote:
> Previous attempt to autodetect well-behaving patched firmware
> introduced in commit 46a0a2c96f0f ("HID: lenovo: Detect quirk-free fw
> on cptkbd and stop applying workaround") has shown that there are
> false-positives on original firmware (on both 1st gen and 2nd gen
> keyboards) which causes the middle button click workaround to be
> mistakenly disabled.
>
> This commit adds explicit parameter to sysfs to control this
> workaround.
>
> Fixes: 46a0a2c96f0f ("HID: lenovo: Detect quirk-free fw on cptkbd and stop applying workaround")
> Fixes: 43527a0094c1 ("HID: lenovo: Restrict detection of patched firmware only to USB cptkbd")
> Signed-off-by: Mikhail Khvainitski <me@khvoinitsky.org>
I am not a huge fan of sysctl device-specific knobs like this, but I guess
this is the best we can get here, unfortunately.
Now applied, thanks a lot.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v2] HID: i2c-hid: Skip SET_POWER SLEEP for Cirque touchpad on system suspend
From: Jiri Kosina @ 2024-01-23 10:26 UTC (permalink / raw)
To: Doug Anderson
Cc: Kai-Heng Feng, benjamin.tissoires, Hans de Goede, Maxime Ripard,
Thomas Weißschuh, Johan Hovold, linux-input, linux-kernel
In-Reply-To: <CAD=FV=VCoB_uBontRk3WVn-nqzZnutSMCHBpdBqNOFwrYv=8xw@mail.gmail.com>
On Wed, 17 Jan 2024, Doug Anderson wrote:
> > There's a Cirque touchpad that wakes system up without anything
> > touched the touchpad. The input report is empty when this happens. The
> > reason is stated in HID over I2C spec, 7.2.8.2: "If the DEVICE wishes
> > to wake the HOST from its low power state, it can issue a wake by
> > asserting the interrupt."
> >
> > This is fine if OS can put system back to suspend by identifying input
> > wakeup count stays the same on resume, like Chrome OS Dark Resume [0].
> > But for regular distro such policy is lacking.
> >
> > Though the change doesn't bring any impact on power consumption for
> > touchpad is minimal, other i2c-hid device may depends on SLEEP control
> > power. So use a quirk to limit the change scope.
> >
> > [0] https://chromium.googlesource.com/chromiumos/platform2/+/HEAD/power_manager/docs/dark_resume.md
> >
> > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > ---
> > v2:
> > - Use quirk instead of applying the change universally.
> >
> > drivers/hid/hid-ids.h | 3 +++
> > drivers/hid/i2c-hid/i2c-hid-core.c | 6 +++++-
> > 2 files changed, 8 insertions(+), 1 deletion(-)
>
> This seems OK to me. As per my repy to v1, it doesn't feel fully
> explained what's going on. Why does it only wake the system without
> touches if you put it to sleep first? Is it truly a hardware quirk with
> the Cirque touchpad or is it a bug on the board you have it hooked up
> to? In any case, perhaps it doesn't matter since you said you measured
> power here and, on this touchpad it doesn't seem to save significant
> extra power to go into sleep mode. ...so I guess I'd be OK w/
>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: nvidia-shield: Add some null pointer checks to the hid-nvidia-shield.c
From: Jiri Kosina @ 2024-01-23 10:23 UTC (permalink / raw)
To: Rahul Rameshbabu
Cc: Kunwu Chan, benjamin.tissoires, linux-input, linux-kernel
In-Reply-To: <87o7dhx2ny.fsf@nvidia.com>
On Thu, 18 Jan 2024, Rahul Rameshbabu wrote:
> > devm_kasprintf() returns a pointer to dynamically allocated memory
> > which can be NULL upon failure. Ensure the allocation was successful
> > by checking the pointer validity.
> >
> > Signed-off-by: Kunwu Chan <chentao@kylinos.cn>
> > ---
>
> Just one comment about the subject for future submissions (this one is
> fine). I would just leave the subject as "HID: nvidia-shield: Add some
> null pointer checks". We know what file is being edited from the commit
> content, so the additional filename detail isn't necessary.
>
> Thanks for the patch. Really appreciate it.
>
> Reviewed-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>
I have tweaked the changelog a little bit and applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: nintendo: Remove some unused functions
From: Jiri Kosina @ 2024-01-23 10:20 UTC (permalink / raw)
To: Jiapeng Chong
Cc: djogorchock, benjamin.tissoires, linux-input, linux-kernel,
Abaci Robot
In-Reply-To: <20240119072729.54499-1-jiapeng.chong@linux.alibaba.com>
On Fri, 19 Jan 2024, Jiapeng Chong wrote:
> These functions are defined in the hid-nintendo.c file, but not called
> elsewhere, so delete these unused functions.
>
> drivers/hid/hid-nintendo.c:670:20: warning: unused function 'joycon_device_is_left_joycon'.
> drivers/hid/hid-nintendo.c:674:20: warning: unused function 'joycon_device_is_right_joycon'
>
> Reported-by: Abaci Robot <abaci@linux.alibaba.com>
> Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=7992
> Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* RE: [HID Patchsets for Samsung driver v2 2/6] HID: Samsung : Fix the checkpatch complain.
From: sandeep.cs @ 2024-01-23 10:04 UTC (permalink / raw)
To: 'Jiri Kosina'
Cc: 'Joe Perches', 'Benjamin Tissoires', gaudium.lee,
ih0923.kim, suhyun_.kim, jitender.s21, junwan.cho, linux-input,
linux-kernel
In-Reply-To: <nycvar.YFH.7.76.2401231049150.29548@cbobk.fhfr.pm>
>> >> - } else
>> >> - if (*rsize == 203 && rdesc[192] == 0x15 && rdesc[193] == 0x0 &&
>> >> + } else if (*rsize == 203 && rdesc[192] == 0x15 && rdesc[193] ==
>> >> +0x0 &&
>> >> rdesc[194] == 0x25 && rdesc[195] == 0x12) {
>> >> samsung_irda_dev_trace(hdev, 203);
>> >> rdesc[193] = 0x1;
>> >> rdesc[195] = 0xf;
>> >> - } else
>> >> - if (*rsize == 135 && rdesc[124] == 0x15 && rdesc[125] == 0x0 &&
>> >> + } else if (*rsize == 135 && rdesc[124] == 0x15 && rdesc[125] ==
>> >> +0x0 &&
>> >> rdesc[126] == 0x25 && rdesc[127] == 0x11) {
>> >> samsung_irda_dev_trace(hdev, 135);
>> >> rdesc[125] = 0x1;
>> >> rdesc[127] = 0xe;
>> >> - } else
>> >> - if (*rsize == 171 && rdesc[160] == 0x15 && rdesc[161] == 0x0 &&
>> >> + } else if (*rsize == 171 && rdesc[160] == 0x15 && rdesc[161] ==
>> >> +0x0 &&
>> >> rdesc[162] == 0x25 && rdesc[163] == 0x01) {
>> >> samsung_irda_dev_trace(hdev, 171);
>> >> rdesc[161] = 0x1;
>> >
>> >For this block, I think a rewrite using memcmp would be clearer.
>> >Something like:
>> Okay . Thanks for your valuable feedback. We will promptly address
>> your suggestions and enhance our code accordingly.
>
>I agree with Joe's suggestion here; are you planning to send v2 of the
series?
>
>The rest of the set looks good to me.
>
>Thanks,
>
>--
>Jiri Kosina
>SUSE Labs
Hello Jiri ,
Yes, I am planning to send the v2 of the series. I will include review
comments and share it with you soon.
Thank & Regards
Sandeep C S
^ permalink raw reply
* RE: [HID Patchsets for Samsung driver v2 2/6] HID: Samsung : Fix the checkpatch complain.
From: Jiri Kosina @ 2024-01-23 9:49 UTC (permalink / raw)
To: sandeep.cs
Cc: 'Joe Perches', 'Benjamin Tissoires', gaudium.lee,
ih0923.kim, suhyun_.kim, jitender.s21, junwan.cho, linux-input,
linux-kernel
In-Reply-To: <020e01da421f$c0d20660$42761320$@samsung.com>
On Mon, 8 Jan 2024, sandeep.cs wrote:
> >> - } else
> >> - if (*rsize == 203 && rdesc[192] == 0x15 && rdesc[193] == 0x0 &&
> >> + } else if (*rsize == 203 && rdesc[192] == 0x15 && rdesc[193] == 0x0
> >> +&&
> >> rdesc[194] == 0x25 && rdesc[195] == 0x12) {
> >> samsung_irda_dev_trace(hdev, 203);
> >> rdesc[193] = 0x1;
> >> rdesc[195] = 0xf;
> >> - } else
> >> - if (*rsize == 135 && rdesc[124] == 0x15 && rdesc[125] == 0x0 &&
> >> + } else if (*rsize == 135 && rdesc[124] == 0x15 && rdesc[125] == 0x0
> >> +&&
> >> rdesc[126] == 0x25 && rdesc[127] == 0x11) {
> >> samsung_irda_dev_trace(hdev, 135);
> >> rdesc[125] = 0x1;
> >> rdesc[127] = 0xe;
> >> - } else
> >> - if (*rsize == 171 && rdesc[160] == 0x15 && rdesc[161] == 0x0 &&
> >> + } else if (*rsize == 171 && rdesc[160] == 0x15 && rdesc[161] == 0x0
> >> +&&
> >> rdesc[162] == 0x25 && rdesc[163] == 0x01) {
> >> samsung_irda_dev_trace(hdev, 171);
> >> rdesc[161] = 0x1;
> >
> >For this block, I think a rewrite using memcmp would be clearer.
> >Something like:
> Okay . Thanks for your valuable feedback. We will promptly address your
> suggestions and enhance our code accordingly.
I agree with Joe's suggestion here; are you planning to send v2 of the
series?
The rest of the set looks good to me.
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v1 0/7] HID: playstation: DS4: LED bugfix, third-party gamepad support
From: Jiri Kosina @ 2024-01-23 9:47 UTC (permalink / raw)
To: Max Staudt
Cc: Roderick Colenbrander, Benjamin Tissoires, linux-input,
linux-kernel
In-Reply-To: <20240115144538.12018-1-max@enpas.org>
On Mon, 15 Jan 2024, Max Staudt wrote:
> Dear hid-playstation maintainers,
>
> Could you please have a look at the enclosed patches for the DualShock 4
> driver in hid-playstation, and upstream them if possible?
>
> There is one bugfix, and a few small patches to enable third-party
> controllers. They sometimes don't implement features that they
> semantically "don't need", but which currently trip the driver.
>
> For example, for the DualShock 4, we don't actually need to know the
> firmware version in order to work with the gamepad - unlike with the
> DualSense, which has different driver logic depending on the version.
>
> Finally, there are two patches to add a DS4 compatible controller with
> an unassigned VID/PID - I'd appreciate your thoughts on that.
>
> If I can make it easier to upstream these patches, please let me know.
>
> Thanks!
>
> Max
>
> Patches in this series:
> [PATCH v1 1/7] HID: playstation: DS4: Fix LED blinking
> [PATCH v1 2/7] HID: playstation: DS4: Don't fail on MAC address
> [PATCH v1 3/7] HID: playstation: DS4: Don't fail on FW/HW version
> [PATCH v1 4/7] HID: playstation: DS4: Don't fail on calibration data
> [PATCH v1 5/7] HID: playstation: DS4: Parse minimal report 0x01
> [PATCH v1 6/7] HID: playstation: Simplify device type ID
> [PATCH v1 7/7] HID: playstation: DS4: Add VID/PID for SZ-MYPOWER
Roderick, any word on this series, please?
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH 1/7] dt-bindings: input: melfas,mms114: add MMS252 compatible
From: Dmitry Torokhov @ 2024-01-23 3:33 UTC (permalink / raw)
To: Bryant Mairs
Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Linus Walleij, linux-arm-msm,
devicetree, linux-kernel, linux-input, Luca Weiss
In-Reply-To: <20231105204759.37107-2-bryant@mai.rs>
On Sun, Nov 05, 2023 at 09:46:16PM +0100, Bryant Mairs wrote:
> From: Luca Weiss <luca@z3ntu.xyz>
>
> Add a compatible for MMS252 touchscreen which appears to work fine with
> the MMS114 driver.
>
> Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
> Signed-off-by: Bryant Mairs <bryant@mai.rs>
Applied, thank you.
--
Dmitry
^ 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