Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Nikolai Artemiev.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67391 )
Change subject: drivers/: Make 'internal_delay' the default unless defined
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/67391
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I17460bc2c0aebcbb48c8dfa052b260991525cc49
Gerrit-Change-Number: 67391
Gerrit-PatchSet: 4
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: 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-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Sat, 17 Sep 2022 07:57:23 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Nico Huber, Angel Pons, Nikolai Artemiev.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67391 )
Change subject: drivers/: Make 'internal_delay' the default unless defined
......................................................................
Patch Set 4:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/67391/comment/fb96ed98_8934fa75
PS3, Line 13: and paves way
: to remove the 'programmer' global handle.
> That would be one option, yes. I'm fine with dropping this part. […]
OK, done.
--
To view, visit https://review.coreboot.org/c/flashrom/+/67391
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I17460bc2c0aebcbb48c8dfa052b260991525cc49
Gerrit-Change-Number: 67391
Gerrit-PatchSet: 4
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: 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-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Sat, 17 Sep 2022 04:49:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net>
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Nikolai Artemiev.
Hello Felix Singer, build bot (Jenkins), Thomas Heijligen, Angel Pons, Nikolai Artemiev, Peter Marheine,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/67391
to look at the new patch set (#4).
Change subject: drivers/: Make 'internal_delay' the default unless defined
......................................................................
drivers/: Make 'internal_delay' the default unless defined
Drop the explicit need to specify the default 'internal_delay'
callback function pointer. This is a reasonable default for
every other driver in the tree with only two exceptions.
Thus this simplifies driver development.
Change-Id: I17460bc2c0aebcbb48c8dfa052b260991525cc49
Signed-off-by: Edward O'Callaghan <quasisec(a)google.com>
---
M atahpt.c
M atapromise.c
M atavia.c
M buspirate_spi.c
M dediprog.c
M developerbox_spi.c
M digilent_spi.c
M drkaiser.c
M dummyflasher.c
M flashrom.c
M ft2232_spi.c
M gfxnvidia.c
M internal.c
M it8212.c
M jlink_spi.c
M linux_mtd.c
M linux_spi.c
M mediatek_i2c_spi.c
M mstarddc_spi.c
M ni845x_spi.c
M nic3com.c
M nicintel.c
M nicintel_eeprom.c
M nicintel_spi.c
M nicnatsemi.c
M nicrealtek.c
M ogp_spi.c
M parade_lspcon.c
M pickit2_spi.c
M pony_spi.c
M raiden_debug_spi.c
M rayer_spi.c
M realtek_mst_i2c_spi.c
M satamv.c
M satasii.c
M stlinkv3_spi.c
M usbblaster_spi.c
37 files changed, 22 insertions(+), 42 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/91/67391/4
--
To view, visit https://review.coreboot.org/c/flashrom/+/67391
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I17460bc2c0aebcbb48c8dfa052b260991525cc49
Gerrit-Change-Number: 67391
Gerrit-PatchSet: 4
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: 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-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Edward O'Callaghan, Jonathon Hall, Nikolai Artemiev, Peter Marheine.
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67695 )
Change subject: drivers: Move (un)map_flash_region from programmer to bus master
......................................................................
Patch Set 2:
(4 comments)
Patchset:
PS2:
Moving the map/unmap functions out of the programmer_entry struct is a good thing. But Imo it then should go into the *_master struct, rather than into an extra struct, getting registered at the same time. If I got the code right, only par_masters, especially the par_master_internal, need to be able to map/unmap memory depending on the chip size. So for the long term it could be better to modify the chip probing such that a mapping function is only called on par_masters.
I'm open for anything that solves this problem sustainably.
File atapromise.c:
https://review.coreboot.org/c/flashrom/+/67695/comment/9f898b01_8a450771
PS2, Line 133: atapromise_map
This may not be needed any longer, because `atapromise_map` has the same implementation as `fallback_map`.
CB:67655
File atavia.c:
https://review.coreboot.org/c/flashrom/+/67695/comment/aaab064e_5191c399
PS2, Line 147: atavia_map
This may also not be needed, CB:67656
But I have to test this on real hardware :/
File ichspi.c:
https://review.coreboot.org/c/flashrom/+/67695/comment/1a71112e_d961e181
PS2, Line 1865: &mapper_phys
I don't think mapping is used here. As far as I know it is only used for the par_master in internal.c
Nico, do you know more?
--
To view, visit https://review.coreboot.org/c/flashrom/+/67695
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I9c3df6ae260bcdb246dfb0cd8e043919609b014b
Gerrit-Change-Number: 67695
Gerrit-PatchSet: 2
Gerrit-Owner: Jonathon Hall <jonathon.hall(a)puri.sm>
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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Jonathon Hall <jonathon.hall(a)puri.sm>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Fri, 16 Sep 2022 22:57:46 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, Nikolai Artemiev, Peter Marheine.
Jonathon Hall has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67695 )
Change subject: drivers: Move (un)map_flash_region from programmer to bus master
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
This enables https://review.coreboot.org/c/flashrom/+/67696/2, which is needed to get flashrom -p internal to work on PCH systems with >16 MB flash.
--
To view, visit https://review.coreboot.org/c/flashrom/+/67695
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I9c3df6ae260bcdb246dfb0cd8e043919609b014b
Gerrit-Change-Number: 67695
Gerrit-PatchSet: 2
Gerrit-Owner: Jonathon Hall <jonathon.hall(a)puri.sm>
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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Fri, 16 Sep 2022 21:58:00 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Jonathon Hall has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/67696 )
Change subject: ichspi: Do not mmap for hwseq
......................................................................
ichspi: Do not mmap for hwseq
Flash chips >16 MB can't be directly mapped into memory, this usually
results in a failure since the attempted mapping would overlap with PCI
devices. The mapping isn't needed for these methods.
This change is likely also correct for ICH swseq and possibly others,
these would need to be tested.
Test: Probe and read firmware on 400-series PCH with 16 MB and 32 MB
SPI flash
Change-Id: Ic7761327b4072dcb9917000db9eaacc9404e6823
Signed-off-by: Jonathon Hall <jonathon.hall(a)puri.sm>
---
M ichspi.c
1 file changed, 22 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/96/67696/1
diff --git a/ichspi.c b/ichspi.c
index 77822fd..5dd5c9c 100644
--- a/ichspi.c
+++ b/ichspi.c
@@ -2207,7 +2207,7 @@
register_opaque_master(&opaque_master_ich_hwseq, NULL, &g_hwseq_data);
} else {
- register_spi_master(&spi_master_ich9, NULL, NULL);
+ register_spi_master(&spi_master_ich9, &mapper_phys, NULL);
}
return 0;
@@ -2252,7 +2252,7 @@
/* Not sure if it speaks all these bus protocols. */
internal_buses_supported &= BUS_LPC | BUS_FWH;
ich_generation = CHIPSET_ICH7;
- register_spi_master(&spi_master_via, NULL, NULL);
+ register_spi_master(&spi_master_via, &mapper_phys, NULL);
msg_pdbg("0x00: 0x%04x (SPIS)\n", mmio_readw(ich_spibar + 0));
msg_pdbg("0x02: 0x%04x (SPIC)\n", mmio_readw(ich_spibar + 2));
--
To view, visit https://review.coreboot.org/c/flashrom/+/67696
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ic7761327b4072dcb9917000db9eaacc9404e6823
Gerrit-Change-Number: 67696
Gerrit-PatchSet: 1
Gerrit-Owner: Jonathon Hall <jonathon.hall(a)puri.sm>
Gerrit-MessageType: newchange
Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/67312 )
Change subject: tests: Add workaround to allow tests mock fileno on FreeBSD
......................................................................
tests: Add workaround to allow tests mock fileno on FreeBSD
fileno can be a function and a macro in FreeBSD, depending on whether
the environment in multi-threaded or single-threaded. For single
thread, macro is used. Macro is expanded into a inline code which
accesses private field of file descriptor. This is impossible to
mock in tests.
Pretending that environment is multi-threaded makes fileno function
to be called instead of a macro. Function can be mocked for tests.
BUG=b:237606255
TEST=ninja test
tests pass on two environments:
1) FreeBSD 13.1-RELEASE-p2 GENERIC amd64
2) Debian 5.17.11 x86_64 GNU/Linux
Change-Id: I3789ea9107a4cf8033cf59bb96d3c82aa58de026
TICKET: https://ticket.coreboot.org/issues/411
Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/67312
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Thomas Heijligen <src(a)posteo.de>
Reviewed-by: Peter Marheine <pmarheine(a)chromium.org>
---
M tests/tests.c
1 file changed, 41 insertions(+), 0 deletions(-)
Approvals:
build bot (Jenkins): Verified
Thomas Heijligen: Looks good to me, approved
Peter Marheine: Looks good to me, but someone else must approve
diff --git a/tests/tests.c b/tests/tests.c
index 8e3fdde..7f20069 100644
--- a/tests/tests.c
+++ b/tests/tests.c
@@ -370,6 +370,17 @@
{
int ret = 0;
+#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
+ /*
+ * Pretending to be a multithreaded environment so that `fileno`
+ * is called as a function (and not as a macro).
+ * fileno macro in FreeBSD is expanded into inline access of
+ * private field of file descriptor, which is impossible to mock.
+ * Calling fileno as a function allows the test to mock it.
+ */
+ __isthreaded = 1;
+#endif
+
if (argc > 1)
cmocka_set_test_filter(argv[1]);
--
To view, visit https://review.coreboot.org/c/flashrom/+/67312
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I3789ea9107a4cf8033cf59bb96d3c82aa58de026
Gerrit-Change-Number: 67312
Gerrit-PatchSet: 7
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: 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-CC: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-CC: Idwer Vollering <vidwer(a)gmail.com>
Gerrit-MessageType: merged
Attention is currently required from: Edward O'Callaghan, Evan Benn.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67472 )
Change subject: flashrom_tester: Fix cargo check and clippy warnings
......................................................................
Patch Set 3: Code-Review+2
(1 comment)
Patchset:
PS3:
If Peter is happy with it, so am I
--
To view, visit https://review.coreboot.org/c/flashrom/+/67472
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I50c5af61e06df1bb6956f347cb6806a7eca6ce0e
Gerrit-Change-Number: 67472
Gerrit-PatchSet: 3
Gerrit-Owner: Evan Benn <evanbenn(a)google.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Evan Benn <evanbenn(a)google.com>
Gerrit-Comment-Date: Fri, 16 Sep 2022 07:32:36 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment