Attention is currently required from: Subrata Banik, Edward O'Callaghan, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62282 )
Change subject: ichspi: Add Jasper Lake support ......................................................................
Patch Set 9:
(2 comments)
Patchset:
PS4:
Unresolving then because it does look like a pch300 although I am not 100% on every tiny idiosyncrasy.
If `ich_number_of_masters()` needs adjusting then so does the switch in `prettyprint_ich_descriptor_master()` too.
NM has always been vaguely defined. The original assumption was that it's 0 based. This didn't work for some niche platforms, IIRC starting with Lewisburg. AIUI, the value was never interpreted by the hardware and only for FIT's convenience. It seems possible that not even FIT cares about it anymore and we are chasing ghosts (i.e. we might have adapted the code to make it work while we actually read a garbage value).
One possible way out of the dilemma would be to let ich_number_of_masters() return static values per platform (or just make it a constant array). Simply use the maximum a descriptor generation supports. From the SPI guides it looks like the arrays like FLMSTR are not dynamic, i.e. always have the same length for a given generation. We should have a look at some descrip- tors to confirm that.
Probably the same for NR on older platforms.
Thing is, that has:
const char *const master_names[] = { "BIOS", "ME", "GbE", "unknown", "EC", };
but the datasheet says on page 42 - [BIOS, ME, GbE, EC] and states there are 4 masters. Nothing indicates if to interpret "number of masters" from the flash descriptor as so called- 0-based or 1-based. Therefore I just assumed `cont->NM`.
If in doubt, it's often good to assume that nothing changed. What makes things harder is that we can't derive how optional things are considered like that "EC" master. I don't think I've ever worked with a board that makes use of it.
Open to some guidance here? My concern is that I got those two functions wrong.
Sometimes it's easiest to do what the existing code for the closest relative does. If it needs to be fixed later, it at least avoids arguments like "why did JSL differ, let's better not touch the code".
File ich_descriptors.c:
https://review.coreboot.org/c/flashrom/+/62282/comment/428fe818_20fc729b PS8, Line 1061: CHIPSET_ELKHART_LAKE
Thanks Nico that is super useful ! I didn't have access to EHL I was asking if Subrata could ask a c […]
Google should have access too. Since ~SKL, both the SPI guide and FIT are part of the CSE/(CS)ME/TXE firmware packages. It's not always easy to find on intel.com, took me hours to find one for JSL. For newer platforms the following search strings might give results "... Lake CSE" "... Lake CSME" "... Lake BKC". It can also help to know the version for a given platform, for this it's best to query the experts: https://www.win-raid.com/t596f39-Intel-Converged-Security-Management-Engine-... (still not easy to keep track of codenames, who would have thought that the EHL PCH is Mule Creek Canyon?)