| Message ID | 20201207090751.9076-1-jingle.wu@emc.com.tw (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/2] Input: elan_i2c - Add new trackpoint report type 0x5F. | expand |
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.
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.
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.
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);
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(-)