[XL] Change in coreboot[main]: mb/intel/ptlrvp: Add Intel Panther Lake RVP as copy of google/fatcat

[Jérémy Compostella (Code Review)]

Attention is currently required from: Bora Guvendik, Cliff Huang, Jamie Ryu, Paul Menzel, Zhixing Ma.

Jérémy Compostella has posted comments on this change by Jérémy Compostella. ( https://review.coreboot.org/c/coreboot/+/84564?usp=email )

Change subject: mb/intel/ptlrvp: Add Intel Panther Lake RVP as copy of google/fatcat
......................................................................

Patch Set 7:

(9 comments)

File src/mainboard/intel/ptlrvp/Kconfig:

https://review.coreboot.org/c/coreboot/+/84564/comment/29347cc2_647f83fb?usp... :
PS6, Line 2:
> forgot how we made UART changes for Windows RVP. […]

This Change List (CL) provides the foundational support for our PTLRVP 
board. I suggest that we consider such an addition in follow-up Change 
Lists (CLs).

https://review.coreboot.org/c/coreboot/+/84564/comment/305b67a3_38248db9?usp... :
PS6, Line 3: config BOARD_INTEL_PTLRVP_COMMON
> Can we add commented flag that we used to use fill_policy instead of fsp_param back?

Could you clarify? I do not comprehend your comment.

https://review.coreboot.org/c/coreboot/+/84564/comment/eb0e117d_01038377?usp... :
PS6, Line 5: 	select BOARD_ROMSIZE_KB_32768
> fatcat has CPU_INTEL_SOCKET_OTHER for SMBios changes.

This was added after e2ea7f22c6355d15515c049ca0dc4352173a0c01. This 
change will be incorporated the next time we sync up, as I do not want 
to void the validation we have performed on this Changelist (CL).

https://review.coreboot.org/c/coreboot/+/84564/comment/b1a5bc1e_617c1f46?usp... :
PS6, Line 69: if BOARD_INTEL_PTLRVP_COMMON
> do we want to remove BOARD_INTEL_PTLRVP_COMMON since we use board_id instead of variants? only keep  […]

I agree that `BOARD_INTEL_PTLRVP_COMMON` is not necessary for PTLRVP 
mainboard definitions. I decided to keep it in an attempt to keep the 
code and architecture as close as possible to the Fatcat board. It 
should help to smooth the integration of changes coming from Fatcat.

File src/mainboard/intel/ptlrvp/chromeos.fmd:

PS6:
> quit a lot of difference compared to recent fatcat. […]

Based on previous experience at the end of last year, I assume some of these changes, in particular the shift to 9 MB instead of 8 MB, were necessary to accommodate the firmware stored in the SI_ALL firmware region. I experimented this morning and it seems that this is not the case. Hence, I have aligned with the FatCat FMD.

File src/mainboard/intel/ptlrvp/variants/baseboard/ptlrvp/include/baseboard/ec.h:

https://review.coreboot.org/c/coreboot/+/84564/comment/b9184273_a4b6b927?usp... :
PS6, Line 78: #define EC_ENABLE_SYNC_IRQ      /* Enable tight timestamp / wake support */
> do we need these two when ISH enabled like Fatcat does?

You are correct, we probably do not. I removed them.

File src/mainboard/intel/ptlrvp/variants/baseboard/ptlrvp/include/baseboard/gpio.h:

https://review.coreboot.org/c/coreboot/+/84564/comment/65d2c7e6_bb6bde0d?usp... :
PS6, Line 13: #define EC_SYNC_IRQ	0
> should we remove these two defines? it is defined in src/mainboard/intel/ptlrvp/variants/ptlrvp/incl […]

Done

File src/mainboard/intel/ptlrvp/variants/ptlrvp/devicetree.cb:

PS6:
> why do we need this file?  We already have variants/baseboard/ptlrvp/devicetree.cb.

Done

File src/mainboard/intel/ptlrvp/variants/ptlrvp/gpio.c:

https://review.coreboot.org/c/coreboot/+/84564/comment/e5d19513_88b16b04?usp... :
PS6, Line 199: 	/* GPP_E07:     Not used */
> in fatcat GPIO, GPP_07 is configured for EC_SOC_INT_ODL unless ISH is enabled. […]

I assumed that on ptlrvp we are in the CONFIG_BOARD_GOOGLE_FATCAT and I therefore set it as NC. Am I missing something ?