* Re: [PATCH v1 00/19] hid-ft260: Fixes for serial driver patch v4
From: Christina Quast @ 2024-02-23 13:07 UTC (permalink / raw)
To: Jiri Kosina, Michael Zaidman
Cc: daniel.beer, linux-kernel, linux-input, linux-serial,
ilpo.jarvinen, johan, gregkh, equinox
In-Reply-To: <nycvar.YFH.7.76.2402131120070.21798@cbobk.fhfr.pm>
Hi Jiri!
I send Patch v5, feel free to have a look!
Best regards,
Christina
On 2/13/24 11:20, Jiri Kosina wrote:
> On Sat, 10 Feb 2024, Michael Zaidman wrote:
>
>> Modifications on top of "[PATCH v4 RESEND] hid-ft260: Add serial driver"
>> https://lore.kernel.org/all/20231218093153.192268-1-contact@christina-quast.de/
>>
>> They are mostly the fixes to the original v4 patch and should be melded into
>> the v5 patch rather than upstreamed as separate patches.
> Agreed; I am not acting upon this series now then, and will wait for v5
> with these folded in.
>
> Thanks,
>
^ permalink raw reply
* Re: [PATCH RFC bpf-next v3 05/16] bpf/verifier: add bpf_timer as a kfunc capable type
From: Eduard Zingerman @ 2024-02-23 14:54 UTC (permalink / raw)
To: Benjamin Tissoires, Alexei Starovoitov, Daniel Borkmann,
John Fastabend, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Shuah Khan
Cc: bpf, linux-kernel, linux-input, linux-doc, linux-kselftest
In-Reply-To: <20240221-hid-bpf-sleepable-v3-5-1fb378ca6301@kernel.org>
On Wed, 2024-02-21 at 17:25 +0100, Benjamin Tissoires wrote:
[...]
> @@ -11973,6 +12006,9 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
> if (ret)
> return ret;
> break;
> + case KF_ARG_PTR_TO_TIMER:
> + /* FIXME: should we do anything here? */
> + break;
I think that here it is necessary to enforce that R1
is PTR_TO_MAP_VALUE and that it points to the timer field of the map value.
As is, the following program leads to in-kernel page fault when
printing verifier log:
--- 8< ----------------------------
struct elem {
struct bpf_timer t;
};
struct {
__uint(type, BPF_MAP_TYPE_ARRAY);
__uint(max_entries, 2);
__type(key, int);
__type(value, struct elem);
} array SEC(".maps");
int bpf_timer_set_sleepable_cb
(struct bpf_timer *timer,
int (callback_fn)(void *map, int *key, struct bpf_timer *timer))
__ksym __weak;
static int cb_sleepable(void *map, int *key, struct bpf_timer *timer)
{
return 0;
}
SEC("fentry/bpf_fentry_test5")
int BPF_PROG2(test_sleepable, int, a)
{
struct bpf_timer *arr_timer;
int array_key = 1;
arr_timer = bpf_map_lookup_elem(&array, &array_key);
if (!arr_timer)
return 0;
bpf_timer_init(arr_timer, &array, CLOCK_MONOTONIC);
bpf_timer_set_sleepable_cb((void *)&arr_timer, // note incorrrect pointer type!
cb_sleepable);
bpf_timer_start(arr_timer, 0, 0);
return 0;
}
---------------------------- >8 ---
I get the page fault when doing:
$ ./veristat -l7 -vvv -f test_sleepable timer.bpf.o
[ 21.014886] BUG: kernel NULL pointer dereference, address: 0000000000000060
...
[ 21.015780] RIP: 0010:print_reg_state (kernel/bpf/log.c:715)
And here is a relevant fragment of print_reg_state():
713 if (type_is_map_ptr(t)) {
714 if (reg->map_ptr->name[0])
715 verbose_a("map=%s", reg->map_ptr->name);
716 verbose_a("ks=%d,vs=%d",
717 reg->map_ptr->key_size,
718 reg->map_ptr->value_size);
719 }
The error is caused by reg->map_ptr being NULL.
The code in check_kfunc_args() allows anything in R1,
including registers for which type is not pointer to map and reg->map_ptr is NULL.
When later the check_kfunc_call() is done it does push_callback_call():
12152 err = push_callback_call(env, insn, insn_idx, meta.subprogno,
12153 set_timer_callback_state);
Which calls set_timer_callback_state(), that sets bogus state for R{1,2,3}:
9683 static int set_timer_callback_state(...)
9684 {
9685 struct bpf_map *map_ptr = caller->regs[BPF_REG_1].map_ptr;
9687
9688 /* bpf_timer_set_callback(struct bpf_timer *timer, void *callback_fn);
9689 * callback_fn(struct bpf_map *map, void *key, void *value);
9690 */
9691 callee->regs[BPF_REG_1].type = CONST_PTR_TO_MAP;
9692 __mark_reg_known_zero(&callee->regs[BPF_REG_1]);
9693 callee->regs[BPF_REG_1].map_ptr = map_ptr;
^^^^^^^^^
This is NULL!
^ permalink raw reply
* [GIT PULL] Immutable branch between MFD, Input and Thermal due for the v6.9 merge window
From: Lee Jones @ 2024-02-23 14:56 UTC (permalink / raw)
To: Biju Das
Cc: Wim Van Sebroeck, Guenter Roeck, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Support Opensource,
Rafael J. Wysocki, Daniel Lezcano, Zhang Rui, Lukasz Luba,
Steve Twiss, linux-input, devicetree, linux-pm, linux-watchdog,
Geert Uytterhoeven, Prabhakar Mahadev Lad, Biju Das,
linux-renesas-soc
In-Reply-To: <20240131102656.3379-1-biju.das.jz@bp.renesas.com>
Sorry for the delay, I was on vacation.
Enjoy!
The following changes since commit 6613476e225e090cc9aad49be7fa504e290dd33d:
Linux 6.8-rc1 (2024-01-21 14:11:32 -0800)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git ib-mfd-input-thermal-v6.9
for you to fetch changes up to f1eb64bf6d4bef5295ab7633874960fbcfadca46:
dt-bindings: mfd: dlg,da9063: Convert da9062 to json-schema (2024-02-08 13:06:03 +0000)
----------------------------------------------------------------
Immutable branch between MFD, Input and Thermal due for the v6.9 merge window
----------------------------------------------------------------
Biju Das (6):
dt-bindings: mfd: da9062: Update watchdog description
dt-bindings: mfd: dlg,da9063: Update watchdog child node
dt-bindings: input: Convert da906{1,2,3} onkey to json-schema
dt-bindings: thermal: Convert da906{1,2} thermal to json-schema
dt-bindings: mfd: dlg,da9063: Sort child devices
dt-bindings: mfd: dlg,da9063: Convert da9062 to json-schema
.../devicetree/bindings/input/da9062-onkey.txt | 47 ----
.../bindings/input/dlg,da9062-onkey.yaml | 38 ++++
Documentation/devicetree/bindings/mfd/da9062.txt | 124 -----------
.../devicetree/bindings/mfd/dlg,da9063.yaml | 248 ++++++++++++++++++---
.../devicetree/bindings/thermal/da9062-thermal.txt | 36 ---
.../bindings/thermal/dlg,da9062-thermal.yaml | 35 +++
MAINTAINERS | 6 +-
7 files changed, 290 insertions(+), 244 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/input/da9062-onkey.txt
create mode 100644 Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml
delete mode 100644 Documentation/devicetree/bindings/mfd/da9062.txt
delete mode 100644 Documentation/devicetree/bindings/thermal/da9062-thermal.txt
create mode 100644 Documentation/devicetree/bindings/thermal/dlg,da9062-thermal.yaml
--
Lee Jones [李琼斯]
^ permalink raw reply
* Re: [PATCH RFC bpf-next v3 06/16] bpf/helpers: introduce bpf_timer_set_sleepable_cb() kfunc
From: Eduard Zingerman @ 2024-02-23 15:10 UTC (permalink / raw)
To: Benjamin Tissoires, Alexei Starovoitov, Daniel Borkmann,
John Fastabend, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Shuah Khan
Cc: bpf, linux-kernel, linux-input, linux-doc, linux-kselftest
In-Reply-To: <20240221-hid-bpf-sleepable-v3-6-1fb378ca6301@kernel.org>
On Wed, 2024-02-21 at 17:25 +0100, Benjamin Tissoires wrote:
[...]
> @@ -12120,6 +12137,16 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
> }
> }
>
> + if (is_async_callback_calling_kfunc(meta.func_id)) {
I think that it's better to check specific kfunc id here:
meta.func_id == special_kfunc_list[KF_bpf_timer_set_sleepable_cb_impl]
In case if some new async callback calling kfunc would be added,
for which set_timer_callback_state() won't be correct.
> + err = push_callback_call(env, insn, insn_idx, meta.subprogno,
> + set_timer_callback_state);
> + if (err) {
> + verbose(env, "kfunc %s#%d failed callback verification\n",
> + func_name, meta.func_id);
> + return err;
> + }
> + }
> +
> rcu_lock = is_kfunc_bpf_rcu_read_lock(&meta);
> rcu_unlock = is_kfunc_bpf_rcu_read_unlock(&meta);
>
>
^ permalink raw reply
* Re: [PATCH RFC bpf-next v3 07/16] bpf/helpers: mark the callback of bpf_timer_set_sleepable_cb() as sleepable
From: Eduard Zingerman @ 2024-02-23 15:35 UTC (permalink / raw)
To: Benjamin Tissoires, Alexei Starovoitov, Daniel Borkmann,
John Fastabend, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Shuah Khan
Cc: bpf, linux-kernel, linux-input, linux-doc, linux-kselftest
In-Reply-To: <20240221-hid-bpf-sleepable-v3-7-1fb378ca6301@kernel.org>
On Wed, 2024-02-21 at 17:25 +0100, Benjamin Tissoires wrote:
[...]
> @@ -626,6 +627,7 @@ struct bpf_subprog_info {
> bool is_async_cb: 1;
> bool is_exception_cb: 1;
> bool args_cached: 1;
> + bool is_sleepable: 1;
>
> u8 arg_cnt;
> struct bpf_subprog_arg_info args[MAX_BPF_FUNC_REG_ARGS];
[...]
> @@ -2421,6 +2424,7 @@ static struct bpf_verifier_state *push_async_cb(struct bpf_verifier_env *env,
> * Initialize it similar to do_check_common().
> */
> elem->st.branches = 1;
> + elem->st.in_sleepable = env->subprog_info[subprog].is_sleepable;
> frame = kzalloc(sizeof(*frame), GFP_KERNEL);
> if (!frame)
> goto err;
[...]
> @@ -9478,6 +9483,7 @@ static int push_callback_call(struct bpf_verifier_env *env, struct bpf_insn *ins
>
> /* there is no real recursion here. timer callbacks are async */
> env->subprog_info[subprog].is_async_cb = true;
> + env->subprog_info[subprog].is_sleepable = is_bpf_timer_set_sleepable_cb_kfunc(insn->imm);
> async_cb = push_async_cb(env, env->subprog_info[subprog].start,
> insn_idx, subprog);
I'd make is_sleepable a parameter for push_async_cb() instead of a field
in struct bpf_subprog_info.
I had to spend some time convincing myself that bpf_subprog_info->is_sleepable
does not have to be computed before do_check() in check_cfg(),
or what would happen if same callback is passed as both sleepable and
non-sleepable callback. These questions won't arise if this is a parameter.
[...]
^ permalink raw reply
* Re: [PATCH 0/4] leds: trigger: Improve handling of led_trigger_event() and simplify mute audio trigger
From: Lee Jones @ 2024-02-23 15:45 UTC (permalink / raw)
To: Heiner Kallweit
Cc: Pavel Machek, Jaroslav Kysela, Takashi Iwai, Dmitry Torokhov,
Thomas Bogendoerfer, linux-leds@vger.kernel.org, linux-sound,
open list:HID CORE LAYER, linux-mips
In-Reply-To: <30d49088-283c-40f3-b97b-fd5f5174a467@gmail.com>
On Tue, 13 Feb 2024, Heiner Kallweit wrote:
> If a simple trigger is assigned to a LED, then the LED may be off until
> the next led_trigger_event() call. This may be an issue for simple
> triggers with rare led_trigger_event() calls, e.g. power supply
> charging indicators (drivers/power/supply/power_supply_leds.c).
> Therefore persist the brightness value of the last led_trigger_event()
> call and use this value if the trigger is assigned to a LED.
> This change allows to use simple triggers in more cases.
> As a first use case simplify handling of the mute audio trigger.
>
> This series touches few subsystems. I'd propose to handle it via
> the LED subsystem.
>
> Heiner Kallweit (4):
> leds: trigger: Store brightness set by led_trigger_event()
> ALSA: control-led: Integrate mute led trigger
> Input: leds: Prepare for removal of config option LEDS_AUDIO_TRIGGER
> leds: trigger: audio: Remove this trigger
>
> arch/mips/configs/ci20_defconfig | 1 -
> drivers/input/input-leds.c | 8 +---
> drivers/leds/led-triggers.c | 6 ++-
> drivers/leds/trigger/Kconfig | 7 ---
> drivers/leds/trigger/Makefile | 1 -
> drivers/leds/trigger/ledtrig-audio.c | 67 ----------------------------
> include/linux/leds.h | 29 ++++++------
> sound/core/Kconfig | 1 -
> sound/core/control_led.c | 20 +++++++--
> 9 files changed, 37 insertions(+), 103 deletions(-)
> delete mode 100644 drivers/leds/trigger/ledtrig-audio.c
Are the sound maintainers on-board with this?
--
Lee Jones [李琼斯]
^ permalink raw reply
* Re: [PATCH 0/4] leds: trigger: Improve handling of led_trigger_event() and simplify mute audio trigger
From: Takashi Iwai @ 2024-02-23 15:47 UTC (permalink / raw)
To: Lee Jones
Cc: Heiner Kallweit, Pavel Machek, Jaroslav Kysela, Takashi Iwai,
Dmitry Torokhov, Thomas Bogendoerfer, linux-leds@vger.kernel.org,
linux-sound, open list:HID CORE LAYER, linux-mips
In-Reply-To: <20240223154559.GF1666215@google.com>
On Fri, 23 Feb 2024 16:45:59 +0100,
Lee Jones wrote:
>
> On Tue, 13 Feb 2024, Heiner Kallweit wrote:
>
> > If a simple trigger is assigned to a LED, then the LED may be off until
> > the next led_trigger_event() call. This may be an issue for simple
> > triggers with rare led_trigger_event() calls, e.g. power supply
> > charging indicators (drivers/power/supply/power_supply_leds.c).
> > Therefore persist the brightness value of the last led_trigger_event()
> > call and use this value if the trigger is assigned to a LED.
> > This change allows to use simple triggers in more cases.
> > As a first use case simplify handling of the mute audio trigger.
> >
> > This series touches few subsystems. I'd propose to handle it via
> > the LED subsystem.
> >
> > Heiner Kallweit (4):
> > leds: trigger: Store brightness set by led_trigger_event()
> > ALSA: control-led: Integrate mute led trigger
> > Input: leds: Prepare for removal of config option LEDS_AUDIO_TRIGGER
> > leds: trigger: audio: Remove this trigger
> >
> > arch/mips/configs/ci20_defconfig | 1 -
> > drivers/input/input-leds.c | 8 +---
> > drivers/leds/led-triggers.c | 6 ++-
> > drivers/leds/trigger/Kconfig | 7 ---
> > drivers/leds/trigger/Makefile | 1 -
> > drivers/leds/trigger/ledtrig-audio.c | 67 ----------------------------
> > include/linux/leds.h | 29 ++++++------
> > sound/core/Kconfig | 1 -
> > sound/core/control_led.c | 20 +++++++--
> > 9 files changed, 37 insertions(+), 103 deletions(-)
> > delete mode 100644 drivers/leds/trigger/ledtrig-audio.c
>
> Are the sound maintainers on-board with this?
See
https://lore.kernel.org/r/87zfw1ewrd.wl-tiwai@suse.de
thanks,
Takashi
^ permalink raw reply
* Re: [PATCH RFC bpf-next v3 08/16] bpf/verifier: do_misc_fixups for is_bpf_timer_set_sleepable_cb_kfunc
From: Eduard Zingerman @ 2024-02-23 16:00 UTC (permalink / raw)
To: Benjamin Tissoires, Alexei Starovoitov, Daniel Borkmann,
John Fastabend, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Shuah Khan
Cc: bpf, linux-kernel, linux-input, linux-doc, linux-kselftest
In-Reply-To: <20240221-hid-bpf-sleepable-v3-8-1fb378ca6301@kernel.org>
On Wed, 2024-02-21 at 17:25 +0100, Benjamin Tissoires wrote:
> This is still a WIP, but I think this can be dropped as we never
> get to this instruction. So what should we do here?
As Alexei replied in a separate sub-thread you probably want this
for sleepable timers. Here is full source code block:
if (insn->imm == BPF_FUNC_timer_set_callback ||
is_bpf_timer_set_sleepable_cb_kfunc(insn->imm)) {
...
struct bpf_insn ld_addrs[2] = {
BPF_LD_IMM64(BPF_REG_3, (long)prog->aux),
};
insn_buf[0] = ld_addrs[0];
insn_buf[1] = ld_addrs[1];
insn_buf[2] = *insn;
cnt = 3;
new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt);
...
}
Effectively, it sets up third function call parameter (R3)
for timer_set_callback() to be prog->aux.
E.g. before bpf_patch_insn_data():
r1 = ... timer ...
r2 = ... callback address ...
call timer_set_callback
After bpf_patch_insn_data():
r1 = ... timer ...
r2 = ... callback address ...
r3 = prog->aux ll
call timer_set_callback
This way it won't be necessary to walk stack in search for ctx.aux
in bpf_timer_set_sleepable_cb().
^ permalink raw reply
* Re: [PATCH 0/4] leds: trigger: Improve handling of led_trigger_event() and simplify mute audio trigger
From: Lee Jones @ 2024-02-23 16:04 UTC (permalink / raw)
To: Takashi Iwai
Cc: Heiner Kallweit, Pavel Machek, Jaroslav Kysela, Takashi Iwai,
Dmitry Torokhov, Thomas Bogendoerfer, linux-leds@vger.kernel.org,
linux-sound, open list:HID CORE LAYER, linux-mips
In-Reply-To: <87cysn2ndu.wl-tiwai@suse.de>
On Fri, 23 Feb 2024, Takashi Iwai wrote:
> On Fri, 23 Feb 2024 16:45:59 +0100,
> Lee Jones wrote:
> >
> > On Tue, 13 Feb 2024, Heiner Kallweit wrote:
> >
> > > If a simple trigger is assigned to a LED, then the LED may be off until
> > > the next led_trigger_event() call. This may be an issue for simple
> > > triggers with rare led_trigger_event() calls, e.g. power supply
> > > charging indicators (drivers/power/supply/power_supply_leds.c).
> > > Therefore persist the brightness value of the last led_trigger_event()
> > > call and use this value if the trigger is assigned to a LED.
> > > This change allows to use simple triggers in more cases.
> > > As a first use case simplify handling of the mute audio trigger.
> > >
> > > This series touches few subsystems. I'd propose to handle it via
> > > the LED subsystem.
> > >
> > > Heiner Kallweit (4):
> > > leds: trigger: Store brightness set by led_trigger_event()
> > > ALSA: control-led: Integrate mute led trigger
> > > Input: leds: Prepare for removal of config option LEDS_AUDIO_TRIGGER
> > > leds: trigger: audio: Remove this trigger
> > >
> > > arch/mips/configs/ci20_defconfig | 1 -
> > > drivers/input/input-leds.c | 8 +---
> > > drivers/leds/led-triggers.c | 6 ++-
> > > drivers/leds/trigger/Kconfig | 7 ---
> > > drivers/leds/trigger/Makefile | 1 -
> > > drivers/leds/trigger/ledtrig-audio.c | 67 ----------------------------
> > > include/linux/leds.h | 29 ++++++------
> > > sound/core/Kconfig | 1 -
> > > sound/core/control_led.c | 20 +++++++--
> > > 9 files changed, 37 insertions(+), 103 deletions(-)
> > > delete mode 100644 drivers/leds/trigger/ledtrig-audio.c
> >
> > Are the sound maintainers on-board with this?
>
> See
> https://lore.kernel.org/r/87zfw1ewrd.wl-tiwai@suse.de
Were you happy with Heiner's response?
--
Lee Jones [李琼斯]
^ permalink raw reply
* Re: [PATCH 0/4] leds: trigger: Improve handling of led_trigger_event() and simplify mute audio trigger
From: Lee Jones @ 2024-02-23 16:05 UTC (permalink / raw)
To: Heiner Kallweit
Cc: Pavel Machek, Jaroslav Kysela, Takashi Iwai, Dmitry Torokhov,
Thomas Bogendoerfer, linux-leds@vger.kernel.org, linux-sound,
open list:HID CORE LAYER, linux-mips
In-Reply-To: <30d49088-283c-40f3-b97b-fd5f5174a467@gmail.com>
On Tue, 13 Feb 2024, Heiner Kallweit wrote:
> If a simple trigger is assigned to a LED, then the LED may be off until
> the next led_trigger_event() call. This may be an issue for simple
> triggers with rare led_trigger_event() calls, e.g. power supply
> charging indicators (drivers/power/supply/power_supply_leds.c).
> Therefore persist the brightness value of the last led_trigger_event()
> call and use this value if the trigger is assigned to a LED.
> This change allows to use simple triggers in more cases.
> As a first use case simplify handling of the mute audio trigger.
>
> This series touches few subsystems. I'd propose to handle it via
> the LED subsystem.
>
> Heiner Kallweit (4):
> leds: trigger: Store brightness set by led_trigger_event()
> ALSA: control-led: Integrate mute led trigger
> Input: leds: Prepare for removal of config option LEDS_AUDIO_TRIGGER
Dmitry, you happy too?
> leds: trigger: audio: Remove this trigger
>
> arch/mips/configs/ci20_defconfig | 1 -
> drivers/input/input-leds.c | 8 +---
> drivers/leds/led-triggers.c | 6 ++-
> drivers/leds/trigger/Kconfig | 7 ---
> drivers/leds/trigger/Makefile | 1 -
> drivers/leds/trigger/ledtrig-audio.c | 67 ----------------------------
> include/linux/leds.h | 29 ++++++------
> sound/core/Kconfig | 1 -
> sound/core/control_led.c | 20 +++++++--
> 9 files changed, 37 insertions(+), 103 deletions(-)
> delete mode 100644 drivers/leds/trigger/ledtrig-audio.c
>
> --
> 2.43.1
>
--
Lee Jones [李琼斯]
^ permalink raw reply
* Re: [PATCH 0/4] leds: trigger: Improve handling of led_trigger_event() and simplify mute audio trigger
From: Takashi Iwai @ 2024-02-23 16:05 UTC (permalink / raw)
To: Lee Jones
Cc: Takashi Iwai, Heiner Kallweit, Pavel Machek, Jaroslav Kysela,
Takashi Iwai, Dmitry Torokhov, Thomas Bogendoerfer,
linux-leds@vger.kernel.org, linux-sound, open list:HID CORE LAYER,
linux-mips
In-Reply-To: <20240223160415.GG1666215@google.com>
On Fri, 23 Feb 2024 17:04:15 +0100,
Lee Jones wrote:
>
> On Fri, 23 Feb 2024, Takashi Iwai wrote:
>
> > On Fri, 23 Feb 2024 16:45:59 +0100,
> > Lee Jones wrote:
> > >
> > > On Tue, 13 Feb 2024, Heiner Kallweit wrote:
> > >
> > > > If a simple trigger is assigned to a LED, then the LED may be off until
> > > > the next led_trigger_event() call. This may be an issue for simple
> > > > triggers with rare led_trigger_event() calls, e.g. power supply
> > > > charging indicators (drivers/power/supply/power_supply_leds.c).
> > > > Therefore persist the brightness value of the last led_trigger_event()
> > > > call and use this value if the trigger is assigned to a LED.
> > > > This change allows to use simple triggers in more cases.
> > > > As a first use case simplify handling of the mute audio trigger.
> > > >
> > > > This series touches few subsystems. I'd propose to handle it via
> > > > the LED subsystem.
> > > >
> > > > Heiner Kallweit (4):
> > > > leds: trigger: Store brightness set by led_trigger_event()
> > > > ALSA: control-led: Integrate mute led trigger
> > > > Input: leds: Prepare for removal of config option LEDS_AUDIO_TRIGGER
> > > > leds: trigger: audio: Remove this trigger
> > > >
> > > > arch/mips/configs/ci20_defconfig | 1 -
> > > > drivers/input/input-leds.c | 8 +---
> > > > drivers/leds/led-triggers.c | 6 ++-
> > > > drivers/leds/trigger/Kconfig | 7 ---
> > > > drivers/leds/trigger/Makefile | 1 -
> > > > drivers/leds/trigger/ledtrig-audio.c | 67 ----------------------------
> > > > include/linux/leds.h | 29 ++++++------
> > > > sound/core/Kconfig | 1 -
> > > > sound/core/control_led.c | 20 +++++++--
> > > > 9 files changed, 37 insertions(+), 103 deletions(-)
> > > > delete mode 100644 drivers/leds/trigger/ledtrig-audio.c
> > >
> > > Are the sound maintainers on-board with this?
> >
> > See
> > https://lore.kernel.org/r/87zfw1ewrd.wl-tiwai@suse.de
>
> Were you happy with Heiner's response?
Yes.
Takashi
^ permalink raw reply
* Re: [PATCH RFC bpf-next v3 00/16] sleepable bpf_timer (was: allow HID-BPF to do device IOs)
From: Eduard Zingerman @ 2024-02-23 16:19 UTC (permalink / raw)
To: Benjamin Tissoires, Alexei Starovoitov, Daniel Borkmann,
John Fastabend, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Shuah Khan
Cc: bpf, linux-kernel, linux-input, linux-doc, linux-kselftest
In-Reply-To: <20240221-hid-bpf-sleepable-v3-0-1fb378ca6301@kernel.org>
On Wed, 2024-02-21 at 17:25 +0100, Benjamin Tissoires wrote:
> [Partly a RFC/formal submission: there are still FIXMEs in the code]
> [Also using bpf-next as the base tree for HID changes as there will
> be conflicting changes otherwise, so I'm personaly fine for the HID
> commits to go through bpf-next]
[...]
Could you please also add verifier selftests, e.g. extend
tools/testing/selftests/bpf/progs/timer.c (bpf side)
tools/testing/selftests/bpf/prog_tests/timer.c (userspace side triggering
bpf side)
Negative tests could be added in
tools/testing/selftests/bpf/progs/timer_failure.c
Please let me know if you need any help setting up local BPF test
environment, I have a short writeup on how to set it up in chroot.
^ permalink raw reply
* Re: [PATCH RFC bpf-next v3 00/16] sleepable bpf_timer (was: allow HID-BPF to do device IOs)
From: Benjamin Tissoires @ 2024-02-23 19:42 UTC (permalink / raw)
To: Eduard Zingerman
Cc: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Jiri Kosina,
Benjamin Tissoires, Jonathan Corbet, Shuah Khan, bpf,
linux-kernel, linux-input, linux-doc, linux-kselftest
In-Reply-To: <e95cebb8f6a7ede5d40b4becf58cb578a5c4cf58.camel@gmail.com>
Hi,
On Feb 23 2024, Eduard Zingerman wrote:
> On Wed, 2024-02-21 at 17:25 +0100, Benjamin Tissoires wrote:
> > [Partly a RFC/formal submission: there are still FIXMEs in the code]
> > [Also using bpf-next as the base tree for HID changes as there will
> > be conflicting changes otherwise, so I'm personaly fine for the HID
> > commits to go through bpf-next]
>
> [...]
>
> Could you please also add verifier selftests, e.g. extend
> tools/testing/selftests/bpf/progs/timer.c (bpf side)
> tools/testing/selftests/bpf/prog_tests/timer.c (userspace side triggering
> bpf side)
> Negative tests could be added in
> tools/testing/selftests/bpf/progs/timer_failure.c
>
> Please let me know if you need any help setting up local BPF test
> environment, I have a short writeup on how to set it up in chroot.
Thanks a lot for your review (and Alexei's). I was actually off today
and will be off next Monday too, but I'll work on those tests next week.
Cheers,
Benjamin
^ permalink raw reply
* Re: [PATCH RFC bpf-next v3 09/16] HID: bpf/dispatch: regroup kfuncs definitions
From: Benjamin Tissoires @ 2024-02-23 19:44 UTC (permalink / raw)
To: Eduard Zingerman
Cc: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Jiri Kosina,
Benjamin Tissoires, Jonathan Corbet, Shuah Khan, bpf,
linux-kernel, linux-input, linux-doc, linux-kselftest
In-Reply-To: <962a182f514e96ce258e7173608c282a6183775a.camel@gmail.com>
On Feb 22 2024, Eduard Zingerman wrote:
> On Wed, 2024-02-21 at 17:25 +0100, Benjamin Tissoires wrote:
>
> [...]
>
> > diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c
> > index e630caf644e8..52abb27426f4 100644
> > --- a/drivers/hid/bpf/hid_bpf_dispatch.c
> > +++ b/drivers/hid/bpf/hid_bpf_dispatch.c
> > @@ -143,48 +143,6 @@ 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();
>
> Note:
> this patch does not apply on top of current bpf-next [0] because
> __bpf_kfunc_start_defs and __bpf_kfunc are not present in [0].
>
> [0] commit 58fd62e0aa50 ("bpf: Clarify batch lookup/lookup_and_delete semantics")
Right... this was in Linus' tree as a late 6.8-rcx addition. Depending
on how bpf-next will be rebased/merged, I'll see if I merge this
subseries through the HID tree or the BPF one.
Cheers,
Benjamin
>
> > -
> > -/**
> > - * hid_bpf_get_data - Get the kernel memory pointer associated with the context @ctx
> > - *
> > - * @ctx: The HID-BPF context
> > - * @offset: The offset within the memory
> > - * @rdwr_buf_size: the const size of the buffer
> > - *
> > - * @returns %NULL on error, an %__u8 memory pointer on success
> > - */
> > -__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;
> > -
> > - if (!ctx)
> > - return NULL;
>
> [...]
^ permalink raw reply
* Re: [PATCH RFC bpf-next v3 02/16] bpf/verifier: introduce in_sleepable() helper
From: Benjamin Tissoires @ 2024-02-23 19:46 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Shuah Khan, bpf,
LKML, open list:HID CORE LAYER, open list:DOCUMENTATION,
open list:KERNEL SELFTEST FRAMEWORK
In-Reply-To: <CAADnVQKotZ0ki6p4DAydj=XE9hU6pPc+FdqU_WQ99NBK-qSe+A@mail.gmail.com>
On Feb 22 2024, Alexei Starovoitov wrote:
> On Wed, Feb 21, 2024 at 8:25 AM Benjamin Tissoires <bentiss@kernel.org> wrote:
> > @@ -18193,7 +18198,7 @@ static int resolve_pseudo_ldimm64(struct bpf_verifier_env *env)
> > return -E2BIG;
> > }
> >
> > - if (env->prog->aux->sleepable)
> > + if (in_sleepable(env))
> > atomic64_inc(&map->sleepable_refcnt);
>
> this one doesn't look correct.
> The verifier didn't start its main loop when resolve_pseudo_ldimm64()
> is called.
> It also loses symmetry with other sleepable_refcnt operations
> in syscall.c and core.c
>
> I reverted this hunk and applied patches 1,2,3
> with minor edits, like removing unnecessary parens in patch 3,
> and patch subject rewords.
Thanks a lot. I'll work on the rest of the series next week.
Cheers,
Benjamin
^ permalink raw reply
* Re: [PATCH v1 19/19] hid-ft260: uart: improve write performance
From: Michael Zaidman @ 2024-02-23 21:22 UTC (permalink / raw)
To: chrysh, daniel.beer, jikos, michael.zaidman
Cc: linux-kernel, linux-input, linux-serial, ilpo.jarvinen, johan,
gregkh, equinox
In-Reply-To: <20240210215147.77629-20-michael.zaidman@gmail.com>
On Sat, Feb 10, 2024 at 11:51:47PM +0200, Michael Zaidman wrote:
> Tx performance with the current buffer size of 256 bytes is lower when
> the data length exceeds the xmit buf size.
>
> [134331.147978] ft260_uart_write: count: 288, len: 256
> [134331.157945] ft260_uart_write: count: 32, len: 32
> [134331.159977] ft260_uart_write: count: 288, len: 256
> [134331.169990] ft260_uart_write: count: 32, len: 32
>
> 1. Increase the xmit buffer size to page size as used in the serial core
> and other tty drivers.
>
> 2. Remove the xmit buffer fulness against the watermark checking and the
> tty_wakeup calling in the ft260_uart_transmit_chars routine. This code is
> taken from other drivers, but other drivers may call the routine from the
> interrupt context. In our case, this condition is always True since xmit
> buffer filling and emptying are serialized and done synchronously.
>
> Tested with picocom ASCII file transfer by 288-byte chunks at 921600
> bauds rate with above 20% performance improvement.
>
> Before:
> 2821.7 Kbytes transferred at 47367 CPS... Done.
>
> After:
> 2821.7 Kbytes transferred at 57788 CPS... Done.
>
Besides the performance improvement, it fixes the bug of outputting
characters to the local terminal when local echo is disabled and
printing every character twice with local echo enabled.
> Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
> ---
> drivers/hid/hid-ft260.c | 11 +++--------
> 1 file changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
> index 6b172bfa4f98..1188b8e09938 100644
> --- a/drivers/hid/hid-ft260.c
> +++ b/drivers/hid/hid-ft260.c
> @@ -308,8 +308,7 @@ enum {
> #define FT260_UART_EN_PW_SAVE_BAUD (4800)
>
> #define UART_COUNT_MAX (4) /* Number of supported UARTs */
> -#define XMIT_FIFO_SIZE (256)
> -#define TTY_WAKEUP_WATERMARK (XMIT_FIFO_SIZE / 2)
> +#define XMIT_FIFO_SIZE (PAGE_SIZE)
>
> static const struct hid_device_id ft260_devices[] = {
> { HID_USB_DEVICE(USB_VENDOR_ID_FUTURE_TECHNOLOGY,
> @@ -1211,7 +1210,7 @@ static int ft260_uart_transmit_chars(struct ft260_device *port)
>
> len = kfifo_out_spinlocked(xmit, rep->data, len, &port->xmit_fifo_lock);
>
> - ret = ft260_hid_output_report(hdev, (u8 *)rep, len + sizeof(*rep));
> + ret = ft260_hid_output_report(hdev, (u8 *)rep, len + 2);
> if (ret < 0)
> goto tty_out;
>
> @@ -1219,10 +1218,6 @@ static int ft260_uart_transmit_chars(struct ft260_device *port)
> port->icount.tx += len;
> } while (data_len > 0);
>
> - len = kfifo_len(xmit);
> - if ((XMIT_FIFO_SIZE - len) > TTY_WAKEUP_WATERMARK)
> - tty_wakeup(tty);
> -
> ret = 0;
>
> tty_out:
^ permalink raw reply
* [PATCH] HID: input: avoid polling stylus battery on Chromebook Pompom
From: Dmitry Torokhov @ 2024-02-23 23:16 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: kenalba, dianders, linux-input, linux-kernel
Internal touchscreen on Trogdor Pompom (AKA Dynabook Chromebook C1)
supports USI stylus. Unfortunately the HID descriptor for the stylus
interface does not contain "Stylus" physical collection, which makes
the kernel to try and pull battery information, resulting in errors.
Apply HID_BATTERY_QUIRK_AVOID_QUERY to the device.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/hid/hid-ids.h | 1 +
drivers/hid/hid-input.c | 2 ++
2 files changed, 3 insertions(+)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index e4d2dfd5d253..05f451390a3b 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -426,6 +426,7 @@
#define I2C_DEVICE_ID_HP_SPECTRE_X360_14T_EA100_V1 0x2BED
#define I2C_DEVICE_ID_HP_SPECTRE_X360_14T_EA100_V2 0x2BEE
#define I2C_DEVICE_ID_HP_ENVY_X360_15_EU0556NG 0x2D02
+#define I2C_DEVICE_ID_CHROMEBOOK_TROGDOR_POMPOM 0x2F81
#define USB_VENDOR_ID_ELECOM 0x056e
#define USB_DEVICE_ID_ELECOM_BM084 0x0061
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index c8b20d44b147..e03d300d2bac 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -411,6 +411,8 @@ static const struct hid_device_id hid_battery_quirks[] = {
HID_BATTERY_QUIRK_IGNORE },
{ HID_I2C_DEVICE(USB_VENDOR_ID_ELAN, I2C_DEVICE_ID_HP_ENVY_X360_15_EU0556NG),
HID_BATTERY_QUIRK_IGNORE },
+ { HID_I2C_DEVICE(USB_VENDOR_ID_ELAN, I2C_DEVICE_ID_CHROMEBOOK_TROGDOR_POMPOM),
+ HID_BATTERY_QUIRK_AVOID_QUERY },
{}
};
--
2.44.0.rc0.258.g7320e95886-goog
--
Dmitry
^ permalink raw reply related
* Re: [PATCH 3/4] Input: leds: Prepare for removal of config option LEDS_AUDIO_TRIGGER
From: Dmitry Torokhov @ 2024-02-23 23:23 UTC (permalink / raw)
To: Heiner Kallweit
Cc: Pavel Machek, Lee Jones, Jaroslav Kysela, Takashi Iwai,
Thomas Bogendoerfer, linux-leds@vger.kernel.org, linux-sound,
open list:HID CORE LAYER, linux-mips
In-Reply-To: <e5ef576b-70ee-4781-88e3-e728c1e7cb9c@gmail.com>
On Tue, Feb 13, 2024 at 08:33:24AM +0100, Heiner Kallweit wrote:
> In a follow-up patch handling of the LED audio trigger will be changed,
> including removal of config symbol LEDS_AUDIO_TRIGGER. Therefore set
> the default trigger unconditionally to "audio-mute". It does no harm
> if a default trigger doesn't exist.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> drivers/input/input-leds.c | 8 +-------
> 1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/drivers/input/input-leds.c b/drivers/input/input-leds.c
> index b16fc8194..176f1da7f 100644
> --- a/drivers/input/input-leds.c
> +++ b/drivers/input/input-leds.c
> @@ -18,12 +18,6 @@
> #define VT_TRIGGER(_name) .trigger = NULL
> #endif
>
> -#if IS_ENABLED(CONFIG_LEDS_TRIGGER_AUDIO)
Should it be simply changed to CONFIG_SND_CTL_LED?
> -#define AUDIO_TRIGGER(_name) .trigger = _name
> -#else
> -#define AUDIO_TRIGGER(_name) .trigger = NULL
> -#endif
> -
> static const struct {
> const char *name;
> const char *trigger;
> @@ -35,7 +29,7 @@ static const struct {
> [LED_KANA] = { "kana", VT_TRIGGER("kbd-kanalock") },
> [LED_SLEEP] = { "sleep" } ,
> [LED_SUSPEND] = { "suspend" },
> - [LED_MUTE] = { "mute", AUDIO_TRIGGER("audio-mute") },
> + [LED_MUTE] = { "mute", "audio-mute" },
> [LED_MISC] = { "misc" },
> [LED_MAIL] = { "mail" },
> [LED_CHARGING] = { "charging" },
> --
> 2.43.1
>
>
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 3/4] Input: leds: Prepare for removal of config option LEDS_AUDIO_TRIGGER
From: Heiner Kallweit @ 2024-02-24 9:31 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Pavel Machek, Lee Jones, Jaroslav Kysela, Takashi Iwai,
Thomas Bogendoerfer, linux-leds@vger.kernel.org, linux-sound,
open list:HID CORE LAYER, linux-mips
In-Reply-To: <Zdko7JAFw_TBV-63@google.com>
On 24.02.2024 00:23, Dmitry Torokhov wrote:
> On Tue, Feb 13, 2024 at 08:33:24AM +0100, Heiner Kallweit wrote:
>> In a follow-up patch handling of the LED audio trigger will be changed,
>> including removal of config symbol LEDS_AUDIO_TRIGGER. Therefore set
>> the default trigger unconditionally to "audio-mute". It does no harm
>> if a default trigger doesn't exist.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>> drivers/input/input-leds.c | 8 +-------
>> 1 file changed, 1 insertion(+), 7 deletions(-)
>>
>> diff --git a/drivers/input/input-leds.c b/drivers/input/input-leds.c
>> index b16fc8194..176f1da7f 100644
>> --- a/drivers/input/input-leds.c
>> +++ b/drivers/input/input-leds.c
>> @@ -18,12 +18,6 @@
>> #define VT_TRIGGER(_name) .trigger = NULL
>> #endif
>>
>> -#if IS_ENABLED(CONFIG_LEDS_TRIGGER_AUDIO)
>
> Should it be simply changed to CONFIG_SND_CTL_LED?
>
This would be another option. What is better IMO is a matter of
personal taste. Setting the default trigger unconditionally may
cause a negligible runtime overhead when the LED is instantiated,
on the other hand it results in less code to be maintained.
Do you have a preference?
>> -#define AUDIO_TRIGGER(_name) .trigger = _name
>> -#else
>> -#define AUDIO_TRIGGER(_name) .trigger = NULL
>> -#endif
>> -
>> static const struct {
>> const char *name;
>> const char *trigger;
>> @@ -35,7 +29,7 @@ static const struct {
>> [LED_KANA] = { "kana", VT_TRIGGER("kbd-kanalock") },
>> [LED_SLEEP] = { "sleep" } ,
>> [LED_SUSPEND] = { "suspend" },
>> - [LED_MUTE] = { "mute", AUDIO_TRIGGER("audio-mute") },
>> + [LED_MUTE] = { "mute", "audio-mute" },
>> [LED_MISC] = { "misc" },
>> [LED_MAIL] = { "mail" },
>> [LED_CHARGING] = { "charging" },
>> --
>> 2.43.1
>>
>>
>
> Thanks.
>
^ permalink raw reply
* Re: Regression with Lenovo ThinkPad Compact USB Keyboard
From: Raphaël Halimi @ 2024-02-24 10:52 UTC (permalink / raw)
To: Linux Stable Mailing List
Cc: Linux Regressions Mailing List, Linux Input Mailing List,
Dmitry Torokhov, Jiri Kosina, Benjamin Tissoires,
Mikhail Khvainitski, Thorsten Leemhuis
In-Reply-To: <c8986411-2bf7-4b7d-8ac1-f702dc7c725a@gmail.com>
Le 20/02/2024 à 19:12, Raphaël Halimi a écrit :
> I'll let a few days pass (remember, the bug doesn't happen immediately
> but only after a varying amount of time) and I'll report here if the
> spurious middle-clicks happened again or not.
As promised, here's my report: using the recompiled hid-lenvo module
without those three patches for more than three days, I didn't
experience a single spurious middle-click, whereas the in-tree module
triggered the bug several times a day, and I had to unplug/replug the
keyboard (or simulate it with a software trick) to get back to a normal
state.
So those three patches did introduce this regression after all (as I
correctly guessed).
Regards,
--
Raphaël Halimi
^ permalink raw reply
* [PATCH] HID: usbhid: hid-pidff: fix initialisation for devices that pre-play effects
From: Jules Noirant @ 2024-02-24 12:00 UTC (permalink / raw)
Cc: jikos, Jules Noirant, Benjamin Tissoires, linux-usb, linux-input,
linux-kernel
Some PID compliant devices play effects outside of the usb drivers when you
power them, for example an autocenter spring. These effects are thus
re-triggered when you reset the device through the driver init sequence,
preventing other effects to be played in some cases.
This fix makes sure all the effects currently played are stopped after
resetting the device. More specifically, it brings compatibility to the
Brunner CLS-P joystick and potentially more of their products.
Signed-off-by: Jules Noirant <jules.noirant@orange.fr>
---
drivers/hid/usbhid/hid-pidff.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
index 3b4ee21cd..aade18f9e 100644
--- a/drivers/hid/usbhid/hid-pidff.c
+++ b/drivers/hid/usbhid/hid-pidff.c
@@ -109,8 +109,9 @@ static const u8 pidff_pool[] = { 0x80, 0x83, 0xa9 };
/* Special field key tables used to put special field keys into arrays */
#define PID_ENABLE_ACTUATORS 0
-#define PID_RESET 1
-static const u8 pidff_device_control[] = { 0x97, 0x9a };
+#define PID_STOP_ALL_EFFECTS 1
+#define PID_RESET 2
+static const u8 pidff_device_control[] = { 0x97, 0x99, 0x9a };
#define PID_CONSTANT 0
#define PID_RAMP 1
@@ -1157,6 +1158,10 @@ static void pidff_reset(struct pidff_device *pidff)
hid_hw_request(hid, pidff->reports[PID_DEVICE_CONTROL], HID_REQ_SET_REPORT);
hid_hw_wait(hid);
+ pidff->device_control->value[0] = pidff->control_id[PID_STOP_ALL_EFFECTS];
+ hid_hw_request(hid, pidff->reports[PID_DEVICE_CONTROL], HID_REQ_SET_REPORT);
+ hid_hw_wait(hid);
+
pidff->device_control->value[0] =
pidff->control_id[PID_ENABLE_ACTUATORS];
hid_hw_request(hid, pidff->reports[PID_DEVICE_CONTROL], HID_REQ_SET_REPORT);
--
2.40.1
^ permalink raw reply related
* Re: Regression with Lenovo ThinkPad Compact USB Keyboard
From: Linux regression tracking (Thorsten Leemhuis) @ 2024-02-24 13:08 UTC (permalink / raw)
To: Raphaël Halimi, Linux Stable Mailing List
Cc: Linux Regressions Mailing List, Linux Input Mailing List,
Dmitry Torokhov, Jiri Kosina, Benjamin Tissoires,
Mikhail Khvainitski
In-Reply-To: <7a5fc584-1520-4e52-9c77-d67a656524c6@gmail.com>
On 24.02.24 11:52, Raphaël Halimi wrote:
> Le 20/02/2024 à 19:12, Raphaël Halimi a écrit :
>> I'll let a few days pass (remember, the bug doesn't happen immediately
>> but only after a varying amount of time) and I'll report here if the
>> spurious middle-clicks happened again or not.
>
> As promised, here's my report: using the recompiled hid-lenvo module
> without those three patches for more than three days, I didn't
> experience a single spurious middle-click, whereas the in-tree module
> triggered the bug several times a day, and I had to unplug/replug the
> keyboard (or simulate it with a software trick) to get back to a normal
> state.
>
> So those three patches did introduce this regression after all (as I
> correctly guessed).
Mikhail, do you have any idea what might be wrong here? The three
commits Raphaël mentioned that seem to cause the issue are all yours afaics.
Raphaël: would nevertheless still be good if you could identify which of
the three causes the problem, as then the developers might consider
simply reverting it.
Ciao, Thorsten
^ permalink raw reply
* Re: Regression with Lenovo ThinkPad Compact USB Keyboard
From: Raphaël Halimi @ 2024-02-24 13:51 UTC (permalink / raw)
To: Linux regressions mailing list
Cc: Linux Input Mailing List, Dmitry Torokhov, Jiri Kosina,
Benjamin Tissoires, Mikhail Khvainitski,
Linux regression tracking (Thorsten Leemhuis),
Linux Stable Mailing List
In-Reply-To: <10022b0c-89c3-43e1-89ba-00e458fe1dfd@leemhuis.info>
Le 24/02/2024 à 14:08, Thorsten Leemhuis a écrit :
> Raphaël: would nevertheless still be good if you could identify which of
> the three causes the problem, as then the developers might consider
> simply reverting it.
Hi,
It can't be the third one (43527a0) since I clearly remember that I
experienced the regression before it was applied to the Debian kernel.
So I'll try applying only the first one (46a0a2c), and report.
(in the meantime I crafted a quick and dirty Debian package to build the
module with DKMS, so it will be easy)
Regards,
--
Raphaël Halimi
^ permalink raw reply
* Re: Regression with Lenovo ThinkPad Compact USB Keyboard
From: Raphaël Halimi @ 2024-02-24 16:15 UTC (permalink / raw)
To: Linux regressions mailing list
Cc: Linux Input Mailing List, Dmitry Torokhov, Jiri Kosina,
Benjamin Tissoires, Mikhail Khvainitski,
Linux regression tracking (Thorsten Leemhuis),
Linux Stable Mailing List
In-Reply-To: <7a8d9d60-a151-4b25-882b-48e6929339a4@gmail.com>
Le 24/02/2024 à 14:51, Raphaël Halimi a écrit :
> It can't be the third one (43527a0) since I clearly remember that I
> experienced the regression before it was applied to the Debian kernel.
>
> So I'll try applying only the first one (46a0a2c), and report.
I can confirm that the module compiled with 46a0a2c alone does produces
spurious middle-clicks.
Maybe "ThinkPad Compact Keyboard with TrackPoint" should also be
excluded, like "ThinkPad TrackPoint Keyboard II" was in commit 43527a0 ?
But then, would 46a0a2c still be relevant ?
Regards,
--
Raphaël Halimi
^ permalink raw reply
* Re: [PATCH 11/42] drivers/hid: Convert snprintf to sysfs_emit
From: Bruno Prémont @ 2024-02-24 16:59 UTC (permalink / raw)
To: Li Zhijian
Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Jonathan Cameron,
Srinivas Pandruvada, linux-input
In-Reply-To: <20240116045151.3940401-9-lizhijian@fujitsu.com>
Hi Li,
While only looking for the snprintf() in your coccinelle analysis you
probably also want to look for sprintf() (see e.g. else clause in
hid-sensor-custom.c and scnprintf() (see picolcd_fb_update_rate_show
in hid-picolcd_fb.c) to replace more s*printf() calls with sysfs_emit*
variants.
diff --git a/drivers/hid/hid-picolcd_fb.c b/drivers/hid/hid-picolcd_fb.c
index d726aaafb146..03074d25d662 100644
--- a/drivers/hid/hid-picolcd_fb.c
+++ b/drivers/hid/hid-picolcd_fb.c
@@ -459,9 +459,9 @@ static ssize_t picolcd_fb_update_rate_show(struct device *dev,
if (ret >= PAGE_SIZE)
break;
else if (i == fb_update_rate)
- ret += scnprintf(buf+ret, PAGE_SIZE-ret, "[%u] ", i);
+ ret += sysfs_emit_at(buf, ret, "[%u] ", i);
else
- ret += scnprintf(buf+ret, PAGE_SIZE-ret, "%u ", i);
+ ret += sysfs_emit_at(buf, ret, "%u ", i);
if (ret > 0)
buf[min(ret, (size_t)PAGE_SIZE)-1] = '\n';
return ret;
For hid-picolcd-*.c,
Acked-by: Bruno Prémont <bonbons@linux-vserver.org>
but preferably with the scnprintf() case is covered too.
Cheers,
Bruno
On Tue, 16 Jan 2024 12:51:20 +0800 Li Zhijian wrote:
> Per Documentation/filesystems/sysfs.rst, show() should only use sysfs_emit()
> or sysfs_emit_at() when formatting the value to be returned to user space.
>
> coccinelle complains that there are still a couple of functions that use
> snprintf(). Convert them to sysfs_emit().
>
> > ./drivers/hid/hid-picolcd_core.c:259:9-17: WARNING: please use sysfs_emit
> > ./drivers/hid/hid-picolcd_core.c:304:8-16: WARNING: please use sysfs_emit
> > ./drivers/hid/hid-sensor-custom.c:375:10-18: WARNING: please use sysfs_emi
>
> No functional change intended
>
> CC: "Bruno Prémont" <bonbons@linux-vserver.org>
> CC: Jiri Kosina <jikos@kernel.org>
> CC: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> CC: Jonathan Cameron <jic23@kernel.org>
> CC: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> CC: linux-input@vger.kernel.org
> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
> ---
> drivers/hid/hid-picolcd_core.c | 6 +++---
> drivers/hid/hid-sensor-custom.c | 3 +--
> 2 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/hid/hid-picolcd_core.c b/drivers/hid/hid-picolcd_core.c
> index bbda231a7ce3..fa46fb6eab3f 100644
> --- a/drivers/hid/hid-picolcd_core.c
> +++ b/drivers/hid/hid-picolcd_core.c
> @@ -256,9 +256,9 @@ static ssize_t picolcd_operation_mode_show(struct device *dev,
> struct picolcd_data *data = dev_get_drvdata(dev);
>
> if (data->status & PICOLCD_BOOTLOADER)
> - return snprintf(buf, PAGE_SIZE, "[bootloader] lcd\n");
> + return sysfs_emit(buf, "[bootloader] lcd\n");
> else
> - return snprintf(buf, PAGE_SIZE, "bootloader [lcd]\n");
> + return sysfs_emit(buf, "bootloader [lcd]\n");
> }
>
> static ssize_t picolcd_operation_mode_store(struct device *dev,
> @@ -301,7 +301,7 @@ static ssize_t picolcd_operation_mode_delay_show(struct device *dev,
> {
> struct picolcd_data *data = dev_get_drvdata(dev);
>
> - return snprintf(buf, PAGE_SIZE, "%hu\n", data->opmode_delay);
> + return sysfs_emit(buf, "%hu\n", data->opmode_delay);
> }
>
> static ssize_t picolcd_operation_mode_delay_store(struct device *dev,
> diff --git a/drivers/hid/hid-sensor-custom.c b/drivers/hid/hid-sensor-custom.c
> index d85398721659..4fe8dccf671d 100644
> --- a/drivers/hid/hid-sensor-custom.c
> +++ b/drivers/hid/hid-sensor-custom.c
> @@ -372,8 +372,7 @@ static ssize_t show_value(struct device *dev, struct device_attribute *attr,
> sizeof(struct hid_custom_usage_desc),
> usage_id_cmp);
> if (usage_desc)
> - return snprintf(buf, PAGE_SIZE, "%s\n",
> - usage_desc->desc);
> + return sysfs_emit(buf, "%s\n", usage_desc->desc);
> else
> return sprintf(buf, "not-specified\n");
Shouldn't the sprintf() in the else clause be replaced as well?
> } else
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox