Re: [PATCH] Driver for Elantech I2C touchpad

2023-10-09 Thread Vladimir 'phcoder' Serbinenko
Can we get this merged? This impacts usability on laptops using this
touchpad

Le dim. 30 juil. 2023, 22:38, Vladimir 'phcoder' Serbinenko <
phco...@gmail.com> a écrit :

>
> -- Forwarded message -
> De : Vladimir 'phcoder' Serbinenko 
> Date: dim. 30 juil. 2023, 23:35
> Subject: Re: [PATCH] Driver for Elantech I2C touchpad
> To: Taylor R Campbell 
>
>
>
> Thank you for reviewing my patch. Main patch attached, replies inline.
> Separate ihidev fix sent separately
>
> Le dim. 30 juil. 2023, 13:48, Taylor R Campbell  a
> écrit :
>
>> > Date: Sat, 15 Jul 2023 03:48:54 +0200
>> > From: "Vladimir 'phcoder' Serbinenko" 
>> >
>> > I've submitted a similar patch for OpenBSD recently and it got merged.
>> It
>> > adds support for Elantech I2C touchpad used on many laptops. Tested on
>> my
>> > Chromebook Elemi
>>
>> Cool!  This looks great.  I don't have any hardware to test, but I can
>> review the code -- nothing major.  Some higher-level questions first:
>>
>> - Is this device interface substantively different from the ihidev(4)
>>   interface?  I notice it uses the same struct i2c_hid_desc; is that a
>>   red herring?
>>
> I2C descriptor is the same. HID descriptor however on my model just
> describes entire packet as "proprietary format". Almost all the command and
> entire report are very different from ihidev. So much that trying to put
> them together is likely to result in regular breakages on both sides for at
> most 10% reduction in line count.
>
>>
>> - Looks like this is missing a pmf handler.  Does the device require
>>   any action to suspend/resume?
>
> Turn off/on and set absolute mode . Done
>
>>
>>
>>   may need to be synchronized with any other routines that issue i2c
>>   commands, probably with a mutex at interrupt level IPL_NONE.
>>
> Done
>
>>
>>
>> > --- a/sys/dev/i2c/i2c.c
>> > +++ b/sys/dev/i2c/i2c.c
>> > @@ -646,7 +646,7 @@ iic_use_direct_match(const struct i2c_attach_args
>> *ia, const cfdata_t cf,
>> >
>> >   if (ia->ia_ncompat > 0 && ia->ia_compat != NULL) {
>> >   *match_resultp = iic_compatible_match(ia, compats);
>> > - return true;
>> > + return *match_resultp != 0;
>> >   }
>>
>> Why did you make this change?
>>
> Because ihidev attaches to everything due to a bug. I wasn't sure where to
> fix it. Fixed and moved to separate patch
>
>>
>>
>>
>> > +#include 
>> > +#include 
>> > +#include 
>> > +#include 
>> > +#include 
>> > +#include 
>>
>> Sort includes like this:
>>
> Done
>
>>
>>
>> Use __BIT instead of shifts here: __BIT(0), __BIT(1), __BIT(2).
>>
> Done
>
>>
>> > +static int ietp_ioctl(void *dev, u_long cmd, void *data, int flag,
>> > +struct lwp *l);
>>
>> Tiny nit: four-space continuation lines, not tab + 3space.
>>
> Done
>
>>
>>
>>
>> Use C99 designated initializers here: `.enable = ietp_enable', &c.
>>
> Done
>
>>
>> > +ietp_match(device_t parent, cfdata_t match, void *aux)
>> > +{
>> > ...
>> > + if (iic_use_direct_match(ia, match, compat_data, &match_result)) {
>> > + return I2C_MATCH_DIRECT_COMPATIBLE;
>> > + }
>>
>> Why does this return I2C_MATCH_DIRECT_COMPATIBLE rather than
>> match_result?
>>
>> The usual idea of iic_use_direct_match is that it returns true if it
>> has an answer, and match_result is the answer (except that you seem to
>> have patched that logic away, but I'm not clear on why).
>>
> I followed buggy ihidev
>
>>
>> > + return (dpi * 10 /254);
>>
>> Tiny nit: space on both sides of the operator.
>>
> Fixed
>
>>
>> > +ietp_attach(device_t parent, device_t self, void *aux)
>> > +{
>> > ...
>> > + ietp_fetch_descriptor(sc);
>>
>> Check return value here?
>>
> Done
>
>>
>> > + sc->sc_ih = acpi_intr_establish(sc->sc_dev, sc->sc_phandle,
>> IPL_TTY, false,
>> > + ietp_intr, sc,
>> device_xname(sc->sc_dev));
>>
>> Tiny nits:
>> - break line before 80 columns
>> - maybe write `/*mpsafe*/false' for clarity (not your fault, why is
>>   this a boolean

Fwd: [PATCH] Driver for Elantech I2C touchpad

2023-07-30 Thread Vladimir 'phcoder' Serbinenko
-- Forwarded message -
De : Vladimir 'phcoder' Serbinenko 
Date: dim. 30 juil. 2023, 23:35
Subject: Re: [PATCH] Driver for Elantech I2C touchpad
To: Taylor R Campbell 



Thank you for reviewing my patch. Main patch attached, replies inline.
Separate ihidev fix sent separately

Le dim. 30 juil. 2023, 13:48, Taylor R Campbell  a
écrit :

> > Date: Sat, 15 Jul 2023 03:48:54 +0200
> > From: "Vladimir 'phcoder' Serbinenko" 
> >
> > I've submitted a similar patch for OpenBSD recently and it got merged. It
> > adds support for Elantech I2C touchpad used on many laptops. Tested on my
> > Chromebook Elemi
>
> Cool!  This looks great.  I don't have any hardware to test, but I can
> review the code -- nothing major.  Some higher-level questions first:
>
> - Is this device interface substantively different from the ihidev(4)
>   interface?  I notice it uses the same struct i2c_hid_desc; is that a
>   red herring?
>
I2C descriptor is the same. HID descriptor however on my model just
describes entire packet as "proprietary format". Almost all the command and
entire report are very different from ihidev. So much that trying to put
them together is likely to result in regular breakages on both sides for at
most 10% reduction in line count.

>
> - Looks like this is missing a pmf handler.  Does the device require
>   any action to suspend/resume?

Turn off/on and set absolute mode . Done

>
>
>   may need to be synchronized with any other routines that issue i2c
>   commands, probably with a mutex at interrupt level IPL_NONE.
>
Done

>
>
> > --- a/sys/dev/i2c/i2c.c
> > +++ b/sys/dev/i2c/i2c.c
> > @@ -646,7 +646,7 @@ iic_use_direct_match(const struct i2c_attach_args
> *ia, const cfdata_t cf,
> >
> >   if (ia->ia_ncompat > 0 && ia->ia_compat != NULL) {
> >   *match_resultp = iic_compatible_match(ia, compats);
> > - return true;
> > + return *match_resultp != 0;
> >   }
>
> Why did you make this change?
>
Because ihidev attaches to everything due to a bug. I wasn't sure where to
fix it. Fixed and moved to separate patch

>
>
>
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
>
> Sort includes like this:
>
Done

>
>
> Use __BIT instead of shifts here: __BIT(0), __BIT(1), __BIT(2).
>
Done

>
> > +static int ietp_ioctl(void *dev, u_long cmd, void *data, int flag,
> > +struct lwp *l);
>
> Tiny nit: four-space continuation lines, not tab + 3space.
>
Done

>
>
>
> Use C99 designated initializers here: `.enable = ietp_enable', &c.
>
Done

>
> > +ietp_match(device_t parent, cfdata_t match, void *aux)
> > +{
> > ...
> > + if (iic_use_direct_match(ia, match, compat_data, &match_result)) {
> > + return I2C_MATCH_DIRECT_COMPATIBLE;
> > + }
>
> Why does this return I2C_MATCH_DIRECT_COMPATIBLE rather than
> match_result?
>
> The usual idea of iic_use_direct_match is that it returns true if it
> has an answer, and match_result is the answer (except that you seem to
> have patched that logic away, but I'm not clear on why).
>
I followed buggy ihidev

>
> > + return (dpi * 10 /254);
>
> Tiny nit: space on both sides of the operator.
>
Fixed

>
> > +ietp_attach(device_t parent, device_t self, void *aux)
> > +{
> > ...
> > + ietp_fetch_descriptor(sc);
>
> Check return value here?
>
Done

>
> > + sc->sc_ih = acpi_intr_establish(sc->sc_dev, sc->sc_phandle,
> IPL_TTY, false,
> > + ietp_intr, sc,
> device_xname(sc->sc_dev));
>
> Tiny nits:
> - break line before 80 columns
> - maybe write `/*mpsafe*/false' for clarity (not your fault, why is
>   this a boolean and not a named flag?)
> - four-space continuation lines
>
Done

>
> > + printf("%s: failed reading product ID\n",
> device_xname(sc->sc_dev));
>
> Use aprint_error_dev(sc->sc_dev, "failed to read product ID\n")
> instead of printf with device_xname here and everywhere in the attach
> routine for this kind of error message.
>
> However, after attach, use device_printf rather than aprint_*_dev.
>
Dinner

>
> > +ietp_detach(device_t self, int flags)
> > +{
>
> This should start with:
>
> error = config_detach_children(self, flags);
> if (error)
> return error;
>
Done

>
>
>   it's worth a shot!)
>
> > + return (0);
>
> Tiny nit: 

Re: [PATCH] Driver for Elantech I2C touchpad

2023-07-30 Thread Vladimir 'phcoder' Serbinenko
Le dim. 30 juil. 2023, 23:35, Vladimir 'phcoder' Serbinenko <
phco...@gmail.com> a écrit :

>
> Thank you for reviewing my patch. Main patch attached, replies inline.
> Separate ihidev fix sent separately
>
> Le dim. 30 juil. 2023, 13:48, Taylor R Campbell  a
> écrit :
>
>> > Date: Sat, 15 Jul 2023 03:48:54 +0200
>> > From: "Vladimir 'phcoder' Serbinenko" 
>> >
>> > I've submitted a similar patch for OpenBSD recently and it got merged.
>> It
>> > adds support for Elantech I2C touchpad used on many laptops. Tested on
>> my
>> > Chromebook Elemi
>>
>> Cool!  This looks great.  I don't have any hardware to test, but I can
>> review the code -- nothing major.  Some higher-level questions first:
>>
>> - Is this device interface substantively different from the ihidev(4)
>>   interface?  I notice it uses the same struct i2c_hid_desc; is that a
>>   red herring?
>>
> I2C descriptor is the same. HID descriptor however on my model just
> describes entire packet as "proprietary format". Almost all the command and
> entire report are very different from ihidev. So much that trying to put
> them together is likely to result in regular breakages on both sides for at
> most 10% reduction in line count.
>
>>
>> - Looks like this is missing a pmf handler.  Does the device require
>>   any action to suspend/resume?
>
> Turn off/on and set absolute mode . Done
>
>>
>>
>>   may need to be synchronized with any other routines that issue i2c
>>   commands, probably with a mutex at interrupt level IPL_NONE.
>>
> Done
>
>>
>>
>> > --- a/sys/dev/i2c/i2c.c
>> > +++ b/sys/dev/i2c/i2c.c
>> > @@ -646,7 +646,7 @@ iic_use_direct_match(const struct i2c_attach_args
>> *ia, const cfdata_t cf,
>> >
>> >   if (ia->ia_ncompat > 0 && ia->ia_compat != NULL) {
>> >   *match_resultp = iic_compatible_match(ia, compats);
>> > - return true;
>> > + return *match_resultp != 0;
>> >   }
>>
>> Why did you make this change?
>>
> Because ihidev attaches to everything due to a bug. I wasn't sure where to
> fix it. Fixed and moved to separate patch
>
>>
>>
>>
>> > +#include 
>> > +#include 
>> > +#include 
>> > +#include 
>> > +#include 
>> > +#include 
>>
>> Sort includes like this:
>>
> Done
>
>>
>>
>> Use __BIT instead of shifts here: __BIT(0), __BIT(1), __BIT(2).
>>
> Done
>
>>
>> > +static int ietp_ioctl(void *dev, u_long cmd, void *data, int flag,
>> > +struct lwp *l);
>>
>> Tiny nit: four-space continuation lines, not tab + 3space.
>>
> Done
>
>>
>>
>>
>> Use C99 designated initializers here: `.enable = ietp_enable', &c.
>>
> Done
>
>>
>> > +ietp_match(device_t parent, cfdata_t match, void *aux)
>> > +{
>> > ...
>> > + if (iic_use_direct_match(ia, match, compat_data, &match_result)) {
>> > + return I2C_MATCH_DIRECT_COMPATIBLE;
>> > + }
>>
>> Why does this return I2C_MATCH_DIRECT_COMPATIBLE rather than
>> match_result?
>>
>> The usual idea of iic_use_direct_match is that it returns true if it
>> has an answer, and match_result is the answer (except that you seem to
>> have patched that logic away, but I'm not clear on why).
>>
> I followed buggy ihidev
>
>>
>> > + return (dpi * 10 /254);
>>
>> Tiny nit: space on both sides of the operator.
>>
> Fixed
>
>>
>> > +ietp_attach(device_t parent, device_t self, void *aux)
>> > +{
>> > ...
>> > + ietp_fetch_descriptor(sc);
>>
>> Check return value here?
>>
> Done
>
>>
>> > + sc->sc_ih = acpi_intr_establish(sc->sc_dev, sc->sc_phandle,
>> IPL_TTY, false,
>> > + ietp_intr, sc,
>> device_xname(sc->sc_dev));
>>
>> Tiny nits:
>> - break line before 80 columns
>> - maybe write `/*mpsafe*/false' for clarity (not your fault, why is
>>   this a boolean and not a named flag?)
>> - four-space continuation lines
>>
> Done
>
>>
>> > + printf("%s: failed reading product ID\n",
>> device_xname(sc->sc_dev));
>>
>> Use aprint_error_dev(sc->sc_dev, "failed to read product ID\n")
>> instead of printf with device_xname here and everywhere in the attach
>> routine for this kind of error message.
>>
>> However, after attach, use device_printf rather than aprint_*_dev.
>>
> Dinner
>
>>
>> > +ietp_detach(device_t self, int flags)
>> > +{
>>
>> This should start with:
>>
>> error = config_detach_children(self, flags);
>> if (error)
>> return error;
>>
> Done
>
>>
>>
>>   it's worth a shot!)
>>
>> > + return (0);
>>
>> Tiny nit: `return 0', no parentheses.
>>
> Fixed
>
>>
>>
>> The ietp_set_power call should go in ietp_detach (after
>> config_detach_children), not in ietp_activate.  I don't think
>> sc->sc_dying is actually needed; more on that in a bit.  So I think
>> the ietp_activate callback can go away.
>>
> Done
>
>>
>>
>> > +ietp_iic_read_reg(struct ietp_softc *sc, uint16_t reg, size_t len,
>> void *val)
>> > +{
>> > + uint8_t cmd[] = {
>> > + reg & 0xff,
>> > + reg >> 8,
>> > + };
>> > +
>> > + return iic_exec(sc->sc_tag, I2C

Re: [PATCH] Driver for Elantech I2C touchpad

2023-07-30 Thread Taylor R Campbell
> Date: Sat, 15 Jul 2023 03:48:54 +0200
> From: "Vladimir 'phcoder' Serbinenko" 
> 
> I've submitted a similar patch for OpenBSD recently and it got merged. It
> adds support for Elantech I2C touchpad used on many laptops. Tested on my
> Chromebook Elemi

Cool!  This looks great.  I don't have any hardware to test, but I can
review the code -- nothing major.  Some higher-level questions first:

- Is this device interface substantively different from the ihidev(4)
  interface?  I notice it uses the same struct i2c_hid_desc; is that a
  red herring?

- Looks like this is missing a pmf handler.  Does the device require
  any action to suspend/resume?  If so, your attach routine should do:

pmf_device_register(self, ietp_suspend, ietp_resume);

  (Technically pmf_device_register returns a boolean success status,
  but in reality it always returns true, and I plan to eliminate the
  return value at some point soon; doing it this way will reduce the
  churn.)

  And your detach routine should do:

pmf_device_deregister(self);

  The ietp_suspend and ietp_resume functions are guaranteed not to be
  called concurrently with attach or detach.  Other than that, they
  may need to be synchronized with any other routines that issue i2c
  commands, probably with a mutex at interrupt level IPL_NONE.

  You can test suspend/resume of just this driver with drvctl(8): use
  `drvctl -S ietp0' to suspend, `drvctl -Q ietp0' to resume.

  If no action is needed, your attach routine should still do:

pmf_device_register(self, NULL, NULL);

> --- a/sys/dev/i2c/i2c.c
> +++ b/sys/dev/i2c/i2c.c
> @@ -646,7 +646,7 @@ iic_use_direct_match(const struct i2c_attach_args *ia, 
> const cfdata_t cf,
>  
>   if (ia->ia_ncompat > 0 && ia->ia_compat != NULL) {
>   *match_resultp = iic_compatible_match(ia, compats);
> - return true;
> + return *match_resultp != 0;
>   }

Why did you make this change?

I don't think this can be right -- and it needs to be considered
separately from one driver import, since the function is used by
zillions of i2c drivers throughout the tree.

> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

Sort includes like this:

#include   // special, always goes first

#include  // lexicographic
#include 
#include 
#include 
#include 

> +#define  IETP_TOUCH_LMB  (1 << 0)
> +#define  IETP_TOUCH_RMB  (1 << 1)
> +#define  IETP_TOUCH_MMB  (1 << 2)

Use __BIT instead of shifts here: __BIT(0), __BIT(1), __BIT(2).

> +static int ietp_ioctl(void *dev, u_long cmd, void *data, int flag,
> +struct lwp *l);

Tiny nit: four-space continuation lines, not tab + 3space.

> +static const struct wsmouse_accessops ietp_mouse_access = {
> + ietp_enable,
> + ietp_ioctl,
> + ietp_disable
> +};

Use C99 designated initializers here: `.enable = ietp_enable', &c.

> +ietp_match(device_t parent, cfdata_t match, void *aux)
> +{
> ...
> + if (iic_use_direct_match(ia, match, compat_data, &match_result)) {
> + return I2C_MATCH_DIRECT_COMPATIBLE;
> + }

Why does this return I2C_MATCH_DIRECT_COMPATIBLE rather than
match_result?

The usual idea of iic_use_direct_match is that it returns true if it
has an answer, and match_result is the answer (except that you seem to
have patched that logic away, but I'm not clear on why).

> + return (dpi * 10 /254);

Tiny nit: space on both sides of the operator.

> +ietp_attach(device_t parent, device_t self, void *aux)
> +{
> ...
> + ietp_fetch_descriptor(sc);

Check return value here?

> + sc->sc_ih = acpi_intr_establish(sc->sc_dev, sc->sc_phandle, IPL_TTY, 
> false,
> + ietp_intr, sc, 
> device_xname(sc->sc_dev));

Tiny nits:
- break line before 80 columns
- maybe write `/*mpsafe*/false' for clarity (not your fault, why is
  this a boolean and not a named flag?)
- four-space continuation lines

> + printf("%s: failed reading product ID\n", 
> device_xname(sc->sc_dev));

Use aprint_error_dev(sc->sc_dev, "failed to read product ID\n")
instead of printf with device_xname here and everywhere in the attach
routine for this kind of error message.

However, after attach, use device_printf rather than aprint_*_dev.

> +ietp_detach(device_t self, int flags)
> +{

This should start with:

error = config_detach_children(self, flags);
if (error)
return error;

Otherwise, there's nothing to disconnect wsmouse(4) when the device is
detached.  You can test the detach path with drvctl(4):

- `drvctl -d ietp0' to detach ietp0
- `drvctl -r iic0' to rescan iic0 (not sure if this will work, but
  it's worth a shot!)

> + return (0);

Tiny nit: `return 0', no parentheses.

> +ietp_activate(device_t self, enum devact act)
> +{
> + struct ietp_softc *sc = device_private(self);
> +
> + DPRINTF(("%s(%d)\n", __func__, act));
> +
> + switch (act) {
> + case DVAC

Re: [PATCH] Driver for Elantech I2C touchpad

2023-07-29 Thread Vladimir 'phcoder' Serbinenko
Ping?

Le sam. 15 juil. 2023, 04:48, Vladimir 'phcoder' Serbinenko <
phco...@gmail.com> a écrit :

> I've submitted a similar patch for OpenBSD recently and it got merged. It
> adds support for Elantech I2C touchpad used on many laptops. Tested on my
> Chromebook Elemi
>


Re: [PATCH] Driver for Elantech I2C touchpad

2023-07-23 Thread Vladimir 'phcoder' Serbinenko
Le mar. 18 juil. 2023, 14:35, Martin Husemann  a écrit :

> On Sat, Jul 15, 2023 at 03:48:54AM +0200, Vladimir 'phcoder' Serbinenko
> wrote:
> > I've submitted a similar patch for OpenBSD recently and it got merged. It
> > adds support for Elantech I2C touchpad used on many laptops. Tested on my
> > Chromebook Elemi
>
> Do you know where the compat list comes from? My notebook has an ELAN0501,
> worth trying to add that to the compat array?
>
The list comes from FreeBSD. Does your touchpad have CID for PNP0C50 or
ACPI0C50? If it does then I2C HID driver might be a better match

>
> Martin
>


Re: [PATCH] Driver for Elantech I2C touchpad

2023-07-18 Thread Martin Husemann
On Sat, Jul 15, 2023 at 03:48:54AM +0200, Vladimir 'phcoder' Serbinenko wrote:
> I've submitted a similar patch for OpenBSD recently and it got merged. It
> adds support for Elantech I2C touchpad used on many laptops. Tested on my
> Chromebook Elemi

Do you know where the compat list comes from? My notebook has an ELAN0501,
worth trying to add that to the compat array?

Martin


[PATCH] Driver for Elantech I2C touchpad

2023-07-14 Thread Vladimir 'phcoder' Serbinenko
I've submitted a similar patch for OpenBSD recently and it got merged. It
adds support for Elantech I2C touchpad used on many laptops. Tested on my
Chromebook Elemi
[PATCH] Add Elantech I2C touchpad driver

---
 share/man/man4/ietp.4   |  47 +++
 sys/arch/amd64/conf/GENERIC |   3 +
 sys/dev/i2c/files.i2c   |   5 +
 sys/dev/i2c/i2c.c   |   2 +-
 sys/dev/i2c/ietp.c  | 727 
 sys/dev/i2c/ietp.h  |  66 
 6 files changed, 849 insertions(+), 1 deletion(-)
 create mode 100644 share/man/man4/ietp.4
 create mode 100644 sys/dev/i2c/ietp.c
 create mode 100644 sys/dev/i2c/ietp.h

diff --git a/share/man/man4/ietp.4 b/share/man/man4/ietp.4
new file mode 100644
index 0..49782adb9
--- /dev/null
+++ b/share/man/man4/ietp.4
@@ -0,0 +1,47 @@
+.\"	$NetBSD: ietp.4,v 1.0 2023/07/05 20:28:00 jmc Exp $
+.\"
+.\" Copyright (c) 2016 joshua stein 
+.\" Copyright (c) 2023 vladimir serbinenko 
+.\"
+.\" Permission to use, copy, modify, and distribute this software for any
+.\" purpose with or without fee is hereby granted, provided that the above
+.\" copyright notice and this permission notice appear in all copies.
+.\"
+.\" THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+.\" WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+.\" MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+.\" ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+.\" WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+.\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+.\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+.\"
+.Dd $Mdocdate: July 5 2023 $
+.Dt IETP 4
+.Os
+.Sh NAME
+.Nm ietp
+.Nd Elantech touchpad
+.Sh SYNOPSIS
+.Cd "ietp* at iic?"
+.Cd "wsmouse* at ietp? mux 0"
+.Sh DESCRIPTION
+The
+.Nm
+driver provides support for Elantech touchpad
+devices connected over Inter-Integrated Circuit (I2C) buses.
+Access to these devices is through the
+.Xr wscons 4
+driver.
+.Sh SEE ALSO
+.Xr iic 4 ,
+.Xr wsmouse 4
+.Sh HISTORY
+The
+.Nm
+device driver first appeared in
+.Ox 7.4 .
+.Sh AUTHORS
+The
+.Nm
+driver was written by
+.An vladimir serbineko Aq Mt phco...@gmail.com .
diff --git a/sys/arch/amd64/conf/GENERIC b/sys/arch/amd64/conf/GENERIC
index 8af488d17..61660f536 100644
--- a/sys/arch/amd64/conf/GENERIC
+++ b/sys/arch/amd64/conf/GENERIC
@@ -612,6 +612,9 @@ ihidev* at iic?
 ims*	at ihidev? reportid ?
 wsmouse* at ims? mux 0
 
+ietp* at iic?   # Elantech touchpad
+wsmouse* at ietp? mux 0
+
 # I2O devices
 iop*	at pci? dev ? function ?	# I/O processor
 iopsp*	at iop? tid ?			# SCSI/FC-AL ports
diff --git a/sys/dev/i2c/files.i2c b/sys/dev/i2c/files.i2c
index c1c61c5f4..e077d76f0 100644
--- a/sys/dev/i2c/files.i2c
+++ b/sys/dev/i2c/files.i2c
@@ -338,6 +338,11 @@ device	imt: hid, hidmt, wsmousedev
 attach	imt at ihidbus
 file	dev/i2c/imt.cimt
 
+# Elantech touchpad
+device	ietp: wsmousedev
+attach	ietp at iic
+file	dev/i2c/ietp.cietp
+
 # Taos TSL256x ambient light sensor
 device	tsllux: sysmon_envsys
 attach	tsllux at iic
diff --git a/sys/dev/i2c/i2c.c b/sys/dev/i2c/i2c.c
index 3619fa464..488f88ada 100644
--- a/sys/dev/i2c/i2c.c
+++ b/sys/dev/i2c/i2c.c
@@ -646,7 +646,7 @@ iic_use_direct_match(const struct i2c_attach_args *ia, const cfdata_t cf,
 
 	if (ia->ia_ncompat > 0 && ia->ia_compat != NULL) {
 		*match_resultp = iic_compatible_match(ia, compats);
-		return true;
+		return *match_resultp != 0;
 	}
 
 	return false;
diff --git a/sys/dev/i2c/ietp.c b/sys/dev/i2c/ietp.c
new file mode 100644
index 0..2494f5154
--- /dev/null
+++ b/sys/dev/i2c/ietp.c
@@ -0,0 +1,727 @@
+/* $NetBSD: ietp.c,v 1.28 2023/07/04 15:14:01 kettenis Exp $ */
+/*
+ * elan-i2c driver
+ *
+ * Copyright (c) 2015, 2016 joshua stein 
+ * Copyright (c) 2020, 2022 Vladimir Kondratyev 
+ * Copyright (c) 2023 vladimir serbinenko 
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+/* Protocol documentation: https://lkml.indiana.edu/hypermail/linux/kernel/1205.0/02551.html.
+   Based on FreeBSD ietp driver.
+*/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+#include 
+#include 
+
+#include 
+#include 
+
+/* #define IETP