[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: Cliff Huang, 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 56:

(17 comments)

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

https://review.coreboot.org/c/coreboot/+/83798/comment/dccdcbc5_72c8a1b8?usp... :
PS55, Line 124: 		printk(BIOS_ERR, "Error: Configuration could not be retrieved.\n");
> shouldn't we return when we have detected a NULL pointer? you have identified a NULL pointer and nex […]

Added to return NULL.

https://review.coreboot.org/c/coreboot/+/83798/comment/ad0b2bca_14264325?usp... :
PS55, Line 148: 	}
> same

Added to return NULL.

https://review.coreboot.org/c/coreboot/+/83798/comment/0bde44b2_6eb9cb8a?usp... :
PS55, Line 161: 	}
> same

Added to return NULL.

https://review.coreboot.org/c/coreboot/+/83798/comment/601eeda1_a0418b53?usp... :
PS55, Line 235: DEBUG_WILL
> what you mean by `DEBUG_WILL` is this kind of macro ?

Not a macro, String to indicate debug prints, corrected to use "DEBUG:"

https://review.coreboot.org/c/coreboot/+/83798/comment/ebb87583_3e1c240a?usp... :
PS55, Line 240: DEBUG_WILL
> same?

Acknowledged

https://review.coreboot.org/c/coreboot/+/83798/comment/2ef81a7c_08e8b9ab?usp... :
PS55, Line 347: 	}
> why using NULL pointer w/o returning?

Added to use return NULL.

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

https://review.coreboot.org/c/coreboot/+/83798/comment/5f3007e6_a21f42cf?usp... :
PS55, Line 46: 4Xe25W
> this is 25W?

Corrected to 15W.

https://review.coreboot.org/c/coreboot/+/83798/comment/481d8d9d_6addd934?usp... :
PS55, Line 46: 	PTL_U_404_4_15W_CORE, /* 4 P-cores +0 E-cores + 4 LP E-cores + 4Xe25W*/
> one space after `+`

Acknowledged

https://review.coreboot.org/c/coreboot/+/83798/comment/0ad7e87a_0c261adc?usp... :
PS55, Line 49: PTL_H_484_45W_CORE
> n

You mean to say, this is not required?

https://review.coreboot.org/c/coreboot/+/83798/comment/02b580b3_90f6c754?usp... :
PS55, Line 68: PCI_DID_INTEL_PTL_H_ID_2
> as per EDS this is 45W SOC part bt you have added this as 25W SKU

Sure, i have updated the config to 45W.

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

https://review.coreboot.org/c/coreboot/+/83798/comment/ef7fc309_e881774d?usp... :
PS50, Line 43: 	PTL_U_404_15W_CORE,
             : 	PTL_H_484_25W_CORE,
             : 	PTL_H_484_45W_CORE,
> > Hi Subrata, 404 means =(4 P-cores +0 E-cores + 4 LP […]

Added.

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

https://review.coreboot.org/c/coreboot/+/83798/comment/58df723d_12042e69?usp... :
PS55, Line 167: 	}
> same , please return if NULL

Added to return NULL.

https://review.coreboot.org/c/coreboot/+/83798/comment/73cdba6a_28e62269?usp... :
PS55, Line 256: soc
> pcd as well ?

Sure, changed.

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

https://review.coreboot.org/c/coreboot/+/83798/comment/40fe6348_8a28838c?usp... :
PS55, Line 129: 	}
> same

Added return.

https://review.coreboot.org/c/coreboot/+/83798/comment/3ad46438_867f644a?usp... :
PS55, Line 171: 	}
> same

Added return.

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

https://review.coreboot.org/c/coreboot/+/83798/comment/9b314b30_fa0c1977?usp... :
PS55, Line 12:  soc_
> pcd

Ack,changed.

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

https://review.coreboot.org/c/coreboot/+/83798/comment/060ff668_e1374d28?usp... :
PS55, Line 47:  soc_
> pcd

Ack, changed.