Re: [PATCH] HID: multitouch: Add MT_QUIRK_FORCE_GET_FEATURE to MT_CLS_WIN_8 quirks

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 5/4/20 9:39 AM, Benjamin Tissoires wrote:
> Hi Hans,
> 
> On Sat, May 2, 2020 at 2:59 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi,
>>
>> On 5/1/20 8:20 PM, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 5/1/20 11:56 AM, Hans de Goede wrote:
>>>> The touchpad on the Dell Venue 11 Pro 7130's keyboard-dock is multi-touch
>>>> capable, using HID_GROUP_MULTITOUCH_WIN_8 and the hid-multitouch driver
>>>> correctly binds to it.
>>>>
>>>> But instead of getting multi-touch HID input reports we still get mouse
>>>> input reports and corresponding linux input (evdev) node events.
>>>>
>>>> Unloading and reloading the hid-multitouch driver works around this.
>>>>
>>>> Adding the MT_QUIRK_FORCE_GET_FEATURE quirk to the MT_CLS_WIN_8 quirks
>>>> makes the driver work correctly the first time it is loaded.
>>>>
>>>> I've chosen to add this quirk to the generic MT_CLS_WIN_8 quirks
>>>> because it seems unlikely that this quirk will causes problems for
>>>> other MT_CLS_WIN_8 devices and if this device needs it other Win 8
>>>> compatible devices might need it too.
>>>>
>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>
>>> Self nack for now, there are more issues with this detachable keyboard,
>>> it sometimes does not work after being unplugged and replugged again
>>> USB_QUIRK_DELAY_INIT seems to help a bit, but is not a total solution...
>>>
>>> Dell has some firmware updates for the kbd. So I'll install Windows and
>>> then update the firmware and we'll see from there.
>>
>> So after installing Windows it turns out that the kbd-dock firmware was
>> already fully up2date, what fun.
>>
>> So it took me quite a long time to get to the bottom of this.
>>
>> The problem is that the Dell K12A kbd-dock needs a HID_QUIRK_NO_INIT_REPORTS
>> quirk; or maybe both of HID_QUIRK_NO_INIT_REPORTS|HID_QUIRK_NOGET I've tested
> 
> I think this is a regression introduced by the high res scrolling
> patch. I have been notified that the new code actually does fetch all
> features on connect, which many devices do not support.
> 
> I don't think I received the patch related to that, but basically the
> problematic code is at
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/hid/hid-input.c#n1558
> 
> The issue is that we should only fetch the current report if the
> HID_GD_RESOLUTION_MULTIPLIER is present. Or we break things.

I don't think that this is related to the high-res scrolling stuff.

The errors I'm seeing on a bad hotplug are coming from
drivers/hid/hid-multitouch.c: mt_get_feature()

Also quite a few other multi-touch devices have a HID_QUIRK_NO_INIT_REPORTS
quirk, at least a bunch of surface keyboards do; and if I'm reading the
git log correctly then at one point in time we used to have a
HID_QUIRK_NO_INIT_REPORTS for at least some of the multi-touch classes
inside hid-multitouch.c. At least there is a commit titled:

"HID: multitouch: do not set HID_QUIRK_NO_INIT_REPORTS"

Which suggests that one point we did set it inside the multi-touch
driver; and I'm wondering since a bunch of surface keyboards need this
if setting this inside the multi-touch driver would not get us closer
to windows behavior.

Anyways if you have an alternative fix you want me to test, let me know.

Regards,

Hans




> 
> Cheers,
> Benjamin
> 
>> with the later version and that fixes both the touchpad initially being
>> stuck in mouse emulation and the dock misbehaving after a hot unplug + replug.
>>
>> I suspect I really only need HID_QUIRK_NO_INIT_REPORTS, I will retest with
>> that and submit a new patch replacing this one.
>>
>> Somewhat related: to make space for the Windows install I nuked the old
>> Fedora 27 install which was on the machine and after installing Windows
>> I did a fresh Fedora 32 install in the space which I left free when
>> installing Windows.
>>
>> This causes an interesting new problem. The touchpad worked fine
>> (with the quirk) in gdm, but it would stop working when I logged into
>> a user GNOME-session.  It took me a while to get to the bottom of
>> this. The problem is that the usersession ends up dbus activating
>> fwupd (probably through gnome-software) and fwupd does some probe
>> of the touchpad which puts it in a mode where it no longer generates
>> any events.
>>
>> sudo rpm -e fwupd gnome-software
>>
>> Works around this, so not a HID bug, but definitely something to keep
>> an eye out for if we get similar bug reports on other devices.
>>
>> I will mail the fwupd maintainer about this with you in the Cc.
>> Note this is an unrelated issue really, but I thought you
>> should be aware of this.
>>
>> Regards,
>>
>> Hans
>>
>>
>>
>>>> ---
>>>>    drivers/hid/hid-multitouch.c | 1 +
>>>>    1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
>>>> index 362805ddf377..f9c0429e7348 100644
>>>> --- a/drivers/hid/hid-multitouch.c
>>>> +++ b/drivers/hid/hid-multitouch.c
>>>> @@ -265,6 +265,7 @@ static const struct mt_class mt_classes[] = {
>>>>                MT_QUIRK_IGNORE_DUPLICATES |
>>>>                MT_QUIRK_HOVERING |
>>>>                MT_QUIRK_CONTACT_CNT_ACCURATE |
>>>> +            MT_QUIRK_FORCE_GET_FEATURE |
>>>>                MT_QUIRK_STICKY_FINGERS |
>>>>                MT_QUIRK_WIN8_PTP_BUTTONS,
>>>>            .export_all_inputs = true },
>>>>
>>
>