hidbus: stop interrupts before finalizing child deactivation

[kevans]

Summary

In the case of usbhid, the parent will drain any transfers that are
pending, which could lead to just a little bit of data being plumbed
through. Stop interrupts *before* we mark the child as inactive so that
anything that was inflight makes it through.

This primarily helps a little with debugging, so that we can tell if
data was dropped as a result of hid_intr_stop().

[aokblast]

sys/dev/hid/hidbus.c

639

I can't understand. If it is the last bit. We should clear the bit then do foreach check? Or it will never deactive?

[kevans]

sys/dev/hid/hidbus.c

639

Clearing the bit up-front means hidbus_intr won't drain any last minute response to the device driver thaat hid_intr_stop (doing a usbd_transfer_drain) triggers. The device driver will either be discarding them anyways (e.g., u2f(4), hidraw(4)) or it's harmless; in either case, it's useful for debugging to be able to observe the response at that level even if it wll be ignored.

[aokblast]

sys/dev/hid/hidbus.c

639

Ok. I think you want to trigger the hidbus_intr in the last minutes and let the decision pass to the underlying driver. But would it actually work? ivar->actiave = false set after the active |= tlc->active, it measn active would never equals to 0 if we don't deactive twice in the same node.

[kevans]

sys/dev/hid/hidbus.c

639

Right, that's why setting active in the loop iss predicated on tlc != ivar -- so that it's explicitly not accounted for, and if every other device is inactive then we will call hid_intr_stop() before clearing tlc->active

[aokblast]

LGTM. Sorry that I though it was copy and paste from line 633. It should work. Thanks!

[aokblast]

sys/dev/hid/hidbus.c

639

Do you think it worth a __unlikely here?