Re: [PATCH] HID: Support Microsoft Surface Duo SPI-based touch controller driver as a module.

[Felipe Balbi <balbi@kernel.org>]

Hi,

Benjamin Tissoires <benjamin.tissoires@redhat.com> writes:
>> > On Thu, Aug 12, 2021 at 2:13 AM Dmitry Antipov <daantipov@gmail.com> 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@microsoft.com
>> >
>> > 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

>> 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.

-- 
balbi