Anastasia Klimchuk has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/84082?usp=email )
Change subject: flashchips: redo add GD25B256E and GD25R256E
......................................................................
Abandoned
Submitted in CB:83998
--
To view, visit https://review.coreboot.org/c/flashrom/+/84082?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: abandon
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I1596628211d508d3057893c049f1a1c95a123073
Gerrit-Change-Number: 84082
Gerrit-PatchSet: 1
Gerrit-Owner: Victor Lim <vlim(a)gigadevice.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Attention is currently required from: ZhiYuanNJ.
Nicholas Chin has posted comments on this change by ZhiYuanNJ. ( https://review.coreboot.org/c/flashrom/+/82776?usp=email )
Change subject: ch347_spi: Add spi clock frequency selection
......................................................................
Patch Set 14: Code-Review+2
(1 comment)
Patchset:
PS14:
> Nicholas, just wanted to check, do you approve the idea that we go ahead with the patch to add the f […]
That's fine with me.
--
To view, visit https://review.coreboot.org/c/flashrom/+/82776?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: If2be48929db540a6598ac0b60b37e64597156db7
Gerrit-Change-Number: 82776
Gerrit-PatchSet: 14
Gerrit-Owner: ZhiYuanNJ
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: ZhiYuanNJ
Gerrit-Comment-Date: Mon, 26 Aug 2024 12:47:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Attention is currently required from: Nikolai Artemiev, Stefan Reinauer, Victor Lim.
Anastasia Klimchuk has posted comments on this change by Victor Lim. ( https://review.coreboot.org/c/flashrom/+/83998?usp=email )
Change subject: flashchips: add GD25B256E and GD25R256E
......................................................................
Patch Set 4: Code-Review+2
(1 comment)
Patchset:
PS4:
Victor, good news, I rebased the patch again and now it got verified successfully! So it's all good, I will submit it tomorrow.
Sorry for inconvenience.
The difference could be that I rebased on top of main branch.
In comparison, for you locally this patch was a part of the chain, and so it was by default rebased on top of parent commit. And that parent commit was already submitted, so there was no actual difference of rebasing on the same parent again.
--
To view, visit https://review.coreboot.org/c/flashrom/+/83998?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: Ie733e0c2e35fa4797f5198f2c8334469b65f402c
Gerrit-Change-Number: 83998
Gerrit-PatchSet: 4
Gerrit-Owner: Victor Lim <vlim(a)gigadevice.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Victor Lim <vlim(a)gigadevice.com>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Mon, 26 Aug 2024 09:20:36 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Attention is currently required from: Nicholas Chin, ZhiYuanNJ.
Anastasia Klimchuk has posted comments on this change by ZhiYuanNJ. ( https://review.coreboot.org/c/flashrom/+/82776?usp=email )
Change subject: ch347_spi: Add spi clock frequency selection
......................................................................
Patch Set 14: Code-Review+2
(3 comments)
Patchset:
PS14:
Nicholas, just wanted to check, do you approve the idea that we go ahead with the patch to add the feature with the same default as before (15MHz)? And then later, if decided so, the default can be changed in a separate patch.
File ch347_spi.c:
https://review.coreboot.org/c/flashrom/+/82776/comment/c594fba9_4068856e?us… :
PS11, Line 291: speed_index = 1;
> But you haven't done? :) […]
Done
File ch347_spi.c:
https://review.coreboot.org/c/flashrom/+/82776/comment/491a81e1_d6d8fb84?us… :
PS13, Line 291: int speed_index = 2;
> Good sense, sorry for the slow response, updated.
All good , thank you!
--
To view, visit https://review.coreboot.org/c/flashrom/+/82776?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: If2be48929db540a6598ac0b60b37e64597156db7
Gerrit-Change-Number: 82776
Gerrit-PatchSet: 14
Gerrit-Owner: ZhiYuanNJ
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: ZhiYuanNJ
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Comment-Date: Mon, 26 Aug 2024 09:12:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Comment-In-Reply-To: ZhiYuanNJ
Attention is currently required from: Anastasia Klimchuk, Nicholas Chin.
ZhiYuanNJ has posted comments on this change by ZhiYuanNJ. ( https://review.coreboot.org/c/flashrom/+/82776?usp=email )
Change subject: ch347_spi: Add spi clock frequency selection
......................................................................
Patch Set 14:
(1 comment)
File ch347_spi.c:
https://review.coreboot.org/c/flashrom/+/82776/comment/de595cb1_572c4c96?us… :
PS13, Line 291: int speed_index = 2;
> Yes, I see you changed speed_index, thank you! […]
Good sense, sorry for the slow response, updated.
--
To view, visit https://review.coreboot.org/c/flashrom/+/82776?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: If2be48929db540a6598ac0b60b37e64597156db7
Gerrit-Change-Number: 82776
Gerrit-PatchSet: 14
Gerrit-Owner: ZhiYuanNJ
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Comment-Date: Mon, 26 Aug 2024 02:17:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Comment-In-Reply-To: ZhiYuanNJ
Attention is currently required from: Anastasia Klimchuk.
Peter Marheine has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/84078?usp=email )
Change subject: tests: Check verify op completed in full if chip memory modified
......................................................................
Patch Set 1:
(3 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/84078/comment/df163ae7_0d9344df?us… :
PS1, Line 18: Note: at the moment the test is not fully working, the following
: write test cases fail (all erase test cases pass):
Looks like it might be a fencepost error (possibly treating the upper bound of a region as exclusive rather than inclusive), since it looks like the unverified bytes might always fall at the end of a region.
File tests/erase_func_algo.c:
https://review.coreboot.org/c/flashrom/+/84078/comment/b54fe4e7_102f0525?us… :
PS1, Line 60: uint8_t
Since this is true/false, just use `bool`.
https://review.coreboot.org/c/flashrom/+/84078/comment/a966e2b2_5bc432f5?us… :
PS1, Line 77: == 0x1
I'd rather use implicit boolean conversion throughout (`if (g_state.was_modified[start])`).
--
To view, visit https://review.coreboot.org/c/flashrom/+/84078?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I3c5d55a0deb20f23f4072caac8c0dce04cc98fd4
Gerrit-Change-Number: 84078
Gerrit-PatchSet: 1
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Sun, 25 Aug 2024 23:39:56 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No