Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37137 )
Change subject: sb/intel/common: Add AHCI code ......................................................................
Patch Set 2:
(4 comments)
https://review.coreboot.org/c/coreboot/+/37137/2/src/include/ahci.h File src/include/ahci.h:
https://review.coreboot.org/c/coreboot/+/37137/2/src/include/ahci.h@49 PS2, Line 49: #define AHCI_VS_MINOR(x) ((x >> 8) & 0xf)
((x) & 0xf ? ((((x) >> 8) & 0xf) * 10 + ((x) & 0xf)) : (((x) >> 8) & 0xf)) […]
when you compare chapter 3.1.5.6 and 3.1.5.1 it's clear that 0.95 is actually 0.9.5 as 1.31 is actually 1.3.1
https://review.coreboot.org/c/coreboot/+/37137/2/src/southbridge/intel/commo... File src/southbridge/intel/common/ahci.c:
https://review.coreboot.org/c/coreboot/+/37137/2/src/southbridge/intel/commo... PS2, Line 55: * Spec: "No other ABAR registers should be accessed before this."
I can't find this quote? […]
it's in AHCI chapter 3.1.2. yes that might be true.
https://review.coreboot.org/c/coreboot/+/37137/2/src/southbridge/intel/commo... PS2, Line 80: reg32 |= AHCI_CAP_CCCS;
Note, this wasn't explicitly set for bd82x6x.
yes, not sure if that was by accident or requirement.
https://review.coreboot.org/c/coreboot/+/37137/2/src/southbridge/intel/commo... PS2, Line 102: if (major >= 1 && minor >= 2) {
Hmmm, it seems ICH9 advertises 1.20, but doesn't document this register.
yes, seems like documentation is wrong. Let's hope hardware is not.