mirror of
https://github.com/Telecominfraproject/OpenCellular.git
synced 2026-01-08 16:41:55 +00:00
a6e82c3acd
keyboard_scan_enable() is called from several contexts. From a skim of
the code I found:
* keyboard_lid_change(), which is called from HOOK_LID_CHANGE
* enable_keyboard(), which is called from HOOK_CHIPSET_RESUME
* lidangle_keyscan_update(), which is called from motion_sense_task.
* check_for_power_off_event() which is called from power_handle_state()
which is called from chipset_task.
* power_button_interrupt(), which is an interrupt
* power_button_change_deferred(), which is a deferred function
So, ummm, it's probably not a good idea to do a read-modify-write of a
variable without any locking. ...and then to act on the resultant state
in various different contexts.
It's presumed that's just what happened to poor Julius. Julius found
himself in the unfortunate situation where he resumed his device (with
the power button, I believe) and that everything worked (including
reading the battery state and including the accelerometer) but the
keyboard didn't work. Now, it should be noted that Julius is a little
strange. Well, maybe he's not strange and maybe just the way he uses
his laptop is strange. He uses his veyron_minnie device as a smart
keyboard/trackpad. Said another way: it is in tablet mode but is docked
to an HDMI monitor, the screen is face flat on his table, and he uses
the builtin keyboard and trackpad. Nobody else that I know does this.
It's pretty darn cool, but I just don't think anyone else would think of
it. Anyway, that might have something to do with how he reproduced
this. ...or it might not. He does that a lot and hasn't seen the
problem before now.
Anyway, I managed to reproduce a number of problems similar to what poor
Julius saw by adding a 200ms sleep in keyboard_scan_enable() after we
read disable_scanning_mask but before we did anything to it (I skipped
the sleep if this happened to be one of those people who was calling
from interrupt).
Since there appears to be no spin_lock_irqsave() in the EC, let's just
have the EC use atomic operations to mess with its masks. Then we'll
leave all heavy lifting to the task.
This requires thinking through the task code a bit.
Conflicts:
common/keyboard_scan.c
...due to commit 6112f20679 ("common: keyboard_scan: Add items to
.bss.slow.") in ToT.
BRANCH=ToT
BUG=chrome-os-partner:48470
TEST=Poke a lot with power button and lid; NTF.
Change-Id: I61b906505100186b0ca2c48e7b1a7ffaaa8a7d3e
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/317896
Reviewed-by: Alec Berg <alecaberg@chromium.org>
Reviewed-by: Randall Spangler <rspangler@chromium.org>
(cherry picked from commit 98ab7484d331a78fced870b58b4d82e79e2e0f4e)
Reviewed-on: https://chromium-review.googlesource.com/318292