Attention is currently required from: Maciej Pijanowski, Michał Żygowski, Jakub Czapiga, Stefan Reinauer, Michal Zygowski.
Karol Zmyslowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/73934 )
Change subject: util/inteltool: Add support for Jasper Lake ......................................................................
Patch Set 10:
(15 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/73934/comment/d77f6950_20c54d22 PS9, Line 10: Documents: Intel Datasheet: 633935, rev. 006
Document number needs to be fixed
Done
Patchset:
PS10:
Still not all of my comments were worked on in the patchset #10
Currently, I still apply changes, regarding pointed notes.
File util/inteltool/gpio_names/jasperlake.h:
https://review.coreboot.org/c/coreboot/+/73934/comment/b54b82e1_ad69fcfe 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 […]
Done
https://review.coreboot.org/c/coreboot/+/73934/comment/fe15eac4_937bb2ce PS9, Line 203: .display = "------- GPIO Group GPP_SYS_JTAG -------",
As far as I can see, others uarchs typically use: "------- GPIO Group JTAG -------"
Done
https://review.coreboot.org/c/coreboot/+/73934/comment/d14cf12c_d0bad2b9 PS9, Line 305: .display = "------- GPIO Group GPP_H -------",
This should be: "------- GPIO Group SPI -------"
Done
https://review.coreboot.org/c/coreboot/+/73934/comment/8b057a0b_1a982b58 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 […]
Ack
https://review.coreboot.org/c/coreboot/+/73934/comment/9bb63867_0b8ee9aa 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.
Ack
https://review.coreboot.org/c/coreboot/+/73934/comment/7b330469_c3c76c48 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.
Ack
https://review.coreboot.org/c/coreboot/+/73934/comment/9ba3d5c7_f9f89c57 PS9, Line 378: .display = "------- GPIO Group GPP_SYS_COMM2 -------",
"------- GPIO Group CPU -------"
Done
https://review.coreboot.org/c/coreboot/+/73934/comment/66e53b1b_da613f18 PS9, Line 384: const char *const jasperlake_pch_group_sys_pcie_names[] = {
"jasperlake_pch_group_pcie_vgpio_names" fits probably better
Ack WiP.
https://review.coreboot.org/c/coreboot/+/73934/comment/14364967_50295e72 PS9, Line 467: .display = "------- GPIO Group GPP_SYS_PCIE -------",
"------- GPIO Group PCIe vGPIO -------"
Done
https://review.coreboot.org/c/coreboot/+/73934/comment/6177e7ce_5d85c0b0 PS9, Line 473: const char *const jasperlake_pch_group_sys_vusb_names[] = {
maybe "jasperlake_pch_group_vgpio_usb_names", or similar
Done.
https://review.coreboot.org/c/coreboot/+/73934/comment/8aa4f756_53f660d3 PS9, Line 496: const char *const jasperlake_pch_group_dsw_names[] = {
This should be: "jasperlake_pch_group_gpd_names"
Ack Corrected.
https://review.coreboot.org/c/coreboot/+/73934/comment/8917a821_07da2413 PS9, Line 510: const struct gpio_group jasperlake_pch_group_dsw = {
This should be: jasperlake_pch_group_gpd
Ack Corrected.
https://review.coreboot.org/c/coreboot/+/73934/comment/a3682b63_b61a5642 PS9, Line 511: .display = "------- GPIO Group GPP_S -------",
This should be: "------- GPIO Group GPD -------:
Done
-
Karol Zmyslowski (Code Review)