Are there any plans to rebase the FW EC code against the more up to date google EC code ?

[jcdutton]

Are there any plans to rebase the FW EC code against the more up to date google EC code ?
I have looked at some of the FW EC code, and some of it is pretty low quality to be honest.
So, my first thought is maybe go through it and improve things but I then looked at the google EC code, and it seems to have already fixed most of the problems.
I compared it to this:
git clone https://chromium.googlesource.com/chromiumos/platform/ec

Now, that is not to say the google code is super high quality. For example, it also has no re-entrant locking around the port80 history ring buffer. it also will not display all port80 values on the console. For example, if 4 port80 values are set in quick succession, it only outputs one of those to the console, thus missing/dropping some.

[DHowett]

For what it's worth, it appears that Google operates the same way. When a program is shipped it gets a branch, and that branch does not typically take updates from main.

There was an effort early on to move hx20 (the Intel 11th Gen) to the new Zephyr-based EC over on Google's Gerrit instance, but it did not work properly and I suspect re-validating the entire program on a new EC would have been prohibitively expensive.

I am sure Framework would appreciate some pull requests that fix the code you've identified as "low quality".

But just a quick note...

not ... super high quality
no re-entrant locking

The embedded controller is single-threaded and cooperatively multitasked. I believe there is no reentrancy in this case because the incoming port80 signals through eSPI trigger an interrupt and subsequent task queuing of the one task that handles port80 IO.

[jcdutton]

Looking at the code it looks like port80 writes happen from multiple sources. the port80 value is treated in some places as a u8, other places a u16, u32 and int. Not particularly consistent. I have not checked the code, but some might be from normal code, and some might be from interrupt context, so that sort of potential problem does not look like it is addressed in the code yet.
Do you happen to know how many bits are used for port80 values coming from the BIOS, as opposed to other sources?
My view of the port80 stuff, is really that I wish to come up with a lookup table I can apply offline that takes any PORT80: XXXX value in the console logs and converts it into a meaningful message. I say offline, because I doubt there is enough space in the flash to contain all the meaningful message texts.
Another part I am looking to add is to add some message in the console log or panic log that shows:

1. Why the system shutdown (to catch some hint as to why some people are seeing unexpected shutdowns). I don't believe the laptop can shut down without the EC helping.
2. Why the system woke. I.e. was it a key press, lid open, WoL packet, or some other source.
3. Provide some way for the ec console log to appear in the Linux syslog and maybe increase the console log buffer size to be able to handle all the logs messages during a laptop power cycle.
4. implement the CCD "chan" command on ectool side. It is useful for filtering log messages.

[JohnAZoidberg]

Every time when we start a new project we merge new upstream code into our branch and keep merging during the early stages of that project. But, just like Google, after a while we have to lock the code down to avoid having to re-validate every single aspect of the system over and over.

That means after shipping we don't usually merge new upstream code because, of course while it can fix some bugs, it has the chance not only to introduce new bug but also change behavior.
Most users would like their system (firmware) to be stable and not behave differently after they bought it.

Regarding fixes from upstream, we do often cherry-pick fixes that we become aware of. However development upstream is happening extremely fast with a huge number of boards, it's easy for us to miss things. If you find fixes that we can cherry-pick, we'd love to take them in.