Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31500 )
Change subject: inteltool: add 300 and C240 Series PCH ......................................................................
Patch Set 5:
(22 comments)
https://review.coreboot.org/#/c/31500/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31500/3//COMMIT_MSG@7 PS3, Line 7: Northbridge
s/Northbridge/PCH/ ? if any bridge, it would be a southbridge.
Done
https://review.coreboot.org/#/c/31500/3/util/inteltool/gpio_groups.c File util/inteltool/gpio_groups.c:
https://review.coreboot.org/#/c/31500/3/util/inteltool/gpio_groups.c@1197 PS3, Line 1197: VDD
VDD2
Done
https://review.coreboot.org/#/c/31500/3/util/inteltool/gpio_groups.c@1209 PS3, Line 1209: "GPP_A23", "ISH_GP5", "n/a", "n/a", "n/a",
missing GPIO_RSVD_0 (coreboot notion, see soc/intel/cannonlake/include/soc/gpio_soc_defs_cnp_h. […]
Done
https://review.coreboot.org/#/c/31500/3/util/inteltool/gpio_groups.c@1210 PS3, Line 1210: };
When all function 4 values are "n/a" you can leave it out and […]
Done
https://review.coreboot.org/#/c/31500/3/util/inteltool/gpio_groups.c@1222 PS3, Line 1222: VRALRERT
VRALERT#
Done
https://review.coreboot.org/#/c/31500/3/util/inteltool/gpio_groups.c@1243 PS3, Line 1243: "GPP_B23", "SML1ALERT#", "PCHHOT#", "n/a", "n/a",
missing GPIO_RSVD_1, GPIO_RSVD_2 […]
Done
https://review.coreboot.org/#/c/31500/3/util/inteltool/gpio_groups.c@1278 PS3, Line 1278: };
`func_count = 3` ?
Done
https://review.coreboot.org/#/c/31500/3/util/inteltool/gpio_groups.c@1291 PS3, Line 1291: SPI2
SPI1
Done
https://review.coreboot.org/#/c/31500/3/util/inteltool/gpio_groups.c@1292 PS3, Line 1292: I2C2SDA
I2C2_SDA
Done
https://review.coreboot.org/#/c/31500/3/util/inteltool/gpio_groups.c@1294 PS3, Line 1294: modem
MODEM
Done
https://review.coreboot.org/#/c/31500/3/util/inteltool/gpio_groups.c@1303 PS3, Line 1303: GSPI
GSPI2
Done
https://review.coreboot.org/#/c/31500/3/util/inteltool/gpio_groups.c@1334 PS3, Line 1334: "GPP_E12", "USB2_OC3#", "n/a", "n/a", "n/a",
`func_count = 3` ?
Done
https://review.coreboot.org/#/c/31500/3/util/inteltool/gpio_groups.c@1368 PS3, Line 1368: "GPP_F23", "DDPF_CTRLDATA", "n/a", "n/a", "n/a",
`func_count = 3` ?
Done
https://review.coreboot.org/#/c/31500/3/util/inteltool/gpio_groups.c@1369 PS3, Line 1369:
drop empty line here
Done
https://review.coreboot.org/#/c/31500/3/util/inteltool/gpio_groups.c@1383 PS3, Line 1383: DATRA2
DATA2
Done
https://review.coreboot.org/#/c/31500/3/util/inteltool/gpio_groups.c@1387 PS3, Line 1387: "GPP_G7", "SD_WP", "n/a", "n/a", "n/a",
coreboot treats these as a separate group (AZA) after G: GPIO_RSVD_3..10 […]
Done
https://review.coreboot.org/#/c/31500/3/util/inteltool/gpio_groups.c@1388 PS3, Line 1388: };
`func_count = 2`?
Done
https://review.coreboot.org/#/c/31500/3/util/inteltool/gpio_groups.c@1421 PS3, Line 1421: "GPP_H23", "TIME_SYNC0", "n/a", "n/a", "n/a",
`func_count = 2` ?
Done
https://review.coreboot.org/#/c/31500/3/util/inteltool/gpio_groups.c@1446 PS3, Line 1446: "GPP_I14", "M2_SKT2_CFG3", "n/a", "n/a", "n/a",
`func_count = 3` ? […]
Done
https://review.coreboot.org/#/c/31500/3/util/inteltool/gpio_groups.c@1468 PS3, Line 1468: "GPP_J11", "A4WP_PRESENT", "n/a", "n/a", "n/a",
`func_count = 3` ?
Done
https://review.coreboot.org/#/c/31500/3/util/inteltool/gpio_groups.c@1502 PS3, Line 1502: "GPP_K23", "IMGCLKOUT1", "n/a", "n/a", "n/a",
`func_count = 2` ?
Done
https://review.coreboot.org/#/c/31500/3/util/inteltool/gpio_groups.c@1524 PS3, Line 1524: "GPD11", "LANPHYPC", "n/a", "n/a", "n/a",
`func_count = 2` ?
Done