Attention is currently required from: Arthur Heymans, Christian Walter, Felix Held, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Shuo Liu, Tim Chu.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/82034?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: soc/intel/xeon_sp: Add _OSC ASL generation utils for IIO stacks ......................................................................
Patch Set 2:
(8 comments)
Patchset:
PS2: I wanted to give this more thought, but seeing superficial reviews (which I blame for the accumulated days of unnecessary work on this topic) again, put me into damage-control mode. Where I'll stay for a while.
File src/soc/intel/xeon_sp/acpi/iiostack.asl:
https://review.coreboot.org/c/coreboot/+/82034/comment/a6f6fab1_b8127e07 : PS2, Line 75: CreateDWordField (Local7, Zero, QSUP) I fail to find any use of this.
https://review.coreboot.org/c/coreboot/+/82034/comment/a5ba77fb_78600255 : PS2, Line 87: (OscArg0 == ToUUID (CXL_HOST_BRIDGE_OSC_UUID))) Doesn't this need an `IsCxlDomain != 0` too?
https://review.coreboot.org/c/coreboot/+/82034/comment/340e1971_919165f5 : PS2, Line 89: 0x02 3?
https://review.coreboot.org/c/coreboot/+/82034/comment/9e53b03b_36aeb66e : PS2, Line 99: ToInteger Isn't a DWord an Integer already?
https://review.coreboot.org/c/coreboot/+/82034/comment/323b5619_e08b2163 : PS2, Line 100: Local1 = GrantedPCIeFeatures Is it actually necessary to use local variables for this?
https://review.coreboot.org/c/coreboot/+/82034/comment/232aab9e_edcacedc : PS2, Line 104: SUPL What is SUPL?
https://review.coreboot.org/c/coreboot/+/82034/comment/2414c0aa_fafefd2e : PS2, Line 117: 0x04 5?