Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35439 )
Change subject: sb/intel/ibexpeak: Add CIR initialization ......................................................................
Patch Set 8:
(14 comments)
https://review.coreboot.org/c/coreboot/+/35439/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35439/4//COMMIT_MSG@9 PS4, Line 9: This properly the chipset initialization registers.
Missing verb: sets?
Done
https://review.coreboot.org/c/coreboot/+/35439/4//COMMIT_MSG@15 PS4, Line 15: from S3.
Can you please summarize what changed, or what the benefits are?
Done
https://review.coreboot.org/c/coreboot/+/35439/1/src/mainboard/lenovo/x201/r... File src/mainboard/lenovo/x201/romstage.c:
https://review.coreboot.org/c/coreboot/+/35439/1/src/mainboard/lenovo/x201/r... PS1, Line 69: 0x02060100
since this is still open (and no change in the latest patch set): what do you mean by this and what is Arthur supposed to do? :-)
All the bits are read/ write to clear and clearing those is supposedly done in ramstage in pch_power_options().
https://review.coreboot.org/c/coreboot/+/35439/1/src/southbridge/intel/ibexp... File src/southbridge/intel/ibexpeak/early_cir.c:
https://review.coreboot.org/c/coreboot/+/35439/1/src/southbridge/intel/ibexp... PS1, Line 21: set
sets
Done
https://review.coreboot.org/c/coreboot/+/35439/1/src/southbridge/intel/ibexp... PS1, Line 30: CIR7
According to intel magic: […]
Done
https://review.coreboot.org/c/coreboot/+/35439/1/src/southbridge/intel/ibexp... PS1, Line 37: 0x3b09
My LPC device ID
Done
https://review.coreboot.org/c/coreboot/+/35439/1/src/southbridge/intel/ibexp... PS1, Line 41: CIR6
Done, however lpc.c l320 does touch this too :/ It seems to work now.
Done
https://review.coreboot.org/c/coreboot/+/35439/4/src/southbridge/intel/ibexp... File src/southbridge/intel/ibexpeak/early_cir.c:
https://review.coreboot.org/c/coreboot/+/35439/4/src/southbridge/intel/ibexp... PS4, Line 21: set
sets
Done
https://review.coreboot.org/c/coreboot/+/35439/4/src/southbridge/intel/ibexp... PS4, Line 25: registers
Registers? Also add (CIR) to the line?
Done
https://review.coreboot.org/c/coreboot/+/35439/4/src/southbridge/intel/ibexp... PS4, Line 56: if (chipset_type == NEHALEM_DESKTOP)
Add a comment with a datasheet section reference?
Done
https://review.coreboot.org/c/coreboot/+/35439/4/src/southbridge/intel/ibexp... PS4, Line 85: die("unsupported CPU!\n");
Print out the model?
Done
https://review.coreboot.org/c/coreboot/+/35439/4/src/southbridge/intel/ibexp... PS4, Line 90: */
Please use one of the recommended comment styles.
Done
https://review.coreboot.org/c/coreboot/+/35439/5/src/southbridge/intel/ibexp... File src/southbridge/intel/ibexpeak/early_cir.c:
https://review.coreboot.org/c/coreboot/+/35439/5/src/southbridge/intel/ibexp... PS5, Line 54: RCBA32_OR(0x3310,0x31);
space required after that ',' (ctx:VxV)
Done
https://review.coreboot.org/c/coreboot/+/35439/6/src/southbridge/intel/ibexp... File src/southbridge/intel/ibexpeak/early_cir.c:
https://review.coreboot.org/c/coreboot/+/35439/6/src/southbridge/intel/ibexp... PS6, Line 54: RCBA32_OR(0x3310,0x31);
space required after that ',' (ctx:VxV)
Done