On Oct 08 2024, Binbin Zhou wrote: > Hi Benjamin: > > First of all, I'm very sorry that PixArt has caused other drivers to > have anomalies. No worries. These things happens, and that's why we revert patches when there is an issue. Also, note that this issue is 2 fold: - for sure, your driver was too greedy - but also Fedora disabled i2c-designware which is used in many machines disabling the touchpad entirely So you didn't break all of the other touchpads. The Fedora config change did. Your driver just prevented the fallback mode to happen. > > On Tue, Oct 1, 2024 at 3:21 PM Benjamin Tissoires <bentiss@xxxxxxxxxx> wrote: > > > > On Oct 01 2024, Dmitry Torokhov wrote: > > > On Tue, Oct 01, 2024 at 01:53:44AM -0700, Dmitry Torokhov wrote: > > > > On Mon, Sep 30, 2024 at 05:59:01PM +0200, Benjamin Tissoires wrote: > > > > > Hi, > > > > > > > > > > On Jul 04 2024, Binbin Zhou wrote: > > > > > > This patch introduces a driver for the PixArt PS/2 touchpad, which > > > > > > supports both clickpad and touchpad types. > > > > > > > > > > > > At the same time, we extended the single data packet length to 16, > > > > > > because according to the current PixArt hardware and FW design, we need > > > > > > 11 bytes/15 bytes to represent the complete three-finger/four-finger data. > > > > > > > > > > > > Co-developed-by: Jon Xie <jon_xie@xxxxxxxxxx> > > > > > > Signed-off-by: Jon Xie <jon_xie@xxxxxxxxxx> > > > > > > Co-developed-by: Jay Lee <jay_lee@xxxxxxxxxx> > > > > > > Signed-off-by: Jay Lee <jay_lee@xxxxxxxxxx> > > > > > > Signed-off-by: Binbin Zhou <zhoubinbin@xxxxxxxxxxx> > > > > > > > > > > It looks like this new driver made in v6.12-rc1 but is already breaking > > > > > other touchpads in fedora: > > > > > > > > > > https://bugzilla.redhat.com/show_bug.cgi?id=2314756 > > > > > > > > > > The reported touchpads used to work properly but are now directed to use > > > > > the PixArt PS2 driver instead of the old one (I would say it should be > > > > > using Synaptics). > > > > > > > > > > I haven't touched PS/2 in a long time, so it's going to be hard to > > > > > pinpoint the error from my side, but it seems that the new driver is a > > > > > little bit too greedy. > > > > > > > > OK, I gonna revert it and hope PixArt folks will figure out less greedy > > > > probing sequence (or maybe we need to push it down a few sports). > > > > > > Although, as I am trying to read the referenced bug, one of the > > > reporters are saying that they touchpad is USB: > > > > > > SysFS ID: /devices/pci0000:00/0000:00:14.0/usb3/3-3/3-3:1.0 > > > ysFS BusID: 3-3:1.0 > > > Hardware Class: unknown > > > Model: "Synaptics Unclassified device" > > > Hotplug: USB > > > Vendor: usb 0x06cb "Synaptics, Inc." > > > > I guess this must be the fingerprint reader or some other synaptics > > device. > > > > In the 6.11 logs (now publicly available), we can see: > > [ 1.601507] psmouse serio1: trackpoint: Elan TrackPoint firmware: 0x92, buttons: 3/3 > > [ 1.614026] input: TPPS/2 Elan TrackPoint as /devices/platform/i8042/serio1/input/input5 > > ... > > [ 2.286700] input: ELAN0672:00 04F3:3187 Mouse as /devices/pci0000:00/0000:00:15.0/i2c_designware.0/i2c-0/i2c-ELAN0672:00/0018:04F3:3187.0002/input/input7 > > [ 2.286834] input: ELAN0672:00 04F3:3187 Touchpad as /devices/pci0000:00/0000:00:15.0/i2c_designware.0/i2c-0/i2c-ELAN0672:00/0018:04F3:3187.0002/input/input9 > > [ 2.286873] hid-generic 0018:04F3:3187.0002: input,hidraw1: I2C HID v1.00 Mouse [ELAN0672:00 04F3:3187] on i2c-ELAN0672:00 > > ... > > [ 2.337123] input: ELAN0672:00 04F3:3187 Mouse as /devices/pci0000:00/0000:00:15.0/i2c_designware.0/i2c-0/i2c-ELAN0672:00/0018:04F3:3187.0002/input/input10 > > [ 2.337173] input: ELAN0672:00 04F3:3187 Touchpad as /devices/pci0000:00/0000:00:15.0/i2c_designware.0/i2c-0/i2c-ELAN0672:00/0018:04F3:3187.0002/input/input12 > > [ 2.337212] hid-multitouch 0018:04F3:3187.0002: input,hidraw1: I2C HID v1.00 Mouse [ELAN0672:00 04F3:3187] on i2c-ELAN0672:00 > > > > > > So the touchpad seems to have the PS/2 fallback, and then switches to > > i2c-HID. However, with PixArt the PS/2 touchpad isn't initialized, and > > doesn't answered to i2c-hid (or is too much initialized, not sure). > > > > Now, we hope to solve this issue further with your help. > > As you can see in the logs: > > [ 1.649119] psmouse serio1: pixart_ps2: init: Unable to query > PixArt touchpad hardware. Yes. And this is already too far. If you are in the ->init(), that means that you are dealing with a PixArt touchpad. If it's not, that means that you should have stopped at ->detect() before. > > We are failing in the pixart_query_hardware() function, guessing that > it is an error in probing the type value. Can you please add the > following patch to the problematic kernel to see the actual type value > being fetched. I was merely the messenger here. But that debug patch will not help you IMO. In pixart_read_tp_mode() you call ps2_command(ps2dev, param, PIXART_CMD_REPORT_FORMAT); assuming that no other brand will answer that command. Turns out that it's not the case, so you can not rely on this only to detect your touchpads. Especially because the test after looks for the value "1" in the first byte, which is definitely not special. A safe way to get this driver in (but painful as hell) is to rely on dmi_matching. This way you have an allowlist of platforms where the devices are known to be there, and then you can be sure you are talking to your devices. Or you have a safe magic sequence like elantech.c or alps.c where you do a set of commands or expect a command to return a set of data that you can be sure only PixArt touchpads will have. Ideally if one commands returns the vendor name "PixArt" in ascii, that would be perfect :) Cheers, Benjamin > > diff --git a/drivers/input/mouse/pixart_ps2.c b/drivers/input/mouse/pixart_ps2.c > index 1993fc760d7b..73ec6926d07d 100644 > --- a/drivers/input/mouse/pixart_ps2.c > +++ b/drivers/input/mouse/pixart_ps2.c > @@ -69,6 +69,8 @@ static int pixart_read_tp_type(struct ps2dev > *ps2dev, u8 *type) > if (error) > return error; > > + pr_info("pixart_read_tp_type code is %x\n", param[0]); > + > *type = param[0] == 0x0e ? PIXART_TYPE_TOUCHPAD : PIXART_TYPE_CLICKPAD; > > return 0; > > Thanks. > Binbin > > > > > > so I am not sure how PS/2 device would interfere with that. > > > > > > Could you give me access to the attachments on the bug so I can take a > > > closer look? And hopefully the original reporter will submit their data. > > > > Sure, done! > > > > Cheers, > > Benjamin
