Hi,
The main reason for this patch series is to deal with the case when
EC/keyboard controller has already latched a scancode in the output
buffer at the same time the host (kernel) is sending a PS/2 command to
the controller/device. The device should stop scanning (keyboard) or
sending coordinate data (mouse), and instead send acknowledge (0xfa) and
then potentially command response, but if the output buffer already
contains scancode byte it can not be substituted with an ACK byte.
The typical scenario for this is user activating a CapsLock function,
with host sending command to toggle CapsLock LED. If at the same time
the keyboard transmitting break code for the key the kernel may mistake
it for garbage command response and get confused.
To work around this scenario, instead of simply dropping the non-ACK/NAK
byte we will pass it on to atkbd/psmouse for normal processing.
In addition to the above there a couple more assorted cleanups and
fixes.
Thanks.
Dmitry Torokhov (7):
Input: libps2 - attach ps2dev instances as serio port's drvdata
Input: libps2 - remove special handling of ACK for command byte
Input: libps2 - rework handling of command response
Input: libps2 - fix NAK handling
Input: libps2 - fix aborting PS/2 commands
Input: libps2 - introduce common interrupt handler
Input: libps2 - do not discard non-ack bytes when controlling LEDs
drivers/input/keyboard/atkbd.c | 94 ++++-----
drivers/input/mouse/psmouse-base.c | 86 +++++----
drivers/input/mouse/psmouse.h | 2 +
drivers/input/mouse/synaptics.c | 10 +-
drivers/input/mouse/trackpoint.c | 2 +-
drivers/input/serio/libps2.c | 293 +++++++++++++++++++++--------
include/linux/libps2.h | 62 +++---
7 files changed, 350 insertions(+), 199 deletions(-)
--
Dmitry
When aborting PS/2 command the kernel should [re]set all flags before
waking up waiters, otherwise waiting thread may read obsolete values
of flags.
Reported-by: Raul Rangel <[email protected]>
Signed-off-by: Dmitry Torokhov <[email protected]>
---
drivers/input/serio/libps2.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/input/serio/libps2.c b/drivers/input/serio/libps2.c
index 14b70a78875d..09eb605364bb 100644
--- a/drivers/input/serio/libps2.c
+++ b/drivers/input/serio/libps2.c
@@ -478,15 +478,22 @@ bool ps2_handle_response(struct ps2dev *ps2dev, u8 data)
}
EXPORT_SYMBOL(ps2_handle_response);
+/*
+ * Clears state of PS/2 device after communication error by resetting majority
+ * of flags and waking up waiters, if any.
+ */
void ps2_cmd_aborted(struct ps2dev *ps2dev)
{
- if (ps2dev->flags & PS2_FLAG_ACK)
+ unsigned long old_flags = ps2dev->flags;
+
+ /* reset all flags except last nak */
+ ps2dev->flags &= PS2_FLAG_NAK;
+
+ if (old_flags & PS2_FLAG_ACK)
ps2dev->nak = 1;
- if (ps2dev->flags & (PS2_FLAG_ACK | PS2_FLAG_CMD))
+ if (old_flags & (PS2_FLAG_ACK | PS2_FLAG_CMD))
wake_up(&ps2dev->wait);
- /* reset all flags except last nack */
- ps2dev->flags &= PS2_FLAG_NAK;
}
EXPORT_SYMBOL(ps2_cmd_aborted);
--
2.40.1.606.ga4b1b128d6-goog
Upon receiving a PS/2 command the device and controller are supposed to
stop sending normal data (scancodes or movement packets) and instead
immediately start delivering ACK/NAK and command response. Unfortunately
often EC has an output buffer which may contain latched data by the time
the EC receives a command from the host. The kernel used to ignore such
data, but that may cause "stuck" keys if the data dropped happens to be a
break code or a part of a break code. This occasionally happens, for
example, on Chromebooks when the kernel tries to toggle CapsLock LED on
a keyboard while user releases Alt+Search keyboard shortcut.
Fix this by passing the first non-ACK byte to the normal handler for a
handful of PS/2 commands that are expected to be used during normal device
operation (as opposed to probe/configuration time).
Signed-off-by: Dmitry Torokhov <[email protected]>
---
drivers/input/serio/libps2.c | 36 ++++++++++++++++++++++++++++++++----
1 file changed, 32 insertions(+), 4 deletions(-)
diff --git a/drivers/input/serio/libps2.c b/drivers/input/serio/libps2.c
index 7c5fc853072a..6d78a1fe00c1 100644
--- a/drivers/input/serio/libps2.c
+++ b/drivers/input/serio/libps2.c
@@ -21,7 +21,10 @@
#define PS2_CMD_SETSCALE11 0x00e6
#define PS2_CMD_SETRES 0x10e8
+#define PS2_CMD_EX_SETLEDS 0x20eb
+#define PS2_CMD_SETLEDS 0x10ed
#define PS2_CMD_GETID 0x02f2
+#define PS2_CMD_SETREP 0x10f3 /* Set repeat rate/set report rate */
#define PS2_CMD_RESET_BAT 0x02ff
#define PS2_RET_BAT 0xaa
@@ -35,6 +38,7 @@
#define PS2_FLAG_CMD1 BIT(2) /* Waiting for the first byte of command response */
#define PS2_FLAG_WAITID BIT(3) /* Command executing is GET ID */
#define PS2_FLAG_NAK BIT(4) /* Last transmission was NAKed */
+#define PS2_FLAG_PASS_NOACK BIT(5) /* Pass non-ACK byte to receive handler */
static int ps2_do_sendbyte(struct ps2dev *ps2dev, u8 byte,
unsigned int timeout, unsigned int max_attempts)
@@ -281,9 +285,28 @@ int __ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command)
serio_pause_rx(ps2dev->serio);
- /* Some mice do not ACK the "get ID" command, prepare to handle this. */
- ps2dev->flags = command == PS2_CMD_GETID ? PS2_FLAG_WAITID : 0;
ps2dev->cmdcnt = receive;
+
+ switch (command) {
+ case PS2_CMD_GETID:
+ /*
+ * Some mice do not ACK the "get ID" command, prepare to
+ * handle this.
+ */
+ ps2dev->flags = PS2_FLAG_WAITID;
+ break;
+
+ case PS2_CMD_SETLEDS:
+ case PS2_CMD_EX_SETLEDS:
+ case PS2_CMD_SETREP:
+ ps2dev->flags = PS2_FLAG_PASS_NOACK;
+ break;
+
+ default:
+ ps2dev->flags = 0;
+ break;
+ }
+
if (receive) {
/* Indicate that we expect response to the command. */
ps2dev->flags |= PS2_FLAG_CMD | PS2_FLAG_CMD1;
@@ -512,14 +535,19 @@ static void ps2_handle_ack(struct ps2dev *ps2dev, u8 data)
* Do not signal errors if we get unexpected reply while
* waiting for an ACK to the initial (first) command byte:
* the device might not be quiesced yet and continue
- * delivering data.
+ * delivering data. For certain commands (such as set leds and
+ * set repeat rate) that can be used during normal device
+ * operation, we even pass this data byte to the normal receive
+ * handler.
* Note that we reset PS2_FLAG_WAITID flag, so the workaround
* for mice not acknowledging the Get ID command only triggers
* on the 1st byte; if device spews data we really want to see
* a real ACK from it.
*/
dev_dbg(&ps2dev->serio->dev, "unexpected %#02x\n", data);
- ps2dev->flags &= ~PS2_FLAG_WAITID;
+ if (ps2dev->flags & PS2_FLAG_PASS_NOACK)
+ ps2dev->receive_handler(ps2dev, data);
+ ps2dev->flags &= ~(PS2_FLAG_WAITID | PS2_FLAG_PASS_NOACK);
return;
}
--
2.40.1.606.ga4b1b128d6-goog
Do not try to process "resend" or "reject" responses from the device
as normal response data for a command.
Signed-off-by: Dmitry Torokhov <[email protected]>
---
drivers/input/serio/libps2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/serio/libps2.c b/drivers/input/serio/libps2.c
index d09450eca9a7..14b70a78875d 100644
--- a/drivers/input/serio/libps2.c
+++ b/drivers/input/serio/libps2.c
@@ -445,7 +445,7 @@ bool ps2_handle_ack(struct ps2dev *ps2dev, u8 data)
ps2dev->flags &= ~PS2_FLAG_ACK;
wake_up(&ps2dev->wait);
- if (data != PS2_RET_ACK)
+ if (!ps2dev->nak && data != PS2_RET_ACK)
ps2_handle_response(ps2dev, data);
return true;
--
2.40.1.606.ga4b1b128d6-goog
Instead of exposing inner workings of libps2 to drivers such as atkbd and
psmouse, have them define pre-receive and receive callbacks, and provide a
common handler that can be used with underlying serio port.
While at this add kerneldoc to the module.
Signed-off-by: Dmitry Torokhov <[email protected]>
---
drivers/input/keyboard/atkbd.c | 73 +++++-----
drivers/input/mouse/psmouse-base.c | 53 +++----
drivers/input/serio/libps2.c | 226 ++++++++++++++++++++---------
include/linux/libps2.h | 61 +++++---
4 files changed, 259 insertions(+), 154 deletions(-)
diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index 2fb2ad73e796..8ef663a589b3 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -398,47 +398,49 @@ static unsigned int atkbd_compat_scancode(struct atkbd *atkbd, unsigned int code
return code;
}
-/*
- * atkbd_interrupt(). Here takes place processing of data received from
- * the keyboard into events.
- */
-
-static irqreturn_t atkbd_interrupt(struct serio *serio, unsigned char data,
- unsigned int flags)
+static enum ps2_disposition atkbd_pre_receive_byte(struct ps2dev *ps2dev,
+ u8 data, unsigned int flags)
{
- struct atkbd *atkbd = atkbd_from_serio(serio);
- struct input_dev *dev = atkbd->dev;
- unsigned int code = data;
- int scroll = 0, hscroll = 0, click = -1;
- int value;
- unsigned short keycode;
+ struct serio *serio = ps2dev->serio;
dev_dbg(&serio->dev, "Received %02x flags %02x\n", data, flags);
#if !defined(__i386__) && !defined (__x86_64__)
- if ((flags & (SERIO_FRAME | SERIO_PARITY)) && (~flags & SERIO_TIMEOUT) && !atkbd->resend && atkbd->write) {
- dev_warn(&serio->dev, "Frame/parity error: %02x\n", flags);
- serio_write(serio, ATKBD_CMD_RESEND);
- atkbd->resend = true;
- goto out;
+ if ((flags & (SERIO_FRAME | SERIO_PARITY)) &&
+ (~flags & SERIO_TIMEOUT)) {
+ struct atkbd *atkbd = container_of(ps2dev, struct atkbd,
+ ps2dev);
+
+ if (!atkbd->resend && atkbd->write) {
+ dev_warn(&serio->dev,
+ "Frame/parity error: %02x\n", flags);
+ serio_write(serio, ATKBD_CMD_RESEND);
+ atkbd->resend = true;
+ return PS2_IGNORE;
+ }
}
if (!flags && data == ATKBD_RET_ACK)
atkbd->resend = false;
#endif
- if (unlikely(atkbd->ps2dev.flags & PS2_FLAG_ACK))
- if (ps2_handle_ack(&atkbd->ps2dev, data))
- goto out;
+ return PS2_PROCESS;
+}
- if (unlikely(atkbd->ps2dev.flags & PS2_FLAG_CMD))
- if (ps2_handle_response(&atkbd->ps2dev, data))
- goto out;
+static void atkbd_receive_byte(struct ps2dev *ps2dev, u8 data)
+{
+ struct serio *serio = ps2dev->serio;
+ struct atkbd *atkbd = container_of(ps2dev, struct atkbd, ps2dev);
+ struct input_dev *dev = atkbd->dev;
+ unsigned int code = data;
+ int scroll = 0, hscroll = 0, click = -1;
+ int value;
+ unsigned short keycode;
pm_wakeup_event(&serio->dev, 0);
if (!atkbd->enabled)
- goto out;
+ return;
input_event(dev, EV_MSC, MSC_RAW, code);
@@ -460,16 +462,16 @@ static irqreturn_t atkbd_interrupt(struct serio *serio, unsigned char data,
case ATKBD_RET_BAT:
atkbd->enabled = false;
serio_reconnect(atkbd->ps2dev.serio);
- goto out;
+ return;
case ATKBD_RET_EMUL0:
atkbd->emul = 1;
- goto out;
+ return;
case ATKBD_RET_EMUL1:
atkbd->emul = 2;
- goto out;
+ return;
case ATKBD_RET_RELEASE:
atkbd->release = true;
- goto out;
+ return;
case ATKBD_RET_ACK:
case ATKBD_RET_NAK:
if (printk_ratelimit())
@@ -477,18 +479,18 @@ static irqreturn_t atkbd_interrupt(struct serio *serio, unsigned char data,
"Spurious %s on %s. "
"Some program might be trying to access hardware directly.\n",
data == ATKBD_RET_ACK ? "ACK" : "NAK", serio->phys);
- goto out;
+ return;
case ATKBD_RET_ERR:
atkbd->err_count++;
dev_dbg(&serio->dev, "Keyboard on %s reports too many keys pressed.\n",
serio->phys);
- goto out;
+ return;
}
code = atkbd_compat_scancode(atkbd, code);
if (atkbd->emul && --atkbd->emul)
- goto out;
+ return;
keycode = atkbd->keycode[code];
@@ -564,8 +566,6 @@ static irqreturn_t atkbd_interrupt(struct serio *serio, unsigned char data,
}
atkbd->release = false;
-out:
- return IRQ_HANDLED;
}
static int atkbd_set_repeat_rate(struct atkbd *atkbd)
@@ -1229,7 +1229,8 @@ static int atkbd_connect(struct serio *serio, struct serio_driver *drv)
goto fail1;
atkbd->dev = dev;
- ps2_init(&atkbd->ps2dev, serio);
+ ps2_init(&atkbd->ps2dev, serio,
+ atkbd_pre_receive_byte, atkbd_receive_byte);
INIT_DELAYED_WORK(&atkbd->event_work, atkbd_event_work);
mutex_init(&atkbd->mutex);
@@ -1385,7 +1386,7 @@ static struct serio_driver atkbd_drv = {
},
.description = DRIVER_DESC,
.id_table = atkbd_serio_ids,
- .interrupt = atkbd_interrupt,
+ .interrupt = ps2_interrupt,
.connect = atkbd_connect,
.reconnect = atkbd_reconnect,
.disconnect = atkbd_disconnect,
diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
index ed5376099fba..a0aac76b1e41 100644
--- a/drivers/input/mouse/psmouse-base.c
+++ b/drivers/input/mouse/psmouse-base.c
@@ -336,17 +336,14 @@ static void psmouse_handle_oob_data(struct psmouse *psmouse, u8 data)
}
}
-/*
- * psmouse_interrupt() handles incoming characters, either passing them
- * for normal processing or gathering them as command response.
- */
-static irqreturn_t psmouse_interrupt(struct serio *serio,
- u8 data, unsigned int flags)
+static enum ps2_disposition psmouse_pre_receive_byte(struct ps2dev *ps2dev,
+ u8 data,
+ unsigned int flags)
{
- struct psmouse *psmouse = psmouse_from_serio(serio);
+ struct psmouse *psmouse = container_of(ps2dev, struct psmouse, ps2dev);
if (psmouse->state == PSMOUSE_IGNORE)
- goto out;
+ return PS2_IGNORE;
if (unlikely((flags & SERIO_TIMEOUT) ||
((flags & SERIO_PARITY) &&
@@ -357,27 +354,25 @@ static irqreturn_t psmouse_interrupt(struct serio *serio,
"bad data from KBC -%s%s\n",
flags & SERIO_TIMEOUT ? " timeout" : "",
flags & SERIO_PARITY ? " bad parity" : "");
- ps2_cmd_aborted(&psmouse->ps2dev);
- goto out;
+ return PS2_ERROR;
}
if (flags & SERIO_OOB_DATA) {
psmouse_handle_oob_data(psmouse, data);
- goto out;
+ return PS2_IGNORE;
}
- if (unlikely(psmouse->ps2dev.flags & PS2_FLAG_ACK))
- if (ps2_handle_ack(&psmouse->ps2dev, data))
- goto out;
+ return PS2_PROCESS;
+}
- if (unlikely(psmouse->ps2dev.flags & PS2_FLAG_CMD))
- if (ps2_handle_response(&psmouse->ps2dev, data))
- goto out;
+static void psmouse_receive_byte(struct ps2dev *ps2dev, u8 data)
+{
+ struct psmouse *psmouse = container_of(ps2dev, struct psmouse, ps2dev);
- pm_wakeup_event(&serio->dev, 0);
+ pm_wakeup_event(&ps2dev->serio->dev, 0);
if (psmouse->state <= PSMOUSE_RESYNCING)
- goto out;
+ return;
if (psmouse->state == PSMOUSE_ACTIVATED &&
psmouse->pktcnt && time_after(jiffies, psmouse->last + HZ/2)) {
@@ -386,7 +381,7 @@ static irqreturn_t psmouse_interrupt(struct serio *serio,
psmouse->badbyte = psmouse->packet[0];
__psmouse_set_state(psmouse, PSMOUSE_RESYNCING);
psmouse_queue_work(psmouse, &psmouse->resync_work, 0);
- goto out;
+ return;
}
psmouse->packet[psmouse->pktcnt++] = data;
@@ -395,21 +390,21 @@ static irqreturn_t psmouse_interrupt(struct serio *serio,
if (unlikely(psmouse->packet[0] == PSMOUSE_RET_BAT && psmouse->pktcnt <= 2)) {
if (psmouse->pktcnt == 1) {
psmouse->last = jiffies;
- goto out;
+ return;
}
if (psmouse->packet[1] == PSMOUSE_RET_ID ||
(psmouse->protocol->type == PSMOUSE_HGPK &&
psmouse->packet[1] == PSMOUSE_RET_BAT)) {
__psmouse_set_state(psmouse, PSMOUSE_IGNORE);
- serio_reconnect(serio);
- goto out;
+ serio_reconnect(ps2dev->serio);
+ return;
}
/* Not a new device, try processing first byte normally */
psmouse->pktcnt = 1;
if (psmouse_handle_byte(psmouse))
- goto out;
+ return;
psmouse->packet[psmouse->pktcnt++] = data;
}
@@ -424,14 +419,11 @@ static irqreturn_t psmouse_interrupt(struct serio *serio,
psmouse->badbyte = psmouse->packet[0];
__psmouse_set_state(psmouse, PSMOUSE_RESYNCING);
psmouse_queue_work(psmouse, &psmouse->resync_work, 0);
- goto out;
+ return;
}
psmouse->last = jiffies;
psmouse_handle_byte(psmouse);
-
- out:
- return IRQ_HANDLED;
}
/*
@@ -1604,7 +1596,8 @@ static int psmouse_connect(struct serio *serio, struct serio_driver *drv)
if (!psmouse || !input_dev)
goto err_free;
- ps2_init(&psmouse->ps2dev, serio);
+ ps2_init(&psmouse->ps2dev, serio,
+ psmouse_pre_receive_byte, psmouse_receive_byte);
INIT_DELAYED_WORK(&psmouse->resync_work, psmouse_resync);
psmouse->dev = input_dev;
snprintf(psmouse->phys, sizeof(psmouse->phys), "%s/input0", serio->phys);
@@ -1786,7 +1779,7 @@ static struct serio_driver psmouse_drv = {
},
.description = DRIVER_DESC,
.id_table = psmouse_serio_ids,
- .interrupt = psmouse_interrupt,
+ .interrupt = ps2_interrupt,
.connect = psmouse_connect,
.reconnect = psmouse_reconnect,
.fast_reconnect = psmouse_fast_reconnect,
diff --git a/drivers/input/serio/libps2.c b/drivers/input/serio/libps2.c
index 09eb605364bb..7c5fc853072a 100644
--- a/drivers/input/serio/libps2.c
+++ b/drivers/input/serio/libps2.c
@@ -19,9 +19,22 @@
#define DRIVER_DESC "PS/2 driver library"
-MODULE_AUTHOR("Dmitry Torokhov <[email protected]>");
-MODULE_DESCRIPTION("PS/2 driver library");
-MODULE_LICENSE("GPL");
+#define PS2_CMD_SETSCALE11 0x00e6
+#define PS2_CMD_SETRES 0x10e8
+#define PS2_CMD_GETID 0x02f2
+#define PS2_CMD_RESET_BAT 0x02ff
+
+#define PS2_RET_BAT 0xaa
+#define PS2_RET_ID 0x00
+#define PS2_RET_ACK 0xfa
+#define PS2_RET_NAK 0xfe
+#define PS2_RET_ERR 0xfc
+
+#define PS2_FLAG_ACK BIT(0) /* Waiting for ACK/NAK */
+#define PS2_FLAG_CMD BIT(1) /* Waiting for a command to finish */
+#define PS2_FLAG_CMD1 BIT(2) /* Waiting for the first byte of command response */
+#define PS2_FLAG_WAITID BIT(3) /* Command executing is GET ID */
+#define PS2_FLAG_NAK BIT(4) /* Last transmission was NAKed */
static int ps2_do_sendbyte(struct ps2dev *ps2dev, u8 byte,
unsigned int timeout, unsigned int max_attempts)
@@ -76,14 +89,17 @@ static int ps2_do_sendbyte(struct ps2dev *ps2dev, u8 byte,
return error;
}
-/*
- * ps2_sendbyte() sends a byte to the device and waits for acknowledge.
- * It doesn't handle retransmission, the caller is expected to handle
+/**
+ * ps2_sendbyte - sends a byte to the device and wait for acknowledgement
+ * @ps2dev: a PS/2 device to send the data to
+ * @byte: data to be sent to the device
+ * @timeout: timeout for sending the data and receiving an acknowledge
+ *
+ * The function doesn't handle retransmission, the caller is expected to handle
* it when needed.
*
* ps2_sendbyte() can only be called from a process context.
*/
-
int ps2_sendbyte(struct ps2dev *ps2dev, u8 byte, unsigned int timeout)
{
int retval;
@@ -99,6 +115,13 @@ int ps2_sendbyte(struct ps2dev *ps2dev, u8 byte, unsigned int timeout)
}
EXPORT_SYMBOL(ps2_sendbyte);
+/**
+ * ps2_begin_command - mark beginning of execution of a complex command
+ * @ps2dev: a PS/2 device executing the command
+ *
+ * Serializes a complex/compound command. Once command is finished
+ * ps2_end_command() should be called.
+ */
void ps2_begin_command(struct ps2dev *ps2dev)
{
struct mutex *m = ps2dev->serio->ps2_cmd_mutex ?: &ps2dev->cmd_mutex;
@@ -107,6 +130,10 @@ void ps2_begin_command(struct ps2dev *ps2dev)
}
EXPORT_SYMBOL(ps2_begin_command);
+/**
+ * ps2_end_command - mark end of execution of a complex command
+ * @ps2dev: a PS/2 device executing the command
+ */
void ps2_end_command(struct ps2dev *ps2dev)
{
struct mutex *m = ps2dev->serio->ps2_cmd_mutex ?: &ps2dev->cmd_mutex;
@@ -115,11 +142,13 @@ void ps2_end_command(struct ps2dev *ps2dev)
}
EXPORT_SYMBOL(ps2_end_command);
-/*
- * ps2_drain() waits for device to transmit requested number of bytes
- * and discards them.
+/**
+ * ps2_drain - waits for device to transmit requested number of bytes
+ * and discards them
+ * @ps2dev: the PS/2 device that should be drained
+ * @maxbytes: maximum number of bytes to be drained
+ * @timeout: time to drain the device
*/
-
void ps2_drain(struct ps2dev *ps2dev, size_t maxbytes, unsigned int timeout)
{
if (maxbytes > sizeof(ps2dev->cmdbuf)) {
@@ -142,11 +171,11 @@ void ps2_drain(struct ps2dev *ps2dev, size_t maxbytes, unsigned int timeout)
}
EXPORT_SYMBOL(ps2_drain);
-/*
- * ps2_is_keyboard_id() checks received ID byte against the list of
- * known keyboard IDs.
+/**
+ * ps2_is_keyboard_id - checks received ID byte against the list of
+ * known keyboard IDs
+ * @id_byte: data byte that should be checked
*/
-
bool ps2_is_keyboard_id(u8 id_byte)
{
static const u8 keyboard_ids[] = {
@@ -167,7 +196,6 @@ EXPORT_SYMBOL(ps2_is_keyboard_id);
* response and tries to reduce remaining timeout to speed up command
* completion.
*/
-
static int ps2_adjust_timeout(struct ps2dev *ps2dev,
unsigned int command, unsigned int timeout)
{
@@ -217,13 +245,19 @@ static int ps2_adjust_timeout(struct ps2dev *ps2dev,
return timeout;
}
-/*
- * ps2_command() sends a command and its parameters to the mouse,
- * then waits for the response and puts it in the param array.
+/**
+ * __ps2_command - send a command to PS/2 device
+ * @ps2dev: the PS/2 device that should execute the command
+ * @param: a buffer containing parameters to be sent along with the command,
+ * or place where the results of the command execution will be deposited,
+ * or both
+ * @command: command word that encodes the command itself, as well as number of
+ * additional parameter bytes that should be sent to the device and expected
+ * length of the command response
*
- * ps2_command() can only be called from a process context
+ * Not serialized. Callers should use ps2_begin_command() and ps2_end_command()
+ * to ensure proper serialization for complex commands.
*/
-
int __ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command)
{
unsigned int timeout;
@@ -327,6 +361,20 @@ int __ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command)
}
EXPORT_SYMBOL(__ps2_command);
+/**
+ * ps2_command - send a command to PS/2 device
+ * @ps2dev: the PS/2 device that should execute the command
+ * @param: a buffer containing parameters to be sent along with the command,
+ * or place where the results of the command execution will be deposited,
+ * or both
+ * @command: command word that encodes the command itself, as well as number of
+ * additional parameter bytes that should be sent to the device and expected
+ * length of the command response
+ *
+ * Note: ps2_command() serializes the command execution so that only one
+ * command can be executed at a time for either individual port or the entire
+ * 8042 controller.
+ */
int ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command)
{
int rc;
@@ -339,14 +387,16 @@ int ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command)
}
EXPORT_SYMBOL(ps2_command);
-/*
- * ps2_sliced_command() sends an extended PS/2 command to the mouse
- * using sliced syntax, understood by advanced devices, such as Logitech
- * or Synaptics touchpads. The command is encoded as:
+/**
+ * ps2_sliced_command - sends an extended PS/2 command to a mouse
+ * @ps2dev: the PS/2 device that should execute the command
+ * @command: command byte
+ *
+ * The command is sent using "sliced" syntax understood by advanced devices,
+ * such as Logitech or Synaptics touchpads. The command is encoded as:
* 0xE6 0xE8 rr 0xE8 ss 0xE8 tt 0xE8 uu where (rr*64)+(ss*16)+(tt*4)+uu
* is the command.
*/
-
int ps2_sliced_command(struct ps2dev *ps2dev, u8 command)
{
int i;
@@ -372,12 +422,22 @@ int ps2_sliced_command(struct ps2dev *ps2dev, u8 command)
}
EXPORT_SYMBOL(ps2_sliced_command);
-/*
- * ps2_init() initializes ps2dev structure
+/**
+ * ps2_init - initializes ps2dev structure
+ * @ps2dev: structure to be initialized
+ * @serio: serio port associated with the PS/2 device
+ * @pre_receive_handler: validation handler to check basic communication state
+ * @receive_handler: main protocol handler
+ *
+ * Prepares ps2dev structure for use in drivers for PS/2 devices.
*/
-
-void ps2_init(struct ps2dev *ps2dev, struct serio *serio)
+void ps2_init(struct ps2dev *ps2dev, struct serio *serio,
+ ps2_pre_receive_handler_t pre_receive_handler,
+ ps2_receive_handler_t receive_handler)
{
+ ps2dev->pre_receive_handler = pre_receive_handler;
+ ps2dev->receive_handler = receive_handler;
+
mutex_init(&ps2dev->cmd_mutex);
lockdep_set_subclass(&ps2dev->cmd_mutex, serio->depth);
init_waitqueue_head(&ps2dev->wait);
@@ -387,11 +447,35 @@ void ps2_init(struct ps2dev *ps2dev, struct serio *serio)
EXPORT_SYMBOL(ps2_init);
/*
- * ps2_handle_ack() is supposed to be used in interrupt handler
- * to properly process ACK/NAK of a command from a PS/2 device.
+ * ps2_handle_response() stores device's response to a command and notifies
+ * the process waiting for completion of the command. Note that there is a
+ * distinction between waiting for the first byte of the response, and
+ * waiting for subsequent bytes. It is done so that callers could shorten
+ * timeouts once first byte of response is received.
*/
+static void ps2_handle_response(struct ps2dev *ps2dev, u8 data)
+{
+ if (ps2dev->cmdcnt)
+ ps2dev->cmdbuf[--ps2dev->cmdcnt] = data;
-bool ps2_handle_ack(struct ps2dev *ps2dev, u8 data)
+ if (ps2dev->flags & PS2_FLAG_CMD1) {
+ ps2dev->flags &= ~PS2_FLAG_CMD1;
+ if (ps2dev->cmdcnt)
+ wake_up(&ps2dev->wait);
+ }
+
+ if (!ps2dev->cmdcnt) {
+ ps2dev->flags &= ~PS2_FLAG_CMD;
+ wake_up(&ps2dev->wait);
+ }
+}
+
+/*
+ * ps2_handle_ack() processes ACK/NAK of a command from a PS/2 device,
+ * possibly applying workarounds for mice not acknowledging the "get ID"
+ * command.
+ */
+static void ps2_handle_ack(struct ps2dev *ps2dev, u8 data)
{
switch (data) {
case PS2_RET_ACK:
@@ -436,53 +520,25 @@ bool ps2_handle_ack(struct ps2dev *ps2dev, u8 data)
*/
dev_dbg(&ps2dev->serio->dev, "unexpected %#02x\n", data);
ps2dev->flags &= ~PS2_FLAG_WAITID;
- return true;
+ return;
}
if (!ps2dev->nak)
ps2dev->flags &= ~PS2_FLAG_NAK;
ps2dev->flags &= ~PS2_FLAG_ACK;
- wake_up(&ps2dev->wait);
if (!ps2dev->nak && data != PS2_RET_ACK)
ps2_handle_response(ps2dev, data);
-
- return true;
-}
-EXPORT_SYMBOL(ps2_handle_ack);
-
-/*
- * ps2_handle_response() is supposed to be used in interrupt handler
- * to properly store device's response to a command and notify process
- * waiting for completion of the command.
- */
-
-bool ps2_handle_response(struct ps2dev *ps2dev, u8 data)
-{
- if (ps2dev->cmdcnt)
- ps2dev->cmdbuf[--ps2dev->cmdcnt] = data;
-
- if (ps2dev->flags & PS2_FLAG_CMD1) {
- ps2dev->flags &= ~PS2_FLAG_CMD1;
- if (ps2dev->cmdcnt)
- wake_up(&ps2dev->wait);
- }
-
- if (!ps2dev->cmdcnt) {
- ps2dev->flags &= ~PS2_FLAG_CMD;
+ else
wake_up(&ps2dev->wait);
- }
-
- return true;
}
-EXPORT_SYMBOL(ps2_handle_response);
/*
* Clears state of PS/2 device after communication error by resetting majority
* of flags and waking up waiters, if any.
*/
-void ps2_cmd_aborted(struct ps2dev *ps2dev)
+static void ps2_cleanup(struct ps2dev *ps2dev)
{
unsigned long old_flags = ps2dev->flags;
@@ -494,6 +550,46 @@ void ps2_cmd_aborted(struct ps2dev *ps2dev)
if (old_flags & (PS2_FLAG_ACK | PS2_FLAG_CMD))
wake_up(&ps2dev->wait);
+}
+/**
+ * ps2_interrupt - common interrupt handler for PS/2 devices
+ * @serio: serio port for the device
+ * @data: a data byte received from the device
+ * @flags: flags such as %SERIO_PARITY or %SERIO_TIMEOUT indicating state of
+ * the data transfer
+ *
+ * ps2_interrupt() invokes pre-receive handler, optionally handles command
+ * acknowledgement and response from the device, and finally passes the data
+ * to the main protocol handler for future processing.
+ */
+irqreturn_t ps2_interrupt(struct serio *serio, u8 data, unsigned int flags) {
+ struct ps2dev *ps2dev = serio_get_drvdata(serio);
+ enum ps2_disposition rc;
+
+ rc = ps2dev->pre_receive_handler(ps2dev, data, flags);
+ switch (rc) {
+ case PS2_ERROR:
+ ps2_cleanup(ps2dev);
+ break;
+
+ case PS2_IGNORE:
+ break;
+
+ case PS2_PROCESS:
+ if (ps2dev->flags & PS2_FLAG_ACK)
+ ps2_handle_ack(ps2dev, data);
+ else if (ps2dev->flags & PS2_FLAG_CMD)
+ ps2_handle_response(ps2dev, data);
+ else
+ ps2dev->receive_handler(ps2dev, data);
+ break;
+ }
+
+ return IRQ_HANDLED;
}
-EXPORT_SYMBOL(ps2_cmd_aborted);
+EXPORT_SYMBOL(ps2_interrupt);
+
+MODULE_AUTHOR("Dmitry Torokhov <[email protected]>");
+MODULE_DESCRIPTION("PS/2 driver library");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/libps2.h b/include/linux/libps2.h
index 193dd53ad18b..9ca9ce4e6e64 100644
--- a/include/linux/libps2.h
+++ b/include/linux/libps2.h
@@ -8,43 +8,59 @@
*/
#include <linux/bitops.h>
+#include <linux/interrupt.h>
#include <linux/mutex.h>
#include <linux/types.h>
#include <linux/wait.h>
-#define PS2_CMD_SETSCALE11 0x00e6
-#define PS2_CMD_SETRES 0x10e8
-#define PS2_CMD_GETID 0x02f2
-#define PS2_CMD_RESET_BAT 0x02ff
+struct ps2dev;
-#define PS2_RET_BAT 0xaa
-#define PS2_RET_ID 0x00
-#define PS2_RET_ACK 0xfa
-#define PS2_RET_NAK 0xfe
-#define PS2_RET_ERR 0xfc
+/**
+ * enum ps2_disposition - indicates how received byte should be handled
+ * @PS2_PROCESS: pass to the main protocol handler, process normally
+ * @PS2_IGNORE: skip the byte
+ * @PS2_ERROR: do not process the byte, abort command in progress
+ */
+enum ps2_disposition {
+ PS2_PROCESS,
+ PS2_IGNORE,
+ PS2_ERROR,
+};
-#define PS2_FLAG_ACK BIT(0) /* Waiting for ACK/NAK */
-#define PS2_FLAG_CMD BIT(1) /* Waiting for a command to finish */
-#define PS2_FLAG_CMD1 BIT(2) /* Waiting for the first byte of command response */
-#define PS2_FLAG_WAITID BIT(3) /* Command executing is GET ID */
-#define PS2_FLAG_NAK BIT(4) /* Last transmission was NAKed */
+typedef enum ps2_disposition (*ps2_pre_receive_handler_t)(struct ps2dev *, u8,
+ unsigned int);
+typedef void (*ps2_receive_handler_t)(struct ps2dev *, u8);
+/**
+ * struct ps2dev - represents a device using PS/2 protocol
+ * @serio: a serio port used by the PS/2 device
+ * @cmd_mutex: a mutex ensuring that only one command is executing at a time
+ * @wait: a waitqueue used to signal completion from the serio interrupt handler
+ * @flags: various internal flags indicating stages of PS/2 command execution
+ * @cmdbuf: buffer holding command response
+ * @cmdcnt: outstanding number of bytes of the command response
+ * @nak: a byte transmitted by the device when it refuses command
+ * @pre_receive_handler: checks communication errors and returns disposition
+ * (&enum ps2_disposition) of the received data byte
+ * @receive_handler: main handler of particular PS/2 protocol, such as keyboard
+ * or mouse protocol
+ */
struct ps2dev {
struct serio *serio;
-
- /* Ensures that only one command is executing at a time */
struct mutex cmd_mutex;
-
- /* Used to signal completion from interrupt handler */
wait_queue_head_t wait;
-
unsigned long flags;
u8 cmdbuf[8];
u8 cmdcnt;
u8 nak;
+
+ ps2_pre_receive_handler_t pre_receive_handler;
+ ps2_receive_handler_t receive_handler;
};
-void ps2_init(struct ps2dev *ps2dev, struct serio *serio);
+void ps2_init(struct ps2dev *ps2dev, struct serio *serio,
+ ps2_pre_receive_handler_t pre_receive_handler,
+ ps2_receive_handler_t receive_handler);
int ps2_sendbyte(struct ps2dev *ps2dev, u8 byte, unsigned int timeout);
void ps2_drain(struct ps2dev *ps2dev, size_t maxbytes, unsigned int timeout);
void ps2_begin_command(struct ps2dev *ps2dev);
@@ -52,9 +68,8 @@ void ps2_end_command(struct ps2dev *ps2dev);
int __ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command);
int ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command);
int ps2_sliced_command(struct ps2dev *ps2dev, u8 command);
-bool ps2_handle_ack(struct ps2dev *ps2dev, u8 data);
-bool ps2_handle_response(struct ps2dev *ps2dev, u8 data);
-void ps2_cmd_aborted(struct ps2dev *ps2dev);
bool ps2_is_keyboard_id(u8 id);
+irqreturn_t ps2_interrupt(struct serio *serio, u8 data, unsigned int flags);
+
#endif /* _LIBPS2_H */
--
2.40.1.606.ga4b1b128d6-goog
When getting unexpected data while waiting for an acknowledgement it does
not matter what command phase is currently executed, and ps2_handle_ack()
should indicate that no further processing is needed for the received data
byte. Remove PS2_FLAG_ACK_CMD and associated handling.
Note that while it is possible to make ps2_handle_ack (and
ps2_handle_repsonse) return void, it will be done when the code will be
converted to common PS/2 interrupt handler later.
Signed-off-by: Dmitry Torokhov <[email protected]>
---
drivers/input/serio/libps2.c | 9 ++-------
include/linux/libps2.h | 1 -
2 files changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/input/serio/libps2.c b/drivers/input/serio/libps2.c
index 764990723847..399cda0d34f5 100644
--- a/drivers/input/serio/libps2.c
+++ b/drivers/input/serio/libps2.c
@@ -253,9 +253,6 @@ int __ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command)
for (i = 0; i < receive; i++)
ps2dev->cmdbuf[(receive - 1) - i] = param[i];
- /* Signal that we are sending the command byte */
- ps2dev->flags |= PS2_FLAG_ACK_CMD;
-
/*
* Some devices (Synaptics) peform the reset before
* ACKing the reset command, and so it can take a long
@@ -267,9 +264,7 @@ int __ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command)
if (rc)
goto out_reset_flags;
- /* Now we are sending command parameters, if any */
- ps2dev->flags &= ~PS2_FLAG_ACK_CMD;
-
+ /* Send command parameters, if any. */
for (i = 0; i < send; i++) {
rc = ps2_do_sendbyte(ps2dev, param[i], 200, 2);
if (rc)
@@ -436,7 +431,7 @@ bool ps2_handle_ack(struct ps2dev *ps2dev, u8 data)
*/
dev_dbg(&ps2dev->serio->dev, "unexpected %#02x\n", data);
ps2dev->flags &= ~PS2_FLAG_WAITID;
- return ps2dev->flags & PS2_FLAG_ACK_CMD;
+ return true;
}
if (!ps2dev->nak) {
diff --git a/include/linux/libps2.h b/include/linux/libps2.h
index 53f7e4d0f4b7..193dd53ad18b 100644
--- a/include/linux/libps2.h
+++ b/include/linux/libps2.h
@@ -28,7 +28,6 @@
#define PS2_FLAG_CMD1 BIT(2) /* Waiting for the first byte of command response */
#define PS2_FLAG_WAITID BIT(3) /* Command executing is GET ID */
#define PS2_FLAG_NAK BIT(4) /* Last transmission was NAKed */
-#define PS2_FLAG_ACK_CMD BIT(5) /* Waiting to ACK the command (first) byte */
struct ps2dev {
struct serio *serio;
--
2.40.1.606.ga4b1b128d6-goog
Hi Dmitry,
kernel test robot noticed the following build errors:
[auto build test ERROR on dtor-input/next]
[also build test ERROR on dtor-input/for-linus hid/for-next linus/master v6.4-rc1 next-20230511]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Dmitry-Torokhov/Input-libps2-attach-ps2dev-instances-as-serio-port-s-drvdata/20230512-025431
base: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
patch link: https://lore.kernel.org/r/20230511185252.386941-7-dmitry.torokhov%40gmail.com
patch subject: [PATCH 6/7] Input: libps2 - introduce common interrupt handler
config: mips-randconfig-r031-20230510 (https://download.01.org/0day-ci/archive/20230512/[email protected]/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project b0fb98227c90adf2536c9ad644a74d5e92961111)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install mips cross compiling tool for clang build
# apt-get install binutils-mipsel-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/1f2ba3a941e6f6a3ad745fa780825ac56570616e
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Dmitry-Torokhov/Input-libps2-attach-ps2dev-instances-as-serio-port-s-drvdata/20230512-025431
git checkout 1f2ba3a941e6f6a3ad745fa780825ac56570616e
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=mips olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash drivers/input/keyboard/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
>> drivers/input/keyboard/atkbd.c:424:3: error: must use 'struct' tag to refer to type 'atkbd'
atkbd->resend = false;
^
struct
>> drivers/input/keyboard/atkbd.c:424:3: error: expected expression
2 errors generated.
vim +424 drivers/input/keyboard/atkbd.c
^1da177e4c3f41 Linus Torvalds 2005-04-16 400
1f2ba3a941e6f6 Dmitry Torokhov 2023-05-11 401 static enum ps2_disposition atkbd_pre_receive_byte(struct ps2dev *ps2dev,
1f2ba3a941e6f6 Dmitry Torokhov 2023-05-11 402 u8 data, unsigned int flags)
^1da177e4c3f41 Linus Torvalds 2005-04-16 403 {
1f2ba3a941e6f6 Dmitry Torokhov 2023-05-11 404 struct serio *serio = ps2dev->serio;
^1da177e4c3f41 Linus Torvalds 2005-04-16 405
a9a1f9c315c27f Dmitry Torokhov 2010-01-06 406 dev_dbg(&serio->dev, "Received %02x flags %02x\n", data, flags);
^1da177e4c3f41 Linus Torvalds 2005-04-16 407
^1da177e4c3f41 Linus Torvalds 2005-04-16 408 #if !defined(__i386__) && !defined (__x86_64__)
1f2ba3a941e6f6 Dmitry Torokhov 2023-05-11 409 if ((flags & (SERIO_FRAME | SERIO_PARITY)) &&
1f2ba3a941e6f6 Dmitry Torokhov 2023-05-11 410 (~flags & SERIO_TIMEOUT)) {
1f2ba3a941e6f6 Dmitry Torokhov 2023-05-11 411 struct atkbd *atkbd = container_of(ps2dev, struct atkbd,
1f2ba3a941e6f6 Dmitry Torokhov 2023-05-11 412 ps2dev);
1f2ba3a941e6f6 Dmitry Torokhov 2023-05-11 413
1f2ba3a941e6f6 Dmitry Torokhov 2023-05-11 414 if (!atkbd->resend && atkbd->write) {
1f2ba3a941e6f6 Dmitry Torokhov 2023-05-11 415 dev_warn(&serio->dev,
1f2ba3a941e6f6 Dmitry Torokhov 2023-05-11 416 "Frame/parity error: %02x\n", flags);
^1da177e4c3f41 Linus Torvalds 2005-04-16 417 serio_write(serio, ATKBD_CMD_RESEND);
a9a1f9c315c27f Dmitry Torokhov 2010-01-06 418 atkbd->resend = true;
1f2ba3a941e6f6 Dmitry Torokhov 2023-05-11 419 return PS2_IGNORE;
1f2ba3a941e6f6 Dmitry Torokhov 2023-05-11 420 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 421 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 422
^1da177e4c3f41 Linus Torvalds 2005-04-16 423 if (!flags && data == ATKBD_RET_ACK)
a9a1f9c315c27f Dmitry Torokhov 2010-01-06 @424 atkbd->resend = false;
^1da177e4c3f41 Linus Torvalds 2005-04-16 425 #endif
^1da177e4c3f41 Linus Torvalds 2005-04-16 426
1f2ba3a941e6f6 Dmitry Torokhov 2023-05-11 427 return PS2_PROCESS;
1f2ba3a941e6f6 Dmitry Torokhov 2023-05-11 428 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 429
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
Hi Dmitry,
kernel test robot noticed the following build errors:
[auto build test ERROR on dtor-input/next]
[also build test ERROR on dtor-input/for-linus hid/for-next linus/master v6.4-rc2 next-20230512]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Dmitry-Torokhov/Input-libps2-attach-ps2dev-instances-as-serio-port-s-drvdata/20230512-025431
base: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
patch link: https://lore.kernel.org/r/20230511185252.386941-7-dmitry.torokhov%40gmail.com
patch subject: [PATCH 6/7] Input: libps2 - introduce common interrupt handler
config: parisc-generic-64bit_defconfig
compiler: hppa-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/1f2ba3a941e6f6a3ad745fa780825ac56570616e
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Dmitry-Torokhov/Input-libps2-attach-ps2dev-instances-as-serio-port-s-drvdata/20230512-025431
git checkout 1f2ba3a941e6f6a3ad745fa780825ac56570616e
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=parisc olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=parisc SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
drivers/input/keyboard/atkbd.c: In function 'atkbd_pre_receive_byte':
>> drivers/input/keyboard/atkbd.c:424:17: error: 'atkbd' undeclared (first use in this function)
424 | atkbd->resend = false;
| ^~~~~
drivers/input/keyboard/atkbd.c:424:17: note: each undeclared identifier is reported only once for each function it appears in
vim +/atkbd +424 drivers/input/keyboard/atkbd.c
^1da177e4c3f415 Linus Torvalds 2005-04-16 400
1f2ba3a941e6f6a Dmitry Torokhov 2023-05-11 401 static enum ps2_disposition atkbd_pre_receive_byte(struct ps2dev *ps2dev,
1f2ba3a941e6f6a Dmitry Torokhov 2023-05-11 402 u8 data, unsigned int flags)
^1da177e4c3f415 Linus Torvalds 2005-04-16 403 {
1f2ba3a941e6f6a Dmitry Torokhov 2023-05-11 404 struct serio *serio = ps2dev->serio;
^1da177e4c3f415 Linus Torvalds 2005-04-16 405
a9a1f9c315c27fe Dmitry Torokhov 2010-01-06 406 dev_dbg(&serio->dev, "Received %02x flags %02x\n", data, flags);
^1da177e4c3f415 Linus Torvalds 2005-04-16 407
^1da177e4c3f415 Linus Torvalds 2005-04-16 408 #if !defined(__i386__) && !defined (__x86_64__)
1f2ba3a941e6f6a Dmitry Torokhov 2023-05-11 409 if ((flags & (SERIO_FRAME | SERIO_PARITY)) &&
1f2ba3a941e6f6a Dmitry Torokhov 2023-05-11 410 (~flags & SERIO_TIMEOUT)) {
1f2ba3a941e6f6a Dmitry Torokhov 2023-05-11 411 struct atkbd *atkbd = container_of(ps2dev, struct atkbd,
1f2ba3a941e6f6a Dmitry Torokhov 2023-05-11 412 ps2dev);
1f2ba3a941e6f6a Dmitry Torokhov 2023-05-11 413
1f2ba3a941e6f6a Dmitry Torokhov 2023-05-11 414 if (!atkbd->resend && atkbd->write) {
1f2ba3a941e6f6a Dmitry Torokhov 2023-05-11 415 dev_warn(&serio->dev,
1f2ba3a941e6f6a Dmitry Torokhov 2023-05-11 416 "Frame/parity error: %02x\n", flags);
^1da177e4c3f415 Linus Torvalds 2005-04-16 417 serio_write(serio, ATKBD_CMD_RESEND);
a9a1f9c315c27fe Dmitry Torokhov 2010-01-06 418 atkbd->resend = true;
1f2ba3a941e6f6a Dmitry Torokhov 2023-05-11 419 return PS2_IGNORE;
1f2ba3a941e6f6a Dmitry Torokhov 2023-05-11 420 }
^1da177e4c3f415 Linus Torvalds 2005-04-16 421 }
^1da177e4c3f415 Linus Torvalds 2005-04-16 422
^1da177e4c3f415 Linus Torvalds 2005-04-16 423 if (!flags && data == ATKBD_RET_ACK)
a9a1f9c315c27fe Dmitry Torokhov 2010-01-06 @424 atkbd->resend = false;
^1da177e4c3f415 Linus Torvalds 2005-04-16 425 #endif
^1da177e4c3f415 Linus Torvalds 2005-04-16 426
1f2ba3a941e6f6a Dmitry Torokhov 2023-05-11 427 return PS2_PROCESS;
1f2ba3a941e6f6a Dmitry Torokhov 2023-05-11 428 }
^1da177e4c3f415 Linus Torvalds 2005-04-16 429
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
Instead of exposing inner workings of libps2 to drivers such as atkbd and
psmouse, have them define pre-receive and receive callbacks, and provide a
common handler that can be used with underlying serio port.
While at this add kerneldoc to the module.
Signed-off-by: Dmitry Torokhov <[email protected]>
---
drivers/input/keyboard/atkbd.c | 81 +++++++------
drivers/input/mouse/psmouse-base.c | 53 ++++----
drivers/input/serio/libps2.c | 226 ++++++++++++++++++++++++++----------
include/linux/libps2.h | 61 ++++++----
4 files changed, 269 insertions(+), 152 deletions(-)
diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index 2fb2ad73e796..c92e544c792d 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -399,46 +399,60 @@ static unsigned int atkbd_compat_scancode(struct atkbd *atkbd, unsigned int code
}
/*
- * atkbd_interrupt(). Here takes place processing of data received from
- * the keyboard into events.
+ * Tries to handle frame or parity error by requesting the keyboard controller
+ * to resend the last byte. This historically not done on x86 as controllers
+ * there typically do not implement this command.
*/
-
-static irqreturn_t atkbd_interrupt(struct serio *serio, unsigned char data,
- unsigned int flags)
+static bool __maybe_unused atkbd_handle_frame_error(struct ps2dev *ps2dev,
+ u8 data, unsigned int flags)
{
- struct atkbd *atkbd = atkbd_from_serio(serio);
- struct input_dev *dev = atkbd->dev;
- unsigned int code = data;
- int scroll = 0, hscroll = 0, click = -1;
- int value;
- unsigned short keycode;
+ struct atkbd *atkbd = container_of(ps2dev, struct atkbd, ps2dev);
+ struct serio *serio = ps2dev->serio;
- dev_dbg(&serio->dev, "Received %02x flags %02x\n", data, flags);
-
-#if !defined(__i386__) && !defined (__x86_64__)
- if ((flags & (SERIO_FRAME | SERIO_PARITY)) && (~flags & SERIO_TIMEOUT) && !atkbd->resend && atkbd->write) {
+ if ((flags & (SERIO_FRAME | SERIO_PARITY)) &&
+ (~flags & SERIO_TIMEOUT) &&
+ !atkbd->resend && atkbd->write) {
dev_warn(&serio->dev, "Frame/parity error: %02x\n", flags);
serio_write(serio, ATKBD_CMD_RESEND);
atkbd->resend = true;
- goto out;
+ return true;
}
if (!flags && data == ATKBD_RET_ACK)
atkbd->resend = false;
+
+ return false;
+}
+
+static enum ps2_disposition atkbd_pre_receive_byte(struct ps2dev *ps2dev,
+ u8 data, unsigned int flags)
+{
+ struct serio *serio = ps2dev->serio;
+
+ dev_dbg(&serio->dev, "Received %02x flags %02x\n", data, flags);
+
+#if !defined(__i386__) && !defined (__x86_64__)
+ if (atkbd_handle_frame_error(ps2dev, data, flags))
+ return PS2_IGNORE;
#endif
- if (unlikely(atkbd->ps2dev.flags & PS2_FLAG_ACK))
- if (ps2_handle_ack(&atkbd->ps2dev, data))
- goto out;
+ return PS2_PROCESS;
+}
- if (unlikely(atkbd->ps2dev.flags & PS2_FLAG_CMD))
- if (ps2_handle_response(&atkbd->ps2dev, data))
- goto out;
+static void atkbd_receive_byte(struct ps2dev *ps2dev, u8 data)
+{
+ struct serio *serio = ps2dev->serio;
+ struct atkbd *atkbd = container_of(ps2dev, struct atkbd, ps2dev);
+ struct input_dev *dev = atkbd->dev;
+ unsigned int code = data;
+ int scroll = 0, hscroll = 0, click = -1;
+ int value;
+ unsigned short keycode;
pm_wakeup_event(&serio->dev, 0);
if (!atkbd->enabled)
- goto out;
+ return;
input_event(dev, EV_MSC, MSC_RAW, code);
@@ -460,16 +474,16 @@ static irqreturn_t atkbd_interrupt(struct serio *serio, unsigned char data,
case ATKBD_RET_BAT:
atkbd->enabled = false;
serio_reconnect(atkbd->ps2dev.serio);
- goto out;
+ return;
case ATKBD_RET_EMUL0:
atkbd->emul = 1;
- goto out;
+ return;
case ATKBD_RET_EMUL1:
atkbd->emul = 2;
- goto out;
+ return;
case ATKBD_RET_RELEASE:
atkbd->release = true;
- goto out;
+ return;
case ATKBD_RET_ACK:
case ATKBD_RET_NAK:
if (printk_ratelimit())
@@ -477,18 +491,18 @@ static irqreturn_t atkbd_interrupt(struct serio *serio, unsigned char data,
"Spurious %s on %s. "
"Some program might be trying to access hardware directly.\n",
data == ATKBD_RET_ACK ? "ACK" : "NAK", serio->phys);
- goto out;
+ return;
case ATKBD_RET_ERR:
atkbd->err_count++;
dev_dbg(&serio->dev, "Keyboard on %s reports too many keys pressed.\n",
serio->phys);
- goto out;
+ return;
}
code = atkbd_compat_scancode(atkbd, code);
if (atkbd->emul && --atkbd->emul)
- goto out;
+ return;
keycode = atkbd->keycode[code];
@@ -564,8 +578,6 @@ static irqreturn_t atkbd_interrupt(struct serio *serio, unsigned char data,
}
atkbd->release = false;
-out:
- return IRQ_HANDLED;
}
static int atkbd_set_repeat_rate(struct atkbd *atkbd)
@@ -1229,7 +1241,8 @@ static int atkbd_connect(struct serio *serio, struct serio_driver *drv)
goto fail1;
atkbd->dev = dev;
- ps2_init(&atkbd->ps2dev, serio);
+ ps2_init(&atkbd->ps2dev, serio,
+ atkbd_pre_receive_byte, atkbd_receive_byte);
INIT_DELAYED_WORK(&atkbd->event_work, atkbd_event_work);
mutex_init(&atkbd->mutex);
@@ -1385,7 +1398,7 @@ static struct serio_driver atkbd_drv = {
},
.description = DRIVER_DESC,
.id_table = atkbd_serio_ids,
- .interrupt = atkbd_interrupt,
+ .interrupt = ps2_interrupt,
.connect = atkbd_connect,
.reconnect = atkbd_reconnect,
.disconnect = atkbd_disconnect,
diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
index ed5376099fba..a0aac76b1e41 100644
--- a/drivers/input/mouse/psmouse-base.c
+++ b/drivers/input/mouse/psmouse-base.c
@@ -336,17 +336,14 @@ static void psmouse_handle_oob_data(struct psmouse *psmouse, u8 data)
}
}
-/*
- * psmouse_interrupt() handles incoming characters, either passing them
- * for normal processing or gathering them as command response.
- */
-static irqreturn_t psmouse_interrupt(struct serio *serio,
- u8 data, unsigned int flags)
+static enum ps2_disposition psmouse_pre_receive_byte(struct ps2dev *ps2dev,
+ u8 data,
+ unsigned int flags)
{
- struct psmouse *psmouse = psmouse_from_serio(serio);
+ struct psmouse *psmouse = container_of(ps2dev, struct psmouse, ps2dev);
if (psmouse->state == PSMOUSE_IGNORE)
- goto out;
+ return PS2_IGNORE;
if (unlikely((flags & SERIO_TIMEOUT) ||
((flags & SERIO_PARITY) &&
@@ -357,27 +354,25 @@ static irqreturn_t psmouse_interrupt(struct serio *serio,
"bad data from KBC -%s%s\n",
flags & SERIO_TIMEOUT ? " timeout" : "",
flags & SERIO_PARITY ? " bad parity" : "");
- ps2_cmd_aborted(&psmouse->ps2dev);
- goto out;
+ return PS2_ERROR;
}
if (flags & SERIO_OOB_DATA) {
psmouse_handle_oob_data(psmouse, data);
- goto out;
+ return PS2_IGNORE;
}
- if (unlikely(psmouse->ps2dev.flags & PS2_FLAG_ACK))
- if (ps2_handle_ack(&psmouse->ps2dev, data))
- goto out;
+ return PS2_PROCESS;
+}
- if (unlikely(psmouse->ps2dev.flags & PS2_FLAG_CMD))
- if (ps2_handle_response(&psmouse->ps2dev, data))
- goto out;
+static void psmouse_receive_byte(struct ps2dev *ps2dev, u8 data)
+{
+ struct psmouse *psmouse = container_of(ps2dev, struct psmouse, ps2dev);
- pm_wakeup_event(&serio->dev, 0);
+ pm_wakeup_event(&ps2dev->serio->dev, 0);
if (psmouse->state <= PSMOUSE_RESYNCING)
- goto out;
+ return;
if (psmouse->state == PSMOUSE_ACTIVATED &&
psmouse->pktcnt && time_after(jiffies, psmouse->last + HZ/2)) {
@@ -386,7 +381,7 @@ static irqreturn_t psmouse_interrupt(struct serio *serio,
psmouse->badbyte = psmouse->packet[0];
__psmouse_set_state(psmouse, PSMOUSE_RESYNCING);
psmouse_queue_work(psmouse, &psmouse->resync_work, 0);
- goto out;
+ return;
}
psmouse->packet[psmouse->pktcnt++] = data;
@@ -395,21 +390,21 @@ static irqreturn_t psmouse_interrupt(struct serio *serio,
if (unlikely(psmouse->packet[0] == PSMOUSE_RET_BAT && psmouse->pktcnt <= 2)) {
if (psmouse->pktcnt == 1) {
psmouse->last = jiffies;
- goto out;
+ return;
}
if (psmouse->packet[1] == PSMOUSE_RET_ID ||
(psmouse->protocol->type == PSMOUSE_HGPK &&
psmouse->packet[1] == PSMOUSE_RET_BAT)) {
__psmouse_set_state(psmouse, PSMOUSE_IGNORE);
- serio_reconnect(serio);
- goto out;
+ serio_reconnect(ps2dev->serio);
+ return;
}
/* Not a new device, try processing first byte normally */
psmouse->pktcnt = 1;
if (psmouse_handle_byte(psmouse))
- goto out;
+ return;
psmouse->packet[psmouse->pktcnt++] = data;
}
@@ -424,14 +419,11 @@ static irqreturn_t psmouse_interrupt(struct serio *serio,
psmouse->badbyte = psmouse->packet[0];
__psmouse_set_state(psmouse, PSMOUSE_RESYNCING);
psmouse_queue_work(psmouse, &psmouse->resync_work, 0);
- goto out;
+ return;
}
psmouse->last = jiffies;
psmouse_handle_byte(psmouse);
-
- out:
- return IRQ_HANDLED;
}
/*
@@ -1604,7 +1596,8 @@ static int psmouse_connect(struct serio *serio, struct serio_driver *drv)
if (!psmouse || !input_dev)
goto err_free;
- ps2_init(&psmouse->ps2dev, serio);
+ ps2_init(&psmouse->ps2dev, serio,
+ psmouse_pre_receive_byte, psmouse_receive_byte);
INIT_DELAYED_WORK(&psmouse->resync_work, psmouse_resync);
psmouse->dev = input_dev;
snprintf(psmouse->phys, sizeof(psmouse->phys), "%s/input0", serio->phys);
@@ -1786,7 +1779,7 @@ static struct serio_driver psmouse_drv = {
},
.description = DRIVER_DESC,
.id_table = psmouse_serio_ids,
- .interrupt = psmouse_interrupt,
+ .interrupt = ps2_interrupt,
.connect = psmouse_connect,
.reconnect = psmouse_reconnect,
.fast_reconnect = psmouse_fast_reconnect,
diff --git a/drivers/input/serio/libps2.c b/drivers/input/serio/libps2.c
index 09eb605364bb..7c5fc853072a 100644
--- a/drivers/input/serio/libps2.c
+++ b/drivers/input/serio/libps2.c
@@ -19,9 +19,22 @@
#define DRIVER_DESC "PS/2 driver library"
-MODULE_AUTHOR("Dmitry Torokhov <[email protected]>");
-MODULE_DESCRIPTION("PS/2 driver library");
-MODULE_LICENSE("GPL");
+#define PS2_CMD_SETSCALE11 0x00e6
+#define PS2_CMD_SETRES 0x10e8
+#define PS2_CMD_GETID 0x02f2
+#define PS2_CMD_RESET_BAT 0x02ff
+
+#define PS2_RET_BAT 0xaa
+#define PS2_RET_ID 0x00
+#define PS2_RET_ACK 0xfa
+#define PS2_RET_NAK 0xfe
+#define PS2_RET_ERR 0xfc
+
+#define PS2_FLAG_ACK BIT(0) /* Waiting for ACK/NAK */
+#define PS2_FLAG_CMD BIT(1) /* Waiting for a command to finish */
+#define PS2_FLAG_CMD1 BIT(2) /* Waiting for the first byte of command response */
+#define PS2_FLAG_WAITID BIT(3) /* Command executing is GET ID */
+#define PS2_FLAG_NAK BIT(4) /* Last transmission was NAKed */
static int ps2_do_sendbyte(struct ps2dev *ps2dev, u8 byte,
unsigned int timeout, unsigned int max_attempts)
@@ -76,14 +89,17 @@ static int ps2_do_sendbyte(struct ps2dev *ps2dev, u8 byte,
return error;
}
-/*
- * ps2_sendbyte() sends a byte to the device and waits for acknowledge.
- * It doesn't handle retransmission, the caller is expected to handle
+/**
+ * ps2_sendbyte - sends a byte to the device and wait for acknowledgement
+ * @ps2dev: a PS/2 device to send the data to
+ * @byte: data to be sent to the device
+ * @timeout: timeout for sending the data and receiving an acknowledge
+ *
+ * The function doesn't handle retransmission, the caller is expected to handle
* it when needed.
*
* ps2_sendbyte() can only be called from a process context.
*/
-
int ps2_sendbyte(struct ps2dev *ps2dev, u8 byte, unsigned int timeout)
{
int retval;
@@ -99,6 +115,13 @@ int ps2_sendbyte(struct ps2dev *ps2dev, u8 byte, unsigned int timeout)
}
EXPORT_SYMBOL(ps2_sendbyte);
+/**
+ * ps2_begin_command - mark beginning of execution of a complex command
+ * @ps2dev: a PS/2 device executing the command
+ *
+ * Serializes a complex/compound command. Once command is finished
+ * ps2_end_command() should be called.
+ */
void ps2_begin_command(struct ps2dev *ps2dev)
{
struct mutex *m = ps2dev->serio->ps2_cmd_mutex ?: &ps2dev->cmd_mutex;
@@ -107,6 +130,10 @@ void ps2_begin_command(struct ps2dev *ps2dev)
}
EXPORT_SYMBOL(ps2_begin_command);
+/**
+ * ps2_end_command - mark end of execution of a complex command
+ * @ps2dev: a PS/2 device executing the command
+ */
void ps2_end_command(struct ps2dev *ps2dev)
{
struct mutex *m = ps2dev->serio->ps2_cmd_mutex ?: &ps2dev->cmd_mutex;
@@ -115,11 +142,13 @@ void ps2_end_command(struct ps2dev *ps2dev)
}
EXPORT_SYMBOL(ps2_end_command);
-/*
- * ps2_drain() waits for device to transmit requested number of bytes
- * and discards them.
+/**
+ * ps2_drain - waits for device to transmit requested number of bytes
+ * and discards them
+ * @ps2dev: the PS/2 device that should be drained
+ * @maxbytes: maximum number of bytes to be drained
+ * @timeout: time to drain the device
*/
-
void ps2_drain(struct ps2dev *ps2dev, size_t maxbytes, unsigned int timeout)
{
if (maxbytes > sizeof(ps2dev->cmdbuf)) {
@@ -142,11 +171,11 @@ void ps2_drain(struct ps2dev *ps2dev, size_t maxbytes, unsigned int timeout)
}
EXPORT_SYMBOL(ps2_drain);
-/*
- * ps2_is_keyboard_id() checks received ID byte against the list of
- * known keyboard IDs.
+/**
+ * ps2_is_keyboard_id - checks received ID byte against the list of
+ * known keyboard IDs
+ * @id_byte: data byte that should be checked
*/
-
bool ps2_is_keyboard_id(u8 id_byte)
{
static const u8 keyboard_ids[] = {
@@ -167,7 +196,6 @@ EXPORT_SYMBOL(ps2_is_keyboard_id);
* response and tries to reduce remaining timeout to speed up command
* completion.
*/
-
static int ps2_adjust_timeout(struct ps2dev *ps2dev,
unsigned int command, unsigned int timeout)
{
@@ -217,13 +245,19 @@ static int ps2_adjust_timeout(struct ps2dev *ps2dev,
return timeout;
}
-/*
- * ps2_command() sends a command and its parameters to the mouse,
- * then waits for the response and puts it in the param array.
+/**
+ * __ps2_command - send a command to PS/2 device
+ * @ps2dev: the PS/2 device that should execute the command
+ * @param: a buffer containing parameters to be sent along with the command,
+ * or place where the results of the command execution will be deposited,
+ * or both
+ * @command: command word that encodes the command itself, as well as number of
+ * additional parameter bytes that should be sent to the device and expected
+ * length of the command response
*
- * ps2_command() can only be called from a process context
+ * Not serialized. Callers should use ps2_begin_command() and ps2_end_command()
+ * to ensure proper serialization for complex commands.
*/
-
int __ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command)
{
unsigned int timeout;
@@ -327,6 +361,20 @@ int __ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command)
}
EXPORT_SYMBOL(__ps2_command);
+/**
+ * ps2_command - send a command to PS/2 device
+ * @ps2dev: the PS/2 device that should execute the command
+ * @param: a buffer containing parameters to be sent along with the command,
+ * or place where the results of the command execution will be deposited,
+ * or both
+ * @command: command word that encodes the command itself, as well as number of
+ * additional parameter bytes that should be sent to the device and expected
+ * length of the command response
+ *
+ * Note: ps2_command() serializes the command execution so that only one
+ * command can be executed at a time for either individual port or the entire
+ * 8042 controller.
+ */
int ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command)
{
int rc;
@@ -339,14 +387,16 @@ int ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command)
}
EXPORT_SYMBOL(ps2_command);
-/*
- * ps2_sliced_command() sends an extended PS/2 command to the mouse
- * using sliced syntax, understood by advanced devices, such as Logitech
- * or Synaptics touchpads. The command is encoded as:
+/**
+ * ps2_sliced_command - sends an extended PS/2 command to a mouse
+ * @ps2dev: the PS/2 device that should execute the command
+ * @command: command byte
+ *
+ * The command is sent using "sliced" syntax understood by advanced devices,
+ * such as Logitech or Synaptics touchpads. The command is encoded as:
* 0xE6 0xE8 rr 0xE8 ss 0xE8 tt 0xE8 uu where (rr*64)+(ss*16)+(tt*4)+uu
* is the command.
*/
-
int ps2_sliced_command(struct ps2dev *ps2dev, u8 command)
{
int i;
@@ -372,12 +422,22 @@ int ps2_sliced_command(struct ps2dev *ps2dev, u8 command)
}
EXPORT_SYMBOL(ps2_sliced_command);
-/*
- * ps2_init() initializes ps2dev structure
+/**
+ * ps2_init - initializes ps2dev structure
+ * @ps2dev: structure to be initialized
+ * @serio: serio port associated with the PS/2 device
+ * @pre_receive_handler: validation handler to check basic communication state
+ * @receive_handler: main protocol handler
+ *
+ * Prepares ps2dev structure for use in drivers for PS/2 devices.
*/
-
-void ps2_init(struct ps2dev *ps2dev, struct serio *serio)
+void ps2_init(struct ps2dev *ps2dev, struct serio *serio,
+ ps2_pre_receive_handler_t pre_receive_handler,
+ ps2_receive_handler_t receive_handler)
{
+ ps2dev->pre_receive_handler = pre_receive_handler;
+ ps2dev->receive_handler = receive_handler;
+
mutex_init(&ps2dev->cmd_mutex);
lockdep_set_subclass(&ps2dev->cmd_mutex, serio->depth);
init_waitqueue_head(&ps2dev->wait);
@@ -387,11 +447,35 @@ void ps2_init(struct ps2dev *ps2dev, struct serio *serio)
EXPORT_SYMBOL(ps2_init);
/*
- * ps2_handle_ack() is supposed to be used in interrupt handler
- * to properly process ACK/NAK of a command from a PS/2 device.
+ * ps2_handle_response() stores device's response to a command and notifies
+ * the process waiting for completion of the command. Note that there is a
+ * distinction between waiting for the first byte of the response, and
+ * waiting for subsequent bytes. It is done so that callers could shorten
+ * timeouts once first byte of response is received.
*/
+static void ps2_handle_response(struct ps2dev *ps2dev, u8 data)
+{
+ if (ps2dev->cmdcnt)
+ ps2dev->cmdbuf[--ps2dev->cmdcnt] = data;
-bool ps2_handle_ack(struct ps2dev *ps2dev, u8 data)
+ if (ps2dev->flags & PS2_FLAG_CMD1) {
+ ps2dev->flags &= ~PS2_FLAG_CMD1;
+ if (ps2dev->cmdcnt)
+ wake_up(&ps2dev->wait);
+ }
+
+ if (!ps2dev->cmdcnt) {
+ ps2dev->flags &= ~PS2_FLAG_CMD;
+ wake_up(&ps2dev->wait);
+ }
+}
+
+/*
+ * ps2_handle_ack() processes ACK/NAK of a command from a PS/2 device,
+ * possibly applying workarounds for mice not acknowledging the "get ID"
+ * command.
+ */
+static void ps2_handle_ack(struct ps2dev *ps2dev, u8 data)
{
switch (data) {
case PS2_RET_ACK:
@@ -436,53 +520,25 @@ bool ps2_handle_ack(struct ps2dev *ps2dev, u8 data)
*/
dev_dbg(&ps2dev->serio->dev, "unexpected %#02x\n", data);
ps2dev->flags &= ~PS2_FLAG_WAITID;
- return true;
+ return;
}
if (!ps2dev->nak)
ps2dev->flags &= ~PS2_FLAG_NAK;
ps2dev->flags &= ~PS2_FLAG_ACK;
- wake_up(&ps2dev->wait);
if (!ps2dev->nak && data != PS2_RET_ACK)
ps2_handle_response(ps2dev, data);
-
- return true;
-}
-EXPORT_SYMBOL(ps2_handle_ack);
-
-/*
- * ps2_handle_response() is supposed to be used in interrupt handler
- * to properly store device's response to a command and notify process
- * waiting for completion of the command.
- */
-
-bool ps2_handle_response(struct ps2dev *ps2dev, u8 data)
-{
- if (ps2dev->cmdcnt)
- ps2dev->cmdbuf[--ps2dev->cmdcnt] = data;
-
- if (ps2dev->flags & PS2_FLAG_CMD1) {
- ps2dev->flags &= ~PS2_FLAG_CMD1;
- if (ps2dev->cmdcnt)
- wake_up(&ps2dev->wait);
- }
-
- if (!ps2dev->cmdcnt) {
- ps2dev->flags &= ~PS2_FLAG_CMD;
+ else
wake_up(&ps2dev->wait);
- }
-
- return true;
}
-EXPORT_SYMBOL(ps2_handle_response);
/*
* Clears state of PS/2 device after communication error by resetting majority
* of flags and waking up waiters, if any.
*/
-void ps2_cmd_aborted(struct ps2dev *ps2dev)
+static void ps2_cleanup(struct ps2dev *ps2dev)
{
unsigned long old_flags = ps2dev->flags;
@@ -494,6 +550,46 @@ void ps2_cmd_aborted(struct ps2dev *ps2dev)
if (old_flags & (PS2_FLAG_ACK | PS2_FLAG_CMD))
wake_up(&ps2dev->wait);
+}
+/**
+ * ps2_interrupt - common interrupt handler for PS/2 devices
+ * @serio: serio port for the device
+ * @data: a data byte received from the device
+ * @flags: flags such as %SERIO_PARITY or %SERIO_TIMEOUT indicating state of
+ * the data transfer
+ *
+ * ps2_interrupt() invokes pre-receive handler, optionally handles command
+ * acknowledgement and response from the device, and finally passes the data
+ * to the main protocol handler for future processing.
+ */
+irqreturn_t ps2_interrupt(struct serio *serio, u8 data, unsigned int flags) {
+ struct ps2dev *ps2dev = serio_get_drvdata(serio);
+ enum ps2_disposition rc;
+
+ rc = ps2dev->pre_receive_handler(ps2dev, data, flags);
+ switch (rc) {
+ case PS2_ERROR:
+ ps2_cleanup(ps2dev);
+ break;
+
+ case PS2_IGNORE:
+ break;
+
+ case PS2_PROCESS:
+ if (ps2dev->flags & PS2_FLAG_ACK)
+ ps2_handle_ack(ps2dev, data);
+ else if (ps2dev->flags & PS2_FLAG_CMD)
+ ps2_handle_response(ps2dev, data);
+ else
+ ps2dev->receive_handler(ps2dev, data);
+ break;
+ }
+
+ return IRQ_HANDLED;
}
-EXPORT_SYMBOL(ps2_cmd_aborted);
+EXPORT_SYMBOL(ps2_interrupt);
+
+MODULE_AUTHOR("Dmitry Torokhov <[email protected]>");
+MODULE_DESCRIPTION("PS/2 driver library");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/libps2.h b/include/linux/libps2.h
index 193dd53ad18b..9ca9ce4e6e64 100644
--- a/include/linux/libps2.h
+++ b/include/linux/libps2.h
@@ -8,43 +8,59 @@
*/
#include <linux/bitops.h>
+#include <linux/interrupt.h>
#include <linux/mutex.h>
#include <linux/types.h>
#include <linux/wait.h>
-#define PS2_CMD_SETSCALE11 0x00e6
-#define PS2_CMD_SETRES 0x10e8
-#define PS2_CMD_GETID 0x02f2
-#define PS2_CMD_RESET_BAT 0x02ff
+struct ps2dev;
-#define PS2_RET_BAT 0xaa
-#define PS2_RET_ID 0x00
-#define PS2_RET_ACK 0xfa
-#define PS2_RET_NAK 0xfe
-#define PS2_RET_ERR 0xfc
+/**
+ * enum ps2_disposition - indicates how received byte should be handled
+ * @PS2_PROCESS: pass to the main protocol handler, process normally
+ * @PS2_IGNORE: skip the byte
+ * @PS2_ERROR: do not process the byte, abort command in progress
+ */
+enum ps2_disposition {
+ PS2_PROCESS,
+ PS2_IGNORE,
+ PS2_ERROR,
+};
-#define PS2_FLAG_ACK BIT(0) /* Waiting for ACK/NAK */
-#define PS2_FLAG_CMD BIT(1) /* Waiting for a command to finish */
-#define PS2_FLAG_CMD1 BIT(2) /* Waiting for the first byte of command response */
-#define PS2_FLAG_WAITID BIT(3) /* Command executing is GET ID */
-#define PS2_FLAG_NAK BIT(4) /* Last transmission was NAKed */
+typedef enum ps2_disposition (*ps2_pre_receive_handler_t)(struct ps2dev *, u8,
+ unsigned int);
+typedef void (*ps2_receive_handler_t)(struct ps2dev *, u8);
+/**
+ * struct ps2dev - represents a device using PS/2 protocol
+ * @serio: a serio port used by the PS/2 device
+ * @cmd_mutex: a mutex ensuring that only one command is executing at a time
+ * @wait: a waitqueue used to signal completion from the serio interrupt handler
+ * @flags: various internal flags indicating stages of PS/2 command execution
+ * @cmdbuf: buffer holding command response
+ * @cmdcnt: outstanding number of bytes of the command response
+ * @nak: a byte transmitted by the device when it refuses command
+ * @pre_receive_handler: checks communication errors and returns disposition
+ * (&enum ps2_disposition) of the received data byte
+ * @receive_handler: main handler of particular PS/2 protocol, such as keyboard
+ * or mouse protocol
+ */
struct ps2dev {
struct serio *serio;
-
- /* Ensures that only one command is executing at a time */
struct mutex cmd_mutex;
-
- /* Used to signal completion from interrupt handler */
wait_queue_head_t wait;
-
unsigned long flags;
u8 cmdbuf[8];
u8 cmdcnt;
u8 nak;
+
+ ps2_pre_receive_handler_t pre_receive_handler;
+ ps2_receive_handler_t receive_handler;
};
-void ps2_init(struct ps2dev *ps2dev, struct serio *serio);
+void ps2_init(struct ps2dev *ps2dev, struct serio *serio,
+ ps2_pre_receive_handler_t pre_receive_handler,
+ ps2_receive_handler_t receive_handler);
int ps2_sendbyte(struct ps2dev *ps2dev, u8 byte, unsigned int timeout);
void ps2_drain(struct ps2dev *ps2dev, size_t maxbytes, unsigned int timeout);
void ps2_begin_command(struct ps2dev *ps2dev);
@@ -52,9 +68,8 @@ void ps2_end_command(struct ps2dev *ps2dev);
int __ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command);
int ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command);
int ps2_sliced_command(struct ps2dev *ps2dev, u8 command);
-bool ps2_handle_ack(struct ps2dev *ps2dev, u8 data);
-bool ps2_handle_response(struct ps2dev *ps2dev, u8 data);
-void ps2_cmd_aborted(struct ps2dev *ps2dev);
bool ps2_is_keyboard_id(u8 id);
+irqreturn_t ps2_interrupt(struct serio *serio, u8 data, unsigned int flags);
+
#endif /* _LIBPS2_H */
On Thu, May 11, 2023 at 11:52:42AM -0700, Dmitry Torokhov wrote:
> When getting unexpected data while waiting for an acknowledgement it does
> not matter what command phase is currently executed, and ps2_handle_ack()
> should indicate that no further processing is needed for the received data
> byte. Remove PS2_FLAG_ACK_CMD and associated handling.
>
> Note that while it is possible to make ps2_handle_ack (and
> ps2_handle_repsonse) return void, it will be done when the code will be
> converted to common PS/2 interrupt handler later.
>
> Signed-off-by: Dmitry Torokhov <[email protected]>
> ---
> drivers/input/serio/libps2.c | 9 ++-------
> include/linux/libps2.h | 1 -
> 2 files changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/input/serio/libps2.c b/drivers/input/serio/libps2.c
> index 764990723847..399cda0d34f5 100644
> --- a/drivers/input/serio/libps2.c
> +++ b/drivers/input/serio/libps2.c
> @@ -253,9 +253,6 @@ int __ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command)
> for (i = 0; i < receive; i++)
> ps2dev->cmdbuf[(receive - 1) - i] = param[i];
>
> - /* Signal that we are sending the command byte */
> - ps2dev->flags |= PS2_FLAG_ACK_CMD;
> -
> /*
> * Some devices (Synaptics) peform the reset before
> * ACKing the reset command, and so it can take a long
> @@ -267,9 +264,7 @@ int __ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command)
> if (rc)
> goto out_reset_flags;
>
> - /* Now we are sending command parameters, if any */
> - ps2dev->flags &= ~PS2_FLAG_ACK_CMD;
> -
> + /* Send command parameters, if any. */
> for (i = 0; i < send; i++) {
> rc = ps2_do_sendbyte(ps2dev, param[i], 200, 2);
> if (rc)
> @@ -436,7 +431,7 @@ bool ps2_handle_ack(struct ps2dev *ps2dev, u8 data)
> */
> dev_dbg(&ps2dev->serio->dev, "unexpected %#02x\n", data);
> ps2dev->flags &= ~PS2_FLAG_WAITID;
> - return ps2dev->flags & PS2_FLAG_ACK_CMD;
> + return true;
> }
>
> if (!ps2dev->nak) {
> diff --git a/include/linux/libps2.h b/include/linux/libps2.h
> index 53f7e4d0f4b7..193dd53ad18b 100644
> --- a/include/linux/libps2.h
> +++ b/include/linux/libps2.h
> @@ -28,7 +28,6 @@
> #define PS2_FLAG_CMD1 BIT(2) /* Waiting for the first byte of command response */
> #define PS2_FLAG_WAITID BIT(3) /* Command executing is GET ID */
> #define PS2_FLAG_NAK BIT(4) /* Last transmission was NAKed */
> -#define PS2_FLAG_ACK_CMD BIT(5) /* Waiting to ACK the command (first) byte */
>
> struct ps2dev {
> struct serio *serio;
Reviewed-by: Raul E Rangel <[email protected]>
On Thu, May 11, 2023 at 11:52:47AM -0700, Dmitry Torokhov wrote:
> Upon receiving a PS/2 command the device and controller are supposed to
> stop sending normal data (scancodes or movement packets) and instead
> immediately start delivering ACK/NAK and command response. Unfortunately
> often EC has an output buffer which may contain latched data by the time
> the EC receives a command from the host. The kernel used to ignore such
> data, but that may cause "stuck" keys if the data dropped happens to be a
> break code or a part of a break code. This occasionally happens, for
> example, on Chromebooks when the kernel tries to toggle CapsLock LED on
> a keyboard while user releases Alt+Search keyboard shortcut.
>
> Fix this by passing the first non-ACK byte to the normal handler for a
> handful of PS/2 commands that are expected to be used during normal device
> operation (as opposed to probe/configuration time).
>
> Signed-off-by: Dmitry Torokhov <[email protected]>
> ---
> drivers/input/serio/libps2.c | 36 ++++++++++++++++++++++++++++++++----
> 1 file changed, 32 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/input/serio/libps2.c b/drivers/input/serio/libps2.c
> index 7c5fc853072a..6d78a1fe00c1 100644
> --- a/drivers/input/serio/libps2.c
> +++ b/drivers/input/serio/libps2.c
> @@ -21,7 +21,10 @@
>
> #define PS2_CMD_SETSCALE11 0x00e6
> #define PS2_CMD_SETRES 0x10e8
> +#define PS2_CMD_EX_SETLEDS 0x20eb
> +#define PS2_CMD_SETLEDS 0x10ed
> #define PS2_CMD_GETID 0x02f2
> +#define PS2_CMD_SETREP 0x10f3 /* Set repeat rate/set report rate */
> #define PS2_CMD_RESET_BAT 0x02ff
>
> #define PS2_RET_BAT 0xaa
> @@ -35,6 +38,7 @@
> #define PS2_FLAG_CMD1 BIT(2) /* Waiting for the first byte of command response */
> #define PS2_FLAG_WAITID BIT(3) /* Command executing is GET ID */
> #define PS2_FLAG_NAK BIT(4) /* Last transmission was NAKed */
> +#define PS2_FLAG_PASS_NOACK BIT(5) /* Pass non-ACK byte to receive handler */
>
> static int ps2_do_sendbyte(struct ps2dev *ps2dev, u8 byte,
> unsigned int timeout, unsigned int max_attempts)
> @@ -281,9 +285,28 @@ int __ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command)
>
> serio_pause_rx(ps2dev->serio);
>
> - /* Some mice do not ACK the "get ID" command, prepare to handle this. */
> - ps2dev->flags = command == PS2_CMD_GETID ? PS2_FLAG_WAITID : 0;
> ps2dev->cmdcnt = receive;
> +
> + switch (command) {
> + case PS2_CMD_GETID:
> + /*
> + * Some mice do not ACK the "get ID" command, prepare to
> + * handle this.
> + */
> + ps2dev->flags = PS2_FLAG_WAITID;
> + break;
> +
> + case PS2_CMD_SETLEDS:
> + case PS2_CMD_EX_SETLEDS:
> + case PS2_CMD_SETREP:
> + ps2dev->flags = PS2_FLAG_PASS_NOACK;
> + break;
> +
> + default:
> + ps2dev->flags = 0;
> + break;
> + }
> +
> if (receive) {
> /* Indicate that we expect response to the command. */
> ps2dev->flags |= PS2_FLAG_CMD | PS2_FLAG_CMD1;
> @@ -512,14 +535,19 @@ static void ps2_handle_ack(struct ps2dev *ps2dev, u8 data)
> * Do not signal errors if we get unexpected reply while
> * waiting for an ACK to the initial (first) command byte:
> * the device might not be quiesced yet and continue
> - * delivering data.
> + * delivering data. For certain commands (such as set leds and
> + * set repeat rate) that can be used during normal device
> + * operation, we even pass this data byte to the normal receive
> + * handler.
> * Note that we reset PS2_FLAG_WAITID flag, so the workaround
> * for mice not acknowledging the Get ID command only triggers
> * on the 1st byte; if device spews data we really want to see
> * a real ACK from it.
> */
> dev_dbg(&ps2dev->serio->dev, "unexpected %#02x\n", data);
> - ps2dev->flags &= ~PS2_FLAG_WAITID;
> + if (ps2dev->flags & PS2_FLAG_PASS_NOACK)
> + ps2dev->receive_handler(ps2dev, data);
> + ps2dev->flags &= ~(PS2_FLAG_WAITID | PS2_FLAG_PASS_NOACK);
> return;
> }
>
Great refactoring. It made this final patch super simple!
Reviewed-by: Raul E Rangel <[email protected]>
On Thu, May 11, 2023 at 11:52:46AM -0700, Dmitry Torokhov wrote:
> Instead of exposing inner workings of libps2 to drivers such as atkbd and
> psmouse, have them define pre-receive and receive callbacks, and provide a
> common handler that can be used with underlying serio port.
>
> While at this add kerneldoc to the module.
>
> Signed-off-by: Dmitry Torokhov <[email protected]>
> ---
> drivers/input/keyboard/atkbd.c | 73 +++++-----
> drivers/input/mouse/psmouse-base.c | 53 +++----
> drivers/input/serio/libps2.c | 226 ++++++++++++++++++++---------
> include/linux/libps2.h | 61 +++++---
> 4 files changed, 259 insertions(+), 154 deletions(-)
>
> diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
> index 2fb2ad73e796..8ef663a589b3 100644
> --- a/drivers/input/keyboard/atkbd.c
> +++ b/drivers/input/keyboard/atkbd.c
> @@ -398,47 +398,49 @@ static unsigned int atkbd_compat_scancode(struct atkbd *atkbd, unsigned int code
> return code;
> }
>
> -/*
> - * atkbd_interrupt(). Here takes place processing of data received from
> - * the keyboard into events.
> - */
> -
> -static irqreturn_t atkbd_interrupt(struct serio *serio, unsigned char data,
> - unsigned int flags)
> +static enum ps2_disposition atkbd_pre_receive_byte(struct ps2dev *ps2dev,
> + u8 data, unsigned int flags)
> {
> - struct atkbd *atkbd = atkbd_from_serio(serio);
> - struct input_dev *dev = atkbd->dev;
> - unsigned int code = data;
> - int scroll = 0, hscroll = 0, click = -1;
> - int value;
> - unsigned short keycode;
> + struct serio *serio = ps2dev->serio;
>
> dev_dbg(&serio->dev, "Received %02x flags %02x\n", data, flags);
>
> #if !defined(__i386__) && !defined (__x86_64__)
> - if ((flags & (SERIO_FRAME | SERIO_PARITY)) && (~flags & SERIO_TIMEOUT) && !atkbd->resend && atkbd->write) {
> - dev_warn(&serio->dev, "Frame/parity error: %02x\n", flags);
> - serio_write(serio, ATKBD_CMD_RESEND);
> - atkbd->resend = true;
> - goto out;
> + if ((flags & (SERIO_FRAME | SERIO_PARITY)) &&
> + (~flags & SERIO_TIMEOUT)) {
> + struct atkbd *atkbd = container_of(ps2dev, struct atkbd,
> + ps2dev);
> +
> + if (!atkbd->resend && atkbd->write) {
> + dev_warn(&serio->dev,
> + "Frame/parity error: %02x\n", flags);
> + serio_write(serio, ATKBD_CMD_RESEND);
> + atkbd->resend = true;
> + return PS2_IGNORE;
> + }
> }
>
> if (!flags && data == ATKBD_RET_ACK)
> atkbd->resend = false;
> #endif
>
> - if (unlikely(atkbd->ps2dev.flags & PS2_FLAG_ACK))
> - if (ps2_handle_ack(&atkbd->ps2dev, data))
> - goto out;
> + return PS2_PROCESS;
> +}
>
> - if (unlikely(atkbd->ps2dev.flags & PS2_FLAG_CMD))
> - if (ps2_handle_response(&atkbd->ps2dev, data))
> - goto out;
> +static void atkbd_receive_byte(struct ps2dev *ps2dev, u8 data)
> +{
> + struct serio *serio = ps2dev->serio;
> + struct atkbd *atkbd = container_of(ps2dev, struct atkbd, ps2dev);
> + struct input_dev *dev = atkbd->dev;
> + unsigned int code = data;
> + int scroll = 0, hscroll = 0, click = -1;
> + int value;
> + unsigned short keycode;
>
> pm_wakeup_event(&serio->dev, 0);
>
> if (!atkbd->enabled)
> - goto out;
> + return;
>
> input_event(dev, EV_MSC, MSC_RAW, code);
>
> @@ -460,16 +462,16 @@ static irqreturn_t atkbd_interrupt(struct serio *serio, unsigned char data,
> case ATKBD_RET_BAT:
> atkbd->enabled = false;
> serio_reconnect(atkbd->ps2dev.serio);
> - goto out;
> + return;
> case ATKBD_RET_EMUL0:
> atkbd->emul = 1;
> - goto out;
> + return;
> case ATKBD_RET_EMUL1:
> atkbd->emul = 2;
> - goto out;
> + return;
> case ATKBD_RET_RELEASE:
> atkbd->release = true;
> - goto out;
> + return;
> case ATKBD_RET_ACK:
> case ATKBD_RET_NAK:
> if (printk_ratelimit())
> @@ -477,18 +479,18 @@ static irqreturn_t atkbd_interrupt(struct serio *serio, unsigned char data,
> "Spurious %s on %s. "
> "Some program might be trying to access hardware directly.\n",
> data == ATKBD_RET_ACK ? "ACK" : "NAK", serio->phys);
> - goto out;
> + return;
> case ATKBD_RET_ERR:
> atkbd->err_count++;
> dev_dbg(&serio->dev, "Keyboard on %s reports too many keys pressed.\n",
> serio->phys);
> - goto out;
> + return;
> }
>
> code = atkbd_compat_scancode(atkbd, code);
>
> if (atkbd->emul && --atkbd->emul)
> - goto out;
> + return;
>
> keycode = atkbd->keycode[code];
>
> @@ -564,8 +566,6 @@ static irqreturn_t atkbd_interrupt(struct serio *serio, unsigned char data,
> }
>
> atkbd->release = false;
> -out:
> - return IRQ_HANDLED;
> }
>
> static int atkbd_set_repeat_rate(struct atkbd *atkbd)
> @@ -1229,7 +1229,8 @@ static int atkbd_connect(struct serio *serio, struct serio_driver *drv)
> goto fail1;
>
> atkbd->dev = dev;
> - ps2_init(&atkbd->ps2dev, serio);
> + ps2_init(&atkbd->ps2dev, serio,
> + atkbd_pre_receive_byte, atkbd_receive_byte);
> INIT_DELAYED_WORK(&atkbd->event_work, atkbd_event_work);
> mutex_init(&atkbd->mutex);
>
> @@ -1385,7 +1386,7 @@ static struct serio_driver atkbd_drv = {
> },
> .description = DRIVER_DESC,
> .id_table = atkbd_serio_ids,
> - .interrupt = atkbd_interrupt,
> + .interrupt = ps2_interrupt,
> .connect = atkbd_connect,
> .reconnect = atkbd_reconnect,
> .disconnect = atkbd_disconnect,
> diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
> index ed5376099fba..a0aac76b1e41 100644
> --- a/drivers/input/mouse/psmouse-base.c
> +++ b/drivers/input/mouse/psmouse-base.c
> @@ -336,17 +336,14 @@ static void psmouse_handle_oob_data(struct psmouse *psmouse, u8 data)
> }
> }
>
> -/*
> - * psmouse_interrupt() handles incoming characters, either passing them
> - * for normal processing or gathering them as command response.
> - */
> -static irqreturn_t psmouse_interrupt(struct serio *serio,
> - u8 data, unsigned int flags)
> +static enum ps2_disposition psmouse_pre_receive_byte(struct ps2dev *ps2dev,
> + u8 data,
> + unsigned int flags)
> {
> - struct psmouse *psmouse = psmouse_from_serio(serio);
> + struct psmouse *psmouse = container_of(ps2dev, struct psmouse, ps2dev);
>
> if (psmouse->state == PSMOUSE_IGNORE)
> - goto out;
> + return PS2_IGNORE;
>
> if (unlikely((flags & SERIO_TIMEOUT) ||
> ((flags & SERIO_PARITY) &&
> @@ -357,27 +354,25 @@ static irqreturn_t psmouse_interrupt(struct serio *serio,
> "bad data from KBC -%s%s\n",
> flags & SERIO_TIMEOUT ? " timeout" : "",
> flags & SERIO_PARITY ? " bad parity" : "");
> - ps2_cmd_aborted(&psmouse->ps2dev);
> - goto out;
> + return PS2_ERROR;
> }
>
> if (flags & SERIO_OOB_DATA) {
> psmouse_handle_oob_data(psmouse, data);
> - goto out;
> + return PS2_IGNORE;
> }
>
> - if (unlikely(psmouse->ps2dev.flags & PS2_FLAG_ACK))
> - if (ps2_handle_ack(&psmouse->ps2dev, data))
> - goto out;
> + return PS2_PROCESS;
> +}
>
> - if (unlikely(psmouse->ps2dev.flags & PS2_FLAG_CMD))
> - if (ps2_handle_response(&psmouse->ps2dev, data))
> - goto out;
> +static void psmouse_receive_byte(struct ps2dev *ps2dev, u8 data)
> +{
> + struct psmouse *psmouse = container_of(ps2dev, struct psmouse, ps2dev);
>
> - pm_wakeup_event(&serio->dev, 0);
> + pm_wakeup_event(&ps2dev->serio->dev, 0);
>
> if (psmouse->state <= PSMOUSE_RESYNCING)
> - goto out;
> + return;
>
> if (psmouse->state == PSMOUSE_ACTIVATED &&
> psmouse->pktcnt && time_after(jiffies, psmouse->last + HZ/2)) {
> @@ -386,7 +381,7 @@ static irqreturn_t psmouse_interrupt(struct serio *serio,
> psmouse->badbyte = psmouse->packet[0];
> __psmouse_set_state(psmouse, PSMOUSE_RESYNCING);
> psmouse_queue_work(psmouse, &psmouse->resync_work, 0);
> - goto out;
> + return;
> }
>
> psmouse->packet[psmouse->pktcnt++] = data;
> @@ -395,21 +390,21 @@ static irqreturn_t psmouse_interrupt(struct serio *serio,
> if (unlikely(psmouse->packet[0] == PSMOUSE_RET_BAT && psmouse->pktcnt <= 2)) {
> if (psmouse->pktcnt == 1) {
> psmouse->last = jiffies;
> - goto out;
> + return;
> }
>
> if (psmouse->packet[1] == PSMOUSE_RET_ID ||
> (psmouse->protocol->type == PSMOUSE_HGPK &&
> psmouse->packet[1] == PSMOUSE_RET_BAT)) {
> __psmouse_set_state(psmouse, PSMOUSE_IGNORE);
> - serio_reconnect(serio);
> - goto out;
> + serio_reconnect(ps2dev->serio);
> + return;
> }
>
> /* Not a new device, try processing first byte normally */
> psmouse->pktcnt = 1;
> if (psmouse_handle_byte(psmouse))
> - goto out;
> + return;
>
> psmouse->packet[psmouse->pktcnt++] = data;
> }
> @@ -424,14 +419,11 @@ static irqreturn_t psmouse_interrupt(struct serio *serio,
> psmouse->badbyte = psmouse->packet[0];
> __psmouse_set_state(psmouse, PSMOUSE_RESYNCING);
> psmouse_queue_work(psmouse, &psmouse->resync_work, 0);
> - goto out;
> + return;
> }
>
> psmouse->last = jiffies;
> psmouse_handle_byte(psmouse);
> -
> - out:
> - return IRQ_HANDLED;
> }
>
> /*
> @@ -1604,7 +1596,8 @@ static int psmouse_connect(struct serio *serio, struct serio_driver *drv)
> if (!psmouse || !input_dev)
> goto err_free;
>
> - ps2_init(&psmouse->ps2dev, serio);
> + ps2_init(&psmouse->ps2dev, serio,
> + psmouse_pre_receive_byte, psmouse_receive_byte);
> INIT_DELAYED_WORK(&psmouse->resync_work, psmouse_resync);
> psmouse->dev = input_dev;
> snprintf(psmouse->phys, sizeof(psmouse->phys), "%s/input0", serio->phys);
> @@ -1786,7 +1779,7 @@ static struct serio_driver psmouse_drv = {
> },
> .description = DRIVER_DESC,
> .id_table = psmouse_serio_ids,
> - .interrupt = psmouse_interrupt,
> + .interrupt = ps2_interrupt,
> .connect = psmouse_connect,
> .reconnect = psmouse_reconnect,
> .fast_reconnect = psmouse_fast_reconnect,
> diff --git a/drivers/input/serio/libps2.c b/drivers/input/serio/libps2.c
> index 09eb605364bb..7c5fc853072a 100644
> --- a/drivers/input/serio/libps2.c
> +++ b/drivers/input/serio/libps2.c
> @@ -19,9 +19,22 @@
>
> #define DRIVER_DESC "PS/2 driver library"
>
> -MODULE_AUTHOR("Dmitry Torokhov <[email protected]>");
> -MODULE_DESCRIPTION("PS/2 driver library");
> -MODULE_LICENSE("GPL");
> +#define PS2_CMD_SETSCALE11 0x00e6
> +#define PS2_CMD_SETRES 0x10e8
> +#define PS2_CMD_GETID 0x02f2
> +#define PS2_CMD_RESET_BAT 0x02ff
> +
> +#define PS2_RET_BAT 0xaa
> +#define PS2_RET_ID 0x00
> +#define PS2_RET_ACK 0xfa
> +#define PS2_RET_NAK 0xfe
> +#define PS2_RET_ERR 0xfc
> +
> +#define PS2_FLAG_ACK BIT(0) /* Waiting for ACK/NAK */
> +#define PS2_FLAG_CMD BIT(1) /* Waiting for a command to finish */
> +#define PS2_FLAG_CMD1 BIT(2) /* Waiting for the first byte of command response */
> +#define PS2_FLAG_WAITID BIT(3) /* Command executing is GET ID */
> +#define PS2_FLAG_NAK BIT(4) /* Last transmission was NAKed */
>
> static int ps2_do_sendbyte(struct ps2dev *ps2dev, u8 byte,
> unsigned int timeout, unsigned int max_attempts)
> @@ -76,14 +89,17 @@ static int ps2_do_sendbyte(struct ps2dev *ps2dev, u8 byte,
> return error;
> }
>
> -/*
> - * ps2_sendbyte() sends a byte to the device and waits for acknowledge.
> - * It doesn't handle retransmission, the caller is expected to handle
> +/**
> + * ps2_sendbyte - sends a byte to the device and wait for acknowledgement
> + * @ps2dev: a PS/2 device to send the data to
> + * @byte: data to be sent to the device
> + * @timeout: timeout for sending the data and receiving an acknowledge
> + *
> + * The function doesn't handle retransmission, the caller is expected to handle
> * it when needed.
> *
> * ps2_sendbyte() can only be called from a process context.
> */
> -
> int ps2_sendbyte(struct ps2dev *ps2dev, u8 byte, unsigned int timeout)
> {
> int retval;
> @@ -99,6 +115,13 @@ int ps2_sendbyte(struct ps2dev *ps2dev, u8 byte, unsigned int timeout)
> }
> EXPORT_SYMBOL(ps2_sendbyte);
>
> +/**
> + * ps2_begin_command - mark beginning of execution of a complex command
> + * @ps2dev: a PS/2 device executing the command
> + *
> + * Serializes a complex/compound command. Once command is finished
> + * ps2_end_command() should be called.
> + */
> void ps2_begin_command(struct ps2dev *ps2dev)
> {
> struct mutex *m = ps2dev->serio->ps2_cmd_mutex ?: &ps2dev->cmd_mutex;
> @@ -107,6 +130,10 @@ void ps2_begin_command(struct ps2dev *ps2dev)
> }
> EXPORT_SYMBOL(ps2_begin_command);
>
> +/**
> + * ps2_end_command - mark end of execution of a complex command
> + * @ps2dev: a PS/2 device executing the command
> + */
> void ps2_end_command(struct ps2dev *ps2dev)
> {
> struct mutex *m = ps2dev->serio->ps2_cmd_mutex ?: &ps2dev->cmd_mutex;
> @@ -115,11 +142,13 @@ void ps2_end_command(struct ps2dev *ps2dev)
> }
> EXPORT_SYMBOL(ps2_end_command);
>
> -/*
> - * ps2_drain() waits for device to transmit requested number of bytes
> - * and discards them.
> +/**
> + * ps2_drain - waits for device to transmit requested number of bytes
> + * and discards them
> + * @ps2dev: the PS/2 device that should be drained
> + * @maxbytes: maximum number of bytes to be drained
> + * @timeout: time to drain the device
> */
> -
> void ps2_drain(struct ps2dev *ps2dev, size_t maxbytes, unsigned int timeout)
> {
> if (maxbytes > sizeof(ps2dev->cmdbuf)) {
> @@ -142,11 +171,11 @@ void ps2_drain(struct ps2dev *ps2dev, size_t maxbytes, unsigned int timeout)
> }
> EXPORT_SYMBOL(ps2_drain);
>
> -/*
> - * ps2_is_keyboard_id() checks received ID byte against the list of
> - * known keyboard IDs.
> +/**
> + * ps2_is_keyboard_id - checks received ID byte against the list of
> + * known keyboard IDs
> + * @id_byte: data byte that should be checked
> */
> -
> bool ps2_is_keyboard_id(u8 id_byte)
> {
> static const u8 keyboard_ids[] = {
> @@ -167,7 +196,6 @@ EXPORT_SYMBOL(ps2_is_keyboard_id);
> * response and tries to reduce remaining timeout to speed up command
> * completion.
> */
> -
> static int ps2_adjust_timeout(struct ps2dev *ps2dev,
> unsigned int command, unsigned int timeout)
> {
> @@ -217,13 +245,19 @@ static int ps2_adjust_timeout(struct ps2dev *ps2dev,
> return timeout;
> }
>
> -/*
> - * ps2_command() sends a command and its parameters to the mouse,
> - * then waits for the response and puts it in the param array.
> +/**
> + * __ps2_command - send a command to PS/2 device
> + * @ps2dev: the PS/2 device that should execute the command
> + * @param: a buffer containing parameters to be sent along with the command,
> + * or place where the results of the command execution will be deposited,
> + * or both
> + * @command: command word that encodes the command itself, as well as number of
> + * additional parameter bytes that should be sent to the device and expected
> + * length of the command response
> *
> - * ps2_command() can only be called from a process context
> + * Not serialized. Callers should use ps2_begin_command() and ps2_end_command()
> + * to ensure proper serialization for complex commands.
> */
> -
> int __ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command)
> {
> unsigned int timeout;
> @@ -327,6 +361,20 @@ int __ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command)
> }
> EXPORT_SYMBOL(__ps2_command);
>
> +/**
> + * ps2_command - send a command to PS/2 device
> + * @ps2dev: the PS/2 device that should execute the command
> + * @param: a buffer containing parameters to be sent along with the command,
> + * or place where the results of the command execution will be deposited,
> + * or both
> + * @command: command word that encodes the command itself, as well as number of
> + * additional parameter bytes that should be sent to the device and expected
> + * length of the command response
> + *
> + * Note: ps2_command() serializes the command execution so that only one
> + * command can be executed at a time for either individual port or the entire
> + * 8042 controller.
> + */
> int ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command)
> {
> int rc;
> @@ -339,14 +387,16 @@ int ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command)
> }
> EXPORT_SYMBOL(ps2_command);
>
> -/*
> - * ps2_sliced_command() sends an extended PS/2 command to the mouse
> - * using sliced syntax, understood by advanced devices, such as Logitech
> - * or Synaptics touchpads. The command is encoded as:
> +/**
> + * ps2_sliced_command - sends an extended PS/2 command to a mouse
> + * @ps2dev: the PS/2 device that should execute the command
> + * @command: command byte
> + *
> + * The command is sent using "sliced" syntax understood by advanced devices,
> + * such as Logitech or Synaptics touchpads. The command is encoded as:
> * 0xE6 0xE8 rr 0xE8 ss 0xE8 tt 0xE8 uu where (rr*64)+(ss*16)+(tt*4)+uu
> * is the command.
> */
> -
> int ps2_sliced_command(struct ps2dev *ps2dev, u8 command)
> {
> int i;
> @@ -372,12 +422,22 @@ int ps2_sliced_command(struct ps2dev *ps2dev, u8 command)
> }
> EXPORT_SYMBOL(ps2_sliced_command);
>
> -/*
> - * ps2_init() initializes ps2dev structure
> +/**
> + * ps2_init - initializes ps2dev structure
> + * @ps2dev: structure to be initialized
> + * @serio: serio port associated with the PS/2 device
> + * @pre_receive_handler: validation handler to check basic communication state
> + * @receive_handler: main protocol handler
> + *
> + * Prepares ps2dev structure for use in drivers for PS/2 devices.
> */
> -
> -void ps2_init(struct ps2dev *ps2dev, struct serio *serio)
> +void ps2_init(struct ps2dev *ps2dev, struct serio *serio,
> + ps2_pre_receive_handler_t pre_receive_handler,
> + ps2_receive_handler_t receive_handler)
> {
> + ps2dev->pre_receive_handler = pre_receive_handler;
> + ps2dev->receive_handler = receive_handler;
> +
> mutex_init(&ps2dev->cmd_mutex);
> lockdep_set_subclass(&ps2dev->cmd_mutex, serio->depth);
> init_waitqueue_head(&ps2dev->wait);
> @@ -387,11 +447,35 @@ void ps2_init(struct ps2dev *ps2dev, struct serio *serio)
> EXPORT_SYMBOL(ps2_init);
>
> /*
> - * ps2_handle_ack() is supposed to be used in interrupt handler
> - * to properly process ACK/NAK of a command from a PS/2 device.
> + * ps2_handle_response() stores device's response to a command and notifies
> + * the process waiting for completion of the command. Note that there is a
> + * distinction between waiting for the first byte of the response, and
> + * waiting for subsequent bytes. It is done so that callers could shorten
> + * timeouts once first byte of response is received.
> */
> +static void ps2_handle_response(struct ps2dev *ps2dev, u8 data)
> +{
> + if (ps2dev->cmdcnt)
> + ps2dev->cmdbuf[--ps2dev->cmdcnt] = data;
>
> -bool ps2_handle_ack(struct ps2dev *ps2dev, u8 data)
> + if (ps2dev->flags & PS2_FLAG_CMD1) {
> + ps2dev->flags &= ~PS2_FLAG_CMD1;
> + if (ps2dev->cmdcnt)
> + wake_up(&ps2dev->wait);
> + }
> +
> + if (!ps2dev->cmdcnt) {
> + ps2dev->flags &= ~PS2_FLAG_CMD;
> + wake_up(&ps2dev->wait);
> + }
> +}
> +
> +/*
> + * ps2_handle_ack() processes ACK/NAK of a command from a PS/2 device,
> + * possibly applying workarounds for mice not acknowledging the "get ID"
> + * command.
> + */
> +static void ps2_handle_ack(struct ps2dev *ps2dev, u8 data)
> {
> switch (data) {
> case PS2_RET_ACK:
> @@ -436,53 +520,25 @@ bool ps2_handle_ack(struct ps2dev *ps2dev, u8 data)
> */
> dev_dbg(&ps2dev->serio->dev, "unexpected %#02x\n", data);
> ps2dev->flags &= ~PS2_FLAG_WAITID;
> - return true;
> + return;
> }
>
> if (!ps2dev->nak)
> ps2dev->flags &= ~PS2_FLAG_NAK;
>
> ps2dev->flags &= ~PS2_FLAG_ACK;
> - wake_up(&ps2dev->wait);
>
> if (!ps2dev->nak && data != PS2_RET_ACK)
> ps2_handle_response(ps2dev, data);
> -
> - return true;
> -}
> -EXPORT_SYMBOL(ps2_handle_ack);
> -
> -/*
> - * ps2_handle_response() is supposed to be used in interrupt handler
> - * to properly store device's response to a command and notify process
> - * waiting for completion of the command.
> - */
> -
> -bool ps2_handle_response(struct ps2dev *ps2dev, u8 data)
> -{
> - if (ps2dev->cmdcnt)
> - ps2dev->cmdbuf[--ps2dev->cmdcnt] = data;
> -
> - if (ps2dev->flags & PS2_FLAG_CMD1) {
> - ps2dev->flags &= ~PS2_FLAG_CMD1;
> - if (ps2dev->cmdcnt)
> - wake_up(&ps2dev->wait);
> - }
> -
> - if (!ps2dev->cmdcnt) {
> - ps2dev->flags &= ~PS2_FLAG_CMD;
> + else
> wake_up(&ps2dev->wait);
> - }
> -
> - return true;
> }
> -EXPORT_SYMBOL(ps2_handle_response);
>
> /*
> * Clears state of PS/2 device after communication error by resetting majority
> * of flags and waking up waiters, if any.
> */
> -void ps2_cmd_aborted(struct ps2dev *ps2dev)
> +static void ps2_cleanup(struct ps2dev *ps2dev)
> {
> unsigned long old_flags = ps2dev->flags;
>
> @@ -494,6 +550,46 @@ void ps2_cmd_aborted(struct ps2dev *ps2dev)
>
> if (old_flags & (PS2_FLAG_ACK | PS2_FLAG_CMD))
> wake_up(&ps2dev->wait);
> +}
>
> +/**
> + * ps2_interrupt - common interrupt handler for PS/2 devices
> + * @serio: serio port for the device
> + * @data: a data byte received from the device
> + * @flags: flags such as %SERIO_PARITY or %SERIO_TIMEOUT indicating state of
> + * the data transfer
> + *
> + * ps2_interrupt() invokes pre-receive handler, optionally handles command
> + * acknowledgement and response from the device, and finally passes the data
> + * to the main protocol handler for future processing.
> + */
> +irqreturn_t ps2_interrupt(struct serio *serio, u8 data, unsigned int flags) {
> + struct ps2dev *ps2dev = serio_get_drvdata(serio);
> + enum ps2_disposition rc;
> +
> + rc = ps2dev->pre_receive_handler(ps2dev, data, flags);
> + switch (rc) {
> + case PS2_ERROR:
> + ps2_cleanup(ps2dev);
> + break;
> +
> + case PS2_IGNORE:
> + break;
> +
> + case PS2_PROCESS:
> + if (ps2dev->flags & PS2_FLAG_ACK)
> + ps2_handle_ack(ps2dev, data);
> + else if (ps2dev->flags & PS2_FLAG_CMD)
> + ps2_handle_response(ps2dev, data);
> + else
> + ps2dev->receive_handler(ps2dev, data);
> + break;
> + }
> +
> + return IRQ_HANDLED;
> }
> -EXPORT_SYMBOL(ps2_cmd_aborted);
> +EXPORT_SYMBOL(ps2_interrupt);
> +
> +MODULE_AUTHOR("Dmitry Torokhov <[email protected]>");
> +MODULE_DESCRIPTION("PS/2 driver library");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/libps2.h b/include/linux/libps2.h
> index 193dd53ad18b..9ca9ce4e6e64 100644
> --- a/include/linux/libps2.h
> +++ b/include/linux/libps2.h
> @@ -8,43 +8,59 @@
> */
>
> #include <linux/bitops.h>
> +#include <linux/interrupt.h>
> #include <linux/mutex.h>
> #include <linux/types.h>
> #include <linux/wait.h>
>
> -#define PS2_CMD_SETSCALE11 0x00e6
> -#define PS2_CMD_SETRES 0x10e8
> -#define PS2_CMD_GETID 0x02f2
> -#define PS2_CMD_RESET_BAT 0x02ff
> +struct ps2dev;
>
> -#define PS2_RET_BAT 0xaa
> -#define PS2_RET_ID 0x00
> -#define PS2_RET_ACK 0xfa
> -#define PS2_RET_NAK 0xfe
> -#define PS2_RET_ERR 0xfc
> +/**
> + * enum ps2_disposition - indicates how received byte should be handled
> + * @PS2_PROCESS: pass to the main protocol handler, process normally
> + * @PS2_IGNORE: skip the byte
> + * @PS2_ERROR: do not process the byte, abort command in progress
> + */
> +enum ps2_disposition {
> + PS2_PROCESS,
> + PS2_IGNORE,
> + PS2_ERROR,
> +};
>
> -#define PS2_FLAG_ACK BIT(0) /* Waiting for ACK/NAK */
> -#define PS2_FLAG_CMD BIT(1) /* Waiting for a command to finish */
> -#define PS2_FLAG_CMD1 BIT(2) /* Waiting for the first byte of command response */
> -#define PS2_FLAG_WAITID BIT(3) /* Command executing is GET ID */
> -#define PS2_FLAG_NAK BIT(4) /* Last transmission was NAKed */
> +typedef enum ps2_disposition (*ps2_pre_receive_handler_t)(struct ps2dev *, u8,
> + unsigned int);
> +typedef void (*ps2_receive_handler_t)(struct ps2dev *, u8);
>
> +/**
> + * struct ps2dev - represents a device using PS/2 protocol
> + * @serio: a serio port used by the PS/2 device
> + * @cmd_mutex: a mutex ensuring that only one command is executing at a time
> + * @wait: a waitqueue used to signal completion from the serio interrupt handler
> + * @flags: various internal flags indicating stages of PS/2 command execution
> + * @cmdbuf: buffer holding command response
> + * @cmdcnt: outstanding number of bytes of the command response
> + * @nak: a byte transmitted by the device when it refuses command
> + * @pre_receive_handler: checks communication errors and returns disposition
> + * (&enum ps2_disposition) of the received data byte
> + * @receive_handler: main handler of particular PS/2 protocol, such as keyboard
> + * or mouse protocol
> + */
> struct ps2dev {
> struct serio *serio;
> -
> - /* Ensures that only one command is executing at a time */
> struct mutex cmd_mutex;
> -
> - /* Used to signal completion from interrupt handler */
> wait_queue_head_t wait;
> -
> unsigned long flags;
> u8 cmdbuf[8];
> u8 cmdcnt;
> u8 nak;
> +
> + ps2_pre_receive_handler_t pre_receive_handler;
> + ps2_receive_handler_t receive_handler;
> };
>
> -void ps2_init(struct ps2dev *ps2dev, struct serio *serio);
> +void ps2_init(struct ps2dev *ps2dev, struct serio *serio,
> + ps2_pre_receive_handler_t pre_receive_handler,
> + ps2_receive_handler_t receive_handler);
> int ps2_sendbyte(struct ps2dev *ps2dev, u8 byte, unsigned int timeout);
> void ps2_drain(struct ps2dev *ps2dev, size_t maxbytes, unsigned int timeout);
> void ps2_begin_command(struct ps2dev *ps2dev);
> @@ -52,9 +68,8 @@ void ps2_end_command(struct ps2dev *ps2dev);
> int __ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command);
> int ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command);
> int ps2_sliced_command(struct ps2dev *ps2dev, u8 command);
> -bool ps2_handle_ack(struct ps2dev *ps2dev, u8 data);
> -bool ps2_handle_response(struct ps2dev *ps2dev, u8 data);
> -void ps2_cmd_aborted(struct ps2dev *ps2dev);
> bool ps2_is_keyboard_id(u8 id);
>
> +irqreturn_t ps2_interrupt(struct serio *serio, u8 data, unsigned int flags);
> +
> #endif /* _LIBPS2_H */
Reviewed-by: Raul E Rangel <[email protected]>
On Thu, May 11, 2023 at 11:52:45AM -0700, Dmitry Torokhov wrote:
> When aborting PS/2 command the kernel should [re]set all flags before
> waking up waiters, otherwise waiting thread may read obsolete values
> of flags.
>
> Reported-by: Raul Rangel <[email protected]>
> Signed-off-by: Dmitry Torokhov <[email protected]>
> ---
> drivers/input/serio/libps2.c | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/input/serio/libps2.c b/drivers/input/serio/libps2.c
> index 14b70a78875d..09eb605364bb 100644
> --- a/drivers/input/serio/libps2.c
> +++ b/drivers/input/serio/libps2.c
> @@ -478,15 +478,22 @@ bool ps2_handle_response(struct ps2dev *ps2dev, u8 data)
> }
> EXPORT_SYMBOL(ps2_handle_response);
>
> +/*
> + * Clears state of PS/2 device after communication error by resetting majority
> + * of flags and waking up waiters, if any.
> + */
> void ps2_cmd_aborted(struct ps2dev *ps2dev)
> {
> - if (ps2dev->flags & PS2_FLAG_ACK)
> + unsigned long old_flags = ps2dev->flags;
> +
> + /* reset all flags except last nak */
> + ps2dev->flags &= PS2_FLAG_NAK;
> +
> + if (old_flags & PS2_FLAG_ACK)
> ps2dev->nak = 1;
>
> - if (ps2dev->flags & (PS2_FLAG_ACK | PS2_FLAG_CMD))
> + if (old_flags & (PS2_FLAG_ACK | PS2_FLAG_CMD))
> wake_up(&ps2dev->wait);
>
> - /* reset all flags except last nack */
> - ps2dev->flags &= PS2_FLAG_NAK;
> }
> EXPORT_SYMBOL(ps2_cmd_aborted);
Reviewed-by: Raul E Rangel <[email protected]>
On Thu, May 11, 2023 at 11:52:44AM -0700, Dmitry Torokhov wrote:
> Do not try to process "resend" or "reject" responses from the device
> as normal response data for a command.
>
> Signed-off-by: Dmitry Torokhov <[email protected]>
> ---
> drivers/input/serio/libps2.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/input/serio/libps2.c b/drivers/input/serio/libps2.c
> index d09450eca9a7..14b70a78875d 100644
> --- a/drivers/input/serio/libps2.c
> +++ b/drivers/input/serio/libps2.c
> @@ -445,7 +445,7 @@ bool ps2_handle_ack(struct ps2dev *ps2dev, u8 data)
> ps2dev->flags &= ~PS2_FLAG_ACK;
> wake_up(&ps2dev->wait);
>
> - if (data != PS2_RET_ACK)
> + if (!ps2dev->nak && data != PS2_RET_ACK)
> ps2_handle_response(ps2dev, data);
>
> return true;
Reviewed-by: Raul E Rangel <[email protected]>