FrameworkComputer / EmbeddedController

Embedded Controller firmware for the Framework Laptop
BSD 3-Clause "New" or "Revised" License
950 stars 64 forks source link

Add ectool support for framework laptop #10

Closed razzius closed 1 year ago

razzius commented 2 years ago

Without this patch:

EmbeddedController $ make utils
  VERSION ec_version.h
  HOSTCC  util/ectool
  HOSTCC  util/lbplay
  HOSTCC  util/stm32mon
  HOSTCC  util/ec_sb_firmware_update
  HOSTCC  util/lbcc
  HOSTCC  util/ec_parse_panicinfo
  HOSTCC  util/cbi-util
  HOSTCC  util/uartupdatetool
EmbeddedController $ sudo ./build/bds/util/ectool led power red
Missing Chromium EC memory map.
Cannot find I2C adapter
Unable to establish host communication
Couldn't find EC

With this patch:

EmbeddedController $ make utils
  VERSION ec_version.h
  HOSTCC  util/ectool
  HOSTCC  util/lbplay
  HOSTCC  util/stm32mon
  HOSTCC  util/ec_sb_firmware_update
  HOSTCC  util/lbcc
  HOSTCC  util/ec_parse_panicinfo
  HOSTCC  util/cbi-util
  HOSTCC  util/uartupdatetool
EmbeddedController $ sudo ./build/bds/util/ectool led power red
EmbeddedController $

And the light changes.

This also fixes an issue where if you run ectool without sudo permission, it errors, leaving behind the lockfile; I had to add the release_gec_lock() in 2 places, so maybe there's a better way to do this.

The ectool support is all taken from https://github.com/DHowett/fw-ectool, which was published before this repository. I opened an issue with the lock file there: https://github.com/DHowett/fw-ectool/issues/1.

I have no idea if y'all are interested in this or if I'm going about it the right way :)

razzius commented 2 years ago

I opened this before seeing https://github.com/FrameworkComputer/EmbeddedController/pull/5 ! Both of these PRs make the ectool work with the framework laptop.

DHowett commented 2 years ago

@Razzius FYI the code in fw-ectool is a bit outdated compared to #5 :smile:

I should merge it back, of course, but I've been a bit busy with life things.

(Thanks!)

DHowett commented 2 years ago

FYI: I've replaced the original (fairly hacky :P) code in fw-ectool with the version I was trying to upstream in #5. It no longer requires --comm=fwk, as it's now another personality for the lpc driver.

DHowett commented 1 year ago

Since #5 merged, this PR has become obsolete -- but thanks for bringing it in @razzius :smile:

razzius commented 1 year ago

Thanks for circling back around @DHowett and for offering your patches!