Attention is currently required from: Michał Żygowski, Jakub Czapiga, Karol Zmyslowski, Stefan Reinauer, Michal Zygowski.
Maciej Pijanowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/73934 )
Change subject: util/inteltool: Add support for Jasper Lake ......................................................................
Patch Set 9:
(14 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/73934/comment/7c841cf4_42dc4c32 PS9, Line 10: Documents: Intel Datasheet: 633935, rev. 006 Document number needs to be fixed
File util/inteltool/gpio_names/jasperlake.h:
https://review.coreboot.org/c/coreboot/+/73934/comment/217a95ea_0ac4abb7 PS9, Line 140: const char *const jasperlake_pch_community1_sys_names[] = { For example, ADL-P defines almost the same group as: "alderlake_pch_p_group_hvcmos_names". Please consult with both datasheets to see if we can come up with a better name.
https://review.coreboot.org/c/coreboot/+/73934/comment/1ab79bd0_31261e4d PS9, Line 190: const char *const jasperlake_pch_community1_jtag_names[] = { If you see at others, the naming convention is typically: "emmitsburg_group_jtag ", We can drop the "community1" here.
https://review.coreboot.org/c/coreboot/+/73934/comment/6815f62a_10ca95b6 PS9, Line 203: .display = "------- GPIO Group GPP_SYS_JTAG -------", As far as I can see, others uarchs typically use: "------- GPIO Group JTAG -------"
https://review.coreboot.org/c/coreboot/+/73934/comment/70119c4a_5d6b4de5 PS9, Line 281: "GSPI2_CLK_LOOPBK", "GSPI2_CLK_LOOPBK", This can probably be included in a group. See alderlake_p.h for example, as a reference.
https://review.coreboot.org/c/coreboot/+/73934/comment/675d3059_aeea8423 PS9, Line 292: const char *const jasperlake_pch_group_spi0_names[] = { "jasperlake_pch_group_spi_names" should be enough
https://review.coreboot.org/c/coreboot/+/73934/comment/099fce75_2454f1eb PS9, Line 305: .display = "------- GPIO Group GPP_H -------", This should be: "------- GPIO Group SPI -------"
https://review.coreboot.org/c/coreboot/+/73934/comment/2b00749b_adab7986 PS9, Line 312: "GSPI0_CLK_LOOPBK", "GSPI0_CLK_LOOPBK", Another instance that can probably be included in already existing group, and this group can be then removed.
https://review.coreboot.org/c/coreboot/+/73934/comment/7f54e21d_a3369f15 PS9, Line 317: .display = "------- GPIO Group GPP_B -------", The display name is incorrect, but it will be gone once we merge loopbacks into some group.
https://review.coreboot.org/c/coreboot/+/73934/comment/1b77bd7c_15274232 PS9, Line 359: const char *const jasperlake_pch_group_sys_comm2_names[] = { This should be "jasperlake_pch_group_cpu_names". Consult alderlake_h.h as an example.
https://review.coreboot.org/c/coreboot/+/73934/comment/368df303_55d1705f PS9, Line 378: .display = "------- GPIO Group GPP_SYS_COMM2 -------", "------- GPIO Group CPU -------"
https://review.coreboot.org/c/coreboot/+/73934/comment/cd17fc26_e180fe2f PS9, Line 384: const char *const jasperlake_pch_group_sys_pcie_names[] = { "jasperlake_pch_group_pcie_vgpio_names" fits probably better
https://review.coreboot.org/c/coreboot/+/73934/comment/59757cab_a0c43b31 PS9, Line 467: .display = "------- GPIO Group GPP_SYS_PCIE -------", "------- GPIO Group PCIe vGPIO -------"
https://review.coreboot.org/c/coreboot/+/73934/comment/e2d60a1d_81774e4d PS9, Line 473: const char *const jasperlake_pch_group_sys_vusb_names[] = { maybe "jasperlake_pch_group_vgpio_usb_names", or similar
-
Maciej Pijanowski (Code Review)