Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35189 )
Change subject: util/inteltool: inteltool extensions ......................................................................
Patch Set 3:
(3 comments)
https://review.coreboot.org/c/coreboot/+/35189/3/util/inteltool/ahci.c File util/inteltool/ahci.c:
https://review.coreboot.org/c/coreboot/+/35189/3/util/inteltool/ahci.c@77 PS3, Line 77: : static const io_register_t sunrise_ahci_sir_registers[] = { : {0x80, 4, "SQUELCH"}, : {0x90, 4, "SATA_MPHY_PG"}, : {0xa4, 4, "OOBRETR"}, : };
If these are part of ABAR, I do not see a reason to separate them.
They aren't. The SIRs need to be accessed by index (via SIRI and SIRD)
https://review.coreboot.org/c/coreboot/+/35189/3/util/inteltool/ahci.c@126 PS3, Line 126: " 0x%08x\n",
I'd say this fits on the previous line?
hmm, I don't like to put text after a "\n". It's done this way anywhere in inteltool
https://review.coreboot.org/c/coreboot/+/35189/3/util/inteltool/ahci.c@155 PS3, Line 155: SIR Registers
I like how this literally means "SATA Initialization Register Registers"
\o/ indeed this needs a fix