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.
So I suspect the command should be renamed to
#define ATKBD_CMD_SET_DEFAULTS_AND_DISABLE 0x00f5
#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?)
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", and I'm not at all
sure you should have used it for the psmouse_cleanup() function. 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 - that happened earlier in
psmouse_deactivate(), when you sent it the PSMOUSE_CMD_DISABLE command.
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?
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".
(You may have good reasons for having picked the "and disable" command. I
just wonder what they are)
Linus
--
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
