search

system76 / ec

System76 Open Source Embedded Controller
GNU General Public License v3.0
318 stars 72 forks source link

Remove GPH7 on IT8587E #476

Closed crawfxrd closed 1 month ago

crawfxrd commented 2 months ago

On IT8587E, pin 3 is VBAT and not a GPIO pin.

Ref: Table 4-1. Pins Listed in Numeric Order (128-pin LQFP) Ref: Table 7-14. GPIO Alternate Function

leviport commented 1 month ago

Since I was testing this on my darp5 which suffers from the IT8587E hang without the latest commit on master, I was trying to build this in a convoluted way, and I don't think I was doing it quite right.

I was building a USB image from firmware-open, so the first thing I did was cd into ec and git checkout master && git pull. Next, I did a git cherrypick it8587e-gph7 to try and pull in this change. But then after flashing, I got EC lockups again when restoring my keymap with keyboard-configurator. I'm not really sure why though, since my git log looks about right:

commit 24fd877c348ecef1edc595df4ccc9804e192e795 (HEAD -> master)
Author: Tim Crawford <tcrawford@system76.com>
Date:   Fri Jul 5 09:07:03 2024 -0600

    Remove GPH7 on IT8587E

    On IT8587E, pin 3 is VBAT and not a GPIO pin.

    Signed-off-by: Tim Crawford <tcrawford@system76.com>

commit 716efd4eb50b5e364662705114b08b15cc254048 (origin/master, origin/HEAD)
Author: Tim Crawford <tcrawford@system76.com>
Date:   Wed Jul 24 13:48:53 2024 -0600

    kbscan: Work around IT8587E hang

    IT8587E is hanging when reading the keyboard matrix.

    - Increasing the delay does not fix it
    - Placing delays between KSO* writes does not fix it
    - Code generated by SDCC looks valid to me

    Rewriting it like this does fix it, although I don't know why.

    Signed-off-by: Tim Crawford <tcrawford@system76.com>

commit 984428b6a8df23a0589cd8dd9e60457d2016e8b8
Author: Tim Crawford <tcrawford@system76.com>
Date:   Thu Jul 18 16:27:54 2024 -0600

---[snip]---

I did rebuild and re-flash firmware-open in the same state one more time, just to make sure it wasn't a fluke, but I got the same lockup again.

Last, I reset ec to origin/master and rebuilt and re-flashed, and the EC lockups stopped. It doesn't seem like this pin should be interfering with the keyboard scanning functionality, so I'm guessing my git meddling is just messing things up.

Can we update this branch on current master? It'll bypass my need to tamper with git history and eliminate the Levi variable from the equation.

crawfxrd commented 1 month ago

Rebased on master.