Attention is currently required from: Felix Singer, Angel Pons, Nikolai Artemiev.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58737 )
Change subject: ichspi: Extract initialisation of swseq and hwseq into a function
......................................................................
Patch Set 9:
(1 comment)
Patchset:
PS9:
Rebased because of the latest changes in CB:58735
--
To view, visit https://review.coreboot.org/c/flashrom/+/58737
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I7d62b1b380e497b82dcae1284d752204cc541bd3
Gerrit-Change-Number: 58737
Gerrit-PatchSet: 9
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Tue, 01 Feb 2022 06:21:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Nico Huber, Edward O'Callaghan, Angel Pons, Nikolai Artemiev, Nicholas Chin.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58735 )
Change subject: ichspi: Split very long init function into two
......................................................................
Patch Set 8:
(3 comments)
Patchset:
PS7:
> Thank you so much Nicolas for help!
Nicolas, sorry for delay, I uploaded new version which (hopefully) fixes ich_spibar and ich_generation initialisation. Is it possible for you to try run it again on your machine? Thank you so much!
PS7:
> My memory tries to tell me something... back in 2012 we were […]
Wow this is a very special computer that you found! ;) Does it still work?
10 years ago... this is impressive! Maybe you can keep it as a memory even if it doesn't work anymore :)
If it still works... maybe you can keep it to test things sometimes?
File ichspi.c:
https://review.coreboot.org/c/flashrom/+/58735/comment/0a290381_292b00e5
PS7, Line 1771: ich_generation = ich_gen;
: ich_spibar = spibar;
> These two are globals and also affect the ICH7 paths. I guess we […]
Thank you so much for your analysis! This is helpful.
ich_spibar is used in program_opcodes ichspi.c#L566, which is called at some point of init (as I understand?), and ich_generation is needed for run_opcode ichspi.c#L1081 ? I missed this :( global state makes it hard to understand what is used where, I am hoping to remove global state one day.
I moved initialisation of both ich_spibar and ich_generation back into entry point ich_init_spi. This last patchset I tested on the device that I have, and now of course it needs to be tested on ICH7.
The question that I have in mind: does the code looks good enough to re-test it again? At the moment I have no way to run init_ich7_spi code, so I only know that the code builds.
--
To view, visit https://review.coreboot.org/c/flashrom/+/58735
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I6789bc456a4878e6555831ae0b80ecbdbf62938b
Gerrit-Change-Number: 58735
Gerrit-PatchSet: 8
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
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-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Comment-Date: Tue, 01 Feb 2022 06:19:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Comment-In-Reply-To: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Nikolai Artemiev, Nicholas Chin.
Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, Nikolai Artemiev,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/58735
to look at the new patch set (#8).
Change subject: ichspi: Split very long init function into two
......................................................................
ichspi: Split very long init function into two
ich_init_spi is very long, but logically it can be split. Init
function detects the chipset and then the rest of operations depends
on the chipset.
Init function is more readable now, it consists of only a switch.
Initialisation of hwseq and swseq that used to happen in the
beginning of init function now moved to init_ich_default, because
hwseq and swseq are only used for chipsets served by init_ich_default.
TEST ME ON ICH7
BUG=b:204488958
TEST=Check that the following scenarios still behave properly:
1) probe-read-verify-erase section-write-reboot
on Intel octopus board with GD25LQ128C/GD25LQ128D/GD25LQ128E
2) probe and read on Panther Point (7 series PCH)
Change-Id: I6789bc456a4878e6555831ae0b80ecbdbf62938b
Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org>
Tested-by: Anastasia Klimchuk <aklm(a)chromium.org>
Tested-by: Nico Huber <nico.h(a)gmx.de>
---
M ichspi.c
1 file changed, 271 insertions(+), 258 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/35/58735/8
--
To view, visit https://review.coreboot.org/c/flashrom/+/58735
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I6789bc456a4878e6555831ae0b80ecbdbf62938b
Gerrit-Change-Number: 58735
Gerrit-PatchSet: 8
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
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: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Felix Singer, Angel Pons, Anastasia Klimchuk, Nikolai Artemiev.
Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, Nikolai Artemiev,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/58737
to look at the new patch set (#9).
Change subject: ichspi: Extract initialisation of swseq and hwseq into a function
......................................................................
ichspi: Extract initialisation of swseq and hwseq into a function
Initialisation of swseq_data and hwseq_data gets its own function,
which is called from init_ich_default. This makes init_ich_default
more readable.
This patch also gives a name to (previously anonymous) struct
swseq_data. Its sibling struct hwseq_data already has a name. Structs
need names to be able to declare function parameters.
BUG=b:204488958
TEST=Check that the following scenarios still behave properly:
1) probe-read-verify-erase section-write-reboot
on Intel octopus board with GD25LQ128C/GD25LQ128D/GD25LQ128E
2) probe and read on Panther Point (7 series PCH)
Change-Id: I7d62b1b380e497b82dcae1284d752204cc541bd3
Tested-by: Anastasia Klimchuk <aklm(a)chromium.org>
Tested-by: Nico Huber <nico.h(a)gmx.de>
Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org>
---
M ichspi.c
1 file changed, 57 insertions(+), 50 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/37/58737/9
--
To view, visit https://review.coreboot.org/c/flashrom/+/58737
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I7d62b1b380e497b82dcae1284d752204cc541bd3
Gerrit-Change-Number: 58737
Gerrit-PatchSet: 9
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
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-MessageType: newpatchset
Attention is currently required from: Felix Singer, Nico Huber, Angel Pons.
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61523 )
Change subject: Makefile: check if librt is an external library
......................................................................
Patch Set 2:
(4 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/61523/comment/05b77a1d_b3669760
PS1, Line 9: OpenBSD
> e.g. […]
Done
https://review.coreboot.org/c/flashrom/+/61523/comment/82335864_474b74d2
PS1, Line 10: to link
> linking
Done
https://review.coreboot.org/c/flashrom/+/61523/comment/697989d9_9ec0cafb
PS1, Line 10: therefor
> therefor_e_
Done
https://review.coreboot.org/c/flashrom/+/61523/comment/ff3f25a4_a080ebc0
PS1, Line 10: Detect
> Thus, detect [... […]
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/61523
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I2c1668a350aa0806fccfb4e9cd8b04861f085ee9
Gerrit-Change-Number: 61523
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: 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-Comment-Date: Mon, 31 Jan 2022 23:41:40 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net>
Gerrit-MessageType: comment
Thomas Heijligen has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/61525 )
Change subject: platform/pci.h: adapt NetBSD include path to pkg-config
......................................................................
platform/pci.h: adapt NetBSD include path to pkg-config
The pkg-config include path resolves the pciutils directory. So the
include is only the pci.h file.
Change-Id: I69dc8184d1d012fb695770cbf6f7c64e5a024453
Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com>
---
M platform/pci.h
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/25/61525/1
diff --git a/platform/pci.h b/platform/pci.h
index f63529b..93bc163 100644
--- a/platform/pci.h
+++ b/platform/pci.h
@@ -15,7 +15,7 @@
#define index shadow_workaround_index
#if defined (__NetBSD__)
-#include <pciutils/pci.h>
+#include <pci.h>
#else
#include <pci/pci.h>
#endif
--
To view, visit https://review.coreboot.org/c/flashrom/+/61525
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I69dc8184d1d012fb695770cbf6f7c64e5a024453
Gerrit-Change-Number: 61525
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-MessageType: newchange
Attention is currently required from: Felix Singer, Nico Huber, Angel Pons.
Hello Felix Singer, Nico Huber, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/61524
to look at the new patch set (#2).
Change subject: Makefile: use pkg-config --libs --static for LDFLAGS
......................................................................
Makefile: use pkg-config --libs --static for LDFLAGS
The --static flag of pkg-config returns also the LDFLAGS which are
required to link the library static. Use this flag to successfully
link against static libraries when the shared variant is not available.
This is the case in OpenBSD with libpci.
Change-Id: I6029a096c1ceca625789d18c88119d912d79bc0e
Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com>
---
M Makefile
M Makefile.include
2 files changed, 1 insertion(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/24/61524/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/61524
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I6029a096c1ceca625789d18c88119d912d79bc0e
Gerrit-Change-Number: 61524
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: 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-MessageType: newpatchset
Attention is currently required from: Nico Huber, Thomas Heijligen, Angel Pons.
Hello Felix Singer, Nico Huber, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/61523
to look at the new patch set (#2).
Change subject: Makefile: check if librt is an external library
......................................................................
Makefile: check if librt is an external library
Some systems, e.g. OpenBSD, have clock_gettime / librt build into the
libc and therefore fail linking against it with -lrt. Thus, detect this
and link only if needed.
Change-Id: I2c1668a350aa0806fccfb4e9cd8b04861f085ee9
Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com>
---
M Makefile
M Makefile.include
2 files changed, 8 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/23/61523/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/61523
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I2c1668a350aa0806fccfb4e9cd8b04861f085ee9
Gerrit-Change-Number: 61523
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: newpatchset