diff mbox series

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

Message ID 20201207090751.9076-1-jingle.wu@emc.com.tw
State Superseded
Headers show
Series [1/2] Input: elan_i2c - Add new trackpoint report type 0x5F. | expand

Commit Message

Jingle.Wu Dec. 7, 2020, 9:07 a.m. UTC
The 0x5F is new trackpoint report type of some module.

Signed-off-by: Jingle Wu <jingle.wu@emc.com.tw>
---
 drivers/input/mouse/elan_i2c_core.c  | 2 ++
 drivers/input/mouse/elan_i2c_smbus.c | 3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)

Comments

Dmitry Torokhov Dec. 10, 2020, 6:14 a.m. UTC | #1
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 <jingle.wu@emc.com.tw>

> ---

>  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
Jingle.Wu Dec. 11, 2020, 2:38 a.m. UTC | #2
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:dmitry.torokhov@gmail.com] 

Sent: Thursday, December 10, 2020 2:14 PM
To: jingle.wu
Cc: linux-kernel@vger.kernel.org; linux-input@vger.kernel.org;
phoenix@emc.com.tw; josh.chen@emc.com.tw; dave.wang@emc.com.tw
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 <jingle.wu@emc.com.tw>

> ---

>  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
Dmitry Torokhov Dec. 11, 2020, 3:04 a.m. UTC | #3
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
diff mbox series

Patch

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);