Attention is currently required from: Subrata Banik, Wonkyu Kim, Rizwan Qureshi, Tim Wawrzynczak, Meera Ravindranath, Patrick Rudolph, Gwendal Grignou. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62662 )
Change subject: mb/intel/adlrvp: Enable UFS for ADL-N RVP ......................................................................
Patch Set 4: Code-Review+1
(4 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/62662/comment/0511bbd9_04b30030 PS4, Line 10: also needs to be enabled. Hence, enable ISH as well. How about:
The PCI Local Bus Specification Revision 3.0 requires that multi-function devices always implement function 0. Because of this, enabling UFS (PCI device 12.7) requires ISH (PCI device 12.0) to be enabled as well.
File src/mainboard/intel/adlrvp/devicetree_n.cb:
https://review.coreboot.org/c/coreboot/+/62662/comment/afc98165_c35de692 PS1, Line 283: device ref ish on end
Is ish part of this CL?
Yes, PCI spec requires that multi-function devices always implement function 0. Because of this, enabling the UFS PCI device (device pci 12.7) requires that the ISH PCI device (device pci 12.0) be enabled as well.
File src/mainboard/intel/adlrvp/devicetree_n.cb:
https://review.coreboot.org/c/coreboot/+/62662/comment/6ea2ed8e_aa387378 PS3, Line 283: on
then how pcie_rp7 is working here https://github. […]
See the code in pcie_rp.c, the `struct pcie_rp_group` arrays in particular. After FSP does RP function swap, coreboot figures out which RPs have been function swapped and adjusts the devicetree accordingly at runtime (somewhere in ramstage).
File src/mainboard/intel/adlrvp/devicetree_n.cb:
https://review.coreboot.org/c/coreboot/+/62662/comment/39c936b1_3bd176a6 PS4, Line 287: device ref ish on end
Can you split patch? enabling ish is not related to enabling UFS
PCI spec requires that multi-function devices always implement function 0. Because of this, enabling the UFS PCI device (device pci 12.7) requires that the ISH PCI device (device pci 12.0) be enabled as well.
-
Angel Pons (Code Review)