2020-12-07 09:13:44

by Jingle.Wu

[permalink] [raw]
Subject: [PATCH 1/2] Input: elan_i2c - Add new trackpoint report type 0x5F.

The 0x5F is new trackpoint report type of some module.

Signed-off-by: Jingle Wu <[email protected]>
---
drivers/input/mouse/elan_i2c_core.c | 2 ++
drivers/input/mouse/elan_i2c_smbus.c | 3 ++-
2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
index 61ed3f5ca219..8f0c4663167c 100644
--- a/drivers/input/mouse/elan_i2c_core.c
+++ b/drivers/input/mouse/elan_i2c_core.c
@@ -52,6 +52,7 @@
#define ETP_REPORT_ID 0x5D
#define ETP_REPORT_ID2 0x60 /* High precision report */
#define ETP_TP_REPORT_ID 0x5E
+#define ETP_TP_REPORT_ID2 0x5F
#define ETP_REPORT_ID_OFFSET 2
#define ETP_TOUCH_INFO_OFFSET 3
#define ETP_FINGER_DATA_OFFSET 4
@@ -1076,6 +1077,7 @@ static irqreturn_t elan_isr(int irq, void *dev_id)
elan_report_absolute(data, report, true);
break;
case ETP_TP_REPORT_ID:
+ case ETP_TP_REPORT_ID2:
elan_report_trackpoint(data, report);
break;
default:
diff --git a/drivers/input/mouse/elan_i2c_smbus.c b/drivers/input/mouse/elan_i2c_smbus.c
index 1820f1cfc1dc..1226d47ec3cf 100644
--- a/drivers/input/mouse/elan_i2c_smbus.c
+++ b/drivers/input/mouse/elan_i2c_smbus.c
@@ -45,6 +45,7 @@
#define ETP_SMBUS_CALIBRATE_QUERY 0xC5

#define ETP_SMBUS_REPORT_LEN 32
+#define ETP_SMBUS_REPORT_LEN2 7
#define ETP_SMBUS_REPORT_OFFSET 2
#define ETP_SMBUS_HELLOPACKET_LEN 5
#define ETP_SMBUS_IAP_PASSWORD 0x1234
@@ -497,7 +498,7 @@ static int elan_smbus_get_report(struct i2c_client *client,
return len;
}

- if (len != ETP_SMBUS_REPORT_LEN) {
+ if ((len != ETP_SMBUS_REPORT_LEN) && (len != ETP_SMBUS_REPORT_LEN2)) {
dev_err(&client->dev,
"wrong report length (%d vs %d expected)\n",
len, ETP_SMBUS_REPORT_LEN);
--
2.17.1


2020-12-10 07:27:38

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH 1/2] Input: elan_i2c - Add new trackpoint report type 0x5F.

Hi Jingle,

On Mon, Dec 07, 2020 at 05:07:51PM +0800, jingle.wu wrote:
> The 0x5F is new trackpoint report type of some module.
>
> Signed-off-by: Jingle Wu <[email protected]>
> ---
> drivers/input/mouse/elan_i2c_core.c | 2 ++
> drivers/input/mouse/elan_i2c_smbus.c | 3 ++-
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
> index 61ed3f5ca219..8f0c4663167c 100644
> --- a/drivers/input/mouse/elan_i2c_core.c
> +++ b/drivers/input/mouse/elan_i2c_core.c
> @@ -52,6 +52,7 @@
> #define ETP_REPORT_ID 0x5D
> #define ETP_REPORT_ID2 0x60 /* High precision report */
> #define ETP_TP_REPORT_ID 0x5E
> +#define ETP_TP_REPORT_ID2 0x5F
> #define ETP_REPORT_ID_OFFSET 2
> #define ETP_TOUCH_INFO_OFFSET 3
> #define ETP_FINGER_DATA_OFFSET 4

I think we might need to move this into elan_i2c.h so that we can
reference it from elan_i2c_smbus.c.

> @@ -1076,6 +1077,7 @@ static irqreturn_t elan_isr(int irq, void *dev_id)
> elan_report_absolute(data, report, true);
> break;
> case ETP_TP_REPORT_ID:
> + case ETP_TP_REPORT_ID2:
> elan_report_trackpoint(data, report);
> break;
> default:
> diff --git a/drivers/input/mouse/elan_i2c_smbus.c b/drivers/input/mouse/elan_i2c_smbus.c
> index 1820f1cfc1dc..1226d47ec3cf 100644
> --- a/drivers/input/mouse/elan_i2c_smbus.c
> +++ b/drivers/input/mouse/elan_i2c_smbus.c
> @@ -45,6 +45,7 @@
> #define ETP_SMBUS_CALIBRATE_QUERY 0xC5
>
> #define ETP_SMBUS_REPORT_LEN 32
> +#define ETP_SMBUS_REPORT_LEN2 7
> #define ETP_SMBUS_REPORT_OFFSET 2
> #define ETP_SMBUS_HELLOPACKET_LEN 5
> #define ETP_SMBUS_IAP_PASSWORD 0x1234
> @@ -497,7 +498,7 @@ static int elan_smbus_get_report(struct i2c_client *client,
> return len;
> }
>
> - if (len != ETP_SMBUS_REPORT_LEN) {
> + if ((len != ETP_SMBUS_REPORT_LEN) && (len != ETP_SMBUS_REPORT_LEN2)) {

I would prefer if we validated report length versus the packet type
before accepting it.

> dev_err(&client->dev,
> "wrong report length (%d vs %d expected)\n",
> len, ETP_SMBUS_REPORT_LEN);
> --
> 2.17.1
>

Thanks.

--
Dmitry

2020-12-11 11:37:19

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH 1/2] Input: elan_i2c - Add new trackpoint report type 0x5F.

Hi Jingle,

On Fri, Dec 11, 2020 at 10:38:22AM +0800, jingle wrote:
> HI Dmitry:

Please do not top post on the kernel mailing lists.

>
> I would prefer if we validated report length versus the packet type before
> accepting it.
>
> -> If the tracking point report is 0x5F, the report length is 7, but the
> touchpad report length is 32.
> -> So, report length will be different with this module.

Right, but we could check the report type when we receive the data and
refuse it if length does not match what is expected for the report type
received. This can happen before we pass the data on to the
elan_i2c_core.

Thanks.

--
Dmitry

2020-12-12 00:48:56

by Jingle.Wu

[permalink] [raw]
Subject: RE: [PATCH 1/2] Input: elan_i2c - Add new trackpoint report type 0x5F.

HI Dmitry:

I would prefer if we validated report length versus the packet type before
accepting it.

-> If the tracking point report is 0x5F, the report length is 7, but the
touchpad report length is 32.
-> So, report length will be different with this module.

THANKS
JINGLE


-----Original Message-----
From: Dmitry Torokhov [mailto:[email protected]]
Sent: Thursday, December 10, 2020 2:14 PM
To: jingle.wu
Cc: [email protected]; [email protected];
[email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 1/2] Input: elan_i2c - Add new trackpoint report type
0x5F.

Hi Jingle,

On Mon, Dec 07, 2020 at 05:07:51PM +0800, jingle.wu wrote:
> The 0x5F is new trackpoint report type of some module.
>
> Signed-off-by: Jingle Wu <[email protected]>
> ---
> drivers/input/mouse/elan_i2c_core.c | 2 ++
> drivers/input/mouse/elan_i2c_smbus.c | 3 ++-
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/input/mouse/elan_i2c_core.c
> b/drivers/input/mouse/elan_i2c_core.c
> index 61ed3f5ca219..8f0c4663167c 100644
> --- a/drivers/input/mouse/elan_i2c_core.c
> +++ b/drivers/input/mouse/elan_i2c_core.c
> @@ -52,6 +52,7 @@
> #define ETP_REPORT_ID 0x5D
> #define ETP_REPORT_ID2 0x60 /* High precision report */
> #define ETP_TP_REPORT_ID 0x5E
> +#define ETP_TP_REPORT_ID2 0x5F
> #define ETP_REPORT_ID_OFFSET 2
> #define ETP_TOUCH_INFO_OFFSET 3
> #define ETP_FINGER_DATA_OFFSET 4

I think we might need to move this into elan_i2c.h so that we can reference
it from elan_i2c_smbus.c.

> @@ -1076,6 +1077,7 @@ static irqreturn_t elan_isr(int irq, void *dev_id)
> elan_report_absolute(data, report, true);
> break;
> case ETP_TP_REPORT_ID:
> + case ETP_TP_REPORT_ID2:
> elan_report_trackpoint(data, report);
> break;
> default:
> diff --git a/drivers/input/mouse/elan_i2c_smbus.c
> b/drivers/input/mouse/elan_i2c_smbus.c
> index 1820f1cfc1dc..1226d47ec3cf 100644
> --- a/drivers/input/mouse/elan_i2c_smbus.c
> +++ b/drivers/input/mouse/elan_i2c_smbus.c
> @@ -45,6 +45,7 @@
> #define ETP_SMBUS_CALIBRATE_QUERY 0xC5
>
> #define ETP_SMBUS_REPORT_LEN 32
> +#define ETP_SMBUS_REPORT_LEN2 7
> #define ETP_SMBUS_REPORT_OFFSET 2
> #define ETP_SMBUS_HELLOPACKET_LEN 5
> #define ETP_SMBUS_IAP_PASSWORD 0x1234
> @@ -497,7 +498,7 @@ static int elan_smbus_get_report(struct i2c_client
*client,
> return len;
> }
>
> - if (len != ETP_SMBUS_REPORT_LEN) {
> + if ((len != ETP_SMBUS_REPORT_LEN) && (len != ETP_SMBUS_REPORT_LEN2))

> +{

I would prefer if we validated report length versus the packet type before
accepting it.

> dev_err(&client->dev,
> "wrong report length (%d vs %d expected)\n",
> len, ETP_SMBUS_REPORT_LEN);
> --
> 2.17.1
>

Thanks.

--
Dmitry