* [PATCH 1/6] Input: psmouse - use switch statement in psmouse_process_byte()
2015-11-29 5:13 [PATCH 0/6] Only probe select protocols on pass through PS/2 prots Dmitry Torokhov
@ 2015-11-29 5:13 ` Dmitry Torokhov
2015-12-11 11:37 ` Pali Rohár
2015-11-29 5:13 ` [PATCH 2/6] Input: psmouse - fix comment style Dmitry Torokhov
` (4 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: Dmitry Torokhov @ 2015-11-29 5:13 UTC (permalink / raw)
To: linux-input
Cc: Hans de Goede, Benjamin Tissoires, Thomas Hellstrom, pali.rohar,
jckeerthan, till2.schaefer, linux-kernel
Instead of a series mostly exclusive "if" statements testing protocol type
of the mouse let's use "switch" statement.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/mouse/psmouse-base.c | 65 ++++++++++++++++++--------------------
1 file changed, 31 insertions(+), 34 deletions(-)
diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
index ad18dab..4e43e7e 100644
--- a/drivers/input/mouse/psmouse-base.c
+++ b/drivers/input/mouse/psmouse-base.c
@@ -138,22 +138,16 @@ psmouse_ret_t psmouse_process_byte(struct psmouse *psmouse)
if (psmouse->pktcnt < psmouse->pktsize)
return PSMOUSE_GOOD_DATA;
-/*
- * Full packet accumulated, process it
- */
-
-/*
- * Scroll wheel on IntelliMice, scroll buttons on NetMice
- */
+ /* Full packet accumulated, process it */
- if (psmouse->type == PSMOUSE_IMPS || psmouse->type == PSMOUSE_GENPS)
+ switch (psmouse->type) {
+ case PSMOUSE_IMPS:
+ /* IntelliMouse has scroll wheel */
input_report_rel(dev, REL_WHEEL, -(signed char) packet[3]);
+ break;
-/*
- * Scroll wheel and buttons on IntelliMouse Explorer
- */
-
- if (psmouse->type == PSMOUSE_IMEX) {
+ case PSMOUSE_IMEX:
+ /* Scroll wheel and buttons on IntelliMouse Explorer */
switch (packet[3] & 0xC0) {
case 0x80: /* vertical scroll on IntelliMouse Explorer 4.0 */
input_report_rel(dev, REL_WHEEL, (int) (packet[3] & 32) - (int) (packet[3] & 31));
@@ -168,39 +162,42 @@ psmouse_ret_t psmouse_process_byte(struct psmouse *psmouse)
input_report_key(dev, BTN_EXTRA, (packet[3] >> 5) & 1);
break;
}
- }
+ break;
-/*
- * Extra buttons on Genius NewNet 3D
- */
+ case PSMOUSE_GENPS:
+ /* Report scroll buttons on NetMice */
+ input_report_rel(dev, REL_WHEEL, -(signed char) packet[3]);
- if (psmouse->type == PSMOUSE_GENPS) {
+ /* Extra buttons on Genius NewNet 3D */
input_report_key(dev, BTN_SIDE, (packet[0] >> 6) & 1);
input_report_key(dev, BTN_EXTRA, (packet[0] >> 7) & 1);
- }
+ break;
-/*
- * Extra button on ThinkingMouse
- */
- if (psmouse->type == PSMOUSE_THINKPS) {
+ case PSMOUSE_THINKPS:
+ /* Extra button on ThinkingMouse */
input_report_key(dev, BTN_EXTRA, (packet[0] >> 3) & 1);
- /* Without this bit of weirdness moving up gives wildly high Y changes. */
+
+ /*
+ * Without this bit of weirdness moving up gives wildly
+ * high Y changes.
+ */
packet[1] |= (packet[0] & 0x40) << 1;
- }
+ break;
-/*
- * Cortron PS2 Trackball reports SIDE button on the 4th bit of the first
- * byte.
- */
- if (psmouse->type == PSMOUSE_CORTRON) {
+ case PSMOUSE_CORTRON:
+ /*
+ * Cortron PS2 Trackball reports SIDE button in the
+ * 4th bit of the first byte.
+ */
input_report_key(dev, BTN_SIDE, (packet[0] >> 3) & 1);
packet[0] |= 0x08;
- }
+ break;
-/*
- * Generic PS/2 Mouse
- */
+ default:
+ break;
+ }
+ /* Generic PS/2 Mouse */
input_report_key(dev, BTN_LEFT, packet[0] & 1);
input_report_key(dev, BTN_MIDDLE, (packet[0] >> 2) & 1);
input_report_key(dev, BTN_RIGHT, (packet[0] >> 1) & 1);
--
2.6.0.rc2.230.g3dd15c0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 2/6] Input: psmouse - fix comment style
2015-11-29 5:13 [PATCH 0/6] Only probe select protocols on pass through PS/2 prots Dmitry Torokhov
2015-11-29 5:13 ` [PATCH 1/6] Input: psmouse - use switch statement in psmouse_process_byte() Dmitry Torokhov
@ 2015-11-29 5:13 ` Dmitry Torokhov
2015-12-11 11:39 ` Pali Rohár
2015-11-29 5:13 ` [PATCH 3/6] Input: psmouse - rearrange Focaltech init code Dmitry Torokhov
` (3 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: Dmitry Torokhov @ 2015-11-29 5:13 UTC (permalink / raw)
To: linux-input
Cc: Hans de Goede, Benjamin Tissoires, Thomas Hellstrom, pali.rohar,
jckeerthan, till2.schaefer, linux-kernel
The module was using non-standard comment style with comment blocks often
starting at the very beginning of a line instead of being aligned with the
code. Let's switch to standard formatting.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/mouse/psmouse-base.c | 279 +++++++++++++++++--------------------
1 file changed, 124 insertions(+), 155 deletions(-)
diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
index 4e43e7e..525de2e 100644
--- a/drivers/input/mouse/psmouse-base.c
+++ b/drivers/input/mouse/psmouse-base.c
@@ -129,7 +129,6 @@ struct psmouse_protocol {
* psmouse_process_byte() analyzes the PS/2 data stream and reports
* relevant events to the input module once full packet has arrived.
*/
-
psmouse_ret_t psmouse_process_byte(struct psmouse *psmouse)
{
struct input_dev *dev = psmouse->dev;
@@ -219,7 +218,6 @@ void psmouse_queue_work(struct psmouse *psmouse, struct delayed_work *work,
/*
* __psmouse_set_state() sets new psmouse state and resets all flags.
*/
-
static inline void __psmouse_set_state(struct psmouse *psmouse, enum psmouse_state new_state)
{
psmouse->state = new_state;
@@ -228,13 +226,11 @@ static inline void __psmouse_set_state(struct psmouse *psmouse, enum psmouse_sta
psmouse->last = jiffies;
}
-
/*
* psmouse_set_state() sets new psmouse state and resets all flags and
* counters while holding serio lock so fighting with interrupt handler
* is not a concern.
*/
-
void psmouse_set_state(struct psmouse *psmouse, enum psmouse_state new_state)
{
serio_pause_rx(psmouse->ps2dev.serio);
@@ -246,7 +242,6 @@ void psmouse_set_state(struct psmouse *psmouse, enum psmouse_state new_state)
* psmouse_handle_byte() processes one byte of the input data stream
* by calling corresponding protocol handler.
*/
-
static int psmouse_handle_byte(struct psmouse *psmouse)
{
psmouse_ret_t rc = psmouse->protocol_handler(psmouse);
@@ -289,7 +284,6 @@ static int psmouse_handle_byte(struct psmouse *psmouse)
* 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,
unsigned char data, unsigned int flags)
{
@@ -332,9 +326,8 @@ static irqreturn_t psmouse_interrupt(struct serio *serio,
}
psmouse->packet[psmouse->pktcnt++] = data;
-/*
- * Check if this is a new device announcement (0xAA 0x00)
- */
+
+ /* Check if this is a new device announcement (0xAA 0x00) */
if (unlikely(psmouse->packet[0] == PSMOUSE_RET_BAT && psmouse->pktcnt <= 2)) {
if (psmouse->pktcnt == 1) {
psmouse->last = jiffies;
@@ -348,9 +341,8 @@ static irqreturn_t psmouse_interrupt(struct serio *serio,
serio_reconnect(serio);
goto out;
}
-/*
- * Not a new device, try processing first byte normally
- */
+
+ /* Not a new device, try processing first byte normally */
psmouse->pktcnt = 1;
if (psmouse_handle_byte(psmouse))
goto out;
@@ -358,9 +350,10 @@ static irqreturn_t psmouse_interrupt(struct serio *serio,
psmouse->packet[psmouse->pktcnt++] = data;
}
-/*
- * See if we need to force resync because mouse was idle for too long
- */
+ /*
+ * See if we need to force resync because mouse was idle for
+ * too long.
+ */
if (psmouse->state == PSMOUSE_ACTIVATED &&
psmouse->pktcnt == 1 && psmouse->resync_time &&
time_after(jiffies, psmouse->last + psmouse->resync_time * HZ)) {
@@ -377,7 +370,6 @@ static irqreturn_t psmouse_interrupt(struct serio *serio,
return IRQ_HANDLED;
}
-
/*
* psmouse_sliced_command() sends an extended PS/2 command to the mouse
* using sliced syntax, understood by advanced devices, such as Logitech
@@ -401,7 +393,6 @@ int psmouse_sliced_command(struct psmouse *psmouse, unsigned char command)
return 0;
}
-
/*
* psmouse_reset() resets the mouse into power-on state.
*/
@@ -421,7 +412,6 @@ int psmouse_reset(struct psmouse *psmouse)
/*
* Here we set the mouse resolution.
*/
-
void psmouse_set_resolution(struct psmouse *psmouse, unsigned int resolution)
{
static const unsigned char params[] = { 0, 1, 2, 2, 3 };
@@ -438,7 +428,6 @@ void psmouse_set_resolution(struct psmouse *psmouse, unsigned int resolution)
/*
* Here we set the mouse report rate.
*/
-
static void psmouse_set_rate(struct psmouse *psmouse, unsigned int rate)
{
static const unsigned char rates[] = { 200, 100, 80, 60, 40, 20, 10, 0 };
@@ -454,7 +443,6 @@ static void psmouse_set_rate(struct psmouse *psmouse, unsigned int rate)
/*
* Here we set the mouse scaling.
*/
-
static void psmouse_set_scale(struct psmouse *psmouse, enum psmouse_scale scale)
{
ps2_command(&psmouse->ps2dev, NULL,
@@ -465,7 +453,6 @@ static void psmouse_set_scale(struct psmouse *psmouse, enum psmouse_scale scale)
/*
* psmouse_poll() - default poll handler. Everyone except for ALPS uses it.
*/
-
static int psmouse_poll(struct psmouse *psmouse)
{
return ps2_command(&psmouse->ps2dev, psmouse->packet,
@@ -599,7 +586,7 @@ static int im_explorer_detect(struct psmouse *psmouse, bool set_properties)
if (param[0] != 4)
return -1;
-/* Magic to enable horizontal scrolling on IntelliMouse 4.0 */
+ /* Magic to enable horizontal scrolling on IntelliMouse 4.0 */
param[0] = 200;
ps2_command(ps2dev, param, PSMOUSE_CMD_SETRATE);
param[0] = 80;
@@ -669,10 +656,10 @@ static int ps2bare_detect(struct psmouse *psmouse, bool set_properties)
if (!psmouse->name)
psmouse->name = "Mouse";
-/*
- * We have no way of figuring true number of buttons so let's
- * assume that the device has 3.
- */
+ /*
+ * We have no way of figuring true number of buttons so let's
+ * assume that the device has 3.
+ */
__set_bit(BTN_MIDDLE, psmouse->dev->keybit);
}
@@ -700,7 +687,6 @@ static int cortron_detect(struct psmouse *psmouse, bool set_properties)
* Apply default settings to the psmouse structure. Most of them will
* be overridden by individual protocol initialization routines.
*/
-
static void psmouse_apply_defaults(struct psmouse *psmouse)
{
struct input_dev *input_dev = psmouse->dev;
@@ -753,13 +739,15 @@ static int psmouse_do_detect(int (*detect)(struct psmouse *psmouse,
* psmouse_extensions() probes for any extensions to the basic PS/2 protocol
* the mouse may have.
*/
-
static int psmouse_extensions(struct psmouse *psmouse,
unsigned int max_proto, bool set_properties)
{
bool synaptics_hardware = false;
-/* Always check for focaltech, this is safe as it uses pnp-id matching */
+ /*
+ * Always check for focaltech, this is safe as it uses pnp-id
+ * matching.
+ */
if (psmouse_do_detect(focaltech_detect, psmouse, set_properties) == 0) {
if (max_proto > PSMOUSE_IMEX) {
if (!set_properties || focaltech_init(psmouse) == 0) {
@@ -777,10 +765,10 @@ static int psmouse_extensions(struct psmouse *psmouse,
}
}
-/*
- * We always check for lifebook because it does not disturb mouse
- * (it only checks DMI information).
- */
+ /*
+ * We always check for LifeBook because it does not disturb mouse
+ * (it only checks DMI information).
+ */
if (psmouse_do_detect(lifebook_detect, psmouse, set_properties) == 0) {
if (max_proto > PSMOUSE_IMEX) {
if (!set_properties || lifebook_init(psmouse) == 0)
@@ -795,53 +783,57 @@ static int psmouse_extensions(struct psmouse *psmouse,
}
}
-/*
- * Try Kensington ThinkingMouse (we try first, because synaptics probe
- * upsets the thinkingmouse).
- */
-
+ /*
+ * Try Kensington ThinkingMouse (we try first, because Synaptics
+ * probe upsets the ThinkingMouse).
+ */
if (max_proto > PSMOUSE_IMEX &&
psmouse_do_detect(thinking_detect, psmouse, set_properties) == 0) {
return PSMOUSE_THINKPS;
}
-/*
- * Try Synaptics TouchPad. Note that probing is done even if Synaptics protocol
- * support is disabled in config - we need to know if it is synaptics so we
- * can reset it properly after probing for intellimouse.
- */
+ /*
+ * Try Synaptics TouchPad. Note that probing is done even if
+ * Synaptics protocol support is disabled in config - we need to
+ * know if it is Synaptics so we can reset it properly after
+ * probing for IntelliMouse.
+ */
if (max_proto > PSMOUSE_PS2 &&
psmouse_do_detect(synaptics_detect, psmouse, set_properties) == 0) {
synaptics_hardware = true;
if (max_proto > PSMOUSE_IMEX) {
-/*
- * Try activating protocol, but check if support is enabled first, since
- * we try detecting Synaptics even when protocol is disabled.
- */
+ /*
+ * Try activating protocol, but check if support is
+ * enabled first, since we try detecting Synaptics
+ * even when protocol is disabled.
+ */
if (IS_ENABLED(CONFIG_MOUSE_PS2_SYNAPTICS) &&
(!set_properties || synaptics_init(psmouse) == 0)) {
return PSMOUSE_SYNAPTICS;
}
-/*
- * Some Synaptics touchpads can emulate extended protocols (like IMPS/2).
- * Unfortunately Logitech/Genius probes confuse some firmware versions so
- * we'll have to skip them.
- */
+ /*
+ * Some Synaptics touchpads can emulate extended
+ * protocols (like IMPS/2). Unfortunately
+ * Logitech/Genius probes confuse some firmware
+ * versions so we'll have to skip them.
+ */
max_proto = PSMOUSE_IMEX;
}
-/*
- * Make sure that touchpad is in relative mode, gestures (taps) are enabled
- */
+
+ /*
+ * Make sure that touchpad is in relative mode, gestures
+ * (taps) are enabled.
+ */
synaptics_reset(psmouse);
}
-/*
- * Try Cypress Trackpad.
- * Must try it before Finger Sensing Pad because Finger Sensing Pad probe
- * upsets some modules of Cypress Trackpads.
- */
+ /*
+ * Try Cypress Trackpad. We must try it before Finger Sensing Pad
+ * because Finger Sensing Pad probe upsets some modules of Cypress
+ * Trackpads.
+ */
if (max_proto > PSMOUSE_IMEX &&
cypress_detect(psmouse, set_properties) == 0) {
if (IS_ENABLED(CONFIG_MOUSE_PS2_CYPRESS)) {
@@ -859,45 +851,34 @@ static int psmouse_extensions(struct psmouse *psmouse,
max_proto = PSMOUSE_IMEX;
}
-/*
- * Try ALPS TouchPad
- */
+ /* Try ALPS TouchPad */
if (max_proto > PSMOUSE_IMEX) {
ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_RESET_DIS);
if (psmouse_do_detect(alps_detect,
psmouse, set_properties) == 0) {
if (!set_properties || alps_init(psmouse) == 0)
return PSMOUSE_ALPS;
-/*
- * Init failed, try basic relative protocols
- */
+
+ /* Init failed, try basic relative protocols */
max_proto = PSMOUSE_IMEX;
}
}
-/*
- * Try OLPC HGPK touchpad.
- */
+ /* Try OLPC HGPK touchpad */
if (max_proto > PSMOUSE_IMEX &&
psmouse_do_detect(hgpk_detect, psmouse, set_properties) == 0) {
if (!set_properties || hgpk_init(psmouse) == 0)
return PSMOUSE_HGPK;
-/*
- * Init failed, try basic relative protocols
- */
+ /* Init failed, try basic relative protocols */
max_proto = PSMOUSE_IMEX;
}
-/*
- * Try Elantech touchpad.
- */
+ /* Try Elantech touchpad */
if (max_proto > PSMOUSE_IMEX &&
psmouse_do_detect(elantech_detect, psmouse, set_properties) == 0) {
if (!set_properties || elantech_init(psmouse) == 0)
return PSMOUSE_ELANTECH;
-/*
- * Init failed, try basic relative protocols
- */
+ /* Init failed, try basic relative protocols */
max_proto = PSMOUSE_IMEX;
}
@@ -919,27 +900,25 @@ static int psmouse_extensions(struct psmouse *psmouse,
return PSMOUSE_TOUCHKIT_PS2;
}
-/*
- * Try Finger Sensing Pad. We do it here because its probe upsets
- * Trackpoint devices (causing TP_READ_ID command to time out).
- */
+ /*
+ * Try Finger Sensing Pad. We do it here because its probe upsets
+ * Trackpoint devices (causing TP_READ_ID command to time out).
+ */
if (max_proto > PSMOUSE_IMEX) {
if (psmouse_do_detect(fsp_detect,
psmouse, set_properties) == 0) {
if (!set_properties || fsp_init(psmouse) == 0)
return PSMOUSE_FSP;
-/*
- * Init failed, try basic relative protocols
- */
+ /* Init failed, try basic relative protocols */
max_proto = PSMOUSE_IMEX;
}
}
-/*
- * Reset to defaults in case the device got confused by extended
- * protocol probes. Note that we follow up with full reset because
- * some mice put themselves to sleep when they see PSMOUSE_RESET_DIS.
- */
+ /*
+ * Reset to defaults in case the device got confused by extended
+ * protocol probes. Note that we follow up with full reset because
+ * some mice put themselves to sleep when they see PSMOUSE_RESET_DIS.
+ */
ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_RESET_DIS);
psmouse_reset(psmouse);
@@ -955,19 +934,19 @@ static int psmouse_extensions(struct psmouse *psmouse,
return PSMOUSE_IMPS;
}
-/*
- * Okay, all failed, we have a standard mouse here. The number of the buttons
- * is still a question, though. We assume 3.
- */
+ /*
+ * Okay, all failed, we have a standard mouse here. The number of
+ * the buttons is still a question, though. We assume 3.
+ */
psmouse_do_detect(ps2bare_detect, psmouse, set_properties);
if (synaptics_hardware) {
-/*
- * We detected Synaptics hardware but it did not respond to IMPS/2 probes.
- * We need to reset the touchpad because if there is a track point on the
- * pass through port it could get disabled while probing for protocol
- * extensions.
- */
+ /*
+ * We detected Synaptics hardware but it did not respond to
+ * IMPS/2 probes. We need to reset the touchpad because if
+ * there is a track point on the pass through port it could
+ * get disabled while probing for protocol extensions.
+ */
psmouse_reset(psmouse);
}
@@ -1167,19 +1146,17 @@ static const struct psmouse_protocol *psmouse_protocol_by_name(const char *name,
/*
* psmouse_probe() probes for a PS/2 mouse.
*/
-
static int psmouse_probe(struct psmouse *psmouse)
{
struct ps2dev *ps2dev = &psmouse->ps2dev;
unsigned char param[2];
-/*
- * First, we check if it's a mouse. It should send 0x00 or 0x03
- * in case of an IntelliMouse in 4-byte mode or 0x04 for IM Explorer.
- * Sunrex K8561 IR Keyboard/Mouse reports 0xff on second and subsequent
- * ID queries, probably due to a firmware bug.
- */
-
+ /*
+ * First, we check if it's a mouse. It should send 0x00 or 0x03 in
+ * case of an IntelliMouse in 4-byte mode or 0x04 for IM Explorer.
+ * Sunrex K8561 IR Keyboard/Mouse reports 0xff on second and
+ * subsequent ID queries, probably due to a firmware bug.
+ */
param[0] = 0xa5;
if (ps2_command(ps2dev, param, PSMOUSE_CMD_GETID))
return -1;
@@ -1188,10 +1165,10 @@ static int psmouse_probe(struct psmouse *psmouse)
param[0] != 0x04 && param[0] != 0xff)
return -1;
-/*
- * Then we reset and disable the mouse so that it doesn't generate events.
- */
-
+ /*
+ * Then we reset and disable the mouse so that it doesn't generate
+ * events.
+ */
if (ps2_command(ps2dev, NULL, PSMOUSE_CMD_RESET_DIS))
psmouse_warn(psmouse, "Failed to reset mouse on %s\n",
ps2dev->serio->phys);
@@ -1202,13 +1179,11 @@ static int psmouse_probe(struct psmouse *psmouse)
/*
* psmouse_initialize() initializes the mouse to a sane state.
*/
-
static void psmouse_initialize(struct psmouse *psmouse)
{
-/*
- * We set the mouse report rate, resolution and scaling.
- */
-
+ /*
+ * We set the mouse report rate, resolution and scaling.
+ */
if (psmouse_max_proto != PSMOUSE_PS2) {
psmouse->set_rate(psmouse, psmouse->rate);
psmouse->set_resolution(psmouse, psmouse->resolution);
@@ -1219,7 +1194,6 @@ static void psmouse_initialize(struct psmouse *psmouse)
/*
* psmouse_activate() enables the mouse so that we get motion reports from it.
*/
-
int psmouse_activate(struct psmouse *psmouse)
{
if (ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_ENABLE)) {
@@ -1233,10 +1207,9 @@ int psmouse_activate(struct psmouse *psmouse)
}
/*
- * psmouse_deactivate() puts the mouse into poll mode so that we don't get motion
- * reports from it unless we explicitly request it.
+ * psmouse_deactivate() puts the mouse into poll mode so that we don't get
+ * motion reports from it unless we explicitly request it.
*/
-
int psmouse_deactivate(struct psmouse *psmouse)
{
if (ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_DISABLE)) {
@@ -1249,11 +1222,9 @@ int psmouse_deactivate(struct psmouse *psmouse)
return 0;
}
-
/*
* psmouse_resync() attempts to re-validate current protocol.
*/
-
static void psmouse_resync(struct work_struct *work)
{
struct psmouse *parent = NULL, *psmouse =
@@ -1273,16 +1244,16 @@ static void psmouse_resync(struct work_struct *work)
psmouse_deactivate(parent);
}
-/*
- * Some mice don't ACK commands sent while they are in the middle of
- * transmitting motion packet. To avoid delay we use ps2_sendbyte()
- * instead of ps2_command() which would wait for 200ms for an ACK
- * that may never come.
- * As an additional quirk ALPS touchpads may not only forget to ACK
- * disable command but will stop reporting taps, so if we see that
- * mouse at least once ACKs disable we will do full reconnect if ACK
- * is missing.
- */
+ /*
+ * Some mice don't ACK commands sent while they are in the middle of
+ * transmitting motion packet. To avoid delay we use ps2_sendbyte()
+ * instead of ps2_command() which would wait for 200ms for an ACK
+ * that may never come.
+ * As an additional quirk ALPS touchpads may not only forget to ACK
+ * disable command but will stop reporting taps, so if we see that
+ * mouse at least once ACKs disable we will do full reconnect if ACK
+ * is missing.
+ */
psmouse->num_resyncs++;
if (ps2_sendbyte(&psmouse->ps2dev, PSMOUSE_CMD_DISABLE, 20)) {
@@ -1291,13 +1262,13 @@ static void psmouse_resync(struct work_struct *work)
} else
psmouse->acks_disable_command = true;
-/*
- * Poll the mouse. If it was reset the packet will be shorter than
- * psmouse->pktsize and ps2_command will fail. We do not expect and
- * do not handle scenario when mouse "upgrades" its protocol while
- * disconnected since it would require additional delay. If we ever
- * see a mouse that does it we'll adjust the code.
- */
+ /*
+ * Poll the mouse. If it was reset the packet will be shorter than
+ * psmouse->pktsize and ps2_command will fail. We do not expect and
+ * do not handle scenario when mouse "upgrades" its protocol while
+ * disconnected since it would require additional delay. If we ever
+ * see a mouse that does it we'll adjust the code.
+ */
if (!failed) {
if (psmouse->poll(psmouse))
failed = true;
@@ -1314,11 +1285,12 @@ static void psmouse_resync(struct work_struct *work)
psmouse_set_state(psmouse, PSMOUSE_RESYNCING);
}
}
-/*
- * Now try to enable mouse. We try to do that even if poll failed and also
- * repeat our attempts 5 times, otherwise we may be left out with disabled
- * mouse.
- */
+
+ /*
+ * Now try to enable mouse. We try to do that even if poll failed
+ * and also repeat our attempts 5 times, otherwise we may be left
+ * out with disabled mouse.
+ */
for (i = 0; i < 5; i++) {
if (!ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_ENABLE)) {
enabled = true;
@@ -1350,7 +1322,6 @@ static void psmouse_resync(struct work_struct *work)
/*
* psmouse_cleanup() resets the mouse into power-on state.
*/
-
static void psmouse_cleanup(struct serio *serio)
{
struct psmouse *psmouse = serio_get_drvdata(serio);
@@ -1375,15 +1346,15 @@ static void psmouse_cleanup(struct serio *serio)
if (psmouse->cleanup)
psmouse->cleanup(psmouse);
-/*
- * Reset the mouse to defaults (bare PS/2 protocol).
- */
+ /*
+ * Reset the mouse to defaults (bare PS/2 protocol).
+ */
ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_RESET_DIS);
-/*
- * Some boxes, such as HP nx7400, get terribly confused if mouse
- * is not fully enabled before suspending/shutting down.
- */
+ /*
+ * Some boxes, such as HP nx7400, get terribly confused if mouse
+ * is not fully enabled before suspending/shutting down.
+ */
ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_ENABLE);
if (parent) {
@@ -1399,7 +1370,6 @@ static void psmouse_cleanup(struct serio *serio)
/*
* psmouse_disconnect() closes and frees.
*/
-
static void psmouse_disconnect(struct serio *serio)
{
struct psmouse *psmouse, *parent = NULL;
@@ -1599,7 +1569,6 @@ static int psmouse_connect(struct serio *serio, struct serio_driver *drv)
goto out;
}
-
static int psmouse_reconnect(struct serio *serio)
{
struct psmouse *psmouse = serio_get_drvdata(serio);
--
2.6.0.rc2.230.g3dd15c0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 3/6] Input: psmouse - rearrange Focaltech init code
2015-11-29 5:13 [PATCH 0/6] Only probe select protocols on pass through PS/2 prots Dmitry Torokhov
2015-11-29 5:13 ` [PATCH 1/6] Input: psmouse - use switch statement in psmouse_process_byte() Dmitry Torokhov
2015-11-29 5:13 ` [PATCH 2/6] Input: psmouse - fix comment style Dmitry Torokhov
@ 2015-11-29 5:13 ` Dmitry Torokhov
2015-12-11 11:44 ` Pali Rohár
2015-11-29 5:13 ` [PATCH 4/6] Input: psmouse - move protocol descriptions around Dmitry Torokhov
` (2 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: Dmitry Torokhov @ 2015-11-29 5:13 UTC (permalink / raw)
To: linux-input
Cc: Hans de Goede, Benjamin Tissoires, Thomas Hellstrom, pali.rohar,
jckeerthan, till2.schaefer, linux-kernel
The fact that we were calling focaltech_init() even when Focaltech support
is disabled was confusing. Rearrange the code so that if support is
disabled we continue to fall through the rest of protocol probing code
until we get to full reset that Focaltech devices need to work properly.
Also, replace focaltech_init() with a stub now that it is only called when
protocol is enabled.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
I know I originally requested doing reset in focaltech_init() even when
focaltech support is disabled, but as I was working with the code I found
it confusing.
drivers/input/mouse/focaltech.c | 22 ++++++----------------
drivers/input/mouse/focaltech.h | 8 ++++++++
drivers/input/mouse/psmouse-base.c | 23 ++++++++++++-----------
3 files changed, 26 insertions(+), 27 deletions(-)
diff --git a/drivers/input/mouse/focaltech.c b/drivers/input/mouse/focaltech.c
index 4d5576d..c8c6a8c 100644
--- a/drivers/input/mouse/focaltech.c
+++ b/drivers/input/mouse/focaltech.c
@@ -49,12 +49,6 @@ int focaltech_detect(struct psmouse *psmouse, bool set_properties)
return 0;
}
-static void focaltech_reset(struct psmouse *psmouse)
-{
- ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_RESET_DIS);
- psmouse_reset(psmouse);
-}
-
#ifdef CONFIG_MOUSE_PS2_FOCALTECH
/*
@@ -300,6 +294,12 @@ static int focaltech_switch_protocol(struct psmouse *psmouse)
return 0;
}
+static void focaltech_reset(struct psmouse *psmouse)
+{
+ ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_RESET_DIS);
+ psmouse_reset(psmouse);
+}
+
static void focaltech_disconnect(struct psmouse *psmouse)
{
focaltech_reset(psmouse);
@@ -456,14 +456,4 @@ fail:
kfree(priv);
return error;
}
-
-#else /* CONFIG_MOUSE_PS2_FOCALTECH */
-
-int focaltech_init(struct psmouse *psmouse)
-{
- focaltech_reset(psmouse);
-
- return 0;
-}
-
#endif /* CONFIG_MOUSE_PS2_FOCALTECH */
diff --git a/drivers/input/mouse/focaltech.h b/drivers/input/mouse/focaltech.h
index ca61ebf..783b28e 100644
--- a/drivers/input/mouse/focaltech.h
+++ b/drivers/input/mouse/focaltech.h
@@ -18,6 +18,14 @@
#define _FOCALTECH_H
int focaltech_detect(struct psmouse *psmouse, bool set_properties);
+
+#ifdef CONFIG_MOUSE_PS2_FOCALTECH
int focaltech_init(struct psmouse *psmouse);
+#else
+static inline int focaltech_init(struct psmouse *psmouse)
+{
+ return -ENOSYS;
+}
+#endif
#endif
diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
index 525de2e..c2bd866 100644
--- a/drivers/input/mouse/psmouse-base.c
+++ b/drivers/input/mouse/psmouse-base.c
@@ -750,19 +750,20 @@ static int psmouse_extensions(struct psmouse *psmouse,
*/
if (psmouse_do_detect(focaltech_detect, psmouse, set_properties) == 0) {
if (max_proto > PSMOUSE_IMEX) {
- if (!set_properties || focaltech_init(psmouse) == 0) {
- if (IS_ENABLED(CONFIG_MOUSE_PS2_FOCALTECH))
- return PSMOUSE_FOCALTECH;
- /*
- * Note that we need to also restrict
- * psmouse_max_proto so that psmouse_initialize()
- * does not try to reset rate and resolution,
- * because even that upsets the device.
- */
- psmouse_max_proto = PSMOUSE_PS2;
- return PSMOUSE_PS2;
+ if (IS_ENABLED(CONFIG_MOUSE_PS2_FOCALTECH) &&
+ (!set_properties || focaltech_init(psmouse) == 0)) {
+ return PSMOUSE_FOCALTECH;
}
}
+ /*
+ * Restrict psmouse_max_proto so that psmouse_initialize()
+ * does not try to reset rate and resolution, because even
+ * that upsets the device.
+ * This also causes us to basically fall through to basic
+ * protocol detection, where we fully reset the mouse,
+ * and set it up as bare PS/2 protocol device.
+ */
+ psmouse_max_proto = max_proto = PSMOUSE_PS2;
}
/*
--
2.6.0.rc2.230.g3dd15c0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 3/6] Input: psmouse - rearrange Focaltech init code
2015-11-29 5:13 ` [PATCH 3/6] Input: psmouse - rearrange Focaltech init code Dmitry Torokhov
@ 2015-12-11 11:44 ` Pali Rohár
0 siblings, 0 replies; 16+ messages in thread
From: Pali Rohár @ 2015-12-11 11:44 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input, Hans de Goede, Benjamin Tissoires, Thomas Hellstrom,
jckeerthan, till2.schaefer, linux-kernel
On Saturday 28 November 2015 21:13:53 Dmitry Torokhov wrote:
> The fact that we were calling focaltech_init() even when Focaltech support
> is disabled was confusing. Rearrange the code so that if support is
> disabled we continue to fall through the rest of protocol probing code
> until we get to full reset that Focaltech devices need to work properly.
>
> Also, replace focaltech_init() with a stub now that it is only called when
> protocol is enabled.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
It's also OK, so
Reviewed-by: Pali Rohár <pali.rohar@gmail.com>
--
Pali Rohár
pali.rohar@gmail.com
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 4/6] Input: psmouse - move protocol descriptions around
2015-11-29 5:13 [PATCH 0/6] Only probe select protocols on pass through PS/2 prots Dmitry Torokhov
` (2 preceding siblings ...)
2015-11-29 5:13 ` [PATCH 3/6] Input: psmouse - rearrange Focaltech init code Dmitry Torokhov
@ 2015-11-29 5:13 ` Dmitry Torokhov
2015-12-11 11:46 ` Pali Rohár
2015-11-29 5:13 ` [PATCH 5/6] Input: psmouse - factor out common protocol probing code Dmitry Torokhov
2015-11-29 5:13 ` [PATCH 6/6] Input: psmouse - limit protocols that we try on passthrough ports Dmitry Torokhov
5 siblings, 1 reply; 16+ messages in thread
From: Dmitry Torokhov @ 2015-11-29 5:13 UTC (permalink / raw)
To: linux-input
Cc: Hans de Goede, Benjamin Tissoires, Thomas Hellstrom, pali.rohar,
jckeerthan, till2.schaefer, linux-kernel
We move protocol descriptions and psmouse_find_by_type() and
pmouse_find_by_name() so that we can use them without forward declarations
in the subsequent patches.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/mouse/psmouse-base.c | 379 ++++++++++++++++++-------------------
1 file changed, 189 insertions(+), 190 deletions(-)
diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
index c2bd866..b92c1bd 100644
--- a/drivers/input/mouse/psmouse-base.c
+++ b/drivers/input/mouse/psmouse-base.c
@@ -683,6 +683,195 @@ static int cortron_detect(struct psmouse *psmouse, bool set_properties)
return 0;
}
+static const struct psmouse_protocol psmouse_protocols[] = {
+ {
+ .type = PSMOUSE_PS2,
+ .name = "PS/2",
+ .alias = "bare",
+ .maxproto = true,
+ .ignore_parity = true,
+ .detect = ps2bare_detect,
+ },
+#ifdef CONFIG_MOUSE_PS2_LOGIPS2PP
+ {
+ .type = PSMOUSE_PS2PP,
+ .name = "PS2++",
+ .alias = "logitech",
+ .detect = ps2pp_init,
+ },
+#endif
+ {
+ .type = PSMOUSE_THINKPS,
+ .name = "ThinkPS/2",
+ .alias = "thinkps",
+ .detect = thinking_detect,
+ },
+#ifdef CONFIG_MOUSE_PS2_CYPRESS
+ {
+ .type = PSMOUSE_CYPRESS,
+ .name = "CyPS/2",
+ .alias = "cypress",
+ .detect = cypress_detect,
+ .init = cypress_init,
+ },
+#endif
+ {
+ .type = PSMOUSE_GENPS,
+ .name = "GenPS/2",
+ .alias = "genius",
+ .detect = genius_detect,
+ },
+ {
+ .type = PSMOUSE_IMPS,
+ .name = "ImPS/2",
+ .alias = "imps",
+ .maxproto = true,
+ .ignore_parity = true,
+ .detect = intellimouse_detect,
+ },
+ {
+ .type = PSMOUSE_IMEX,
+ .name = "ImExPS/2",
+ .alias = "exps",
+ .maxproto = true,
+ .ignore_parity = true,
+ .detect = im_explorer_detect,
+ },
+#ifdef CONFIG_MOUSE_PS2_SYNAPTICS
+ {
+ .type = PSMOUSE_SYNAPTICS,
+ .name = "SynPS/2",
+ .alias = "synaptics",
+ .detect = synaptics_detect,
+ .init = synaptics_init,
+ },
+ {
+ .type = PSMOUSE_SYNAPTICS_RELATIVE,
+ .name = "SynRelPS/2",
+ .alias = "synaptics-relative",
+ .detect = synaptics_detect,
+ .init = synaptics_init_relative,
+ },
+#endif
+#ifdef CONFIG_MOUSE_PS2_ALPS
+ {
+ .type = PSMOUSE_ALPS,
+ .name = "AlpsPS/2",
+ .alias = "alps",
+ .detect = alps_detect,
+ .init = alps_init,
+ },
+#endif
+#ifdef CONFIG_MOUSE_PS2_LIFEBOOK
+ {
+ .type = PSMOUSE_LIFEBOOK,
+ .name = "LBPS/2",
+ .alias = "lifebook",
+ .init = lifebook_init,
+ },
+#endif
+#ifdef CONFIG_MOUSE_PS2_TRACKPOINT
+ {
+ .type = PSMOUSE_TRACKPOINT,
+ .name = "TPPS/2",
+ .alias = "trackpoint",
+ .detect = trackpoint_detect,
+ },
+#endif
+#ifdef CONFIG_MOUSE_PS2_TOUCHKIT
+ {
+ .type = PSMOUSE_TOUCHKIT_PS2,
+ .name = "touchkitPS/2",
+ .alias = "touchkit",
+ .detect = touchkit_ps2_detect,
+ },
+#endif
+#ifdef CONFIG_MOUSE_PS2_OLPC
+ {
+ .type = PSMOUSE_HGPK,
+ .name = "OLPC HGPK",
+ .alias = "hgpk",
+ .detect = hgpk_detect,
+ },
+#endif
+#ifdef CONFIG_MOUSE_PS2_ELANTECH
+ {
+ .type = PSMOUSE_ELANTECH,
+ .name = "ETPS/2",
+ .alias = "elantech",
+ .detect = elantech_detect,
+ .init = elantech_init,
+ },
+#endif
+#ifdef CONFIG_MOUSE_PS2_SENTELIC
+ {
+ .type = PSMOUSE_FSP,
+ .name = "FSPPS/2",
+ .alias = "fsp",
+ .detect = fsp_detect,
+ .init = fsp_init,
+ },
+#endif
+ {
+ .type = PSMOUSE_CORTRON,
+ .name = "CortronPS/2",
+ .alias = "cortps",
+ .detect = cortron_detect,
+ },
+#ifdef CONFIG_MOUSE_PS2_FOCALTECH
+ {
+ .type = PSMOUSE_FOCALTECH,
+ .name = "FocalTechPS/2",
+ .alias = "focaltech",
+ .detect = focaltech_detect,
+ .init = focaltech_init,
+ },
+#endif
+#ifdef CONFIG_MOUSE_PS2_VMMOUSE
+ {
+ .type = PSMOUSE_VMMOUSE,
+ .name = VMMOUSE_PSNAME,
+ .alias = "vmmouse",
+ .detect = vmmouse_detect,
+ .init = vmmouse_init,
+ },
+#endif
+ {
+ .type = PSMOUSE_AUTO,
+ .name = "auto",
+ .alias = "any",
+ .maxproto = true,
+ },
+};
+
+static const struct psmouse_protocol *psmouse_protocol_by_type(enum psmouse_type type)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(psmouse_protocols); i++)
+ if (psmouse_protocols[i].type == type)
+ return &psmouse_protocols[i];
+
+ WARN_ON(1);
+ return &psmouse_protocols[0];
+}
+
+static const struct psmouse_protocol *psmouse_protocol_by_name(const char *name, size_t len)
+{
+ const struct psmouse_protocol *p;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(psmouse_protocols); i++) {
+ p = &psmouse_protocols[i];
+
+ if ((strlen(p->name) == len && !strncmp(p->name, name, len)) ||
+ (strlen(p->alias) == len && !strncmp(p->alias, name, len)))
+ return &psmouse_protocols[i];
+ }
+
+ return NULL;
+}
+
/*
* Apply default settings to the psmouse structure. Most of them will
* be overridden by individual protocol initialization routines.
@@ -954,196 +1143,6 @@ static int psmouse_extensions(struct psmouse *psmouse,
return PSMOUSE_PS2;
}
-static const struct psmouse_protocol psmouse_protocols[] = {
- {
- .type = PSMOUSE_PS2,
- .name = "PS/2",
- .alias = "bare",
- .maxproto = true,
- .ignore_parity = true,
- .detect = ps2bare_detect,
- },
-#ifdef CONFIG_MOUSE_PS2_LOGIPS2PP
- {
- .type = PSMOUSE_PS2PP,
- .name = "PS2++",
- .alias = "logitech",
- .detect = ps2pp_init,
- },
-#endif
- {
- .type = PSMOUSE_THINKPS,
- .name = "ThinkPS/2",
- .alias = "thinkps",
- .detect = thinking_detect,
- },
-#ifdef CONFIG_MOUSE_PS2_CYPRESS
- {
- .type = PSMOUSE_CYPRESS,
- .name = "CyPS/2",
- .alias = "cypress",
- .detect = cypress_detect,
- .init = cypress_init,
- },
-#endif
- {
- .type = PSMOUSE_GENPS,
- .name = "GenPS/2",
- .alias = "genius",
- .detect = genius_detect,
- },
- {
- .type = PSMOUSE_IMPS,
- .name = "ImPS/2",
- .alias = "imps",
- .maxproto = true,
- .ignore_parity = true,
- .detect = intellimouse_detect,
- },
- {
- .type = PSMOUSE_IMEX,
- .name = "ImExPS/2",
- .alias = "exps",
- .maxproto = true,
- .ignore_parity = true,
- .detect = im_explorer_detect,
- },
-#ifdef CONFIG_MOUSE_PS2_SYNAPTICS
- {
- .type = PSMOUSE_SYNAPTICS,
- .name = "SynPS/2",
- .alias = "synaptics",
- .detect = synaptics_detect,
- .init = synaptics_init,
- },
- {
- .type = PSMOUSE_SYNAPTICS_RELATIVE,
- .name = "SynRelPS/2",
- .alias = "synaptics-relative",
- .detect = synaptics_detect,
- .init = synaptics_init_relative,
- },
-#endif
-#ifdef CONFIG_MOUSE_PS2_ALPS
- {
- .type = PSMOUSE_ALPS,
- .name = "AlpsPS/2",
- .alias = "alps",
- .detect = alps_detect,
- .init = alps_init,
- },
-#endif
-#ifdef CONFIG_MOUSE_PS2_LIFEBOOK
- {
- .type = PSMOUSE_LIFEBOOK,
- .name = "LBPS/2",
- .alias = "lifebook",
- .init = lifebook_init,
- },
-#endif
-#ifdef CONFIG_MOUSE_PS2_TRACKPOINT
- {
- .type = PSMOUSE_TRACKPOINT,
- .name = "TPPS/2",
- .alias = "trackpoint",
- .detect = trackpoint_detect,
- },
-#endif
-#ifdef CONFIG_MOUSE_PS2_TOUCHKIT
- {
- .type = PSMOUSE_TOUCHKIT_PS2,
- .name = "touchkitPS/2",
- .alias = "touchkit",
- .detect = touchkit_ps2_detect,
- },
-#endif
-#ifdef CONFIG_MOUSE_PS2_OLPC
- {
- .type = PSMOUSE_HGPK,
- .name = "OLPC HGPK",
- .alias = "hgpk",
- .detect = hgpk_detect,
- },
-#endif
-#ifdef CONFIG_MOUSE_PS2_ELANTECH
- {
- .type = PSMOUSE_ELANTECH,
- .name = "ETPS/2",
- .alias = "elantech",
- .detect = elantech_detect,
- .init = elantech_init,
- },
-#endif
-#ifdef CONFIG_MOUSE_PS2_SENTELIC
- {
- .type = PSMOUSE_FSP,
- .name = "FSPPS/2",
- .alias = "fsp",
- .detect = fsp_detect,
- .init = fsp_init,
- },
-#endif
- {
- .type = PSMOUSE_CORTRON,
- .name = "CortronPS/2",
- .alias = "cortps",
- .detect = cortron_detect,
- },
-#ifdef CONFIG_MOUSE_PS2_FOCALTECH
- {
- .type = PSMOUSE_FOCALTECH,
- .name = "FocalTechPS/2",
- .alias = "focaltech",
- .detect = focaltech_detect,
- .init = focaltech_init,
- },
-#endif
-#ifdef CONFIG_MOUSE_PS2_VMMOUSE
- {
- .type = PSMOUSE_VMMOUSE,
- .name = VMMOUSE_PSNAME,
- .alias = "vmmouse",
- .detect = vmmouse_detect,
- .init = vmmouse_init,
- },
-#endif
- {
- .type = PSMOUSE_AUTO,
- .name = "auto",
- .alias = "any",
- .maxproto = true,
- },
-};
-
-static const struct psmouse_protocol *psmouse_protocol_by_type(enum psmouse_type type)
-{
- int i;
-
- for (i = 0; i < ARRAY_SIZE(psmouse_protocols); i++)
- if (psmouse_protocols[i].type == type)
- return &psmouse_protocols[i];
-
- WARN_ON(1);
- return &psmouse_protocols[0];
-}
-
-static const struct psmouse_protocol *psmouse_protocol_by_name(const char *name, size_t len)
-{
- const struct psmouse_protocol *p;
- int i;
-
- for (i = 0; i < ARRAY_SIZE(psmouse_protocols); i++) {
- p = &psmouse_protocols[i];
-
- if ((strlen(p->name) == len && !strncmp(p->name, name, len)) ||
- (strlen(p->alias) == len && !strncmp(p->alias, name, len)))
- return &psmouse_protocols[i];
- }
-
- return NULL;
-}
-
-
/*
* psmouse_probe() probes for a PS/2 mouse.
*/
--
2.6.0.rc2.230.g3dd15c0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 5/6] Input: psmouse - factor out common protocol probing code
2015-11-29 5:13 [PATCH 0/6] Only probe select protocols on pass through PS/2 prots Dmitry Torokhov
` (3 preceding siblings ...)
2015-11-29 5:13 ` [PATCH 4/6] Input: psmouse - move protocol descriptions around Dmitry Torokhov
@ 2015-11-29 5:13 ` Dmitry Torokhov
2015-12-02 15:20 ` Hans de Goede
2015-11-29 5:13 ` [PATCH 6/6] Input: psmouse - limit protocols that we try on passthrough ports Dmitry Torokhov
5 siblings, 1 reply; 16+ messages in thread
From: Dmitry Torokhov @ 2015-11-29 5:13 UTC (permalink / raw)
To: linux-input
Cc: Hans de Goede, Benjamin Tissoires, Thomas Hellstrom, pali.rohar,
jckeerthan, till2.schaefer, linux-kernel
In preparation of limiting protocols that we try on pass-through ports,
let's rework initialization code and factor common code into
psmouse_try_protocol() that accepts protocol type (instead of detec()
function pointer) and can, for most protocols, perform both detection and
initialization.
Note that this removes option of forcing Lifebook protocol on devices that
are not recognized by lifebook_detect() as having the hardware, but I do
not recall anyone using this option.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/mouse/psmouse-base.c | 159 +++++++++++++++++++------------------
1 file changed, 82 insertions(+), 77 deletions(-)
diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
index b92c1bd..ee59b0e 100644
--- a/drivers/input/mouse/psmouse-base.c
+++ b/drivers/input/mouse/psmouse-base.c
@@ -767,6 +767,7 @@ static const struct psmouse_protocol psmouse_protocols[] = {
.type = PSMOUSE_LIFEBOOK,
.name = "LBPS/2",
.alias = "lifebook",
+ .detect = lifebook_detect,
.init = lifebook_init,
},
#endif
@@ -844,7 +845,7 @@ static const struct psmouse_protocol psmouse_protocols[] = {
},
};
-static const struct psmouse_protocol *psmouse_protocol_by_type(enum psmouse_type type)
+static const struct psmouse_protocol *__psmouse_protocol_by_type(enum psmouse_type type)
{
int i;
@@ -852,6 +853,17 @@ static const struct psmouse_protocol *psmouse_protocol_by_type(enum psmouse_type
if (psmouse_protocols[i].type == type)
return &psmouse_protocols[i];
+ return NULL;
+}
+
+static const struct psmouse_protocol *psmouse_protocol_by_type(enum psmouse_type type)
+{
+ const struct psmouse_protocol *proto;
+
+ proto = __psmouse_protocol_by_type(type);
+ if (proto)
+ return proto;
+
WARN_ON(1);
return &psmouse_protocols[0];
}
@@ -910,18 +922,37 @@ static void psmouse_apply_defaults(struct psmouse *psmouse)
psmouse->pt_deactivate = NULL;
}
-/*
- * Apply default settings to the psmouse structure and call specified
- * protocol detection or initialization routine.
- */
-static int psmouse_do_detect(int (*detect)(struct psmouse *psmouse,
- bool set_properties),
- struct psmouse *psmouse, bool set_properties)
+static bool psmouse_try_protocol(struct psmouse *psmouse,
+ enum psmouse_type type,
+ unsigned int *max_proto,
+ bool set_properties, bool do_init)
{
+ const struct psmouse_protocol *proto;
+
+ proto = __psmouse_protocol_by_type(type);
+ if (!proto)
+ return false;
+
if (set_properties)
psmouse_apply_defaults(psmouse);
- return detect(psmouse, set_properties);
+ if (proto->detect(psmouse, set_properties) != 0)
+ return false;
+
+ if (set_properties && do_init && proto->init) {
+ if (proto->init(psmouse) != 0) {
+ /*
+ * We detected device, but init failed. Adjust
+ * max_proto so we only try standard protocols.
+ */
+ if (*max_proto > PSMOUSE_IMEX)
+ *max_proto = PSMOUSE_IMEX;
+
+ return false;
+ }
+ }
+
+ return true;
}
/*
@@ -937,7 +968,8 @@ static int psmouse_extensions(struct psmouse *psmouse,
* Always check for focaltech, this is safe as it uses pnp-id
* matching.
*/
- if (psmouse_do_detect(focaltech_detect, psmouse, set_properties) == 0) {
+ if (psmouse_try_protocol(psmouse, PSMOUSE_FOCALTECH,
+ &max_proto, set_properties, false)) {
if (max_proto > PSMOUSE_IMEX) {
if (IS_ENABLED(CONFIG_MOUSE_PS2_FOCALTECH) &&
(!set_properties || focaltech_init(psmouse) == 0)) {
@@ -959,26 +991,21 @@ static int psmouse_extensions(struct psmouse *psmouse,
* We always check for LifeBook because it does not disturb mouse
* (it only checks DMI information).
*/
- if (psmouse_do_detect(lifebook_detect, psmouse, set_properties) == 0) {
- if (max_proto > PSMOUSE_IMEX) {
- if (!set_properties || lifebook_init(psmouse) == 0)
- return PSMOUSE_LIFEBOOK;
- }
- }
+ if (psmouse_try_protocol(psmouse, PSMOUSE_LIFEBOOK, &max_proto,
+ set_properties, max_proto > PSMOUSE_IMEX))
+ return PSMOUSE_LIFEBOOK;
- if (psmouse_do_detect(vmmouse_detect, psmouse, set_properties) == 0) {
- if (max_proto > PSMOUSE_IMEX) {
- if (!set_properties || vmmouse_init(psmouse) == 0)
- return PSMOUSE_VMMOUSE;
- }
- }
+ if (psmouse_try_protocol(psmouse, PSMOUSE_VMMOUSE, &max_proto,
+ set_properties, max_proto > PSMOUSE_IMEX))
+ return PSMOUSE_VMMOUSE;
/*
* Try Kensington ThinkingMouse (we try first, because Synaptics
* probe upsets the ThinkingMouse).
*/
if (max_proto > PSMOUSE_IMEX &&
- psmouse_do_detect(thinking_detect, psmouse, set_properties) == 0) {
+ psmouse_try_protocol(psmouse, PSMOUSE_THINKPS, &max_proto,
+ set_properties, true)) {
return PSMOUSE_THINKPS;
}
@@ -989,7 +1016,8 @@ static int psmouse_extensions(struct psmouse *psmouse,
* probing for IntelliMouse.
*/
if (max_proto > PSMOUSE_PS2 &&
- psmouse_do_detect(synaptics_detect, psmouse, set_properties) == 0) {
+ psmouse_try_protocol(psmouse, PSMOUSE_SYNAPTICS, &max_proto,
+ set_properties, false)) {
synaptics_hardware = true;
if (max_proto > PSMOUSE_IMEX) {
@@ -1025,68 +1053,48 @@ static int psmouse_extensions(struct psmouse *psmouse,
* Trackpads.
*/
if (max_proto > PSMOUSE_IMEX &&
- cypress_detect(psmouse, set_properties) == 0) {
- if (IS_ENABLED(CONFIG_MOUSE_PS2_CYPRESS)) {
- if (cypress_init(psmouse) == 0)
- return PSMOUSE_CYPRESS;
-
- /*
- * Finger Sensing Pad probe upsets some modules of
- * Cypress Trackpad, must avoid Finger Sensing Pad
- * probe if Cypress Trackpad device detected.
- */
- return PSMOUSE_PS2;
- }
-
- max_proto = PSMOUSE_IMEX;
+ psmouse_try_protocol(psmouse, PSMOUSE_CYPRESS, &max_proto,
+ set_properties, true)) {
+ return PSMOUSE_CYPRESS;
}
/* Try ALPS TouchPad */
if (max_proto > PSMOUSE_IMEX) {
ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_RESET_DIS);
- if (psmouse_do_detect(alps_detect,
- psmouse, set_properties) == 0) {
- if (!set_properties || alps_init(psmouse) == 0)
- return PSMOUSE_ALPS;
-
- /* Init failed, try basic relative protocols */
- max_proto = PSMOUSE_IMEX;
- }
+ if (psmouse_try_protocol(psmouse, PSMOUSE_ALPS,
+ &max_proto, set_properties, true))
+ return PSMOUSE_ALPS;
}
/* Try OLPC HGPK touchpad */
if (max_proto > PSMOUSE_IMEX &&
- psmouse_do_detect(hgpk_detect, psmouse, set_properties) == 0) {
- if (!set_properties || hgpk_init(psmouse) == 0)
- return PSMOUSE_HGPK;
- /* Init failed, try basic relative protocols */
- max_proto = PSMOUSE_IMEX;
+ psmouse_try_protocol(psmouse, PSMOUSE_HGPK, &max_proto,
+ set_properties, true)) {
+ return PSMOUSE_HGPK;
}
/* Try Elantech touchpad */
if (max_proto > PSMOUSE_IMEX &&
- psmouse_do_detect(elantech_detect, psmouse, set_properties) == 0) {
- if (!set_properties || elantech_init(psmouse) == 0)
- return PSMOUSE_ELANTECH;
- /* Init failed, try basic relative protocols */
- max_proto = PSMOUSE_IMEX;
+ psmouse_try_protocol(psmouse, PSMOUSE_ELANTECH,
+ &max_proto, set_properties, true)) {
+ return PSMOUSE_ELANTECH;
}
if (max_proto > PSMOUSE_IMEX) {
- if (psmouse_do_detect(genius_detect,
- psmouse, set_properties) == 0)
+ if (psmouse_try_protocol(psmouse, PSMOUSE_GENPS,
+ &max_proto, set_properties, true))
return PSMOUSE_GENPS;
- if (psmouse_do_detect(ps2pp_init,
- psmouse, set_properties) == 0)
+ if (psmouse_try_protocol(psmouse, PSMOUSE_PS2PP,
+ &max_proto, set_properties, true))
return PSMOUSE_PS2PP;
- if (psmouse_do_detect(trackpoint_detect,
- psmouse, set_properties) == 0)
+ if (psmouse_try_protocol(psmouse, PSMOUSE_TRACKPOINT,
+ &max_proto, set_properties, true))
return PSMOUSE_TRACKPOINT;
- if (psmouse_do_detect(touchkit_ps2_detect,
- psmouse, set_properties) == 0)
+ if (psmouse_try_protocol(psmouse, PSMOUSE_TOUCHKIT_PS2,
+ &max_proto, set_properties, true))
return PSMOUSE_TOUCHKIT_PS2;
}
@@ -1094,14 +1102,10 @@ static int psmouse_extensions(struct psmouse *psmouse,
* Try Finger Sensing Pad. We do it here because its probe upsets
* Trackpoint devices (causing TP_READ_ID command to time out).
*/
- if (max_proto > PSMOUSE_IMEX) {
- if (psmouse_do_detect(fsp_detect,
- psmouse, set_properties) == 0) {
- if (!set_properties || fsp_init(psmouse) == 0)
- return PSMOUSE_FSP;
- /* Init failed, try basic relative protocols */
- max_proto = PSMOUSE_IMEX;
- }
+ if (max_proto > PSMOUSE_IMEX &&
+ psmouse_try_protocol(psmouse, PSMOUSE_FSP,
+ &max_proto, set_properties, true)) {
+ return PSMOUSE_FSP;
}
/*
@@ -1113,14 +1117,14 @@ static int psmouse_extensions(struct psmouse *psmouse,
psmouse_reset(psmouse);
if (max_proto >= PSMOUSE_IMEX &&
- psmouse_do_detect(im_explorer_detect,
- psmouse, set_properties) == 0) {
+ psmouse_try_protocol(psmouse, PSMOUSE_IMEX,
+ &max_proto, set_properties, true)) {
return PSMOUSE_IMEX;
}
if (max_proto >= PSMOUSE_IMPS &&
- psmouse_do_detect(intellimouse_detect,
- psmouse, set_properties) == 0) {
+ psmouse_try_protocol(psmouse, PSMOUSE_IMPS,
+ &max_proto, set_properties, true)) {
return PSMOUSE_IMPS;
}
@@ -1128,7 +1132,8 @@ static int psmouse_extensions(struct psmouse *psmouse,
* Okay, all failed, we have a standard mouse here. The number of
* the buttons is still a question, though. We assume 3.
*/
- psmouse_do_detect(ps2bare_detect, psmouse, set_properties);
+ psmouse_try_protocol(psmouse, PSMOUSE_PS2,
+ &max_proto, set_properties, true);
if (synaptics_hardware) {
/*
--
2.6.0.rc2.230.g3dd15c0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 5/6] Input: psmouse - factor out common protocol probing code
2015-11-29 5:13 ` [PATCH 5/6] Input: psmouse - factor out common protocol probing code Dmitry Torokhov
@ 2015-12-02 15:20 ` Hans de Goede
2015-12-02 19:18 ` Dmitry Torokhov
0 siblings, 1 reply; 16+ messages in thread
From: Hans de Goede @ 2015-12-02 15:20 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input
Cc: Benjamin Tissoires, Thomas Hellstrom, pali.rohar, jckeerthan,
till2.schaefer, linux-kernel
Hi,
Thanks for splitting out the series, patches 1 - 4 look good to me and are:
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
I've some comments inline for this one.
On 29-11-15 06:13, Dmitry Torokhov wrote:
> In preparation of limiting protocols that we try on pass-through ports,
> let's rework initialization code and factor common code into
> psmouse_try_protocol() that accepts protocol type (instead of detec()
> function pointer) and can, for most protocols, perform both detection and
> initialization.
>
> Note that this removes option of forcing Lifebook protocol on devices that
> are not recognized by lifebook_detect() as having the hardware, but I do
> not recall anyone using this option.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> drivers/input/mouse/psmouse-base.c | 159 +++++++++++++++++++------------------
> 1 file changed, 82 insertions(+), 77 deletions(-)
>
> diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
> index b92c1bd..ee59b0e 100644
> --- a/drivers/input/mouse/psmouse-base.c
> +++ b/drivers/input/mouse/psmouse-base.c
> @@ -767,6 +767,7 @@ static const struct psmouse_protocol psmouse_protocols[] = {
> .type = PSMOUSE_LIFEBOOK,
> .name = "LBPS/2",
> .alias = "lifebook",
> + .detect = lifebook_detect,
> .init = lifebook_init,
> },
> #endif
> @@ -844,7 +845,7 @@ static const struct psmouse_protocol psmouse_protocols[] = {
> },
> };
>
> -static const struct psmouse_protocol *psmouse_protocol_by_type(enum psmouse_type type)
> +static const struct psmouse_protocol *__psmouse_protocol_by_type(enum psmouse_type type)
> {
> int i;
>
> @@ -852,6 +853,17 @@ static const struct psmouse_protocol *psmouse_protocol_by_type(enum psmouse_type
> if (psmouse_protocols[i].type == type)
> return &psmouse_protocols[i];
>
> + return NULL;
> +}
> +
> +static const struct psmouse_protocol *psmouse_protocol_by_type(enum psmouse_type type)
> +{
> + const struct psmouse_protocol *proto;
> +
> + proto = __psmouse_protocol_by_type(type);
> + if (proto)
> + return proto;
> +
> WARN_ON(1);
> return &psmouse_protocols[0];
> }
> @@ -910,18 +922,37 @@ static void psmouse_apply_defaults(struct psmouse *psmouse)
> psmouse->pt_deactivate = NULL;
> }
>
> -/*
> - * Apply default settings to the psmouse structure and call specified
> - * protocol detection or initialization routine.
> - */
> -static int psmouse_do_detect(int (*detect)(struct psmouse *psmouse,
> - bool set_properties),
> - struct psmouse *psmouse, bool set_properties)
> +static bool psmouse_try_protocol(struct psmouse *psmouse,
> + enum psmouse_type type,
> + unsigned int *max_proto,
> + bool set_properties, bool do_init)
> {
> + const struct psmouse_protocol *proto;
> +
> + proto = __psmouse_protocol_by_type(type);
> + if (!proto)
> + return false;
> +
> if (set_properties)
> psmouse_apply_defaults(psmouse);
>
> - return detect(psmouse, set_properties);
> + if (proto->detect(psmouse, set_properties) != 0)
> + return false;
> +
> + if (set_properties && do_init && proto->init) {
> + if (proto->init(psmouse) != 0) {
> + /*
> + * We detected device, but init failed. Adjust
> + * max_proto so we only try standard protocols.
> + */
> + if (*max_proto > PSMOUSE_IMEX)
> + *max_proto = PSMOUSE_IMEX;
> +
> + return false;
> + }
> + }
> +
> + return true;
> }
>
> /*
> @@ -937,7 +968,8 @@ static int psmouse_extensions(struct psmouse *psmouse,
> * Always check for focaltech, this is safe as it uses pnp-id
> * matching.
> */
> - if (psmouse_do_detect(focaltech_detect, psmouse, set_properties) == 0) {
> + if (psmouse_try_protocol(psmouse, PSMOUSE_FOCALTECH,
> + &max_proto, set_properties, false)) {
> if (max_proto > PSMOUSE_IMEX) {
> if (IS_ENABLED(CONFIG_MOUSE_PS2_FOCALTECH) &&
> (!set_properties || focaltech_init(psmouse) == 0)) {
> @@ -959,26 +991,21 @@ static int psmouse_extensions(struct psmouse *psmouse,
> * We always check for LifeBook because it does not disturb mouse
> * (it only checks DMI information).
> */
> - if (psmouse_do_detect(lifebook_detect, psmouse, set_properties) == 0) {
> - if (max_proto > PSMOUSE_IMEX) {
> - if (!set_properties || lifebook_init(psmouse) == 0)
> - return PSMOUSE_LIFEBOOK;
> - }
> - }
> + if (psmouse_try_protocol(psmouse, PSMOUSE_LIFEBOOK, &max_proto,
> + set_properties, max_proto > PSMOUSE_IMEX))
> + return PSMOUSE_LIFEBOOK;
>
> - if (psmouse_do_detect(vmmouse_detect, psmouse, set_properties) == 0) {
> - if (max_proto > PSMOUSE_IMEX) {
> - if (!set_properties || vmmouse_init(psmouse) == 0)
> - return PSMOUSE_VMMOUSE;
> - }
> - }
> + if (psmouse_try_protocol(psmouse, PSMOUSE_VMMOUSE, &max_proto,
> + set_properties, max_proto > PSMOUSE_IMEX))
> + return PSMOUSE_VMMOUSE;
>
> /*
> * Try Kensington ThinkingMouse (we try first, because Synaptics
> * probe upsets the ThinkingMouse).
> */
> if (max_proto > PSMOUSE_IMEX &&
> - psmouse_do_detect(thinking_detect, psmouse, set_properties) == 0) {
> + psmouse_try_protocol(psmouse, PSMOUSE_THINKPS, &max_proto,
> + set_properties, true)) {
> return PSMOUSE_THINKPS;
> }
>
> @@ -989,7 +1016,8 @@ static int psmouse_extensions(struct psmouse *psmouse,
> * probing for IntelliMouse.
> */
> if (max_proto > PSMOUSE_PS2 &&
> - psmouse_do_detect(synaptics_detect, psmouse, set_properties) == 0) {
> + psmouse_try_protocol(psmouse, PSMOUSE_SYNAPTICS, &max_proto,
> + set_properties, false)) {
> synaptics_hardware = true;
>
> if (max_proto > PSMOUSE_IMEX) {
> @@ -1025,68 +1053,48 @@ static int psmouse_extensions(struct psmouse *psmouse,
> * Trackpads.
> */
> if (max_proto > PSMOUSE_IMEX &&
> - cypress_detect(psmouse, set_properties) == 0) {
> - if (IS_ENABLED(CONFIG_MOUSE_PS2_CYPRESS)) {
> - if (cypress_init(psmouse) == 0)
> - return PSMOUSE_CYPRESS;
> -
> - /*
> - * Finger Sensing Pad probe upsets some modules of
> - * Cypress Trackpad, must avoid Finger Sensing Pad
> - * probe if Cypress Trackpad device detected.
> - */
> - return PSMOUSE_PS2;
> - }
> -
> - max_proto = PSMOUSE_IMEX;
> + psmouse_try_protocol(psmouse, PSMOUSE_CYPRESS, &max_proto,
> + set_properties, true)) {
> + return PSMOUSE_CYPRESS;
> }
The protocols array has the CYPRESS entry wrapped in "#ifdef CONFIG_MOUSE_PS2_CYPRESS"
so this bit of the patches changes behavior of the probe order if
CONFIG_MOUSE_PS2_CYPRESS is not enabled. Before this patch the max_proto would
get limited to PSMOUSE_IMEX, but now the:
proto = __psmouse_protocol_by_type(type);
if (!proto)
return false;
Part of psmouse_try_protocol will trigger and then max_proto will stay unmodified.
I think this can best be fixed by always including CYPRESS in the proto array,
and have init be a stub which always fails when CYPRESS is not actually enabled.
> /* Try ALPS TouchPad */
> if (max_proto > PSMOUSE_IMEX) {
> ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_RESET_DIS);
> - if (psmouse_do_detect(alps_detect,
> - psmouse, set_properties) == 0) {
> - if (!set_properties || alps_init(psmouse) == 0)
> - return PSMOUSE_ALPS;
> -
> - /* Init failed, try basic relative protocols */
> - max_proto = PSMOUSE_IMEX;
> - }
> + if (psmouse_try_protocol(psmouse, PSMOUSE_ALPS,
> + &max_proto, set_properties, true))
> + return PSMOUSE_ALPS;
> }
>
> /* Try OLPC HGPK touchpad */
> if (max_proto > PSMOUSE_IMEX &&
> - psmouse_do_detect(hgpk_detect, psmouse, set_properties) == 0) {
> - if (!set_properties || hgpk_init(psmouse) == 0)
> - return PSMOUSE_HGPK;
> - /* Init failed, try basic relative protocols */
> - max_proto = PSMOUSE_IMEX;
> + psmouse_try_protocol(psmouse, PSMOUSE_HGPK, &max_proto,
> + set_properties, true)) {
> + return PSMOUSE_HGPK;
> }
>
> /* Try Elantech touchpad */
> if (max_proto > PSMOUSE_IMEX &&
> - psmouse_do_detect(elantech_detect, psmouse, set_properties) == 0) {
> - if (!set_properties || elantech_init(psmouse) == 0)
> - return PSMOUSE_ELANTECH;
> - /* Init failed, try basic relative protocols */
> - max_proto = PSMOUSE_IMEX;
> + psmouse_try_protocol(psmouse, PSMOUSE_ELANTECH,
> + &max_proto, set_properties, true)) {
> + return PSMOUSE_ELANTECH;
> }
>
> if (max_proto > PSMOUSE_IMEX) {
> - if (psmouse_do_detect(genius_detect,
> - psmouse, set_properties) == 0)
> + if (psmouse_try_protocol(psmouse, PSMOUSE_GENPS,
> + &max_proto, set_properties, true))
> return PSMOUSE_GENPS;
>
> - if (psmouse_do_detect(ps2pp_init,
> - psmouse, set_properties) == 0)
> + if (psmouse_try_protocol(psmouse, PSMOUSE_PS2PP,
> + &max_proto, set_properties, true))
> return PSMOUSE_PS2PP;
The PS2PP entry in the protocols table has an init function, by passing
in true here you are causing this to get called, where as it was not
being called before.
>
> - if (psmouse_do_detect(trackpoint_detect,
> - psmouse, set_properties) == 0)
> + if (psmouse_try_protocol(psmouse, PSMOUSE_TRACKPOINT,
> + &max_proto, set_properties, true))
> return PSMOUSE_TRACKPOINT;
>
> - if (psmouse_do_detect(touchkit_ps2_detect,
> - psmouse, set_properties) == 0)
> + if (psmouse_try_protocol(psmouse, PSMOUSE_TOUCHKIT_PS2,
> + &max_proto, set_properties, true))
> return PSMOUSE_TOUCHKIT_PS2;
> }
>
> @@ -1094,14 +1102,10 @@ static int psmouse_extensions(struct psmouse *psmouse,
> * Try Finger Sensing Pad. We do it here because its probe upsets
> * Trackpoint devices (causing TP_READ_ID command to time out).
> */
> - if (max_proto > PSMOUSE_IMEX) {
> - if (psmouse_do_detect(fsp_detect,
> - psmouse, set_properties) == 0) {
> - if (!set_properties || fsp_init(psmouse) == 0)
> - return PSMOUSE_FSP;
> - /* Init failed, try basic relative protocols */
> - max_proto = PSMOUSE_IMEX;
> - }
> + if (max_proto > PSMOUSE_IMEX &&
> + psmouse_try_protocol(psmouse, PSMOUSE_FSP,
> + &max_proto, set_properties, true)) {
> + return PSMOUSE_FSP;
> }
>
> /*
> @@ -1113,14 +1117,14 @@ static int psmouse_extensions(struct psmouse *psmouse,
> psmouse_reset(psmouse);
>
> if (max_proto >= PSMOUSE_IMEX &&
> - psmouse_do_detect(im_explorer_detect,
> - psmouse, set_properties) == 0) {
> + psmouse_try_protocol(psmouse, PSMOUSE_IMEX,
> + &max_proto, set_properties, true)) {
> return PSMOUSE_IMEX;
> }
>
> if (max_proto >= PSMOUSE_IMPS &&
> - psmouse_do_detect(intellimouse_detect,
> - psmouse, set_properties) == 0) {
> + psmouse_try_protocol(psmouse, PSMOUSE_IMPS,
> + &max_proto, set_properties, true)) {
> return PSMOUSE_IMPS;
> }
>
> @@ -1128,7 +1132,8 @@ static int psmouse_extensions(struct psmouse *psmouse,
> * Okay, all failed, we have a standard mouse here. The number of
> * the buttons is still a question, though. We assume 3.
> */
> - psmouse_do_detect(ps2bare_detect, psmouse, set_properties);
> + psmouse_try_protocol(psmouse, PSMOUSE_PS2,
> + &max_proto, set_properties, true);
>
> if (synaptics_hardware) {
> /*
>
Regards,
Hans
p.s.
After this change, a whole lot of the code has the form of:
if (max_proto >= PSMOUSE_FOO &&
psmouse_try_protocol(psmouse, ...)) {
return PSMOUSE_BAR;
}
Maybe you can do a follow-up which makes PSMOUSE_FOO a parameter to
psmouse_try_protocol and move the test into psmouse_try_protocol?
Also you may want to consider to drop the {} around the single return
statement most of this code-blocks now have. Maybe best to do
this in a followup patch to keep the diff readable.
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 5/6] Input: psmouse - factor out common protocol probing code
2015-12-02 15:20 ` Hans de Goede
@ 2015-12-02 19:18 ` Dmitry Torokhov
2015-12-03 8:33 ` Hans de Goede
0 siblings, 1 reply; 16+ messages in thread
From: Dmitry Torokhov @ 2015-12-02 19:18 UTC (permalink / raw)
To: Hans de Goede
Cc: linux-input, Benjamin Tissoires, Thomas Hellstrom, pali.rohar,
jckeerthan, till2.schaefer, linux-kernel
Hi Hans,
On Wed, Dec 02, 2015 at 04:20:48PM +0100, Hans de Goede wrote:
> Hi,
>
> Thanks for splitting out the series, patches 1 - 4 look good to me and are:
>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
>
> I've some comments inline for this one.
Thanks for spending time on reviewing this.
> >@@ -1025,68 +1053,48 @@ static int psmouse_extensions(struct psmouse *psmouse,
> > * Trackpads.
> > */
> > if (max_proto > PSMOUSE_IMEX &&
> >- cypress_detect(psmouse, set_properties) == 0) {
> >- if (IS_ENABLED(CONFIG_MOUSE_PS2_CYPRESS)) {
> >- if (cypress_init(psmouse) == 0)
> >- return PSMOUSE_CYPRESS;
> >-
> >- /*
> >- * Finger Sensing Pad probe upsets some modules of
> >- * Cypress Trackpad, must avoid Finger Sensing Pad
> >- * probe if Cypress Trackpad device detected.
> >- */
> >- return PSMOUSE_PS2;
> >- }
> >-
> >- max_proto = PSMOUSE_IMEX;
> >+ psmouse_try_protocol(psmouse, PSMOUSE_CYPRESS, &max_proto,
> >+ set_properties, true)) {
> >+ return PSMOUSE_CYPRESS;
> > }
>
> The protocols array has the CYPRESS entry wrapped in "#ifdef CONFIG_MOUSE_PS2_CYPRESS"
> so this bit of the patches changes behavior of the probe order if
> CONFIG_MOUSE_PS2_CYPRESS is not enabled. Before this patch the max_proto would
> get limited to PSMOUSE_IMEX, but now the:
>
> proto = __psmouse_protocol_by_type(type);
> if (!proto)
> return false;
>
> Part of psmouse_try_protocol will trigger and then max_proto will stay unmodified.
>
> I think this can best be fixed by always including CYPRESS in the proto array,
> and have init be a stub which always fails when CYPRESS is not actually enabled.
I would not want to do that as that would make cypress be in the list of
supported protocols whereas it is actually compiled out.
BUT, there is actually no problem, the original code was misleading.
cypress_detect() is already a stub returning -ENOSYS when
CONFIG_MOUSE_PS2_CYPRESS is not enabled, so we would never went into
that "if" body in that case.
I'll make a patch cleaning it up, but won't repost the whole series so
as to not clutter the list.
> >
> > if (max_proto > PSMOUSE_IMEX) {
> >- if (psmouse_do_detect(genius_detect,
> >- psmouse, set_properties) == 0)
> >+ if (psmouse_try_protocol(psmouse, PSMOUSE_GENPS,
> >+ &max_proto, set_properties, true))
> > return PSMOUSE_GENPS;
> >
> >- if (psmouse_do_detect(ps2pp_init,
> >- psmouse, set_properties) == 0)
> >+ if (psmouse_try_protocol(psmouse, PSMOUSE_PS2PP,
> >+ &max_proto, set_properties, true))
> > return PSMOUSE_PS2PP;
>
> The PS2PP entry in the protocols table has an init function, by passing
> in true here you are causing this to get called, where as it was not
> being called before.
No, it has detect function only (but called ps2pp_init), I'll post the
patch renaming it to ps2pp_detect.
>
> p.s.
>
> After this change, a whole lot of the code has the form of:
>
> if (max_proto >= PSMOUSE_FOO &&
> psmouse_try_protocol(psmouse, ...)) {
> return PSMOUSE_BAR;
> }
>
> Maybe you can do a follow-up which makes PSMOUSE_FOO a parameter to
> psmouse_try_protocol and move the test into psmouse_try_protocol?
I considered it, but some protocols we detect unconditionally and some
conditionally, passing a parameter woudl somewhat hide it. Plus
try_protocol() already has a lot of parameters.
>
> Also you may want to consider to drop the {} around the single return
> statement most of this code-blocks now have. Maybe best to do
> this in a followup patch to keep the diff readable.
I like to keep the braces if "if" condition is multi-line to better see
where condition ends and body starts.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 5/6] Input: psmouse - factor out common protocol probing code
2015-12-02 19:18 ` Dmitry Torokhov
@ 2015-12-03 8:33 ` Hans de Goede
0 siblings, 0 replies; 16+ messages in thread
From: Hans de Goede @ 2015-12-03 8:33 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input, Benjamin Tissoires, Thomas Hellstrom, pali.rohar,
jckeerthan, till2.schaefer, linux-kernel
Hi,
On 02-12-15 20:18, Dmitry Torokhov wrote:
> Hi Hans,
>
> On Wed, Dec 02, 2015 at 04:20:48PM +0100, Hans de Goede wrote:
>> Hi,
>>
>> Thanks for splitting out the series, patches 1 - 4 look good to me and are:
>>
>> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
>>
>> I've some comments inline for this one.
>
> Thanks for spending time on reviewing this.
>
>>> @@ -1025,68 +1053,48 @@ static int psmouse_extensions(struct psmouse *psmouse,
>>> * Trackpads.
>>> */
>>> if (max_proto > PSMOUSE_IMEX &&
>>> - cypress_detect(psmouse, set_properties) == 0) {
>>> - if (IS_ENABLED(CONFIG_MOUSE_PS2_CYPRESS)) {
>>> - if (cypress_init(psmouse) == 0)
>>> - return PSMOUSE_CYPRESS;
>>> -
>>> - /*
>>> - * Finger Sensing Pad probe upsets some modules of
>>> - * Cypress Trackpad, must avoid Finger Sensing Pad
>>> - * probe if Cypress Trackpad device detected.
>>> - */
>>> - return PSMOUSE_PS2;
>>> - }
>>> -
>>> - max_proto = PSMOUSE_IMEX;
>>> + psmouse_try_protocol(psmouse, PSMOUSE_CYPRESS, &max_proto,
>>> + set_properties, true)) {
>>> + return PSMOUSE_CYPRESS;
>>> }
>>
>> The protocols array has the CYPRESS entry wrapped in "#ifdef CONFIG_MOUSE_PS2_CYPRESS"
>> so this bit of the patches changes behavior of the probe order if
>> CONFIG_MOUSE_PS2_CYPRESS is not enabled. Before this patch the max_proto would
>> get limited to PSMOUSE_IMEX, but now the:
>>
>> proto = __psmouse_protocol_by_type(type);
>> if (!proto)
>> return false;
>>
>> Part of psmouse_try_protocol will trigger and then max_proto will stay unmodified.
>>
>> I think this can best be fixed by always including CYPRESS in the proto array,
>> and have init be a stub which always fails when CYPRESS is not actually enabled.
>
> I would not want to do that as that would make cypress be in the list of
> supported protocols whereas it is actually compiled out.
>
> BUT, there is actually no problem, the original code was misleading.
> cypress_detect() is already a stub returning -ENOSYS when
> CONFIG_MOUSE_PS2_CYPRESS is not enabled, so we would never went into
> that "if" body in that case.
Ah, so the whole "if (IS_ENABLED(CONFIG_MOUSE_PS2_CYPRESS))" check in the original
code was not really necessary, I see.
> I'll make a patch cleaning it up, but won't repost the whole series so
> as to not clutter the list.
Ack.
>>>
>>> if (max_proto > PSMOUSE_IMEX) {
>>> - if (psmouse_do_detect(genius_detect,
>>> - psmouse, set_properties) == 0)
>>> + if (psmouse_try_protocol(psmouse, PSMOUSE_GENPS,
>>> + &max_proto, set_properties, true))
>>> return PSMOUSE_GENPS;
>>>
>>> - if (psmouse_do_detect(ps2pp_init,
>>> - psmouse, set_properties) == 0)
>>> + if (psmouse_try_protocol(psmouse, PSMOUSE_PS2PP,
>>> + &max_proto, set_properties, true))
>>> return PSMOUSE_PS2PP;
>>
>> The PS2PP entry in the protocols table has an init function, by passing
>> in true here you are causing this to get called, where as it was not
>> being called before.
>
> No, it has detect function only (but called ps2pp_init), I'll post the
> patch renaming it to ps2pp_detect.
Right, I misread that as it having an init function.
>> p.s.
>>
>> After this change, a whole lot of the code has the form of:
>>
>> if (max_proto >= PSMOUSE_FOO &&
>> psmouse_try_protocol(psmouse, ...)) {
>> return PSMOUSE_BAR;
>> }
>>
>> Maybe you can do a follow-up which makes PSMOUSE_FOO a parameter to
>> psmouse_try_protocol and move the test into psmouse_try_protocol?
>
> I considered it, but some protocols we detect unconditionally and some
> conditionally, passing a parameter woudl somewhat hide it. Plus
> try_protocol() already has a lot of parameters.
Ok.
>> Also you may want to consider to drop the {} around the single return
>> statement most of this code-blocks now have. Maybe best to do
>> this in a followup patch to keep the diff readable.
>
> I like to keep the braces if "if" condition is multi-line to better see
> where condition ends and body starts.
Fair enough, I've no problem with that.
With the above explanations this patch is:
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Regards,
Hans
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 6/6] Input: psmouse - limit protocols that we try on passthrough ports
2015-11-29 5:13 [PATCH 0/6] Only probe select protocols on pass through PS/2 prots Dmitry Torokhov
` (4 preceding siblings ...)
2015-11-29 5:13 ` [PATCH 5/6] Input: psmouse - factor out common protocol probing code Dmitry Torokhov
@ 2015-11-29 5:13 ` Dmitry Torokhov
2015-12-02 15:21 ` Hans de Goede
2015-12-11 11:50 ` Pali Rohár
5 siblings, 2 replies; 16+ messages in thread
From: Dmitry Torokhov @ 2015-11-29 5:13 UTC (permalink / raw)
To: linux-input
Cc: Hans de Goede, Benjamin Tissoires, Thomas Hellstrom, pali.rohar,
jckeerthan, till2.schaefer, linux-kernel
PS/2 protocol is slow, and using it with pass-through port (where we
encapsulate PS/2 into PS/2) is slower yet so it takes quite a bit of time
to do full protocol discovery for device attached to a pass-through port.
However, so far we have not see anything but trackpoints or basic PS/2
mice on pass-through ports, so let's limit protocols that we probe there
to Trackpoint, IntelliMouse Explorer, IntelliMouse, and bare PS/2 protocol,
and avoid other extended protocols, such as Synaptics, ALPS, etc.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/mouse/psmouse-base.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
index ee59b0e..e909c6e 100644
--- a/drivers/input/mouse/psmouse-base.c
+++ b/drivers/input/mouse/psmouse-base.c
@@ -119,6 +119,7 @@ struct psmouse_protocol {
enum psmouse_type type;
bool maxproto;
bool ignore_parity; /* Protocol should ignore parity errors from KBC */
+ bool try_passthru; /* Try protocol also on passthrough ports */
const char *name;
const char *alias;
int (*detect)(struct psmouse *, bool);
@@ -691,6 +692,7 @@ static const struct psmouse_protocol psmouse_protocols[] = {
.maxproto = true,
.ignore_parity = true,
.detect = ps2bare_detect,
+ .try_passthru = true,
},
#ifdef CONFIG_MOUSE_PS2_LOGIPS2PP
{
@@ -728,6 +730,7 @@ static const struct psmouse_protocol psmouse_protocols[] = {
.maxproto = true,
.ignore_parity = true,
.detect = intellimouse_detect,
+ .try_passthru = true,
},
{
.type = PSMOUSE_IMEX,
@@ -736,6 +739,7 @@ static const struct psmouse_protocol psmouse_protocols[] = {
.maxproto = true,
.ignore_parity = true,
.detect = im_explorer_detect,
+ .try_passthru = true,
},
#ifdef CONFIG_MOUSE_PS2_SYNAPTICS
{
@@ -777,6 +781,7 @@ static const struct psmouse_protocol psmouse_protocols[] = {
.name = "TPPS/2",
.alias = "trackpoint",
.detect = trackpoint_detect,
+ .try_passthru = true,
},
#endif
#ifdef CONFIG_MOUSE_PS2_TOUCHKIT
@@ -933,6 +938,11 @@ static bool psmouse_try_protocol(struct psmouse *psmouse,
if (!proto)
return false;
+ if (psmouse->ps2dev.serio->id.type == SERIO_PS_PSTHRU &&
+ !proto->try_passthru) {
+ return false;
+ }
+
if (set_properties)
psmouse_apply_defaults(psmouse);
--
2.6.0.rc2.230.g3dd15c0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 6/6] Input: psmouse - limit protocols that we try on passthrough ports
2015-11-29 5:13 ` [PATCH 6/6] Input: psmouse - limit protocols that we try on passthrough ports Dmitry Torokhov
@ 2015-12-02 15:21 ` Hans de Goede
2015-12-11 11:50 ` Pali Rohár
1 sibling, 0 replies; 16+ messages in thread
From: Hans de Goede @ 2015-12-02 15:21 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input
Cc: Benjamin Tissoires, Thomas Hellstrom, pali.rohar, jckeerthan,
till2.schaefer, linux-kernel
Hi,
On 29-11-15 06:13, Dmitry Torokhov wrote:
> PS/2 protocol is slow, and using it with pass-through port (where we
> encapsulate PS/2 into PS/2) is slower yet so it takes quite a bit of time
> to do full protocol discovery for device attached to a pass-through port.
> However, so far we have not see anything but trackpoints or basic PS/2
> mice on pass-through ports, so let's limit protocols that we probe there
> to Trackpoint, IntelliMouse Explorer, IntelliMouse, and bare PS/2 protocol,
> and avoid other extended protocols, such as Synaptics, ALPS, etc.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Regards,
Hans
> ---
> drivers/input/mouse/psmouse-base.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
> index ee59b0e..e909c6e 100644
> --- a/drivers/input/mouse/psmouse-base.c
> +++ b/drivers/input/mouse/psmouse-base.c
> @@ -119,6 +119,7 @@ struct psmouse_protocol {
> enum psmouse_type type;
> bool maxproto;
> bool ignore_parity; /* Protocol should ignore parity errors from KBC */
> + bool try_passthru; /* Try protocol also on passthrough ports */
> const char *name;
> const char *alias;
> int (*detect)(struct psmouse *, bool);
> @@ -691,6 +692,7 @@ static const struct psmouse_protocol psmouse_protocols[] = {
> .maxproto = true,
> .ignore_parity = true,
> .detect = ps2bare_detect,
> + .try_passthru = true,
> },
> #ifdef CONFIG_MOUSE_PS2_LOGIPS2PP
> {
> @@ -728,6 +730,7 @@ static const struct psmouse_protocol psmouse_protocols[] = {
> .maxproto = true,
> .ignore_parity = true,
> .detect = intellimouse_detect,
> + .try_passthru = true,
> },
> {
> .type = PSMOUSE_IMEX,
> @@ -736,6 +739,7 @@ static const struct psmouse_protocol psmouse_protocols[] = {
> .maxproto = true,
> .ignore_parity = true,
> .detect = im_explorer_detect,
> + .try_passthru = true,
> },
> #ifdef CONFIG_MOUSE_PS2_SYNAPTICS
> {
> @@ -777,6 +781,7 @@ static const struct psmouse_protocol psmouse_protocols[] = {
> .name = "TPPS/2",
> .alias = "trackpoint",
> .detect = trackpoint_detect,
> + .try_passthru = true,
> },
> #endif
> #ifdef CONFIG_MOUSE_PS2_TOUCHKIT
> @@ -933,6 +938,11 @@ static bool psmouse_try_protocol(struct psmouse *psmouse,
> if (!proto)
> return false;
>
> + if (psmouse->ps2dev.serio->id.type == SERIO_PS_PSTHRU &&
> + !proto->try_passthru) {
> + return false;
> + }
> +
> if (set_properties)
> psmouse_apply_defaults(psmouse);
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 6/6] Input: psmouse - limit protocols that we try on passthrough ports
2015-11-29 5:13 ` [PATCH 6/6] Input: psmouse - limit protocols that we try on passthrough ports Dmitry Torokhov
2015-12-02 15:21 ` Hans de Goede
@ 2015-12-11 11:50 ` Pali Rohár
1 sibling, 0 replies; 16+ messages in thread
From: Pali Rohár @ 2015-12-11 11:50 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input, Hans de Goede, Benjamin Tissoires, Thomas Hellstrom,
jckeerthan, till2.schaefer, linux-kernel
On Saturday 28 November 2015 21:13:56 Dmitry Torokhov wrote:
> PS/2 protocol is slow, and using it with pass-through port (where we
> encapsulate PS/2 into PS/2) is slower yet so it takes quite a bit of time
> to do full protocol discovery for device attached to a pass-through port.
> However, so far we have not see anything but trackpoints or basic PS/2
> mice on pass-through ports, so let's limit protocols that we probe there
> to Trackpoint, IntelliMouse Explorer, IntelliMouse, and bare PS/2 protocol,
> and avoid other extended protocols, such as Synaptics, ALPS, etc.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Reviewed-by: Pali Rohár <pali.rohar@gmail.com>
Thanks for this patch series!
--
Pali Rohár
pali.rohar@gmail.com
^ permalink raw reply [flat|nested] 16+ messages in thread