Return-Path: MIME-Version: 1.0 In-Reply-To: <1332793209-2950-1-git-send-email-dh.herrmann@googlemail.com> References: <1332793209-2950-1-git-send-email-dh.herrmann@googlemail.com> Date: Tue, 27 Mar 2012 15:43:07 -0300 Message-ID: Subject: Re: [RFC v2 0/1] User-space HID I/O Driver From: Joao Paulo Rechi Vita To: David Herrmann Cc: linux-input@vger.kernel.org, jkosina@suse.cz, chen.ganir@ti.com, claudio.takahasi@openbossa.org, linux-bluetooth@vger.kernel.org, anderson.lizardo@openbossa.org, marcel@holtmann.org Content-Type: text/plain; charset=UTF-8 List-ID: Hello all, On Mon, Mar 26, 2012 at 5:20 PM, David Herrmann wrote: > Hi > > This is the second revision of the UHID driver. It basically allows to re= gister > hid_ll_drivers in user-space. This is needed to implement Bluetooth HoG (= HID > over GATT) as discussed earlier. > We have a first prototype implementation of the HoG plugin working with the uhid driver \o/ I'll send a RFC to the linux-bluetooth mailing list later, but I think it's nice to give some feedback on the uhid module here as well. The only problem I've encountered while testing it was that it doesn't seem to work if I set the 'bus' field on the struct uhid_create_req to BUS_BLUETOOTH. Initially I thought this was just metadata, but changing it to BUS_USB makes it work. Additionally, I don't know if you forgot or did it on purpose, but you didn't add the uhid example to samples/Kconfig and sample/Makefile. > Changes: > =C2=A0- Improved documentation > =C2=A0- Added example which emulates a 3-button mouse with wheel in user-= space I've (partially) tested the example, on a X-less virtual machine. I'll try it on a box with X so I can attach the virtual mouse to a pointer in X and see the cursor moving. > =C2=A0- Fixed some remaining bugs > > I didn't change any major parts. I checked that writev/readv work correct= ly and > I documented how to use it in ./Documentation/hid/uhid.txt. > > For a full example please see ./samples/uhid/uhid-example.c. > > While developing it I also considered moving HIDP to user-space. If we do= BT-HoG > in user-space, it should be easier to also move HIDP to user-space. The c= urrent > in-kernel HIDP is buggy, anyway. > > Still open points: > =C2=A0- How do I get a proper "parent" pointer for uhid->hid->dev.parent?= I could use > =C2=A0 the uhid misc device as parent or is there some device-struct insi= de "struct > =C2=A0 file"? > =C2=A0- Should user-space be able to provide "uniq" and "phys" names for = HID-devices? > =C2=A0- uhid_event is quite big (>4KB) which makes uhid_device really big= (~70KB). > =C2=A0 Should I allocate the ring-buffer dynamically to avoid this? > =C2=A0- Should I provide the quirks field to user-space? Probably as part= of > =C2=A0 UHID_START? > =C2=A0- Could someone review x32 COMPAT stuff? I tried to align all the p= ublic > =C2=A0 structures correctly so I could even remove the __packed attribute= . Anyway, I > =C2=A0 thought keeping it is probably better for future extensions. > > I spent some time on the locks and they seem all fine. I couldn't find an= y > races. > > Jiri, Marcel, could you review the design and tell me what you think? > > A short notice about the protocol: > User-space is allowed to send only as much data as is needed for the even= t. That > is, when sending the UHID_DESTROY event (which has no payload) it would b= e > actually enough to only send a __u32 field (ev->type) with write(). > This is not to speed up the write()s or read()s but rather to allow exten= ding > the uhid_event structure in the future. We can actually add arbitrary ext= ra > event-types and still keep ABI compatibility. Therefore, I thought of add= ing an > "uhid_version" field to UHID_CREATE so user-space can define what UHID pr= otocol > it is using. > > Regards > David > > David Herrmann (1): > =C2=A0HID: User-space I/O driver support for HID subsystem > > =C2=A0Documentation/hid/uhid.txt =C2=A0| =C2=A0152 ++++++++++++++ > =C2=A0drivers/hid/Kconfig =C2=A0 =C2=A0 =C2=A0 =C2=A0 | =C2=A0 21 ++ > =C2=A0drivers/hid/Makefile =C2=A0 =C2=A0 =C2=A0 =C2=A0| =C2=A0 =C2=A02 +- > =C2=A0drivers/hid/uhid.c =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0| =C2=A0473 ++= +++++++++++++++++++++++++++++++++++++++++ > =C2=A0include/linux/Kbuild =C2=A0 =C2=A0 =C2=A0 =C2=A0| =C2=A0 =C2=A01 + > =C2=A0include/linux/uhid.h =C2=A0 =C2=A0 =C2=A0 =C2=A0| =C2=A0 84 +++++++= + > =C2=A0samples/uhid/Makefile =C2=A0 =C2=A0 =C2=A0 | =C2=A0 10 + > =C2=A0samples/uhid/uhid-example.c | =C2=A0381 +++++++++++++++++++++++++++= +++++++ > =C2=A08 files changed, 1123 insertions(+), 1 deletion(-) > =C2=A0create mode 100644 Documentation/hid/uhid.txt > =C2=A0create mode 100644 drivers/hid/uhid.c > =C2=A0create mode 100644 include/linux/uhid.h > =C2=A0create mode 100644 samples/uhid/Makefile > =C2=A0create mode 100644 samples/uhid/uhid-example.c > > -- > 1.7.9.4 > --=20 Jo=C3=A3o Paulo Rechi Vita Openbossa Labs - INdT