Nico Huber has posted comments on this change. ( https://review.coreboot.org/20937 )
Change subject: ich_descriptors: Fix off-by-one error ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/#/c/20937/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/20937/1//COMMIT_MSG@9 PS1, Line 9: The "number of masters" (NM) reflects the value that begins at 1. Seems that this is right for Lewisburg, awesome! more mess for us to handle.
I've never found a definition if it starts at 0 or 1 and past practice was to count from 0 (== 1 master)! All descriptors in the wild that I've encountered were for platforms with 3 masters and always had NM set to 2 ;) Now the SPI guide for Lewisburg states it should be 6 (and there are only 6 masters mentioned), hmmm.
I guess the lack of an explicit definition confused Intel's developers even more than us and now they disagree. No prob- lem for them, as they make one (binary) program for each platform anyway (I suppose).
Two solutions come to mind: Take the guessed platform into account (Lewisburg would need its own constant). Or we could ignore a too high NM and just return MAX_NUM_MASTERS (and drop a lot of sanity checks as we could return an unsigned value).
https://review.coreboot.org/#/c/20937/1//COMMIT_MSG@17 PS1, Line 17: even more wrong than it was before. I don't see any loop doing it wrong, nor any caller adding 1 to the return value.
-
Nico Huber (Code Review)