Change in flashrom[master]: Add support for Intel Emmitsburg PCH

[David Hendricks (Code Review)]

Attention is currently required from: Jonathan Zhang.
David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54965 )

Change subject: Add support for Intel Emmitsburg PCH
......................................................................

Patch Set 3:

(2 comments)

Patchset:

PS1:
> I'm not talking about the detection. I mean if anybody checked that
> all the code activated by this patch matches the hardware and descriptor.
> IOW if all the paths chosen for CHIPSET_C620_SERIES_LEWISBURG are
> suitable or just work by chance for rudimentary flashing.

LBG and EBG are generally very similar, if not identical. For our purposes with this patch FLMAP registers are the same layout and have the same default values listed in the SPI PG (document numbers 559021 and 619386).

However, the documentation says PSL should be 0x8B, but Intel's own reference images use 0x80. Some of the older chipsets don't show a default value at all, like the ICH8 and ICH9 (document numbers 547138 and 355845). So unfortunately we can't rely on documentation alone to determine what value to compare against here. The values in this detection logic appear to be guesses based on what shows up in the field. As you suggest, it seems works by chance for rudimentary flashing on all these chipsets.
> Now that I looked at the merged change, it seems obvious that it breaks
> Ibex Peak detection. Its SPI guide is 403598. I'm not sure if Intel still
> keeps these files.

Hmm, I don't see 403598 on developer.intel.com. Not sure if I need to request that doc specifically or if it's just no longer available as you suggest.

File ich_descriptors.c:

https://review.coreboot.org/c/flashrom/+/54965/comment/b2da173d_a525ad43 
PS3, Line 943: 			return CHIPSET_C620_SERIES_LEWISBURG;
> Was this reviewed at all? It assumes that CHIPSET_5_SERIES_IBEX_PEAK […]

OK, so how about if we start using '==' operators?

if (content->ISL == 16)
        return CHIPSET_5_SERIES_IBEX_PEAK;
if (content->ISL == 80)
        return CHIPSET_C620_SERIES_LEWISBURG;
return CHIPSET_ICH_UNKNOWN;

That will at least get us past the current dilemma.