Attention is currently required from: Cliff Huang, Kapil Porwal, Pranava Y N, Saurabh Mishra.
Subrata Banik has posted comments on this change by Saurabh Mishra. ( https://review.coreboot.org/c/coreboot/+/83798?usp=email )
Change subject: soc/intel/ptl: Do initial Panther Lake SoC commit till ramstage ......................................................................
Patch Set 50:
(18 comments)
File src/soc/intel/pantherlake/chip.h:
https://review.coreboot.org/c/coreboot/+/83798/comment/93aa4630_683e33fe?usp... : PS50, Line 408: }; during MTL, we had one useful UPD `lower_basic_mem_test_size` which is now not listed under PTL chip.h. Can you please check ?
Also, few more into the list (maybe you should consider adding remaining useful UPDs that existed into MTL but not present currently inside PTL as TODO for the parent bug)
File src/soc/intel/pantherlake/chip.c:
https://review.coreboot.org/c/coreboot/+/83798/comment/67e91df0_90b9b271?usp... : PS50, Line 12: #include <intelblocks/gpio.h> do we need this header ?
File src/soc/intel/pantherlake/chipset.cb:
https://review.coreboot.org/c/coreboot/+/83798/comment/d186dfed_963bfe3e?usp... : PS50, Line 10: can you please add a power limit entry for 25W SoC? we will be using PTL-UH for sometime
https://review.coreboot.org/c/coreboot/+/83798/comment/e927d0eb_1c6f606a?usp... : PS50, Line 15: can you please gather `tcc_offset` value as well and keep it here ?
https://review.coreboot.org/c/coreboot/+/83798/comment/21f260f2_f6cc9e9d?usp... : PS50, Line 119: shared_sram isn't this is still known as `pmc_shared_sram` ?
https://review.coreboot.org/c/coreboot/+/83798/comment/a4a4967f_efce3ca0?usp... : PS50, Line 127: heci1 what is the difference between HECI devices part of the `B: 0 / D: 19 / F: 0-2` vs `B: 0 / D: 22 / F: 0-5`. you have listed both as `HECI1` and `HECI_1`
File src/soc/intel/pantherlake/cpu.c:
https://review.coreboot.org/c/coreboot/+/83798/comment/afed64e4_c1a781b9?usp... : PS50, Line 143: disable_three_strike_error please check if below limitation still exists for PTL ESx and QSx samples ?
``` if (CONFIG(SOC_INTEL_METEORLAKE_PRE_PRODUCTION_SILICON)) disable_three_strike_error(); else disable_signaling_three_strike_event(); ```
File src/soc/intel/pantherlake/elog.c:
https://review.coreboot.org/c/coreboot/+/83798/comment/36a7b990_6a052e62?usp... : PS50, Line 44: use tab?
File src/soc/intel/pantherlake/include/soc/crashlog.h:
https://review.coreboot.org/c/coreboot/+/83798/comment/2dca1016_59de3f3b?usp... : PS50, Line 11: #define TEL_DVSEC_NEXT_CAP 0x2 same
https://review.coreboot.org/c/coreboot/+/83798/comment/6f965847_0237fe37?usp... : PS50, Line 21: #define CRASHLOG_POINTER_SIZE_FIELD_OFFSET 0x04 who is the consumer ?
File src/soc/intel/pantherlake/include/soc/systemagent.h:
https://review.coreboot.org/c/coreboot/+/83798/comment/a88443c3_4b0a0628?usp... : PS50, Line 53: DSM_BASE_ADDR_REG why we need this when we have below macro in common code systemagent.h
``` #define BDSM 0xb0 /* Base Data Stolen Memory */ ```
https://review.coreboot.org/c/coreboot/+/83798/comment/da333848_69c8221e?usp... : PS50, Line 54: DPR_REG same true for
``` #define DPR 0x5C /* DMA Protected Range Register */ ```
File src/soc/intel/pantherlake/p2sb.c:
https://review.coreboot.org/c/coreboot/+/83798/comment/3da14c0d_5a14449e?usp... : PS50, Line 53: soc as this is local SOC code, can you please check if you can rename `soc_p2sb_ops` to `pcd_p2sb_ops` ?
File src/soc/intel/pantherlake/pmutil.c:
https://review.coreboot.org/c/coreboot/+/83798/comment/86d0ac39_aa9da5ee?usp... : PS50, Line 32: same
https://review.coreboot.org/c/coreboot/+/83798/comment/a14a2776_680dc555?usp... : PS50, Line 64: same
https://review.coreboot.org/c/coreboot/+/83798/comment/16d35c38_19fabd46?usp... : PS50, Line 91: avoid empty line
https://review.coreboot.org/c/coreboot/+/83798/comment/64d7a8e9_23d70985?usp... : PS50, Line 249: return (uint16_t) ACPI_BASE_ADDRESS; ``` return (uint16_t)ACPI_BASE_ADDRESS; ```
File src/soc/intel/pantherlake/systemagent.c:
https://review.coreboot.org/c/coreboot/+/83798/comment/4e1bd035_0bf56947?usp... : PS50, Line 103: DSM_BASE_ADDR_REG `BDSM` is enough.