Attention is currently required from: Robert Zieba, Jason Glenesk, Raul Rangel, Martin Roth, Marshall Dawson, Paul Menzel, Fred Reitberger, Rob Barnes, Karthik Ramasubramanian. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61259 )
Change subject: soc/amd/cezanne: Turn off gpp clock request for disabled devices ......................................................................
Patch Set 34:
(3 comments)
File src/soc/amd/cezanne/fch.c:
https://review.coreboot.org/c/coreboot/+/61259/comment/088ec307_ceef9a6d PS34, Line 183: assert(gpp_req_index >= 0 && gpp_req_index < gpp_clk_config_num); by default asserts aren't fatal, so it'll complain but still continue; not sure if this is intended here. i'd check this and if the conditions aren't met, print a warning/info and do a continue to not run the rest of the code in the loop.
https://review.coreboot.org/c/coreboot/+/61259/comment/3c9305c4_6a769007 PS34, Line 196: /* PCIe devices haven't been fully set up yet, so directly read the vendor id the code below reads both the vendor and product id; both ids are just 16 bit with the vendor id starting at 0x00 and the device id starting at 0x02. the code behaves as i'd expect, but the comment should mention the device id too
https://review.coreboot.org/c/coreboot/+/61259/comment/90ed5664_813c3241 PS34, Line 200: && (id != 0x0000ffff) && (id != 0xffff0000) not sure if those two cases are needed. if something doesn't get decoded i'd expect it to return all-ones. all-zeros might also be failure case. vendor id all-ones and device id all-zeros or vice versa looks like an odd failure case to me. checking those cases too shouldn't hurt, but looked a bit unexpected to me