May 2024 - flashrom-gerrit - mail.coreboot.org

[M] Change in flashrom[main]: flashchips: Add support for MXIC MX25R4035F
by DZ (Code Review) May 29, 2024

May 29, 2024

Attention is currently required from: Anastasia Klimchuk, Nikolai Artemiev, Stefan Reinauer. DZ has posted comments on this change by DZ. ( https://review.coreboot.org/c/flashrom/+/81838?usp=email ) Change subject: flashchips: Add support for MXIC MX25R4035F ...................................................................... Patch Set 5: (1 comment) Patchset: PS3: > Daniel, just checking, do you have time to rebase and fix merge conflict? Because otherwise the patc […] Thank you for your help. You can just rebase it and solve the conflict, thanks. -- To view, visit https://review.coreboot.org/c/flashrom/+/81838?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: I91dbc4735bf232e0b1dce72c7f06be967d35ebfb Gerrit-Change-Number: 81838 Gerrit-PatchSet: 5 Gerrit-Owner: DZ <danielzhang(a)mxic.com.cn> 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: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Wed, 29 May 2024 06:11:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[M] Change in flashrom[main]: flashchips: Add support for MXIC MX25R4035F
by Anastasia Klimchuk (Code Review) May 29, 2024

May 29, 2024

Attention is currently required from: DZ, Nikolai Artemiev, Stefan Reinauer. Anastasia Klimchuk has posted comments on this change by DZ. ( https://review.coreboot.org/c/flashrom/+/81838?usp=email ) Change subject: flashchips: Add support for MXIC MX25R4035F ...................................................................... Patch Set 5: (1 comment) Patchset: PS3: > Okay another one got submitted, so this one only need to be rebased on top of head Daniel, just checking, do you have time to rebase and fix merge conflict? Because otherwise the patch is ready. If you currently busy, I can try help. But I thought to ask you first? -- To view, visit https://review.coreboot.org/c/flashrom/+/81838?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: I91dbc4735bf232e0b1dce72c7f06be967d35ebfb Gerrit-Change-Number: 81838 Gerrit-PatchSet: 5 Gerrit-Owner: DZ <danielzhang(a)mxic.com.cn> 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: DZ <danielzhang(a)mxic.com.cn> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Wed, 29 May 2024 05:46:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[S] Change in flashrom[main]: ch347_spi: Add driver support for CH347F packaging
by Anastasia Klimchuk (Code Review) May 29, 2024

May 29, 2024

Attention is currently required from: Nicholas Chin, Paul Menzel, ZhiYuanNJ. Anastasia Klimchuk has posted comments on this change by ZhiYuanNJ. ( https://review.coreboot.org/c/flashrom/+/82193?usp=email ) Change subject: ch347_spi: Add driver support for CH347F packaging ...................................................................... Patch Set 11: (4 comments) Patchset: PS11: ZhiYuanNJ thank you so much for your work! I have two small comments, but that would be the last ones. PS11: > The current code looks fine to me, and my CH347T still seems to work fine with it (I'm assuming ZhiY […] Ah okay, got it. I usually have a quick look before submitting anyway. By the way, you review was really good! File ch347_spi.c: https://review.coreboot.org/c/flashrom/+/82193/comment/28f5b8e0_3e113581?us… : PS11, Line 330: Could you please return back these two lines? (330 and 332) It's easier to read with some spacing between logical blocks. https://review.coreboot.org/c/flashrom/+/82193/comment/1a12aa9c_a2f1a161?us… : PS11, Line 70: /* TODO: Set this depending on the mode */ I guess this comment can be removed? Thank you for fixing the TODO. -- To view, visit https://review.coreboot.org/c/flashrom/+/82193?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: I693baf1a0d9dc20757f56fba626b5f5ad20f71dd Gerrit-Change-Number: 82193 Gerrit-PatchSet: 11 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Attention: ZhiYuanNJ Gerrit-Comment-Date: Wed, 29 May 2024 04:54:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Nicholas Chin <nic.c3.14(a)gmail.com>

1 0

[S] Change in flashrom[main]: ch347_spi: Add driver support for CH347F packaging
by Nicholas Chin (Code Review) May 29, 2024

May 29, 2024

Attention is currently required from: Anastasia Klimchuk, Paul Menzel, ZhiYuanNJ. Nicholas Chin has posted comments on this change by ZhiYuanNJ. ( https://review.coreboot.org/c/flashrom/+/82193?usp=email ) Change subject: ch347_spi: Add driver support for CH347F packaging ...................................................................... Patch Set 11: (1 comment) Patchset: PS11: > Nicholas, just checking, do you approve the patch? […] The current code looks fine to me, and my CH347T still seems to work fine with it (I'm assuming ZhiYuanNJ tested it with a CH347F; I don't have that variant). I approve of it, but I did want someone else to take a look as I'm not very active in flashrom development/review. -- To view, visit https://review.coreboot.org/c/flashrom/+/82193?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: I693baf1a0d9dc20757f56fba626b5f5ad20f71dd Gerrit-Change-Number: 82193 Gerrit-PatchSet: 11 Gerrit-Owner: ZhiYuanNJ Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: ZhiYuanNJ Gerrit-Comment-Date: Wed, 29 May 2024 03:11:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[S] Change in flashrom[main]: ch347_spi: Add driver support for CH347F packaging
by Anastasia Klimchuk (Code Review) May 29, 2024

May 29, 2024

Attention is currently required from: Paul Menzel, ZhiYuanNJ. Anastasia Klimchuk has posted comments on this change by ZhiYuanNJ. ( https://review.coreboot.org/c/flashrom/+/82193?usp=email ) Change subject: ch347_spi: Add driver support for CH347F packaging ...................................................................... Patch Set 11: (1 comment) Patchset: PS11: Nicholas, just checking, do you approve the patch? You did a great review, thank you so much! -- To view, visit https://review.coreboot.org/c/flashrom/+/82193?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: I693baf1a0d9dc20757f56fba626b5f5ad20f71dd Gerrit-Change-Number: 82193 Gerrit-PatchSet: 11 Gerrit-Owner: ZhiYuanNJ Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: ZhiYuanNJ Gerrit-Comment-Date: Wed, 29 May 2024 02:49:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[S] Change in flashrom[main]: flashchips: Add XTX XT25F128B
by Anastasia Klimchuk (Code Review) May 29, 2024

May 29, 2024

Attention is currently required from: Nikolai Artemiev, Robert Marko, Stefan Reinauer. Anastasia Klimchuk has posted comments on this change by Robert Marko. ( https://review.coreboot.org/c/flashrom/+/82676?usp=email ) Change subject: flashchips: Add XTX XT25F128B ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/82676?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: I37084bd66bc7a8f93d6533ab0d67aa2528786299 Gerrit-Change-Number: 82676 Gerrit-PatchSet: 1 Gerrit-Owner: Robert Marko <robimarko(a)gmail.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: Robert Marko <robimarko(a)gmail.com> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Wed, 29 May 2024 02:19:38 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[S] Change in flashrom[main]: ch347_spi: Add driver support for CH347F packaging
by Nicholas Chin (Code Review) May 29, 2024

May 29, 2024

Attention is currently required from: Paul Menzel, ZhiYuanNJ. Nicholas Chin has posted comments on this change by ZhiYuanNJ. ( https://review.coreboot.org/c/flashrom/+/82193?usp=email ) Change subject: ch347_spi: Add driver support for CH347F packaging ...................................................................... Patch Set 11: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/82193?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: I693baf1a0d9dc20757f56fba626b5f5ad20f71dd Gerrit-Change-Number: 82193 Gerrit-PatchSet: 11 Gerrit-Owner: ZhiYuanNJ Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: ZhiYuanNJ Gerrit-Comment-Date: Wed, 29 May 2024 02:07:26 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[S] Change in flashrom[main]: Correct get_flash_region() to use inclusive upper bounds
by Anastasia Klimchuk (Code Review) May 29, 2024

May 29, 2024

Attention is currently required from: Aarya, Nikolai Artemiev, Peter Marheine. Anastasia Klimchuk has posted comments on this change by Peter Marheine. ( https://review.coreboot.org/c/flashrom/+/82496?usp=email ) Change subject: Correct get_flash_region() to use inclusive upper bounds ...................................................................... Patch Set 7: (1 comment) Patchset: PS6: > I happened to be doing some other firmware work on the same machine I tested reading on and discover […] Oh wow! So your patch not only removed tech debt, but also helped to find another bug! very cool :) -- To view, visit https://review.coreboot.org/c/flashrom/+/82496?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: Ia0ca867aef33b598c2697fc264807fa5e29683ec Gerrit-Change-Number: 82496 Gerrit-PatchSet: 7 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Wed, 29 May 2024 02:01:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org>

1 0

[S] Change in flashrom[main]: flashchips.c: Add support for XM25RU256C
by Anastasia Klimchuk (Code Review) May 29, 2024

May 29, 2024

Attention is currently required from: Georg Gottleuber, Nikolai Artemiev, Stefan Reinauer. Anastasia Klimchuk has posted comments on this change by Georg Gottleuber. ( https://review.coreboot.org/c/flashrom/+/82101?usp=email ) Change subject: flashchips.c: Add support for XM25RU256C ...................................................................... Patch Set 3: (2 comments) Patchset: PS3: > I double-checked with the datasheet: looks good. Only one additional ". […] Is this for 4K sector? You can add it to the array of block_erasers, next to existing SPI_BLOCK_ERASE_20 entry. I saw this before, you can check there is a bunch of chip definitions in flashchips.c with both SPI_BLOCK_ERASE_21 and SPI_BLOCK_ERASE_20 defined. If you will be adding 21, then if you can run erase once again on this chip, that would be perfect! File flashchips.c: https://review.coreboot.org/c/flashrom/+/82101/comment/b9058c31_815e102b?us… : PS3, Line 22118: TEST_UNTESTED This probably slipped from a previous version of the patch? You already changed tested status to `TEST_OK_PREW` which is what you need (because you tested). -- To view, visit https://review.coreboot.org/c/flashrom/+/82101?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: I431474a662304d09438e274706d3fc9cfbbe0bd6 Gerrit-Change-Number: 82101 Gerrit-PatchSet: 3 Gerrit-Owner: Georg Gottleuber <ggo(a)tuxedo.de> 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: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Georg Gottleuber <ggo(a)tuxedo.de> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Wed, 29 May 2024 01:59:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Georg Gottleuber <ggo(a)tuxedo.de>

1 0

[S] Change in flashrom[main]: Correct get_flash_region() to use inclusive upper bounds
by Peter Marheine (Code Review) May 29, 2024

May 29, 2024

Attention is currently required from: Aarya, Nikolai Artemiev. Peter Marheine has posted comments on this change by Peter Marheine. ( https://review.coreboot.org/c/flashrom/+/82496?usp=email ) Change subject: Correct get_flash_region() to use inclusive upper bounds ...................................................................... Patch Set 7: (1 comment) Patchset: PS6: > I tested reading flash on a yaviks board both with and without this patch, and it looks okay. […] I happened to be doing some other firmware work on the same machine I tested reading on and discovered a `region.end - 1` in erase code that only exists in ChromeOS which breaks erase, but after patching that the whole read/erase/write process worked fine. -- To view, visit https://review.coreboot.org/c/flashrom/+/82496?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: Ia0ca867aef33b598c2697fc264807fa5e29683ec Gerrit-Change-Number: 82496 Gerrit-PatchSet: 7 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Wed, 29 May 2024 01:13:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org>

1 0