Hi,

On 5/13/20 10:00 AM, Benjamin Tissoires wrote:

Hi Hans,

[sorry for the delay, I am knee deep in fdo admin stuffs]

On Mon, May 4, 2020 at 11:31 AM Hans de Goede [off-list ref] wrote:

quoted

Hi,

On 5/4/20 9:39 AM, Benjamin Tissoires wrote:

quoted

Hi Hans,

On Sat, May 2, 2020 at 2:59 PM Hans de Goede [off-list ref] wrote:

quoted

Hi,

On 5/1/20 8:20 PM, Hans de Goede wrote:

quoted

Hi,

On 5/1/20 11:56 AM, Hans de Goede wrote:

quoted

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

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.

Well, it is in the way that the high-res scrolling changed the way we talked to HID device. Before that, I carefully ensured that hid-generic would never issue a get_feature or get_input, and after, it does.

quoted

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

It might be that the device doesn't like to be polled too often on one feature and gets in a stuck mode.

I don't think so. I believe the reason why I did not notice this before
is because I simply did not really test it before.

As I mentioned at the start of the thread, I noticed this while working
on SW_TABLET_MODE support, and thus doing a lot of attaching / detaching
of the keyboard. I believe that this issue has existed since we stopped
setting HID_QUIRK_NO_INIT_REPORTS in the hid-multitouch code.

Anyways the proof is in the pudding, see below.

quoted

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.

This quirk is legacy, and should have stayed that way (well, it doesn't
hurt anyway). As mentioned, in the past, the hid core stack used to fetch
all input and feature reports on plug in. This was known to break many
devices, and we had to use the no-init-report quirk for them. Then, we
realized that it was not correct to do that way, and I removed this
behavior. However, I couldn't remove the quirk entirely because of hiddev
IIRC. In the hiddev case, userspace expects the device to have known values
for the features, but that is not 100% working. So to not break userspace,
I had to keep that quirk around for this use case.

quoted

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

Peter has a patch, we were waiting for the reporter to test it, but it's
been crickets since last week.

Here it is:


Subject: [DRAFT PATCH] HID: input: do not run GET_REPORT unless there's a Resolution Multiplier

Ok, so I've given this a try with the HID_QUIRK_NO_INIT_REPORTS patch
I did reverted and the kbd and touchpad failed to send events after
the first detach + re-attach.

With the HID_QUIRK_NO_INIT_REPORTS patch in place detach + re-attach is
no problem.

Regards,

Hans