Attention is currently required from: Furquan Shaikh, Martin Roth, Duncan Laurie, Angel Pons, Karthik Ramasubramanian. Mathew King has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50005 )
Change subject: mb/google/mi: Rename guybrush mainboard to monkey_island ......................................................................
Patch Set 3:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/50005/comment/ff1c037d_50ffaa9d PS2, Line 7: guybrush
Personally, I would use the following structure: […]
We will have a book and a box reference. What kind of issues did you see on puff?
Patchset:
PS2:
We have 2 reference designs that will need seperate baseboards and I did not want copy zork having separate gpio files in the same directory. Seems that we should have a directory under baseboard for each reference board
I think it's okay to add two subdirectories under baseboard -- one for guybrush reference and second for other reference. There is also going to be common code which will be utilized by both the references too.
Now we have paths that look like variants/baseboard/guybrush/... and variants/guybrush. Looking at that a baseboard is not a variant itself so why is it under the variants directory?
baseboard is all the code that is provided for the reference design that can be utilized by a variant if it has the same hardware design as the baseboard. However, it still has the capability to perform any override if required. On the other hand, anything that is directly under mainboard/monkey_island/ is what is used by all variants -- mainly code flows that can query the variants/ to perform certain actions. Example: GPIO configuration is done by mainboard.c but it asks the variant code for the GPIO table. That is the reason that baseboard/ is kept under variants. It is the special common code that can be utilized by any variant if required.
This also keeps the layout consistent with all other Chrome OS boards which is easier for the partners to understand and use.
It still feels to me that baseboard belongs under the mainboard and not under variants and it has confused me why it was there. If this layout will confuse partners I have no problem changing it back.
-
Mathew King (Code Review)