Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36087 )
Change subject: soc/intel/tigerlake: Do initial SoC commit ......................................................................
Patch Set 10:
That doesn't mean we shouldn't review the Tiger Lake addition. And I think we all agreed already that a 8k LOC patch isn't reviewable. And of course, if we had more common code already, it would be much easier... and we would have a better code base if 3ee54bb hadn't slipped in. Let's not repeat that error.
Sure, i believe Arthur has done great work of taking pain to review this CL. we have address majority of his feedback and few more is there. will continue to address also will see if we can submit this CL into 4 part
- Bootblock (soc/intel/tigerlake/bootblock/)
- romstage (soc/intel/tigerlake/romstage/)
- ramstage (soc/intel/tigerlake/)
- acpi (soc/intel/tigerlake/acpi/)
I still don't think "this CL" is a good idea, even if split up. Any commit history that first copies stale code and then adapts it in later commits (instead of adding the adapted code step by step) will lose lots of information.
For instance, I assume some developers have already looked at the copied code and identified chunks that can stay the same. A copy first, adapt later commit history will not contain this information. It would contain details about what was changed, but not about what wasn't. OTOH, if the commit that adds (pieces of) unchanged code targets a single topic, and the commit message states that this was confirmed to work on the new platform, nobody will have to doubt if that was already checked. And the information will be in the Git history and can help future maintenance.