2018-05-16 22:04 UTC+0100 ~ Sean Young <sean@xxxxxxxx>
> Add support for BPF_PROG_RAWIR_EVENT. This type of BPF program can call
> rc_keydown() to reported decoded IR scancodes, or rc_repeat() to report
> that the last key should be repeated.
>
> The bpf program can be attached to using the bpf(BPF_PROG_ATTACH) syscall;
> the target_fd must be the /dev/lircN device.
>
> Signed-off-by: Sean Young <sean@xxxxxxxx>
> ---
> drivers/media/rc/Kconfig | 13 ++
> drivers/media/rc/Makefile | 1 +
> drivers/media/rc/bpf-rawir-event.c | 363 +++++++++++++++++++++++++++++
> drivers/media/rc/lirc_dev.c | 24 ++
> drivers/media/rc/rc-core-priv.h | 24 ++
> drivers/media/rc/rc-ir-raw.c | 14 +-
> include/linux/bpf_rcdev.h | 30 +++
> include/linux/bpf_types.h | 3 +
> include/uapi/linux/bpf.h | 55 ++++-
> kernel/bpf/syscall.c | 7 +
> 10 files changed, 531 insertions(+), 3 deletions(-)
> create mode 100644 drivers/media/rc/bpf-rawir-event.c
> create mode 100644 include/linux/bpf_rcdev.h
>
[...]
Hi Sean,
Please find below some nitpicks on the documentation for the two helpers.
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index d94d333a8225..243e141e8a5b 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
[...]
> @@ -1902,6 +1904,35 @@ union bpf_attr {
> * egress otherwise). This is the only flag supported for now.
> * Return
> * **SK_PASS** on success, or **SK_DROP** on error.
> + *
> + * int bpf_rc_keydown(void *ctx, u32 protocol, u32 scancode, u32 toggle)
> + * Description
> + * Report decoded scancode with toggle value. For use in
> + * BPF_PROG_TYPE_RAWIR_EVENT, to report a successfully
Could you please use bold RST markup for constants and function names?
Typically for BPF_PROG_TYPE_RAWIR_EVENT here and the enum below.
> + * decoded scancode. This is will generate a keydown event,
s/This is will/This will/?
> + * and a keyup event once the scancode is no longer repeated.
> + *
> + * *ctx* pointer to bpf_rawir_event, *protocol* is decoded
> + * protocol (see RC_PROTO_* enum).
This documentation is intended to be compiled as a man page. Could you
please use a complete sentence here?
Also, this could do with additional markup as well: **struct
bpf_rawir_event**.
> + *
> + * Some protocols include a toggle bit, in case the button
> + * was released and pressed again between consecutive scancodes,
> + * copy this bit into *toggle* if it exists, else set to 0.
> + *
> + * Return
The "Return" lines here and in the second helper use space indent
instead as tabs (as all other lines do). Would you mind fixing it for
consistency?
> + * Always return 0 (for now)
Other helpers use just "0" in that case, but I do not really mind.
Out of curiosity, do you have anything specific in mind for changing the
return value here in the future?
> + *
> + * int bpf_rc_repeat(void *ctx)
> + * Description
> + * Repeat the last decoded scancode; some IR protocols like
> + * NEC have a special IR message for repeat last button,
s/repeat/repeating/?
> + * in case user is holding a button down; the scancode is
> + * not repeated.
> + *
> + * *ctx* pointer to bpf_rawir_event.
Please use a complete sentence here as well, if you do not mind.
> + *
> + * Return
> + * Always return 0 (for now)
> */
Thanks,
Quentin
