November 2017 - flashrom-gerrit

Change in flashrom[staging]: spi25_statusreg: Return defined value on failed RDSR
by build bot (Jenkins) (Code Review) 05 Nov '17

05 Nov '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/22017 ) Change subject: spi25_statusreg: Return defined value on failed RDSR ...................................................................... Patch Set 2: Build Successful https://qa.coreboot.org/job/flashrom-customrules/828/ : SUCCESS -- To view, visit https://review.coreboot.org/22017 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: I714b20001a5443bba665c2e0061ca14069777581 Gerrit-Change-Number: 22017 Gerrit-PatchSet: 2 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sun, 05 Nov 2017 14:37:02 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: spi25_statusreg: Return defined value on failed RDSR
by Nico Huber (Code Review) 05 Nov '17

05 Nov '17

Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/22017 ) Change subject: spi25_statusreg: Return defined value on failed RDSR ...................................................................... spi25_statusreg: Return defined value on failed RDSR The interface of spi_read_status_register() is broken and can't return errors. Let's not return random stack data at least. Change-Id: I714b20001a5443bba665c2e0061ca14069777581 Signed-off-by: Nico Huber <nico.h(a)gmx.de> Reviewed-on: https://review.coreboot.org/22017 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: David Hendricks <david.hendricks(a)gmail.com> --- M spi25_statusreg.c 1 file changed, 4 insertions(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified David Hendricks: Looks good to me, approved diff --git a/spi25_statusreg.c b/spi25_statusreg.c index 01a6862..05c7acf 100644 --- a/spi25_statusreg.c +++ b/spi25_statusreg.c @@ -117,8 +117,11 @@ /* Read Status Register */ ret = spi_send_command(flash, sizeof(cmd), sizeof(readarr), cmd, readarr); - if (ret) + if (ret) { msg_cerr("RDSR failed!\n"); + /* FIXME: We should propagate the error. */ + return 0; + } return readarr[0]; } -- To view, visit https://review.coreboot.org/22017 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: merged Gerrit-Change-Id: I714b20001a5443bba665c2e0061ca14069777581 Gerrit-Change-Number: 22017 Gerrit-PatchSet: 2 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[staging]: linux_spi: Dynamically detect max buffer size
by Nico Huber (Code Review) 05 Nov '17

05 Nov '17

Nico Huber has posted comments on this change. ( https://review.coreboot.org/22344 ) Change subject: linux_spi: Dynamically detect max buffer size ...................................................................... Patch Set 3: (1 comment) https://review.coreboot.org/#/c/22344/3/linux_spi.c File linux_spi.c: https://review.coreboot.org/#/c/22344/3/linux_spi.c@199 PS3, Line 199: The size of the command and the buffer may add up to no more than max_kernel_buf_size. Why? rx/tx buffers seem to be handled separately. -- To view, visit https://review.coreboot.org/22344 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: Ic541e548ced8488f074d388f1c92174cad123064 Gerrit-Change-Number: 22344 Gerrit-PatchSet: 3 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: david.hendricks(a)gmail.com Gerrit-Comment-Date: Sun, 05 Nov 2017 11:34:52 +0000 Gerrit-HasComments: Yes

1 0

Change in flashrom[staging]: linux_spi: Dynamically detect max buffer size
by Nico Huber (Code Review) 05 Nov '17

05 Nov '17

Nico Huber has posted comments on this change. ( https://review.coreboot.org/22344 ) Change subject: linux_spi: Dynamically detect max buffer size ...................................................................... Patch Set 3: (4 comments) https://review.coreboot.org/#/c/22344/3//COMMIT_MSG Commit Message: https://review.coreboot.org/#/c/22344/3//COMMIT_MSG@9 PS3, Line 9: The max buffer size of the linux kernel's SPI device is a compile-time : parameter. That's not true, and I can say that from this patch: If we read it from a module parameter representation in sysfs it's obviously not compile-time. https://review.coreboot.org/#/c/22344/1/linux_spi.c File linux_spi.c: https://review.coreboot.org/#/c/22344/1/linux_spi.c@141 PS1, Line 141: if (param_fd != -1) : close(param_fd); > May as well put this in the else clause above. No, the else path is not hit if open() succeeds but read() fails. https://review.coreboot.org/#/c/22344/3/linux_spi.c File linux_spi.c: https://review.coreboot.org/#/c/22344/3/linux_spi.c@134 PS3, Line 134: int param_fd = open("/sys/module/spidev/parameters/bufsiz", O_RDONLY); Isn't there a way to query this over the device interface? https://review.coreboot.org/#/c/22344/3/linux_spi.c@135 PS3, Line 135: if (param_fd == -1 || read(param_fd, &buf, sizeof(buf) - 1) == -1) { What if read() returns a short byte count? What if it gets inter- rupted (I have to admit, unlikely on sysfs). Generally, using the low level file functions is a bad idea if you can use stream func- tions (e.g. fread()) instead. -- To view, visit https://review.coreboot.org/22344 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: Ic541e548ced8488f074d388f1c92174cad123064 Gerrit-Change-Number: 22344 Gerrit-PatchSet: 3 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: david.hendricks(a)gmail.com Gerrit-Comment-Date: Sun, 05 Nov 2017 11:18:33 +0000 Gerrit-HasComments: Yes

1 0

Change in flashrom[1.0.x]: ichspi: Disable software sequencing by default for Skylake
by build bot (Jenkins) (Code Review) 05 Nov '17

05 Nov '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/22348 ) Change subject: ichspi: Disable software sequencing by default for Skylake ...................................................................... Patch Set 1: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom_gerrit/724/ : SUCCESS https://qa.coreboot.org/job/flashrom-customrules/827/ : SUCCESS -- To view, visit https://review.coreboot.org/22348 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.0.x Gerrit-MessageType: comment Gerrit-Change-Id: I8a13fb9c3ca679b3f7d39ad1dc56d5efdc80045b Gerrit-Change-Number: 22348 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sun, 05 Nov 2017 10:58:05 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[1.0.x]: ichspi: Disable software sequencing by default for Skylake
by Nico Huber (Code Review) 05 Nov '17

05 Nov '17

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/22348 Change subject: ichspi: Disable software sequencing by default for Skylake ...................................................................... ichspi: Disable software sequencing by default for Skylake Skylake is a mess, especially with coreboot. We have now a present and configured software sequencing interface with SCGO supposedly being readonly (Apollo Lake has that feature and a strap documented, Skylake behaviour might be the same). As we can't easily check if it's read- only, just enable hardware sequencing by default (even if the software sequencing interface seems usable). Change-Id: I8a13fb9c3ca679b3f7d39ad1dc56d5efdc80045b Signed-off-by: Nico Huber <nico.huber(a)secunet.com> --- M ichspi.c 1 file changed, 5 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/48/22348/1 diff --git a/ichspi.c b/ichspi.c index ec81628..859d55f 100644 --- a/ichspi.c +++ b/ichspi.c @@ -1951,6 +1951,11 @@ ich_spi_mode = ich_hwseq; } + if (ich_spi_mode == ich_auto && ich_gen == CHIPSET_100_SERIES_SUNRISE_POINT) { + msg_pdbg("Enabling hardware sequencing by default for 100 series PCH.\n"); + ich_spi_mode = ich_hwseq; + } + if (ich_spi_mode == ich_hwseq) { if (!desc_valid) { msg_perr("Hardware sequencing was requested " -- To view, visit https://review.coreboot.org/22348 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.0.x Gerrit-MessageType: newchange Gerrit-Change-Id: I8a13fb9c3ca679b3f7d39ad1dc56d5efdc80045b Gerrit-Change-Number: 22348 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>

1 0

Change in flashrom[staging]: ichspi: Disable software sequencing by default for Skylake
by Nico Huber (Code Review) 05 Nov '17

05 Nov '17

Nico Huber has posted comments on this change. ( https://review.coreboot.org/22274 ) Change subject: ichspi: Disable software sequencing by default for Skylake ...................................................................... Patch Set 1: (1 comment) https://review.coreboot.org/#/c/22274/1/ichspi.c File ichspi.c: https://review.coreboot.org/#/c/22274/1/ichspi.c@1957 PS1, Line 1957: } > Would it make sense to move this to line 1800, closer to where ich_spi_mode I've intentionally placed it here. This way we still see from the output if there was any other reason to use hwseq. -- To view, visit https://review.coreboot.org/22274 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: I8a13fb9c3ca679b3f7d39ad1dc56d5efdc80045b Gerrit-Change-Number: 22274 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sun, 05 Nov 2017 10:57:18 +0000 Gerrit-HasComments: Yes

1 0

Change in flashrom[staging]: linux_spi: Dynamically detect max buffer size
by Anonymous Coward (Code Review) 05 Nov '17

05 Nov '17

david.hendricks(a)gmail.com has posted comments on this change. ( https://review.coreboot.org/22344 ) Change subject: linux_spi: Dynamically detect max buffer size ...................................................................... Patch Set 1: (4 comments) https://review.coreboot.org/#/c/22344/1//COMMIT_MSG Commit Message: https://review.coreboot.org/#/c/22344/1//COMMIT_MSG@9 PS1, Line 9: The max buffer size of the linux kernel's SPI device is a compile-time parameter. : Read it on initialization and use it to inform the max transfer size. We allow : up to 5 bytes for the SPI command. On master, SPI commands are always 4 bytes, : but 4ba support will raise that to 5. Choosing 5 makes sure this will keep long lines https://review.coreboot.org/#/c/22344/1//COMMIT_MSG@12 PS1, Line 12: but 4ba support will raise that to 5. Choosing 5 makes sure this will keep : working when 4ba support is merged. 4BA has been merged, so I updated the wording here. https://review.coreboot.org/#/c/22344/1/linux_spi.c File linux_spi.c: https://review.coreboot.org/#/c/22344/1/linux_spi.c@138 PS1, Line 138: atoi(buf); Updated to use strtol() and add some paranoid error checking. https://review.coreboot.org/#/c/22344/1/linux_spi.c@141 PS1, Line 141: if (param_fd != -1) : close(param_fd); May as well put this in the else clause above. -- To view, visit https://review.coreboot.org/22344 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: Ic541e548ced8488f074d388f1c92174cad123064 Gerrit-Change-Number: 22344 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: david.hendricks(a)gmail.com Gerrit-Comment-Date: Sun, 05 Nov 2017 04:12:19 +0000 Gerrit-HasComments: Yes

1 0

Change in flashrom[staging]: linux_spi: Dynamically detect max buffer size
by build bot (Jenkins) (Code Review) 05 Nov '17

05 Nov '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/22344 ) Change subject: linux_spi: Dynamically detect max buffer size ...................................................................... Patch Set 3: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/826/ : SUCCESS -- To view, visit https://review.coreboot.org/22344 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: Ic541e548ced8488f074d388f1c92174cad123064 Gerrit-Change-Number: 22344 Gerrit-PatchSet: 3 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sun, 05 Nov 2017 04:10:12 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: linux_spi: Dynamically detect max buffer size
by Anonymous Coward (Code Review) 05 Nov '17

05 Nov '17

david.hendricks(a)gmail.com has uploaded a new patch set (#3) to the change originally created by David Hendricks. ( https://review.coreboot.org/22344 ) Change subject: linux_spi: Dynamically detect max buffer size ...................................................................... linux_spi: Dynamically detect max buffer size The max buffer size of the linux kernel's SPI device is a compile-time parameter. Read it on initialization and use it to inform the max transfer size. Change-Id: Ic541e548ced8488f074d388f1c92174cad123064 Signed-off-by: Keno Fischer <keno(a)juliacomputing.com> Signed-off-by: David Hendricks <dhendricks(a)fb.com> --- M linux_spi.c 1 file changed, 18 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/44/22344/3 -- To view, visit https://review.coreboot.org/22344 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic541e548ced8488f074d388f1c92174cad123064 Gerrit-Change-Number: 22344 Gerrit-PatchSet: 3 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit November 2017