Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35426 )
Change subject: mb/supermicro: convert x11ssh to variants-capable tree structure ......................................................................
Patch Set 2:
(5 comments)
Patch Set 2:
Patch Set 2:
Patch Set 2:
Please use lenovo/t530 as reference.
What _exactly_ do you mean?
If you mean the baseboard scheme: that would not be correct as there is no such. x11ssh and x11ssm and others are very similiar but are different boards. I used the Gigabyte boards' scheme.
Which gigabyte boards? The GA-H61M-S2PV? The scheme here looks a bit different, though.
Also naming the basefolder x11 seems to make no sense as there are 28 boards with that naming and at least one isn't compatible to x11ssh.
Then suggest a better naming, please. Naming it "x11ss" wouldn't fit, same problem. Naming it "x11ssh" would be wrong, too.
The Gigabyte boards have the name of the first board that was added. If I were to rename them, I thought of "ga-h61m-series" because they all have the H61 PCH.
For these boards, I think the PCH differs, but the socket is constant. So I'd probably use "x11-lga1150-series" or something like that.
Or if you can't find a name that everybody likes, just leave the current name as-is and cut out the bikeshedding :-)
https://review.coreboot.org/c/coreboot/+/35426/2/src/mainboard/supermicro/x1... File src/mainboard/supermicro/x11/Kconfig:
https://review.coreboot.org/c/coreboot/+/35426/2/src/mainboard/supermicro/x1... PS2, Line 45: default 0xb00000 if VBOOT Did this change on purpose?
https://review.coreboot.org/c/coreboot/+/35426/2/src/mainboard/supermicro/x1... PS2, Line 75: source "src/mainboard/supermicro/x11/variants/*/Kconfig" Is there a need for this? Now there will be more files to maintain :/
https://review.coreboot.org/c/coreboot/+/35426/2/src/mainboard/supermicro/x1... File src/mainboard/supermicro/x11/Kconfig.name:
https://review.coreboot.org/c/coreboot/+/35426/2/src/mainboard/supermicro/x1... PS2, Line 1: source "src/mainboard/supermicro/x11/variants/*/Kconfig.name" Is there a need for this either? Now there will be a lot of very short files :/
Plus this file usually shows which variants are in a mainboard, for instance.
https://review.coreboot.org/c/coreboot/+/35426/2/src/mainboard/supermicro/x1... File src/mainboard/supermicro/x11/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/35426/2/src/mainboard/supermicro/x1... PS2, Line 174: I'd suggest using tabs for this
https://review.coreboot.org/c/coreboot/+/35426/2/src/mainboard/supermicro/x1... PS2, Line 177: Otherwise, misalignments happen like this one