cb22 / macbook12-spi-driver

WIP input driver for the SPI touchpad / keyboard found in the 12" MacBook (MacBook8,1 + MacBook9,1)
GNU General Public License v2.0
300 stars 106 forks source link

Touchpad not initilized properly on MacBookPro13,3 #6

Closed roadrunner2 closed 7 years ago

roadrunner2 commented 7 years ago

Generally on boot the touchpad is unavailable; one or more 'sudo rmmod applespi && sudo modprobe applespi' will get it working, though. This seems to indicate some initialization issue. I was able to narrow things down a bit to lines 282 - 285 (sending the applespi_init_commands): if I put a loop around just those to run them 10 times, then I have a functioning touchpad on boot in about 50% of the cases. This obviously isn't a fix or even a usable workaround, of course.

Where did those magic applespi_init_commands come from? Any suggestions on how to proceed further here? Do we need to somehow trace the driver in MacOS?

cb22 commented 7 years ago

The magic command to perform the modeswitch came from monitoring what the Windows driver does and basically sending the same command. It actually does a whole host of other commands (such as getting the name and vendor of the device), but that seemed to be the one that switched it in to multitouch mode.

Here's how I got the Windows driver to dump the data it was sending over the bus. Each command does have a response associated with it.

It might work simply sending the modeswitch command repeatedly until receiving back the correct response. However, the Windows driver seems to not have to do such things, which leads me to believe there's something else (timing? additional reset commands?) at play.

Unfortunately, I don't have any deeper insight in to the protocol format at this point.

Dunedan commented 7 years ago

I also had lots of troubles with touchpad initialization. While I noticed that altering the touchpad initalization lead to no improvement for me I found something else what fixed the problem for me every time so far. That's simply:

rmmod spi_pxa2xx_platform && modprobe spi_pxa2xx_platform
Dunedan commented 7 years ago

The solution mentioned above is also only working the same percentage of time as reloading the applespi driver itself. :disappointed:

Another thing I noticed is that even while the touchpad is in that not properly initialized state, touches are somehow registered. They don't act as clicks, but at least @roadrunner2 appletb driver is capable of waking up the TouchBar again on such a press. I don't know if that's already known, but I thought it's worth mentioning it.

cb22 commented 7 years ago

@Dunedan Yeah, so what happens is the touchpad can operate in two modes; one providing only basic functionality, and other giving full multitouch functionality.

On boot, it is in the first mode, and we do a modeswitch to get it into the second. The way it reports movements is very different, but clicks are still the same. Hence the clicking still working - the modeswitch didn't complete successfully.

Unfortunately I've been very short on time, so I haven't been able to look in to this much.

roadrunner2 commented 7 years ago

The solution mentioned above is also only working the same percentage of time as reloading the applespi driver itself. 😞

@Dunedan I came to the same conclusion.

Unfortunately I've been very short on time, so I haven't been able to look in to this much.

@cb22 No problem, we all get busy at times. I think we need traces from a MacBookPro13,* machine, anyway, since this appears to be specific to those.

tudorbarascu commented 7 years ago

@roadrunner2 I have a 13,1 machine, I can try to provide traces from it if it's needed.

roadrunner2 commented 7 years ago

@tudorbarascu That would be excellent! I'd really like traces from both the initialization (this issue) as well as when pressing caps-lock (issue #3), if that's possible.

tudorbarascu commented 7 years ago

@roadrunner2 I'm installing windows right now and I hope to finish recording the traces in 1 hour max. If there's anything else I can do from Windows or if you want Teamviewer etc. access now would be a good time to say so as I will delete it afterwards. Thanks.

roadrunner2 commented 7 years ago

No, for the moment that will be all. But if can you wait with deleting till I've had I chance to look at and play with the traces, and confirm that I have enough info to get things working here, that would be useful - in case I realize I need more info or traces.

tudorbarascu commented 7 years ago

@roadrunner2 Sure, I'll keep windows a little longer.

Here's a zip file with the trace: https://www.dropbox.com/s/juv1r5yyggvib4w/macbook13-1.zip?dl=0 I saved the traces as evtx, as I suppose that's how you want them. I pressed caps lock multiple times. What should I do for the initialization part? Disable/enable the trackpad?

roadrunner2 commented 7 years ago

Thanks. Found a utility that can parse this stuff on Linux.

Unfortunately there are 3643 (non-empty) "events" in there, and 754 of them appear to send a buffer worth of data - I'm having a hard time figuring out which of those are related to what. First of all, I presume these are all SPI bus events only? Next, can you give me some hint as to which events are, say, related to the pressing the caps-lock key? (I can't imagine all 3600 of them are...)

As to the initialization, hmm, you may be right, disabling/enabling might be an easy way to capture the commands.

tudorbarascu commented 7 years ago

@roadrunner2 My e-mail is tudorbarascu at yahoo dot com . We can do a shared screen / audio to speed things up if you want and to keep the noise down from this thread.

From what I can see at #3 you need to look for the 1023 events,. I'll try and trace better.

mhkabir commented 7 years ago

@tudorbarascu @roadrunner2 Awesome work! I'm following this thread. :)

tudorbarascu commented 7 years ago

@roadrunner2 I've made 2 traces again that you can find here: https://www.dropbox.com/s/mf88wy797siwezx/macbook13-1_sec_log.zip?dl=0

One was made while pressing the caps lock multiple times and the other while enabling disabling the trackpad. I didn't had the option of disabling the Apple keyboard in Device Manager so.. I didn't capture it.

Regarding what's what, I sincerely don't know, but I followed the exact steps from #3 , meaning I captured the log from SPB-ClassExtension.

@mhkabir Cool

roadrunner2 commented 7 years ago

@tudorbarascu Thanks again. I'm starting to slowly grok the events and what they represent, and have basic caps-lock-led management working, though it's not reliable yet - will push when it's solid. After than I'll tackle the init sequence - I started with the led's, because they're a bit simpler and I had some idea of what to look for. In any case, it'll be a few more days before I can look into this further, but wanted to post an update here.

tudorbarascu commented 7 years ago

@roadrunner2 Really cool! I'm available for testing or for anything else you need! Keep up the good work! Thanks!

roadrunner2 commented 7 years ago

Just another quick update: I've got both initialization and caps-lock led working reliably now - yay! I'll push the fixes later this week (need to finish cleaning things up). For those curious: the issue was missing delays before the cs-toggle, and one too many cs-toggles - this should also obviate the need for reducing the frequency, something I've removed now.

tudorbarascu commented 7 years ago

@roadrunner2 whohooo! can't wait! much obliged!

cb22 commented 7 years ago

@roadrunner2 that's great! I remember being completely puzzled by how it needed to run at a lower speed to work - must have just been masking the symptoms.

roadrunner2 commented 7 years ago

@cb22 Would it be possible for you to send/attach/upload the DSDT from your MacBook? I got/get some info from there, and would like verify similar info is available on your model. For that matter, could you @tudorbarascu also attach yours? (aml or dsl is fine) TIA.

roadrunner2 commented 7 years ago

@cb22 Oh, and yes, I figured the lower speed introduced enough delay that it sometimes/more-often worked. But at least for writing (and on the MBP) it was not enough to make it reliable.

tudorbarascu commented 7 years ago

@roadrunner2 Sure, I suppose you want the original one:

dsdt.zip

cb22 commented 7 years ago

@roadrunner2 I uploaded a copy to the kernel Bugzilla a while back. Unfortunately I don't have my laptop available at the moment to take a clean DSDT dump; in the one I linked in, it was the modified one. (just had the OSDW change)

roadrunner2 commented 7 years ago

Thanks to both of you, @tudorbarascu and @cb22 - looks like the _DSM function for the SPIT device is identical in all DSDT's, so I can make use of that.

loligans commented 7 years ago

Is this issue still open?

I have a MacBook Pro 13,2 that you are welcome to use for any testing you want. I already have Windows 10 installed and I can put Linux on it too if need be. Let me know, I've been lurking on this repo for awhile now and am exciting to see it get so much attention.

Dunedan commented 7 years ago

Should be fixed since #14 got merged. I at least had no problem with touchpad initialization anymore since that with my MacBookPro13,2.

peterychuang commented 7 years ago

I'm not sure if this is the same bug, but on my MacBook Pro 14,1, the touchpad sometimes doesn't work on boot. sudo modprobe -r applespi && sudo modprobe applespi gets the touchadpad to work again, but then the keyboard backlight is turned off.

I'm using @roadrunner2's spi-properties branch at the moment, but this problem exists in other branches too.

roadrunner2 commented 7 years ago

@peterychuang The spi-properties branch requires you to be using a kernel with l1k's spi_properties_v3 patches (see #29). Instead, I'd recommend trying out my applespi-fixes branch and seeing if that helps.

peterychuang commented 7 years ago

@roadrunner2 yes I've patched the kernel with l1k's patchset. I used touchbar-driver-hid-driver branch before and had the same problems with initialisation.

Thanks for pointing to applespi-fixes, I'll give it a try when I have time to rebuild the kernel without the l1k's patches.

roadrunner2 commented 7 years ago

@peterychuang The original driver (which includes the applespi-fixes branch) should work even with the patched kernel, i.e. I'm pretty sure you don't need to rebuild your kernel just to try out the branch.

For everyone who is still seeing intermittent issues: please try my applespi-fixes branch - it contains one fix for initialization, though I'm not convinced it solves the flakiness some folks are reporting.

peterychuang commented 7 years ago

@roadrunner2 Unfortunately neither of the branches works on 4.13. I see you have a new branch for 4.13. Does that branch contain those fixes for initialisation?

roadrunner2 commented 7 years ago

@peterychuang Yes, it does. However, I've just added the 4.13 fixes to the applespi-fixes branch so just re-fetch that one.

peterychuang commented 7 years ago

@roadrunner2 Thanks very much, it's working now. I'll let you know if I still encounter initialisation problems after I've the chance to reboot the machine a few times.

peterychuang commented 7 years ago

@roadrunner2 Unfortunately I'm still having initialisation problems with the touchpad (also the wifi, but that's a problem for another time).

Let me know if there are anything you may need (logs, etc) to look into the issues, and thanks very much for your work, as always.

roadrunner2 commented 7 years ago

@peterychuang Thanks for the feedback. I'm actually not too surprised it didn't fix it - it does fix a real issue, just not this one.

As to logs etc, yes, please. Create a /etc/modprobe.d/applespi.conf with the following contents:

options appespi dyndbg="func applespi_sync_* +p"

Then rebuild your ramdisk and reboot, and finally send me the output from dmesg | grep applespi.

Alternatively, instead of creating the modprobe.d conf and rebuilding the ramdisk, add the following to your kernel command line:

applespi.dyndbg="+p"

However this will result in a lot of output, and you'll want to turn it off again with

echo 'module applespi -p' | sudo tee /sys/kernel/debug/dynamic_debug/control

(despite the docs claiming it works, adding appespi.dyndbg="func applespi_sync_* +p" to the kernel command line does not work for me, possibly due to some issue with modprobe).

peterychuang commented 7 years ago

@roadrunner2 The option does give a lot of ouput, so much that the dmesg output gets truncated pretty quickly. Since the problem doesn't occur frequent enough, I haven't got the chance to get the full log yet.

Without that kernel command line option, the following is the output of dmesg | grep applespi when the touchpad initialisation fails on 4.13-rc2. The only thing noteworthy are the two errors before applespi: modeswith done. Those two errors don't appear if the touchpad is initialised successfully.

[    1.682679] applespi: loading out-of-tree module taints kernel.
[    1.682985] applespi: No spi-master device found for device APP000D - waiting for it to be registered
[    1.682985] applespi: acpi-device probe done: APP000D
[    1.750538] applespi: Got spi-master device for device APP000D
[    1.751033] applespi: Error reading from device: -108
[    1.751034] applespi: Error writing to device: -108
[    1.751035] applespi: modeswitch done.
[    1.751066] applespi: spi-device probe done: spi-APP000D:00
[    1.751068] applespi: Added spi device spi-APP000D:00

Will try to get you the full log when I have more time to poke around.

roadrunner2 commented 7 years ago

@peterychuang Oh, sorry, I assumed you had checked your logs already, and since you hadn't mentioned any errors I assumed there weren't any. My bad - I should've asked.

Anyway, if this error always shows up when it fails to initialize, then I don't need the full traces. Error 108 is ESHUTDOWN, and looking at the sources it looks like that is returned when the associated spi controller is "running" (yet). Are there any other spi-related messages/errors in the log?

peterychuang commented 7 years ago

@roadrunner2 sorry about the confusion. I've only just discovered the errors quite recently.

I've just encountered one initialisation failure, and I don't see other errors specifically related to applespi other than those two lines. If it helps, I have a full dmesg output here, still without applespi.dyndbg="+p" option.

roadrunner2 commented 7 years ago

@peterychuang Thanks for the dmesg output. Right it looks like probably a race condition during initialization of the spi device. I'll take a closer look in a couple days.

roadrunner2 commented 7 years ago

@peterychuang I think I've found and fixed the issue (it was indeed a race). Please (re)pull my touchbar-driver-hid-driver branch and give it a whirl.

peterychuang commented 7 years ago

@roadrunner2 I've just hit one init problem with the touchpad, but since I'm playing with linux-next, it could be the problem with the kernel. Another odd thing with linux-next is that the machine always fails to register the first keystroke at boot. So when I type the passphrase to unlock the encrypted partition, I always have to type the first key of the passphrase twice. Not sure about mainline, but I don't recall having this problem with 4.12.

roadrunner2 commented 7 years ago

@peterychuang Did you see the init issue with my latest fix? If so, was there anything in the logs?

Regarding the missing first keystroke: that sounds a lot like an issue I've seen on an older laptop (whose internal keyboard uses regular usb) with an older kernel, so this may be something unrelated to this specific driver - does waiting a few seconds before typing help? (i.e. trying to understand if this a timing issue or really an issue with the first keystroke)

peterychuang commented 7 years ago

@roadrunner2 yes it was with your latest fix. In fairness I think it has been a lot more reliable. And also since I was playing with linux-next the past few days, I suppose there could be something wrong there.

As for log, I only took a quick glance, and it looked more or less the same, except these two lines:

applespi: Error reading from device: -108
applespi: Error writing to device: -108

became something like this:

applespi: Error writing to device: b3 b3 b3 b3

I'm going to swtich back to 4.12 for a few days and see how it goes. At least there's no first-keystroke problem on 4.12.

As always, thanks very much for your work!

Edit The first-key-stroke problem seems to have gone in 3 Aug's linux-next.

roadrunner2 commented 7 years ago

@peterychuang Good, so it looks like my patch indeed fixed the race you were seeing - unfortunately there seems to be another issue with the actual init messages now - that response indicates we're probably missing a delay somewhere.

I've just pushed an update to my branch with an extra delay during init - can you try that out?

peterychuang commented 7 years ago

@roadrunner2 I have tried the latest fixes for the past two or three days, and haven't encountered any initialisation problem so far, so it's all good now.

Thanks again for your work!

roadrunner2 commented 7 years ago

@peterychuang Thanks for testing! So we're on the right track.

However, I was looking at the various delays again, and noticed an inconsistency where we were adding delays between cs assert and read in a couple places, but not doing so during init. So I'm suspecting that that might be the actual culprit. I've pushed another update to my branch, which has this delay added for the flush-read during init too, but which makes the read-to-write delay you tested optional (disabled by default). Could I ask you to try this out (without the optional delay enabled) and see if this also fixes your init issues? Basically I'm trying to find some consistency in all these delays and hopefully make things a bit less brittle and ad-hoc.

peterychuang commented 7 years ago

@roadrunner2 just to confirm: the last commit message says "Compile with -DINIT_FLUSH_DELAY to enable this extra delay," so should I put CFLAGS=-DINIT_FLUSH_DELAY in the Makefile?

Thanks

roadrunner2 commented 7 years ago

Sorry, no, don't add that (yet): I'd like to see if the commit before that ("Always wait a bit between asserting cs and reading") by itself fixes things too (I probably should've just removed the last commit altogether). TIA.

peterychuang commented 7 years ago

@roadrunner2 thanks, I'll try the latest commit for a bit and see how it goes.

peterychuang commented 7 years ago

@roadrunner2 I've used the latest version (no flags added) for the past 2 days and haven't encountered any init problem so far.