On Wed, 18 Dec 2013 15:44:48 +0100 David Herrmann <dh.herrmann@xxxxxxxxx> wrote: > Hi > > On Wed, Dec 18, 2013 at 3:27 PM, Antonio Ospite > <ospite@xxxxxxxxxxxxxxxxx> wrote: > > Hi David, > > > > thanks for the update. > > > > On Tue, 17 Dec 2013 16:48:52 +0100 > > David Herrmann <dh.herrmann@xxxxxxxxx> wrote: > > > >> As we painfully noticed during the 3.12 merge-window our > >> EVIOCGABS/EVIOCSABS API is limited to ABS_MAX<=0x3f. We tried several > >> hacks to work around it but if we ever decide to increase ABS_MAX, the > >> EVIOCSABS ioctl ABI might overflow into the next byte causing horrible > >> misinterpretations in the kernel that we cannot catch. > >> > >> Therefore, we decided to go with ABS_MAX2/CNT2 and introduce two new > >> ioctls to get/set abs-params. They no longer encode the ABS code in the > >> ioctl number and thus allow up to 4 billion ABS codes. > >> > > > > Just a question: did you consider the possibility of not exposing _MAX2 > > and _CNT2 in a header file at all but let those be queried? Or maybe > > this is not necessary if we assume that userspace programs are supposed > > to know what is the MAX _they_ support? > > > > BTW I was thinking for instance to a program compiled with ABS_MAX2 = > > 0x4f but working with future kernels with an increased value, it still > > won't be able to support values greater than 0x4f. > > Why would a program that was compiled with ABS_MAX2=0x4f want to use > ABS_* values greater than 0x4f? It doesn't know of any new values so > even if it could query ABS_MAX2, it could never know anything about > the new constants. The only place where it is useful is for generic > programs that just foward ABS_* codes. But these can just ignore > ABS_MAX2 entirely and use the whole int32_t range for codes. > OK, that makes sense, thanks. > If there is a specific use-case that'd require a dynamic way to > retrieve ABS_MAX2, I can consider changing the API. But unless there's > such a use-case, I'd like to keep it the same as for KEY_*, SW_*, ... > > >> The new API also allows to query multiple ABS values with one call. To > >> allow EVIOCSABS2(code = 0, cnt = ABS_CNT2) we need to silently ignore > >> writes to ABS_MT_SLOT. Furthermore, for better compatibility with > >> newer user-space, we ignore writes to unknown codes. Hence, if we ever > >> increase ABS_MAX2, new user-space will work with code=0,cnt=ABS_CNT2 just > >> fine even on old kernels. > >> > > > > To my inexperienced eye it's not obvious why ABS_MT_SLOT must be handled > > as a special case. Maybe because I don't even know what > > code=0,cnt=ABS_CNT2 is used for. > > ABS_MT_SLOT describes the current MT-slot that is reported via > ABS_MT_* bits. It is read-only so we cannot let user-space write to > it. > I see. > >> Note that we also need to increase EV_VERSION so user-space can reliably > >> know whether ABS2 is supported. Unfortunately, we return EINVAL instead of > >> ENOSYS for unknown evdev ioctls so it's nearly impossible to catch > >> reliably without EVIOCGVERSION. > >> > > > > I'll check how to use that in evtest. > > > > Just one more comment below. > > > >> Signed-off-by: David Herrmann <dh.herrmann@xxxxxxxxx> > >> --- > >> drivers/hid/hid-debug.c | 2 +- > >> drivers/hid/hid-input.c | 2 +- > >> drivers/input/evdev.c | 95 +++++++++++++++++++++++++++++++- > >> drivers/input/input.c | 14 ++--- > >> drivers/input/keyboard/goldfish_events.c | 6 +- > >> drivers/input/keyboard/hil_kbd.c | 2 +- > >> drivers/input/misc/uinput.c | 6 +- > >> include/linux/hid.h | 2 +- > >> include/linux/input.h | 6 +- > >> include/uapi/linux/input.h | 42 +++++++++++++- > >> include/uapi/linux/uinput.h | 2 +- > >> 11 files changed, 155 insertions(+), 24 deletions(-) > >> > > > > [...] > >> diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h > >> index bd24470..1856461 100644 > >> --- a/include/uapi/linux/input.h > >> +++ b/include/uapi/linux/input.h > > [...] > >> > >> /* > >> + * Due to API restrictions the legacy evdev API only supports ABS values up to > >> + * ABS_MAX/CNT. Use the extended *ABS2 ioctls to operate on any ABS values in > >> + * between ABS_MAX and ABS_MAX2. > >> + */ > >> +#define ABS_MAX2 0x3f > >> +#define ABS_CNT2 (ABS_MAX2+1) > >> + > > > > Maybe it's just my English, but when you say "between ABS_MAX and > > ABS_MAX2" it sounds like the old protocol still _must_ be used for > > values <= ABS_MAX. > > > > IIUC this is true "only" for compatibility with older kernels right? > > If a program decides to support only newer kernels it can check the > > protocol version and use only the new ioctls, right? > > > > Maybe you can be more explicit about that in the comment? > > See the comment on "struct input_absinfo2". It describes the API, this > comment just describes the ABS_* values. But if anyone has a better > phrase to use here, I will gladly adjust it. > That comment says: EVIOC[G/S]ABS2 ioctls [...] do the same as the old EVIOC [G/S]ABS ioctls but... My bad, I was confused by the fact that the new ioctls are presented as a replacement for the old ones, but then I interpreted the later comment as suggesting to use them only for values between ABS_MAX and ABS_MAX2. Looking at how you use both mechanisms combined in libevdev it is clear they can be used together for backward compatibility. I like keywords, so I'd stress in the commit message that the new protocol can either _replace_ or _extend_ the old one. Ciao, Antonio -- Antonio Ospite http://ao2.it A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
