Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29967 )
Change subject: qclib: Add qclib support with interface tables ......................................................................
Patch Set 33:
(1 comment)
https://review.coreboot.org/c/coreboot/+/29967/30/src/mainboard/google/mistr... File src/mainboard/google/mistral/romstage.c:
https://review.coreboot.org/c/coreboot/+/29967/30/src/mainboard/google/mistr... PS30, Line 32: qclib_load_and_run();
Hi Julius/Patrick, […]
Does QCLIB need to enable USB-related clocks? Ideally it shouldn't -- USB should be fully controlled by coreboot. So if possible, please try to fix this in QCLIB. The separation between coreboot and QCLIB responsibilities should be clear and QCLIB should not try to do any more than necessary.
Fundamentally, there's no hard reason why you can't run this after QCLIB. But the whole point of having a reset_usb() here (as opposed to with the rest of the USB code in ramstage) is that we can enforce the required minimum reset delay of the DWC3 controller. If you do the reset after QCLIB, there may not be a lot of time between the reset_usb() and the setup_usb() call (because QCLIB is the biggest piece here). So depending on how much minimum reset delay your DWC3 controller needs exactly, you may be violating its requirement. (And we don't want to just udelay() instead because that costs us boot time.)