Attention is currently required from: Simon Buhrow, Thomas Heijligen, Edward O'Callaghan, Aarya.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/74872 )
Change subject: flashrom.c:Enable erase path optimisation
......................................................................
Patch Set 3: Code-Review+1
(4 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/74872/comment/24fcfb05_a166c97d
PS3, Line 7: :E
Add space after :
https://review.coreboot.org/c/flashrom/+/74872/comment/5abd943c_694fd71d
PS3, Line 9: flase
false
https://review.coreboot.org/c/flashrom/+/74872/comment/737d530c_087b5b86
PS3, Line 10: optmisatoptimisation
just one "optimisation" is enough :)
Patchset:
PS3:
I noticed the room for improvement in commit message
--
To view, visit https://review.coreboot.org/c/flashrom/+/74872
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ie13e43b18b20dbb956b569e554953a19eb32ea22
Gerrit-Change-Number: 74872
Gerrit-PatchSet: 3
Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Simon Buhrow
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Simon Buhrow
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Comment-Date: Fri, 05 May 2023 09:24:43 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Thomas Heijligen, Peter Marheine.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/74924 )
Change subject: makefile: remove gitconfig target
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/74924
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I278408bd515c5a5599b5c45c597cc66485a87082
Gerrit-Change-Number: 74924
Gerrit-PatchSet: 3
Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Fri, 05 May 2023 06:41:50 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Thomas Heijligen, Peter Marheine.
Hello build bot (Jenkins), Thomas Heijligen,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/74963
to look at the new patch set (#2).
Change subject: ni845x_spi: support building with Meson
......................................................................
ni845x_spi: support building with Meson
This adds a dependency definition for the ni845x library (attempting to
find it in the default install location, allowing the user to specify a
custom path) and adds ni845x_spi as a supported programmer for Meson.
Full functionality has not been verified because the ni845x.h header
shipped with driver version 2022 Q3 causes compile errors relating to
`kNI845XExport` not being defined when built with GCC 12 as shipped by
MSYS2, but this seems to be unrelated to Meson specifically and likely
needs to be fixed more comprehensively through some other method.
Build configuration has been manually inspected to check that the
library is being linked appropriately, since I am unable to compile
the ni845x_spi programmer successfully.
Ticket: https://ticket.coreboot.org/issues/487
Signed-off-by: Peter Marheine <pmarheine(a)chromium.org>
Change-Id: I90e85c424f97898b47bf0a3d78672d6c25af12b0
---
M meson.build
M meson_options.txt
2 files changed, 56 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/63/74963/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/74963
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I90e85c424f97898b47bf0a3d78672d6c25af12b0
Gerrit-Change-Number: 74963
Gerrit-PatchSet: 2
Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Anastasia Klimchuk, Sergii Dmytruk.
Hello build bot (Jenkins), Edward O'Callaghan, Anastasia Klimchuk, Sergii Dmytruk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/74930
to look at the new patch set (#4).
Change subject: flashrom: Use WP-based unlocking on opaque masters
......................................................................
flashrom: Use WP-based unlocking on opaque masters
Flashrom only tries to use WP-based unlocking if it detects that WP
operations are supported. However WP support was detected in a way that
ignored WP operations provided by opaque masters.
This stopped flashrom from automatically unlocking with some opaque
masters, particularly linux_mtd.
This commit also deletes part of a test that required the chip unlock
function to be called before read/write/erase operations because WP
unlocking is now used instead of chip unlocking.
BUG=b:280111380
BRANCH=none
TEST=Checked flashrom automatically unlocked flash on strongbad (MTD)
Change-Id: I1774ad64d82ae47cd085df6045e17e283855c01f
Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com>
---
M flashrom.c
M include/flash.h
M spi25_statusreg.c
M tests/chip.c
4 files changed, 28 insertions(+), 42 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/30/74930/4
--
To view, visit https://review.coreboot.org/c/flashrom/+/74930
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I1774ad64d82ae47cd085df6045e17e283855c01f
Gerrit-Change-Number: 74930
Gerrit-PatchSet: 4
Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-MessageType: newpatchset
Peter Marheine has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/74963 )
Change subject: ni845x_spi: support building with Meson
......................................................................
ni845x_spi: support building with Meson
This adds a dependency definition for the ni845x library (attempting to
find it in the default install location, allowing the user to specify a
custom path) and adds ni845x_spi as a supported programmer for Meson.
Full functionality has not been verified because the ni845x.h header
shipped with driver version 2022 Q3 causes compile errors relating to
`kNI845XExport` not being defined when built with GCC 12 as shipped by
MSYS2, but this seems to be unrelated to Meson specifically and likely
needs to be fixed more comprehensively through some other method.
Build configuration has been manually inspected to check that the
library is being linked appropriately, since I am unable to compile
the ni845x_spi programmer successfully.
Ticket: https://ticket.coreboot.org/issues/487
Change-Id: I90e85c424f97898b47bf0a3d78672d6c25af12b0
---
M meson.build
M meson_options.txt
2 files changed, 55 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/63/74963/1
diff --git a/meson.build b/meson.build
index 2784c34..f987abc 100644
--- a/meson.build
+++ b/meson.build
@@ -12,6 +12,8 @@
],
)
+fs = import('fs')
+
if get_option('classic_cli').enabled() and get_option('default_library') == 'shared'
error('''
Cannot build cli_classic with shared libflashrom. Use \'-Dclassic_cli=disabled\' to disable the cli,
@@ -140,6 +142,24 @@
libftdi1 = dependency('libftdi1', required : group_ftdi)
libjaylink = dependency('libjaylink', required : group_jlink)
+ni845x_path = []
+foreach p : get_option('ni845x_search_dir')
+ # include_directories() below fails if any given directory doesn't exist
+ if fs.is_dir(p)
+ ni845x_path += p
+ endif
+endforeach
+libni845x = declare_dependency(
+ dependencies : cc.find_library(
+ 'ni845x',
+ dirs : ni845x_path,
+ has_headers : [ 'ni845x.h' ],
+ header_include_directories : include_directories(ni845x_path),
+ required : get_option('programmer').contains('ni845x_spi'),
+ ),
+ include_directories : [ni845x_path],
+)
+
subdir('platform')
if systems_hwaccess.contains(host_machine.system())
@@ -361,6 +381,13 @@
'flags' : [ '-DCONFIG_MSTARDDC_SPI=1' ],
'default' : false
},
+ 'ni845x_spi': {
+ 'systems' : [ 'windows' ],
+ 'deps' : [ libni845x ],
+ 'srcs' : files('ni845x_spi.c'),
+ 'flags' : [ '-DCONFIG_NI845X_SPI=1' ],
+ 'default' : false
+ },
'nic3com' : {
'systems' : systems_hwaccess,
'cpu_families' : cpus_port_io,
diff --git a/meson_options.txt b/meson_options.txt
index 91d3045..c2b2d12 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -13,10 +13,13 @@
'asm106x', 'atahpt', 'atapromise', 'atavia', 'buspirate_spi', 'ch341a_spi', 'ch347_spi','dediprog',
'developerbox_spi', 'digilent_spi', 'dirtyjtag_spi', 'drkaiser', 'dummy', 'ft2232_spi',
'gfxnvidia', 'internal', 'it8212', 'jlink_spi', 'linux_mtd', 'linux_spi', 'mediatek_i2c_spi',
- 'mstarddc_spi', 'nic3com', 'nicintel', 'nicintel_eeprom', 'nicintel_spi', 'nicnatsemi',
+ 'mstarddc_spi', 'ni845x_spi', 'nic3com', 'nicintel', 'nicintel_eeprom', 'nicintel_spi', 'nicnatsemi',
'nicrealtek', 'ogp_spi', 'parade_lspcon', 'pickit2_spi', 'pony_spi', 'raiden_debug_spi',
'rayer_spi', 'realtek_mst_i2c_spi', 'satamv', 'satasii', 'serprog', 'stlinkv3_spi', 'usbblaster_spi',
], description: 'Active programmers')
+option('ni845x_search_dir', type : 'array',
+ value : ['C:/Program Files (x86)/National Instruments/NI-845x/MS Visual C/', 'C:/Program Files/National Instruments/NI-845x/MS Visual C/'],
+ description : 'directories to search for NI-845x libraries')
option('llvm_cov', type : 'feature', value : 'disabled', description : 'build for llvm code coverage')
option('man-pages', type : 'feature', value : 'auto', description : 'build the man-page for classic_cli')
option('documentation', type : 'feature', value : 'auto', description : 'build the html documentation')
--
To view, visit https://review.coreboot.org/c/flashrom/+/74963
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I90e85c424f97898b47bf0a3d78672d6c25af12b0
Gerrit-Change-Number: 74963
Gerrit-PatchSet: 1
Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-MessageType: newchange
Attention is currently required from: Anastasia Klimchuk, Sergii Dmytruk.
Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/74930 )
Change subject: flashrom: Use WP-based unlocking on opaque masters
......................................................................
Patch Set 3:
(2 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/74930/comment/a611c1ea_3eac0aa9
PS2, Line 9: detectes
> nit: `detects`
Done
File tests/chip.c:
https://review.coreboot.org/c/flashrom/+/74930/comment/a9ecb171_f936e942
PS2, Line 78: static int unlock_chip(struct flashctx *flash)
> Why was this stuff dropped?
This test was failing because it expected the chip unlock function to be called, but flashrom now uses the dummy WP to unlock the chip instead.
After removing the parts of the test that checked for unlock calls, there was no reason to keep the injector around.
Also it is less important to test that unlock is called now that WP unlocking is the default supported path.
--
To view, visit https://review.coreboot.org/c/flashrom/+/74930
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I1774ad64d82ae47cd085df6045e17e283855c01f
Gerrit-Change-Number: 74930
Gerrit-PatchSet: 3
Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Comment-Date: Fri, 05 May 2023 03:49:31 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-MessageType: comment
Attention is currently required from: Thomas Heijligen, Anastasia Klimchuk.
Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/74923 )
Change subject: doc: add information about contributing patches
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS2:
> Thank you so much for starting this Peter! You have done a big part of work to convert this to rst. […]
Sure, I'll leave this around as a reference but leave the actual migration up to you.
--
To view, visit https://review.coreboot.org/c/flashrom/+/74923
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I12c5fac99f1dc9c05e3dd4d5554e937a57f64d6f
Gerrit-Change-Number: 74923
Gerrit-PatchSet: 3
Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Fri, 05 May 2023 02:53:06 +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: Thomas Heijligen, Anastasia Klimchuk.
Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/74924 )
Change subject: makefile: remove gitconfig target
......................................................................
Patch Set 3:
(2 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/74924/comment/a3ac4507_26038480
PS2, Line 14:
> Could you please add […]
Done
Patchset:
PS2:
> I have updated the current dev guidelines on wiki (https://www.flashrom. […]
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/74924
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I278408bd515c5a5599b5c45c597cc66485a87082
Gerrit-Change-Number: 74924
Gerrit-PatchSet: 3
Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Fri, 05 May 2023 02:51:55 +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: Thomas Heijligen, Peter Marheine.
Hello build bot (Jenkins), Thomas Heijligen, Anastasia Klimchuk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/74923
to look at the new patch set (#3).
Change subject: doc: add information about contributing patches
......................................................................
doc: add information about contributing patches
This is copied from the "Development Guidelines" page on the wiki, with
some unimportant historical bits removed (description of old branch
names that are no longer meaningful) and notes on adding flash chips
moved to a new page. Sections are also reordered to (hopefully) more
closely match the order in which prospective contributors will care
about them.
Change-Id: I12c5fac99f1dc9c05e3dd4d5554e937a57f64d6f
Signed-off-by: Peter Marheine <pmarheine(a)chromium.org>
---
M doc/contact.rst
A doc/dev_guide/contributing.rst
M doc/dev_guide/index.rst
A doc/dev_guide/new_chips.rst
4 files changed, 438 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/23/74923/3
--
To view, visit https://review.coreboot.org/c/flashrom/+/74923
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I12c5fac99f1dc9c05e3dd4d5554e937a57f64d6f
Gerrit-Change-Number: 74923
Gerrit-PatchSet: 3
Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Thomas Heijligen, Peter Marheine.
Hello build bot (Jenkins), Thomas Heijligen, Anastasia Klimchuk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/74924
to look at the new patch set (#3).
Change subject: makefile: remove gitconfig target
......................................................................
makefile: remove gitconfig target
There's basically no benefit to running `make` over directly running the
script that configures hooks, and implementing similar support in Meson
is difficult. Remove the Makefile target to achieve feature parity
between the build systems.
Ticket: https://ticket.coreboot.org/issues/486
Signed-off-by: Peter Marheine <pmarheine(a)chromium.org>
Change-Id: I278408bd515c5a5599b5c45c597cc66485a87082
---
M Makefile
1 file changed, 16 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/24/74924/3
--
To view, visit https://review.coreboot.org/c/flashrom/+/74924
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I278408bd515c5a5599b5c45c597cc66485a87082
Gerrit-Change-Number: 74924
Gerrit-PatchSet: 3
Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-MessageType: newpatchset