[RFC] rc-core: report protocol information to userspace
diff mbox

Message ID 149346313232.25459.10475301883786006034.stgit@zeus.hardeman.nu
State New
Headers show

Commit Message

David Härdeman April 29, 2017, 10:52 a.m. UTC
Whether we decide to go for any new keytable ioctl():s or not in rc-core, we
should provide the protocol information of keypresses to userspace.

Note that this means that the RC_TYPE_* definitions become part of the
userspace <-> kernel API/ABI (meaning a full patch should maybe move those
defines under include/uapi).

This would also need to be ack:ed by the input maintainers.
---
 drivers/media/rc/rc-main.c             |    1 +
 include/uapi/linux/input-event-codes.h |    1 +
 2 files changed, 2 insertions(+)

Comments

Sean Young May 1, 2017, 10:38 a.m. UTC | #1
On Sat, Apr 29, 2017 at 12:52:12PM +0200, David Härdeman wrote:
> Whether we decide to go for any new keytable ioctl():s or not in rc-core, we
> should provide the protocol information of keypresses to userspace.
> 
> Note that this means that the RC_TYPE_* definitions become part of the
> userspace <-> kernel API/ABI (meaning a full patch should maybe move those
> defines under include/uapi).
> 
> This would also need to be ack:ed by the input maintainers.

This was already NACKed in the past.

http://www.spinics.net/lists/linux-input/msg46941.html

> ---
>  drivers/media/rc/rc-main.c             |    1 +
>  include/uapi/linux/input-event-codes.h |    1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
> index e0f9b322ab02..a38c1f3569ee 100644
> --- a/drivers/media/rc/rc-main.c
> +++ b/drivers/media/rc/rc-main.c
> @@ -773,6 +773,7 @@ static void ir_do_keydown(struct rc_dev *dev, enum rc_type protocol,
>  	if (new_event && dev->keypressed)
>  		ir_do_keyup(dev, false);
>  
> +	input_event(dev->input_dev, EV_MSC, MSC_PROTOCOL, protocol);
>  	input_event(dev->input_dev, EV_MSC, MSC_SCAN, scancode);
>  
>  	if (new_event && keycode != KEY_RESERVED) {
> diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> index 3af60ee69053..1a8c3554cbcb 100644
> --- a/include/uapi/linux/input-event-codes.h
> +++ b/include/uapi/linux/input-event-codes.h
> @@ -794,6 +794,7 @@
>  #define MSC_RAW			0x03
>  #define MSC_SCAN		0x04
>  #define MSC_TIMESTAMP		0x05
> +#define MSC_PROTOCOL		0x06
>  #define MSC_MAX			0x07
>  #define MSC_CNT			(MSC_MAX+1)
>
David Härdeman May 1, 2017, 12:49 p.m. UTC | #2
On Mon, May 01, 2017 at 11:38:30AM +0100, Sean Young wrote:
>On Sat, Apr 29, 2017 at 12:52:12PM +0200, David Härdeman wrote:
>> Whether we decide to go for any new keytable ioctl():s or not in rc-core, we
>> should provide the protocol information of keypresses to userspace.
>> 
>> Note that this means that the RC_TYPE_* definitions become part of the
>> userspace <-> kernel API/ABI (meaning a full patch should maybe move those
>> defines under include/uapi).
>> 
>> This would also need to be ack:ed by the input maintainers.
>
>This was already NACKed in the past.
>
>http://www.spinics.net/lists/linux-input/msg46941.html
>

Didn't know that, thanks for the pointer. I still think we should
revisit this though. Even if we don't add protocol-aware EVIOC[SG]KEY_V2
ioctls, that information is useful for a configuration tool when
creating keymaps for a new remote.

And examining the parent hardware device (as Dmitry seemed to suggest)
doesn't help with protocol identification.

Another option if we don't want to touch the input layer would be to
export the last_* members from struct rc_dev in sysfs (and I'm guessing
a timestamp would be necessary then). Seems like a lot of work to
accomplish what would otherwise be a one-line change in the input layer
though (one-line since I'm assuming we could provide the protocol
defines in a separate header, other than input-event-codes.h as the
protocols are subsystem-specific).
Sean Young May 1, 2017, 5:05 p.m. UTC | #3
On Mon, May 01, 2017 at 02:49:57PM +0200, David Härdeman wrote:
> On Mon, May 01, 2017 at 11:38:30AM +0100, Sean Young wrote:
> >On Sat, Apr 29, 2017 at 12:52:12PM +0200, David Härdeman wrote:
> >> Whether we decide to go for any new keytable ioctl():s or not in rc-core, we
> >> should provide the protocol information of keypresses to userspace.
> >> 
> >> Note that this means that the RC_TYPE_* definitions become part of the
> >> userspace <-> kernel API/ABI (meaning a full patch should maybe move those
> >> defines under include/uapi).
> >> 
> >> This would also need to be ack:ed by the input maintainers.
> >
> >This was already NACKed in the past.
> >
> >http://www.spinics.net/lists/linux-input/msg46941.html
> >
> 
> Didn't know that, thanks for the pointer. I still think we should
> revisit this though. Even if we don't add protocol-aware EVIOC[SG]KEY_V2
> ioctls, that information is useful for a configuration tool when
> creating keymaps for a new remote.
> 
> And examining the parent hardware device (as Dmitry seemed to suggest)
> doesn't help with protocol identification.
> 
> Another option if we don't want to touch the input layer would be to
> export the last_* members from struct rc_dev in sysfs (and I'm guessing
> a timestamp would be necessary then). Seems like a lot of work to
> accomplish what would otherwise be a one-line change in the input layer
> though (one-line since I'm assuming we could provide the protocol
> defines in a separate header, other than input-event-codes.h as the
> protocols are subsystem-specific).

So I have some patches for reading and writing scancodes, which will
give you a u64 scancode, protocol and other bits of information. This
can also be used for transmit where the IR encoders will be used.

http://www.spinics.net/lists/linux-media/msg109836.html

I wanted to make sure that these patches are also sufficient for sending
scancodes for the lirc_zilog driver before merging, which is what I'm
working on right now.


Sean

Patch
diff mbox

diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index e0f9b322ab02..a38c1f3569ee 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -773,6 +773,7 @@  static void ir_do_keydown(struct rc_dev *dev, enum rc_type protocol,
 	if (new_event && dev->keypressed)
 		ir_do_keyup(dev, false);
 
+	input_event(dev->input_dev, EV_MSC, MSC_PROTOCOL, protocol);
 	input_event(dev->input_dev, EV_MSC, MSC_SCAN, scancode);
 
 	if (new_event && keycode != KEY_RESERVED) {
diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
index 3af60ee69053..1a8c3554cbcb 100644
--- a/include/uapi/linux/input-event-codes.h
+++ b/include/uapi/linux/input-event-codes.h
@@ -794,6 +794,7 @@ 
 #define MSC_RAW			0x03
 #define MSC_SCAN		0x04
 #define MSC_TIMESTAMP		0x05
+#define MSC_PROTOCOL		0x06
 #define MSC_MAX			0x07
 #define MSC_CNT			(MSC_MAX+1)