Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42447 )
Change subject: nb/intel/sandybridge/gma.c: Fix lock bits setting ......................................................................
Patch Set 1: Code-Review+1
(2 comments)
https://review.coreboot.org/c/coreboot/+/42447/1/src/northbridge/intel/sandy... File src/northbridge/intel/sandybridge/gma.c:
https://review.coreboot.org/c/coreboot/+/42447/1/src/northbridge/intel/sandy... PS1, Line 381: 5
The numbering refers to the sequence as it is documented in the SA BIOS Spec […]
Oh, so the numbering comes from the SA BIOS Spec. Then, please let's keep the original numbering.
Does the SA BIOS Spec say this bit needs to be set here? Reference code doesn't write anything like that.
https://review.coreboot.org/c/coreboot/+/42447/1/src/northbridge/intel/sandy... PS1, Line 376: gtt_write(0xa004, 0x00000000);
Why write 0 and not skip it like the old step 5? What do you know about these registers that you did […]
Reference code writes zero at this point, and locks it later.