Attention is currently required from: Ashish Kumar Mishra, Dinesh Gehlot, Eran Mitrani, Felix Singer, Jakub Czapiga, Kapil Porwal, Subrata Banik, Tarun.
Saurabh Mishra has posted comments on this change by Saurabh Mishra. ( https://review.coreboot.org/c/coreboot/+/83354?usp=email )
Change subject: soc/intel/ptl: Do initial Panther Lake SoC commit till bootblock ......................................................................
Patch Set 15:
(19 comments)
File src/soc/intel/pantherlake/espi.c:
https://review.coreboot.org/c/coreboot/+/83354/comment/226f08ba_55c8edfb?usp... : PS14, Line 27: #if ENV_RAMSTAGE
I believe this is not required. […]
Acknowledged
File src/soc/intel/pantherlake/include/soc/iomap.h:
https://review.coreboot.org/c/coreboot/+/83354/comment/e77768d1_188334f4?usp... : PS14, Line 10: */
Please add a new line here.
Acknowledged
https://review.coreboot.org/c/coreboot/+/83354/comment/81e66d83_805fa979?usp... : PS14, Line 59: /*
need white space around
Acknowledged
File src/soc/intel/pantherlake/include/soc/iomap.h:
https://review.coreboot.org/c/coreboot/+/83354/comment/39febaca_9a6d24fb?usp... : PS8, Line 37: fc800000
Acknowledged […]
We can close this comment query? since no DMI3BAR for PTL Mobile.
https://review.coreboot.org/c/coreboot/+/83354/comment/d73b07f7_933b7488?usp... : PS8, Line 74: 0x3fff0800000
IOM should be anywhere of size 64KB. […]
Let me verify with FSP and come back on this.
File src/soc/intel/pantherlake/include/soc/pci_devs.h:
https://review.coreboot.org/c/coreboot/+/83354/comment/a916b8c9_a077f223?usp... : PS14, Line 9: #if !defined(__SIMPLE_DEVICE__)
Suggestion: […]
Keeping as it is, based on previous SOC platform.
https://review.coreboot.org/c/coreboot/+/83354/comment/37b531c0_4ff9bb4d?usp... : PS14, Line 165: #define PCI_DEVFN_PCIE7 _PCI_DEVFN(PCIE_1, 6)
Please fix spaces.
Acknowledged
https://review.coreboot.org/c/coreboot/+/83354/comment/54e5fc8d_742baa1b?usp... : PS14, Line 174:
need two more space
Acknowledged
https://review.coreboot.org/c/coreboot/+/83354/comment/e6387661_087d6f39?usp... : PS14, Line 211: #endif
Suggestion: For such nested endif, please add a comment for which #if, for better readability.
Added in line:219
https://review.coreboot.org/c/coreboot/+/83354/comment/d6d2c217_93c7f828?usp... : PS14, Line 219: #endif
Same as line 211 above.
Acknowledged
File src/soc/intel/pantherlake/include/soc/pcr_ids.h:
https://review.coreboot.org/c/coreboot/+/83354/comment/5ef84a87_916b088f?usp... : PS14, Line 6: /* : * Port ids : */
These multiline comments at file header follows is seen in earlier soc files as well, but if we use […]
Acknowledged
File src/soc/intel/pantherlake/include/soc/pm.h:
https://review.coreboot.org/c/coreboot/+/83354/comment/81d9557b_8b375837?usp... : PS14, Line 80: #define GPE_STS_RSVD GPE_STD
Do we need duplicating macros?
Hi Ashish, currently the macro is not used anywhere for PTL. Keeping it for future supports.
https://review.coreboot.org/c/coreboot/+/83354/comment/74843496_7aeaed9c?usp... : PS14, Line 97: #define PME_B0_EN_BIT 13
If this macro is unused anywhere else, we could remove this and follow same (1 << x) as earlier line […]
Hi Ashish, currently the macro is not used anywhere for PTL. Keeping for future extensions for wake support.
File src/soc/intel/pantherlake/include/soc/soc_info.h:
https://review.coreboot.org/c/coreboot/+/83354/comment/816a9884_dc999f8d?usp... : PS14, Line 1: /* SPDX-License-Identifier: GPL-2.0-only */
please give one new line
Acknowledged
https://review.coreboot.org/c/coreboot/+/83354/comment/03c2b562_11919f5d?usp... : PS14, Line 5: enum { : NOT_DETECTED = 0, : PTLU, : PTLH, : };
why needed ?
Removed and followed MTL refactoring for PTL. 81111: soc/intel/mtl: Improve functions in soc_info.c | https://review.coreboot.org/c/coreboot/+/81111
File src/soc/intel/pantherlake/soc_info.c:
https://review.coreboot.org/c/coreboot/+/83354/comment/844512fd_9a92c38e?usp... : PS14, Line 2:
Removed.
https://review.coreboot.org/c/coreboot/+/83354/comment/c8a499fd_14072bdd?usp... : PS14, Line 10: get_soctype
why do we need this ? which problem this new API will solve. […]
Removed.
https://review.coreboot.org/c/coreboot/+/83354/comment/1f36fa24_88c291bf?usp... : PS14, Line 69: printk(BIOS_DEBUG, "soc_info: max_pcie_clock:%d\n", pcie_clock);
why we need these prints ?
Acknowledged
https://review.coreboot.org/c/coreboot/+/83354/comment/2b98cb7b_e3bc0c90?usp... : PS14, Line 92: }
can you follow mtl soc_info. […]
Removed and followed MTL refactoring for PTL. 81111: soc/intel/mtl: Improve functions in soc_info.c | https://review.coreboot.org/c/coreboot/+/81111