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
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.
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
[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.]
On 04.03.24 09:35, Takashi Iwai wrote:
>
> 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
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.
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
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
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
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
On Mon, Mar 04, 2024 at 09:21:19PM +0100, Javier Carrasco wrote:
>
> 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?
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.
>
> 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.
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