On 16.08.2015 17:33, Andreas Mohr wrote: > Hi, > > [rogue out-of-band reply, sorry - lkml.org mail info is broken] Hi Andreas, > >> There are trackpoint devices that fail to respond to the PS2 command >> PSMOUSE_CMD_GETID if immediately queried after the parent device is >> deactivated. Add a small delay for the hardware to get in a sane state >> before sending any PS2 commands. > > Hmm, "deactivated"? > Probably a parent needs to be "activated" for a passthrough device > (child device?) > to be able to communicate? (I don't know much about these things though...) Comment from few lines above where I put the code. /* * If this is a pass-through port deactivate parent so the device * connected to this port can be successfully identified */ > > >> + usleep_range(10000, 15000); > > Ah, used _range() API - strong bonus points > for caring about wakeup minimization! :) > > (and I take it you surely cared to check proper device operation > at both cases of doing usleep() > with either upper or lower delay amount specified... ;) Yes, I went as low as 10ms and figured it still works. Then I added another 5ms if the kernel wants to wake up at a later time. If that's too much we can decrease the upper limit, although this should only happen once per boot. > > > > > In general it's somewhat sad > to see an unconditional implementation > via woefully imprecise delay-only operation here - > is there a way to have it implemented > as a properly *handshaked* protocol, > i.e. try (re-)doing some other query type > which would fail until init is ok or timeout? > OTOH in that case PSMOUSE_CMD_GETID probably happens to be > just that kind of "handshaked success" query type to be used here... Thought about that too, but I know too little about the hardware to give you a proper answer. I'm not even sure why PSMOUSE_CMD_GETID sometimes fails at all. I assume that hardware needs some time to settle down after the parent gets deactivated to accept commands. > > > Or, IOW (pseudo code): > > delay; > if (!query()) fail; > > sounds rather worse from a handshaked-protocol POV than > > while (retries_remaining) > if (query()) break; > delay; > > > This reasoning would probably suggest > that such a loop should be added *within* psmouse_probe(), at the first > check (i.e., PSMOUSE_CMD_GETID). > > OTOH that handshaked loop is only feasible > if doing repeated PSMOUSE_CMD_GETID query attempts > is actually supported (tolerated) by devices > (vs. no-op delaying and *then* trying one time only), > i.e. that repeated PSMOUSE_CMD_GETID queries will succeed > rather than fail or even block... > > > > BTW, to make it obvious why non-handshaked operation may easily end up worse: > certain devices (out of a couple thousands of different > China-made human interface device models > which will be relevant here ;) > might perhaps *require* getting queried immediately *without* any prior delay, > in which case the first (AND LAST!) PSMOUSE_CMD_GETID query > after the (your) delay > would now already fail for those devices... All you're saying is correct. Please note that I carefully limited the sleep to only SERIO_PS_PSTHRU devices, so the majority of devices out there is not going to see this at all. IIRC, PSMOUSE_CMD_GETID is the initial command to be sent to a PS2 device, so a delay before that should not do any harm. Of course I cannot prove that this will not have any side-effect on some random hardware, but I'm really trying to narrow down the number of affected devices to those who may profit from the change. Thanks for the feedback. Stefan -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
