Bug 96771
Description
Yuxin Wu
2015-04-16 10:16:57 UTC
Yuxin, To best get help decoding the new format, providing all available information is needed. It could be a new usb structure, a new way of turning on multitouch, additional fields in the package, the wrong end node, etc etc. Please turn on the debug parameter in the bcm5974 module and provide the output of lsusb -vvv (as root) dmesg (for bcm5974 debug) That should get things going. Thanks, Henrik Created attachment 174241 [details]
lsusb & dmesg output
1. `sudo lsusb -vvv`
2. `dmesg` after bind the device to bcm5974 driver. The error is because the driver failed to find a config associate with the new id.
Hi Yuxin, From the USB HID descriptor it looks like the format /could/ be the same as the one you tried, TYPE3, but with fewer number of fingers per burst. The two attached patches might be worth a try. It think the reason you got the "length 8" usb error was that the interface was still bound to the hid core. Henrik Created attachment 174301 [details]
mbp12 support, bcm5974
Created attachment 174311 [details]
mbp12 support, hid
Hi Henrik, Thanks a lot for your help! I tried with the following: $ cd /sys/bus/usb/drivers/usbhid $ echo "1-5:1.2" > unbind $ modprobe -r bcm5974 $ insmod ./bcm5974.ko # a patched one Then I saw 1-5:1.2 automatically appears in /sys/bus/usb/drivers/bcm5974. But then the touchpad still doesn't function at all, and the only log I got from dmesg is "bad trackpad package, length: 8". Is this the correct way to test the driver? Do I also need to install the patched hid module somehow? Thanks, Created attachment 174981 [details] Output from hid-recorder Hi Henrik, Yuxin I've tried compiling a kernel with the two patches, and I can confirm that the driver binds to bcm5974, but that the dmesg log still just has "bad trackpad package, length: 8". Regardless of how many fingers are on the trackpad, or if it is depressed. I also tried doing the hid-replay thing you mentioned in https://bugzilla.kernel.org/show_bug.cgi?id=60181. I had to leave the usbhid driver bound for this, as I didn't see the hidraw device for the touchpad without it. I don't know if that invalidates the test. Anyway attached is the output from hid-recorder for the different tests proposed in the webpage. Thanks for all your effort. -John Created attachment 174991 [details] Dmesg long debug Also, don't know if this will help, but I've replicated the patch from https://bugzilla.kernel.org/attachment.cgi?id=106341 and attached the output from dmesg. -John Created attachment 175301 [details]
Initial support for multitouch in trackpad
This is my initial attempt at a patch for multitouch on the MBP12,1.
The trackpad on this model needs to be probed to get it to supply multitouch data. It can also be told to turn off the haptic feedback, though this isn't implemented, the main reason to do this is if you wanted to tell it to click yourself, so that you can do the force touch thing. I can look into this if there is a way of exposing this type of functionality. If not, we can simply not turn off the default haptic feedback.
After that the multitouch data that is send includes an extra pressure field per finger, so I had to create a different struct to cast over the data per finger. I'm also not currently reporting the finger pressure, though I could report it to synaptics as you currently do, but as there is a value per finger it is probably better to stick with that. The value seems to range between 0 and 1000. The mt driver supports pressure, so I just need to figure out what units it needs.
The main issue right now is that there is a narrow area on the left and right side of the trackpad, that if your finger initially lands into, then no matter where your finger moves the events aren't tracked. I'm not sure why, as I can see the events in report_tp_state. I'll keep looking into that.
Other than that, everything else seems to work.
John, Excellent work, congratulations. Regarding the patch, I see it can be substantially reduced by recognizing that the whole finger data block should be shifted two bytes in the original code, and four bytes in this version. All the zero cruft then lands at the very end of the finger struct, for both versions of the protocol. This shift works because the very first value, origin, is only used for the first contact in the array. I have long suspected that it is actually part of the header. With this change, and by making the contact block size a configuration parameter, it will be possible to seamlessly use the same structure for both versions, saving a lot of branching and code duplication. Thanks again, great work. Henrik Thanks Henrik, I agree with having the data block size be a parameter, though I'm not so sure about the origin belonging to the header rather than finger data. Each subsequent finger generates 30 bytes of data on my machine, or 28 in earlier models, so 28/30 bytes back from one finger data would still claim the origin value. Shall I do a rework of the patch to have a parameter? -John John, Thanks for continuing your work on this. Please make sure code duplication is kept at a minimum. Since the contact fields in use are all in the same sequential order, this should be readily doable. Thanks, Henrik Created attachment 175401 [details]
second version of patch for trackpad on MBP12,1
Hey Henrik,
Here is a reworking of my previous patch. Nothing new has been added, so I still have the bug where I have a weird area on the left and right side of the trackpad. And for now I left it reporting pressure data to synaptics, matching how the other trackpads work. Though as this trackpad seems to have very precise per finger data, it would probably be better to report that to the multitouch driver.
Is this more in line with what you want from the patch?
-John
*** Bug 96781 has been marked as a duplicate of this bug. *** Hi John, Yes, thank you, this is more like it. There were still some pieces to merge more seamlessly with the rest of the code, so I wrote a preparatory patch onto which a modified version of your patch could be applied. Do the three patches below work for you? In particular, is ABS_PRESSURE and ABS_TOOL_WIDTH still reported as expected? I am asking since you had the extra finger field inserted after the origin field before, which may need to be handled differently. If it works, I think we are almost there. Thanks, Henrik Created attachment 175471 [details]
0001 - preparations
Created attachment 175481 [details]
0002 - hid patch
Created attachment 175491 [details]
0003 - bcm5974 patch
Created attachment 175571 [details]
Debug log of finger not being picked up
Hey Henrik,
That mostly works, but there is a bug when you are first probing the device. I needed to set data[1] to 0x1 in order to get it to supply multitouch data.
I've also attached a log of the data I'm getting back for a finger movement(I just threw a dprintk into report_tp_state after you put the figer data into the index field) I just cast the origin field wherever it landed because the only place you were using it was in the report synaptics data, and I figure if we supplied the pressure data to the mt driver, then I'd never be using the origin data anyway, so what does it matter. So what ends up in origin now, mightn't be the same as what would have ended up there before.
The log attached is what is in each of the tp_finger fields in the order they appear in the struct (without the unused two). There is also a part for a finger movement that starts in the very left of the trackpad and moves around without ever getting tracked. Perhaps you can spot what is going wrong?
Thanks,
-John
I tried your patch John, on 3.19, after changing the input_mt_assign_slots call to use the four argument version per 3.19 header, but the mouse did not work at all. It also messed up the keyboard (hid-apple) any ideas? Hey sickvolo, I'm sorry to hear that. First I'd ask in what way the keyboard was messed up. I followed your patch on github from the ubuntu forum, and one difference between the two is that you marked the keyboard as ANSI, while I considered it ISO. The consequence of this should be that the ISO quirk was applied and swapped the grave and tilda keys. If this is the case then you can disable the quirk by echoing 0 into /sys/module/hid_apple/parameters/iso_layout. I know that isn't the best solution, but I just want to make sure that the keyboard isn't messed up beyond that. After that, I guess the next step might be to try and get some logs to see if we can figure out what is going wrong. So if I use lsusb I can see the trackpad device is attached to bus 1 (it should be the same for you, but just in case). Then I load the usbmon module and unload the bcm5974 one. Then I go to /sys/kernel/debug/usb/usbmon and I run "cat 1u | Co:1:005" and then load the bcm5974 module. What you should see is something like: ffff880225cc3240 2505029197 S Co:1:005:0 s 21 09 0302 0002 0002 2 = 0201 ffff880225cc3240 2505029326 C Co:1:005:0 0 2 > ffff88025a9f7780 2505057488 S Co:1:005:0 s 21 09 0302 0002 0002 2 = 0200 ffff88025a9f7780 2505057673 C Co:1:005:0 0 2 > ffff880225cc3d80 2505086025 S Co:1:005:0 s 21 09 0302 0002 0002 2 = 0201 ffff880225cc3d80 2505086156 C Co:1:005:0 0 2 > After that do "cat 1u | grep Ii:1:005:3" ffff880037b21840 2692037835 C Ii:1:005:3 0:1 76 = 02000000 00000000 75e22004 73324d0a 00010797 02000600 1e000000 00000100 ffff880037b21840 2692037840 S Ii:1:005:3 -115:1 526 < When you put 1 finger on the trackpad you should see it sending 76 bytes of data, and increasing by 30 bytes for each additional finger. At that point we can at least say that it is sending multitouch data, and there is a problem in the decoding of it. If I get that much, it should give us a very good place to start figuring out what went wrong. The patch was initially written against 3.19, so beyond having to change the input_mt_assign_slots, there is no reason why it shouldn't be able to work. -John Created attachment 177881 [details]
Fixes bug in bcm5964.c
Hi John and Henrik, I found a bug in bcm5964.c from what Henrik prepared. For TYPE4 Wellspring devices, data[1] stores the on/off switch instead of data[0]. Once I made this change multi-touch started working on Ubuntu 15.04 (kernel 3.19) (In reply to George Hilios from comment #22) > Created attachment 177881 [details] > Fixes bug in bcm5964.c Hello George, I tested your patch together with Henrik's hid patch, now my Macbook 2015's keyboard and trackpad works like a charm, thank you. Fedora 22 (kernel 4.0.0) so, Tested-by: Yang Hongyang <burnef@gmail.com> Hello George, Hongyang, Henrik, I can confirm that with George's patch and Henrik's patch, my MacBook Pro Retina 2015's keyboard and trackpad can work with multitouch event on Linux Kernel 4.1-rc5. Hope these patches can integrate to Linux Kernel soon :) Tested-by: Yen-Chin, Lee <coldnew.tw@gmail.com> Hi, Thanks a lot. As this bug reports now treats BOTH issues : * Keyboard Fn keys not working as device not recognized by hid-apple attachment 175481 [details]: (https://bugzilla.kernel.org/attachment.cgi?id=175481&action=diff) * Trackpad only working in single mouse click mode attachment 177881 [details]: (https://bugzilla.kernel.org/attachment.cgi?id=177881&action=diff) I propose to rename the bug to reflect "keyboard" and "trackpad". Is there anything that one could do to help getting these two fixes integrated into the kernel ? Tormen Please note that the keyboard is not completely fixed by attachment 175481 [details], non-ISO keyboards are detected as ISO.
I'd love to help out. I've never contributed to the Linux kernel before, so this process would be entirely new to me. Would you mind walking me through it or pointing me someplace that describes the steps? Created attachment 179161 [details]
Photo of my 2015 MacBook Pro keyboard (ordered as US international keyboard) - identified as 0x0273 !
(In reply to rekoil from comment #27) > Please note that the keyboard is not completely fixed by attachment 175481 [details] > [details], non-ISO keyboards are detected as ISO. Hi. About that .... :) What is the difference between ANSI and ISO ? I tried to google this yesterday and found this: https://apple.stackexchange.com/questions/106058/difference-between-us-qwerty-and-international-qwerty-apple-keyboards According to them: > The International keyboard (‘keyboard type’ = ISO) also has one more key than > the US keyboard (‘keyboard type’ = ANSI). I took a photo of my keyboard (ordered as US international keyboard). And I have a 0x0273. So it gets detected as an ISO. (which as I do have a key more (between Left-Shift and z), seems to be fine ??) When I type * on the key left beside the "1" I don't get the paragraph-sign, but a "`" (so the character of the key between Left-Shift and Z). * on the key between Left-Shift and Z, I get: a "\" So it is also NOT that the two keys are inversed. I did not yet try to load the keyboard as non-ISO. But the question is : Is the 0x0273 an ISO keyboard or not ? (see the attached picture). I applied both patches from comment #26 and I can confirm problems with trackpad and fn key are now fixed. My keyboard is identified as 0x0273 (German layout). Thank you so much for the hard work! > What is the difference between ANSI and ISO ? >[...] > When I type > * on the key left beside the "1" I don't get the paragraph-sign, but a > "`" (so the character of the key between Left-Shift and Z). > * on the key between Left-Shift and Z, I get: a "\" > > So it is also NOT that the two keys are inversed. > I did not yet try to load the keyboard as non-ISO. The purpose of the ISO quirk is to swap the keys labeled § and `. What symbols these keys are actually mapped to, is a product of that keymap you are using. If you are using one that gives you a \, then it sounds like you are using the wrong keymap for this keyboard. I'm using an English(UK,Macintosh) keymap, and there is also an English(Macintosh) map, these two are very similar except the £ symbol is above the 3 on the UK one and the §` are swapped. You should try one of these keymaps. You can also manually disable the quirk. Google "hid_apple iso_layout" and you will see how. > But the question is : Is the 0x0273 an ISO keyboard or not ? > (see the attached picture). It is, though personally I think the ISO quirk is a bit of a hack. It would probably be a better solution to invert the keys in the European Macintosh keymaps and drop the quirk from all but one or two ISO keyboards that it is currently not applied to. The keymaps have been in place for a long time though, so I don't think we would get much traction changing those, and as disabling the quirk is fairly trivial, I suspect for now, it should be applied to 0x273 keyboards. -John (In reply to John Horan from comment #32) Thanks a lot for your reply !! Given the fact that I do have the extra key between Left-Shift + "Z"-key and given your opinion : OK, so the "English international" keyboard ordered with Apple in Germany/Europe (USB identifier 0x0273) is a ISO layout (I saw today that lsusb also states : "bCountryCode 13 International (ISO)") :) > It is, though personally I think the ISO quirk is a bit of a hack. It would > probably be a better solution to invert the keys in the European Macintosh > keymaps and drop the quirk from all but one or two ISO keyboards that it is > currently not applied to. The keymaps have been in place for a long time > though, so I don't think we would get much traction changing those, and as > disabling the quirk is fairly trivial, I suspect for now, it should be > applied to 0x273 keyboards. > The purpose of the ISO quirk is to swap the keys labeled § and `. About the hid_apple.iso_layout parameter: I agree with your statement :) I tried 0 and 1 and for me the "`" key is outputted correctly (when pressing the key labled "`";)) only if I set the hid_apple.iso_layout parameter to 0. Which seems not very intuitive given the fact that I do have an ISO layout and the parameter is called iso_layout. The parameters description "Enable/Disable hardcoded ISO-layout of the keyboard." does also not help as with an ISO layout one would assume that the "hardcoded ISO layout" should be enabled. That I need to change this parameter seems like a bug, no ? To determine if this is just a problem of the keymap (I suppose the keymap maps the keycode to a keysym(bol) ?): * hitting the "`" labled key xev shows me: keycode 49 (keysym 0x60, grave) * hitting the "§" labled key xev shows me: keycode 94 (keysym 0x5c, backslash) The above is the case using hid_apple.iso_layout=0. > What > symbols these keys are actually mapped to, is a product of that keymap you > are using. If you are using one that gives you a \, then it sounds like you > are using the wrong keymap for this keyboard. I'm using an > English(UK,Macintosh) keymap, and there is also an English(Macintosh) map, > these two are very similar except the £ symbol is above the 3 on the UK one > and the §` are swapped. For now I am using: setxkbmap us altgr-intl, because for me I had an US international keyboard (I didn't realize that "us" == PC us ... of course ;)) ... and I love the altr-intl option(!). So I'll have a look at the layouts you suggested :) Tormen --------------------------- Sidenote: I collected all issues, fixes, quirks for my MacBookPro12,1 model in this nice Wiki here (in addition to the info that was already there - thanks!): https://wiki.archlinux.org/index.php/MacBook#Early_2015_13.22.2F15.22_-_Version_12.2Cx (In reply to John Horan from comment #32) > I'm using an > English(UK,Macintosh) keymap, and there is also an English(Macintosh) map, > these two are very similar except the £ symbol is above the 3 on the UK one > and the §` are swapped. Hi again, Could you possibly post your output of : setxkbmap -print -query ? To know exactly what your settings are :) ... I am not a keyboard specialist, but so far only saw that there are lots of options... e.g. the geometry applealu_iso. Tormen Got the patches (attachment 175481 [details] and 177881) working as is from on Ubuntu 15.04 kernel 3.19. Working great. Thank you very much for getting it to this stage!
Tried it on kernel 4.0.6, exactly the same compile/install procedure, but neither bcm5974 nor hid-apple are working. I see bcm5974 being registered by usbcore, but it's not attaching to an input device. As far as hid-apple goes I see no differences in syslog between the kernel 3.19 when it works, and kernel 4.0.6 when it does not.
Any ideas on why it would be different?
(In reply to sickvolo from comment #35) > Got the patches (attachment 175481 [details] and 177881) working as is from > on Ubuntu 15.04 kernel 3.19. Working great. Thank you very much for getting > it to this stage! > > Tried it on kernel 4.0.6, exactly the same compile/install procedure, but > neither bcm5974 nor hid-apple are working. I see bcm5974 being registered by > usbcore, but it's not attaching to an input device. As far as hid-apple goes > I see no differences in syslog between the kernel 3.19 when it works, and > kernel 4.0.6 when it does not. > > Any ideas on why it would be different? Hi. IMHO : There must be something else wrong (e.g. with your patch). Because I had the patches successfully applied to 4.0.5 and now applied them to 4.1.0. Tormen Created attachment 181421 [details] Working (4.0.5 and 4.1.0) patch for trackpad + keyboard Here is the patch that I successfully applied to kernel kernel 4.0.5 and kernel 4.1.0 (opensuse stable kernel from http://build.opensuse.org) without any problems (everything is working fine: Same result on 4.0.1 than on 4.0.5, than before on 3.19). I am using this patches with 4.1.0 and was using them with 4.0.6 before that. Works like a charm. Got it working on 3.19, 4.0.x and 4.1.2. Thank you for the nice work! Now, while the patch is making it upstream (if ever), I've created a stop gap on github - https://github.com/SicVolo (In reply to Sic Volo from comment #39) > Now, while the patch is making it upstream (if ever) For that someone needs to submit it properly upstream. Henrik, is it mergeable? I see there are sign offs missing... On 07/23/2015 07:05 PM, bugzilla-daemon@bugzilla.kernel.org wrote: > https://bugzilla.kernel.org/show_bug.cgi?id=96771 > > --- Comment #40 from Dmitry Torokhov <dmitry.torokhov@gmail.com> --- > (In reply to Sic Volo from comment #39) >> Now, while the patch is making it upstream (if ever) > > For that someone needs to submit it properly upstream. Henrik, is it > mergeable? > I see there are sign offs missing... Hi Dmitry, Yes, I wanted to wait to see if the ISO/ANSI issue would settle. It appears to be the case now, so things can progress. Thanks everyone for testing it out and reporting your findings here. The main points referring to the patches in comments 16-18 appears to be: 1. Fix the data[0] bug - should be data[1] in this case. 2. Leave the usbid-to-keyboard-layout mapping as is. That is, 0x0273 will be treated as an ISO keyboard. I will repack this and send it to you. Thanks, Henrik 05ac:0273 is the device identifier used in my laptop, and my layout is not ISO. To elaborate, my keyboard is the non-international US keyboard (no key between left-shift and Z, enter-key is on one row), ` and ~ is left of 1 in the number row. In order to have normal operation with the patches currently provided in this bugzilla I have to use the kernel option "hid_apple.iso_layout=0". This keyboard is identified as 05ac:0273. (In reply to rekoil from comment #42) > 05ac:0273 is the device identifier used in my laptop, and my layout is not > ISO. Hi. For me it is still not clear what ISO keyboard means. I also have a 0x0273: Bus 001 Device 003: ID 05ac:0273 Apple, Inc. And my keyboard looks like this : https://bugzilla.kernel.org/attachment.cgi?id=179161 So one can constate "ISO keyboard" does NOT mean: (a) That one has a key between left Shift and Z (because I have, rekoil does not and we are both 0x0273 (b) That the key left beside the "1" (above the TAB and below the ESC) is the `/~ key, because for rekoil this is the case, but not for me and we are both 0x0273 THIS DOES NOT MAKE SENSE, DOES IT ?! :( Tormen Created attachment 183501 [details] attachment-27747-0.html For the record, my keyboard (05ac:0273): http://cl.ly/c2UM On Thu, 23 Jul 2015 at 21:38 <bugzilla-daemon@bugzilla.kernel.org> wrote: > https://bugzilla.kernel.org/show_bug.cgi?id=96771 > > --- Comment #44 from Tormen <tormen@diplomail.ch> --- > (In reply to rekoil from comment #42) > > 05ac:0273 is the device identifier used in my laptop, and my layout is > not > > ISO. > > Hi. > > For me it is still not clear what ISO keyboard means. > > I also have a 0x0273: > Bus 001 Device 003: ID 05ac:0273 Apple, Inc. > And my keyboard looks like this : > https://bugzilla.kernel.org/attachment.cgi?id=179161 > > So one can constate "ISO keyboard" does NOT mean: > (a) That one has a key between left Shift and Z (because I have, rekoil > does not and we are both 0x0273 > (b) That the key left beside the "1" (above the TAB and below the ESC) > is > the `/~ key, because for rekoil this is the case, but not for me and we are > both 0x0273 > > > THIS DOES NOT MAKE SENSE, DOES IT ?! :( > > > Tormen > > -- > You are receiving this mail because: > You are on the CC list for the bug. > Ok, not so settled after all... So it is clear (and has been for some time) that the usb id encoding no longer is sufficient to determine the type of keyboard. However, this issue is quite separate from the one at hand. Can we agree to leave it for a later patchset? After all, somebody will have to look into it and find a good solution. Henrik Maybe posting output of lsusb -v of both keyboards will give us some additional data pointers we could use to differentiate? But I'd open a separate bug for that. I think it qualifies under "not fully functional" to be honest. Here is my output from lsusb -v: http://pastie.org/pastes/10308606/text John, all, Here is what appears to be the final version, applied to -rc3. Since the patches are not identical to what was tested, it would be great to get a confirmation that they work. Thanks, Henrik From ecbc4d55bb12e68099a385bbe00900a18dbcef3e Mon Sep 17 00:00:00 2001 From: Henrik Rydberg <rydberg@bitmath.org> Date: Thu, 23 Jul 2015 21:17:26 +0200 Subject: [PATCH 1/3] Input: bcm5974 - Prepare for a new trackpad generation With the advent of the Macbook Pro 12, we see a new generation of trackpads, capable of force sensoring and haptic feedback. This patch prepares for the new device by adding configuration data for the code paths that would otherwise look different. Almost-tested-by: John Horan <knasher@gmail.com> Signed-off-by: Henrik Rydberg <rydberg@bitmath.org> --- drivers/input/mouse/bcm5974.c | 132 ++++++++++++++++++++++++++---------------- 1 file changed, 81 insertions(+), 51 deletions(-) diff --git a/drivers/input/mouse/bcm5974.c b/drivers/input/mouse/bcm5974.c index b10709f..a596b9b 100644 --- a/drivers/input/mouse/bcm5974.c +++ b/drivers/input/mouse/bcm5974.c @@ -184,17 +184,37 @@ enum tp_type { }; /* trackpad finger data offsets, le16-aligned */ -#define FINGER_TYPE1 (13 * sizeof(__le16)) -#define FINGER_TYPE2 (15 * sizeof(__le16)) -#define FINGER_TYPE3 (19 * sizeof(__le16)) +#define HEADER_TYPE1 (13 * sizeof(__le16)) +#define HEADER_TYPE2 (15 * sizeof(__le16)) +#define HEADER_TYPE3 (19 * sizeof(__le16)) /* trackpad button data offsets */ +#define BUTTON_TYPE1 0 #define BUTTON_TYPE2 15 #define BUTTON_TYPE3 23 /* list of device capability bits */ #define HAS_INTEGRATED_BUTTON 1 +/* trackpad finger data block size */ +#define FSIZE_TYPE1 (14 * sizeof(__le16)) +#define FSIZE_TYPE2 (14 * sizeof(__le16)) +#define FSIZE_TYPE3 (14 * sizeof(__le16)) + +/* offset from header to finger struct */ +#define DELTA_TYPE1 (0 * sizeof(__le16)) +#define DELTA_TYPE2 (0 * sizeof(__le16)) +#define DELTA_TYPE3 (0 * sizeof(__le16)) + +/* usb control message mode switch data */ +#define USBMSG_TYPE1 8, 0x300, 0, 0, 0x1, 0x8 +#define USBMSG_TYPE2 8, 0x300, 0, 0, 0x1, 0x8 +#define USBMSG_TYPE3 8, 0x300, 0, 0, 0x1, 0x8 + +/* Wellspring initialization constants */ +#define BCM5974_WELLSPRING_MODE_READ_REQUEST_ID 1 +#define BCM5974_WELLSPRING_MODE_WRITE_REQUEST_ID 9 + /* trackpad finger structure, le16-aligned */ struct tp_finger { __le16 origin; /* zero when switching track finger */ @@ -213,8 +233,6 @@ struct tp_finger { /* trackpad finger data size, empirically at least ten fingers */ #define MAX_FINGERS 16 -#define SIZEOF_FINGER sizeof(struct tp_finger) -#define SIZEOF_ALL_FINGERS (MAX_FINGERS * SIZEOF_FINGER) #define MAX_FINGER_ORIENTATION 16384 /* device-specific parameters */ @@ -232,8 +250,17 @@ struct bcm5974_config { int bt_datalen; /* data length of the button interface */ int tp_ep; /* the endpoint of the trackpad interface */ enum tp_type tp_type; /* type of trackpad interface */ - int tp_offset; /* offset to trackpad finger data */ + int tp_header; /* bytes in header block */ int tp_datalen; /* data length of the trackpad interface */ + int tp_button; /* offset to button data */ + int tp_fsize; /* bytes in single finger block */ + int tp_delta; /* offset from header to finger struct */ + int um_size; /* usb control message length */ + int um_req_val; /* usb control message value */ + int um_req_idx; /* usb control message index */ + int um_switch_idx; /* usb control message mode switch index */ + int um_switch_on; /* usb control message mode switch on */ + int um_switch_off; /* usb control message mode switch off */ struct bcm5974_param p; /* finger pressure limits */ struct bcm5974_param w; /* finger width limits */ struct bcm5974_param x; /* horizontal limits */ @@ -259,6 +286,24 @@ struct bcm5974 { int slots[MAX_FINGERS]; /* slot assignments */ }; +/* trackpad finger block data, le16-aligned */ +static const struct tp_finger *get_tp_finger(const struct bcm5974 *dev, int i) +{ + const struct bcm5974_config *c = &dev->cfg; + u8 *f_base = dev->tp_data + c->tp_header + c->tp_delta; + + return (const struct tp_finger *)(f_base + i * c->tp_fsize); +} + +#define DATAFORMAT(type) \ + type, \ + HEADER_##type, \ + HEADER_##type + (MAX_FINGERS) * (FSIZE_##type), \ + BUTTON_##type, \ + FSIZE_##type, \ + DELTA_##type, \ + USBMSG_##type + /* logical signal quality */ #define SN_PRESSURE 45 /* pressure signal-to-noise ratio */ #define SN_WIDTH 25 /* width signal-to-noise ratio */ @@ -273,7 +318,7 @@ static const struct bcm5974_config bcm5974_config_table[] = { USB_DEVICE_ID_APPLE_WELLSPRING_JIS, 0, 0x84, sizeof(struct bt_data), - 0x81, TYPE1, FINGER_TYPE1, FINGER_TYPE1 + SIZEOF_ALL_FINGERS, + 0x81, DATAFORMAT(TYPE1), { SN_PRESSURE, 0, 256 }, { SN_WIDTH, 0, 2048 }, { SN_COORD, -4824, 5342 }, @@ -286,7 +331,7 @@ static const struct bcm5974_config bcm5974_config_table[] = { USB_DEVICE_ID_APPLE_WELLSPRING2_JIS, 0, 0x84, sizeof(struct bt_data), - 0x81, TYPE1, FINGER_TYPE1, FINGER_TYPE1 + SIZEOF_ALL_FINGERS, + 0x81, DATAFORMAT(TYPE1), { SN_PRESSURE, 0, 256 }, { SN_WIDTH, 0, 2048 }, { SN_COORD, -4824, 4824 }, @@ -299,7 +344,7 @@ static const struct bcm5974_config bcm5974_config_table[] = { USB_DEVICE_ID_APPLE_WELLSPRING3_JIS, HAS_INTEGRATED_BUTTON, 0x84, sizeof(struct bt_data), - 0x81, TYPE2, FINGER_TYPE2, FINGER_TYPE2 + SIZEOF_ALL_FINGERS, + 0x81, DATAFORMAT(TYPE2), { SN_PRESSURE, 0, 300 }, { SN_WIDTH, 0, 2048 }, { SN_COORD, -4460, 5166 }, @@ -312,7 +357,7 @@ static const struct bcm5974_config bcm5974_config_table[] = { USB_DEVICE_ID_APPLE_WELLSPRING4_JIS, HAS_INTEGRATED_BUTTON, 0x84, sizeof(struct bt_data), - 0x81, TYPE2, FINGER_TYPE2, FINGER_TYPE2 + SIZEOF_ALL_FINGERS, + 0x81, DATAFORMAT(TYPE2), { SN_PRESSURE, 0, 300 }, { SN_WIDTH, 0, 2048 }, { SN_COORD, -4620, 5140 }, @@ -325,7 +370,7 @@ static const struct bcm5974_config bcm5974_config_table[] = { USB_DEVICE_ID_APPLE_WELLSPRING4A_JIS, HAS_INTEGRATED_BUTTON, 0x84, sizeof(struct bt_data), - 0x81, TYPE2, FINGER_TYPE2, FINGER_TYPE2 + SIZEOF_ALL_FINGERS, + 0x81, DATAFORMAT(TYPE2), { SN_PRESSURE, 0, 300 }, { SN_WIDTH, 0, 2048 }, { SN_COORD, -4616, 5112 }, @@ -338,7 +383,7 @@ static const struct bcm5974_config bcm5974_config_table[] = { USB_DEVICE_ID_APPLE_WELLSPRING5_JIS, HAS_INTEGRATED_BUTTON, 0x84, sizeof(struct bt_data), - 0x81, TYPE2, FINGER_TYPE2, FINGER_TYPE2 + SIZEOF_ALL_FINGERS, + 0x81, DATAFORMAT(TYPE2), { SN_PRESSURE, 0, 300 }, { SN_WIDTH, 0, 2048 }, { SN_COORD, -4415, 5050 }, @@ -351,7 +396,7 @@ static const struct bcm5974_config bcm5974_config_table[] = { USB_DEVICE_ID_APPLE_WELLSPRING6_JIS, HAS_INTEGRATED_BUTTON, 0x84, sizeof(struct bt_data), - 0x81, TYPE2, FINGER_TYPE2, FINGER_TYPE2 + SIZEOF_ALL_FINGERS, + 0x81, DATAFORMAT(TYPE2), { SN_PRESSURE, 0, 300 }, { SN_WIDTH, 0, 2048 }, { SN_COORD, -4620, 5140 }, @@ -364,7 +409,7 @@ static const struct bcm5974_config bcm5974_config_table[] = { USB_DEVICE_ID_APPLE_WELLSPRING5A_JIS, HAS_INTEGRATED_BUTTON, 0x84, sizeof(struct bt_data), - 0x81, TYPE2, FINGER_TYPE2, FINGER_TYPE2 + SIZEOF_ALL_FINGERS, + 0x81, DATAFORMAT(TYPE2), { SN_PRESSURE, 0, 300 }, { SN_WIDTH, 0, 2048 }, { SN_COORD, -4750, 5280 }, @@ -377,7 +422,7 @@ static const struct bcm5974_config bcm5974_config_table[] = { USB_DEVICE_ID_APPLE_WELLSPRING6A_JIS, HAS_INTEGRATED_BUTTON, 0x84, sizeof(struct bt_data), - 0x81, TYPE2, FINGER_TYPE2, FINGER_TYPE2 + SIZEOF_ALL_FINGERS, + 0x81, DATAFORMAT(TYPE2), { SN_PRESSURE, 0, 300 }, { SN_WIDTH, 0, 2048 }, { SN_COORD, -4620, 5140 }, @@ -390,7 +435,7 @@ static const struct bcm5974_config bcm5974_config_table[] = { USB_DEVICE_ID_APPLE_WELLSPRING7_JIS, HAS_INTEGRATED_BUTTON, 0x84, sizeof(struct bt_data), - 0x81, TYPE2, FINGER_TYPE2, FINGER_TYPE2 + SIZEOF_ALL_FINGERS, + 0x81, DATAFORMAT(TYPE2), { SN_PRESSURE, 0, 300 }, { SN_WIDTH, 0, 2048 }, { SN_COORD, -4750, 5280 }, @@ -403,7 +448,7 @@ static const struct bcm5974_config bcm5974_config_table[] = { USB_DEVICE_ID_APPLE_WELLSPRING7A_JIS, HAS_INTEGRATED_BUTTON, 0x84, sizeof(struct bt_data), - 0x81, TYPE2, FINGER_TYPE2, FINGER_TYPE2 + SIZEOF_ALL_FINGERS, + 0x81, DATAFORMAT(TYPE2), { SN_PRESSURE, 0, 300 }, { SN_WIDTH, 0, 2048 }, { SN_COORD, -4750, 5280 }, @@ -416,7 +461,7 @@ static const struct bcm5974_config bcm5974_config_table[] = { USB_DEVICE_ID_APPLE_WELLSPRING8_JIS, HAS_INTEGRATED_BUTTON, 0, sizeof(struct bt_data), - 0x83, TYPE3, FINGER_TYPE3, FINGER_TYPE3 + SIZEOF_ALL_FINGERS, + 0x83, DATAFORMAT(TYPE3), { SN_PRESSURE, 0, 300 }, { SN_WIDTH, 0, 2048 }, { SN_COORD, -4620, 5140 }, @@ -549,19 +594,18 @@ static int report_tp_state(struct bcm5974 *dev, int size) struct input_dev *input = dev->input; int raw_n, i, n = 0; - if (size < c->tp_offset || (size - c->tp_offset) % SIZEOF_FINGER != 0) + if (size < c->tp_header || (size - c->tp_header) % c->tp_fsize != 0) return -EIO; - /* finger data, le16-aligned */ - f = (const struct tp_finger *)(dev->tp_data + c->tp_offset); - raw_n = (size - c->tp_offset) / SIZEOF_FINGER; + raw_n = (size - c->tp_header) / c->tp_fsize; for (i = 0; i < raw_n; i++) { - if (raw2int(f[i].touch_major) == 0) + f = get_tp_finger(dev, i); + if (raw2int(f->touch_major) == 0) continue; - dev->pos[n].x = raw2int(f[i].abs_x); - dev->pos[n].y = c->y.min + c->y.max - raw2int(f[i].abs_y); - dev->index[n++] = &f[i]; + dev->pos[n].x = raw2int(f->abs_x); + dev->pos[n].y = c->y.min + c->y.max - raw2int(f->abs_y); + dev->index[n++] = f; } input_mt_assign_slots(input, dev->slots, dev->pos, n, 0); @@ -572,32 +616,22 @@ static int report_tp_state(struct bcm5974 *dev, int size) input_mt_sync_frame(input); - report_synaptics_data(input, c, f, raw_n); + report_synaptics_data(input, c, get_tp_finger(dev, 0), raw_n); - /* type 2 reports button events via ibt only */ - if (c->tp_type == TYPE2) { - int ibt = raw2int(dev->tp_data[BUTTON_TYPE2]); + /* later types report button events via integrated button only */ + if (c->caps & HAS_INTEGRATED_BUTTON) { + int ibt = raw2int(dev->tp_data[c->tp_button]); input_report_key(input, BTN_LEFT, ibt); } - if (c->tp_type == TYPE3) - input_report_key(input, BTN_LEFT, dev->tp_data[BUTTON_TYPE3]); - input_sync(input); return 0; } -/* Wellspring initialization constants */ -#define BCM5974_WELLSPRING_MODE_READ_REQUEST_ID 1 -#define BCM5974_WELLSPRING_MODE_WRITE_REQUEST_ID 9 -#define BCM5974_WELLSPRING_MODE_REQUEST_VALUE 0x300 -#define BCM5974_WELLSPRING_MODE_REQUEST_INDEX 0 -#define BCM5974_WELLSPRING_MODE_VENDOR_VALUE 0x01 -#define BCM5974_WELLSPRING_MODE_NORMAL_VALUE 0x08 - static int bcm5974_wellspring_mode(struct bcm5974 *dev, bool on) { + const struct bcm5974_config *c = &dev->cfg; int retval = 0, size; char *data; @@ -605,7 +639,7 @@ static int bcm5974_wellspring_mode(struct bcm5974 *dev, bool on) if (dev->cfg.tp_type == TYPE3) return 0; - data = kmalloc(8, GFP_KERNEL); + data = kmalloc(c->um_size, GFP_KERNEL); if (!data) { dev_err(&dev->intf->dev, "out of memory\n"); retval = -ENOMEM; @@ -616,28 +650,24 @@ static int bcm5974_wellspring_mode(struct bcm5974 *dev, bool on) size = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0), BCM5974_WELLSPRING_MODE_READ_REQUEST_ID, USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE, - BCM5974_WELLSPRING_MODE_REQUEST_VALUE, - BCM5974_WELLSPRING_MODE_REQUEST_INDEX, data, 8, 5000); + c->um_req_val, c->um_req_idx, data, c->um_size, 5000); - if (size != 8) { + if (size != c->um_size) { dev_err(&dev->intf->dev, "could not read from device\n"); retval = -EIO; goto out; } /* apply the mode switch */ - data[0] = on ? - BCM5974_WELLSPRING_MODE_VENDOR_VALUE : - BCM5974_WELLSPRING_MODE_NORMAL_VALUE; + data[c->um_switch_idx] = on ? c->um_switch_on : c->um_switch_off; /* write configuration */ size = usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0), BCM5974_WELLSPRING_MODE_WRITE_REQUEST_ID, USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE, - BCM5974_WELLSPRING_MODE_REQUEST_VALUE, - BCM5974_WELLSPRING_MODE_REQUEST_INDEX, data, 8, 5000); + c->um_req_val, c->um_req_idx, data, c->um_size, 5000); - if (size != 8) { + if (size != c->um_size) { dev_err(&dev->intf->dev, "could not write to device\n"); retval = -EIO; goto out; Created attachment 183521 [details]
0001 - final
Created attachment 183531 [details]
0002 - final
Created attachment 183541 [details]
0003 - final
On a side-note: The ISO decision for me does not really matter, as in any case you'll need to select the right keyboard layout and I for my part did not yet find the keyboard layout that works out of the box (a 100%). localectl status System Locale: LANG=POSIX LC_CTYPE=en_US.UTF-8 VC Keymap: mac-us X11 Layout: us And I have tried on a tty console "loadkeys mac-uk", ..., ... but could not find any that would have provided the right output for my US international keyboard (see the photo). P.S.: If someone has a (out of the box) working keyboard layout setup, please share your "localectl status"! (In reply to Henrik Rydberg from comment #52) > Created attachment 183541 [details] > 0003 - final Thanks a lot !! I agree 100% about already getting the principal support into the kernel first. I anyway adapt the keyboard with xmodmap I will integrate the patches into my kernel: https://software.opensuse.org/package/kernel-desktop and test them :) Thanks a lot again :)) I just tested the new patches on top of *4.2.0-rc3*. The base system is Debian stretch with KDE 4. Now everything is working: - Function keys, special keys(back light, keyboard back light control, volume,mute) - Touchpad: tapping, scrolling, multitouch(right,middle click) - "<>" and "^°" keys are no longer switched (German keyboard) I also tested the old patches before and they were working, except for the two switched keys. My local is: localectl status System Locale: LANG=de_DE.UTF-8 VC Keymap: n/a X11 Layout: de X11 Model: pc105 Jochen, Thanks for testing. The patches have now been sent. The final patchset that you tested just changed one line that needed confirmation on the relevant hardware configuration. Everything else is identical to what everyone else already tried out. Hence, everyone replying here with a full name and email have been added as Tested-by. Thanks, Henrik Henrik, Thanks again for the work ! Just one thing I realized after Jochens reply: For me keyboard backlight never worked and does still (with latest patch-set) not work: find /sys -iname "*bright*" /sys/devices/pci0000:00/0000:00:02.0/drm/card0/card0-eDP-1/intel_backlight/brightness /sys/devices/pci0000:00/0000:00:02.0/drm/card0/card0-eDP-1/intel_backlight/max_brightness /sys/devices/pci0000:00/0000:00:02.0/drm/card0/card0-eDP-1/intel_backlight/actual_brightness /sys/devices/platform/applesmc.768/leds/smc::kbd_backlight/brightness /sys/devices/platform/applesmc.768/leds/smc::kbd_backlight/max_brightness /sys/module/i915/parameters/invert_brightness /sys/module/video/parameters/brightness_switch_enabled cat /sys/devices/platform/applesmc.768/leds/smc::kbd_backlight/brightness 0 cat /sys/devices/platform/applesmc.768/leds/smc::kbd_backlight/max_brightness 255 for a in 10 25 50 100 125 127 128 255; do echo "$a" > /sys/devices/platform/applesmc.768/leds/smc::kbd_backlight/brightness sleep 1 cat /sys/devices/platform/applesmc.768/leds/smc::kbd_backlight/brightness # $a # <<< returns $a ;) done But no keyboard backlight. No hardware defect, as under MacOS it's working. Should this be working ? Anything I could do further to test / debug ? Tormen Hi Tormen,
> Should this be working ?
> Anything I could do further to test / debug ?
Opening a /new/ bugzilla would help ;-)
Looking at the applesmc code, maybe they changed the backlight value range from
one byte to two bytes or something. It could be worth trying to write a number
to backlight_state[1] as well, for instance.
Replies on another ticket, please.
Henrik
.... damn it. Sorry, the above manual modification of the value in the brightness file DOES actually work ! I did not notice because there was too much ambient light ;) I really thought I would be able to see the keyboard backlight, but no :/ Although : The KDE function "reduce keyboard backlight" is not working out of the box, but manually modifying the value in : /sys/devices/platform/applesmc.768/leds/smc::kbd_backlight/brightness works! Is that then a KDE bug or could that be a bug in the driver that avoids KDE to modify the keyboard LED backlight ? Tormen @Tormen Keyboard backlight adjustment works perfectly for me in KDE out of the box. KDE 4.14.6 with Kubuntu 15.04, kernel 3.19 and up. And yes, less than 10% barely registers in any ambient light, but 100% is plenty bright. Try patching up to the latest, or submitting a separate bug for the appropriate module - applesmc. Running with this patch using an ANSI keyboard does *not* swap the tilde and grave keys (same key, shift and non-shift respectively, to the left of the "1") as others have said, but instead turns them into `>` and `<` respectively. No other keys appear to be changed from the glyphs printed on the keyboard itself. The touchpad patches have been merged in mainline and will be released in 4.2-rc5. For keyboard issues please open a separate issue. These patches absolutely break my system, and while I'm not 100% sure where the snag is, I think it might still be best to mention it. Firstly, I'm running a 13" rMBP with an ANSI keyboard. The following issues occur both with 4.1.3 (with the three attached patches applied) and the latest build off of git (which is, of course, 4.2-rc4 plus some): - Trackpad is completely dead. No input at all from it. On 4.1.3-ARCH as provided by Arch Linux's core repository, the trackpad at least works as a basic mouse. - Tilde key is switched to < and > (modified by the Shift key) On the bright side, the FN key seems to work as it should. I've got xf86 input drivers for evdev, libinput, synaptics, and mtrack all installed, so SOMETHING out of that cocktail of drivers should be able to detect the trackpad. Instead I have no mouse movement at all until I plug in a USB mouse. Interestingly, if I click the trackpad, I get haptic feedback (but no click event is sent). I'd be curious to see if anyone has any feedback on this; I'm definitely not ruling out PEBKAC here. (In reply to Josh Klar from comment #63) > These patches absolutely break my system, and while I'm not 100% sure where > the snag is, I think it might still be best to mention it. It worked on my macbook pro 12,2. I applied on 4.1.3. Now my trackpad and FN keys are working. (my trackpad is a bit sensitive though) However I had to manually fix ` and ~, I was getting < and > too. $ echo options hid_apple iso_layout=0 | sudo tee -a /etc/modprobe.d/hid_apple.conf I have just tried 4.2-rc5 (from aur mainline repository) on my 13" MacBookPro12,1 (Early 2015) and have had some problems similar to Josh Klar: The keyboard layout seems to be wrong by default? My ~ key now works as <. The trackpad is completely non-functional. Is there any information I can provide that will help diagnose this? On a positive note, the function keys seemed to work correctly. Created attachment 184641 [details]
dmesg of system where trackpad is broken
For what it's worth, here's the dmesg of my system running on 4.2-rc5 (from miffe's linux-mainline packages on Arch x64).
With the aforementioned modprobe options, my keyboard is fully functional (tilde key included). Trackpad is completely bricked and shows no signs of life at all (no xev events or mouse cursor movements).
Created attachment 185091 [details]
xorg config to fix broken trackpad on 4.2-rc6
The kernel driver is NOT the issue here (as I was starting to suspect given only us Arch folk had broken trackpads). I stole a config from the Arch BBS for Bluetooth Magic Trackpads (which, of course, is only half-relevant here since Bluetooth is broken on this device, too) to force all "Trackpad"s to be handled by synaptics.
The config, fixing the internal trackpad on rMBP 12,1 models, is attached. Should be located at `/etc/X11/xorg.conf.d/60-magictrackpad.conf`
I just tried out 4.2-rc6 from mainline repo on my 13in (12,1) and had no keyboard functionality at all. (In reply to Red from comment #68) > I just tried out 4.2-rc6 from mainline repo on my 13in (12,1) and had no > keyboard functionality at all. And 4.1 final with the same config works? (In reply to Dmitry Torokhov from comment #69) > (In reply to Red from comment #68) > > I just tried out 4.2-rc6 from mainline repo on my 13in (12,1) and had no > > keyboard functionality at all. > > And 4.1 final with the same config works? No same results. At boot up I get dropped in a recovery shell "Warning /lib/modules/4.2.0-rc6-mainline/modules.devname not found -ignoring starting version 224" Then it hangs at finding the root drive "ERROR Unable to find root device UUID=xxxx" Should the fact it can't find my root drive keep it from loading the drivers and cause loss of functionality to both my keyboard and ports? Switched to the fallback img and found the issue. changed mkinitcpio.conf HOOKS="base udev autodetect block modconf filesystems keyboard fsck" to HOOKS="base udev block autodetect modconf filesystems keyboard fsck" Test it in 4.2-r1, keyboard and mouse works except the force touch. However, I have one problem in my new Macbook pro 13" 2015, the left-up key under ESC has changed from ~ to < Could someone told me how to fix it? Thanks. (In reply to Peiding CHEN from comment #72) > Test it in 4.2-r1, keyboard and mouse works except the force touch. However, > I have one problem in my new Macbook pro 13" 2015, the left-up key under > ESC has changed from ~ to < Could someone told me how to fix it? Thanks. This is because the current driver doesn't detect whether the keyboard is ANSI or ISO correctly (I have the same problem, ANSI keyboard that gets detected as ISO), the solution is to manually set whether ISO mode should be enabled. If you read the previous messages there are instructions on how to force the correct mode at boot. Thanks(In reply to Adrian Bjugård from comment #73) > (In reply to Peiding CHEN from comment #72) > > Test it in 4.2-r1, keyboard and mouse works except the force touch. > However, > > I have one problem in my new Macbook pro 13" 2015, the left-up key under > > ESC has changed from ~ to < Could someone told me how to fix it? Thanks. > > This is because the current driver doesn't detect whether the keyboard is > ANSI or ISO correctly (I have the same problem, ANSI keyboard that gets > detected as ISO), the solution is to manually set whether ISO mode should be > enabled. > > If you read the previous messages there are instructions on how to force the > correct mode at boot. Thanks. I am sorry to skip that part. two fingers (right click) sensitivity seems not good. In OSX, I can easily push down two fingers to get right click. but in Linux, it should be very accurate at the same time and the distance of two fingers should not be too far. Is there a way to adjust it? Thanks |