Keyboard/trackpad not responsive on resume for Macbook 8,1 (12" 2015) #49
Comments
|
Were there any related errors in |
|
No error messages, both of those messages you mentioned appeared. Here's the output: https://gist.github.com/rquast/519ade5d36e2eac73b20fbdf29a4fbe8 |
|
Have tested to see if this worked after fixing #50 but still doesn't resume. |
|
Since there are no error messages, let's see what's going on with the initialization command. Just before suspending (assuming a working keyboard at this point), can you set the debug param to enable logging of some of the SPI packets: After resume you can turn off that logging by setting the debug param to 0 again. This should generate a moderate amount of output (a hexdump of each 256 byte packet, one packet for each key event plus a few others). Then provide the dmesg output for that, please. |
|
There's no debug output by the look. Is there something wrong there? root@rquast-MacBook:/var/log# cat /sys/module/applespi/parameters/debug https://gist.github.com/rquast/8dac9b6bc946ee82f8e4b27d13f4aab1 |
|
That's very odd! Did you type anything on the (internal) keyboard after enabling the debug logs? At the very least before the suspend any such key events should've gotten logged. |
|
Yes, there's output when typing a few keys and so on before suspend, but nothing in the suspend itself. https://gist.github.com/rquast/8559a388330ddecfd259f37160219e01 |
|
Don't know if it helps, but the keyboard doesn't light up again after resume either. Also, no packets captured after resume. |
|
Thanks for the info. Ok, I suspect we're running into a race here where a command is sent during shutdown but it's response is not received, and hence the write-outstanding flag is still set when we resume. Can you do the following:
--- a/applespi.c
+++ b/applespi.c
@@ -1459,6 +1459,10 @@ static int applespi_resume(struct device *dev)
struct applespi_data *applespi = spi_get_drvdata(spi);
acpi_status status;
+ applespi->cmd_msg_queued = false;
+ applespi->write_active = false;
+ applespi->read_active = false;
+
status = acpi_enable_gpe(NULL, applespi->gpe);
if (ACPI_FAILURE(status)) {
pr_err("Failed to re-enable GPE handler for GPE %d: %s\n",Again, please show me the dmesg output (whether this patch helps or not). |
|
Applied the patch, but had to manually do that because it's different from master (wrong line number). Unfortunately, no change. https://gist.github.com/rquast/d5e970fb4891714fabdaf4c8839de279 |
|
My apologies.. I didn't notice the change in the debug flag. Will re-run and paste the output. |
|
I don't think it gave any more information with the 0x507 flag: https://gist.github.com/rquast/b9dfbe14643729933fc0ba7852a9a663 |
|
Thanks. Sorry about the wrong line numbers - have some changes sitting in my tree and I figured So we're getting closer to the culprit. Can you apply this patch instead (the line numbers should be accurate now): --- a/applespi.c
+++ b/applespi.c
@@ -674,6 +674,10 @@ applespi_send_cmd_msg(struct applespi_data *applespi)
u16 crc;
int sts;
+ pr_info("drain=%d cmd_msg_queued=%d init_cmd_idx=%d\n",
+ applespi->drain, applespi->cmd_msg_queued,
+ applespi->init_cmd_idx);
+
/* check if draining */
if (applespi->drain)
return 0;
@@ -749,6 +753,7 @@ applespi_send_cmd_msg(struct applespi_data *applespi)
sts = applespi_async(applespi, &applespi->wr_m,
applespi_async_write_complete);
+ pr_info("applespi_async: sts=%d\n", sts);
if (sts != 0) {
pr_warn("Error queueing async write to device: %d\n", sts);And again show the dmesg output. Right now it's looking like One reason for this could be an issue with the SIST ACPI call - try adding this patch too: --- a/applespi.c
+++ b/applespi.c
@@ -617,9 +617,11 @@ static int applespi_enable_spi(struct applespi_data *applespi)
long long unsigned int spi_status;
/* Check if SPI is already enabled, so we can skip the delay below */
+ /*
result = acpi_evaluate_integer(applespi->sist, NULL, NULL, &spi_status);
if (ACPI_SUCCESS(result) && spi_status)
return 0;
+ */
/* SIEN(1) will enable SPI communication */
result = acpi_execute_simple_method(applespi->sien, NULL, 1); |
|
No luck, but here's the output from the new info you are logging: https://gist.github.com/rquast/7b9cfe53cfe1a94bd926ac7722e18dc3 |
|
Note: I don't have the earlier patch applied to this one. Worked from a fresh applespi.c with the new patches you provided. |
|
Thanks - this basically confirms that the issue appears to be with the spi device or driver, rather than a logic issue in this applespi driver: we're queueing an init message, but never get a completion. So the spi device or driver is not processing the messages. I'm sorry, but this is going to take some more back and forth to figure out. Here's another patch (I'll attach this one - I'm guessing that's easier to grab than copy pasting): |
|
No this is great! Thanks for helping like this. I am more than happy to take the time to keep working at it. Here is the output: https://gist.github.com/rquast/b8a579ce053776784cef76512c2a5eac |
|
@rquast Sorry for the delay - things were busy. Thank you for the output. That's helpful - the controller state looks fine, as far as what was printed. Here's an expanded version of the patch which will print some more details, especially from the low-level driver: resume-dbg-2.patch.txt - same steps as for the last one. Oh, forgot: this latest patch requires that you have the spi-pxa2xx.h header file for your kernel (either download just that, or get the full sources for the kernel) - and then adjust the first |
|
Has this issue been fixed? |
|
@shrijitsingh99 No, this issue is still open AFAIK. And yes, you could help by doing the following:
So far the issue is looking to be in the existing kernel code, rather than this driver, so the next steps will most likely involve patching the kernel proper to get more debug info from there. |
|
Trying to build the patched applespi module give me this make error I made some small changes to the file to fix the build error and then built and loaded the module. |
|
Apologies, should've tested the patch against the latest code. Here is an updated patch: Rather than building via dkms it might be easier to just build the module via |
|
Thanks for the info will use |
|
@shrijitsingh99 Thanks for running this and getting the output. It confirms what I stated earlier: the spi message is being queued in the spi core, but for some reason the actual device-specific driver is not sending the message (we never get a completion callback). So it looks to me like the SPI controller device, or its driver, are not being woken up correctly. @andy-shev I see you've done work on the Also: the resume appears to work fine on other MacBook(Pro)'s (though it's not been widely tested, so there may be issues on some others too, but it does work fine on MBP13,3 with a |
|
On Broadwell a different PCI driver is used, indeed. Though, the SPI host controller driver is still the same. We did a lot of self-test against SPI in PIO/DMA modes, so, I doubt is anything wrong there. The symptoms looks like you didn't get an interrupt on receiver side which might mean that SPI is working on MAC in half-duplex mode. I dunno if it's supported by SPI PXA2xx driver. |
|
@TheChatty (or anybody else interested): I was just wondering if for some reason the keyboard's SPI interface needs to be forcefully re-enabled after resume on the MB8,1 (from my reading of the DSDT it should already be re-enabling the SPI interface on resume, but...). Can you try this quick patch: --- a/applespi.c
+++ b/applespi.c
@@ -813,9 +813,11 @@ static int applespi_enable_spi(struct applespi_data *applespi)
unsigned long long spi_status;
/* check if SPI is already enabled, so we can skip the delay below */
+ /*
result = acpi_evaluate_integer(applespi->sist, NULL, NULL, &spi_status);
if (ACPI_SUCCESS(result) && spi_status)
return 0;
+ */
/* SIEN(1) will enable SPI communication */
result = acpi_execute_simple_method(applespi->sien, NULL, 1);If that doesn't work, then maybe this: --- a/applespi.c
+++ b/applespi.c
@@ -813,11 +813,16 @@ static int applespi_enable_spi(struct applespi_data *applespi)
unsigned long long spi_status;
/* check if SPI is already enabled, so we can skip the delay below */
+ /*
result = acpi_evaluate_integer(applespi->sist, NULL, NULL, &spi_status);
if (ACPI_SUCCESS(result) && spi_status)
return 0;
+ */
/* SIEN(1) will enable SPI communication */
+ result = acpi_execute_simple_method(applespi->sien, NULL, 0);
+ if (ACPI_FAILURE(result))
+ pr_err("SIEN(0) failed: %s\n", acpi_format_exception(result));
result = acpi_execute_simple_method(applespi->sien, NULL, 1);
if (ACPI_FAILURE(result)) {
pr_err("SIEN failed: %s\n", acpi_format_exception(result)); |
|
@roadrunner2: I tested your first and second patch and later even added info output that SIEN(0|1) get called even but to no avail. After resume the SPI transfers still time out. |
|
the same issue for me, Ubuntu 19.04, kernel 4.16.0-041600-generic, after resume - keyboard & touch doesnt work, tried the patches, alse the patch with SIEN(0|1), tried even multiple times calling applespi_enable_spi() and added some timeouts before SIEN(1), but still no luck: applied debug using please let me know if you need any other information! |
|
@denisix Looking at the dmesg.fail log, it looks like you are unloading |
|
@roadrunner2 I experimented in many different ways and it was configured in now I commented all these options, tried with the provided modifications of The other strange situation detected after system resume, when I tried to unload the module with attached logs: thank you! |
|
sorry, any update on this? |
|
@roadrunner2 Hi! I tried your suggested patch with the current source: diff --git a/drivers/input/keyboard/applespi.c b/drivers/input/keyboard/applespi.c
index eda1b23002b5..9e59dfa4f239 100644
--- a/drivers/input/keyboard/applespi.c
+++ b/drivers/input/keyboard/applespi.c
@@ -688,12 +688,20 @@ static int applespi_enable_spi(struct applespi_data *applespi)
unsigned long long spi_status;
/* check if SPI is already enabled, so we can skip the delay below */
+ /*
acpi_sts = acpi_evaluate_integer(applespi->sist, NULL, NULL,
&spi_status);
if (ACPI_SUCCESS(acpi_sts) && spi_status)
return 0;
+ */
/* SIEN(1) will enable SPI communication */
+ acpi_sts = acpi_execute_simple_method(applespi->sien, NULL, 0);
+ if (ACPI_FAILURE(acpi_sts)) {
+ dev_err(&applespi->spi->dev, "SIEN failed: %s\n",
+ acpi_format_exception(acpi_sts));
+ return -ENODEV;
+ }
acpi_sts = acpi_execute_simple_method(applespi->sien, NULL, 1);
if (ACPI_FAILURE(acpi_sts)) {
dev_err(&applespi->spi->dev, "SIEN failed: %s\n",Here is the dmesg output with and without applying the patch: I also tried to turn on the debugging as suggested: # echo 0x507 | tee /sys/module/applespi/parameters/debug
tee: /sys/module/applespi/parameters/debug: Permission deniedIs it still being done like that or is there another way? i also tried to apply the patch $ make -j $(nproc) && make -j $(nproc) modules_install
DESCEND objtool
CALL scripts/atomic/check-atomics.sh
CALL scripts/checksyscalls.sh
CHK include/generated/compile.h
CC [M] drivers/input/keyboard/applespi.o
In file included from ./include/linux/printk.h:7,
from ./include/linux/kernel.h:19,
from ./include/linux/list.h:9,
from ./include/linux/kobject.h:19,
from ./include/linux/of.h:17,
from ./include/linux/irqdomain.h:35,
from ./include/linux/acpi.h:13,
from drivers/input/keyboard/applespi.c:44:
drivers/input/keyboard/applespi.c: In function ‘applespi_send_cmd_msg’:
./include/linux/kern_levels.h:5:18: warning: format ‘%d’ expects argument of type ‘int’, but argument 3 has type ‘ktime_t’ {aka ‘long long int’} [-Wformat=]
5 | #define KERN_SOH "\001" /* ASCII Start Of Header */
| ^~~~~~
./include/linux/kern_levels.h:14:19: note: in expansion of macro ‘KERN_SOH’
14 | #define KERN_INFO KERN_SOH "6" /* informational */
| ^~~~~~~~
./include/linux/printk.h:420:9: note: in expansion of macro ‘KERN_INFO’
420 | printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~~~
drivers/input/keyboard/applespi.c:771:2: note: in expansion of macro ‘pr_info’
771 | pr_info("drain=%d cmd_msg_queued=%d\n",
| ^~~~~~~
drivers/input/keyboard/applespi.c: At top level:
drivers/input/keyboard/applespi.c:1925:10: fatal error: spi-pxa2xx.h: No such file or directory
1925 | #include "spi-pxa2xx.h"
| ^~~~~~~~~~~~~~
compilation terminated.
make[3]: *** [scripts/Makefile.build:271: drivers/input/keyboard/applespi.o] Error 1
make[2]: *** [scripts/Makefile.build:514: drivers/input/keyboard] Error 2
make[1]: *** [scripts/Makefile.build:514: drivers/input] Error 2
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1841: drivers] Error 2Unsure why it cannot find the I would be happy to assist in any way needed to get this to work on macbook 8,1 |
In newer kernels the header has been moved under include/linux/spi/ folder IIRC. |
|
Thanks! Have managed to compile it now with the following changes: Here is the dmesg log from suspend and resume: Maybe it already provides some insight. The third pr_info block sadly does not compile because of the following two errors: full log: applespi_compile_log.txt I guess the pxa-2xx code has changed in the meantime, but i am really new to kernel and C development so i have not been able to figure out how to fix it. |
Yes. code changed a lot lately. This one removed the custom pump transfer loop. Nevertheless, if you really want to debug SPI transfers, you need to enable SPI trace events (/sys/kernel/trace/events/spi/enable IIRC, but you may google for it) and collect corresponding events during the driver's resume/suspend cycle. Note, you may also enable / disable events programmatically from SPI client driver. |
|
Hi, are there any known workarounds for this issue currently? |
|
Hi, any ideas on this issue? Maybe we need to setup bounty for this issue? :) |
|
Still not working after resume in current kernels 😞 |
|
I'm posting my personal workaround here; Turn off suspend/resume feature and make the OS automatically shutdown if the laptop is folded without a power plug for a long time. It suits my use-case because I just want to prevent my Mac from draining too much battery in "suspended" state. Of course this workaround makes you lose all unsaved documents but it doesn't matter for me because I mostly use VSCode for editing texts and the editor is robust for sudden shutdown. Add an user to perform auto-shutdownPut an auto-shutdown scriptSave as Setup a cron entryMake it enable for cron-invoked processes to call
|
|
By the way, do you folks managed to use Bluetooth on Macbook8,1? |
|
Just to offer another workaround, you can use suspend-to-idle (s2idle, aka. S0ix) instead of deep suspend (S3) to avoid the broken keyboard and touchpad on resume. s2idle keeps more of the system running, so the power draw is higher, but at least you can use your computer afterwards :) On my macbook8,1 the difference is about 1.1% battery drain per hour with deep suspend and 1.9% with s2idle. To switch to s2idle, run this command and reboot:
(To undo the change, |
|
FWIW, I spent some time fiddling around with various ACPI commands and insmod/rmmod sequences, but nothing seems to bring the SPI channel back from the dead after an S3 resume. Any other ideas? |
The keyboard and trackpad don't resume after suspend for me. I am using Ubuntu 17.04 and have tried this with both the 4.10 kernel and the mainline 4.14 rc5 kernel that both have the same issue. The issue appears to have affected other models in the past: #30
The text was updated successfully, but these errors were encountered: