Attention is currently required from: Nikolai Artemiev, Stefan Reinauer, Victor Lim.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/82332?usp=email )
Change subject: flashchips: Add (or update) chip models <list models> ......................................................................
Patch Set 1:
(3 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/82332/comment/a4cc4023_cc188528 : PS1, Line 7: Add (or update) chip models <list models> For this commit you can be more specific, because you know what model you adding. So instead the title (title is first line) should be:
`flashchips: Add support for GD25LF128E`
The rest of commit message is all good, keep it.
Patchset:
PS1: Great, you are getting closer, this patch is almost there! I added two comments what you need to do. If you have questions, you can reply to comments (Reply button on the comment and then Reply blue button on the top).
File flashchips.c:
https://review.coreboot.org/c/flashrom/+/82332/comment/5af4ddef_435fc060 : PS1, Line 6506: .model_id = GIGADEVICE_GD25LF128E, This macro should be added in the same patch as chip definition. So you need to add to this commit a change in include/flashchips.h , one line adding this macro for this model ID.
I can see you made CB:82190 with all the model IDs added, but you will need to add IDs in the same patch where the chip definition is added.
As an example, if you remember the other patch that was discussed on the mailing list: CB:58025. It's an example, but you can see it adds two chip definitions in flashchips.c, and two model IDs in flashchips.h.
In this patch, you will have one definition and one new model ID for it.
-
Anastasia Klimchuk (Code Review)