Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44353 )
Change subject: soc/amd/common/espi_util: slightly refactor espi_open_io_window ......................................................................
Patch Set 3:
(3 comments)
https://review.coreboot.org/c/coreboot/+/44353/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44353/3//COMMIT_MSG@7 PS3, Line 7: slightly refactor espi_open_io_window No need to change it, but I might've called it "clarify" vs. "slightly refactor".
Also, unsure whether you would've meant to omit the "generic" portion of the name.
https://review.coreboot.org/c/coreboot/+/44353/3//COMMIT_MSG@9 PS3, Line 9: If the espi_open_generic_io_window gets run Haha, normally I feel like your written English is better than mine. In this case I had trouble understanding the commit message until I looked at the code.
Here, how about "Executing espi_open_generic_io_window() depends on..."
https://review.coreboot.org/c/coreboot/+/44353/3//COMMIT_MSG@10 PS3, Line 10: the if statement before Nit, something like "the preceding if statement" might be more clear to the reader. Something about "the if <anything>" initially appears to be a typo for me.