Attention is currently required from: Subrata Banik, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63584 )
Change subject: ichspi: Add support for Ice Lake ......................................................................
Patch Set 1:
(2 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/63584/comment/24f32b8d_06d1de4e PS1, Line 7: ichspi: Add support for Ice Lake
Technically, it's already supported. Why do we need to do more than […]
As discussed during review of the first of these patches, adding all the clutter that a new enum entry brings with it is wrong and should be fixed. They were only merged because Edwards said he could look into it later. Most of what these patches do needs to be undone even- tually.
I assume this work (to check that the existing 300-series code behaves the same) was already done for Ice Lake. Just without the noise people make in other patches.
File chipset_enable.c:
https://review.coreboot.org/c/flashrom/+/63584/comment/88b9d8dc_2c2850b1 PS1, Line 2149: 0x34a4
Is the SPI device guaranteed not to be hidden on Ice Lake? […]
But can it be hidden? Maybe not everybody have updated their code to comply with the changed FAS.
The safest way, IMO, would be to add the SPI controller ID and leave the entry with the PCH ID as is. This way, it can't regress in case there is a device with that ID and hidden SPI controller.
-
Nico Huber (Code Review)