[XL] Change in coreboot[main]: soc/intel/ptl: Do initial Panther Lake SoC commit till ramstage

[Saurabh Mishra (Code Review)]

Attention is currently required from: Jérémy Compostella, Kapil Porwal, Pranava Y N, Subrata Banik.

Saurabh Mishra 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 72:

(15 comments)

File src/soc/intel/pantherlake/Makefile.mk:

https://review.coreboot.org/c/coreboot/+/83798/comment/c8b53de2_d2f42393?usp... :
PS71, Line 48: ramstage-y += soc_info.c
> can you please follow the order ?

Acknowledged

File src/soc/intel/pantherlake/acpi.c:

https://review.coreboot.org/c/coreboot/+/83798/comment/a33536af_a0da58b5?usp... :
PS71, Line 126: 	if (!initialized) {
> why not bail out early ? w/o need to give one extra tab? […]

Sure, refactored the code accordingly.

https://review.coreboot.org/c/coreboot/+/83798/comment/40ac1083_af3dfaf6?usp... :
PS71, Line 251:
> I believe it would fit in line above?

Acknowledged

https://review.coreboot.org/c/coreboot/+/83798/comment/8438ed70_7bfadd9a?usp... :
PS71, Line 262:
> why empty line?

Acknowledged

https://review.coreboot.org/c/coreboot/+/83798/comment/0ef47e9e_93d6863f?usp... :
PS71, Line 280:
> same

Acknowledged

https://review.coreboot.org/c/coreboot/+/83798/comment/4ce372ec_886869ff?usp... :
PS71, Line 286: V_P2SB_CFG_IBDF_FUNC
> this would fit into line above

Acknowledged

https://review.coreboot.org/c/coreboot/+/83798/comment/d2da9253_0e15003d?usp... :
PS71, Line 287: 	current += acpi_create_dmar_ds_msi_hpet(current,
              : 			0, V_P2SB_CFG_HBDF_BUS, V_P2SB_CFG_HBDF_DEV,
              : 			V_P2SB_CFG_HBDF_FUNC);
> try to reflow the line till 96 char

Acknowledged

https://review.coreboot.org/c/coreboot/+/83798/comment/0d5e5eb4_18bea2c4?usp... :
PS71, Line 296: 		current += acpi_create_dmar_rmrr(current, 0,
              : 				sa_get_gsm_base(), sa_get_tolud_base() - 1);
              : 		current += acpi_create_dmar_ds_pci(current, 0, PCI_DEV_SLOT_IGD, 0);
> in all previous code you have given one empty line between `acpi_create_dmar_drhd` and `acpi_create_ […]

Sure, i have now followed to keep a space after each <acpi_dmar_<>_fixup> call.

https://review.coreboot.org/c/coreboot/+/83798/comment/7b5206a3_944c0dc8?usp... :
PS71, Line 303:
> why empty line

Removed.

https://review.coreboot.org/c/coreboot/+/83798/comment/a0d36dc3_5afe2d7a?usp... :
PS71, Line 305: 
              : 		current += acpi_create_dmar_satc(current, ATC_REQUIRED, 0);
              : 			current += acpi_create_dmar_ds_pci(current, 0, PCI_DEV_SLOT_IGD, 0);
> same feedback as above

Removed.

File src/soc/intel/pantherlake/chip.h:

https://review.coreboot.org/c/coreboot/+/83798/comment/513a1721_95204472?usp... :
PS71, Line 60: 	TDP_45W = 45
> nit […]

Acknowledged

File src/soc/intel/pantherlake/chip.c:

https://review.coreboot.org/c/coreboot/+/83798/comment/28fd6580_83507479?usp... :
PS71, Line 80:
> please use consistent tab

Acknowledged

https://review.coreboot.org/c/coreboot/+/83798/comment/dd259e23_edef8ee3?usp... :
PS71, Line 202: 	if (CONFIG(SOC_INTEL_CSE_SEND_EOP_EARLY) ||
              : 	    CONFIG(SOC_INTEL_CSE_SEND_EOP_ASYNC)) {
> I hope it would fit within char < 96

Acknowledged

File src/soc/intel/pantherlake/include/soc/dptf.h:

https://review.coreboot.org/c/coreboot/+/83798/comment/8087ca71_4bac8b91?usp... :
PS71, Line 6:
> drop one empty line

Acknowledged

File src/soc/intel/pantherlake/include/soc/iomap.h:

https://review.coreboot.org/c/coreboot/+/83798/comment/c3e915c9_b87e5d44?usp... :
PS71, Line 23: DMI
>

Acknowledged