awokd@danwin1210.me has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38495 )
Change subject: [WIP] vc/amd/agesa/[...]/Proc/Mem: Fix uninitialized scalar variable ......................................................................
Patch Set 3:
(2 comments)
This change is ready for review.
https://review.coreboot.org/c/coreboot/+/38495/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38495/2//COMMIT_MSG@15 PS2, Line 15: Tested on : Lenovo G505s.
Please put it on a separate line (with blank line between paragraphs).
Done
https://review.coreboot.org/c/coreboot/+/38495/2/src/vendorcode/amd/agesa/f1... File src/vendorcode/amd/agesa/f15tn/Proc/Mem/Tech/mtthrcSeedTrain.c:
https://review.coreboot.org/c/coreboot/+/38495/2/src/vendorcode/amd/agesa/f1... PS2, Line 508: (RxEnMemClkTested[ByteLane][MemClkCycle] ? RxEnMemClkSt[ByteLane][MemClkCycle] == TRUE : (TechPtr->ByteLaneError[ByteLane] && DlyWrittenToReg[ByteLane]))
Do you know what this condition does? Moving the assignment out of the if statement changes logic, b […]
In no particular order, 0x10 is subtracted per 5b of 2.9.5.9.3 in Fam 15h BKDG: 5. Process the array of results and determine a pass-to-fail transition. A. DqsRcvEnCycle = the total delay value of the pass result. B. Program D18F2x9C_x0000_00[2A:10]_dct[1:0]_mp[1:0][DqsRcvEnGrossDelay, DqsRcvEnFineDe- lay] = DqsRcvEnCycle - 10h.
I am struggling following the cited condition. I think it has something to do with this part in 2.9.5.9.4: 3. For each read DQS delay value in D18F2x9C_x0000_0[3:0]0[6:5]_dct[1:0]_mp[1:0] from 0 to 1 UI: Program the read DQS delay value for the current lane. Read the DRAM training pattern from the test address. Record the result for the current settings as a pass or fail depending if the pattern is read correctly. Process the array of results and determine the longest string of consecutive passing read DQS delay values. If the read DQS delay results for the current lane contain three or more consecutive passing delay values, then program D18F2x9C_x0000_0[3:0]0[6:5]_dct[1:0]_mp[1:0] with the average value of the smallest and largest delay values in the string of consecutive passing results. If the average value of passing read DQS delay for the lane is negative, then adjust the input receiver DQ delay in D18F2x9C_x0D0F_0[F,7:0]1F_dct[1:0]_mp[1:0] for the lane as follows: IF (RxBypass3rd4thStg == 1) program RxBypass3rd4thStg=0 and repeat step 3 above for all ranks and lanes. ELSEIF (Rx4thStgEn == 0) program Rx4thStgEn=1 and repeat step 3 above for all ranks and lanes. ELSE program the read DQS delay for the lane with a value of zero.
However, the two places mentioned in the commit message are the only two that reference FinalRxEnCycle[]. Moving the assignment out of the if changes the logic only in the case Coverity was complaining about which currently results in it assigning an uninitialized value. My other thought was to put an initialization loop at the top and assign 0s to FinalRxEnCycle[], but if that was the intent it seemed to me it would be coded somewhere as an explicit assignment. I can test that approach and see what happens, though.
Marking this as a [WIP].