On Fri, Aug 13, 2021 at 7:13 AM Felipe Balbi <balbi@xxxxxxxxxx> wrote: > > > Hi, > > Benjamin Tissoires <benjamin.tissoires@xxxxxxxxxx> writes: > >> > On Thu, Aug 12, 2021 at 2:13 AM Dmitry Antipov <daantipov@xxxxxxxxx> wrote: > >> >> > >> >> Surface Duo uses a touch digitizer that communicates to the main SoC via SPI > >> >> and presents itself as a HID device. This patch contains the changes needed > >> >> for the driver to work as a module: HID Core affordances for SPI devices, > >> >> addition of the new Device IDs, and a new quirk in hid-microsoft. The driver > >> >> itself is being prepared for a submission in the near future. > >> >> > >> >> Signed-off-by: Dmitry Antipov dmanti@xxxxxxxxxxxxx > >> > > >> > Though I really appreciate seeing a microsoft.com submission, the > >> > commit description makes me wonder if we should not postpone the > >> > inclusion of this patch with the "submission in the near future". > >> > > >> > AFAIK, HID is not SPI aware. So basically, we are introducing dead > >> > code in the upstream kernel if we take this patch. > >> > >> Right, unfortunately spec isn't public yet (albeit having products > >> shipped using it and a driver available in a public tree somewhere I > >> couldn't find). > >> > >> I _do_ have one question though: > >> > >> Is there a way to tell hid core that $this device wants hidraw? If we > > > > Depending on what you want and can do I can think of several solutions: > > - add a quirk for this device (either at boot time, either in > > hid-quirks.c) There must be one that tells to only bind to hidraw > > - provide an out of the tree driver that declares the BUS:VID:PID, and > > start the HID device with HIDRAW only. > > sounds good I did some more digging this morning. The quirk option is not especially good, there is no "hidraw only" quirk. However, there is a "HID_QUIRK_HAVE_SPECIAL_DRIVER" that prevents hid-generic from binding to your device. This has the same effect as adding the device in the hid-quirks.c in this submission, so for development purposes, if the device is messed up by hid-generic, I would advise to add this quirk (maybe from the development spi transport driver as a temporary work around). To have the device bound to hidraw only, then yes, if you provide a simple out of the tree module that binds to this module, hid-generic will release the device and let this out of the tree module deal with it. AFAICT, the hid-core changes you are asking here should not block the development process if they are not merged. You'll get an "UNKOWN" bus in the logs, and the SPI_HID_DEVICE macro can be defined in the out of the tree driver. > > >> remove the hid-microsoft changes, hid-generic will pick the device as > >> expected, but this really needs hidraw. Any hints? > > > > I am fine with the hid-microsoft changes, I just want them in a > > separate commit. But I don't see why we would take only the first one > > without the SPI transport and the hid-microsoft code... > > > > Basically: not sure why you need hidraw for it. > > Yeah, the touch controller is "peculiar". It sends to the host raw > frames which needs to be processed by a userspace application. We don't > get coordinates, pressure, etc. We get raw values from the touch > digitizer; these are passed to a daemon which runs your usual filters > (palm rejection, denoising, yada yada yada) and produces the actual > touch inputs. Those are, in turn, given to uinput. > In that case, maybe hidraw is not the best way to forward the events. V4L has a capability to export raw touch events. You can have a look at drivers/input/rmi4/rmi_f54.c, drivers/input/touchscreen/sur40.c or drivers/input/touchscreen/atmel_mxt_ts.c for some examples. The nice thing is you'll get parallel processing and DMA between the kernel and userspace. Also, there must be userspace tools around that are already capable of dealing with that kind of input. Though I also understand the need to have your own sauce. Cheers, Benjamin
