[flashrom-gerrit] [M] Change in flashrom[main]: ichspi.c: Add support for region 9 and beyond in Meteor Lake

[Anastasia Klimchuk (Code Review)]

Attention is currently required from: Hsuan Ting Chen, Hsuan-ting Chen, Subrata Banik.

Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81357?usp=email )

Change subject: ichspi.c: Add support for region 9 and beyond in Meteor Lake
......................................................................

Patch Set 10:

(11 comments)

Commit Message:

https://review.coreboot.org/c/flashrom/+/81357/comment/e96b43c1_539e5275 :
PS10, Line 28: 
Is MTL a Meteor Lake and ADL Alder Point?

Is it possible to test the chipsets that used to be after Meteor in the enum, and now they are before?

File ichspi.c:

https://review.coreboot.org/c/flashrom/+/81357/comment/f25256b6_1456dcf5 :
PS10, Line 1853: mmio_readw
Why it is `readw` (reads into uint16_t) here, but `readl` below (reads into uint32_t)?
You declare `region_read_access` and `region_write_access` as uint32_t actually

https://review.coreboot.org/c/flashrom/+/81357/comment/e789d35a_3e357092 :
PS10, Line 1854: 		*region_write_access = mmio_readw(ich_spibar + ICH_REG_BIOS_BM_WAP);
Maybe you can also add some debugging prints in this branch too

https://review.coreboot.org/c/flashrom/+/81357/comment/47f1115f_18bd15d7 :
PS10, Line 1876: inline
I don't think this needs to be inline?

https://review.coreboot.org/c/flashrom/+/81357/comment/8a8684c3_be89553c :
PS10, Line 1877: if (ich_generation >= CHIPSET_METEOR_LAKE)
               : 		return 16;
               : 	return 8;
maybe make it one line
`return (ich_generation >= CHIPSET_METEOR_LAKE) ? 16 : 8;`

https://review.coreboot.org/c/flashrom/+/81357/comment/2b4ac08b_fdc32cc8 :
PS10, Line 2109: case CHIPSET_METEOR_LAKE:
Let's re-order Meteor Lake here too

https://review.coreboot.org/c/flashrom/+/81357/comment/735165af_ce23a858 :
PS10, Line 2149: case CHIPSET_METEOR_LAKE:
and here

https://review.coreboot.org/c/flashrom/+/81357/comment/119727ec_9cb0c1f7 :
PS10, Line 2211: case CHIPSET_METEOR_LAKE:
and here

https://review.coreboot.org/c/flashrom/+/81357/comment/7ed5d9c0_fe2647b9 :
PS10, Line 2229:  
you can leave the word "registers", so that it is "... and region access registers"

https://review.coreboot.org/c/flashrom/+/81357/comment/923bd8a6_b0f2d5b8 :
PS10, Line 2291: case CHIPSET_METEOR_LAKE:
and here

https://review.coreboot.org/c/flashrom/+/81357/comment/e5a66a3d_d85dc523 :
PS10, Line 2331: case CHIPSET_METEOR_LAKE:
and here