Attention is currently required from: Nikolai Artemiev.
Brian Norris has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/75991?usp=email )
Change subject: flashrom: only perform WP unlock for write/erase operations
......................................................................
Patch Set 3: Code-Review+1
--
To view, visit https://review.coreboot.org/c/flashrom/+/75991?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: I5dc66474a0b7969b51b86ac9f5daa2c95ae968f1
Gerrit-Change-Number: 75991
Gerrit-PatchSet: 3
Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Brian Norris <briannorris(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Wed, 12 Jul 2023 22:41:21 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Angel Pons, Thomas Heijligen, WereCatf.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58173?usp=email )
Change subject: flashchips: Add XTX XT25F64B
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS2:
> Hello WereCatf, I am sorry that your patches got lost :( I mean this one and CB:58134. […]
Okay, looks like you are busy or left, I updated the patchset and replaced func pointers with enums, also rebased.
--
To view, visit https://review.coreboot.org/c/flashrom/+/58173?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: I369db9ccfd5319d28424d10f77aab49ec73a8836
Gerrit-Change-Number: 58173
Gerrit-PatchSet: 3
Gerrit-Owner: WereCatf <werecatf(a)outlook.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: WereCatf <werecatf(a)outlook.com>
Gerrit-Comment-Date: Wed, 12 Jul 2023 13:10:42 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Angel Pons, Thomas Heijligen, WereCatf.
Anastasia Klimchuk has uploaded a new patch set (#3) to the change originally created by WereCatf. ( https://review.coreboot.org/c/flashrom/+/58173?usp=email )
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: flashchips: Add XTX XT25F64B
......................................................................
flashchips: Add XTX XT25F64B
Datasheet:
http://file2.dzsc.com/product/19/06/22/216185_132959081.pdf
Tested probe, read, erase and write with CH341a.
Signed-off-by: Nita Vesa <werecatf(a)outlook.com>
Change-Id: I369db9ccfd5319d28424d10f77aab49ec73a8836
Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org>
---
M flashchips.c
M include/flashchips.h
2 files changed, 43 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/73/58173/3
--
To view, visit https://review.coreboot.org/c/flashrom/+/58173?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: I369db9ccfd5319d28424d10f77aab49ec73a8836
Gerrit-Change-Number: 58173
Gerrit-PatchSet: 3
Gerrit-Owner: WereCatf <werecatf(a)outlook.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: WereCatf <werecatf(a)outlook.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Anne Macedo.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/76251?usp=email )
Change subject: flashchips: Add support for PUYA P25Q40H
......................................................................
Patch Set 1:
(5 comments)
Patchset:
PS1:
Anne, thank you for your patch! I have few comments. If you have any questions, you are welcome to ask.
File flashchips.c:
https://review.coreboot.org/c/flashrom/+/76251/comment/378d4ce6_c0576523 :
PS1, Line 13793: .manufacture_id = 0x85,
: .model_id = 0x6013,
These two ids should go to include/flashchips.h
At the moment there is no section for Puya, so it needs to be added.
There is a patch under review right now, which adds the Puya section and few ids (not the same, different chips), here is that patch CB:58134
I am not sure that original author of CB:58134 will return back, so it could be me finishing that patch.
So maybe you can add Puya section and your ids into include/flashchips.h (using CB:58134 as example), and then I can rebase CB:58134 on the top of your patch.
https://review.coreboot.org/c/flashrom/+/76251/comment/0c7d3b58_bcf5bb8a :
PS1, Line 13797: .feature_bits = FEATURE_WRSR_WREN,
It also supports FEATURE_OTP
https://review.coreboot.org/c/flashrom/+/76251/comment/55426530_fa3b36aa :
PS1, Line 13813: .eraseblocks = { {512 * 1024, 1} },
: .block_erase = SPI_BLOCK_ERASE_81,
Command 81 is doing page erase for 256 bytes. This is the smallest eraseblock, should be the first (the current definition sets this as if it is full chip erase, which is not).
And, there is a chip erase in the datasheet, command 60 or C7, these two should come last in the list.
https://review.coreboot.org/c/flashrom/+/76251/comment/5d93e54b_d87aced9 :
PS1, Line 13817: SPI_PRETTYPRINT_STATUS_REGISTER_BP3_SRWD
I think this should be BP4, and same for the unlock
--
To view, visit https://review.coreboot.org/c/flashrom/+/76251?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: I58c2c0467ef19f98b7435e84b6978550f029af70
Gerrit-Change-Number: 76251
Gerrit-PatchSet: 1
Gerrit-Owner: Anne Macedo <retpolanne(a)posteo.net>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Anne Macedo <retpolanne(a)posteo.net>
Gerrit-Comment-Date: Wed, 12 Jul 2023 10:08:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Brian Norris, Nikolai Artemiev.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/75991?usp=email )
Change subject: flashrom: only perform WP unlock for write/erase operations
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/75991?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: I5dc66474a0b7969b51b86ac9f5daa2c95ae968f1
Gerrit-Change-Number: 75991
Gerrit-PatchSet: 3
Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Brian Norris <briannorris(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Brian Norris <briannorris(a)chromium.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Tue, 11 Jul 2023 00:01:56 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Alexander Goncharov, Anastasia Klimchuk, Angel Pons, Edward O'Callaghan, Nikolai Artemiev, Stefan Reinauer, Swift Geek (Sebastian Grzywna), Thomas Heijligen.
Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/75906?usp=email )
Change subject: doc: Add new Development guidelines
......................................................................
Patch Set 5: Code-Review+2
(4 comments)
File doc/dev_guide/development_guide.rst:
https://review.coreboot.org/c/flashrom/+/75906/comment/94f860b3_c9c4888c :
PS5, Line 161: approproately
typo: appropriately
https://review.coreboot.org/c/flashrom/+/75906/comment/deec150a_25262a4f :
PS5, Line 235: -s
If the commit is already signed-off, will this add another signoff (probably not intended), or will it avoid adding a new one?
https://review.coreboot.org/c/flashrom/+/75906/comment/faaaf5be_3d899126 :
PS5, Line 235: you
typo: your
File doc/dev_guide/development_guidelines.rst:
https://review.coreboot.org/c/flashrom/+/75906/comment/0254372f_7d87ff4e :
PS2, Line 7: Set up the git repository and dev environment
> Renamed to development guide, I am happy to rename this doc! […]
Fine by me; as long as the document title isn't overly specific (renaming it fixed that) and the section headings make sense (my earlier suggestions worked toward that) then it shouldn't be difficult to navigate.
--
To view, visit https://review.coreboot.org/c/flashrom/+/75906?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: I7fe9ab2e27fead8e795138294219b11240f15928
Gerrit-Change-Number: 75906
Gerrit-PatchSet: 5
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
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-CC: Swift Geek (Sebastian Grzywna) <swiftgeek(a)gmail.com>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: Swift Geek (Sebastian Grzywna) <swiftgeek(a)gmail.com>
Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Comment-Date: Mon, 10 Jul 2023 04:12:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Anastasia Klimchuk, Patrick Georgi, Stefan Reinauer, Thomas Heijligen.
Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/75727?usp=email )
Change subject: doc: Add documentation license
......................................................................
Patch Set 3: Code-Review+1
(2 comments)
File doc/documentation_license.rst:
https://review.coreboot.org/c/flashrom/+/75727/comment/d4e08f92_57a3b7af :
PS3, Line 2: flashrom documentation license
> I don't think it's critical. […]
My point is that we do not write "flashrom developers documentation" or "flashrom contact" because it's obvious that all stuff is related to flashrom and not coreboot or whatever.
Also, it takes up more than one line in the toctree. But if everybody is fine with that, then I don't mind.
https://review.coreboot.org/c/flashrom/+/75727/comment/b2dd3eba_b117490c :
PS3, Line 5: doc/
> The way we did it for coreboot (and its wiki) was that every time we found a useful nugget of inform […]
Thanks for the clarification! Then resolved.
--
To view, visit https://review.coreboot.org/c/flashrom/+/75727?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: Ied858b5f1e9c4a83a6eb21dcefb288c4474b08c0
Gerrit-Change-Number: 75727
Gerrit-PatchSet: 3
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org>
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: Patrick Georgi <patrick(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: Fri, 07 Jul 2023 11:45:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Patrick Georgi <patrick(a)coreboot.org>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Comment-In-Reply-To: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-MessageType: comment