Change in coreboot[master]: mb/teleplatforms/D4E4S16P8: Add new CRB teleplatforms/D4E4S16P8
Attention is currently required from: Dmitry Ponamorev, Mariusz Szafrański, Suresh Bellampalli, Vanessa Eusebio, Angel Pons, Michal Motyl, Patrick Rudolph, King Sumo. Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57194 )
Change subject: mb/teleplatforms/D4E4S16P8: Add new CRB teleplatforms/D4E4S16P8 ......................................................................
Patch Set 22:
(10 comments)
File src/mainboard/teleplatforms/D4E4S16P8/Kconfig:
https://review.coreboot.org/c/coreboot/+/57194/comment/43c8f227_3217672a PS19, Line 31: default "1.0.12"
I meant there is a visible choice in menuconfig named LOCALVERSION. […]
Done
File src/mainboard/teleplatforms/D4E4S16P8/Kconfig:
https://review.coreboot.org/c/coreboot/+/57194/comment/b96aa412_66b5e23c PS21, Line 17: string
Not needed since CB:56553
Done
https://review.coreboot.org/c/coreboot/+/57194/comment/0ee6e3c7_3db32d83 PS21, Line 21: string
Not needed since CB:56554
Done
https://review.coreboot.org/c/coreboot/+/57194/comment/d81a894e_dbfc1c99 PS21, Line 24: config MAINBOARD_VENDOR : string : default "teleplatforms"
Already set in `src/mainboard/teleplatforms/Kconfig`
Done
File src/mainboard/teleplatforms/D4E4S16P8/acpi/mainboard_pci_irqs.asl:
https://review.coreboot.org/c/coreboot/+/57194/comment/dde3909e_e09c7579 PS21, Line 3: board specific
CB:60102
Well it's coupled with devicetree.cb, so both should change then?
File src/mainboard/teleplatforms/D4E4S16P8/acpi/thermal.asl:
PS21:
If there are no plans to add more stuff here, I'd drop the file.
Done
File src/mainboard/teleplatforms/D4E4S16P8/acpi_tables.c:
https://review.coreboot.org/c/coreboot/+/57194/comment/fa546e84_a146466f PS21, Line 13: /* Disable USB ports in S5 */ : gnvs->s5u0 = 0; : gnvs->s5u1 = 0;
These values are only used with mainboard-specific SMI handlers and/or ACPI. […]
Done
https://review.coreboot.org/c/coreboot/+/57194/comment/26974e4a_a2a2d711 PS21, Line 17: /* TPM Present */ : gnvs->tpmp = 0;
Doesn't seem to be used anywhere in coreboot. Default is already zero, so I'd drop this.
Done
File src/mainboard/teleplatforms/D4E4S16P8/ramstage.c:
https://review.coreboot.org/c/coreboot/+/57194/comment/0f0e0e0c_250f392c PS21, Line 26: 0x56
EEPROM_ADDR
Done
File src/mainboard/teleplatforms/D4E4S16P8/romstage.c:
https://review.coreboot.org/c/coreboot/+/57194/comment/3ed2ea05_cbf70450 PS20, Line 437: {SOUTH_GROUP0_CTBTRIGINOUT,
Would be interesting to use the coreboot GPIO configuration mechanism instead, which scaleway/tagada […]
Followup, if it ever gets done like that.
-
Kyösti Mälkki (Code Review)