[coreboot-gerrit] Change in ...coreboot[master]: sb/intel/i82801gx: Detect if the southbridge supports AHCI

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30822 )

Change subject: sb/intel/i82801gx: Detect if the southbridge supports AHCI
......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/#/c/30822/5/src/southbridge/intel/i82801gx/sata.... 
File src/southbridge/intel/i82801gx/sata.c:

https://review.coreboot.org/#/c/30822/5/src/southbridge/intel/i82801gx/sata.... 
PS5, Line 75: 		ret = ahci_supported && config->sata_no_ahci;
> Without this option, the `else if (config->ide_legacy_combined)`
> below would be unreachable on boards that support AHCI. I wouldn't
> mind to remove combined mode altogether. But as long as we maintain
> it, we should provide reasonable devicetree options.
>
> IMHO, it should be an `enum sata_mode { ahci, ide, legacy, legacy_
> combined };`. Unless we'd ditch combined mode. Then we could also
> unify the sata code with everything up to BDW.

Iirc combined mode does not even work properly with this code...