* [PATCH HID 12/13] HID: bpf: Artist24: remove unused variable
From: Benjamin Tissoires @ 2024-05-28 13:14 UTC (permalink / raw)
To: Shuah Khan, Jiri Kosina, Jonathan Corbet, Alexei Starovoitov
Cc: linux-kselftest, linux-kernel, bpf, linux-input, linux-doc,
Benjamin Tissoires, Peter Hutterer
In-Reply-To: <20240528-hid_bpf_struct_ops-v1-0-8c6663df27d8@kernel.org>
warning: unused variable ‘tilt’ [-Wunused-variable]
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
drivers/hid/bpf/progs/XPPen__Artist24.bpf.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/hid/bpf/progs/XPPen__Artist24.bpf.c b/drivers/hid/bpf/progs/XPPen__Artist24.bpf.c
index bc0b85c38445..d4d062c3a653 100644
--- a/drivers/hid/bpf/progs/XPPen__Artist24.bpf.c
+++ b/drivers/hid/bpf/progs/XPPen__Artist24.bpf.c
@@ -158,7 +158,6 @@ int BPF_PROG(xppen_24_fix_eraser, struct hid_bpf_ctx *hctx)
__u8 *data = hid_bpf_get_data(hctx, 0 /* offset */, 10 /* size */);
__u8 current_state, changed_state;
bool prev_tip;
- __u16 tilt;
if (!data)
return 0; /* EPERM check */
--
2.44.0
^ permalink raw reply related
* [PATCH HID 13/13] HID: bpf: error on warnings when compiling bpf objects
From: Benjamin Tissoires @ 2024-05-28 13:14 UTC (permalink / raw)
To: Shuah Khan, Jiri Kosina, Jonathan Corbet, Alexei Starovoitov
Cc: linux-kselftest, linux-kernel, bpf, linux-input, linux-doc,
Benjamin Tissoires
In-Reply-To: <20240528-hid_bpf_struct_ops-v1-0-8c6663df27d8@kernel.org>
There is no real reasons to paper over warnings for such small programs.
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
drivers/hid/bpf/progs/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/bpf/progs/Makefile b/drivers/hid/bpf/progs/Makefile
index 63ed7e02adf1..ec1fc642fd63 100644
--- a/drivers/hid/bpf/progs/Makefile
+++ b/drivers/hid/bpf/progs/Makefile
@@ -56,7 +56,7 @@ clean:
%.bpf.o: %.bpf.c vmlinux.h $(BPFOBJ) | $(OUTPUT)
$(call msg,BPF,$@)
- $(Q)$(CLANG) -g -O2 --target=bpf $(INCLUDES) \
+ $(Q)$(CLANG) -g -O2 --target=bpf -Wall -Werror $(INCLUDES) \
-c $(filter %.c,$^) -o $@ && \
$(LLVM_STRIP) -g $@
--
2.44.0
^ permalink raw reply related
* Re: [PATCH v2 0/2] arm64: dts: allwinner: H616: Add LRADC
From: Chen-Yu Tsai @ 2024-05-28 15:42 UTC (permalink / raw)
To: Hans de Goede, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jernej Skrabec, Samuel Holland, Andre Przywara,
James McGregor
Cc: linux-input, devicetree, linux-arm-kernel, linux-sunxi
In-Reply-To: <20240426092924.15489-1-jamcgregor@protonmail.com>
On Fri, 26 Apr 2024 09:29:37 +0000, James McGregor wrote:
> Version 2 moves the LRADC DT node to the right place. It was out of
> order before.
>
> The Allwinner H616 series of SoCs have a low-rate ADC (LRADC) with
> 6-bit resolution and one input channel. They're compatible with the
> existing drivers, so it only needs to be enabled in the DT.
>
> [...]
Applied to sunxi/dt-for-6.11 in sunxi/linux.git, thanks!
[1/2] dt-bindings: input: sun4i-lradc-keys: Add H616 compatible
https://git.kernel.org/sunxi/linux/c/3086803a1f43
[2/2] ARM: dts: sun50i: Add LRADC node
https://git.kernel.org/sunxi/linux/c/7adc2d68f4a6
Best regards,
--
Chen-Yu Tsai <wens@csie.org>
^ permalink raw reply
* Re: [PATCH v2 0/2] arm64: dts: allwinner: H616: Add LRADC
From: Chen-Yu Tsai @ 2024-05-28 16:18 UTC (permalink / raw)
To: Samuel Holland, James McGregor, Jernej Skrabec
Cc: linux-input, devicetree, linux-arm-kernel, linux-sunxi,
Conor Dooley, Andre Przywara, Hans de Goede, Dmitry Torokhov,
Rob Herring, Krzysztof Kozlowski
In-Reply-To: <171691092979.680152.13758975851829859883.b4-ty@csie.org>
On Tue, May 28, 2024 at 11:42 PM Chen-Yu Tsai <wens@csie.org> wrote:
>
> On Fri, 26 Apr 2024 09:29:37 +0000, James McGregor wrote:
> > Version 2 moves the LRADC DT node to the right place. It was out of
> > order before.
> >
> > The Allwinner H616 series of SoCs have a low-rate ADC (LRADC) with
> > 6-bit resolution and one input channel. They're compatible with the
> > existing drivers, so it only needs to be enabled in the DT.
> >
> > [...]
>
> Applied to sunxi/dt-for-6.11 in sunxi/linux.git, thanks!
>
> [1/2] dt-bindings: input: sun4i-lradc-keys: Add H616 compatible
> https://git.kernel.org/sunxi/linux/c/3086803a1f43
> [2/2] ARM: dts: sun50i: Add LRADC node
> https://git.kernel.org/sunxi/linux/c/7adc2d68f4a6
I had to do a quick rebase as the branch start point was incorrect. The
commit hashes will have changed. Rest assured that the patch is indeed
merged.
ChenYu
^ permalink raw reply
* Re: [PATCH 1/2] input: Add event code for accessibility key
From: Dmitry Torokhov @ 2024-05-28 20:17 UTC (permalink / raw)
To: Aseda Aboagye; +Cc: Jiri Kosina, Benjamin Tissoires, linux-input
In-Reply-To: <Zk7bbiOwF4ODEE6H@google.com>
On Thu, May 23, 2024 at 01:00:14AM -0500, Aseda Aboagye wrote:
> HUTRR116 added support for a new usage titled "System Accessibility
> Binding" which toggles a system-wide bound accessibility UI or command.
> This commit simply adds a new event code for the usage.
>
> Signed-off-by: Aseda Aboagye <aaboagye@chromium.org>
> ---
> drivers/hid/hid-debug.c | 1 +
> drivers/hid/hid-input.c | 3 +++
> include/uapi/linux/input-event-codes.h | 2 ++
> 3 files changed, 6 insertions(+)
>
> diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
> index e7ef1ea107c9..7749c81b6227 100644
> --- a/drivers/hid/hid-debug.c
> +++ b/drivers/hid/hid-debug.c
> @@ -974,6 +974,7 @@ static const char *keys[KEY_MAX + 1] = {
> [KEY_CAMERA_ACCESS_ENABLE] = "CameraAccessEnable",
> [KEY_CAMERA_ACCESS_DISABLE] = "CameraAccessDisable",
> [KEY_CAMERA_ACCESS_TOGGLE] = "CameraAccessToggle",
> + [KEY_ACCESSIBILITY] = "Accessibility",
> [KEY_DICTATE] = "Dictate",
> [KEY_MICMUTE] = "MicrophoneMute",
> [KEY_BRIGHTNESS_MIN] = "BrightnessMin",
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index 851ee86eff32..6d2dbb75ba65 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -829,6 +829,9 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
> if ((usage->hid & 0xf0) == 0xa0) { /* SystemControl */
> switch (usage->hid & 0xf) {
> case 0x9: map_key_clear(KEY_MICMUTE); break;
> + case 0xa:
> + map_key_clear(KEY_ACCESSIBILITY);
> + break;
Please keep the style to match with the rest of the file.
> default: goto ignore;
> }
> break;
> diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> index 022a520e31fc..980ef7fefd2b 100644
> --- a/include/uapi/linux/input-event-codes.h
> +++ b/include/uapi/linux/input-event-codes.h
> @@ -617,6 +617,8 @@
> #define KEY_CAMERA_ACCESS_ENABLE 0x24b /* Enables programmatic access to camera devices. (HUTRR72) */
> #define KEY_CAMERA_ACCESS_DISABLE 0x24c /* Disables programmatic access to camera devices. (HUTRR72) */
> #define KEY_CAMERA_ACCESS_TOGGLE 0x24d /* Toggles the current state of the camera access control. (HUTRR72) */
> +/* Toggles the system bound accessibility UI/command (HUTRR116) */
> +#define KEY_ACCESSIBILITY 0x24e
Please have the comment after the value to match with the rest of the
file (even though it results in a long line).
>
> #define KEY_BRIGHTNESS_MIN 0x250 /* Set Brightness to Minimum */
> #define KEY_BRIGHTNESS_MAX 0x251 /* Set Brightness to Maximum */
>
> base-commit: 5128de84d8fc849400d00f7a6982711f129699ea
> --
> 2.45.1.288.g0e0cd299f1-goog
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 2/2] input: Add support for "Do Not Disturb"
From: Dmitry Torokhov @ 2024-05-28 20:18 UTC (permalink / raw)
To: Aseda Aboagye; +Cc: Jiri Kosina, Benjamin Tissoires, linux-input
In-Reply-To: <Zk7csk3RHEqCruWU@google.com>
On Thu, May 23, 2024 at 01:05:38AM -0500, Aseda Aboagye wrote:
> HUTRR94 added support for a new usage titled "System Do Not Disturb"
> which toggles a system-wide Do Not Disturb setting. This commit simply
> adds a new event code for the usage.
>
> Signed-off-by: Aseda Aboagye <aaboagye@chromium.org>
> ---
> drivers/hid/hid-debug.c | 1 +
> drivers/hid/hid-input.c | 11 +++++++++++
> include/uapi/linux/input-event-codes.h | 2 ++
> 3 files changed, 14 insertions(+)
>
> diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
> index 7749c81b6227..78b2dd10cba2 100644
> --- a/drivers/hid/hid-debug.c
> +++ b/drivers/hid/hid-debug.c
> @@ -975,6 +975,7 @@ static const char *keys[KEY_MAX + 1] = {
> [KEY_CAMERA_ACCESS_DISABLE] = "CameraAccessDisable",
> [KEY_CAMERA_ACCESS_TOGGLE] = "CameraAccessToggle",
> [KEY_ACCESSIBILITY] = "Accessibility",
> + [KEY_DONOTDISTURB] = "DoNotDisturb",
> [KEY_DICTATE] = "Dictate",
> [KEY_MICMUTE] = "MicrophoneMute",
> [KEY_BRIGHTNESS_MIN] = "BrightnessMin",
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index 6d2dbb75ba65..7fda66f7b437 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -826,6 +826,17 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
> break;
> }
>
> + if ((usage->hid & 0xf0) == 0x90) { /* SystemControl*/
> + switch (usage->hid & 0xf) {
> + case 0xb:
> + map_key_clear(KEY_DONOTDISTURB);
> + break;
> + default:
> + goto ignore;
> + }
> + break;
> + }
> +
> if ((usage->hid & 0xf0) == 0xa0) { /* SystemControl */
> switch (usage->hid & 0xf) {
> case 0x9: map_key_clear(KEY_MICMUTE); break;
> diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> index 980ef7fefd2b..b8abc239d660 100644
> --- a/include/uapi/linux/input-event-codes.h
> +++ b/include/uapi/linux/input-event-codes.h
> @@ -619,6 +619,8 @@
> #define KEY_CAMERA_ACCESS_TOGGLE 0x24d /* Toggles the current state of the camera access control. (HUTRR72) */
> /* Toggles the system bound accessibility UI/command (HUTRR116) */
> #define KEY_ACCESSIBILITY 0x24e
> +/* Toggles the system-wide "Do Not Disturb" control (HUTRR94)*/
> +#define KEY_DONOTDISTURB 0x24f
Could we have this as KEY_DO_NOT_DISTURB?
Also the same comments as on the previous patch.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 2/2] HID: intel-ish-hid: fix endian-conversion
From: kernel test robot @ 2024-05-28 21:06 UTC (permalink / raw)
To: Arnd Bergmann, Srinivas Pandruvada, Jiri Kosina,
Benjamin Tissoires, Zhang Lixu
Cc: oe-kbuild-all, Arnd Bergmann, linux-input, linux-kernel
In-Reply-To: <20240528115802.3122955-2-arnd@kernel.org>
Hi Arnd,
kernel test robot noticed the following build errors:
[auto build test ERROR on hid/for-next]
[also build test ERROR on next-20240528]
[cannot apply to linus/master v6.10-rc1]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Arnd-Bergmann/HID-intel-ish-hid-fix-endian-conversion/20240528-200100
base: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
patch link: https://lore.kernel.org/r/20240528115802.3122955-2-arnd%40kernel.org
patch subject: [PATCH 2/2] HID: intel-ish-hid: fix endian-conversion
config: i386-allmodconfig (https://download.01.org/0day-ci/archive/20240529/202405290420.DtwUdg3b-lkp@intel.com/config)
compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240529/202405290420.DtwUdg3b-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/202405290420.DtwUdg3b-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/hid/intel-ish-hid/ishtp/loader.c: In function 'prepare_dma_bufs':
>> drivers/hid/intel-ish-hid/ishtp/loader.c:178:51: error: 'dma' undeclared (first use in this function); did you mean 'cma'?
178 | &dma, GFP_KERNEL);
| ^~~
| cma
drivers/hid/intel-ish-hid/ishtp/loader.c:178:51: note: each undeclared identifier is reported only once for each function it appears in
vim +178 drivers/hid/intel-ish-hid/ishtp/loader.c
154
155 /**
156 * prepare_dma_bufs() - Prepare the DMA buffer for transferring firmware fragments
157 * @dev: The ISHTP device
158 * @ish_fw: The ISH firmware
159 * @fragment: The ISHTP firmware fragment descriptor
160 * @dma_bufs: The array of DMA fragment buffers
161 * @fragment_size: The size of a single DMA fragment
162 *
163 * Return: 0 on success, negative error code on failure
164 */
165 static int prepare_dma_bufs(struct ishtp_device *dev,
166 const struct firmware *ish_fw,
167 struct loader_xfer_dma_fragment *fragment,
168 void **dma_bufs, u32 fragment_size, u32 fragment_count)
169 {
170 dma_addr_t dma_addr;
171 u32 offset = 0;
172 u32 length;
173 int i;
174
175 for (i = 0; i < fragment->fragment_cnt && offset < ish_fw->size; i++) {
176 dma_bufs[i] = dma_alloc_coherent(dev->devc, fragment_size, &dma_addr, GFP_KERNEL);
177 dma_bufs[i] = dma_alloc_coherent(dev->devc, fragment_size,
> 178 &dma, GFP_KERNEL);
179 if (!dma_bufs[i])
180 return -ENOMEM;
181
182 fragment->fragment_tbl[i].ddr_adrs = cpu_to_le64(dma_addr);
183
184 memcpy(dma_bufs[i], ish_fw->data + offset, le32_to_cpu(fragment->fragment_tbl[i].length));
185 dma_wmb();
186 fragment->fragment_tbl[i].ddr_adrs = cpu_to_le64(dma);
187 length = clamp(ish_fw->size - offset, 0, fragment_size);
188 fragment->fragment_tbl[i].length = cpu_to_le32(length);
189 fragment->fragment_tbl[i].fw_off = cpu_to_le32(offset);
190
191 offset += length;
192 }
193
194 return 0;
195 }
196
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH 2/2] HID: intel-ish-hid: fix endian-conversion
From: kernel test robot @ 2024-05-28 21:17 UTC (permalink / raw)
To: Arnd Bergmann, Srinivas Pandruvada, Jiri Kosina,
Benjamin Tissoires, Zhang Lixu
Cc: llvm, oe-kbuild-all, Arnd Bergmann, linux-input, linux-kernel
In-Reply-To: <20240528115802.3122955-2-arnd@kernel.org>
Hi Arnd,
kernel test robot noticed the following build errors:
[auto build test ERROR on hid/for-next]
[also build test ERROR on next-20240528]
[cannot apply to linus/master v6.10-rc1]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Arnd-Bergmann/HID-intel-ish-hid-fix-endian-conversion/20240528-200100
base: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
patch link: https://lore.kernel.org/r/20240528115802.3122955-2-arnd%40kernel.org
patch subject: [PATCH 2/2] HID: intel-ish-hid: fix endian-conversion
config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20240529/202405290447.n14W21hZ-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240529/202405290447.n14W21hZ-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/202405290447.n14W21hZ-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/hid/intel-ish-hid/ishtp/loader.c:178:9: error: use of undeclared identifier 'dma'
178 | &dma, GFP_KERNEL);
| ^
drivers/hid/intel-ish-hid/ishtp/loader.c:186:52: error: use of undeclared identifier 'dma'
186 | fragment->fragment_tbl[i].ddr_adrs = cpu_to_le64(dma);
| ^
include/linux/byteorder/generic.h:86:21: note: expanded from macro 'cpu_to_le64'
86 | #define cpu_to_le64 __cpu_to_le64
| ^
2 errors generated.
vim +/dma +178 drivers/hid/intel-ish-hid/ishtp/loader.c
154
155 /**
156 * prepare_dma_bufs() - Prepare the DMA buffer for transferring firmware fragments
157 * @dev: The ISHTP device
158 * @ish_fw: The ISH firmware
159 * @fragment: The ISHTP firmware fragment descriptor
160 * @dma_bufs: The array of DMA fragment buffers
161 * @fragment_size: The size of a single DMA fragment
162 *
163 * Return: 0 on success, negative error code on failure
164 */
165 static int prepare_dma_bufs(struct ishtp_device *dev,
166 const struct firmware *ish_fw,
167 struct loader_xfer_dma_fragment *fragment,
168 void **dma_bufs, u32 fragment_size, u32 fragment_count)
169 {
170 dma_addr_t dma_addr;
171 u32 offset = 0;
172 u32 length;
173 int i;
174
175 for (i = 0; i < fragment->fragment_cnt && offset < ish_fw->size; i++) {
176 dma_bufs[i] = dma_alloc_coherent(dev->devc, fragment_size, &dma_addr, GFP_KERNEL);
177 dma_bufs[i] = dma_alloc_coherent(dev->devc, fragment_size,
> 178 &dma, GFP_KERNEL);
179 if (!dma_bufs[i])
180 return -ENOMEM;
181
182 fragment->fragment_tbl[i].ddr_adrs = cpu_to_le64(dma_addr);
183
184 memcpy(dma_bufs[i], ish_fw->data + offset, le32_to_cpu(fragment->fragment_tbl[i].length));
185 dma_wmb();
186 fragment->fragment_tbl[i].ddr_adrs = cpu_to_le64(dma);
187 length = clamp(ish_fw->size - offset, 0, fragment_size);
188 fragment->fragment_tbl[i].length = cpu_to_le32(length);
189 fragment->fragment_tbl[i].fw_off = cpu_to_le32(offset);
190
191 offset += length;
192 }
193
194 return 0;
195 }
196
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* [PATCH v2 1/2] input: Add event code for accessibility key
From: Aseda Aboagye @ 2024-05-28 22:44 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Dmitry Torokhov, linux-input
HUTRR116 added support for a new usage titled "System Accessibility
Binding" which toggles a system-wide bound accessibility UI or command.
This commit simply adds a new event code for the usage.
Signed-off-by: Aseda Aboagye <aaboagye@chromium.org>
---
Changes from v1:
- Modified formatting to match existing code, ignoring checkpatch.pl.
drivers/hid/hid-debug.c | 1 +
drivers/hid/hid-input.c | 1 +
include/uapi/linux/input-event-codes.h | 1 +
3 files changed, 3 insertions(+)
diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
index e7ef1ea107c9..7749c81b6227 100644
--- a/drivers/hid/hid-debug.c
+++ b/drivers/hid/hid-debug.c
@@ -974,6 +974,7 @@ static const char *keys[KEY_MAX + 1] = {
[KEY_CAMERA_ACCESS_ENABLE] = "CameraAccessEnable",
[KEY_CAMERA_ACCESS_DISABLE] = "CameraAccessDisable",
[KEY_CAMERA_ACCESS_TOGGLE] = "CameraAccessToggle",
+ [KEY_ACCESSIBILITY] = "Accessibility",
[KEY_DICTATE] = "Dictate",
[KEY_MICMUTE] = "MicrophoneMute",
[KEY_BRIGHTNESS_MIN] = "BrightnessMin",
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 851ee86eff32..1ecc5ad57b56 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -829,6 +829,7 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
if ((usage->hid & 0xf0) == 0xa0) { /* SystemControl */
switch (usage->hid & 0xf) {
case 0x9: map_key_clear(KEY_MICMUTE); break;
+ case 0xa: map_key_clear(KEY_ACCESSIBILITY); break;
default: goto ignore;
}
break;
diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
index 022a520e31fc..7ff6eeef1af0 100644
--- a/include/uapi/linux/input-event-codes.h
+++ b/include/uapi/linux/input-event-codes.h
@@ -617,6 +617,7 @@
#define KEY_CAMERA_ACCESS_ENABLE 0x24b /* Enables programmatic access to camera devices. (HUTRR72) */
#define KEY_CAMERA_ACCESS_DISABLE 0x24c /* Disables programmatic access to camera devices. (HUTRR72) */
#define KEY_CAMERA_ACCESS_TOGGLE 0x24d /* Toggles the current state of the camera access control. (HUTRR72) */
+#define KEY_ACCESSIBILITY 0x24e /* Toggles the system bound accessibility UI/command (HUTRR116) */
#define KEY_BRIGHTNESS_MIN 0x250 /* Set Brightness to Minimum */
#define KEY_BRIGHTNESS_MAX 0x251 /* Set Brightness to Maximum */
base-commit: 5128de84d8fc849400d00f7a6982711f129699ea
--
2.45.1.288.g0e0cd299f1-goog
^ permalink raw reply related
* [PATCH v2 2/2] input: Add support for "Do Not Disturb"
From: Aseda Aboagye @ 2024-05-28 22:51 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Dmitry Torokhov, linux-input
In-Reply-To: <ZlZeMVHsquYbQzGG@google.com>
HUTRR94 added support for a new usage titled "System Do Not Disturb"
which toggles a system-wide Do Not Disturb setting. This commit simply
adds a new event code for the usage.
Signed-off-by: Aseda Aboagye <aaboagye@chromium.org>
---
Changes from v1:
- Modified formatting to match existing code, ignoring checkpatch.pl.
drivers/hid/hid-debug.c | 1 +
drivers/hid/hid-input.c | 8 ++++++++
include/uapi/linux/input-event-codes.h | 1 +
3 files changed, 10 insertions(+)
diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
index 7749c81b6227..78b2dd10cba2 100644
--- a/drivers/hid/hid-debug.c
+++ b/drivers/hid/hid-debug.c
@@ -975,6 +975,7 @@ static const char *keys[KEY_MAX + 1] = {
[KEY_CAMERA_ACCESS_DISABLE] = "CameraAccessDisable",
[KEY_CAMERA_ACCESS_TOGGLE] = "CameraAccessToggle",
[KEY_ACCESSIBILITY] = "Accessibility",
+ [KEY_DONOTDISTURB] = "DoNotDisturb",
[KEY_DICTATE] = "Dictate",
[KEY_MICMUTE] = "MicrophoneMute",
[KEY_BRIGHTNESS_MIN] = "BrightnessMin",
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 1ecc5ad57b56..f023f51b9c08 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -826,6 +826,14 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
break;
}
+ if ((usage->hid & 0xf0) == 0x90) { /* SystemControl*/
+ switch (usage->hid & 0xf) {
+ case 0xb: map_key_clear(KEY_DONOTDISTURB); break;
+ default: goto ignore;
+ }
+ break;
+ }
+
if ((usage->hid & 0xf0) == 0xa0) { /* SystemControl */
switch (usage->hid & 0xf) {
case 0x9: map_key_clear(KEY_MICMUTE); break;
diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
index 7ff6eeef1af0..c971d542e525 100644
--- a/include/uapi/linux/input-event-codes.h
+++ b/include/uapi/linux/input-event-codes.h
@@ -618,6 +618,7 @@
#define KEY_CAMERA_ACCESS_DISABLE 0x24c /* Disables programmatic access to camera devices. (HUTRR72) */
#define KEY_CAMERA_ACCESS_TOGGLE 0x24d /* Toggles the current state of the camera access control. (HUTRR72) */
#define KEY_ACCESSIBILITY 0x24e /* Toggles the system bound accessibility UI/command (HUTRR116) */
+#define KEY_DONOTDISTURB 0x24f /* Toggles the system-wide "Do Not Disturb" control (HUTRR94)*/
#define KEY_BRIGHTNESS_MIN 0x250 /* Set Brightness to Minimum */
#define KEY_BRIGHTNESS_MAX 0x251 /* Set Brightness to Maximum */
--
2.45.1.288.g0e0cd299f1-goog
^ permalink raw reply related
* Re: [PATCH v2 1/2] input: Add event code for accessibility key
From: Dmitry Torokhov @ 2024-05-28 23:31 UTC (permalink / raw)
To: Aseda Aboagye; +Cc: Jiri Kosina, Benjamin Tissoires, linux-input
In-Reply-To: <ZlZeMVHsquYbQzGG@google.com>
On Tue, May 28, 2024 at 05:44:01PM -0500, Aseda Aboagye wrote:
> HUTRR116 added support for a new usage titled "System Accessibility
> Binding" which toggles a system-wide bound accessibility UI or command.
> This commit simply adds a new event code for the usage.
>
> Signed-off-by: Aseda Aboagye <aaboagye@chromium.org>
> ---
> Changes from v1:
> - Modified formatting to match existing code, ignoring checkpatch.pl.
>
> drivers/hid/hid-debug.c | 1 +
> drivers/hid/hid-input.c | 1 +
> include/uapi/linux/input-event-codes.h | 1 +
> 3 files changed, 3 insertions(+)
>
> diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
> index e7ef1ea107c9..7749c81b6227 100644
> --- a/drivers/hid/hid-debug.c
> +++ b/drivers/hid/hid-debug.c
> @@ -974,6 +974,7 @@ static const char *keys[KEY_MAX + 1] = {
> [KEY_CAMERA_ACCESS_ENABLE] = "CameraAccessEnable",
> [KEY_CAMERA_ACCESS_DISABLE] = "CameraAccessDisable",
> [KEY_CAMERA_ACCESS_TOGGLE] = "CameraAccessToggle",
> + [KEY_ACCESSIBILITY] = "Accessibility",
> [KEY_DICTATE] = "Dictate",
> [KEY_MICMUTE] = "MicrophoneMute",
> [KEY_BRIGHTNESS_MIN] = "BrightnessMin",
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index 851ee86eff32..1ecc5ad57b56 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -829,6 +829,7 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
> if ((usage->hid & 0xf0) == 0xa0) { /* SystemControl */
> switch (usage->hid & 0xf) {
> case 0x9: map_key_clear(KEY_MICMUTE); break;
> + case 0xa: map_key_clear(KEY_ACCESSIBILITY); break;
> default: goto ignore;
> }
> break;
> diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> index 022a520e31fc..7ff6eeef1af0 100644
> --- a/include/uapi/linux/input-event-codes.h
> +++ b/include/uapi/linux/input-event-codes.h
> @@ -617,6 +617,7 @@
> #define KEY_CAMERA_ACCESS_ENABLE 0x24b /* Enables programmatic access to camera devices. (HUTRR72) */
> #define KEY_CAMERA_ACCESS_DISABLE 0x24c /* Disables programmatic access to camera devices. (HUTRR72) */
> #define KEY_CAMERA_ACCESS_TOGGLE 0x24d /* Toggles the current state of the camera access control. (HUTRR72) */
> +#define KEY_ACCESSIBILITY 0x24e /* Toggles the system bound accessibility UI/command (HUTRR116) */
For input:
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Feel free to merge through HID tree.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2 2/2] input: Add support for "Do Not Disturb"
From: Dmitry Torokhov @ 2024-05-28 23:32 UTC (permalink / raw)
To: Aseda Aboagye; +Cc: Jiri Kosina, Benjamin Tissoires, linux-input
In-Reply-To: <ZlZgByQ3TlycC-A_@google.com>
On Tue, May 28, 2024 at 05:51:51PM -0500, Aseda Aboagye wrote:
> HUTRR94 added support for a new usage titled "System Do Not Disturb"
> which toggles a system-wide Do Not Disturb setting. This commit simply
> adds a new event code for the usage.
>
> Signed-off-by: Aseda Aboagye <aaboagye@chromium.org>
> ---
> Changes from v1:
> - Modified formatting to match existing code, ignoring checkpatch.pl.
>
> drivers/hid/hid-debug.c | 1 +
> drivers/hid/hid-input.c | 8 ++++++++
> include/uapi/linux/input-event-codes.h | 1 +
> 3 files changed, 10 insertions(+)
>
> diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
> index 7749c81b6227..78b2dd10cba2 100644
> --- a/drivers/hid/hid-debug.c
> +++ b/drivers/hid/hid-debug.c
> @@ -975,6 +975,7 @@ static const char *keys[KEY_MAX + 1] = {
> [KEY_CAMERA_ACCESS_DISABLE] = "CameraAccessDisable",
> [KEY_CAMERA_ACCESS_TOGGLE] = "CameraAccessToggle",
> [KEY_ACCESSIBILITY] = "Accessibility",
> + [KEY_DONOTDISTURB] = "DoNotDisturb",
> [KEY_DICTATE] = "Dictate",
> [KEY_MICMUTE] = "MicrophoneMute",
> [KEY_BRIGHTNESS_MIN] = "BrightnessMin",
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index 1ecc5ad57b56..f023f51b9c08 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -826,6 +826,14 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
> break;
> }
>
> + if ((usage->hid & 0xf0) == 0x90) { /* SystemControl*/
> + switch (usage->hid & 0xf) {
> + case 0xb: map_key_clear(KEY_DONOTDISTURB); break;
> + default: goto ignore;
> + }
> + break;
> + }
> +
> if ((usage->hid & 0xf0) == 0xa0) { /* SystemControl */
> switch (usage->hid & 0xf) {
> case 0x9: map_key_clear(KEY_MICMUTE); break;
> diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> index 7ff6eeef1af0..c971d542e525 100644
> --- a/include/uapi/linux/input-event-codes.h
> +++ b/include/uapi/linux/input-event-codes.h
> @@ -618,6 +618,7 @@
> #define KEY_CAMERA_ACCESS_DISABLE 0x24c /* Disables programmatic access to camera devices. (HUTRR72) */
> #define KEY_CAMERA_ACCESS_TOGGLE 0x24d /* Toggles the current state of the camera access control. (HUTRR72) */
> #define KEY_ACCESSIBILITY 0x24e /* Toggles the system bound accessibility UI/command (HUTRR116) */
> +#define KEY_DONOTDISTURB 0x24f /* Toggles the system-wide "Do Not Disturb" control (HUTRR94)*/
Please spare a few underscores: KEY_DO_NOT_DISTURB.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2 2/2] input: Add support for "Do Not Disturb"
From: Aseda Aboagye @ 2024-05-29 0:06 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Jiri Kosina, Benjamin Tissoires, linux-input
In-Reply-To: <ZlZpkrUuBonHDPaj@google.com>
On Tue, May 28, 2024 at 04:32:34PM -0700, Dmitry Torokhov wrote:
> On Tue, May 28, 2024 at 05:51:51PM -0500, Aseda Aboagye wrote:
> > HUTRR94 added support for a new usage titled "System Do Not Disturb"
> > which toggles a system-wide Do Not Disturb setting. This commit simply
> > adds a new event code for the usage.
> >
> > Signed-off-by: Aseda Aboagye <aaboagye@chromium.org>
> > ---
> > Changes from v1:
> > - Modified formatting to match existing code, ignoring checkpatch.pl.
> >
> > drivers/hid/hid-debug.c | 1 +
> > drivers/hid/hid-input.c | 8 ++++++++
> > include/uapi/linux/input-event-codes.h | 1 +
> > 3 files changed, 10 insertions(+)
> >
> > diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
> > index 7749c81b6227..78b2dd10cba2 100644
> > --- a/drivers/hid/hid-debug.c
> > +++ b/drivers/hid/hid-debug.c
> > @@ -975,6 +975,7 @@ static const char *keys[KEY_MAX + 1] = {
> > [KEY_CAMERA_ACCESS_DISABLE] = "CameraAccessDisable",
> > [KEY_CAMERA_ACCESS_TOGGLE] = "CameraAccessToggle",
> > [KEY_ACCESSIBILITY] = "Accessibility",
> > + [KEY_DONOTDISTURB] = "DoNotDisturb",
> > [KEY_DICTATE] = "Dictate",
> > [KEY_MICMUTE] = "MicrophoneMute",
> > [KEY_BRIGHTNESS_MIN] = "BrightnessMin",
> > diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> > index 1ecc5ad57b56..f023f51b9c08 100644
> > --- a/drivers/hid/hid-input.c
> > +++ b/drivers/hid/hid-input.c
> > @@ -826,6 +826,14 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
> > break;
> > }
> >
> > + if ((usage->hid & 0xf0) == 0x90) { /* SystemControl*/
> > + switch (usage->hid & 0xf) {
> > + case 0xb: map_key_clear(KEY_DONOTDISTURB); break;
> > + default: goto ignore;
> > + }
> > + break;
> > + }
> > +
> > if ((usage->hid & 0xf0) == 0xa0) { /* SystemControl */
> > switch (usage->hid & 0xf) {
> > case 0x9: map_key_clear(KEY_MICMUTE); break;
> > diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> > index 7ff6eeef1af0..c971d542e525 100644
> > --- a/include/uapi/linux/input-event-codes.h
> > +++ b/include/uapi/linux/input-event-codes.h
> > @@ -618,6 +618,7 @@
> > #define KEY_CAMERA_ACCESS_DISABLE 0x24c /* Disables programmatic access to camera devices. (HUTRR72) */
> > #define KEY_CAMERA_ACCESS_TOGGLE 0x24d /* Toggles the current state of the camera access control. (HUTRR72) */
> > #define KEY_ACCESSIBILITY 0x24e /* Toggles the system bound accessibility UI/command (HUTRR116) */
> > +#define KEY_DONOTDISTURB 0x24f /* Toggles the system-wide "Do Not Disturb" control (HUTRR94)*/
>
> Please spare a few underscores: KEY_DO_NOT_DISTURB.
>
My apologies, I didn't see your reply on v1 before I sent this. I will
address this in the next version.
>
> --
> Dmitry
--
Aseda Aboagye
^ permalink raw reply
* Re: [PATCH v2 1/2] input: Add event code for accessibility key
From: Aseda Aboagye @ 2024-05-29 0:08 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Jiri Kosina, Benjamin Tissoires, linux-input
In-Reply-To: <ZlZpN9ohRY-Qx7E4@google.com>
On Tue, May 28, 2024 at 04:31:03PM -0700, Dmitry Torokhov wrote:
> On Tue, May 28, 2024 at 05:44:01PM -0500, Aseda Aboagye wrote:
> > HUTRR116 added support for a new usage titled "System Accessibility
> > Binding" which toggles a system-wide bound accessibility UI or command.
> > This commit simply adds a new event code for the usage.
> >
> > Signed-off-by: Aseda Aboagye <aaboagye@chromium.org>
> > ---
> > Changes from v1:
> > - Modified formatting to match existing code, ignoring checkpatch.pl.
> >
> > drivers/hid/hid-debug.c | 1 +
> > drivers/hid/hid-input.c | 1 +
> > include/uapi/linux/input-event-codes.h | 1 +
> > 3 files changed, 3 insertions(+)
> >
> > diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
> > index e7ef1ea107c9..7749c81b6227 100644
> > --- a/drivers/hid/hid-debug.c
> > +++ b/drivers/hid/hid-debug.c
> > @@ -974,6 +974,7 @@ static const char *keys[KEY_MAX + 1] = {
> > [KEY_CAMERA_ACCESS_ENABLE] = "CameraAccessEnable",
> > [KEY_CAMERA_ACCESS_DISABLE] = "CameraAccessDisable",
> > [KEY_CAMERA_ACCESS_TOGGLE] = "CameraAccessToggle",
> > + [KEY_ACCESSIBILITY] = "Accessibility",
> > [KEY_DICTATE] = "Dictate",
> > [KEY_MICMUTE] = "MicrophoneMute",
> > [KEY_BRIGHTNESS_MIN] = "BrightnessMin",
> > diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> > index 851ee86eff32..1ecc5ad57b56 100644
> > --- a/drivers/hid/hid-input.c
> > +++ b/drivers/hid/hid-input.c
> > @@ -829,6 +829,7 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
> > if ((usage->hid & 0xf0) == 0xa0) { /* SystemControl */
> > switch (usage->hid & 0xf) {
> > case 0x9: map_key_clear(KEY_MICMUTE); break;
> > + case 0xa: map_key_clear(KEY_ACCESSIBILITY); break;
> > default: goto ignore;
> > }
> > break;
> > diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> > index 022a520e31fc..7ff6eeef1af0 100644
> > --- a/include/uapi/linux/input-event-codes.h
> > +++ b/include/uapi/linux/input-event-codes.h
> > @@ -617,6 +617,7 @@
> > #define KEY_CAMERA_ACCESS_ENABLE 0x24b /* Enables programmatic access to camera devices. (HUTRR72) */
> > #define KEY_CAMERA_ACCESS_DISABLE 0x24c /* Disables programmatic access to camera devices. (HUTRR72) */
> > #define KEY_CAMERA_ACCESS_TOGGLE 0x24d /* Toggles the current state of the camera access control. (HUTRR72) */
> > +#define KEY_ACCESSIBILITY 0x24e /* Toggles the system bound accessibility UI/command (HUTRR116) */
>
> For input:
>
> Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>
> Feel free to merge through HID tree.
>
Thank you so much for the review, I really appreciate it!
> Thanks.
>
> --
> Dmitry
--
Aseda Aboagye
^ permalink raw reply
* Re: [PATCH] hid-asus: use hid for brightness control on keyboard
From: Luke Jones @ 2024-05-29 0:37 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Jiri Kosina, Hans de Goede, corentin.chary, platform-driver-x86,
LKML, linux-input, bentiss
In-Reply-To: <17aa68e2-4af6-68ad-e81a-abc714517f6b@linux.intel.com>
On Tue, 28 May 2024, at 8:36 PM, Ilpo Järvinen wrote:
> On Tue, 28 May 2024, Luke D. Jones wrote:
>
> > On almost all ASUS ROG series laptops the MCU used for the USB keyboard
> > also has a HID packet used for setting the brightness. This is usually
> > the same as the WMI method. But in some laptops the WMI method either
> > is missing or doesn't work, so we should default to the HID control.
> >
> > Signed-off-by: Luke D. Jones <luke@ljones.dev>
> > ---
> > drivers/hid/hid-asus.c | 19 ++++++++++++-
> > drivers/platform/x86/asus-wmi.c | 3 ++-
> > include/linux/platform_data/x86/asus-wmi.h | 31 ++++++++++++++++++++++
> > 3 files changed, 51 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> > index 02de2bf4f790..9389a3e733e3 100644
> > --- a/drivers/hid/hid-asus.c
> > +++ b/drivers/hid/hid-asus.c
> > @@ -101,6 +101,7 @@ struct asus_kbd_leds {
> > unsigned int brightness;
> > spinlock_t lock;
> > bool removed;
> > + int report_id;
> > };
> >
> > struct asus_touchpad_info {
> > @@ -473,7 +474,7 @@ static enum led_brightness asus_kbd_backlight_get(struct led_classdev *led_cdev)
> > static void asus_kbd_backlight_work(struct work_struct *work)
> > {
> > struct asus_kbd_leds *led = container_of(work, struct asus_kbd_leds, work);
> > - u8 buf[] = { FEATURE_KBD_REPORT_ID, 0xba, 0xc5, 0xc4, 0x00 };
> > + u8 buf[] = { led->report_id, 0xba, 0xc5, 0xc4, 0x00 };
> > int ret;
> > unsigned long flags;
> >
> > @@ -492,12 +493,18 @@ static void asus_kbd_backlight_work(struct work_struct *work)
> > */
> > static bool asus_kbd_wmi_led_control_present(struct hid_device *hdev)
> > {
> > + struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
> > u32 value;
> > int ret;
> >
> > if (!IS_ENABLED(CONFIG_ASUS_WMI))
> > return false;
> >
> > + if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD && asus_use_hidraw_led()) {
> > + hid_info(hdev, "using hidraw for asus::kbd_backlight\n");
> > + return false;
> > + }
> > +
> > ret = asus_wmi_evaluate_method(ASUS_WMI_METHODID_DSTS,
> > ASUS_WMI_DEVID_KBD_BACKLIGHT, 0, &value);
> > hid_dbg(hdev, "WMI backlight check: rc %d value %x", ret, value);
> > @@ -507,6 +514,12 @@ static bool asus_kbd_wmi_led_control_present(struct hid_device *hdev)
> > return !!(value & ASUS_WMI_DSTS_PRESENCE_BIT);
> > }
> >
> > +static bool asus_kbd_is_input_led(void)
> > +{
> > + return dmi_match(DMI_PRODUCT_NAME, "GU605")
> > + || dmi_match(DMI_PRODUCT_NAME, "GA403");
> > +}
> > +
> > static int asus_kbd_register_leds(struct hid_device *hdev)
> > {
> > struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
> > @@ -549,6 +562,10 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
> > if (!drvdata->kbd_backlight)
> > return -ENOMEM;
> >
> > + drvdata->kbd_backlight->report_id = FEATURE_KBD_REPORT_ID;
> > + if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD && asus_kbd_is_input_led())
> > + drvdata->kbd_backlight->report_id = FEATURE_KBD_LED_REPORT_ID1;
> > +
> > drvdata->kbd_backlight->removed = false;
> > drvdata->kbd_backlight->brightness = 0;
> > drvdata->kbd_backlight->hdev = hdev;
> > diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> > index 3f9b6285c9a6..a58df18a70ad 100644
> > --- a/drivers/platform/x86/asus-wmi.c
> > +++ b/drivers/platform/x86/asus-wmi.c
> > @@ -1681,7 +1681,8 @@ static int asus_wmi_led_init(struct asus_wmi *asus)
> > goto error;
> > }
> >
> > - if (!kbd_led_read(asus, &led_val, NULL)) {
> > + if (!kbd_led_read(asus, &led_val, NULL) && !asus_use_hidraw_led()) {
> > + pr_info("using asus-wmi for asus::kbd_backlight\n");
> > asus->kbd_led_wk = led_val;
> > asus->kbd_led.name = "asus::kbd_backlight";
> > asus->kbd_led.flags = LED_BRIGHT_HW_CHANGED;
> > diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
> > index 3eb5cd6773ad..79a50102440d 100644
> > --- a/include/linux/platform_data/x86/asus-wmi.h
> > +++ b/include/linux/platform_data/x86/asus-wmi.h
> > @@ -160,4 +160,35 @@ static inline int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1,
> > }
> > #endif
> >
> > +/* To be used by both hid-asus and asus-wmi to determine which controls kbd_brightness */
> > +#if IS_REACHABLE(CONFIG_ASUS_WMI)
> > +static bool asus_use_hidraw_led(void)
>
> Since it's in a header, it's missing inline. However, this function looks
> quite complicated so putting it into a header file is questionable to
> begin with so I'd prefer it to be in a .c file.
Thanks for the review y'all. All recommendations implemented including this and splitting to two commits.
> > +{
> > + const char *product, *board;
> > +
> > + product = dmi_get_system_info(DMI_PRODUCT_FAMILY);
> > + if (!product)
> > + return false;
> > +
> > + /* These product ranges should all be using HID for keyboard LED */
> > + if (strstr(product, "ROG Zephyrus")
> > + || strstr(product, "ROG Strix")
> > + || strstr(product, "ROG Flow")
> > + || strstr(product, "GA403")
> > + || strstr(product, "GU605"))
>
> Please align these properly but consider using array and loop.
>
> > + return true;
> > +
> > + board = dmi_get_system_info(DMI_BOARD_NAME);
> > + if (!board)
> > + return false;
> > +
> > + return strstr(board, "RC71L"); /* ROG Ally specific */
> > +}
> > +#else
> > +static inline bool asus_use_hidraw_led(void)
> > +{
> > + return true;
> > +}
> > +#endif
> > +
> > #endif /* __PLATFORM_DATA_X86_ASUS_WMI_H */
> >
>
> --
> i.
>
>
^ permalink raw reply
* Re: [PATCH] Adding quirks for 2024 HP Spectre x360 touchpads
From: Jon Moeller @ 2024-05-28 18:21 UTC (permalink / raw)
To: Jon Moeller; +Cc: linux-input
In-Reply-To: <20240429070950.247183-1-jon@moeller.io>
Is there anything special I need to do to get this merged? I was
hoping this would get worked into 6.10.
On Mon, Apr 29, 2024 at 12:09 AM Jon Moeller <jon@moeller.io> wrote:
>
> ---
> drivers/hid/hid-multitouch.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index 04a014cd2a2f..7a7918191628 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -212,6 +212,7 @@ static void mt_post_parse(struct mt_device *td, struct mt_application *app);
> #define MT_CLS_GOOGLE 0x0111
> #define MT_CLS_RAZER_BLADE_STEALTH 0x0112
> #define MT_CLS_SMART_TECH 0x0113
> +#define MT_CLS_HP_SPECTRE_ELAN_HAPTIC 0x0114
>
> #define MT_DEFAULT_MAXCONTACT 10
> #define MT_MAX_MAXCONTACT 250
> @@ -396,6 +397,13 @@ static const struct mt_class mt_classes[] = {
> MT_QUIRK_CONTACT_CNT_ACCURATE |
> MT_QUIRK_SEPARATE_APP_REPORT,
> },
> + { .name = MT_CLS_HP_SPECTRE_ELAN_HAPTIC,
> + .quirks = MT_QUIRK_ALWAYS_VALID |
> + MT_QUIRK_SLOT_IS_CONTACTID |
> + MT_QUIRK_CONTACT_CNT_ACCURATE |
> + MT_QUIRK_CONFIDENCE |
> + MT_QUIRK_WIN8_PTP_BUTTONS,
> + },
> { }
> };
>
> @@ -1992,6 +2000,12 @@ static const struct hid_device_id mt_devices[] = {
> HID_DEVICE(BUS_I2C, HID_GROUP_MULTITOUCH_WIN_8,
> USB_VENDOR_ID_ELAN, 0x3148) },
>
> + { .driver_data = MT_CLS_HP_SPECTRE_ELAN_HAPTIC,
> + HID_I2C_DEVICE(USB_VENDOR_ID_ELAN, 0x32c8) },
> +
> + { .driver_data = MT_CLS_HP_SPECTRE_ELAN_HAPTIC,
> + HID_I2C_DEVICE(USB_VENDOR_ID_ELAN, 0x310a) },
> +
> /* Elitegroup panel */
> { .driver_data = MT_CLS_SERIAL,
> MT_USB_DEVICE(USB_VENDOR_ID_ELITEGROUP,
> --
> 2.44.0
>
> Signed-off-by: Jon Moeller <jon@moeller.io>
^ permalink raw reply
* [PATCH v1 1/2] hid-asus: use hid for brightness control on keyboard
From: Luke D. Jones @ 2024-05-29 1:24 UTC (permalink / raw)
To: jikos
Cc: hdegoede, ilpo.jarvinen, corentin.chary, platform-driver-x86,
linux-kernel, linux-input, bentiss, Luke D. Jones
On almost all ASUS ROG series laptops the MCU used for the USB keyboard
also has a HID packet used for setting the brightness. This is usually
the same as the WMI method. But in some laptops the WMI method either
is missing or doesn't work, so we should default to the HID control.
Signed-off-by: Luke D. Jones <luke@ljones.dev>
---
drivers/hid/hid-asus.c | 6 ++++
drivers/platform/x86/asus-wmi.c | 35 +++++++++++++++++++++-
include/linux/platform_data/x86/asus-wmi.h | 10 +++++++
3 files changed, 50 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 02de2bf4f790..4cba8e143031 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -492,12 +492,18 @@ static void asus_kbd_backlight_work(struct work_struct *work)
*/
static bool asus_kbd_wmi_led_control_present(struct hid_device *hdev)
{
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
u32 value;
int ret;
if (!IS_ENABLED(CONFIG_ASUS_WMI))
return false;
+ if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD && asus_use_hid_led()) {
+ hid_info(hdev, "using HID for asus::kbd_backlight\n");
+ return false;
+ }
+
ret = asus_wmi_evaluate_method(ASUS_WMI_METHODID_DSTS,
ASUS_WMI_DEVID_KBD_BACKLIGHT, 0, &value);
hid_dbg(hdev, "WMI backlight check: rc %d value %x", ret, value);
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 3f9b6285c9a6..54cb07c79fcf 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -144,6 +144,15 @@ module_param(fnlock_default, bool, 0444);
static const char * const ashs_ids[] = { "ATK4001", "ATK4002", NULL };
+static const char * const use_hid_led_matches[] = {
+ "ROG Zephyrus",
+ "ROG Strix",
+ "ROG Flow",
+ "GA403",
+ "GU605",
+ "RC71L",
+};
+
static int throttle_thermal_policy_write(struct asus_wmi *);
static bool ashs_present(void)
@@ -1642,6 +1651,29 @@ static int micmute_led_set(struct led_classdev *led_cdev,
return err < 0 ? err : 0;
}
+bool asus_use_hid_led(void)
+{
+ const char *product, *board;
+ int i;
+
+ product = dmi_get_system_info(DMI_PRODUCT_FAMILY);
+ if (!product)
+ return false;
+
+ board = dmi_get_system_info(DMI_BOARD_NAME);
+ if (!board)
+ return false;
+
+ for (i = 0; i < ARRAY_SIZE(use_hid_led_matches); i++) {
+ if (strstr(product, use_hid_led_matches[i]))
+ return true;
+ if (strstr(board, use_hid_led_matches[i]))
+ return true;
+ }
+ return false;
+}
+EXPORT_SYMBOL_GPL(asus_use_hid_led);
+
static void asus_wmi_led_exit(struct asus_wmi *asus)
{
led_classdev_unregister(&asus->kbd_led);
@@ -1681,7 +1713,8 @@ static int asus_wmi_led_init(struct asus_wmi *asus)
goto error;
}
- if (!kbd_led_read(asus, &led_val, NULL)) {
+ if (!kbd_led_read(asus, &led_val, NULL) && !asus_use_hid_led()) {
+ pr_info("using asus-wmi for asus::kbd_backlight\n");
asus->kbd_led_wk = led_val;
asus->kbd_led.name = "asus::kbd_backlight";
asus->kbd_led.flags = LED_BRIGHT_HW_CHANGED;
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index 3eb5cd6773ad..6833035f7006 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -160,4 +160,14 @@ static inline int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1,
}
#endif
+/* To be used by both hid-asus and asus-wmi to determine which controls kbd_brightness */
+#if IS_ENABLED(CONFIG_ASUS_WMI)
+bool asus_use_hid_led(void);
+#else
+static inline bool asus_use_hid_led(void)
+{
+ return true;
+}
+#endif
+
#endif /* __PLATFORM_DATA_X86_ASUS_WMI_H */
--
2.45.1
^ permalink raw reply related
* [PATCH v1 2/2] hid-asus: change the report_id used for HID LED control
From: Luke D. Jones @ 2024-05-29 1:24 UTC (permalink / raw)
To: jikos
Cc: hdegoede, ilpo.jarvinen, corentin.chary, platform-driver-x86,
linux-kernel, linux-input, bentiss, Luke D. Jones
In-Reply-To: <20240529012447.145088-1-luke@ljones.dev>
On some laptops the report_id used for LED brightness control must be
0x5D instead of 0x5A.
Signed-off-by: Luke D. Jones <luke@ljones.dev>
---
drivers/hid/hid-asus.c | 26 +++++++++++++++++++++++++-
1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 4cba8e143031..ec3556cc4eef 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -94,6 +94,8 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
#define TRKID_SGN ((TRKID_MAX + 1) >> 1)
+static const char * const use_alt_led_report_id[] = { "GU605", "GA403" };
+
struct asus_kbd_leds {
struct led_classdev cdev;
struct hid_device *hdev;
@@ -101,6 +103,7 @@ struct asus_kbd_leds {
unsigned int brightness;
spinlock_t lock;
bool removed;
+ int report_id;
};
struct asus_touchpad_info {
@@ -473,7 +476,7 @@ static enum led_brightness asus_kbd_backlight_get(struct led_classdev *led_cdev)
static void asus_kbd_backlight_work(struct work_struct *work)
{
struct asus_kbd_leds *led = container_of(work, struct asus_kbd_leds, work);
- u8 buf[] = { FEATURE_KBD_REPORT_ID, 0xba, 0xc5, 0xc4, 0x00 };
+ u8 buf[] = { led->report_id, 0xba, 0xc5, 0xc4, 0x00 };
int ret;
unsigned long flags;
@@ -513,6 +516,23 @@ static bool asus_kbd_wmi_led_control_present(struct hid_device *hdev)
return !!(value & ASUS_WMI_DSTS_PRESENCE_BIT);
}
+static bool asus_kbd_is_input_led(void)
+{
+ const char *product;
+ int i;
+
+ product = dmi_get_system_info(DMI_PRODUCT_NAME);
+ if (!product)
+ return false;
+
+ for (i = 0; i < ARRAY_SIZE(use_alt_led_report_id); i++) {
+ if (strstr(product, use_alt_led_report_id[i]))
+ return true;
+ }
+
+ return false;
+}
+
static int asus_kbd_register_leds(struct hid_device *hdev)
{
struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
@@ -555,6 +575,10 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
if (!drvdata->kbd_backlight)
return -ENOMEM;
+ drvdata->kbd_backlight->report_id = FEATURE_KBD_REPORT_ID;
+ if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD && asus_kbd_is_input_led())
+ drvdata->kbd_backlight->report_id = FEATURE_KBD_LED_REPORT_ID1;
+
drvdata->kbd_backlight->removed = false;
drvdata->kbd_backlight->brightness = 0;
drvdata->kbd_backlight->hdev = hdev;
--
2.45.1
^ permalink raw reply related
* [PATCH v1 0/2] asus wmi and hid: use HID LED for brightness
From: Luke D. Jones @ 2024-05-29 1:28 UTC (permalink / raw)
To: jikos
Cc: hdegoede, ilpo.jarvinen, corentin.chary, platform-driver-x86,
linux-kernel, linux-input, bentiss, Luke D. Jones
Changelog:
- v1
- Split the patch in two
- Move function body to asus-wmi and export
- Use array of names and for loops
History:
- https://lore.kernel.org/linux-input/20240528013959.14661-1-luke@ljones.dev/T/#u
Luke D. Jones (2):
hid-asus: use hid for brightness control on keyboard
hid-asus: change the report_id used for HID LED control
drivers/hid/hid-asus.c | 32 +++++++++++++++++++-
drivers/platform/x86/asus-wmi.c | 35 +++++++++++++++++++++-
include/linux/platform_data/x86/asus-wmi.h | 10 +++++++
3 files changed, 75 insertions(+), 2 deletions(-)
--
2.45.1
^ permalink raw reply
* [PATCH v1 1/2] hid-asus: use hid for brightness control on keyboard
From: Luke D. Jones @ 2024-05-29 1:28 UTC (permalink / raw)
To: jikos
Cc: hdegoede, ilpo.jarvinen, corentin.chary, platform-driver-x86,
linux-kernel, linux-input, bentiss, Luke D. Jones
In-Reply-To: <20240529012827.146005-1-luke@ljones.dev>
On almost all ASUS ROG series laptops the MCU used for the USB keyboard
also has a HID packet used for setting the brightness. This is usually
the same as the WMI method. But in some laptops the WMI method either
is missing or doesn't work, so we should default to the HID control.
Signed-off-by: Luke D. Jones <luke@ljones.dev>
---
drivers/hid/hid-asus.c | 6 ++++
drivers/platform/x86/asus-wmi.c | 35 +++++++++++++++++++++-
include/linux/platform_data/x86/asus-wmi.h | 10 +++++++
3 files changed, 50 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 02de2bf4f790..4cba8e143031 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -492,12 +492,18 @@ static void asus_kbd_backlight_work(struct work_struct *work)
*/
static bool asus_kbd_wmi_led_control_present(struct hid_device *hdev)
{
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
u32 value;
int ret;
if (!IS_ENABLED(CONFIG_ASUS_WMI))
return false;
+ if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD && asus_use_hid_led()) {
+ hid_info(hdev, "using HID for asus::kbd_backlight\n");
+ return false;
+ }
+
ret = asus_wmi_evaluate_method(ASUS_WMI_METHODID_DSTS,
ASUS_WMI_DEVID_KBD_BACKLIGHT, 0, &value);
hid_dbg(hdev, "WMI backlight check: rc %d value %x", ret, value);
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 3f9b6285c9a6..54cb07c79fcf 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -144,6 +144,15 @@ module_param(fnlock_default, bool, 0444);
static const char * const ashs_ids[] = { "ATK4001", "ATK4002", NULL };
+static const char * const use_hid_led_matches[] = {
+ "ROG Zephyrus",
+ "ROG Strix",
+ "ROG Flow",
+ "GA403",
+ "GU605",
+ "RC71L",
+};
+
static int throttle_thermal_policy_write(struct asus_wmi *);
static bool ashs_present(void)
@@ -1642,6 +1651,29 @@ static int micmute_led_set(struct led_classdev *led_cdev,
return err < 0 ? err : 0;
}
+bool asus_use_hid_led(void)
+{
+ const char *product, *board;
+ int i;
+
+ product = dmi_get_system_info(DMI_PRODUCT_FAMILY);
+ if (!product)
+ return false;
+
+ board = dmi_get_system_info(DMI_BOARD_NAME);
+ if (!board)
+ return false;
+
+ for (i = 0; i < ARRAY_SIZE(use_hid_led_matches); i++) {
+ if (strstr(product, use_hid_led_matches[i]))
+ return true;
+ if (strstr(board, use_hid_led_matches[i]))
+ return true;
+ }
+ return false;
+}
+EXPORT_SYMBOL_GPL(asus_use_hid_led);
+
static void asus_wmi_led_exit(struct asus_wmi *asus)
{
led_classdev_unregister(&asus->kbd_led);
@@ -1681,7 +1713,8 @@ static int asus_wmi_led_init(struct asus_wmi *asus)
goto error;
}
- if (!kbd_led_read(asus, &led_val, NULL)) {
+ if (!kbd_led_read(asus, &led_val, NULL) && !asus_use_hid_led()) {
+ pr_info("using asus-wmi for asus::kbd_backlight\n");
asus->kbd_led_wk = led_val;
asus->kbd_led.name = "asus::kbd_backlight";
asus->kbd_led.flags = LED_BRIGHT_HW_CHANGED;
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index 3eb5cd6773ad..6833035f7006 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -160,4 +160,14 @@ static inline int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1,
}
#endif
+/* To be used by both hid-asus and asus-wmi to determine which controls kbd_brightness */
+#if IS_ENABLED(CONFIG_ASUS_WMI)
+bool asus_use_hid_led(void);
+#else
+static inline bool asus_use_hid_led(void)
+{
+ return true;
+}
+#endif
+
#endif /* __PLATFORM_DATA_X86_ASUS_WMI_H */
--
2.45.1
^ permalink raw reply related
* [PATCH v1 2/2] hid-asus: change the report_id used for HID LED control
From: Luke D. Jones @ 2024-05-29 1:28 UTC (permalink / raw)
To: jikos
Cc: hdegoede, ilpo.jarvinen, corentin.chary, platform-driver-x86,
linux-kernel, linux-input, bentiss, Luke D. Jones
In-Reply-To: <20240529012827.146005-1-luke@ljones.dev>
On some laptops the report_id used for LED brightness control must be
0x5D instead of 0x5A.
Signed-off-by: Luke D. Jones <luke@ljones.dev>
---
drivers/hid/hid-asus.c | 26 +++++++++++++++++++++++++-
1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 4cba8e143031..ec3556cc4eef 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -94,6 +94,8 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
#define TRKID_SGN ((TRKID_MAX + 1) >> 1)
+static const char * const use_alt_led_report_id[] = { "GU605", "GA403" };
+
struct asus_kbd_leds {
struct led_classdev cdev;
struct hid_device *hdev;
@@ -101,6 +103,7 @@ struct asus_kbd_leds {
unsigned int brightness;
spinlock_t lock;
bool removed;
+ int report_id;
};
struct asus_touchpad_info {
@@ -473,7 +476,7 @@ static enum led_brightness asus_kbd_backlight_get(struct led_classdev *led_cdev)
static void asus_kbd_backlight_work(struct work_struct *work)
{
struct asus_kbd_leds *led = container_of(work, struct asus_kbd_leds, work);
- u8 buf[] = { FEATURE_KBD_REPORT_ID, 0xba, 0xc5, 0xc4, 0x00 };
+ u8 buf[] = { led->report_id, 0xba, 0xc5, 0xc4, 0x00 };
int ret;
unsigned long flags;
@@ -513,6 +516,23 @@ static bool asus_kbd_wmi_led_control_present(struct hid_device *hdev)
return !!(value & ASUS_WMI_DSTS_PRESENCE_BIT);
}
+static bool asus_kbd_is_input_led(void)
+{
+ const char *product;
+ int i;
+
+ product = dmi_get_system_info(DMI_PRODUCT_NAME);
+ if (!product)
+ return false;
+
+ for (i = 0; i < ARRAY_SIZE(use_alt_led_report_id); i++) {
+ if (strstr(product, use_alt_led_report_id[i]))
+ return true;
+ }
+
+ return false;
+}
+
static int asus_kbd_register_leds(struct hid_device *hdev)
{
struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
@@ -555,6 +575,10 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
if (!drvdata->kbd_backlight)
return -ENOMEM;
+ drvdata->kbd_backlight->report_id = FEATURE_KBD_REPORT_ID;
+ if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD && asus_kbd_is_input_led())
+ drvdata->kbd_backlight->report_id = FEATURE_KBD_LED_REPORT_ID1;
+
drvdata->kbd_backlight->removed = false;
drvdata->kbd_backlight->brightness = 0;
drvdata->kbd_backlight->hdev = hdev;
--
2.45.1
^ permalink raw reply related
* Re: [PATCH v1 2/2] hid-asus: change the report_id used for HID LED control
From: Luke Jones @ 2024-05-29 1:30 UTC (permalink / raw)
To: Jiri Kosina
Cc: Hans de Goede, Ilpo Järvinen, corentin.chary,
platform-driver-x86, linux-kernel, linux-input, bentiss
In-Reply-To: <20240529012447.145088-2-luke@ljones.dev>
Sorry about the doubleup. Wifi is spotty and it looked like the initial send didn't work.
See https://lore.kernel.org/linux-input/20240529012827.146005-1-luke@ljones.dev/T/#t for cover-letter
On Wed, 29 May 2024, at 1:24 PM, Luke D. Jones wrote:
> On some laptops the report_id used for LED brightness control must be
> 0x5D instead of 0x5A.
>
> Signed-off-by: Luke D. Jones <luke@ljones.dev>
> ---
> drivers/hid/hid-asus.c | 26 +++++++++++++++++++++++++-
> 1 file changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 4cba8e143031..ec3556cc4eef 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -94,6 +94,8 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
>
> #define TRKID_SGN ((TRKID_MAX + 1) >> 1)
>
> +static const char * const use_alt_led_report_id[] = { "GU605", "GA403" };
> +
> struct asus_kbd_leds {
> struct led_classdev cdev;
> struct hid_device *hdev;
> @@ -101,6 +103,7 @@ struct asus_kbd_leds {
> unsigned int brightness;
> spinlock_t lock;
> bool removed;
> + int report_id;
> };
>
> struct asus_touchpad_info {
> @@ -473,7 +476,7 @@ static enum led_brightness asus_kbd_backlight_get(struct led_classdev *led_cdev)
> static void asus_kbd_backlight_work(struct work_struct *work)
> {
> struct asus_kbd_leds *led = container_of(work, struct asus_kbd_leds, work);
> - u8 buf[] = { FEATURE_KBD_REPORT_ID, 0xba, 0xc5, 0xc4, 0x00 };
> + u8 buf[] = { led->report_id, 0xba, 0xc5, 0xc4, 0x00 };
> int ret;
> unsigned long flags;
>
> @@ -513,6 +516,23 @@ static bool asus_kbd_wmi_led_control_present(struct hid_device *hdev)
> return !!(value & ASUS_WMI_DSTS_PRESENCE_BIT);
> }
>
> +static bool asus_kbd_is_input_led(void)
> +{
> + const char *product;
> + int i;
> +
> + product = dmi_get_system_info(DMI_PRODUCT_NAME);
> + if (!product)
> + return false;
> +
> + for (i = 0; i < ARRAY_SIZE(use_alt_led_report_id); i++) {
> + if (strstr(product, use_alt_led_report_id[i]))
> + return true;
> + }
> +
> + return false;
> +}
> +
> static int asus_kbd_register_leds(struct hid_device *hdev)
> {
> struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
> @@ -555,6 +575,10 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
> if (!drvdata->kbd_backlight)
> return -ENOMEM;
>
> + drvdata->kbd_backlight->report_id = FEATURE_KBD_REPORT_ID;
> + if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD && asus_kbd_is_input_led())
> + drvdata->kbd_backlight->report_id = FEATURE_KBD_LED_REPORT_ID1;
> +
> drvdata->kbd_backlight->removed = false;
> drvdata->kbd_backlight->brightness = 0;
> drvdata->kbd_backlight->hdev = hdev;
> --
> 2.45.1
>
>
^ permalink raw reply
* Re: [PATCH HID 03/13] HID: bpf: implement HID-BPF through bpf_struct_ops
From: Alexei Starovoitov @ 2024-05-29 4:02 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Shuah Khan, Jiri Kosina, Jonathan Corbet, Alexei Starovoitov,
open list:KERNEL SELFTEST FRAMEWORK, LKML, bpf,
open list:HID CORE LAYER, open list:DOCUMENTATION
In-Reply-To: <20240528-hid_bpf_struct_ops-v1-3-8c6663df27d8@kernel.org>
On Tue, May 28, 2024 at 6:15 AM Benjamin Tissoires <bentiss@kernel.org> wrote:
>
> We do this implementation in several steps to not have the CI failing:
> - first (this patch), we add struct_ops while keeping the existing infra
> available
> - then we change the selftests, the examples and the existing in-tree
> HID-BPF programs
> - then we remove the existing trace points making old HID-BPF obsolete
>
> There are a few advantages of struct_ops over tracing:
> - compatibility with sleepable programs (for hid_hw_raw_request() in
> a later patch)
> - a lot simpler in the kernel: it's a simple rcu protected list
> - we can add more parameters to the function called without much trouble
> - the "attach" is now generic through BPF-core: the caller just needs to
> set hid_id and flags before calling __load().
> - all the BPF tough part is not handled in BPF-core through generic
> processing
> - hid_bpf_ctx is now only writable where it needs be
>
> Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
> ---
> drivers/hid/bpf/Makefile | 2 +-
> drivers/hid/bpf/hid_bpf_dispatch.c | 52 +++++++-
> drivers/hid/bpf/hid_bpf_dispatch.h | 4 +
> drivers/hid/bpf/hid_bpf_jmp_table.c | 3 +
> drivers/hid/bpf/hid_bpf_struct_ops.c | 246 +++++++++++++++++++++++++++++++++++
> include/linux/hid_bpf.h | 64 ++++++++-
> 6 files changed, 362 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/hid/bpf/Makefile b/drivers/hid/bpf/Makefile
> index cf55120cf7d6..1cb3f31e9335 100644
> --- a/drivers/hid/bpf/Makefile
> +++ b/drivers/hid/bpf/Makefile
> @@ -8,4 +8,4 @@ LIBBPF_INCLUDE = $(srctree)/tools/lib
> obj-$(CONFIG_HID_BPF) += hid_bpf.o
> CFLAGS_hid_bpf_dispatch.o += -I$(LIBBPF_INCLUDE)
> CFLAGS_hid_bpf_jmp_table.o += -I$(LIBBPF_INCLUDE)
> -hid_bpf-objs += hid_bpf_dispatch.o hid_bpf_jmp_table.o
> +hid_bpf-objs += hid_bpf_dispatch.o hid_bpf_jmp_table.o hid_bpf_struct_ops.o
> diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c
> index c8bb79ce2354..7216c3c7713d 100644
> --- a/drivers/hid/bpf/hid_bpf_dispatch.c
> +++ b/drivers/hid/bpf/hid_bpf_dispatch.c
> @@ -58,6 +58,7 @@ dispatch_hid_bpf_device_event(struct hid_device *hdev, enum hid_report_type type
> },
> .data = hdev->bpf.device_data,
> };
> + struct hid_bpf_ops *e;
> int ret;
>
> if (type >= HID_REPORT_TYPES)
> @@ -70,9 +71,25 @@ dispatch_hid_bpf_device_event(struct hid_device *hdev, enum hid_report_type type
> memset(ctx_kern.data, 0, hdev->bpf.allocated_data);
> memcpy(ctx_kern.data, data, *size);
>
> + rcu_read_lock();
> + list_for_each_entry_rcu(e, &hdev->bpf.prog_list, list) {
In the typical case there will be only one prog per device, right?
The for_each is future proofing?
> + if (e->hid_device_event) {
> + ret = e->hid_device_event(&ctx_kern.ctx, type);
> + if (ret < 0) {
> + rcu_read_unlock();
> + return ERR_PTR(ret);
> + }
> +
> + if (ret)
> + ctx_kern.ctx.retval = ret;
> + }
> + }
> + rcu_read_unlock();
> +
> ret = hid_bpf_prog_run(hdev, HID_BPF_PROG_TYPE_DEVICE_EVENT, &ctx_kern);
> if (ret < 0)
> return ERR_PTR(ret);
> + ret = ctx_kern.ctx.retval;
>
> if (ret) {
> if (ret > ctx_kern.ctx.allocated_size)
> @@ -122,7 +139,10 @@ u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, u8 *rdesc, unsigned int *s
>
> memcpy(ctx_kern.data, rdesc, min_t(unsigned int, *size, HID_MAX_DESCRIPTOR_SIZE));
>
> - ret = hid_bpf_prog_run(hdev, HID_BPF_PROG_TYPE_RDESC_FIXUP, &ctx_kern);
> + if (hdev->bpf.rdesc_ops)
> + ret = hdev->bpf.rdesc_ops->hid_rdesc_fixup(&ctx_kern.ctx);
> + else
> + ret = hid_bpf_prog_run(hdev, HID_BPF_PROG_TYPE_RDESC_FIXUP, &ctx_kern);
This is for backward compat?
I don't see it's being removed in the later patches.
> if (ret < 0)
> goto ignore_bpf;
>
> @@ -150,7 +170,7 @@ static int device_match_id(struct device *dev, const void *id)
> return hdev->id == *(int *)id;
> }
>
> -static struct hid_device *hid_get_device(unsigned int hid_id)
> +struct hid_device *hid_get_device(unsigned int hid_id)
> {
> struct device *dev;
>
> @@ -164,7 +184,7 @@ static struct hid_device *hid_get_device(unsigned int hid_id)
> return to_hid_device(dev);
> }
>
> -static void hid_put_device(struct hid_device *hid)
> +void hid_put_device(struct hid_device *hid)
> {
> put_device(&hid->dev);
> }
> @@ -205,7 +225,7 @@ static int __hid_bpf_allocate_data(struct hid_device *hdev, u8 **data, u32 *size
> return 0;
> }
>
> -static int hid_bpf_allocate_event_data(struct hid_device *hdev)
> +int hid_bpf_allocate_event_data(struct hid_device *hdev)
> {
> /* hdev->bpf.device_data is already allocated, abort */
> if (hdev->bpf.device_data)
> @@ -592,14 +612,22 @@ static const struct btf_kfunc_id_set hid_bpf_syscall_kfunc_set = {
>
> int hid_bpf_connect_device(struct hid_device *hdev)
> {
> - struct hid_bpf_prog_list *prog_list;
> + bool need_to_allocate = false;
> + struct hid_bpf_ops *e;
>
> rcu_read_lock();
> - prog_list = rcu_dereference(hdev->bpf.progs[HID_BPF_PROG_TYPE_DEVICE_EVENT]);
> + list_for_each_entry_rcu(e, &hdev->bpf.prog_list, list) {
> + if (e->hid_device_event) {
> + need_to_allocate = true;
> + break;
> + }
> + }
> + if (rcu_dereference(hdev->bpf.progs[HID_BPF_PROG_TYPE_DEVICE_EVENT]))
> + need_to_allocate = true;
> rcu_read_unlock();
>
> /* only allocate BPF data if there are programs attached */
> - if (!prog_list)
> + if (!need_to_allocate)
> return 0;
>
> return hid_bpf_allocate_event_data(hdev);
> @@ -623,12 +651,15 @@ void hid_bpf_destroy_device(struct hid_device *hdev)
> hdev->bpf.destroyed = true;
>
> __hid_bpf_destroy_device(hdev);
> + __hid_bpf_ops_destroy_device(hdev);
> }
> EXPORT_SYMBOL_GPL(hid_bpf_destroy_device);
>
> void hid_bpf_device_init(struct hid_device *hdev)
> {
> spin_lock_init(&hdev->bpf.progs_lock);
> + INIT_LIST_HEAD(&hdev->bpf.prog_list);
> + mutex_init(&hdev->bpf.prog_list_lock);
> }
> EXPORT_SYMBOL_GPL(hid_bpf_device_init);
>
> @@ -662,6 +693,13 @@ static int __init hid_bpf_init(void)
> return 0;
> }
>
> + /* register struct_ops kfuncs after we are sure we can load our preloaded bpf program */
> + err = register_btf_kfunc_id_set(BPF_PROG_TYPE_STRUCT_OPS, &hid_bpf_kfunc_set);
> + if (err) {
> + pr_warn("error while setting HID BPF tracing kfuncs: %d", err);
> + return 0;
> + }
> +
> /* register syscalls after we are sure we can load our preloaded bpf program */
> err = register_btf_kfunc_id_set(BPF_PROG_TYPE_SYSCALL, &hid_bpf_syscall_kfunc_set);
> if (err) {
> diff --git a/drivers/hid/bpf/hid_bpf_dispatch.h b/drivers/hid/bpf/hid_bpf_dispatch.h
> index fbe0639d09f2..e52c43d81650 100644
> --- a/drivers/hid/bpf/hid_bpf_dispatch.h
> +++ b/drivers/hid/bpf/hid_bpf_dispatch.h
> @@ -10,12 +10,16 @@ struct hid_bpf_ctx_kern {
> u8 *data;
> };
>
> +struct hid_device *hid_get_device(unsigned int hid_id);
> +void hid_put_device(struct hid_device *hid);
> +int hid_bpf_allocate_event_data(struct hid_device *hdev);
> int hid_bpf_preload_skel(void);
> void hid_bpf_free_links_and_skel(void);
> 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,
> struct bpf_prog *prog, __u32 flags);
> void __hid_bpf_destroy_device(struct hid_device *hdev);
> +void __hid_bpf_ops_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);
> int hid_bpf_reconnect(struct hid_device *hdev);
> diff --git a/drivers/hid/bpf/hid_bpf_jmp_table.c b/drivers/hid/bpf/hid_bpf_jmp_table.c
> index aa8e1c79cdf5..8a54ba447718 100644
> --- a/drivers/hid/bpf/hid_bpf_jmp_table.c
> +++ b/drivers/hid/bpf/hid_bpf_jmp_table.c
> @@ -81,6 +81,9 @@ static int hid_bpf_program_count(struct hid_device *hdev,
> if (type >= HID_BPF_PROG_TYPE_MAX)
> return -EINVAL;
>
> + if (type == HID_BPF_PROG_TYPE_RDESC_FIXUP && hdev->bpf.rdesc_ops)
> + n += 1;
> +
> FOR_ENTRIES(i, jmp_table.tail, jmp_table.head) {
> struct hid_bpf_prog_entry *entry = &jmp_table.entries[i];
>
> diff --git a/drivers/hid/bpf/hid_bpf_struct_ops.c b/drivers/hid/bpf/hid_bpf_struct_ops.c
> new file mode 100644
> index 000000000000..be514a98e55b
> --- /dev/null
> +++ b/drivers/hid/bpf/hid_bpf_struct_ops.c
> @@ -0,0 +1,246 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +/*
> + * HID-BPF support for Linux
> + *
> + * Copyright (c) 2024 Benjamin Tissoires
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/bpf_verifier.h>
> +#include <linux/bpf.h>
> +#include <linux/btf.h>
> +#include <linux/btf_ids.h>
> +#include <linux/filter.h>
> +#include <linux/hid.h>
> +#include <linux/hid_bpf.h>
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/workqueue.h>
> +#include "hid_bpf_dispatch.h"
> +
> +static struct btf *hid_bpf_ops_btf;
> +
> +static int hid_bpf_ops_init(struct btf *btf)
> +{
> + hid_bpf_ops_btf = btf;
> + return 0;
> +}
> +
> +static bool hid_bpf_ops_is_valid_access(int off, int size,
> + enum bpf_access_type type,
> + const struct bpf_prog *prog,
> + struct bpf_insn_access_aux *info)
> +{
> + return bpf_tracing_btf_ctx_access(off, size, type, prog, info);
> +}
> +
> +static int hid_bpf_ops_check_member(const struct btf_type *t,
> + const struct btf_member *member,
> + const struct bpf_prog *prog)
> +{
> + u32 moff = __btf_member_bit_offset(t, member) / 8;
> +
this is good...
> + switch (moff) {
> + case offsetof(struct hid_bpf_ops, hid_rdesc_fixup):
> + break;
> + default:
> + if (prog->sleepable)
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int hid_bpf_ops_btf_struct_access(struct bpf_verifier_log *log,
> + const struct bpf_reg_state *reg,
> + int off, int size)
> +{
> + const struct btf_type *state;
> + const struct btf_type *t;
> + s32 type_id;
> +
> + type_id = btf_find_by_name_kind(reg->btf, "hid_bpf_ctx",
> + BTF_KIND_STRUCT);
> + if (type_id < 0)
> + return -EINVAL;
> +
> + t = btf_type_by_id(reg->btf, reg->btf_id);
> + state = btf_type_by_id(reg->btf, type_id);
> + if (t != state) {
> + bpf_log(log, "only access to hid_bpf_ctx is supported\n");
> + return -EACCES;
> + }
> +
> + /* out-of-bound access in hid_bpf_ctx */
> + if (off + size > sizeof(struct hid_bpf_ctx)) {
> + bpf_log(log, "write access at off %d with size %d\n", off, size);
> + return -EACCES;
> + }
> +
> + if (off < offsetof(struct hid_bpf_ctx, retval)) {
> + bpf_log(log,
> + "write access at off %d with size %d on read-only part of hid_bpf_ctx\n",
> + off, size);
> + return -EACCES;
> + }
> +
> + return NOT_INIT;
> +}
> +
> +static const struct bpf_verifier_ops hid_bpf_verifier_ops = {
> + .is_valid_access = hid_bpf_ops_is_valid_access,
> + .btf_struct_access = hid_bpf_ops_btf_struct_access,
> +};
> +
> +static int hid_bpf_ops_init_member(const struct btf_type *t,
> + const struct btf_member *member,
> + void *kdata, const void *udata)
> +{
> + u32 flags;
> +
> + switch (member->offset) {
> + case offsetof(struct hid_bpf_ops, hid_id) * 8:
but here you're open coding it and adding ugly * 8
Just do:
moff = __btf_member_bit_offset(t, member) / 8;
switch (moff) {
case offsetof(struct tcp_congestion_ops, flags):
the way bpf_tcp_ca.c is doing?
Open code of ->offset is asking for trouble.
> + /* For hid_id and flags fields, this function has to copy it
> + * and return 1 to indicate that the data has been handled by
> + * the struct_ops type, or the verifier will reject the map if
> + * the value of those fields is not zero.
> + */
> + ((struct hid_bpf_ops *)kdata)->hid_id = ((struct hid_bpf_ops *)udata)->hid_id;
> + return 1;
> + case offsetof(struct hid_bpf_ops, flags) * 8:
> + flags = ((struct hid_bpf_ops *)udata)->flags;
> + if (flags & ~HID_BPF_FLAG_MASK)
> + return -EINVAL;
> + ((struct hid_bpf_ops *)kdata)->flags = flags;
> + return 1;
> + }
> + return 0;
> +}
> +
> +static int hid_bpf_reg(void *kdata)
> +{
> + struct hid_bpf_ops *ops = kdata;
> + struct hid_device *hdev;
> + int count, err = 0;
> +
> + hdev = hid_get_device(ops->hid_id);
> + if (IS_ERR(hdev))
> + return PTR_ERR(hdev);
> +
> + ops->hdev = hdev;
> +
> + mutex_lock(&hdev->bpf.prog_list_lock);
> +
> + count = list_count_nodes(&hdev->bpf.prog_list);
> + if (count >= HID_BPF_MAX_PROGS_PER_DEV) {
> + err = -E2BIG;
> + goto out_unlock;
> + }
> +
> + if (ops->hid_rdesc_fixup) {
> + if (hdev->bpf.rdesc_ops) {
> + err = -EINVAL;
> + goto out_unlock;
> + }
> +
> + hdev->bpf.rdesc_ops = ops;
> + }
> +
> + if (ops->hid_device_event) {
> + err = hid_bpf_allocate_event_data(hdev);
> + if (err)
> + goto out_unlock;
> + }
> +
> + if (ops->flags & HID_BPF_FLAG_INSERT_HEAD)
> + list_add_rcu(&ops->list, &hdev->bpf.prog_list);
> + else
> + list_add_tail_rcu(&ops->list, &hdev->bpf.prog_list);
Looks like future proofing, but I feel it's too little to materialize.
Take a look at include/linux/bpf_mprog.h
I suspect it might be useful here too.
> +
> +out_unlock:
> + mutex_unlock(&hdev->bpf.prog_list_lock);
> +
> + if (err) {
> + if (hdev->bpf.rdesc_ops == ops)
> + hdev->bpf.rdesc_ops = NULL;
> + hid_put_device(hdev);
> + } else if (ops->hid_rdesc_fixup) {
> + hid_bpf_reconnect(hdev);
> + }
> +
> + return err;
> +}
> +
> +static void hid_bpf_unreg(void *kdata)
> +{
> + struct hid_bpf_ops *ops = kdata;
> + struct hid_device *hdev;
> + bool reconnect = false;
> +
> + hdev = ops->hdev;
> +
> + /* check if __hid_bpf_ops_destroy_device() has been called */
> + if (!hdev)
> + return;
> +
> + mutex_lock(&hdev->bpf.prog_list_lock);
> +
> + list_del_rcu(&ops->list);
> +
> + reconnect = hdev->bpf.rdesc_ops == ops;
> + if (reconnect)
> + hdev->bpf.rdesc_ops = NULL;
> +
> + mutex_unlock(&hdev->bpf.prog_list_lock);
> +
> + if (reconnect)
> + hid_bpf_reconnect(hdev);
> +
> + hid_put_device(hdev);
> +}
> +
> +static int __hid_bpf_device_event(struct hid_bpf_ctx *ctx, enum hid_report_type type)
> +{
> + return 0;
> +}
> +
> +static int __hid_bpf_rdesc_fixup(struct hid_bpf_ctx *ctx)
> +{
> + return 0;
> +}
> +
> +static struct hid_bpf_ops __bpf_hid_bpf_ops = {
> + .hid_device_event = __hid_bpf_device_event,
> + .hid_rdesc_fixup = __hid_bpf_rdesc_fixup,
> +};
> +
> +static struct bpf_struct_ops bpf_hid_bpf_ops = {
> + .verifier_ops = &hid_bpf_verifier_ops,
> + .init = hid_bpf_ops_init,
> + .check_member = hid_bpf_ops_check_member,
> + .init_member = hid_bpf_ops_init_member,
> + .reg = hid_bpf_reg,
> + .unreg = hid_bpf_unreg,
> + .name = "hid_bpf_ops",
> + .cfi_stubs = &__bpf_hid_bpf_ops,
> + .owner = THIS_MODULE,
> +};
> +
> +void __hid_bpf_ops_destroy_device(struct hid_device *hdev)
> +{
> + struct hid_bpf_ops *e;
> +
> + rcu_read_lock();
> + list_for_each_entry_rcu(e, &hdev->bpf.prog_list, list) {
> + hid_put_device(hdev);
> + e->hdev = NULL;
> + }
> + rcu_read_unlock();
> +}
> +
> +static int __init hid_bpf_struct_ops_init(void)
> +{
> + return register_bpf_struct_ops(&bpf_hid_bpf_ops, hid_bpf_ops);
> +}
> +late_initcall(hid_bpf_struct_ops_init);
> diff --git a/include/linux/hid_bpf.h b/include/linux/hid_bpf.h
> index a66103618e6e..96495e977204 100644
> --- a/include/linux/hid_bpf.h
> +++ b/include/linux/hid_bpf.h
> @@ -65,11 +65,12 @@ struct hid_bpf_ctx {
> * @HID_BPF_FLAG_INSERT_HEAD: insert the given program before any other program
> * currently attached to the device. This doesn't
> * guarantee that this program will always be first
> - * @HID_BPF_FLAG_MAX: sentinel value, not to be used by the callers
> */
> enum hid_bpf_attach_flags {
> HID_BPF_FLAG_NONE = 0,
> HID_BPF_FLAG_INSERT_HEAD = _BITUL(0),
> +
> + /* private: internal use only */
> HID_BPF_FLAG_MAX,
> };
>
> @@ -112,6 +113,63 @@ struct hid_ops {
>
> extern struct hid_ops *hid_ops;
>
> +/**
> + * struct hid_bpf_ops - A BPF struct_ops of callbacks allowing to attach HID-BPF
> + * programs to a HID device
> + * @hid_id: the HID uniq ID to attach to. This is writeable before ``load()``, and
> + * cannot be changed after
> + * @flags: &enum hid_bpf_attach_flags to assign flags before ``load()``.
> + * Writeable only before ``load()``
> + */
> +struct hid_bpf_ops {
> + /* hid_id needs to stay first so we can easily change it
> + * from userspace.
> + */
hmm.
patch 5 just does:
+ skel->struct_ops.mouse_invert->hid_id = hid_id;
I don't see a reason why it needs to be first.
> + int hid_id;
> + u32 flags;
> +
> + /* private: internal use only */
> + struct list_head list;
> +
> + /* public: rest is public */
> +
> +/* fast path fields are put first to fill one cache line */
> +
> + /**
> + * @hid_device_event: called whenever an event is coming in from the device
> + *
> + * It has the following arguments:
> + *
> + * ``ctx``: The HID-BPF context as &struct hid_bpf_ctx
> + *
> + * Return: %0 on success and keep processing; a positive
> + * value to change the incoming size buffer; a negative
> + * error code to interrupt the processing of this event
> + *
> + * Context: Interrupt context.
> + */
> + int (*hid_device_event)(struct hid_bpf_ctx *ctx, enum hid_report_type report_type);
> +
> +/* control/slow paths put last */
> +
> + /**
> + * @hid_rdesc_fixup: called when the probe function parses the report descriptor
> + * of the HID device
> + *
> + * It has the following arguments:
> + *
> + * ``ctx``: The HID-BPF context as &struct hid_bpf_ctx
> + *
> + * Return: %0 on success and keep processing; a positive
> + * value to change the incoming size buffer; a negative
> + * error code to interrupt the processing of this device
> + */
> + int (*hid_rdesc_fixup)(struct hid_bpf_ctx *ctx);
It's fine since you want to keep the prog mostly unchanged,
but since the whole api is improving maybe it's time
to do something about hid_bpf_ctx in all callbacks?
Pass into prog what prog needs to see.
In the old api all progs had to have a common hid_bpf_ctx.
With struct-ops it can be fine tuned.
Overall it looks great.
hid_id did the trick :)
^ permalink raw reply
* RE: [PATCH 1/2] HID: intel-ish-hid: fix cache management mistake
From: Zhang, Lixu @ 2024-05-29 6:46 UTC (permalink / raw)
To: Arnd Bergmann, Srinivas Pandruvada, Jiri Kosina,
Benjamin Tissoires
Cc: Arnd Bergmann, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20240528115802.3122955-1-arnd@kernel.org>
>-----Original Message-----
>From: Arnd Bergmann <arnd@kernel.org>
>Sent: Tuesday, May 28, 2024 7:58 PM
>To: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>; Jiri Kosina
><jikos@kernel.org>; Benjamin Tissoires <bentiss@kernel.org>; Zhang, Lixu
><lixu.zhang@intel.com>
>Cc: Arnd Bergmann <arnd@arndb.de>; linux-input@vger.kernel.org; linux-
>kernel@vger.kernel.org
>Subject: [PATCH 1/2] HID: intel-ish-hid: fix cache management mistake
>
>From: Arnd Bergmann <arnd@arndb.de>
>
>The low-level cache operation on a coherent buffer is incorrect, as coherent
>DMA memory may not actually be cached. Instead, use a DMA barrier that
>ensures that the data is visible to the DMA master before the address is and
>move the memcpy() before the reference.
>
>Fixes: 579a267e4617 ("HID: intel-ish-hid: Implement loading firmware from
>host feature")
>Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>---
>I noticed this one while looking at the bug that was fixed in
>236049723826 ("HID: intel-ish-hid: Fix build error for COMPILE_TEST")
>---
> drivers/hid/intel-ish-hid/ishtp/loader.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/hid/intel-ish-hid/ishtp/loader.c b/drivers/hid/intel-ish-
>hid/ishtp/loader.c
>index 2785b04a2f5a..062d1b25eaa7 100644
>--- a/drivers/hid/intel-ish-hid/ishtp/loader.c
>+++ b/drivers/hid/intel-ish-hid/ishtp/loader.c
>@@ -33,7 +33,6 @@
>
> #define dev_fmt(fmt) "ISH loader: " fmt
>
>-#include <linux/cacheflush.h>
> #include <linux/container_of.h>
> #include <linux/dev_printk.h>
> #include <linux/dma-mapping.h>
>@@ -175,10 +174,11 @@ static int prepare_dma_bufs(struct ishtp_device
>*dev,
> return -ENOMEM;
>
> fragment->fragment_tbl[i].ddr_adrs =
>cpu_to_le64(dma_addr);
>+
>+ memcpy(dma_bufs[i], ish_fw->data + offset, fragment-
>>fragment_tbl[i].length);
fragment->fragment_tbl[i].length was used before assignment.
>+ dma_wmb();
I tested it on the platform, but it didn't wok.
Thanks,
Lixu
> fragment->fragment_tbl[i].length = clamp(ish_fw->size -
>offset, 0, fragment_size);
> fragment->fragment_tbl[i].fw_off = offset;
>- memcpy(dma_bufs[i], ish_fw->data + offset, fragment-
>>fragment_tbl[i].length);
>- clflush_cache_range(dma_bufs[i], fragment_size);
>
> offset += fragment->fragment_tbl[i].length;
> }
>--
>2.39.2
^ permalink raw reply
* RE: [PATCH 2/2] HID: intel-ish-hid: fix endian-conversion
From: Zhang, Lixu @ 2024-05-29 7:05 UTC (permalink / raw)
To: Arnd Bergmann, Srinivas Pandruvada, Jiri Kosina,
Benjamin Tissoires
Cc: Arnd Bergmann, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20240528115802.3122955-2-arnd@kernel.org>
>-----Original Message-----
>From: Arnd Bergmann <arnd@kernel.org>
>Sent: Tuesday, May 28, 2024 7:58 PM
>To: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>; Jiri Kosina
><jikos@kernel.org>; Benjamin Tissoires <bentiss@kernel.org>; Zhang, Lixu
><lixu.zhang@intel.com>
>Cc: Arnd Bergmann <arnd@arndb.de>; linux-input@vger.kernel.org; linux-
>kernel@vger.kernel.org
>Subject: [PATCH 2/2] HID: intel-ish-hid: fix endian-conversion
>
>From: Arnd Bergmann <arnd@arndb.de>
>
>The newly added file causes a ton of sparse warnings about the incorrect use of
>__le32 and similar types:
>
>drivers/hid/intel-ish-hid/ishtp/loader.c:179:17: warning: cast from restricted
>__le32
>drivers/hid/intel-ish-hid/ishtp/loader.c:182:50: warning: incorrect type in
>assignment (different base types)
>drivers/hid/intel-ish-hid/ishtp/loader.c:182:50: expected restricted __le32
>[usertype] length
>drivers/hid/intel-ish-hid/ishtp/loader.c:182:50: got restricted __le16
>[usertype]
>drivers/hid/intel-ish-hid/ishtp/loader.c:183:50: warning: incorrect type in
>assignment (different base types)
>drivers/hid/intel-ish-hid/ishtp/loader.c:183:50: expected restricted __le32
>[usertype] fw_off
>drivers/hid/intel-ish-hid/ishtp/loader.c:183:50: got restricted __le16
>[usertype]
>
>Add the necessary conversions and use temporary variables where appropriate
>to avoid converting back.
>
>Fixes: 579a267e4617 ("HID: intel-ish-hid: Implement loading firmware from
>host feature")
>Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>---
>I noticed this one while looking at the bug that was fixed in
>236049723826 ("HID: intel-ish-hid: Fix build error for COMPILE_TEST")
>---
> drivers/hid/intel-ish-hid/ishtp/loader.c | 69 +++++++++++++-----------
>drivers/hid/intel-ish-hid/ishtp/loader.h | 33 +++++++-----
> 2 files changed, 58 insertions(+), 44 deletions(-)
>
>diff --git a/drivers/hid/intel-ish-hid/ishtp/loader.c b/drivers/hid/intel-ish-
>hid/ishtp/loader.c
>index 062d1b25eaa7..1d4cb99d2130 100644
>--- a/drivers/hid/intel-ish-hid/ishtp/loader.c
>+++ b/drivers/hid/intel-ish-hid/ishtp/loader.c
...
>@@ -162,25 +165,30 @@ static void release_dma_bufs(struct ishtp_device
>*dev, static int prepare_dma_bufs(struct ishtp_device *dev,
> const struct firmware *ish_fw,
> struct loader_xfer_dma_fragment *fragment,
>- void **dma_bufs, u32 fragment_size)
>+ void **dma_bufs, u32 fragment_size, u32
>fragment_count)
See below.
> {
> dma_addr_t dma_addr;
> u32 offset = 0;
>+ u32 length;
> int i;
>
> for (i = 0; i < fragment->fragment_cnt && offset < ish_fw->size; i++) {
You added a parameter fragment_count, but you didn't use it. Did you intend to use it here?
Thanks,
Lixu
> dma_bufs[i] = dma_alloc_coherent(dev->devc, fragment_size,
>&dma_addr, GFP_KERNEL);
>+ dma_bufs[i] = dma_alloc_coherent(dev->devc, fragment_size,
>+ &dma, GFP_KERNEL);
> if (!dma_bufs[i])
> return -ENOMEM;
^ 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