[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <449417ca-aae1-4868-a96f-a99ac5d187d6@gmail.com>
Date: Mon, 4 Mar 2024 21:21:19 +0100
From: Javier Carrasco <javier.carrasco.cruz@...il.com>
To: Takashi Iwai <tiwai@...e.de>,
Javier Carrasco <javier.carrasco@...fvision.net>
Cc: Dmitry Torokhov <dmitry.torokhov@...il.com>, linux-input@...r.kernel.org,
linux-kernel@...r.kernel.org, regressions@...ts.linux.dev,
Henrik Rydberg <rydberg@...omail.se>, John Horan <knasher@...il.com>
Subject: Re: [REGRESSION] Missing bcm5974 touchpad on Macbooks
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
Powered by blists - more mailing lists