Attention is currently required from: Subrata Banik.

View Change

2 comments:

• File ichspi.c:

  • Patch Set #1, Line 1848: case CHIPSET_METEOR_LAKE:

    this might increase the maintenance burden going forward ? for example: we need to add PTL, WCL etc. […]

    It was your previous CL that put MTL in that position (https://review.coreboot.org/c/flashrom/+/62783) Could you share if (1) We could move it to the very end (2) Will there be any other chipset name which will be moved to make the order makes sense?

  • Patch Set #1, Line 2236: ich9_handle_frap

    isn't `ich9_handle_frap` function takes only takes 3 arguments ?

    In line#1889, I changed it to

    ```
    static enum ich_access_protection ich9_handle_frap(struct fd_region *fd_regions,
    						   uint32_t region_read_access,
    						   uint32_t region_write_access, unsigned int i)
    ```

    Since if we use `BIOS_BM`, we will derive two seperated bitmask, and I don't think we need to merge them.

    Maybe `ich9_handle_frap` should be renamed to something like `ich9_handle_region_access` ?

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