Attention is currently required from: Anastasia Klimchuk, Hsuan Ting Chen, Hsuan-ting Chen, Sam McNally.

View Change

2 comments:

• Commit Message:

  • Patch Set #2, Line 9: Panther Lake

    I am wondering, maybe there is a datasheet available? Asking for myself.

    yes, Intel doc number: 815466

• File include/programmer.h:

  • Patch Set #1, Line 364: /* All chipsets after METEOR_LAKE should support checking BIOS_BM to get read/write access to of FREG0~15 */

    I think what Hsuan-ting meant to say (but correct me if I am wrong), is that we have 100 places in all ich* code that switch through the cases of ich_chipset enum, and the order is not the same between these 100 places.
    For example in `programmer.h` and `ichspi.c` Meteor Lake goes after Elkhart, but in `ich_descriptors.c` Meteor is before Elkhart.

    I think it would be more readable to have the same order where it's possible (it is not always be possible, like pprint_freq is different). But in a separate commit, because this one adds support for Panther Lake.

    what do you think?

    why would be the order is important inside the switch case ?

    for example:
    ```
    switch (chipset):
     case 0:
     case 5:
     case 2:
        fool()
        break;
     ```
     
     in above example, the execution of "foo" in important for all cases like 0/2/5 but order in which they are being added in switch case not important at all.
     
     As mentioned earlier, the only case where the order was meaningful earlier was fixed with CB:83143. Apart from there, there are only one newly added logic that cares about the index value in which the Chipset numbers being added post MTL
     
     ```
     static unsigned int ich_get_defined_region_count(void) {
        return (ich_generation >= CHIPSET_METEOR_LAKE) ? 16 : 8;
    }
     ```
     
     other than that I don't think the order make sense elsewhere.

To view, visit change 83144. To unsubscribe, or for help writing mail filters, visit settings.