On Thu, Dec 31, 2009 at 11:50:59AM -0800, Linus Torvalds wrote: > > > On Wed, 30 Dec 2009, Dmitry Torokhov wrote: > > > > Dmitry Torokhov (7): > > Input: speed up suspend/shutdown for PS/2 mice and keyboards > > Btw, the thing I like best about this commit is how it adds some comments > on the keyboard commands. > > That said, the naming and the comments aren't all that wonderful. I don't > think you should use the word "RESET" for command f5/f6: the command is > called "Set Default" (and ".. Disable" for f5), and there is no actual > reset involved. > I guess it is just a matter of wording. For me "reset" and "revert to initial state" are pretty much the same. > So I suspect the command should be renamed to > > #define ATKBD_CMD_SET_DEFAULTS_AND_DISABLE 0x00f5 ATKBD_CMD_SET_DEFAULTS_AND_DISABLE_DATA_REPORTING :) I was just trying to follow the existing naming style. It was there before me and it does make sense to me still so I did not feel the neet to change it. > #define ATKBD_CMD_SET_DEFAULTS 0x00f6 > > and then the comments wouldn't even be needed (and anybody reading the > code would not have to look them up in the header file). > > And then "ATKBD_CMD_RESET_BAT" should probably just be "ATKBD_CMD_RESET" > (I wonder what the "BAT" part is all about?) That came from Vojtech but I am pretty sure this stands for Basic Assurance Test since full reset for touchpads for example involves recalibration. See this excerpt from Synaptics docs: "At power-on, the PS/2 TouchPad performs a self-test and calibration, then transmits the completion code $AA and ID code $00. If the TouchPad fails its self-test, it transmits error code $FC and ID code $00. This processing also occurs when a software Reset ($FF) command is received. The host should not attempt to send commands to the TouchPad until the calibration/self-test is complete." > > Oh, and command F5h is not always a "set defaults and disable". For some > (all?) PS/2 mice it seems to be _just_ a "disable", Exactly, that is why it is called PSMOUSE_CMD_DISABLE when used in mouse context. However snippets of docs I have indicate that for keyboards it also causes them to reset to defaults. > and I'm not at all > sure you should have used it for the psmouse_cleanup() function. I am using PSMOUSE_CMD_RESET_DIS (0xf6) in psmouse_cleanup(). > You used > to do a > > psmouse_reset(psmouse); > > which sent a full reset to the mouse (and waits for the two-byte ACK). It > did _not_ disable the mouse - It does actually, at least for some devices (incidentally the ones that may give trouble to the BIOS if left as is): "The reset state of the TouchPad is as follows: - Reported sample rate is 100 samples per second (see page 30). - Reported resolution is 4 counts per mm (see page 32). - Scaling is 1:1. - Stream mode is selected. - Data reporting is disabled. <--------------- - Absolute mode is disabled." > that happened earlier in > psmouse_deactivate(), when you sent it the PSMOUSE_CMD_DISABLE command. What happened before doing psmouse_reset() (which is PSMOUSE_CMD_RESET_BAT) should not matter because all previous state should be reset after it. > > You changed that 'psmouse_reset()' to > > /* > * Reset the mouse to defaults (bare PS/2 protocol). > */ > ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_RESET_DIS); > > which now disables it again, only to then later do > > /* > * Some boxes, such as HP nx7400, get terribly confused if mouse > * is not fully enabled before suspending/shutting down. > */ > ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_ENABLE); > > which seems odd. IOW, Why do you do a "set defaults and disable" followed > by a "enable", when you already had it disabled? We do not have "light reset" option, so we have to use "light reset and disable" to return to the bare 3-byte PS/2 protocol. Then we have to re-enable the device because BIOS on some HP notebooks would get confused and not suspend (or resume, I don't quite remember) if mouse left disabled. > > I think the PSMOUSE_CMD_RESET that _used_ to be there would re-enable most > mice, but now that you don't even do that any more, I get the feeling that > the "set defaults and disable" should be just a "set defaults". There is no such command for mice unfortunately. > > (You may have good reasons for having picked the "and disable" command. I > just wonder what they are) > > Linus -- Dmitry -- 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
