September 2023 - flashrom-gerrit

[S] Change in flashrom[master]: Remove dependency on C23 __has_include()
by Anastasia Klimchuk (Code Review) 08 Sep '23

08 Sep '23

Attention is currently required from: Anton Samsonov, Anton Samsonov. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/77089?usp=email ) Change subject: Remove dependency on C23 __has_include() ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: I would avoid introducing new pre-processor conditionals, because we are planning to go in the opposite direction and git rid of them all. Instead resolve this with the help of build system. Meson build file already has a few of examples of `cc.has_header` and `cc.has_function` and it can decide to use different source files/headers depending on whether header present or not. This effort (removing pre-processor conditionals) needs makefile to be removed first, which in turn can only happen after the release is done. But, all of that is on the near future plans. Anton, how do you feel about it? -- To view, visit https://review.coreboot.org/c/flashrom/+/77089?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic544963ffd29626ae0a21bdddb1c78850cc43ec6 Gerrit-Change-Number: 77089 Gerrit-PatchSet: 1 Gerrit-Owner: Anton Samsonov <devel(a)zxlab.ru> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Anton Samsonov <avscomputing(a)gmail.com> Gerrit-Attention: Anton Samsonov <avscomputing(a)gmail.com> Gerrit-Attention: Anton Samsonov <devel(a)zxlab.ru> Gerrit-Comment-Date: Fri, 08 Sep 2023 10:53:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[master]: doc: Add html_logo and html_title to sphinx config
by Anastasia Klimchuk (Code Review) 08 Sep '23

08 Sep '23

Attention is currently required from: Alexander Goncharov, Stefan Reinauer, Thomas Heijligen. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/77438?usp=email ) Change subject: doc: Add html_logo and html_title to sphinx config ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > What's the point of displaying logo on every page? It's not a commercial logo, we're not selling any […] I guess I was trying to do the same as wiki (re displaying logo on every page). But maybe it's not necessary. You are right about alignment, it hurts eyes a little bit. From a quick look, I didn't find the easy way to change alignment (apart from providing custom css files, which I don't want to do). Maybe I will leave this patch hanging for some time, I will read documentation again to see what are all available config options to display logo. -- To view, visit https://review.coreboot.org/c/flashrom/+/77438?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9a530f7143880393fffc2a97327416f2cd688586 Gerrit-Change-Number: 77438 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Fri, 08 Sep 2023 08:06:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Alexander Goncharov <chat(a)joursoir.net> Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[master]: doc: Add html_logo and html_title to sphinx config
by Alexander Goncharov (Code Review) 05 Sep '23

05 Sep '23

Attention is currently required from: Anastasia Klimchuk, Stefan Reinauer, Thomas Heijligen. Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/77438?usp=email ) Change subject: doc: Add html_logo and html_title to sphinx config ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > I missed the elephant in the room, there is a way to add logo in sphinx config. […] What's the point of displaying logo on every page? It's not a commercial logo, we're not selling anything. And to be honest, I don't like the way it displays. Toctree is left-aligned, but the logo is centered, so it looks kind of weird. At least the "flashrom" title should be centered as well, but I have no idea whether that's possible to do via the standard sphinx configuration. -- To view, visit https://review.coreboot.org/c/flashrom/+/77438?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9a530f7143880393fffc2a97327416f2cd688586 Gerrit-Change-Number: 77438 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 05 Sep 2023 10:17:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: Remove dependency on C23 __has_include()
by Alexander Goncharov (Code Review) 05 Sep '23

05 Sep '23

Attention is currently required from: Anastasia Klimchuk, Anton Samsonov, Anton Samsonov. Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/77089?usp=email ) Change subject: Remove dependency on C23 __has_include() ...................................................................... Patch Set 1: (3 comments) Patchset: PS1: > In my understanding, this patch may be needed for all compilers based on legacy versions of EDG fron […] Appreciate your explanation. I don't think there's any point in describing it in the commit message. However, if you include something like "This patch has been tested on EDG 4.14 / GCC 5.5" (using the version you tested on, of course) it would make it easier for anyone who wants to test this patch to reproduce it File include/cli_classic.h: https://review.coreboot.org/c/flashrom/+/77089/comment/a283f041_ff4d2e23 : PS1, Line 21: #if !defined(__has_include) && (__STDC_VERSION__ < 202300L) : #include <getopt.h> : #define HAVE_GETOPT_H 1 If we cannot identify whether this header file exists on the user's system, then let's assume it doesn't. So I suggest we drop this branch https://review.coreboot.org/c/flashrom/+/77089/comment/adc99f67_4fb99312 : PS1, Line 18: #if HAVE_GETOPT_H : #include <getopt.h> : #elif !defined(HAVE_GETOPT_H) : #if !defined(__has_include) && (__STDC_VERSION__ < 202300L) : #include <getopt.h> : #define HAVE_GETOPT_H 1 : #elif __has_include(<getopt.h>) : #include <getopt.h> : #define HAVE_GETOPT_H 1 : #else : #define HAVE_GETOPT_H 0 : #endif /* __has_include() */ : #endif /* HAVE_GETOPT_H */ : : #if !HAVE_GETOPT_H Our and coreboot code styles say nothing about directives. I'm having trouble reading if's branches without nesting. I would suggest to use indent after `#`, at least the coding style does not forbid it (I hope so). What do you think, Anastasia? -- To view, visit https://review.coreboot.org/c/flashrom/+/77089?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic544963ffd29626ae0a21bdddb1c78850cc43ec6 Gerrit-Change-Number: 77089 Gerrit-PatchSet: 1 Gerrit-Owner: Anton Samsonov <devel(a)zxlab.ru> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Anton Samsonov <avscomputing(a)gmail.com> Gerrit-Attention: Anton Samsonov <avscomputing(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Anton Samsonov <devel(a)zxlab.ru> Gerrit-Comment-Date: Tue, 05 Sep 2023 09:59:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anton Samsonov <avscomputing(a)gmail.com> Comment-In-Reply-To: Alexander Goncharov <chat(a)joursoir.net> Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[master]: flashchips: Add WP features for Winbond W25X20
by Anastasia Klimchuk (Code Review) 03 Sep '23

03 Sep '23

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/77530?usp=email ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: flashchips: Add WP features for Winbond W25X20 ...................................................................... flashchips: Add WP features for Winbond W25X20 WP-related registers list from official datasheet https://www.winbond.com/resource-files/w25x20cl_revf%2020150806.pdf Commandline options tested with ft2232_spi-based "Tigard" programmer: wp-disable wp-enable wp-list wp-status wp-range=0,0 wp-range=0,0x00040000 Signed-off-by: Vasily Galkin <galkin-vv(a)ya.ru> Change-Id: I82c0cc52ca2a78d27f513234cc12d3e09d8905a5 Reviewed-on: https://review.coreboot.org/c/flashrom/+/77530 Reviewed-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M flashchips.c 1 file changed, 8 insertions(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Sergii Dmytruk: Looks good to me, approved diff --git a/flashchips.c b/flashchips.c index ce68e95..009624f 100644 --- a/flashchips.c +++ b/flashchips.c @@ -19803,7 +19803,7 @@ .total_size = 256, .page_size = 256, .feature_bits = FEATURE_WRSR_WREN, - .tested = TEST_OK_PREW, + .tested = TEST_OK_PREWB, .probe = PROBE_SPI_RDID, .probe_timing = TIMING_ZERO, .block_erasers = @@ -19824,6 +19824,13 @@ .write = SPI_CHIP_WRITE256, .read = SPI_CHIP_READ, .voltage = {2700, 3600}, + .reg_bits = + { + .srp = {STATUS1, 7, RW}, + .bp = {{STATUS1, 2, RW}, {STATUS1, 3, RW}}, + .tb = {STATUS1, 5, RW}, + }, + .decode_range = DECODE_RANGE_SPI25, }, { -- To view, visit https://review.coreboot.org/c/flashrom/+/77530?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I82c0cc52ca2a78d27f513234cc12d3e09d8905a5 Gerrit-Change-Number: 77530 Gerrit-PatchSet: 3 Gerrit-Owner: Vasily Galkin Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[XS] Change in flashrom[master]: spi25_statusreg: add verbose output on status registers read results
by Anastasia Klimchuk (Code Review) 03 Sep '23

03 Sep '23

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/77529?usp=email ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: spi25_statusreg: add verbose output on status registers read results ...................................................................... spi25_statusreg: add verbose output on status registers read results Useful during enabling/debugging WP operations on various SPI chips Signed-off-by: Vasily Galkin <galkin-vv(a)ya.ru> Change-Id: Ibc8e9229ab5d6578479564d11cc7aff9442e24ad Reviewed-on: https://review.coreboot.org/c/flashrom/+/77529 Reviewed-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M spi25_statusreg.c 1 file changed, 1 insertion(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved Sergii Dmytruk: Looks good to me, approved diff --git a/spi25_statusreg.c b/spi25_statusreg.c index fa276f7..bb56ee1 100644 --- a/spi25_statusreg.c +++ b/spi25_statusreg.c @@ -262,6 +262,7 @@ } *value = readarr[0]; + msg_cspew("%s: read_cmd 0x%02x returned 0x%02x\n", __func__, read_cmd, readarr[0]); return 0; } -- To view, visit https://review.coreboot.org/c/flashrom/+/77529?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ibc8e9229ab5d6578479564d11cc7aff9442e24ad Gerrit-Change-Number: 77529 Gerrit-PatchSet: 3 Gerrit-Owner: Vasily Galkin Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[master]: flashchips: add WP features for W25X* analogous to tested W25X20
by Anastasia Klimchuk (Code Review) 01 Sep '23

01 Sep '23

Attention is currently required from: Nikolai Artemiev, Sergii Dmytruk, Stefan Reinauer, Vasily Galkin. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/77531?usp=email ) Change subject: flashchips: add WP features for W25X* analogous to tested W25X20 ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > Note about "Merge conflict" status of this change. […] Yes you are right the "Merge conflict" status is because this change depends on previous one in the chain (it is modifying the same file). -- To view, visit https://review.coreboot.org/c/flashrom/+/77531?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie69660a6f69e3cac31c5565792f401e69d43f8b8 Gerrit-Change-Number: 77531 Gerrit-PatchSet: 2 Gerrit-Owner: Vasily Galkin Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Vasily Galkin Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Fri, 01 Sep 2023 13:41:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Vasily Galkin Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[master]: flashchips: Add WP features for Winbond W25X20
by Anastasia Klimchuk (Code Review) 01 Sep '23

01 Sep '23

Attention is currently required from: Nikolai Artemiev, Stefan Reinauer, Vasily Galkin. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/77530?usp=email ) Change subject: flashchips: Add WP features for Winbond W25X20 ...................................................................... Patch Set 2: (1 comment) Patchset: PS1: > It's better to rebase. […] If rebase is auto, without any merge conflicts, Gerrit keeps the code-review votes on the patch. Which means if the patch is approved it stays approved. If rebase requires any manual resolving conflicts then Gerrit removes existing code review votes. -- To view, visit https://review.coreboot.org/c/flashrom/+/77530?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I82c0cc52ca2a78d27f513234cc12d3e09d8905a5 Gerrit-Change-Number: 77530 Gerrit-PatchSet: 2 Gerrit-Owner: Vasily Galkin Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Vasily Galkin Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Fri, 01 Sep 2023 13:34:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Vasily Galkin Comment-In-Reply-To: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashchips: add WP features for W25X* analogous to tested W25X20
by Vasily Galkin (Code Review) 01 Sep '23

01 Sep '23

Attention is currently required from: Nikolai Artemiev, Sergii Dmytruk, Stefan Reinauer. Vasily Galkin has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/77531?usp=email ) Change subject: flashchips: add WP features for W25X* analogous to tested W25X20 ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: Note about "Merge conflict" status of this change. I just rebased this patch series, but it is still here. It seems to be a misundestanding between Gerrit and me. This change is a part of a patch series (same topic) and to make this change mergeable to master - the previous changes are needed. It is direct descendant of https://review.coreboot.org/c/flashrom/+/77530 in git history. No any complex trees here, just a linear patch series So I suppose that it will disappear after merging previous patch, and see no way to remove it earlier. -- To view, visit https://review.coreboot.org/c/flashrom/+/77531?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie69660a6f69e3cac31c5565792f401e69d43f8b8 Gerrit-Change-Number: 77531 Gerrit-PatchSet: 2 Gerrit-Owner: Vasily Galkin Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Fri, 01 Sep 2023 13:23:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[master]: flashchips: Add support for MXIC MX25U25643G
by Anastasia Klimchuk (Code Review) 01 Sep '23

01 Sep '23

Attention is currently required from: Name of user not set #1005084, Nikolai Artemiev, Stefan Reinauer. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/77581?usp=email ) Change subject: flashchips: Add support for MXIC MX25U25643G ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: xianzheng, I wanted to check, is this patch a new version of CB:76853 ? Is this ready for review? For the future, in a case like this, you need to push a new patchset to existing patch (instead of creating a new patch). This means locally you do git commit --amend and then you check that Change-Id stays the same in the commit message. Change-Id is the way Gerrit knows the change belongs to the same patch. For this patch you can leave as is, for future keep an eye on Change-Id and create new patchsets *for the same patch*. We also have info about working with git and gerrit in dev guides: https://www.flashrom.org/dev_guide/development_guide.html thank you for contributing! -- To view, visit https://review.coreboot.org/c/flashrom/+/77581?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id6c4254a02800f04c091009d19a628435fc20fce Gerrit-Change-Number: 77581 Gerrit-PatchSet: 1 Gerrit-Owner: Name of user not set #1005084 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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Name of user not set #1005084 Gerrit-Comment-Date: Fri, 01 Sep 2023 13:17:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit September 2023