[L] Change in coreboot[main]: mb/intel/adlrvp: Add ADL-S DDR5 UDIMM 1DPC

[Maximilian Brune (Code Review)]

Attention is currently required from: Angel Pons, Felix Singer, Lean Sheng Tan, Martin L Roth, Paul Menzel, Subrata Banik, Tim Wawrzynczak.

Maximilian Brune has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/66545?usp=email )

Change subject: mb/intel/adlrvp: Add ADL-S DDR5 UDIMM 1DPC
......................................................................

Patch Set 24:

(8 comments)

File src/mainboard/intel/adlrvp/devicetree_s.cb:

https://review.coreboot.org/c/coreboot/+/66545/comment/73457982_863a6681 :
PS22, Line 2:
> move conflicting stuff to overridetrees, keep common stuff in devicetrees.

The issue is that I don't know yet what is common and what is conflicting. So should I put everything into this devicetree for now and only add an empty override tree for now?

File src/mainboard/intel/adlrvp/devicetree_s.cb:

https://review.coreboot.org/c/coreboot/+/66545/comment/4bce2f67_a647054d :
PS23, Line 4: 	register "usb2_ports[6]" = "USB2_PORT_MID(OC7)"   # USB3/2 Type A port7
            : 	register "usb2_ports[9]" = "USB2_PORT_MID(OC7)"   # USB3/2 Type A port10
            : 	register "usb2_ports[10]" = "USB2_PORT_MID(OC0)"  # USB3/2 Type A port11
            : 	r
> +1

Done

https://review.coreboot.org/c/coreboot/+/66545/comment/41563ad8_dcb9491b :
PS23, Line 9: # DDI_PORT_A: Combo PHY A # ADL-S RVP UDIMM 1DPC eDP1.4 Connector
            : 	# DDI_PORT_1: Combo PHY B # ADL-S RVP UDIMM 1DPC HDMI 1.4b CRLS
            : 	# DDI_PORT_2: Combo PHY C # ADL-S RVP UDIMM 1DPC DP1.4a Connector
            : 	# DDI_PORT_3: Combo PHY D # ADL-S RVP UDIMM 1DPC HDMI 2.0b ALS
            : 	# DDI_PORT_4: Combo PHY E # ADL-S RVP UDIMM 1DPC DP1.4a Connector
            : 	# Enable eDP in PortA #TODO test
            : 	#register "ddi_portA_config" = "1"
            : 	#[DDI_PORT_A] = DDI_ENABLE_HPD
            : 	register "ddi_ports_config" = "{
            : 		[DDI_PORT_1] = DDI_ENABLE_HPD,
            : 		[DDI_PORT_2] = DDI_ENABLE_HPD | DDI_ENABLE_DDC,
            : 		[DDI_PORT_3] = DDI_ENABLE_HPD,
            : 		[DDI_PORT_4] = DDI_ENABLE_HPD | DDI_ENABLE_DDC,
            : 	}"
> +1

Done

https://review.coreboot.org/c/coreboot/+/66545/comment/99ebc0ac_1e7d332f :
PS23, Line 33: 	[4] = 0,
             : 		[5] = 0,
> Remove, zero by default.

Done

https://review.coreboot.org/c/coreboot/+/66545/comment/50dc8bfd_babc8782 :
PS23, Line 24: register "sata_salp_support" = "1"
             : 	register "sata_ports_enable" = "{
             : 		[4] = 1,
             : 		[5] = 1,
             : 		[6] = 1,
             : 		[7] = 1,
             : 	}"
             : 
             : 	register "sata_ports_dev_slp" = "{
             : 		[4] = 0,
             : 		[5] = 0,
             : 		[6] = 1,
             : 		[7] = 1,
             : 	}"
> +1

Done

File src/mainboard/intel/adlrvp/variants/adlrvp_s_ddr5_udimm_1dpc/overridetree.cb:

https://review.coreboot.org/c/coreboot/+/66545/comment/ec001c26_b89dba35 :
PS23, Line 3: 	device domain 0 on
> Use tabs between the device name and the on keyword.

why? I have never seen that in any devicetree.

https://review.coreboot.org/c/coreboot/+/66545/comment/df307813_2a3f122d :
PS23, Line 23: 		device ref pcie5_1 off end
> Remove?

Done

https://review.coreboot.org/c/coreboot/+/66545/comment/24edec44_c353499e :
PS23, Line 60: #device ref pcie_rp9 on (doesn't work)
             : 		#	# PCIE x4 Slot 2 (Document 626352 is wrong here. it states that clk_src should be 16)
             : 		#	register "pch_pcie_rp[PCH_RP(9)]" = "{
             : 		#		.clk_src = 14,
             : 		#		.clk_req = 14,
             : 		#		.flags = PCIE_RP_CLK_REQ_DETECT,
             : 		#	}"
             : 		#end
> Can be removed?

not sure if I should keep this here, since the configuration should be correct, but I currently have no way of verifying it. It may be that I broke the rootport somehow.