Page MenuHomeFreeBSD

Contributor Reviews (base)Project
ActivePublic

Recent Activity

Feb 8 2021

wulf added a comment to D28521: dev/usb/input/wsp: Add sysctl tunable for Z-Axis inversion.

Thanks! Would you be able to commit it for me?

Feb 8 2021, 9:47 PM · Contributor Reviews (base)
wulf closed D28521: dev/usb/input/wsp: Add sysctl tunable for Z-Axis inversion.
Feb 8 2021, 9:36 PM · Contributor Reviews (base)
james.wright_digital-chaos.com added a comment to D28521: dev/usb/input/wsp: Add sysctl tunable for Z-Axis inversion.
In D28521#638470, @wulf wrote:

LGTM

Feb 8 2021, 6:20 PM · Contributor Reviews (base)

Feb 7 2021

wulf added a comment to D28521: dev/usb/input/wsp: Add sysctl tunable for Z-Axis inversion.

Is device supported by LibINPUT or IICHID? If so why not control it with "Narual Scroll" in /usr/local/share/X11/xorg.conf.d/40-libinput.conf :

wsp(4) still does not support evdev, so no touchpad libinput options can be applied to it.

Feb 7 2021, 7:09 PM · Contributor Reviews (base)
wulf accepted D28521: dev/usb/input/wsp: Add sysctl tunable for Z-Axis inversion.

LGTM

Feb 7 2021, 7:04 PM · Contributor Reviews (base)
tomek_cedro.info added a comment to D28521: dev/usb/input/wsp: Add sysctl tunable for Z-Axis inversion.

Hello world :-)

Feb 7 2021, 3:42 PM · Contributor Reviews (base)
hselasky added a comment to D28521: dev/usb/input/wsp: Add sysctl tunable for Z-Axis inversion.

wulf@ Can you please have a look?

Feb 7 2021, 3:41 PM · Contributor Reviews (base)
hselasky added a reviewer for D28521: dev/usb/input/wsp: Add sysctl tunable for Z-Axis inversion: wulf.
Feb 7 2021, 3:41 PM · Contributor Reviews (base)
james.wright_digital-chaos.com requested review of D28521: dev/usb/input/wsp: Add sysctl tunable for Z-Axis inversion.
Feb 7 2021, 3:00 PM · Contributor Reviews (base)

Jan 1 2021

rozhuk.im-gmail.com abandoned D19690: mount/unmount events to devd.
Jan 1 2021, 10:51 PM · Contributor Reviews (base)
rozhuk.im-gmail.com added a comment to D19690: mount/unmount events to devd.

Done in https://reviews.freebsd.org/D25969

Jan 1 2021, 10:50 PM · Contributor Reviews (base)

Dec 21 2020

hselasky added a comment to D11140: OSS: allow unplug soundcars without apps close devices.

I think you approach may work for the mixer case, assuming no IOCTLs are sleeping. Please make sure that destroy_dev() will flush all pending system calls.

Dec 21 2020, 10:48 AM · Contributor Reviews (base), multimedia
rozhuk.im-gmail.com updated the diff for D11140: OSS: allow unplug soundcars without apps close devices.

This is only mixer unplug fix.

Dec 21 2020, 4:14 AM · Contributor Reviews (base), multimedia

Jul 25 2020

marc.priggemeyer_gmail.com updated the summary of D16698: First draft HID over I2C support (Mouse only).
Jul 25 2020, 9:52 AM · Contributor Reviews (base)

Jul 22 2020

KOT_MATPOCKuH.Ru added a comment to D16698: First draft HID over I2C support (Mouse only).

Are you having issues with sysutils/iichid or wulf's respository? He fixed serious issues and provided a HID layer that you should stick to.

Jul 22 2020, 12:05 PM · Contributor Reviews (base)

Jul 15 2020

marc.priggemeyer_gmail.com added a comment to D16698: First draft HID over I2C support (Mouse only).

I'm using patches from this review for last two years.
Please apply this patch to FreeBSD base code.

Jul 15 2020, 1:27 PM · Contributor Reviews (base)
KOT_MATPOCKuH.Ru added a comment to D16698: First draft HID over I2C support (Mouse only).

I'm using patches from this review for last two years.
Please apply this patch to FreeBSD base code.

Jul 15 2020, 11:02 AM · Contributor Reviews (base)

Jun 26 2020

cperciva accepted D18482: Amazon EC2: Disable floppy devices (fdc0, fd0,) and parallel port device (pp0).

Ah, on t2.micro I see warnings about fdc (but not about ppc). Normally I aim to minimize the difference between EC2 and "stock" FreeBSD, but I guess there's no harm in making this change even if it only affects older instance types.

Jun 26 2020, 12:55 AM · Contributor Reviews (base)

Jun 25 2020

imp accepted D18482: Amazon EC2: Disable floppy devices (fdc0, fd0,) and parallel port device (pp0).
Jun 25 2020, 11:25 PM · Contributor Reviews (base)
james.wright_digital-chaos.com added a comment to D18482: Amazon EC2: Disable floppy devices (fdc0, fd0,) and parallel port device (pp0).

Which instance type are you seeing this on? I haven't been able to reproduce it.

Jun 25 2020, 10:20 PM · Contributor Reviews (base)
cperciva added a comment to D18482: Amazon EC2: Disable floppy devices (fdc0, fd0,) and parallel port device (pp0).

Which instance type are you seeing this on? I haven't been able to reproduce it.

Jun 25 2020, 9:09 PM · Contributor Reviews (base)
lwhsu accepted D18482: Amazon EC2: Disable floppy devices (fdc0, fd0,) and parallel port device (pp0).

@cperciva I think this patch is good, can you check it? Thanks!

Jun 25 2020, 1:28 PM · Contributor Reviews (base)

Feb 15 2020

jhibbits abandoned D7697: Add AER register reporting support via sysctl.

No longer working on this. Someone else can take it up.

Feb 15 2020, 3:49 PM · Contributor Reviews (base), PCI

Dec 25 2019

zarychtam_plan-b.pwste.edu.pl added a comment to D16698: First draft HID over I2C support (Mouse only).

I don't know if comments to this review are still relevant, but I'd like to report that this driver built on recent 12.1-STABLE can work fine after resume. To enable suspend/resume support the module acpi_iichid has to be unloaded and loaded again which can be done for instance with the help of rc.{suspend,resume} scripts.

Dec 25 2019, 2:41 AM · Contributor Reviews (base)

Dec 18 2019

lwhsu added a member for Contributor Reviews (base): lwhsu.
Dec 18 2019, 11:25 PM

Oct 7 2019

wulf added a comment to D16698: First draft HID over I2C support (Mouse only).

Thanks for the remarks. I'd suggest to stop riding a dead horse and make the switch altogether.

This driver still has some advantages over mine. It supports 12.0-release, sysmouse(8) protocol and it does not require ig4 patching. So, I would prefer it for non-HID trackpads like some Synaptics models. Al least for now.

Oct 7 2019, 7:11 PM · Contributor Reviews (base)
marc.priggemeyer_gmail.com added a comment to D16698: First draft HID over I2C support (Mouse only).

Thanks for the remarks. I'd suggest to stop riding a dead horse and make the switch altogether. It has been pointed out previously that the separation of ACPI and HID over I2C code like it was done here, especially the interrupt handling, was ill-conceived.
Abstraction of HID code was initially considered, but not implemented on my part. Since wulf has already done these things and even has the multitouch part working, switching over is the most sensible thing. This code here was intended to be a first draft and has served its purpose.

Oct 7 2019, 6:16 PM · Contributor Reviews (base)
imp added a comment to D16698: First draft HID over I2C support (Mouse only).
In D16698#478767, @imp wrote:

So with the suggested changes, I'm not able to get very far with the patches in this review. With wulf's code I'm able to get the device probed. With this code, I see the following

Is this code still really considered? wulf's repo is currently actively developed (now even includes the long-awaited and very necessary "HID bus" abstraction), this was last updated in August.

data point: I'm using wulf's code on my Pixelbook, multi-touch works great for both the touchpad and touchscreen.

Oct 7 2019, 5:26 PM · Contributor Reviews (base)
greg_unrelenting.technology added a comment to D16698: First draft HID over I2C support (Mouse only).
In D16698#478767, @imp wrote:

So with the suggested changes, I'm not able to get very far with the patches in this review. With wulf's code I'm able to get the device probed. With this code, I see the following

Oct 7 2019, 2:05 PM · Contributor Reviews (base)
imp added a comment to D16698: First draft HID over I2C support (Mouse only).

So with the suggested changes, I'm not able to get very far with the patches in this review. With wulf's code I'm able to get the device probed. With this code, I see the following:

Oct 7 2019, 1:12 AM · Contributor Reviews (base)
imp added inline comments to D16698: First draft HID over I2C support (Mouse only).
Oct 7 2019, 1:08 AM · Contributor Reviews (base)

Sep 29 2019

rozhuk.im-gmail.com added a comment to D16698: First draft HID over I2C support (Mouse only).

kernel trap 22 with interrupts disabled
panic: spin locks can only use msleep_spin
cpuid = 7 time = 1569790352
KDB: stack backtrace:
#0 0xffffffff805da3fd at kdb_backtrace+0x6d
#1 0xffffffff80594fed at vpanic+0x19d
#2 0xffffffff80594e43 at panic+0x43
#3 0xffffffff8057df32 at unlock_spin+0x12
#4 0xffffffff8053f577 at _cv_wait_sig+0x127
#5 0xffffffff81d1dace at iichid_read+0x8e
#6 0xffffffff804b690a at devfs_read_f+0xda
#7 0xffffffff805f4752 at kern_readv+0x92
#8 0xffffffff805f46b4 at sys_read+0x84
#9 0xffffffff808451d8 at amd64_syscall+0x218
#10 0xffffffff80821270 at fast_syscall_common+0x101

Sep 29 2019, 9:08 PM · Contributor Reviews (base)
wulf added inline comments to D16698: First draft HID over I2C support (Mouse only).
Sep 29 2019, 9:45 AM · Contributor Reviews (base)
wulf added inline comments to D16698: First draft HID over I2C support (Mouse only).
Sep 29 2019, 9:26 AM · Contributor Reviews (base)
wulf added inline comments to D16698: First draft HID over I2C support (Mouse only).
Sep 29 2019, 9:09 AM · Contributor Reviews (base)
rozhuk.im-gmail.com added a comment to D16698: First draft HID over I2C support (Mouse only).

Panic on kldload acpi_iichid in iichid_read()->cv_wait_sig().
FreeBSD 12.1 amd64 @ Asus 505z

Sep 29 2019, 5:04 AM · Contributor Reviews (base)

Aug 25 2019

imp added a comment to D16698: First draft HID over I2C support (Mouse only).

I built this on -current and got an insta-panic when trying to lock a null spin lock in attach...

Aug 25 2019, 5:56 AM · Contributor Reviews (base)
imp added inline comments to D16698: First draft HID over I2C support (Mouse only).
Aug 25 2019, 5:52 AM · Contributor Reviews (base)

Aug 23 2019

wulf added a comment to D16698: First draft HID over I2C support (Mouse only).
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?

Aug 23 2019, 1:01 AM · Contributor Reviews (base)

Aug 22 2019

imp added a comment to D16698: First draft HID over I2C support (Mouse only).
In D16698#461247, @wulf wrote:

Hi @marc.priggemeyer_gmail.com,

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.

The extension walks through ACPI children of I2C controller and for each device found it:

  1. Creates iicbus child device
  2. Adds I2C address to ivars
  3. Adds ACPI-handle to iicbus-child ivars, so you can evaluate acpi_get_handle(dev); on iicbus child
  4. Copies all newbus resources from corresponding ACPIbus-hosted device to I2Cbus-hosted one.

It effectively makes a copy of existing ACPI device on top of I2C bus.
It should also fix ig4 bug that brokes allocation of interrupts on ig4 children (Only partially tested yet, I have access only to GPIO devices now)

I think using of this extension should eliminate nasty ACPI<->I2C devices interaction that is found in the patch from current review.
iicbus(4) extension support in my driver (for reference): https://github.com/wulf7/iichid/commit/6f5c3d3eb008848ab9d18e1dcf8fd35ca80e517e

Patch could be found here: https://people.freebsd.org/~wulf/acpi_iicbus.patch

Aug 22 2019, 7:06 PM · Contributor Reviews (base)

Aug 12 2019

wulf added a comment to D16698: First draft HID over I2C support (Mouse only).

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.

Aug 12 2019, 12:15 AM · Contributor Reviews (base)

Jul 29 2019

nikola.lecic_anthesphoria.net added a comment to D16698: First draft HID over I2C support (Mouse only).
In D16698#457967, @wulf wrote:

Could you send me your DSDT table with email? It is too big to be posted here. It can be obtained with

# acpidump -dt

Sure, done.

Jul 29 2019, 1:46 PM · Contributor Reviews (base)
wulf added a comment to D16698: First draft HID over I2C support (Mouse only).

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
Jul 29 2019, 1:20 PM · Contributor Reviews (base)
nikola.lecic_anthesphoria.net added a comment to D16698: First draft HID over I2C support (Mouse only).

try to replace 'IG4_CTL_SPEED_STD' string in /usr/src/sys/dev/ichiic/ig4_iic.c file with digit '6' than rebuild and reload ig4.ko .

Jul 29 2019, 1:04 AM · Contributor Reviews (base)
wulf added a comment to D16698: First draft HID over I2C support (Mouse only).
In D16698#457883, @wulf 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
Jul 29 2019, 12:16 AM · Contributor Reviews (base)
nikola.lecic_anthesphoria.net added a comment to D16698: First draft HID over I2C support (Mouse only).
In D16698#457883, @wulf 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

Jul 29 2019, 12:03 AM · Contributor Reviews (base)

Jul 28 2019

wulf added a comment to D16698: First draft HID over I2C support (Mouse only).

Yes, it still prints.

Jul 28 2019, 11:30 PM · Contributor Reviews (base)
nikola.lecic_anthesphoria.net added a comment to D16698: First draft HID over I2C support (Mouse only).
In D16698#457878, @wulf wrote:
In D16698#457872, @wulf 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".

It should print continuously if you'd move your finger

Jul 28 2019, 11:18 PM · Contributor Reviews (base)
wulf added a comment to D16698: First draft HID over I2C support (Mouse only).
In D16698#457872, @wulf 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".

Jul 28 2019, 10:53 PM · Contributor Reviews (base)
nikola.lecic_anthesphoria.net added a comment to D16698: First draft HID over I2C support (Mouse only).
In D16698#457872, @wulf 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?

Jul 28 2019, 10:41 PM · Contributor Reviews (base)