diff mbox

[v1,1/1] Input: elantech - fix absolute mode setting on some Asus touchpads

Message ID 1427318875-12142-2-git-send-email-ulrik.debie-os@e2big.org (mailing list archive)
State New, archived
Headers show

Commit Message

Ulrik De Bie March 25, 2015, 9:27 p.m. UTC
On Asus TP500LN and X750JN, the touchpad absolute mode is reset each
time set_rate is done.

In order to fix this, we will verify the firmware version, and if it
matches the one in those laptops, the set_rate function is overloaded
with a function elantech_set_rate_restore_reg_07 that perfoms the
set_rate with the original function, followed by a restore of reg_07
(the register that sets the absolute mode on elantech v4 hardware).

Also the asus TP500LN and X750JN firmware version, capabilities, and
button constellation is added to elantech.c

Reported-and-tested-by: George Moutsopoulos <gmoutso@yahoo.co.uk>
Signed-off-by: Ulrik De Bie <ulrik.debie-os@e2big.org>
---
 drivers/input/mouse/elantech.c | 22 ++++++++++++++++++++++
 drivers/input/mouse/elantech.h |  1 +
 2 files changed, 23 insertions(+)

Comments

Dmitry Torokhov March 25, 2015, 10:52 p.m. UTC | #1
Hi Ulrik,

On Wed, Mar 25, 2015 at 10:27:55PM +0100, Ulrik De Bie wrote:
> On Asus TP500LN and X750JN, the touchpad absolute mode is reset each
> time set_rate is done.
> 
> In order to fix this, we will verify the firmware version, and if it
> matches the one in those laptops, the set_rate function is overloaded
> with a function elantech_set_rate_restore_reg_07 that perfoms the
> set_rate with the original function, followed by a restore of reg_07
> (the register that sets the absolute mode on elantech v4 hardware).
> 
> Also the asus TP500LN and X750JN firmware version, capabilities, and
> button constellation is added to elantech.c
> 
> Reported-and-tested-by: George Moutsopoulos <gmoutso@yahoo.co.uk>
> Signed-off-by: Ulrik De Bie <ulrik.debie-os@e2big.org>
> ---
>  drivers/input/mouse/elantech.c | 22 ++++++++++++++++++++++
>  drivers/input/mouse/elantech.h |  1 +
>  2 files changed, 23 insertions(+)
> 
> diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
> index 6e22682..991dc6b 100644
> --- a/drivers/input/mouse/elantech.c
> +++ b/drivers/input/mouse/elantech.c
> @@ -893,6 +893,21 @@ static psmouse_ret_t elantech_process_byte(struct psmouse *psmouse)
>  }
>  
>  /*
> + * This writes the reg_07 value again to the hardware at the end of every
> + * set_rate call because the register loses its value. reg_07 allows setting
> + * absolute mode on v4 hardware
> + */
> +static void elantech_set_rate_restore_reg_07(struct psmouse *psmouse,
> +		unsigned int rate)
> +{
> +	struct elantech_data *etd = psmouse->private;
> +
> +	etd->original_set_rate(psmouse, rate);
> +	if (elantech_write_reg(psmouse, 0x07, etd->reg_07))
> +		psmouse_err(psmouse, "restoring reg_07 failed\n");

I wonder if set rate is useful for Elantech and if it would not be
simpler to just have an empty stub for set_rate, maybe even for all
Elantech devices?

Thanks.
Ulrik De Bie March 26, 2015, 5:37 p.m. UTC | #2
On Wed, Mar 25, 2015 at 03:52:22PM -0700, Dmitry Torokhov wrote:
> Hi Ulrik,
> 
> On Wed, Mar 25, 2015 at 10:27:55PM +0100, Ulrik De Bie wrote:
> > On Asus TP500LN and X750JN, the touchpad absolute mode is reset each
> > time set_rate is done.
> > 
> > In order to fix this, we will verify the firmware version, and if it
> > matches the one in those laptops, the set_rate function is overloaded
> > with a function elantech_set_rate_restore_reg_07 that perfoms the
> > set_rate with the original function, followed by a restore of reg_07
> > (the register that sets the absolute mode on elantech v4 hardware).
> > 
> > Also the asus TP500LN and X750JN firmware version, capabilities, and
> > button constellation is added to elantech.c
> > 
> > Reported-and-tested-by: George Moutsopoulos <gmoutso@yahoo.co.uk>
> > Signed-off-by: Ulrik De Bie <ulrik.debie-os@e2big.org>
> > ---
> >  drivers/input/mouse/elantech.c | 22 ++++++++++++++++++++++
> >  drivers/input/mouse/elantech.h |  1 +
> >  2 files changed, 23 insertions(+)
> > 
> > diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
> > index 6e22682..991dc6b 100644
> > --- a/drivers/input/mouse/elantech.c
> > +++ b/drivers/input/mouse/elantech.c
> > @@ -893,6 +893,21 @@ static psmouse_ret_t elantech_process_byte(struct psmouse *psmouse)
> >  }
> >  
> >  /*
> > + * This writes the reg_07 value again to the hardware at the end of every
> > + * set_rate call because the register loses its value. reg_07 allows setting
> > + * absolute mode on v4 hardware
> > + */
> > +static void elantech_set_rate_restore_reg_07(struct psmouse *psmouse,
> > +		unsigned int rate)
> > +{
> > +	struct elantech_data *etd = psmouse->private;
> > +
> > +	etd->original_set_rate(psmouse, rate);
> > +	if (elantech_write_reg(psmouse, 0x07, etd->reg_07))
> > +		psmouse_err(psmouse, "restoring reg_07 failed\n");
> 
> I wonder if set rate is useful for Elantech and if it would not be
> simpler to just have an empty stub for set_rate, maybe even for all
> Elantech devices?

1) all Elantech devices ?
I've considered it for all elantech devices, but I've no idea how this
might effect all elantech devices out there.
I did a simple test on my work laptop with elantech. Setting the rate to
a value lower than 10 does disable the mouse, setting it to a value higher
than 10 enables it again. I would not describe that as 'useful' but
not performing a set_rate 100 could give issues on touchpads that do work now.
=> I think best not all elantech devices because of backwards compatibility
issues

2) selectivity of Elantech device
In this patch I used the firmware version, which will probably match with
all (and no more than the ) elantech chips out there that have this issue.
I could go for dmi table code, but then we will probably have to extend this 
for a couple of years with more and more rows in the table.
=> preference for firmare version above dmi

3) empty stub instead of set_rate+fixup
Could work, but needs verification because currently set_rate is called with
100 and I don't know what the default is. Of course, ideally the sysfs would
then report back that the rate reading/writing is not supported, or the
rate sysfs file would not exist at all.
=> small preference for actually performing the set_rate




Kind regards,
Ulrik

--
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
Dmitry Torokhov April 6, 2015, 10:38 p.m. UTC | #3
On Thu, Mar 26, 2015 at 06:37:06PM +0100, ulrik.debie-os@e2big.org wrote:
> On Wed, Mar 25, 2015 at 03:52:22PM -0700, Dmitry Torokhov wrote:
> > Hi Ulrik,
> > 
> > On Wed, Mar 25, 2015 at 10:27:55PM +0100, Ulrik De Bie wrote:
> > > On Asus TP500LN and X750JN, the touchpad absolute mode is reset each
> > > time set_rate is done.
> > > 
> > > In order to fix this, we will verify the firmware version, and if it
> > > matches the one in those laptops, the set_rate function is overloaded
> > > with a function elantech_set_rate_restore_reg_07 that perfoms the
> > > set_rate with the original function, followed by a restore of reg_07
> > > (the register that sets the absolute mode on elantech v4 hardware).
> > > 
> > > Also the asus TP500LN and X750JN firmware version, capabilities, and
> > > button constellation is added to elantech.c
> > > 
> > > Reported-and-tested-by: George Moutsopoulos <gmoutso@yahoo.co.uk>
> > > Signed-off-by: Ulrik De Bie <ulrik.debie-os@e2big.org>
> > > ---
> > >  drivers/input/mouse/elantech.c | 22 ++++++++++++++++++++++
> > >  drivers/input/mouse/elantech.h |  1 +
> > >  2 files changed, 23 insertions(+)
> > > 
> > > diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
> > > index 6e22682..991dc6b 100644
> > > --- a/drivers/input/mouse/elantech.c
> > > +++ b/drivers/input/mouse/elantech.c
> > > @@ -893,6 +893,21 @@ static psmouse_ret_t elantech_process_byte(struct psmouse *psmouse)
> > >  }
> > >  
> > >  /*
> > > + * This writes the reg_07 value again to the hardware at the end of every
> > > + * set_rate call because the register loses its value. reg_07 allows setting
> > > + * absolute mode on v4 hardware
> > > + */
> > > +static void elantech_set_rate_restore_reg_07(struct psmouse *psmouse,
> > > +		unsigned int rate)
> > > +{
> > > +	struct elantech_data *etd = psmouse->private;
> > > +
> > > +	etd->original_set_rate(psmouse, rate);
> > > +	if (elantech_write_reg(psmouse, 0x07, etd->reg_07))
> > > +		psmouse_err(psmouse, "restoring reg_07 failed\n");
> > 
> > I wonder if set rate is useful for Elantech and if it would not be
> > simpler to just have an empty stub for set_rate, maybe even for all
> > Elantech devices?
> 
> 1) all Elantech devices ?
> I've considered it for all elantech devices, but I've no idea how this
> might effect all elantech devices out there.
> I did a simple test on my work laptop with elantech. Setting the rate to
> a value lower than 10 does disable the mouse, setting it to a value higher
> than 10 enables it again. I would not describe that as 'useful' but
> not performing a set_rate 100 could give issues on touchpads that do work now.
> => I think best not all elantech devices because of backwards compatibility
> issues
> 
> 2) selectivity of Elantech device
> In this patch I used the firmware version, which will probably match with
> all (and no more than the ) elantech chips out there that have this issue.
> I could go for dmi table code, but then we will probably have to extend this 
> for a couple of years with more and more rows in the table.
> => preference for firmare version above dmi
> 
> 3) empty stub instead of set_rate+fixup
> Could work, but needs verification because currently set_rate is called with
> 100 and I don't know what the default is. Of course, ideally the sysfs would
> then report back that the rate reading/writing is not supported, or the
> rate sysfs file would not exist at all.
> => small preference for actually performing the set_rate

OK, fair enough, applied, thank you.
diff mbox

Patch

diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index 6e22682..991dc6b 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -893,6 +893,21 @@  static psmouse_ret_t elantech_process_byte(struct psmouse *psmouse)
 }
 
 /*
+ * This writes the reg_07 value again to the hardware at the end of every
+ * set_rate call because the register loses its value. reg_07 allows setting
+ * absolute mode on v4 hardware
+ */
+static void elantech_set_rate_restore_reg_07(struct psmouse *psmouse,
+		unsigned int rate)
+{
+	struct elantech_data *etd = psmouse->private;
+
+	etd->original_set_rate(psmouse, rate);
+	if (elantech_write_reg(psmouse, 0x07, etd->reg_07))
+		psmouse_err(psmouse, "restoring reg_07 failed\n");
+}
+
+/*
  * Put the touchpad into absolute mode
  */
 static int elantech_set_absolute_mode(struct psmouse *psmouse)
@@ -1094,6 +1109,8 @@  static int elantech_get_resolution_v4(struct psmouse *psmouse,
  * Asus K53SV              0x450f01        78, 15, 0c      2 hw buttons
  * Asus G46VW              0x460f02        00, 18, 0c      2 hw buttons
  * Asus G750JX             0x360f00        00, 16, 0c      2 hw buttons
+ * Asus TP500LN            0x381f17        10, 14, 0e      clickpad
+ * Asus X750JN             0x381f17        10, 14, 0e      clickpad
  * Asus UX31               0x361f00        20, 15, 0e      clickpad
  * Asus UX32VD             0x361f02        00, 15, 0e      clickpad
  * Avatar AVIU-145A2       0x361f00        ?               clickpad
@@ -1635,6 +1652,11 @@  int elantech_init(struct psmouse *psmouse)
 		goto init_fail;
 	}
 
+	if (etd->fw_version == 0x381f17) {
+		etd->original_set_rate = psmouse->set_rate;
+		psmouse->set_rate = elantech_set_rate_restore_reg_07;
+	}
+
 	if (elantech_set_input_params(psmouse)) {
 		psmouse_err(psmouse, "failed to query touchpad range.\n");
 		goto init_fail;
diff --git a/drivers/input/mouse/elantech.h b/drivers/input/mouse/elantech.h
index 6f3afec..f965d15 100644
--- a/drivers/input/mouse/elantech.h
+++ b/drivers/input/mouse/elantech.h
@@ -142,6 +142,7 @@  struct elantech_data {
 	struct finger_pos mt[ETP_MAX_FINGERS];
 	unsigned char parity[256];
 	int (*send_cmd)(struct psmouse *psmouse, unsigned char c, unsigned char *param);
+	void (*original_set_rate)(struct psmouse *psmouse, unsigned int rate);
 };
 
 #ifdef CONFIG_MOUSE_PS2_ELANTECH