Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41473 )
Change subject: inteltool: Support dumping IOBP register values ......................................................................
Patch Set 3:
(10 comments)
https://review.coreboot.org/c/coreboot/+/41473/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/41473/3//COMMIT_MSG@13 PS3, Line 13: SATA_IOBP_SP*G3IR : (does LynxPoint really use them?) They don't seem to exist.
https://review.coreboot.org/c/coreboot/+/41473/3/util/inteltool/iobp.c File util/inteltool/iobp.c:
https://review.coreboot.org/c/coreboot/+/41473/3/util/inteltool/iobp.c@8 PS3, Line 8: x please use braces around macro parameters: `(x)`
https://review.coreboot.org/c/coreboot/+/41473/3/util/inteltool/iobp.c@8 PS3, Line 8: rcba this hidden parameter is ugly.
https://review.coreboot.org/c/coreboot/+/41473/3/util/inteltool/iobp.c@13 PS3, Line 13: #define IOBPIRI 0x2330 : #define IOBPD 0x2334 : #define IOBPS 0x2338 : #define IOBPS_READY 0x0001 : #define IOBPS_TX_MASK 0x0006 : #define IOBPS_MASK 0xff00 : #define IOBPS_READ 0x0600 : #define IOBPS_WRITE 0x0700 : #define IOBPU 0x233a : #define IOBPU_MAGIC 0xf000 Align values with tabs?
https://review.coreboot.org/c/coreboot/+/41473/3/util/inteltool/iobp.c@27 PS3, Line 27: no space on unary operators
https://review.coreboot.org/c/coreboot/+/41473/3/util/inteltool/iobp.c@84 PS3, Line 84: {0xea000051, "SATA_IOBP_SP1G3IR"}, missing per-port registers: (address is for port 0, calculate the other addresses accordingly)
SECRT88 EA002488h SECRT8C EA00248Ch SECRT90 EA002490h SECRT98 EA002498h SECRR1C EA00251Ch SECRR58 EA002558h
https://review.coreboot.org/c/coreboot/+/41473/3/util/inteltool/iobp.c@85 PS3, Line 85: {0xea002550, "SATA_IOBP_SP0DTLE_DATA"}, This is SECRR50, and there's up to six of them (one per SATA port):
Port 0: EA002550h Port 1: EA002750h Port 2: EA000950h Port 3: EA000B50h Port 4: EA002150h Port 5: EA002350h
https://review.coreboot.org/c/coreboot/+/41473/3/util/inteltool/iobp.c@86 PS3, Line 86: {0xea002554, "SATA_IOBP_SP0DTLE_EDGE"}, This is SECRR54 and is the same story as SECRR50
https://review.coreboot.org/c/coreboot/+/41473/3/util/inteltool/iobp.c@88 PS3, Line 88: {0xea002754, "SATA_IOBP_SP1DTLE_EDGE"}, Missing global registers:
SECRF00 EA008100h SECRF04 EA008104h
https://review.coreboot.org/c/coreboot/+/41473/3/util/inteltool/iobp.c@90 PS3, Line 90: USBP01 These registers are officially called U2ECR