Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35537 )
Change subject: sb/intel/spi: Use different SPIOPS for some SST flashes ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/35537/2/src/southbridge/intel/commo... File src/southbridge/intel/common/spi.c:
https://review.coreboot.org/c/coreboot/+/35537/2/src/southbridge/intel/commo... PS2, Line 1056: : const char *(aai_write_flash[]) = { : "SST25VF040B", : "SST25VF080B", : "SST25VF080", : "SST25VF016B", : "SST25VF032B", : "SST25WF512", : "SST25WF010", : "SST25WF020", : "SST25WF040", : "SST25WF080", : "SST25WF080B" : };
Except id_code 0x4b all others use AAI_WRITE. Should that be checked for? amd/soc does it like that.
flashchips.c also mentions a lot that use 0xaf (and don't support 0xad I guess). Those seem to be older versions, though.
Generally, I wouldn't mind a simpler check. As most of SST seems to be AAI, it would even be better to assume AAI for SST and make 0x02 the exception.
https://review.coreboot.org/c/coreboot/+/35537/2/src/southbridge/intel/commo... PS2, Line 1102: const struct spi_flash *flash = boot_device_spi_flash(); can it return NULL?