Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49012 )
Change subject: soc/intel/jasperlake: Enable USB PG for s0ix qualification ......................................................................
Patch Set 3:
(5 comments)
https://review.coreboot.org/c/coreboot/+/49012/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/49012/3//COMMIT_MSG@10 PS3, Line 10: USB is not power gated "USB PHY SUS well" since there are more than one qualification bits for USB.
Also, is this USB2 or both USB2 and USB3?
https://review.coreboot.org/c/coreboot/+/49012/3/src/soc/intel/jasperlake/fi... File src/soc/intel/jasperlake/finalize.c:
https://review.coreboot.org/c/coreboot/+/49012/3/src/soc/intel/jasperlake/fi... PS3, Line 69: * Can you please add a comment here indicating why this is required? It will be helpful when someone looks at this code later.
https://review.coreboot.org/c/coreboot/+/49012/3/src/soc/intel/jasperlake/fi... PS3, Line 71: USBSUSPGQDIS Not for this change, but I think we should do the configuration of disqualification bits (CPPMVRIC1/2/3) in coreboot for all platforms. On every platform, we keep running into this and tweak a couple of bits to make things work. Instead we should evaluate all the bits and determine when one would want to set a particular bit.
https://review.coreboot.org/c/coreboot/+/49012/3/src/soc/intel/jasperlake/in... File src/soc/intel/jasperlake/include/soc/pmc.h:
https://review.coreboot.org/c/coreboot/+/49012/3/src/soc/intel/jasperlake/in... PS3, Line 125: Use space like other entries here.
https://review.coreboot.org/c/coreboot/+/49012/3/src/soc/intel/jasperlake/in... PS3, Line 126: Use space like other entries and align like XTALSDQDIS above.