diff mbox series

[v2] Input: synaptics - enable InterTouch for ThinkPad X1E/P1 2nd gen

Message ID 20200930112437.13705-1-Jason@zx2c4.com (mailing list archive)
State New, archived
Headers show
Series [v2] Input: synaptics - enable InterTouch for ThinkPad X1E/P1 2nd gen | expand

Commit Message

Jason A. Donenfeld Sept. 30, 2020, 11:24 a.m. UTC
With the new RMI4 F3A support posted yesterday, this appears to maybe
work, but requires us to add support for the newer bootloader, which
this commit does.

Cc: Lyude Paul <lyude@redhat.com>
Cc: Vincent Huang <vincent.huang@tw.synaptics.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 drivers/input/mouse/synaptics.c | 1 +
 drivers/input/rmi4/rmi_f34v7.c  | 7 +++++--
 2 files changed, 6 insertions(+), 2 deletions(-)

Comments

Lyude Paul Sept. 30, 2020, 4:05 p.m. UTC | #1
Maybe correct the comment in smbus_pnp_ids to reflect this handles both the X1
Extreme and P2 2nd Gen. Then I'd probably split the bootloader change into a
commit that comes before adding the new PnP IDs.

Otherwise though:

Acked-by: Lyude Paul <lyude@redhat.com>

Let's see what the folks from synaptics say

On Wed, 2020-09-30 at 13:24 +0200, Jason A. Donenfeld wrote:
> With the new RMI4 F3A support posted yesterday, this appears to maybe
> work, but requires us to add support for the newer bootloader, which
> this commit does.
> 
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: Vincent Huang <vincent.huang@tw.synaptics.com>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
>  drivers/input/mouse/synaptics.c | 1 +
>  drivers/input/rmi4/rmi_f34v7.c  | 7 +++++--
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/input/mouse/synaptics.c
> b/drivers/input/mouse/synaptics.c
> index 8a54efd6eb95..9d6fec84047b 100644
> --- a/drivers/input/mouse/synaptics.c
> +++ b/drivers/input/mouse/synaptics.c
> @@ -180,6 +180,7 @@ static const char * const smbus_pnp_ids[] = {
>  	"LEN0096", /* X280 */
>  	"LEN0097", /* X280 -> ALPS trackpoint */
>  	"LEN0099", /* X1 Extreme 1st */
> +	"LEN0402", /* X1 Extreme 2nd */
>  	"LEN009b", /* T580 */
>  	"LEN200f", /* T450s */
>  	"LEN2044", /* L470  */
> diff --git a/drivers/input/rmi4/rmi_f34v7.c b/drivers/input/rmi4/rmi_f34v7.c
> index 74f7c6f214ff..8cfaa2f19ed5 100644
> --- a/drivers/input/rmi4/rmi_f34v7.c
> +++ b/drivers/input/rmi4/rmi_f34v7.c
> @@ -1364,9 +1364,12 @@ int rmi_f34v7_probe(struct f34_data *f34)
>  		f34->bl_version = 6;
>  	} else if (f34->bootloader_id[1] == 7) {
>  		f34->bl_version = 7;
> +	} else if (f34->bootloader_id[1] == 8) {
> +		f34->bl_version = 8;
>  	} else {
> -		dev_err(&f34->fn->dev, "%s: Unrecognized bootloader
> version\n",
> -				__func__);
> +		dev_err(&f34->fn->dev, "%s: Unrecognized bootloader version:
> %d (%c) %d (%c)\n",
> +				__func__, f34->bootloader_id[0], f34-
> >bootloader_id[0],
> +				f34->bootloader_id[1], f34->bootloader_id[1]);
>  		return -EINVAL;
>  	}
>
Jason A. Donenfeld Sept. 30, 2020, 4:18 p.m. UTC | #2
On Wed, Sep 30, 2020 at 6:05 PM Lyude Paul <lyude@redhat.com> wrote:
>
> Maybe correct the comment in smbus_pnp_ids to reflect this handles both the X1
> Extreme and P2 2nd Gen. Then I'd probably split the bootloader change into a
> commit that comes before adding the new PnP IDs.

Okay, I'll submit a v3.

One thing I should note is that the sensitivity sysfs entry doesn't
seem to do anything at all. push_to_click works, but not sensitivity.
I don't know if this has bitrotted over the years and I shouldn't
expect it to work, as it rarely does or something, but thought I
should mention this.

Jason

>
> Otherwise though:
>
> Acked-by: Lyude Paul <lyude@redhat.com>
>
> Let's see what the folks from synaptics say
>
> On Wed, 2020-09-30 at 13:24 +0200, Jason A. Donenfeld wrote:
> > With the new RMI4 F3A support posted yesterday, this appears to maybe
> > work, but requires us to add support for the newer bootloader, which
> > this commit does.
> >
> > Cc: Lyude Paul <lyude@redhat.com>
> > Cc: Vincent Huang <vincent.huang@tw.synaptics.com>
> > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> > ---
> >  drivers/input/mouse/synaptics.c | 1 +
> >  drivers/input/rmi4/rmi_f34v7.c  | 7 +++++--
> >  2 files changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/input/mouse/synaptics.c
> > b/drivers/input/mouse/synaptics.c
> > index 8a54efd6eb95..9d6fec84047b 100644
> > --- a/drivers/input/mouse/synaptics.c
> > +++ b/drivers/input/mouse/synaptics.c
> > @@ -180,6 +180,7 @@ static const char * const smbus_pnp_ids[] = {
> >       "LEN0096", /* X280 */
> >       "LEN0097", /* X280 -> ALPS trackpoint */
> >       "LEN0099", /* X1 Extreme 1st */
> > +     "LEN0402", /* X1 Extreme 2nd */
> >       "LEN009b", /* T580 */
> >       "LEN200f", /* T450s */
> >       "LEN2044", /* L470  */
> > diff --git a/drivers/input/rmi4/rmi_f34v7.c b/drivers/input/rmi4/rmi_f34v7.c
> > index 74f7c6f214ff..8cfaa2f19ed5 100644
> > --- a/drivers/input/rmi4/rmi_f34v7.c
> > +++ b/drivers/input/rmi4/rmi_f34v7.c
> > @@ -1364,9 +1364,12 @@ int rmi_f34v7_probe(struct f34_data *f34)
> >               f34->bl_version = 6;
> >       } else if (f34->bootloader_id[1] == 7) {
> >               f34->bl_version = 7;
> > +     } else if (f34->bootloader_id[1] == 8) {
> > +             f34->bl_version = 8;
> >       } else {
> > -             dev_err(&f34->fn->dev, "%s: Unrecognized bootloader
> > version\n",
> > -                             __func__);
> > +             dev_err(&f34->fn->dev, "%s: Unrecognized bootloader version:
> > %d (%c) %d (%c)\n",
> > +                             __func__, f34->bootloader_id[0], f34-
> > >bootloader_id[0],
> > +                             f34->bootloader_id[1], f34->bootloader_id[1]);
> >               return -EINVAL;
> >       }
> >
> --
> Cheers,
>         Lyude Paul (she/her)
>         Software Engineer at Red Hat
Lyude Paul Sept. 30, 2020, 7:55 p.m. UTC | #3
On Wed, 2020-09-30 at 18:18 +0200, Jason A. Donenfeld wrote:
> On Wed, Sep 30, 2020 at 6:05 PM Lyude Paul <lyude@redhat.com> wrote:
> > Maybe correct the comment in smbus_pnp_ids to reflect this handles both the
> > X1
> > Extreme and P2 2nd Gen. Then I'd probably split the bootloader change into a
> > commit that comes before adding the new PnP IDs.
> 
> Okay, I'll submit a v3.
> 
> One thing I should note is that the sensitivity sysfs entry doesn't
> seem to do anything at all. push_to_click works, but not sensitivity.
> I don't know if this has bitrotted over the years and I shouldn't
> expect it to work, as it rarely does or something, but thought I
> should mention this.

Interesting-it's entirely possible that maybe the firmware on this trackpoint is
different from the previous ones (only mention this possibility since it looks
like at some point in time they switched over from using the legitimate IBM
trackpoint modules to getting other manufacturers to make them). I know the
sensitivity setting works on my laptop with PS/2 through RMI4 though.

Could you maybe enable rmi4 debugging by passing rmi_core.debug_flags=0xff when
you boot your machine and get me the dmesg output from that after you've tried
changing the sensitivity value? Not sure I could fix it, but it'd be interesting
to see what's happening on the ps/2 side here
> 
> Jason
> 
> > Otherwise though:
> > 
> > Acked-by: Lyude Paul <lyude@redhat.com>
> > 
> > Let's see what the folks from synaptics say
> > 
> > On Wed, 2020-09-30 at 13:24 +0200, Jason A. Donenfeld wrote:
> > > With the new RMI4 F3A support posted yesterday, this appears to maybe
> > > work, but requires us to add support for the newer bootloader, which
> > > this commit does.
> > > 
> > > Cc: Lyude Paul <lyude@redhat.com>
> > > Cc: Vincent Huang <vincent.huang@tw.synaptics.com>
> > > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> > > ---
> > >  drivers/input/mouse/synaptics.c | 1 +
> > >  drivers/input/rmi4/rmi_f34v7.c  | 7 +++++--
> > >  2 files changed, 6 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/input/mouse/synaptics.c
> > > b/drivers/input/mouse/synaptics.c
> > > index 8a54efd6eb95..9d6fec84047b 100644
> > > --- a/drivers/input/mouse/synaptics.c
> > > +++ b/drivers/input/mouse/synaptics.c
> > > @@ -180,6 +180,7 @@ static const char * const smbus_pnp_ids[] = {
> > >       "LEN0096", /* X280 */
> > >       "LEN0097", /* X280 -> ALPS trackpoint */
> > >       "LEN0099", /* X1 Extreme 1st */
> > > +     "LEN0402", /* X1 Extreme 2nd */
> > >       "LEN009b", /* T580 */
> > >       "LEN200f", /* T450s */
> > >       "LEN2044", /* L470  */
> > > diff --git a/drivers/input/rmi4/rmi_f34v7.c
> > > b/drivers/input/rmi4/rmi_f34v7.c
> > > index 74f7c6f214ff..8cfaa2f19ed5 100644
> > > --- a/drivers/input/rmi4/rmi_f34v7.c
> > > +++ b/drivers/input/rmi4/rmi_f34v7.c
> > > @@ -1364,9 +1364,12 @@ int rmi_f34v7_probe(struct f34_data *f34)
> > >               f34->bl_version = 6;
> > >       } else if (f34->bootloader_id[1] == 7) {
> > >               f34->bl_version = 7;
> > > +     } else if (f34->bootloader_id[1] == 8) {
> > > +             f34->bl_version = 8;
> > >       } else {
> > > -             dev_err(&f34->fn->dev, "%s: Unrecognized bootloader
> > > version\n",
> > > -                             __func__);
> > > +             dev_err(&f34->fn->dev, "%s: Unrecognized bootloader version:
> > > %d (%c) %d (%c)\n",
> > > +                             __func__, f34->bootloader_id[0], f34-
> > > > bootloader_id[0],
> > > +                             f34->bootloader_id[1], f34-
> > > >bootloader_id[1]);
> > >               return -EINVAL;
> > >       }
> > > 
> > --
> > Cheers,
> >         Lyude Paul (she/her)
> >         Software Engineer at Red Hat
Jason A. Donenfeld Sept. 30, 2020, 10:55 p.m. UTC | #4
On Wed, Sep 30, 2020 at 9:55 PM Lyude Paul <lyude@redhat.com> wrote:
>
> On Wed, 2020-09-30 at 18:18 +0200, Jason A. Donenfeld wrote:
> > On Wed, Sep 30, 2020 at 6:05 PM Lyude Paul <lyude@redhat.com> wrote:
> > > Maybe correct the comment in smbus_pnp_ids to reflect this handles both the
> > > X1
> > > Extreme and P2 2nd Gen. Then I'd probably split the bootloader change into a
> > > commit that comes before adding the new PnP IDs.
> >
> > Okay, I'll submit a v3.
> >
> > One thing I should note is that the sensitivity sysfs entry doesn't
> > seem to do anything at all. push_to_click works, but not sensitivity.
> > I don't know if this has bitrotted over the years and I shouldn't
> > expect it to work, as it rarely does or something, but thought I
> > should mention this.
>
> Interesting-it's entirely possible that maybe the firmware on this trackpoint is
> different from the previous ones (only mention this possibility since it looks
> like at some point in time they switched over from using the legitimate IBM
> trackpoint modules to getting other manufacturers to make them).

Yea, a bummer. The P1 gen 2 has an ELAN. Far cry from all the nice
features supported by the IBM ones.

> I know the
> sensitivity setting works on my laptop with PS/2 through RMI4 though.

Right. My prior P50 worked fine with it.


>
> Could you maybe enable rmi4 debugging by passing rmi_core.debug_flags=0xff when
> you boot your machine and get me the dmesg output from that after you've tried
> changing the sensitivity value? Not sure I could fix it, but it'd be interesting
> to see what's happening on the ps/2 side here

Sure. Writing 200 into sensitivity gives:

[28653.834012] rmi4_f03 rmi4-00.fn03: rmi_f03_pt_write: Wrote f5 to
PS/2 passthrough address
[28653.834989] rmi4_smbus 0-002c: wrote 1 bytes at 0x02: 0 (f5)
[28653.848217] rmi4_f03 rmi4-00.fn03: rmi_f03_attention: Received fa
from PS2 guest T: N P: N
[28653.848301] rmi4_f03 rmi4-00.fn03: rmi_f03_pt_write: Wrote e2 to
PS/2 passthrough address
[28653.849079] rmi4_smbus 0-002c: wrote 1 bytes at 0x02: 0 (e2)
[28653.857787] rmi4_f03 rmi4-00.fn03: rmi_f03_attention: Received fa
from PS2 guest T: N P: N
[28653.857868] rmi4_f03 rmi4-00.fn03: rmi_f03_pt_write: Wrote 81 to
PS/2 passthrough address
[28653.858643] rmi4_smbus 0-002c: wrote 1 bytes at 0x02: 0 (81)
[28653.865220] rmi4_f03 rmi4-00.fn03: rmi_f03_attention: Received fa
from PS2 guest T: N P: N
[28653.865285] rmi4_f03 rmi4-00.fn03: rmi_f03_pt_write: Wrote 4a to
PS/2 passthrough address
[28653.866053] rmi4_smbus 0-002c: wrote 1 bytes at 0x02: 0 (4a)
[28653.872889] rmi4_f03 rmi4-00.fn03: rmi_f03_attention: Received fa
from PS2 guest T: N P: N
[28653.872952] rmi4_f03 rmi4-00.fn03: rmi_f03_pt_write: Wrote c8 to
PS/2 passthrough address
[28653.873927] rmi4_smbus 0-002c: wrote 1 bytes at 0x02: 0 (c8)
[28653.880331] rmi4_f03 rmi4-00.fn03: rmi_f03_attention: Received fa
from PS2 guest T: N P: N
[28653.880397] rmi4_f03 rmi4-00.fn03: rmi_f03_pt_write: Wrote f4 to
PS/2 passthrough address
[28653.881156] rmi4_smbus 0-002c: wrote 1 bytes at 0x02: 0 (f4)
[28653.888285] rmi4_f03 rmi4-00.fn03: rmi_f03_attention: Received fa
from PS2 guest T: N P: N

Subsequently, moving the trackpoint around gives the usual output:

[28765.017676] rmi4_f03 rmi4-00.fn03: rmi_f03_attention: Received 05
from PS2 guest T: N P: N
[28765.017677] rmi4_f03 rmi4-00.fn03: rmi_f03_attention: Received ff
from PS2 guest T: N P: N
[28765.025214] rmi4_f03 rmi4-00.fn03: rmi_f03_attention: Received 28
from PS2 guest T: N P: N
[28765.025216] rmi4_f03 rmi4-00.fn03: rmi_f03_attention: Received 06
from PS2 guest T: N P: N
[28765.025216] rmi4_f03 rmi4-00.fn03: rmi_f03_attention: Received ff
from PS2 guest T: N P: N
[28765.050927] rmi4_f03 rmi4-00.fn03: rmi_f03_attention: Received 28
from PS2 guest T: N P: N
[28765.050929] rmi4_f03 rmi4-00.fn03: rmi_f03_attention: Received 05
from PS2 guest T: N P: N
[28765.050951] rmi4_f03 rmi4-00.fn03: rmi_f03_attention: Received ff
from PS2 guest T: N P: N
[28765.050958] rmi4_f03 rmi4-00.fn03: rmi_f03_attention: Received 08
from PS2 guest T: N P: N
[28765.050959] rmi4_f03 rmi4-00.fn03: rmi_f03_attention: Received 04
from PS2 guest T: N P: N
[28765.050959] rmi4_f03 rmi4-00.fn03: rmi_f03_attention: Received 00
from PS2 guest T: N P: N
[28765.099642] rmi4_f03 rmi4-00.fn03: rmi_f03_attention: Received 18
from PS2 guest T: N P: N
[28765.099644] rmi4_f03 rmi4-00.fn03: rmi_f03_attention: Received ff
from PS2 guest T: N P: N
[28765.099644] rmi4_f03 rmi4-00.fn03: rmi_f03_attention: Received 00
from PS2 guest T: N P: N
[28765.099651] rmi4_f03 rmi4-00.fn03: rmi_f03_attention: Received 18
from PS2 guest T: N P: N
[28765.099652] rmi4_f03 rmi4-00.fn03: rmi_f03_attention: Received ff
from PS2 guest T: N P: N
[28765.099652] rmi4_f03 rmi4-00.fn03: rmi_f03_attention: Received 00
from PS2 guest T: N P: N
[28765.230269] rmi4_f03 rmi4-00.fn03: rmi_f03_attention: Received 08
from PS2 guest T: N P: N
[28765.230276] rmi4_f03 rmi4-00.fn03: rmi_f03_attention: Received 00
from PS2 guest T: N P: N
[28765.230279] rmi4_f03 rmi4-00.fn03: rmi_f03_attention: Received 01
from PS2 guest T: N P: N
[28765.270171] rmi4_f03 rmi4-00.fn03: rmi_f03_attention: Received 08
from PS2 guest T: N P: N
[28765.270178] rmi4_f03 rmi4-00.fn03: rmi_f03_attention: Received 00
from PS2 guest T: N P: N
[28765.270181] rmi4_f03 rmi4-00.fn03: rmi_f03_attention: Received 01
from PS2 guest T: N P: N
diff mbox series

Patch

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 8a54efd6eb95..9d6fec84047b 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -180,6 +180,7 @@  static const char * const smbus_pnp_ids[] = {
 	"LEN0096", /* X280 */
 	"LEN0097", /* X280 -> ALPS trackpoint */
 	"LEN0099", /* X1 Extreme 1st */
+	"LEN0402", /* X1 Extreme 2nd */
 	"LEN009b", /* T580 */
 	"LEN200f", /* T450s */
 	"LEN2044", /* L470  */
diff --git a/drivers/input/rmi4/rmi_f34v7.c b/drivers/input/rmi4/rmi_f34v7.c
index 74f7c6f214ff..8cfaa2f19ed5 100644
--- a/drivers/input/rmi4/rmi_f34v7.c
+++ b/drivers/input/rmi4/rmi_f34v7.c
@@ -1364,9 +1364,12 @@  int rmi_f34v7_probe(struct f34_data *f34)
 		f34->bl_version = 6;
 	} else if (f34->bootloader_id[1] == 7) {
 		f34->bl_version = 7;
+	} else if (f34->bootloader_id[1] == 8) {
+		f34->bl_version = 8;
 	} else {
-		dev_err(&f34->fn->dev, "%s: Unrecognized bootloader version\n",
-				__func__);
+		dev_err(&f34->fn->dev, "%s: Unrecognized bootloader version: %d (%c) %d (%c)\n",
+				__func__, f34->bootloader_id[0], f34->bootloader_id[0],
+				f34->bootloader_id[1], f34->bootloader_id[1]);
 		return -EINVAL;
 	}