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

View Change

5 comments:

• Patchset:

  • Patch Set #2:

    Thank you for the patch!

• Commit Message:

  • Patch Set #2, Line 9: Panther Lake

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

• 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 */

    > > We attempted to move METEOR_LAKE to the bottom of the ich_chipset enum, but this patch reveals t […]

    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?

• File util/ich_descriptors_tool/ich_descriptors_tool.c:

  • Patch Set #2, Line 123: the chipset series with the '-c' parameter and one of the possible arguments

    I think you need to add panther here too

  • Patch Set #2, Line 251:

    		else if (strcmp(csn, "panther") == 0)
    			cs = CHIPSET_PANTHER_LAKE;
    

    I think the ich_descriptor_tool is run separately from flashrom.
    Can you run it and add as one more test scenario? (and in a commit message too)
    Thanks!

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