Attention is currently required from: Felix Singer, Nico Huber, Paul Menzel, Angel Pons.
Light has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62725 )
Change subject: libflashrom.c: Fix unintialized value passed to function
......................................................................
Patch Set 17:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/62725/comment/527b9404_f11e27ee
PS15, Line 11: value
> Yeah, I agree with Nico. […]
I think the current explanation should be satisfactory.
--
To view, visit https://review.coreboot.org/c/flashrom/+/62725
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Iacbd7bf9cdf897cc2a732c1dc6568845a4ab804d
Gerrit-Change-Number: 62725
Gerrit-PatchSet: 17
Gerrit-Owner: Light <aarya.chaumal(a)gmail.com>
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: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Tue, 29 Mar 2022 08:08:13 +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: Light <aarya.chaumal(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Nico Huber, Paul Menzel, Angel Pons, Light.
Hello Felix Singer, build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/62725
to look at the new patch set (#17).
Change subject: libflashrom.c: Fix unintialized value passed to function
......................................................................
libflashrom.c: Fix unintialized value passed to function
In function flash_layout_read_from_ifd variable chip_layout remains
uninitialized if prepare_flash_access returns false. This uninitialized
variable (which contains a garbage value) is passed to
flashrom_layout_release. Thus initialize it with NULL. For completeness,
also initialize dump_layout with NULL.
Change-Id: Iacbd7bf9cdf897cc2a732c1dc6568845a4ab804d
Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com>
---
M libflashrom.c
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/25/62725/17
--
To view, visit https://review.coreboot.org/c/flashrom/+/62725
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Iacbd7bf9cdf897cc2a732c1dc6568845a4ab804d
Gerrit-Change-Number: 62725
Gerrit-PatchSet: 17
Gerrit-Owner: Light <aarya.chaumal(a)gmail.com>
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: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Light <aarya.chaumal(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Felix Singer, Idwer Vollering, Angel Pons, Light, Keith Hui.
Hello Felix Singer, Keith Hui, build bot (Jenkins), Nico Huber, Angel Pons, Anastasia Klimchuk, Keith Hui,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/62761
to look at the new patch set (#11).
Change subject: board_enable.c: Remove unnecessary assignment
......................................................................
board_enable.c: Remove unnecessary assignment
In function board_asus_p3b_f there were two consecutive lines which
modified the value of variable b
// Do something with b
b=INB(0x80);
b=INB(smbba);
//Do something with b
Since the value of b is not used after first assignment, remove the
first assignment.
Change-Id: I7458b416a69fd5e2aa300ca49d1352b6074ad0bc
Tested-by: Keith Hui <buurin(a)gmail.com>
Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com>
---
M board_enable.c
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/61/62761/11
--
To view, visit https://review.coreboot.org/c/flashrom/+/62761
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I7458b416a69fd5e2aa300ca49d1352b6074ad0bc
Gerrit-Change-Number: 62761
Gerrit-PatchSet: 11
Gerrit-Owner: Light <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Keith Hui <buurin(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Idwer Vollering <vidwer(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Idwer Vollering <vidwer(a)gmail.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Light <aarya.chaumal(a)gmail.com>
Gerrit-Attention: Keith Hui <buurin(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Felix Singer, Idwer Vollering, Angel Pons, Light, Keith Hui.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62761 )
Change subject: board_enable.c: Remove unnecessary assignment
......................................................................
Patch Set 10:
(1 comment)
Patchset:
PS10:
> I saw it but missed replying. My bad. […]
Thanks so much Keith!
Aarya, you need to add a tag into commit message, above or below Signed-off-by (just add manually):
Tested-by: Keith Hui <buurin(a)gmail.com>
and remember this for future :)
You may need this again, if someone else tests your patch. Then you ask whether it's ok and add a tag (if yes). Testing is an important part of making changes to the code, it is a good idea to recognise people who helped you with that.
--
To view, visit https://review.coreboot.org/c/flashrom/+/62761
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I7458b416a69fd5e2aa300ca49d1352b6074ad0bc
Gerrit-Change-Number: 62761
Gerrit-PatchSet: 10
Gerrit-Owner: Light <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Keith Hui <buurin(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Idwer Vollering <vidwer(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Idwer Vollering <vidwer(a)gmail.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Light <aarya.chaumal(a)gmail.com>
Gerrit-Attention: Keith Hui <buurin(a)gmail.com>
Gerrit-Comment-Date: Tue, 29 Mar 2022 07:00:13 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Comment-In-Reply-To: Keith Hui <buurin(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, Angel Pons, Light.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62764 )
Change subject: ich_descriptors.c: Ensure unsigned types >=0 on to prevent underflow
......................................................................
Patch Set 16:
(1 comment)
File ich_descriptors.c:
https://review.coreboot.org/c/flashrom/+/62764/comment/d93bf89f_bd50579a
PS14, Line 501: for (j = 0; j < (size_t)min(num_regions, 12); j++)
> Yeah that's fine :)
That was a question for Edward :) Let's wait for reply.
But, good to know that Aarya agrees with the idea too!
--
To view, visit https://review.coreboot.org/c/flashrom/+/62764
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I5921cc571f3dca5188ca1973dba6ececbcbe2f39
Gerrit-Change-Number: 62764
Gerrit-PatchSet: 16
Gerrit-Owner: Light <aarya.chaumal(a)gmail.com>
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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Light <aarya.chaumal(a)gmail.com>
Gerrit-Comment-Date: Tue, 29 Mar 2022 06:37:11 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
Comment-In-Reply-To: Light <aarya.chaumal(a)gmail.com>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk.
Light has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62764 )
Change subject: ich_descriptors.c: Ensure unsigned types >=0 on to prevent underflow
......................................................................
Patch Set 16:
(1 comment)
File ich_descriptors.c:
https://review.coreboot.org/c/flashrom/+/62764/comment/80147788_cbac6579
PS14, Line 501: for (j = 0; j < (size_t)min(num_regions, 12); j++)
> I agree this is not the max number of regions (initialised as 10 or 16 above), but let's assume it's […]
Yeah that's fine :)
--
To view, visit https://review.coreboot.org/c/flashrom/+/62764
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I5921cc571f3dca5188ca1973dba6ececbcbe2f39
Gerrit-Change-Number: 62764
Gerrit-PatchSet: 16
Gerrit-Owner: Light <aarya.chaumal(a)gmail.com>
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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
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-Comment-Date: Tue, 29 Mar 2022 05:07:53 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
Comment-In-Reply-To: Light <aarya.chaumal(a)gmail.com>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons.
Sam McNally has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62282 )
Change subject: ichspi: Add Jasper Lake support
......................................................................
Patch Set 6:
(3 comments)
File chipset_enable.c:
https://review.coreboot.org/c/flashrom/+/62282/comment/9e0475cf_50fcb86e
PS6, Line 1013: jlk
Jasper lake is usually abbreviated as JSL.
File ich_descriptors.c:
https://review.coreboot.org/c/flashrom/+/62282/comment/0a138fe7_88417718
PS6, Line 313: case CHIPSET_JASPER_LAKE:
From the SPI programming guide I've seen (v0.8), this should use freq_str[1].
File ichspi.c:
https://review.coreboot.org/c/flashrom/+/62282/comment/a3fabf29_72a2e0da
PS6, Line 2063: msg_pdbg("Enabling hardware sequencing by default for Apollo/Gemini/Elkhart Lake.\n");
Update the message to include Jasper Lake.
--
To view, visit https://review.coreboot.org/c/flashrom/+/62282
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ib942d0b8942fe0a991b2af0b187414818485153d
Gerrit-Change-Number: 62282
Gerrit-PatchSet: 6
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Sam McNally <sammc(a)google.com>
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-Comment-Date: Tue, 29 Mar 2022 02:52:52 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Idwer Vollering, Angel Pons, Light.
Keith Hui has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62761 )
Change subject: board_enable.c: Remove unnecessary assignment
......................................................................
Patch Set 10: Code-Review+1
(1 comment)
Patchset:
PS10:
> Not sure whether Keith you have seen it, or maybe you don't care about Tested-by tag? […]
I saw it but missed replying. My bad.
It's true though I seem to be the only one to have build tested and reported back. Fine by me either way, and go ahead and merge.
ps. flashrom is userspace, and I/O port reads actually call into the kernel. There's plenty delay right there per read. Not so-and-so microseconds accurate, but enough for our use case.
--
To view, visit https://review.coreboot.org/c/flashrom/+/62761
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I7458b416a69fd5e2aa300ca49d1352b6074ad0bc
Gerrit-Change-Number: 62761
Gerrit-PatchSet: 10
Gerrit-Owner: Light <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Keith Hui <buurin(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Idwer Vollering <vidwer(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Idwer Vollering <vidwer(a)gmail.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Light <aarya.chaumal(a)gmail.com>
Gerrit-Comment-Date: Tue, 29 Mar 2022 02:08:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, Light.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63130 )
Change subject: pony_spi.c: Extract out get_params to simplify init
......................................................................
Patch Set 3:
(3 comments)
Patchset:
PS3:
Thanks! :)
File pony_spi.c:
https://review.coreboot.org/c/flashrom/+/63130/comment/b0fe3489_1357547b
PS3, Line 167: bool have_device;
This should probably be a separate patch? (changing variable type int -> bool)
https://review.coreboot.org/c/flashrom/+/63130/comment/8492ee3e_70ef320b
PS3, Line 190: serialport_shutdown
This one line should probably also go to a very useful separate patch.
--
To view, visit https://review.coreboot.org/c/flashrom/+/63130
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I364febc05c870683cbad114583762b0c006f4bac
Gerrit-Change-Number: 63130
Gerrit-PatchSet: 3
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Light <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Light <aarya.chaumal(a)gmail.com>
Gerrit-Comment-Date: Tue, 29 Mar 2022 01:07:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment