Attention is currently required from: Angel Pons, Kevin Yu. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79340?usp=email ) Change subject: Add Idaville platform into chipset enable list and add update IRC region support ...................................................................... Patch Set 6: (4 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/79340/comment/cbf2f644_ad8ed0ae : PS6, Line 7: Add Idaville platform into chipset enable list and add update IRC region support Commit title and message are limited by 72 chars width, so you need to shorten your commit title. For example "Add support for Idaville HCC platform and iRC regions" And then the rest of commit description can be provide full details of the commit. https://review.coreboot.org/c/flashrom/+/79340/comment/2c8bc947_ef80464e : PS6, Line 11: You need to add full testing info into commit message (which platforms you tested on, which commands etc). Patchset: PS6: Hello Kevin! Sorry there was some delay in reply! It's because by coincidence your patch fell into the holidays season here, and most people would be away (between mid-December and mid-January that is). I am looking at it now, and I have few bigger questions, before diving into details: 1) This seems platform-specific feature (Intel), so flashrom.c is probably not the right place for the code, I think the code should live in ichspi.c 2) If you could give me more details on what is IRC region, any docs you can share? I want to understand what that is. There are few other comments that I have, you can have a look. Also I think we need few more reviewers on the patch, but that's fine. Thank you for patch! And let's start working on it :) File flashrom.c: https://review.coreboot.org/c/flashrom/+/79340/comment/c6310cbe_b2b19c43 : PS6, Line 527: }else{ A generic comment to all of your code (applies to all the files): code should follow the coding style which is described in our dev guidelines: https://www.flashrom.org/dev_guide/development_guide.html#coding-style (please don't miss to click the link and open Linux kernel style guide). You need to read the code style first, and then go through your code and make sure all of it follows the style. Thank you! -- To view, visit https://review.coreboot.org/c/flashrom/+/79340?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Icc526b22a9efc8071e972bb1d8cfc51d6a83651b Gerrit-Change-Number: 79340 Gerrit-PatchSet: 6 Gerrit-Owner: Kevin Yu <ykevin@celestica.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm@chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus@gmail.com> Gerrit-Attention: Kevin Yu <ykevin@celestica.com> Gerrit-Comment-Date: Mon, 15 Jan 2024 03:16:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment