In D16698#464917, @imp wrote:

We really have three problems that we need to solve:
(1) iichid
(2) how to get the acpi stuff to automatically add iichid devices

iichid device is a child of acpi bus also, so it can be rephrased as "how to get the acpi stuff to automatically add acpi device with given CID"

(3) How to cope with resume and resets not being quite right for the ig4.

It is not clear which reset is mentioned:

1. IG4 controller reset (set designware specific parameters like timing counters and so on)
2. iicbus reset - FIFO flushing, setting of symbol speed and slave address
3. I2C hid device reset - performed with special I2C command

If it is mentioned in context of @johalun suspend/resume patch, then most probably it misses iicbus reset as it is performed by ig4 driver in a lazy way at a start of next xfer. sc->slave_valid bool variable should be reset to trigger it and I do not see that in @johalun resume method.

Would we make better progress with we split those three issues up into their own reviews?

Finally, I made separate small kernel module ng_ubt_intel which shares object file with ng_ubt.
It inherits device methods from ng_ubt while overriding it's probe() method.
That allows move vendor-specific code out of ng_ubt and avoid code duplication the same time.
Firmware is still downloaded with libusb as I don't see any benefits from moving one-shot driver to kernelspace.

I wrote iicbus(4) extension which performs ACPI-based enumeration of I2C devices connected to a controller. It is not limited to HID devices and fires at iicbus attach stage.

In D16698#457886, @nikola.lecic_anthesphoria.net wrote:

However, loading iichid.ko doesn't go well:

Jul 29 03:00:32 thorium kernel: acpi_iichid0: <HID over I2C (ACPI)> on acpi0
Jul 29 03:00:32 thorium kernel: imt0:   ACPI Hardware ID  : ELAN1200
Jul 29 03:00:32 thorium kernel: imt0:   IICbus addr       : 0x15
Jul 29 03:00:32 thorium kernel: imt0:   HID descriptor reg: 0x01
Jul 29 03:00:32 thorium kernel: imt0: could not retrieve HID descriptor from the device: 3

In D16698#457884, @nikola.lecic_anthesphoria.net wrote:

In D16698#457883, @wulf wrote:

In D16698#457882, @nikola.lecic_anthesphoria.net wrote:

Yes, it still prints.

Ok. than uncomment pair of debugging printfs in iichid_event_task(). They lie several lines below iichid_intr() sub.
If you get one line with hex data per "something", post it here

If you mean these lines:

device_printf(sc->dev, "no data received\n");

and

DPRINTF(sc, "%*D\n", actual, sc->ibuf, " ");

then I get just this (a single touch):

Jul 29 02:01:08 thorium kernel: something
Jul 29 02:01:08 thorium syslogd: last message repeated 2 times
Jul 29 02:01:08 thorium kernel: imt0: no data received
Jul 29 02:01:08 thorium kernel: something
Jul 29 02:01:08 thorium syslogd: last message repeated 4 times
Jul 29 02:01:08 thorium kernel: imt0: no data received
Jul 29 02:01:08 thorium syslogd: last message repeated 1 times
Jul 29 02:01:08 thorium kernel: something
Jul 29 02:01:08 thorium syslogd: last message repeated 3 times
Jul 29 02:01:08 thorium kernel: imt0: no data received
Jul 29 02:01:08 thorium kernel: something
Jul 29 02:01:08 thorium syslogd: last message repeated 2 times
Jul 29 02:01:08 thorium kernel: imt0: no data received
Jul 29 02:01:08 thorium syslogd: last message repeated 1 times
Jul 29 02:01:08 thorium kernel: something
Jul 29 02:01:08 thorium syslogd: last message repeated 5 times
Jul 29 02:01:08 thorium kernel: imt0: no data received
Jul 29 02:01:08 thorium kernel: something
Jul 29 02:01:08 thorium syslogd: last message repeated 3 times
Jul 29 02:01:08 thorium kernel: imt0: no data received

In D16698#457882, @nikola.lecic_anthesphoria.net wrote:

Yes, it still prints.

In D16698#457877, @nikola.lecic_anthesphoria.net wrote:

In D16698#457872, @wulf wrote:

In D16698#457858, @nikola.lecic_anthesphoria.net wrote:

In D16698#457847, @wulf wrote:

then comment out imt_set_input_mode() call in imt_attach() subroutine (imt.c) and insert printf("something\n"); into iichid_intr() sub (iichid.c)
If it print something on the console when you touching trackpad surface?

Yes! :) It prints "something".

In D16698#457858, @nikola.lecic_anthesphoria.net wrote:

In D16698#457847, @wulf wrote:

In D16698#457762, @nikola.lecic_anthesphoria.net wrote:

Here it is.

In D16698#457802, @zarychtam_plan-b.pwste.edu.pl wrote:

Updated D16698 builds and runs fine on 12-STABLE though it still lacks resume support.
I must admit I wasn't able to test https://github.com/wulf7/iichid. The module didn't work for me:
Jul 28 18:20:49 bsdondell kernel: imt0: IRQ allocation failed. Fallback to sampling.

In D16698#457728, @nikola.lecic_anthesphoria.net wrote:

All done, no difference, nothing happens with evemu-record. :(

In D16698#456379, @nikola.lecic_anthesphoria.net wrote:

Touching/pressing/doing anything with touchpad produces absolutely nothing.

In D16698#456096, @nikola.lecic_anthesphoria.net wrote:

Sorry, no difference.

I enabled debugging output in imt.c so please:

In D16698#455908, @nikola.lecic_anthesphoria.net wrote:

The only problem -- the device is still dead;

I fixed one bug, so try one more time

it generates nothing with libinput debug-events, and no events in xev.

libinput debug-events is not the right tool to debug evdev. evemu-record from devel/evemu port is better

In D16698#455794, @nikola.lecic_anthesphoria.net wrote:

Unlike with previous driver, I don't see /dev/input/event3 (HID over IIC) anymore.

That is expected as it ignored any touchpads

In D16698#455451, @nikola.lecic_anthesphoria.net wrote:

@wulf, please explain how to test this driver. Should I just compile new iichid.ko (running make in the input directory),

just unpack it at your $HOME. Than

make && sudo kldload ./iichid.ko

leaving all other modules built from the source of this thread?

drivers from this review should be at least unloaded from kernel. There is no need to revert D16698 patch. Just don't try to kldload both modules in between reboots.

Should I remove evdev support from the kernel?

No need. It links with evdev unconditionally

Can you please share your xorg configuration?

I do not have any specific xorg.conf options. Any evdev-awared autoconfiguration backend should work out of box. See https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=196678 for example.

In D16698#455438, @nikola.lecic_anthesphoria.net wrote:

Parsed:

0x05, 0x0D,        // Usage Page (Digitizer)
0x09, 0x05,        // Usage (Touch Pad)
0xA1, 0x01,        // Collection (Application)
0x85, 0x04,        //   Report ID (4)
0x09, 0x22,        //   Usage (Finger)
0xA1, 0x02,        //   Collection (Logical)

OS driver or userland applications just do not have enough information as finger coords or count to support doublefinger scroll or multifinger taps.

I wrote a simple util to dump report descriptor from I2C HID device

In D16698#455238, @nikola.lecic_anthesphoria.net wrote:

I see. Sigh. Anyway, some people from this thread reported that two finger scrolling *does* work, including @johalun himself in his announcement:

If I do two finger horizontal scroll I don't get any event callbacks at all while I do for vertical. Could this be something that has to be enabled by sending some command to the device?

In D16698#455200, @greg_unrelenting.technology wrote:

We have the wmt driver but right now it's *touchscreen*-only and (of course) USB-only.

In D16698#452632, @markj wrote:

Oh, nice. Your iichid_identify() method is basically identical to what I wrote. You're right, it does not strictly require a separate module, it just seemed cleaner to have a generic acpi_iichid bus.

I don't object to acpi_iichid module existence. It just adds an extra requirements for module load order which I was not able to solve quickly.

I think it would be best to try and base the ims driver on top of your code. Are you interested in trying to integrate them?

Yes, I am going to submit this code to the project when it will be ready, so it is better to have it insync with ims

I wrote a driver for I2C MT touchscreens https://github.com/wulf7/iichid which is heavily based on the code in this review.
It does not require dedicated ACPI module at all and has some I2C/HID layers separation which is missed in current review.
So I think it may have sense to join efforts and codebases

In D20507#442964, @jan.kokemueller_gmail.com wrote:

This was tried some years ago, but then reverted as the probing was deemed too intrusive (see r255152 and r255210). I don't know if something changed in the meantime.

Synaptics and Elantech probe() does not depend on loader tuneables at all. Magic knock and capability queries are always performed regardless of kernel boot parameters so it cannot be called "too intrusive".
But IBM trackpoint probe() is different thing. It is performed in different ways depending on 'synaptics_support' variable value.
Browsing through the commit log shows me that there were several patches submitted by you to fix various trackpoint issues circa 2015 so it's worth trying to enable it by default again as support of this device is definitely changed since 2013.

In D20507#443152, @phascolarctos_protonmail.ch wrote:

I tested and it works. Thanks!
Unless solution 1 has some side effects for someone (I think to people using a mouse instead of a touchpad),

I doubt it can produce side effects. Albeit psm packet parser is replaced with Xorg`s one, PS/2 device recognition is still relies on in-kernel probe() function. So Xorg can not handle any PS/2 devices unknown to base system.

I think this is the solution to adopt: if we want touchpads to work out of the box, we can not ask users to start moused before xorg. Of course, they can add moused_enable="YES" to their rc.conf, but if we can make a small change to the xf86-input-mouse port it looks much easier: it affects only users of xorg and avoids to run a daemon on the system.

Better solution is to run moused from devd exactly like it is done for ums(4), but it does not work for me now.

In D20507#443112, @phascolarctos_protonmail.ch wrote:

I prefer solution 1.
If you can tell me which is the line to comment or provide a patch I can test it.

After some digging it appeared that xf86-input-mouse unconditionally enables so called "native operation level" being attached to /dev/psm0 device node. It means that it effectively replaces our kernel driver with his own PS/2 packet parser.
So we have 2 choices:

1. Forbid enabling of "native operation level" in xf86-input-mouse driver. It looks like commenting out one #define line
2. Force all touchpad users to run moused to grab /dev/psm0 device and reroute its packets via /dev/sysmouse before Xorg is started.

In D20507#443011, @phascolarctos_protonmail.ch wrote:

(written while wulf was writing his comment. Now I'm going to try his suggestions.)

In D20507#442998, @phascolarctos_protonmail.ch wrote:

I think libinput does not see my touchpad at all. Here is a log with the mouse driver installed:

Count this "accept" as "commit approval"

Given the fact that horizontal scrolling with sysmouse protocol is not supported by userland software out of boх, I joined both hor & ver sysctls into common "natural_scroll" one to not confuse users.

One more question:

Hi Nyan,
Could you upload the diff as fullcontext diff next time. See https://wiki.freebsd.org/Phabricator

git diff -U999999 other-branch > change.diff
git show -U999999 <commit-hash> > change.diff
svn diff --diff-cmd=diff -x -U999999 > change.diff`

It is not necessary to reupload current diff

I hope you won't mind if I change sysctl name from "input_id" to "id" and disable exposure of optional properties like "uniq" and "phys" if they are not set. Just to be consistent with ioctl interface.

In D18694#409332, @greg_unrelenting.technology wrote:

added missing EVDEV_UNLOCK

One thing I dislike in this patch is a native/cuse evdev handling inconsistency.
To handle native device creation one should listen for EVDEV devd events while to handle cuse devices CDEV devd events should be processed.
You can not just listen for CDEV devd events as there is a race window between cdev and sysctl creation.
I think this can be fixed with using of single sysctl node. See kern.geom.confxml or kern.geom.conftxt for example.

linux/input.h is a list of magic numbers required at build time only. It should be kept in BUILD_DEPENDS IMO.

I am AFK for now and will be for ~2 weeks

In D18694#398967, @greg_unrelenting.technology wrote:

Though I wouldn't say it's only a "deprecated" option — we're not going to require userspace uinput drivers to explicitly identify themselves..

I am objecting to EvdevProbe()-derived part (device type autodetection). It is better to add a KPI to set device type directly rather then via execution of libmagic. We have about 10 evdev drivers in source tree so it's possible to update them all. EvdevProbe()-derived part could be used as (deprecated) fallback option in that case.

I hope -1700 value is tested. It does not work for my "softbuttons bar at a bottom" laptop leaving softbutton area outside reportable region so I have to use -2200 to make it working. But "softbuttons bar at a top" touchpad should behave differently here.

Please assign hw.psm.synaptics.softbuttons_y sysctl some negative value so /dev/psm0 users get benefits from this change too.

I am unaware of any current ill effects

D17687 committed as r339917 made remaining parts of this revision senseless.