Attention is currently required from: Felix Singer, Nico Huber, Angel Pons.
Light has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62763 )
Change subject: ich_descriptors.c: Initialize structures
......................................................................
Patch Set 8:
(2 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/62763/comment/953464c6_3a11a68f
PS7, Line 9: Initialize structures so that they do not produce garbage
> nit (not blocking): Expand to 72 chars.
Done
File ich_descriptors.c:
https://review.coreboot.org/c/flashrom/+/62763/comment/7c181315_19a295d5
PS7, Line 1364: int ret = read_ich_descriptors_from_dump(dump, len, &cs, &desc);
> Is there anything in `desc` that is not filled when this call returns 0? […]
As far as I can see, all fields are getting filled before returning 0.
--
To view, visit https://review.coreboot.org/c/flashrom/+/62763
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I6e5bc84c6199a0731db0a9c8ef56f1215686dab2
Gerrit-Change-Number: 62763
Gerrit-PatchSet: 8
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: 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: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Fri, 25 Mar 2022 05:21:41 +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>
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Nico Huber, Anastasia Klimchuk.
Light has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62747 )
Change subject: flashrom.c, sfdp.c: Initialize dynamically allocated memory using calloc
......................................................................
Patch Set 11:
(2 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/62747/comment/8ea34064_aaa29177
PS9, Line 11: when later used in need_erase could result in undefined behaviour.
> Can that happen generally, or only when there is another bug? If it […]
Maybe intitializing them with zeroes using calloc and then onlycopying "flashrom bug!" once should be enough, instead of a repeated pattern.
https://review.coreboot.org/c/flashrom/+/62747/comment/c8acb65f_120d8fca
PS9, Line 12: Similiar
> Similar
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/62747
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I6b9269129968fb3b55b0d2a2e384c8a1aeba43ab
Gerrit-Change-Number: 62747
Gerrit-PatchSet: 11
Gerrit-Owner: Light <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
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: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Fri, 25 Mar 2022 05:09:33 +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>
Gerrit-MessageType: comment
Attention is currently required from: Light, Anastasia Klimchuk.
Hello build bot (Jenkins), Nico Huber, Anastasia Klimchuk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/62747
to look at the new patch set (#11).
Change subject: flashrom.c, sfdp.c: Initialize dynamically allocated memory using calloc
......................................................................
flashrom.c, sfdp.c: Initialize dynamically allocated memory using calloc
In flashrom_image_write variables curcontents and oldcontents are
dynamically allocated using malloc. These could remain uninitialized and
when later used in need_erase could result in undefined behaviour.
Similar reasoning for variables hbuf, hdrs, tbuf in function
prob_spi_sfdp.
So allocate them using calloc to initialize them to zeroes. Also, copy
"FLASHROM BUG!" to detect future bugs.
Change-Id: I6b9269129968fb3b55b0d2a2e384c8a1aeba43ab
Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com>
---
M flashrom.c
M sfdp.c
2 files changed, 9 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/47/62747/11
--
To view, visit https://review.coreboot.org/c/flashrom/+/62747
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I6b9269129968fb3b55b0d2a2e384c8a1aeba43ab
Gerrit-Change-Number: 62747
Gerrit-PatchSet: 11
Gerrit-Owner: Light <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Light <aarya.chaumal(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Nico Huber, Subrata Banik, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk.
Subrata Banik has uploaded a new patch set (#9) to the change originally created by Subrata Banik. ( https://review.coreboot.org/c/flashrom/+/62869 )
Change subject: ichspi: Refactor Flashrom HW Sequencing Operation
......................................................................
ichspi: Refactor Flashrom HW Sequencing Operation
List of changes:
1. Add a unified `execute SPI flash transfer` function that does:
- Check the SCIP bit prior initiate new operation.
- Start the transfer by setting address and length for transfer,
finally set FGO bit.
- Wait for the transaction to get completed/failed/timed out.
2. All HW Sequencing SPI operation uses `execute SPI flash transfer`
function
BUG=b:223630977
TEST=Able to perform read/write/erase operation on PCH 600 series
chipset (board name: Brya).
Signed-off-by: Subrata Banik <subratabanik(a)google.com>
Change-Id: Ic9fd50841449e02f476a8834f4642d6ecad36dc3
---
M ichspi.c
1 file changed, 42 insertions(+), 52 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/69/62869/9
--
To view, visit https://review.coreboot.org/c/flashrom/+/62869
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ic9fd50841449e02f476a8834f4642d6ecad36dc3
Gerrit-Change-Number: 62869
Gerrit-PatchSet: 9
Gerrit-Owner: Subrata Banik <subratabanik(a)google.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: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
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-MessageType: newpatchset
Peter Marheine has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/63104 )
Change subject: flashrom.8.tmpl: document lspcon_i2c_spi
......................................................................
flashrom.8.tmpl: document lspcon_i2c_spi
This programmer operates much the same as realtek_mst_i2c_spi, so the
I2C options are moved to a new section describing both programmers
and a short description is added for this programmer itself.
Signed-off-by: Peter Marheine <pmarheine(a)chromium.org>
Change-Id: I9ccb9694fdea29e68f062cc049efc0204917a139
---
M flashrom.8.tmpl
1 file changed, 22 insertions(+), 11 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/04/63104/1
diff --git a/flashrom.8.tmpl b/flashrom.8.tmpl
index a6637d2..952b4e3 100644
--- a/flashrom.8.tmpl
+++ b/flashrom.8.tmpl
@@ -389,6 +389,8 @@
.sp
.BR "* realtek_mst_i2c_spi" " (for SPI flash ROMs attached to Realtek DisplayPort hubs accessible through I2C)"
.sp
+.BR "* lspcon_i2c_spi" " (for SPI flash ROMs attached to Parade Technologies LSPCONs)"
+.sp
Some programmers have optional or mandatory parameters which are described
in detail in the
.B PROGRAMMER-SPECIFIC INFORMATION
@@ -1411,21 +1413,25 @@
If the passed frequency is not supported by the adapter the nearest lower
supported frequency will be used.
.SS
-.BR "realtek_mst_i2c_spi " programmer
+.BR "realtek_mst_i2c_spi " and " lspcon_i2c_spi " programmers
.IP
-This programmer supports SPI flash programming for chips attached to Realtek
-DisplayPort MST hubs, themselves accessed through I2C (tunneling SPI flash
-commands through the MST hub's I2C connection with the host).
-
-The I2C bus on which the hub is reachable must be specified by either a device
-path with the \fBdevpath\fP option:
+These programmers tunnel SPI commands through I2C-connected devices. The I2C
+bus over which communication occurs must be specified either by device path
+with the \fBdevpath\fP option:
.sp
.B " flashrom \-p realtek_mst_i2c_spi:devpath=/dev/i2c-8"
.sp
or by a bus number with the \fBbus\fP option, which implies a device path like
/dev/i2c-N where N is the specified bus number:
.sp
-.B " flashrom \-p realtek_mst_i2c_spi:bus=8"
+.B " flashrom \-p lspcon_i2c_spi:bus=8"
+
+.SS
+.BR "realtek_mst_i2c_spi " programmer
+.IP
+This programmer supports SPI flash programming for chips attached to Realtek
+DisplayPort MST hubs, themselves accessed through I2C (tunneling SPI flash
+commands through the MST hub's I2C connection with the host).
.TP
.B In-system programming (ISP) mode
.sp
@@ -1450,7 +1456,12 @@
.B " flashrom -p realtek_mst_i2c_spi:bus=0,enter-isp=1,reset-mcu=0 -E"
.br
.B " flashrom -p realtek_mst_i2c_spi:bus=0,enter-isp=0,reset-mcu=1 -w new.bin"
-.sp
+.SS
+.BR "lspcon_i2c_spi " programmer
+.IP
+This programmer supports SPI flash programming for chips attached to Parade
+Technologies DisplayPort-to-HDMI level shifter/protocol converters (LSPCONs).
+Communication to the SPI flash is tunneled through the LSPCON over I2C.
.SH EXAMPLES
To back up and update your BIOS, run
@@ -1529,8 +1540,8 @@
.B ogp
needs PCI configuration space read access and raw memory access.
.sp
-.B realtek_mst_i2c_spi
-needs userspace access to the selected I2C bus.
+.BR realtek_mst_i2c_spi " and " lspcon_i2c_spi
+need userspace access to the selected I2C bus.
.sp
On OpenBSD, you can obtain raw access permission by setting
.B "securelevel=-1"
--
To view, visit https://review.coreboot.org/c/flashrom/+/63104
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I9ccb9694fdea29e68f062cc049efc0204917a139
Gerrit-Change-Number: 63104
Gerrit-PatchSet: 1
Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-MessageType: newchange
Attention is currently required from: Felix Singer, Idwer Vollering, Angel Pons.
Light has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62761 )
Change subject: board_enable.c: Remove unnecessary assignment
......................................................................
Patch Set 8:
(2 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/62761/comment/b2bd474f_27081558
PS7, Line 7: Fix dead assignment
> Well, actually this doesn't fix something. How about: […]
Done
https://review.coreboot.org/c/flashrom/+/62761/comment/54d9a400_f6bed945
PS7, Line 9: In function board_asus_p3b_f there were two consecutive lines which
: modified the value of variable b -- b=INB(0x80);b=INB(smbba);. Since the
: value of b is not used after first assignment I have removed that
: assignment.
> If you want to comment on code in a commit message, I would format this as I did below to make it mo […]
Done
--
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: 8
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-Comment-Date: Fri, 25 Mar 2022 04:32:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net>
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Paul Menzel, Angel Pons.
Light has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62726 )
Change subject: serprog.c: Avoid calling memcpy with NULL pointer arguments
......................................................................
Patch Set 14:
(2 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/62726/comment/4474ac52_d00fcf4d
PS12, Line 10: Although, since parmlen is also 0 at that time I
: don't think it would make much difference.
> The function `sp_stream_buffer_op` is used multiple times and also with different values for parmlen […]
I'm not sure I understood you wanted to say here. I am referring to the definition of sp_stream_buffer_op.
https://review.coreboot.org/c/flashrom/+/62726/comment/35d80022_416d67ee
PS12, Line 11: Still, add a NULL check before calling memcpy to be safe.
> Wrap at 72 chars.
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/62726
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I850123237e328f9548ba7f77a01888be2cbc9e7b
Gerrit-Change-Number: 62726
Gerrit-PatchSet: 14
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: Nico Huber <nico.h(a)gmx.de>
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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Fri, 25 Mar 2022 04:31:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net>
Gerrit-MessageType: comment
Attention is currently required from: Paul Menzel, Angel Pons, Light.
Hello build bot (Jenkins), Nico Huber, Angel Pons, Anastasia Klimchuk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/62726
to look at the new patch set (#14).
Change subject: serprog.c: Avoid calling memcpy with NULL pointer arguments
......................................................................
serprog.c: Avoid calling memcpy with NULL pointer arguments
In function sp_stream_buffer_op, the variable parms might be NULL when
passed to memcpy. Although, since parmlen is also 0 at that time I
don't think it would make much difference. Still, add a NULL check
before calling memcpy to be safe.
Change-Id: I850123237e328f9548ba7f77a01888be2cbc9e7b
Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com>
---
M serprog.c
1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/26/62726/14
--
To view, visit https://review.coreboot.org/c/flashrom/+/62726
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I850123237e328f9548ba7f77a01888be2cbc9e7b
Gerrit-Change-Number: 62726
Gerrit-PatchSet: 14
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: Nico Huber <nico.h(a)gmx.de>
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: 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