Attention is currently required from: Felix Singer, Benjamin Doron, Paul Menzel, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35523 )
Change subject: mb/acer: Add Acer Aspire VN7-572G ......................................................................
Patch Set 174:
(1 comment)
Patchset:
PS174:
What should be done before this change can be committed? I can write documentation if I have to.
There are no formal requirements like presence of documentation. Generally, the less you put into one commit the easier it is to get somebody to ack (+2) it. So if you plan on writing documentation you should definitely put it into a separate commit. Review doesn't scale linearly.
This commit is already pretty big as is. And it's not 100% tidy. For instance, what I spotted skimming through random files: * The commit message says vboot is untested but there are a lot of vboot references in the code. If some advanced feature is untested, please (re)move it. * There's explicit dead code, e.g. `#if 0`. Anything like that is a burden for the reviewer and future maintenance, it's unlikely to be acked. * There are comments that disagree with the code. For instance in `gpio.c` a lot of `// NC` but the code configures a native function. The commit doesn't have to be perfect, but it needs to be consistent with itself. If you want you can keep unfinished things in separate commits on Gerrit for future reference.
I know you put a lot of work into this, and so did reviewers AFAICS. So we definitely want to get it in! I suggest to trim it down a little to make it easier to digest in one go. Anything that is untested/unfinished or otherwise smells funny should be left for a later review.