Fixed file path in diff for Phabricator

After commandeering the revision, updating it to make sure my diff is the same as the previously submitted diff.

Adding @adrian and @rpaulo as reviewers, as they have been in this code before and might have input.

After commandeering the revision, updating it to make sure my diff is
the same as the previously submitted diff.

I'll take over this diff and get it committed to -CURRENT in the next few days.

In D18395#391434, @dab wrote:

Thanks. I'll take over this diff and get it committed to -CURRENT in the next few days. On what branch or revision did you test this? Do you need it applied to the stable/11 or stable/12?

Thanks. I'll take over this diff and get it committed to -CURRENT in the next few days. On what branch or revision did you test this? Do you need it applied to the stable/11 or stable/12?

In D18395#391412, @dab wrote:

@james.wright_jigsawdezign.com, do you need someone to help get this committed? I could do that if so. Can you update the review with information on testing you've done?

@james.wright_jigsawdezign.com, do you need someone to help get this committed? I could do that if so. Can you update the review with information on testing you've done?

@james.wright_jigsawdezign.com, do you need someone to help get this committed? I could do that if so. Can you update the review with information on testing you've done?

Many thanks for these files and patches.
Now my Asus Vivobook S15 touchpad is working running FreeBSD 12.RC1, kernel source revision 340904.

Thanks for doing this! Works great on my Dell 7490.
Suspend/resume not tested. Please also add EVDEV to this driver so we can get things like horizontal scrolling :)

Awesome Mark.
It works on my Huawei Matebook X Pro. Sorry for comming back so late.
I had 10 minutes to test and here are my initial findings.

@marc.priggemeyer_gmail.com
sorry for bothering you guys
I have managed to kldload acpi_iicbus.ko
but my mosuepad doesn't work
here is my latest FreeBSD dmesg output:
ig4iic_pci0: <Intel Sunrise Point-LP I2C Controller-0> mem 0xd2336000-0xd2336fff at device 21.0 on pci0
ig4iic_pci0: Using MSI
iicbus0: <Philips I2C bus> on ig4iic_pci0
acpi_iichid0: <HID over I2C (ACPI)> irq 51 on acpi0
acpi_iichid0: descriptor register address is 1
acpi_iichid0: - irq: 51
acpi_iichid0: parent device is "\134_SB_.PCI0.I2C0"
iichid0: <HID over I2C> at addr 0x15 on iicbus0
iichid0: ADDR 0x15 REG 0x1
hid_get_item: Number of items(256) truncated to 255
hid_get_item: Number of items(672) truncated to 255
hid_get_item: Number of items(256) truncated to 255
hid_get_item: Number of items(672) truncated to 255
hid_get_item: Number of items(256) truncated to 255
hid_get_item: Number of items(672) truncated to 255
hid_get_item: Number of items(256) truncated to 255
hid_get_item: Number of items(672) truncated to 255
hid_get_item: Number of items(256) truncated to 255
hid_get_item: Number of items(672) truncated to 255
hid_get_item: Number of items(256) truncated to 255
hid_get_item: Number of items(672) truncated to 255
hid_get_item: Number of items(256) truncated to 255
hid_get_item: Number of items(256) truncated to 255
hid_get_item: Number of items(672) truncated to 255
iichid0: 3 buttons and [XY] coordinates ID=1
hid_get_item: Number of items(256) truncated to 255
hid_get_item: Number of items(672) truncated to 255
hid_get_item: Number of items(256) truncated to 255
hid_get_item: Number of items(672) truncated to 255
hid_get_item: Number of items(256) truncated to 255
hid_get_item: Number of items(672) truncated to 255
hid_get_item: Number of items(256) truncated to 255
hid_get_item: Number of items(672) truncated to 255
hid_get_item: Number of items(256) truncated to 255
hid_get_item: Number of items(672) truncated to 255
hid_get_item: Number of items(256) truncated to 255
hid_get_item: Number of items(672) truncated to 255
hid_get_item: Number of items(256) truncated to 255
hid_get_item: Number of items(256) truncated to 255
hid_get_item: Number of items(672) truncated to 255
iichid0: 3 buttons and [XY] coordinates ID=1
hid_get_item: Number of items(256) truncated to 255
hid_get_item: Number of items(672) truncated to 255
acpi_iichid0: added iichid0 ADDR 0x15 REG 0x1 to iicbus0
acpi_iichid0: allocated irq at 0xfffff801088b6780 and rid 0
acpi_iichid0: successfully setup interrupt

@marc.priggemeyer_gmail.com hello sir can you be so kind to help me out, when I try to kldload acpi_iichid.ko it says 'parent is not attached', what should I kldload beforehand or something should be done?

In D16698#374733, @marc.priggemeyer_gmail.com wrote:

It works, but there is only one button and no two finger scrolling, whereas with psm I have 3 buttons (one, two and three finger taps) and two finger scrolling.

Thanks for testing. That your two finger scroll does not work is apparent from the descriptor line identifying only two axis, namely X and Y. For scrolling to work, there should be a Z axis as well. I believe that your touchpad does not only support the mouse reports but also multitouch trackpad reports, but they are not yet considered by iichid. Nevertheless you should be able to use two buttons.

In D16698#374594, @markj wrote:

...
I haven't spent any significant amount of time looking at the diff yet, but I'll take a stab at answering these.
...

First, good job!

I was able to use this on a Dell latitude 7480 to get a working trackpad. Thank you!

I got some open questions that someone actually reviewing this code or just stumbling by might be able to answer:

1. is PI_TTY the preferred thread priority, or should it be something else?
2. I read locking(9), mutex(9), bus_setup_intr(9) and a paper on locking mechanisms in FreeBSD and think that I got a fair idea about the basic principles, but I think I still didn't get a grasp on when sleeping is allowed when using an interrupt routine. From what I understand, I almost always need to create a task queue due to the fact that almost all bus implementations call mtx_sleep at some point. Is that correct? Then again I think I saw some modules calling bus operations (that possibly call mtx_sleep) during interrupt handling directly from the associated ithread routine. Are they doing it wrong?
3. If I use a task queue to handle interrupts and retrieve data from a device and that retrieval method goes to sleep eventually. From my understanding I should release my locks before and reacquire them after the bus operation. What is the preferred method to prevent another thread trying to detach the sleeping module? Possible solutions I figured that might be applicable are flag and condition variable or nested mutex. But what is the right way to do it, or is it even unnecessary because some internal mechanism prevents detaching (device busy msg)?

Thanks for your help.

I just uploaded a new diff that lacks the changes to usb. iichid for now depends on usb but should be easier to test.

Besides that, I think you really will have to fix your ig4 problem first (https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=221777). By the way, the ACPI information you provided there suggests that the HID device is a trackpad. That is not supported yet, since iichid only searches for mouse information in the descriptor.

In D16698#372549, @b4_gewt.net wrote:

Hmmm. Does my ACPI table not _know_ there should be an i2c mouse?

I'm applied this patch to my FreeBSD 12 r338342 box on Dell Latitude 5290 and applied this configuration to rc.conf:
kld_list="/boot/modules/i915kms.ko /boot/modules/iichid.ko /boot/modules/acpi_iichid.ko"
moused_port="/dev/ims0"

Table 'FACP' at 0x78f10960
FACP: Found table at 0x78f10960
ppc: ppc-1 already exists; skipping it
driver bug: Unable to set devclass (class: ppc devname: (unknown))
Table 'FACP' at 0x78f10960
FACP: Found table at 0x78f10960
Table 'FACP' at 0x78f10960
FACP: Found table at 0x78f10960
Table 'FACP' at 0x78f10960
FACP: Found table at 0x78f10960
Table 'FACP' at 0x78f10960
FACP: Found table at 0x78f10960
Table 'FACP' at 0x78f10960
FACP: Found table at 0x78f10960
Table 'FACP' at 0x78f10960
FACP: Found table at 0x78f10960
Table 'FACP' at 0x78f10960
FACP: Found table at 0x78f10960
Table 'FACP' at 0x78f10960
FACP: Found table at 0x78f10960
Table 'FACP' at 0x78f10960
FACP: Found table at 0x78f10960
Table 'FACP' at 0x78f10960
FACP: Found table at 0x78f10960
Table 'FACP' at 0x78f10960
FACP: Found table at 0x78f10960

driver bug: Unable to set devclass (class: ppc devname: (unknown))
driver bug: Unable to set devclass (class: ppc devname: (unknown))
ig4iic_acpi0: <Designware I2C Controller> iomem 0xe1122000-0xe1122fff irq 7 on acpi0
ig4iic_acpi0: controller error during attach-1
driver bug: Unable to set devclass (class: ppc devname: (unknown))
ig4iic_acpi1: <Designware I2C Controller> iomem 0xe1124000-0xe1124fff irq 7 on acpi0
ig4iic_acpi1: controller error during attach-1
iicbus0: <Philips I2C bus> on ig4iic_acpi0
iic0: <I2C generic I/O> on iicbus0
iicbus1: <Philips I2C bus> on ig4iic_acpi1
iic1: <I2C generic I/O> on iicbus1
driver bug: Unable to set devclass (class: ppc devname: (unknown))
wlan0: Ethernet address: 6c:29:95:4e:a3:68

In D16698#372199, @tfagart_brozs.net wrote:

Hello Marc,

Thanks for your quick answer.

My revision is r339210. So I guess I should try 338291.

Regards

Thomas

Hello Marc,

Hi Thomas,

First thanks for your work. I've got a Dell Latitude 5590. Scanning I2C bus now work.
root@pcb-prdtfa-07:~ # i2c -f /dev/iic1 -s
Hardware may not support START/STOP scanning; trying less-reliable read method.
Scanning I2C devices on /dev/iic1: 2c

• Tested against base revision 338291.
• Sampling mode was added to periodically poll for reporrts (see Test Plan for further information)
• Still make sure to have a running iicbus implementation
• it was reported in https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=221777 that a panic occures when trying to load iichid and acpi_iichid by bootloader kld_list in rc.conf is still the way to go if you tested manually beforehand.