Change in coreboot[master]: mb/google/brya: Reset XHCI controller while preparing for S5
Attention is currently required from: Nico Huber, Maulik V Vaghela, Paul Menzel, Sridhar Siricilla, Arthur Heymans, Nick Vaccaro, Eric Lai, Felix Held. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63552 )
Change subject: mb/google/brya: Reset XHCI controller while preparing for S5 ......................................................................
Patch Set 6:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/63552/comment/38edfef9_8863d026 PS4, Line 37: SMI handler
Sorry, thinking error on my part. coreboot of course does not run anymore during shutdown (besides the installed handlers).
But, why can’t this be worked around/fixed in the OS driver?
fixing in FW is comparatively easier 😊 and could apply MB specific W/As as well.
How so?
- the fixes in FW are effective in timely manner where one can't land those changes easily in kernel.
Not sure if you are you saying coreboot is a dump for things you can't land easily in the kernel. In any case, this only seems true because you work for a company that dominates coreboot development and is shy when it comes to OS development. But should that affect where we place things in the ecosystem? I don't think so.
- FW qualification is less costly compared to OS.
Don't you have to qualify both eventually?
- we could add device specific W/A as we have mainboard smi handler (originally designed for the same).
You can also have mainboard-specific ASL code. Designed for the same? IMO, SMI handlers should be a last resort. Security sensitive applications that demand no or as little SMI handlers as possible should be supported too.
Implementing XHCI reset as part of _PTS control method sounds good and SMI free.
But let's not get worked up on this. We should wait for what the TA says. If this won't stay board specific, I would prefer to do it in the OS.
-
Subrata Banik (Code Review)