2020-09-04 02:53:15

by Jingle.Wu

[permalink] [raw]
Subject: [PATCH] Input: trackpoint - Add Suspend mode for Elan TrackPoint

From: "jingle.wu" <[email protected]>

Add suspend command for elan trackpoint
---
drivers/input/mouse/trackpoint.c | 16 ++++++++++++++++
drivers/input/mouse/trackpoint.h | 1 +
2 files changed, 17 insertions(+)

diff --git a/drivers/input/mouse/trackpoint.c b/drivers/input/mouse/trackpoint.c
index 3eefee2ee2a1..b3cba973a4eb 100644
--- a/drivers/input/mouse/trackpoint.c
+++ b/drivers/input/mouse/trackpoint.c
@@ -389,6 +389,20 @@ static int trackpoint_reconnect(struct psmouse *psmouse)
return 0;
}

+static void trackpoint_cleanup(struct psmouse *psmouse)
+{
+ struct trackpoint_data *tp = psmouse->private;
+ u8 param[3] = { TP_TOGGLE, TP_TOGGLE_BURST, TP_TOGGLE_ELAN_SLEEP };
+
+ if (tp->variant_id == TP_VARIANT_ELAN) {
+ if (ps2_command(&psmouse->ps2dev, param,
+ MAKE_PS2_CMD(3, 0, TP_COMMAND))) {
+ psmouse_err(psmouse,
+ "failed to suspend trackpont.\n");
+ }
+ }
+}
+
int trackpoint_detect(struct psmouse *psmouse, bool set_properties)
{
struct ps2dev *ps2dev = &psmouse->ps2dev;
@@ -421,6 +435,8 @@ int trackpoint_detect(struct psmouse *psmouse, bool set_properties)
psmouse->reconnect = trackpoint_reconnect;
psmouse->disconnect = trackpoint_disconnect;

+ psmouse->cleanup = trackpoint_cleanup;
+
if (variant_id != TP_VARIANT_IBM) {
/* Newer variants do not support extended button query. */
button_info = 0x33;
diff --git a/drivers/input/mouse/trackpoint.h b/drivers/input/mouse/trackpoint.h
index 5cb93ed26085..c7fa75452976 100644
--- a/drivers/input/mouse/trackpoint.h
+++ b/drivers/input/mouse/trackpoint.h
@@ -107,6 +107,7 @@
#define TP_TOGGLE_EXT_TAG 0x22 /* Bit 3 of the first packet coming from the
external device will be forced to 1 */
#define TP_MASK_EXT_TAG 0x04
+#define TP_TOGGLE_ELAN_SLEEP 0x8


/* Power on Self Test Results */
--
2.17.1


2022-03-24 21:11:21

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH] Input: trackpoint - Add Suspend mode for Elan TrackPoint

Hi Jingle,

On Fri, Sep 04, 2020 at 10:42:31AM +0800, jingle.wu wrote:
> From: "jingle.wu" <[email protected]>
>
> Add suspend command for elan trackpoint

Could you please elaborate what this mode does to the trackpint?

Also, your signed-off-by is missing.

> ---
> drivers/input/mouse/trackpoint.c | 16 ++++++++++++++++
> drivers/input/mouse/trackpoint.h | 1 +
> 2 files changed, 17 insertions(+)
>
> diff --git a/drivers/input/mouse/trackpoint.c b/drivers/input/mouse/trackpoint.c
> index 3eefee2ee2a1..b3cba973a4eb 100644
> --- a/drivers/input/mouse/trackpoint.c
> +++ b/drivers/input/mouse/trackpoint.c
> @@ -389,6 +389,20 @@ static int trackpoint_reconnect(struct psmouse *psmouse)
> return 0;
> }
>
> +static void trackpoint_cleanup(struct psmouse *psmouse)
> +{
> + struct trackpoint_data *tp = psmouse->private;
> + u8 param[3] = { TP_TOGGLE, TP_TOGGLE_BURST, TP_TOGGLE_ELAN_SLEEP };
> +
> + if (tp->variant_id == TP_VARIANT_ELAN) {
> + if (ps2_command(&psmouse->ps2dev, param,

I'd combine the 2 "ifs".

> + MAKE_PS2_CMD(3, 0, TP_COMMAND))) {
> + psmouse_err(psmouse,

psmouse_warn() since it is not a hard error.

> + "failed to suspend trackpont.\n");

I'd say "failed to suspend Elan trackpont\n".

> + }
> + }
> +}
> +
> int trackpoint_detect(struct psmouse *psmouse, bool set_properties)
> {
> struct ps2dev *ps2dev = &psmouse->ps2dev;
> @@ -421,6 +435,8 @@ int trackpoint_detect(struct psmouse *psmouse, bool set_properties)
> psmouse->reconnect = trackpoint_reconnect;
> psmouse->disconnect = trackpoint_disconnect;
>
> + psmouse->cleanup = trackpoint_cleanup;
> +
> if (variant_id != TP_VARIANT_IBM) {
> /* Newer variants do not support extended button query. */
> button_info = 0x33;
> diff --git a/drivers/input/mouse/trackpoint.h b/drivers/input/mouse/trackpoint.h
> index 5cb93ed26085..c7fa75452976 100644
> --- a/drivers/input/mouse/trackpoint.h
> +++ b/drivers/input/mouse/trackpoint.h
> @@ -107,6 +107,7 @@
> #define TP_TOGGLE_EXT_TAG 0x22 /* Bit 3 of the first packet coming from the
> external device will be forced to 1 */
> #define TP_MASK_EXT_TAG 0x04
> +#define TP_TOGGLE_ELAN_SLEEP 0x8

It seems to me that TP_TOGGLE_ELAN_SLEEP is a bit in TP_TOGGLE_BURST
subcommand, so it is better to move it next to TP_MASK_BURST and call it
TP_MASK_ELAN_SLEEP to match the rest.

Thanks.

--
Dmitry

2022-03-25 10:07:34

by Josh.Chen

[permalink] [raw]
Subject: RE: [PATCH] Input: trackpoint - Add Suspend mode for Elan TrackPoint

Hi Dmitry,

When the trackpoint device receives this command,

it enters the power saving mode (power consumption <400uA),

the trackpoint would be woken up after touching it.


This patch was tested on 5.4, do we have to test it again on 5.10?

Thanks,
Josh

-----Original Message-----
From: Jingle.Wu [mailto:[email protected]]
Sent: Thursday, March 24, 2022 9:13 AM
To: 'Dmitry Torokhov'; 'Dave.Wang'; 'Josh.Chen'
Cc: [email protected]; [email protected];
[email protected]; 'jingle.wu'
Subject: RE: [PATCH] Input: trackpoint - Add Suspend mode for Elan
TrackPoint

Loop Josh, Dave.

-----Original Message-----
From: Dmitry Torokhov [mailto:[email protected]]
Sent: Thursday, March 24, 2022 1:28 AM
To: jingle.wu <[email protected]>
Cc: [email protected]; [email protected];
[email protected]; jingle.wu <[email protected]>
Subject: Re: [PATCH] Input: trackpoint - Add Suspend mode for Elan
TrackPoint

Hi Jingle,

On Fri, Sep 04, 2020 at 10:42:31AM +0800, jingle.wu wrote:
> From: "jingle.wu" <[email protected]>
>
> Add suspend command for elan trackpoint

Could you please elaborate what this mode does to the trackpint?

Also, your signed-off-by is missing.

> ---
> drivers/input/mouse/trackpoint.c | 16 ++++++++++++++++
> drivers/input/mouse/trackpoint.h | 1 +
> 2 files changed, 17 insertions(+)
>
> diff --git a/drivers/input/mouse/trackpoint.c
> b/drivers/input/mouse/trackpoint.c
> index 3eefee2ee2a1..b3cba973a4eb 100644
> --- a/drivers/input/mouse/trackpoint.c
> +++ b/drivers/input/mouse/trackpoint.c
> @@ -389,6 +389,20 @@ static int trackpoint_reconnect(struct psmouse
*psmouse)
> return 0;
> }
>
> +static void trackpoint_cleanup(struct psmouse *psmouse) {
> + struct trackpoint_data *tp = psmouse->private;
> + u8 param[3] = { TP_TOGGLE, TP_TOGGLE_BURST, TP_TOGGLE_ELAN_SLEEP };
> +
> + if (tp->variant_id == TP_VARIANT_ELAN) {
> + if (ps2_command(&psmouse->ps2dev, param,

I'd combine the 2 "ifs".

> + MAKE_PS2_CMD(3, 0, TP_COMMAND))) {
> + psmouse_err(psmouse,

psmouse_warn() since it is not a hard error.

> + "failed to suspend trackpont.\n");

I'd say "failed to suspend Elan trackpont\n".

> + }
> + }
> +}
> +
> int trackpoint_detect(struct psmouse *psmouse, bool set_properties) {
> struct ps2dev *ps2dev = &psmouse->ps2dev; @@ -421,6 +435,8 @@ int
> trackpoint_detect(struct psmouse *psmouse, bool set_properties)
> psmouse->reconnect = trackpoint_reconnect;
> psmouse->disconnect = trackpoint_disconnect;
>
> + psmouse->cleanup = trackpoint_cleanup;
> +
> if (variant_id != TP_VARIANT_IBM) {
> /* Newer variants do not support extended button query. */
> button_info = 0x33;
> diff --git a/drivers/input/mouse/trackpoint.h
> b/drivers/input/mouse/trackpoint.h
> index 5cb93ed26085..c7fa75452976 100644
> --- a/drivers/input/mouse/trackpoint.h
> +++ b/drivers/input/mouse/trackpoint.h
> @@ -107,6 +107,7 @@
> #define TP_TOGGLE_EXT_TAG 0x22 /* Bit 3 of the first packet coming
from the
> external device will be forced to
1 */
> #define TP_MASK_EXT_TAG 0x04
> +#define TP_TOGGLE_ELAN_SLEEP 0x8

It seems to me that TP_TOGGLE_ELAN_SLEEP is a bit in TP_TOGGLE_BURST
subcommand, so it is better to move it next to TP_MASK_BURST and call it
TP_MASK_ELAN_SLEEP to match the rest.

Thanks.

--
Dmitry

2022-03-25 10:33:25

by Jingle.Wu

[permalink] [raw]
Subject: RE: [PATCH] Input: trackpoint - Add Suspend mode for Elan TrackPoint

Loop Josh, Dave.

-----Original Message-----
From: Dmitry Torokhov [mailto:[email protected]]
Sent: Thursday, March 24, 2022 1:28 AM
To: jingle.wu <[email protected]>
Cc: [email protected]; [email protected];
[email protected]; jingle.wu <[email protected]>
Subject: Re: [PATCH] Input: trackpoint - Add Suspend mode for Elan
TrackPoint

Hi Jingle,

On Fri, Sep 04, 2020 at 10:42:31AM +0800, jingle.wu wrote:
> From: "jingle.wu" <[email protected]>
>
> Add suspend command for elan trackpoint

Could you please elaborate what this mode does to the trackpint?

Also, your signed-off-by is missing.

> ---
> drivers/input/mouse/trackpoint.c | 16 ++++++++++++++++
> drivers/input/mouse/trackpoint.h | 1 +
> 2 files changed, 17 insertions(+)
>
> diff --git a/drivers/input/mouse/trackpoint.c
> b/drivers/input/mouse/trackpoint.c
> index 3eefee2ee2a1..b3cba973a4eb 100644
> --- a/drivers/input/mouse/trackpoint.c
> +++ b/drivers/input/mouse/trackpoint.c
> @@ -389,6 +389,20 @@ static int trackpoint_reconnect(struct psmouse
*psmouse)
> return 0;
> }
>
> +static void trackpoint_cleanup(struct psmouse *psmouse) {
> + struct trackpoint_data *tp = psmouse->private;
> + u8 param[3] = { TP_TOGGLE, TP_TOGGLE_BURST, TP_TOGGLE_ELAN_SLEEP };
> +
> + if (tp->variant_id == TP_VARIANT_ELAN) {
> + if (ps2_command(&psmouse->ps2dev, param,

I'd combine the 2 "ifs".

> + MAKE_PS2_CMD(3, 0, TP_COMMAND))) {
> + psmouse_err(psmouse,

psmouse_warn() since it is not a hard error.

> + "failed to suspend trackpont.\n");

I'd say "failed to suspend Elan trackpont\n".

> + }
> + }
> +}
> +
> int trackpoint_detect(struct psmouse *psmouse, bool set_properties)
> {
> struct ps2dev *ps2dev = &psmouse->ps2dev; @@ -421,6 +435,8 @@ int
> trackpoint_detect(struct psmouse *psmouse, bool set_properties)
> psmouse->reconnect = trackpoint_reconnect;
> psmouse->disconnect = trackpoint_disconnect;
>
> + psmouse->cleanup = trackpoint_cleanup;
> +
> if (variant_id != TP_VARIANT_IBM) {
> /* Newer variants do not support extended button query. */
> button_info = 0x33;
> diff --git a/drivers/input/mouse/trackpoint.h
> b/drivers/input/mouse/trackpoint.h
> index 5cb93ed26085..c7fa75452976 100644
> --- a/drivers/input/mouse/trackpoint.h
> +++ b/drivers/input/mouse/trackpoint.h
> @@ -107,6 +107,7 @@
> #define TP_TOGGLE_EXT_TAG 0x22 /* Bit 3 of the first packet coming
from the
> external device will be forced to
1 */
> #define TP_MASK_EXT_TAG 0x04
> +#define TP_TOGGLE_ELAN_SLEEP 0x8

It seems to me that TP_TOGGLE_ELAN_SLEEP is a bit in TP_TOGGLE_BURST
subcommand, so it is better to move it next to TP_MASK_BURST and call it
TP_MASK_ELAN_SLEEP to match the rest.

Thanks.

--
Dmitry