On 04.03.24 13:45, Takashi Iwai wrote: > On Mon, 04 Mar 2024 12:26:48 +0100, > Javier Carrasco wrote: >> >> On 04.03.24 09:35, Takashi Iwai wrote: >>> Hi, >>> >>> we've received a few regression reports for openSUSE Leap about the >>> missing touchpad on Macbooks. After debugging, this turned out to be >>> the backport of the commit 2b9c3eb32a699acdd4784d6b93743271b4970899 >>> Input: bcm5974 - check endpoint type before starting traffic >>> >>> And, the same regression was confirmed on the upstream 6.8-rc6 >>> kernel. >>> >>> Reverting the commit above fixes the problem, the touchpad reappears. >>> >>> The detailed hardware info is found at: >>> https://bugzilla.suse.com/show_bug.cgi?id=1220030 >>> >>> Feel free to join the bugzilla above, or let me know if you need >>> something for debugging, then I'll delegate on the bugzilla. >>> >>> >>> thanks, >>> >>> Takashi >>> >> >> Hi Takashi, >> >> The commit adds a check to ensure that the endpoint type is interrupt. >> >> According to that report, the issue arose with a MacBook Pro 5.1 (no >> button, only trackpad endpoint), so the check on the tp_ep address >> (0x81) returns false. I assume that you see an error message >> ("Unexpected non-int endpoint) and the probe function fails returning >> -ENODEV. > > Right, there is the message. > >> Do you see any warning in the logs when you revert the commit? It was >> added to prevent using wrong endpoint types, which will display the >> following warning: "BOGUS urb xfer, pipe "some_number" != type >> "another_number"" > > The revert was tested on the downstream kernel, but it has also the > check of bogus pipe, and there was no such warning, as far as I see > the report. > >> I am just wondering if for some reason the check on interrupt type is >> wrong here. > > I'll ask reporters to give the lsusb -v output so that we can take a > deeper look. Also, I'm building a test kernel based on 6.8-rc7 with > the revert, and ask reporters to test with it, just to be sure. > > > thanks, > > Takashi I retrieved the relevant node from the report that was uploaded a few minutes ago: Bus 003 Device 003: ID 05ac:0237 Apple, Inc. Internal Keyboard/Trackpad (ISO) Device Descriptor: bLength 18 bDescriptorType 1 bcdUSB 2.00 bDeviceClass 0 bDeviceSubClass 0 bDeviceProtocol 0 bMaxPacketSize0 8 idVendor 0x05ac Apple, Inc. idProduct 0x0237 Internal Keyboard/Trackpad (ISO) bcdDevice 0.77 iManufacturer 1 Apple, Inc. iProduct 2 Apple Internal Keyboard / Trackpad iSerial 0 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 0x0054 bNumInterfaces 3 bConfigurationValue 1 iConfiguration 0 bmAttributes 0xa0 (Bus Powered) Remote Wakeup MaxPower 40mA Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 0 bAlternateSetting 0 bNumEndpoints 1 bInterfaceClass 3 Human Interface Device bInterfaceSubClass 1 Boot Interface Subclass bInterfaceProtocol 1 Keyboard iInterface 3 Apple Internal Keyboard HID Device Descriptor: bLength 9 bDescriptorType 33 bcdHID 1.11 bCountryCode 13 International (ISO) bNumDescriptors 1 bDescriptorType 34 Report wDescriptorLength 156 Report Descriptors: ** UNAVAILABLE ** Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x83 EP 3 IN bmAttributes 3 Transfer Type Interrupt Synch Type None Usage Type Data wMaxPacketSize 0x000a 1x 10 bytes bInterval 8 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 1 bAlternateSetting 0 bNumEndpoints 1 bInterfaceClass 3 Human Interface Device bInterfaceSubClass 0 bInterfaceProtocol 0 iInterface 4 Touchpad HID Device Descriptor: bLength 9 bDescriptorType 33 bcdHID 1.11 bCountryCode 0 Not supported bNumDescriptors 1 bDescriptorType 34 Report wDescriptorLength 27 Report Descriptors: ** UNAVAILABLE ** Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes 3 Transfer Type Interrupt Synch Type None Usage Type Data wMaxPacketSize 0x0040 1x 64 bytes bInterval 2 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 2 bAlternateSetting 0 bNumEndpoints 1 bInterfaceClass 3 Human Interface Device bInterfaceSubClass 1 Boot Interface Subclass bInterfaceProtocol 2 Mouse iInterface 4 Touchpad HID Device Descriptor: bLength 9 bDescriptorType 33 bcdHID 1.11 bCountryCode 0 Not supported bNumDescriptors 1 bDescriptorType 34 Report wDescriptorLength 52 Report Descriptors: ** UNAVAILABLE ** Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x84 EP 4 IN bmAttributes 3 Transfer Type Interrupt Synch Type None Usage Type Data wMaxPacketSize 0x0008 1x 8 bytes bInterval 8 Device Status: 0x0000 (Bus Powered) There is indeed an interrupt endpoint with address 0x81, but the driver defines bInterfaceProtocol = 2 (Mouse), and the endpoint in that interface is 0x84: #define BCM5974_DEVICE(prod) { \ .match_flags = (USB_DEVICE_ID_MATCH_DEVICE | \ USB_DEVICE_ID_MATCH_INT_CLASS | \ USB_DEVICE_ID_MATCH_INT_PROTOCOL), \ .idVendor = USB_VENDOR_ID_APPLE, \ .idProduct = (prod), \ .bInterfaceClass = USB_INTERFACE_CLASS_HID, \ .bInterfaceProtocol = USB_INTERFACE_PROTOCOL_MOUSE \ } where USB_INTERFACE_PROTOCOL_MOUSE = 2. My interpretation is that the driver is checking if the endpoint with address 0x81 form the interface with bInterfaceProtocol = 2 (that is the last interface of the list, the one with bInterfaceNumber = 2), but it is not found, because its only endpoint has a different address (0x84). Interestingly, 0x84 is the address given to the endpoint of the button interface. The button interface should not be relevant for Macbook 5,1 (TYPE 2 in the driver), according to 43f482b48d03 ("Input: bcm5974 - only setup button urb for TYPE1 devices"). If that is true, does anyone know why bInterfaceProtocol is always set to USB_INTERFACE_PROTOCOL_MOUSE, and why the driver works anyway with bEndpointAddress = 0x81 for the trackpad? The urb setup for 0x84 is only executed for TYPE 1 devices, and the mouse interface does not have an endpoint with address 0x81. Or am I missing something? We could revert the patch in question, but I see no reason why checking an expected interrupt endpoint should cause trouble. It looks like there is something fishy going on. Best regards, Javier Carrasco
