Missing bcm5974 touchpad on Macbooks

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.

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""

I am just wondering if for some reason the check on interrupt type is wrong here.

Best regards,
Javier Carrasco

Right, there is the message.

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'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

Getting the output of lsusb would be awesome, thank you.

The bcm9547 driver has always made the assumption that the endpoint type is interrupt, and the expected output from lsusb would be something like

bEndpointAddress     0x81  EP 1 IN
bmAttributes         3
Transfer Type        Interrupt

which is what the reverted commit checks.

I don't have the specific piece of hardware the report mentions, but I triggered the probe with the endpoint type = interrupt and the check was fine i.e. the probe did not fail. That made me think that the endpoint type could be different, but I am dubious about that.

I will keep an eye on the bugzilla you linked, in case we get feedback quickly.

Best regards,
Javier Carrasco

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

The driver is naughty, it binds to the 3rd interface (bInterfaceNumber
2) but actually pokes into the 2nd interface with endpoint 0x84 without actually claiming it. Your check expects that the endpoint belongs to the interface that the driver binds to and thus fails.

Yes, the driver needs to claim both interfaces and when checking use the right one. I will revert the patch for now given that it causes regression and we can try fixing it again.

Thanks.

Dmitry

[TLDR: I'm adding this report to the list of tracked Linux kernel regressions; the text you find below is based on a few templates paragraphs you might have encountered already in similar form. See link in footer if these mails annoy you.]

Thanks for the report. To be sure the issue doesn't fall through the cracks unnoticed, I'm adding it to regzbot, the Linux kernel regression tracking bot:

#regzbot ^introduced 2b9c3eb32a699ac
#regzbot title Input: missing bcm5974 touchpad on Macbooks #regzbot duplicate: https://bugzilla.suse.com/show_bug.cgi?id=1220030
#regzbot ignore-activity

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) --
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
That page also explains what to do if mails like this annoy you.