Bug 96771 - MacbookPro12,1 (Early 2015) touchpad is not fully functional
Summary: MacbookPro12,1 (Early 2015) touchpad is not fully functional
Status: RESOLVED CODE_FIX
Alias: None
Product: Drivers
Classification: Unclassified
Component: Input Devices (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: drivers_input-devices
URL:
Keywords:
: 96781 (view as bug list)
Depends on:
Blocks:
 
Reported: 2015-04-16 10:16 UTC by Yuxin Wu
Modified: 2017-04-22 12:21 UTC (History)
29 users (show)

See Also:
Kernel Version: 3.9+
Subsystem:
Regression: No
Bisected commit-id:


Attachments
lsusb & dmesg output (23.00 KB, application/gzip)
2015-04-17 02:22 UTC, Yuxin Wu
Details
mbp12 support, bcm5974 (2.01 KB, patch)
2015-04-17 21:36 UTC, Henrik Rydberg
Details | Diff
mbp12 support, hid (4.47 KB, patch)
2015-04-17 21:36 UTC, Henrik Rydberg
Details | Diff
Output from hid-recorder (5.78 KB, application/x-gzip)
2015-04-24 10:36 UTC, John Horan
Details
Dmesg long debug (6.65 KB, application/x-gzip)
2015-04-24 12:33 UTC, John Horan
Details
Initial support for multitouch in trackpad (13.66 KB, patch)
2015-04-29 19:03 UTC, John Horan
Details | Diff
second version of patch for trackpad on MBP12,1 (11.40 KB, patch)
2015-05-01 00:08 UTC, John Horan
Details | Diff
0001 - preparations (12.00 KB, patch)
2015-05-01 18:14 UTC, Henrik Rydberg
Details | Diff
0002 - hid patch (3.94 KB, patch)
2015-05-01 18:14 UTC, Henrik Rydberg
Details | Diff
0003 - bcm5974 patch (4.46 KB, patch)
2015-05-01 18:14 UTC, Henrik Rydberg
Details | Diff
Debug log of finger not being picked up (17.34 KB, text/plain)
2015-05-02 20:10 UTC, John Horan
Details
Fixes bug in bcm5964.c (14.42 KB, patch)
2015-05-26 03:03 UTC, George Hilios
Details | Diff
Photo of my 2015 MacBook Pro keyboard (ordered as US international keyboard) - identified as 0x0273 ! (104.69 KB, image/jpeg)
2015-06-08 17:02 UTC, Tormen
Details
Working (4.0.5 and 4.1.0) patch for trackpad + keyboard (12.90 KB, patch)
2015-06-29 23:31 UTC, Tormen
Details | Diff
attachment-27747-0.html (1.75 KB, text/html)
2015-07-23 19:50 UTC, Adrian Bjugård
Details
0001 - final (12.08 KB, patch)
2015-07-23 22:02 UTC, Henrik Rydberg
Details | Diff
0002 - final (3.92 KB, patch)
2015-07-23 22:03 UTC, Henrik Rydberg
Details | Diff
0003 - final (4.47 KB, patch)
2015-07-23 22:03 UTC, Henrik Rydberg
Details | Diff
dmesg of system where trackpad is broken (67.44 KB, text/x-log)
2015-08-11 02:47 UTC, Josh Klar
Details
xorg config to fix broken trackpad on 4.2-rc6 (138 bytes, text/plain)
2015-08-17 01:40 UTC, Josh Klar
Details

Description Yuxin Wu 2015-04-16 10:16:57 UTC
The MacbookPro early2015 model has a new touchpad device. The device (05ac:0273) is not supported by current bcm5974 multitouch driver.

By default, the device is associated with the generic 'usbhid' driver which drives the touchpad as a single-button mouse in functionality.

I unbind the device from usbhid via sysfs, and added the new vendor-id and product-id to the bcm5974 module via sysfs (/sys/bus/usb/drivers/bcm5974/new_id), then the driver fails with "could not read from device" and "mode switch failed".

I looked at the bcm5974.c and found that this is because 'bcm5974_get_config()' failed to find a proper config for this new device. I managed to make it return the lastest config (associated with USB_DEVICE_ID_APPLE_WELLSPRING8), and then the driver fails with "bad trackpad package, length: 8".

It looks like the new device has some new protocols. Can somebody fix this, or maybe share some ideas about the new protocol so I could try to make it work.
Comment 1 Henrik Rydberg 2015-04-16 17:50:12 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
Comment 2 Yuxin Wu 2015-04-17 02:22:07 UTC
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.
Comment 3 Henrik Rydberg 2015-04-17 21:35:27 UTC
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
Comment 4 Henrik Rydberg 2015-04-17 21:36:24 UTC
Created attachment 174301 [details]
mbp12 support, bcm5974
Comment 5 Henrik Rydberg 2015-04-17 21:36:50 UTC
Created attachment 174311 [details]
mbp12 support, hid
Comment 6 Yuxin Wu 2015-04-18 02:01:20 UTC
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,
Comment 7 John Horan 2015-04-24 10:36:37 UTC
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
Comment 8 John Horan 2015-04-24 12:33:43 UTC
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
Comment 9 John Horan 2015-04-29 19:03:37 UTC
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.
Comment 10 Henrik Rydberg 2015-04-29 19:40:36 UTC
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
Comment 11 John Horan 2015-04-29 20:35:31 UTC
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
Comment 12 Henrik Rydberg 2015-04-29 21:28:39 UTC
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
Comment 13 John Horan 2015-05-01 00:08:55 UTC
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
Comment 14 Doro Rose 2015-05-01 02:12:11 UTC
*** Bug 96781 has been marked as a duplicate of this bug. ***
Comment 15 Henrik Rydberg 2015-05-01 18:13:19 UTC
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
Comment 16 Henrik Rydberg 2015-05-01 18:14:09 UTC
Created attachment 175471 [details]
0001 - preparations
Comment 17 Henrik Rydberg 2015-05-01 18:14:28 UTC
Created attachment 175481 [details]
0002 - hid patch
Comment 18 Henrik Rydberg 2015-05-01 18:14:52 UTC
Created attachment 175491 [details]
0003 - bcm5974 patch
Comment 19 John Horan 2015-05-02 20:10:01 UTC
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
Comment 20 Sic Volo 2015-05-15 00:18:26 UTC
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?
Comment 21 John Horan 2015-05-15 10:35:09 UTC
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
Comment 22 George Hilios 2015-05-26 03:03:18 UTC
Created attachment 177881 [details]
Fixes bug in bcm5964.c
Comment 23 George Hilios 2015-05-26 03:04:50 UTC
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)
Comment 24 Yang Hongyang 2015-05-27 05:58:57 UTC
(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>
Comment 25 Yen-Chin, Lee 2015-05-28 13:28:45 UTC
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>
Comment 26 Tormen 2015-06-08 12:53:55 UTC
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
Comment 27 Adrian Bjugård 2015-06-08 13:41:47 UTC
Please note that the keyboard is not completely fixed by attachment 175481 [details], non-ISO keyboards are detected as ISO.
Comment 28 George Hilios 2015-06-08 16:22:11 UTC
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?
Comment 29 Tormen 2015-06-08 17:02:58 UTC
Created attachment 179161 [details]
Photo of my 2015 MacBook Pro keyboard (ordered as US international keyboard) - identified as 0x0273 !
Comment 30 Tormen 2015-06-08 17:03:30 UTC
(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).
Comment 31 Janez Urevc 2015-06-08 21:10:58 UTC
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!
Comment 32 John Horan 2015-06-09 11:03:28 UTC
> 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
Comment 33 Tormen 2015-06-17 09:34:33 UTC
(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
Comment 34 Tormen 2015-06-17 10:16:52 UTC
(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
Comment 35 Sic Volo 2015-06-29 23:13:13 UTC
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?
Comment 36 Tormen 2015-06-29 23:21:07 UTC
(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
Comment 37 Tormen 2015-06-29 23:31:18 UTC
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).
Comment 38 Janez Urevc 2015-06-30 08:53:36 UTC
I am using this patches with 4.1.0 and was using them with 4.0.6 before that. Works like a charm.
Comment 39 Sic Volo 2015-07-23 04:40:44 UTC
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
Comment 40 Dmitry Torokhov 2015-07-23 17:05:42 UTC
(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...
Comment 41 Henrik Rydberg 2015-07-23 18:39:31 UTC
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
Comment 42 Adrian Bjugård 2015-07-23 19:03:10 UTC
05ac:0273 is the device identifier used in my laptop, and my layout is not ISO.
Comment 43 Adrian Bjugård 2015-07-23 19:20:43 UTC
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.
Comment 44 Tormen 2015-07-23 19:38:33 UTC
(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
Comment 45 Adrian Bjugård 2015-07-23 19:50:35 UTC
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.
>
Comment 46 Henrik Rydberg 2015-07-23 19:58:37 UTC
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
Comment 47 Dmitry Torokhov 2015-07-23 20:39:18 UTC
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.
Comment 48 Adrian Bjugård 2015-07-23 21:19:39 UTC
I think it qualifies under "not fully functional" to be honest.

Here is my output from lsusb -v: http://pastie.org/pastes/10308606/text
Comment 49 Henrik Rydberg 2015-07-23 21:52:52 UTC
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;
Comment 50 Henrik Rydberg 2015-07-23 22:02:35 UTC
Created attachment 183521 [details]
0001 - final
Comment 51 Henrik Rydberg 2015-07-23 22:03:02 UTC
Created attachment 183531 [details]
0002 - final
Comment 52 Henrik Rydberg 2015-07-23 22:03:22 UTC
Created attachment 183541 [details]
0003 - final
Comment 53 Tormen 2015-07-24 07:37:31 UTC
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"!
Comment 54 Tormen 2015-07-24 07:47:56 UTC
(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 :))
Comment 55 jradmacher 2015-07-24 17:08:12 UTC
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
Comment 56 Henrik Rydberg 2015-07-24 17:41:13 UTC
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
Comment 57 Tormen 2015-07-26 14:23:30 UTC
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
Comment 58 Henrik Rydberg 2015-07-26 14:37:10 UTC
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
Comment 59 Tormen 2015-07-26 14:48:47 UTC
.... 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
Comment 60 Sic Volo 2015-07-26 17:19:39 UTC
@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.
Comment 61 Griffin Smith 2015-07-29 14:48:05 UTC
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.
Comment 62 Dmitry Torokhov 2015-07-30 21:31:16 UTC
The touchpad patches have been merged in mainline and will be released in 4.2-rc5. For keyboard issues please open a separate issue.
Comment 63 Josh Klar 2015-08-03 03:06:27 UTC
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.
Comment 64 Fabio Falci 2015-08-06 07:52:10 UTC
(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
Comment 65 Nathan Spaeth 2015-08-08 03:45:24 UTC
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.
Comment 66 Josh Klar 2015-08-11 02:47:24 UTC
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).
Comment 67 Josh Klar 2015-08-17 01:40:36 UTC
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`
Comment 68 Red 2015-08-17 12:10:25 UTC
I just tried out 4.2-rc6 from mainline repo on my 13in (12,1) and had no keyboard functionality at all.
Comment 69 Dmitry Torokhov 2015-08-17 21:55:15 UTC
(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?
Comment 70 Red 2015-08-17 22:26:18 UTC
(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?
Comment 71 Red 2015-08-17 23:16:50 UTC
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"
Comment 72 Peiding CHEN 2015-09-23 09:05:57 UTC
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.
Comment 73 Adrian Bjugård 2015-09-23 09:11:35 UTC
(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.
Comment 74 Peiding CHEN 2015-09-23 12:14:01 UTC
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.
Comment 75 Peiding CHEN 2015-09-30 15:09:37 UTC
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

Note You need to log in before you can comment on or make changes to this bug.