Nico Huber 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 8:
(5 comments)
https://review.coreboot.org/#/c/30822/8/src/southbridge/intel/i82801gx/chip.... File src/southbridge/intel/i82801gx/chip.h:
https://review.coreboot.org/#/c/30822/8/src/southbridge/intel/i82801gx/chip.... PS8, Line 20: #include <southbridge/intel/i82801gx/i82801gx.h> Would it be better to define the enum here?
https://review.coreboot.org/#/c/30822/8/src/southbridge/intel/i82801gx/ide.c File src/southbridge/intel/i82801gx/ide.c:
https://review.coreboot.org/#/c/30822/8/src/southbridge/intel/i82801gx/ide.c... PS8, Line 22: #include "chip.h" ...
https://review.coreboot.org/#/c/30822/8/src/southbridge/intel/i82801gx/sata.... File src/southbridge/intel/i82801gx/sata.c:
https://review.coreboot.org/#/c/30822/8/src/southbridge/intel/i82801gx/sata.... PS8, Line 53: static int ahci_supported = -1; Some idea: Update `config->sata_mode` instead. That would save some lines, only have to be called once and probably make the `switch` logic in sata_init() easier.
https://review.coreboot.org/#/c/30822/8/src/southbridge/intel/i82801gx/sata.... PS8, Line 58: pcidev_path_on_root or:
pcidev_on_root(31, 0)
https://review.coreboot.org/#/c/30822/8/src/southbridge/intel/i82801gx/sata.... PS8, Line 160: ahci_bar[3] = config->sata_ports_implemented; break here?