Attention is currently required from: Nico Huber.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52877 )
Change subject: buspirate_spi.c: Separate shutdown from failed init cleanup
......................................................................
Patch Set 5:
(1 comment)
File buspirate_spi.c:
https://review.coreboot.org/c/flashrom/+/52877/comment/0c5958ac_55421c9c
PS4, Line 620: ret = 1;
> Actually yes, return values come from serialport_write and serialport_read, and those seems to retur […]
So yes, I will do this and also suggestion from CB:52958. Just wanted to say I am off for next week, and then return back and do everything.
--
To view, visit https://review.coreboot.org/c/flashrom/+/52877
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I04d57e2552d23a1a4a906c68539f0ccefc13e8a0
Gerrit-Change-Number: 52877
Gerrit-PatchSet: 5
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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Miklós Márton <martonmiklosqdev(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Comment-Date: Fri, 11 Jun 2021 01:12:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Sam McNally, Edward O'Callaghan, Peter Marheine.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55403 )
Change subject: lspcon: restart MPU on programmer shutdown
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
File lspcon_i2c_spi.c:
https://review.coreboot.org/c/flashrom/+/55403/comment/860483c5_f512210f
PS1, Line 314: (unsigned)1e8
h-how does this work
--
To view, visit https://review.coreboot.org/c/flashrom/+/55403
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I66cd68f8f6905a2bfaf5b085bf08dcb218f42855
Gerrit-Change-Number: 55403
Gerrit-PatchSet: 1
Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Sam McNally <sammc(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Sam McNally <sammc(a)google.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Fri, 11 Jun 2021 00:32:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, Angel Pons.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52958 )
Change subject: buspirate_spi.c: Refactor singleton states into reentrant pattern
......................................................................
Patch Set 4:
(1 comment)
File buspirate_spi.c:
https://review.coreboot.org/c/flashrom/+/52958/comment/2d1f9618_c15b328f
PS4, Line 315: int buspirate_spi_init(void)
> Perhaps a bit out of scope for this commit however you may wish to add one or two change before this […]
I agree it is large. Also I agree this is out of scope for this commit. So just to confirm, you suggest to add one or two patches before this one in this chain, do I understand correctly?
--
To view, visit https://review.coreboot.org/c/flashrom/+/52958
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I418bbfff15fb126b042fbc9be09dbf59f4d243b8
Gerrit-Change-Number: 52958
Gerrit-PatchSet: 4
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: 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-Comment-Date: Fri, 11 Jun 2021 00:27:05 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52877 )
Change subject: buspirate_spi.c: Separate shutdown from failed init cleanup
......................................................................
Patch Set 4:
(1 comment)
File buspirate_spi.c:
https://review.coreboot.org/c/flashrom/+/52877/comment/b46394a6_cd22575f
PS4, Line 620: ret = 1;
> For another patch: We could get rid of these `ret = 1;` statements and just use the existing non-zer […]
Actually yes, return values come from serialport_write and serialport_read, and those seems to return 1s for any error.
--
To view, visit https://review.coreboot.org/c/flashrom/+/52877
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I04d57e2552d23a1a4a906c68539f0ccefc13e8a0
Gerrit-Change-Number: 52877
Gerrit-PatchSet: 4
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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Miklós Márton <martonmiklosqdev(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Comment-Date: Fri, 11 Jun 2021 00:19:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52946 )
Change subject: programmer_table: move each entry to the associated programmer source
......................................................................
Patch Set 12:
(1 comment)
File programmer.h:
https://review.coreboot.org/c/flashrom/+/52946/comment/9b8d2853_5ddd7da9
PS4, Line 402: #if CONFIG_DUMMY == 1
> > Please just drop all these `#if`. The only difference it makes is […]
This is invaluable, thank you so much! I saved (copied) this whole thread for myself.
--
To view, visit https://review.coreboot.org/c/flashrom/+/52946
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I3d02bd789f0299e936eb86819b3b15b5ea2bb921
Gerrit-Change-Number: 52946
Gerrit-PatchSet: 12
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
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: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Xiang W <wxjstz(a)126.com>
Gerrit-Comment-Date: Fri, 11 Jun 2021 00:04:11 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Peter Marheine has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/55403 )
Change subject: lspcon: restart MPU on programmer shutdown
......................................................................
lspcon: restart MPU on programmer shutdown
Programmer initialization stops the on-chip MPU, and it was never
restarted. Leaving it stopped seems to prevent some display detection
from working, so implement restarting the MPU on programmer shutdown.
BUG=b:190359231
TEST=display hotplug works reliably after device communication
Change-Id: I66cd68f8f6905a2bfaf5b085bf08dcb218f42855
---
M lspcon_i2c_spi.c
1 file changed, 12 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/03/55403/1
diff --git a/lspcon_i2c_spi.c b/lspcon_i2c_spi.c
index 9f3bb9a..f8df9e4 100644
--- a/lspcon_i2c_spi.c
+++ b/lspcon_i2c_spi.c
@@ -327,6 +327,16 @@
return ret;
}
+static int lspcon_i2c_spi_start_mpu(int fd)
+{
+ int ret;
+ // Assert reset
+ if ((ret = lspcon_i2c_spi_write_register(fd, MPU, 0xc0)) != 0)
+ return ret;
+ // Release reset and start MPU
+ return lspcon_i2c_spi_write_register(fd, MPU, 0);
+}
+
static int lspcon_i2c_spi_map_page(int fd, unsigned int offset)
{
int ret = 0;
@@ -418,15 +428,16 @@
.write_aai = lspcon_i2c_spi_write_aai,
};
-/* TODO: MPU still stopped at this point, probably need to reset it. */
static int lspcon_i2c_spi_shutdown(void *data)
{
int ret = 0;
struct lspcon_i2c_spi_data *lspcon_data =
(struct lspcon_i2c_spi_data *)data;
int fd = lspcon_data->fd;
+
ret |= lspcon_i2c_spi_enable_write_protection(fd);
ret |= lspcon_i2c_spi_toggle_register_protection(fd, 0);
+ ret |= lspcon_i2c_spi_start_mpu(fd);
i2c_close(fd);
free(data);
--
To view, visit https://review.coreboot.org/c/flashrom/+/55403
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I66cd68f8f6905a2bfaf5b085bf08dcb218f42855
Gerrit-Change-Number: 55403
Gerrit-PatchSet: 1
Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-MessageType: newchange
Attention is currently required from: Nico Huber.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55358 )
Change subject: dummyflasher: Replace another case of `sizeof(struct ...)`
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> I have +2 ??! I almost dropped my tea cup when I saw this :)
I meant to say, thank you, it is an honour for me!
--
To view, visit https://review.coreboot.org/c/flashrom/+/55358
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: If88ba394095f86c598dcc5cf1751e1c23b132d04
Gerrit-Change-Number: 55358
Gerrit-PatchSet: 1
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Comment-Date: Thu, 10 Jun 2021 22:18:08 +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: Xiang W, Edward O'Callaghan, Angel Pons.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49255 )
Change subject: bitbang-spi.c: support clock polarity and phase
......................................................................
Patch Set 26:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/49255/comment/fff68f52_b85ef938
PS1, Line 8:
> 1. implements a better mode 0
Actually, it's not there yet, but there is potential. I see two
things in the original implementation:
* the spurious mosi settings
* the very first clock setting in the first loop-iteration for each byte could be skipped.
--
To view, visit https://review.coreboot.org/c/flashrom/+/49255
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I04c1dfe132d756119229b27c3cd611d1be1abc8d
Gerrit-Change-Number: 49255
Gerrit-PatchSet: 26
Gerrit-Owner: Xiang W <wxjstz(a)126.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: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
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: Shawn C <citypw(a)hardenedlinux.org>
Gerrit-Attention: Xiang W <wxjstz(a)126.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Thu, 10 Jun 2021 20:58:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Xiang W <wxjstz(a)126.com>
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment