November 2017 - flashrom-gerrit

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 5: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom_gerrit/728/ : SUCCESS https://qa.coreboot.org/job/flashrom-customrules/832/ : 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: 5 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> 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-Reviewer: david.hendricks(a)gmail.com Gerrit-Comment-Date: Sun, 05 Nov 2017 21:39:21 +0000 Gerrit-HasComments: No

1 0

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

05 Nov '17

Hello david.hendricks(a)gmail.com, Paul Menzel, build bot (Jenkins), Nico Huber, I'd like you to reexamine a change. Please visit https://review.coreboot.org/22344 to look at the new patch set (#5). Change subject: linux_spi: Dynamically detect max buffer size ...................................................................... linux_spi: Dynamically detect max buffer size Read max buffer size from sysfs if available. 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, 45 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/44/22344/5 -- 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: 5 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> 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-Reviewer: david.hendricks(a)gmail.com

1 0

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

05 Nov '17

David Hendricks has posted comments on this change. ( https://review.coreboot.org/22344 ) Change subject: linux_spi: Dynamically detect max buffer size ...................................................................... Patch Set 4: (5 comments) Thanks for the quick (and thorough) review as always. I ended up re-writing a significant portion of this patch. Assuming the changes look good, what's the "right" thing to do to get things merged? Merge everything in this patch, or split my updates into a follow-up patch? https://review.coreboot.org/#/c/22344/3//COMMIT_MSG Commit Message: https://review.coreboot.org/#/c/22344/3//COMMIT_MSG@9 PS3, Line 9: Read max buffer size from sysfs if available. : > That's not true, and I can say that from this patch: If we I guess the original author meant that it's a compile-time constant in the kernel? In any case, I went ahead and simplified this further to remove any possible confusion. 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: fp = fopen(BUF_SIZE_FROM_SYSFS, "r"); > Isn't there a way to query this over the device interface? Not sure... I don't see any other obvious way of getting at it. There's no ioctl for this AFAICT by looking thru the spidev documentation. The only clue seems to be: - There's a limit on the number of bytes each I/O request can transfer to the SPI device. It defaults to one page, but that can be changed using a module parameter. So either read the module parameter via sysfs or default to using getpagesize() which the code currently does. https://review.coreboot.org/#/c/22344/3/linux_spi.c@135 PS3, Line 135: if (!fp) { > What if read() returns a short byte count? What if it gets inter- Done. https://review.coreboot.org/#/c/22344/3/linux_spi.c@199 PS3, Line 199: The implementation currently does not support requests that > Why? rx/tx buffers seem to be handled separately. Re "why": As to not rely on the driver to split large transactions? I'm not if this is a big deal for spidev, but I have had issues with MTD in the past (https://chromium-review.googlesource.com/505409). Re rx/tx being handled separately: Are you referring to the current (old) code? That may be an error, I think Keno was correct to handle read and write buffers the same and account for command size. https://review.coreboot.org/#/c/22344/4/linux_spi.c File linux_spi.c: https://review.coreboot.org/#/c/22344/4/linux_spi.c@222 PS4, Line 222: * oops, i'll fix this... -- 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: 4 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> 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-Reviewer: david.hendricks(a)gmail.com Gerrit-Comment-Date: Sun, 05 Nov 2017 21:38:37 +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 4: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom_gerrit/727/ : SUCCESS https://qa.coreboot.org/job/flashrom-customrules/831/ : 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: 4 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 21:31:30 +0000 Gerrit-HasComments: No

1 0

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

05 Nov '17

Hello david.hendricks(a)gmail.com, Paul Menzel, build bot (Jenkins), Nico Huber, I'd like you to reexamine a change. Please visit https://review.coreboot.org/22344 to look at the new patch set (#4). Change subject: linux_spi: Dynamically detect max buffer size ...................................................................... linux_spi: Dynamically detect max buffer size Read max buffer size from sysfs if available. 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, 45 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/44/22344/4 -- 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: 4 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

1 0

Change in flashrom[staging]: this is a test, please ignore
by David Hendricks (Code Review) 05 Nov '17

05 Nov '17

David Hendricks has abandoned this change. ( https://review.coreboot.org/22353 ) Change subject: this is a test, please ignore ...................................................................... Abandoned -- To view, visit https://review.coreboot.org/22353 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: abandon Gerrit-Change-Id: I9aa1dab92b6c69b27a2cb90e9f6154cb73214c26 Gerrit-Change-Number: 22353 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[staging]: this is a test, please ignore
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/22353 ) Change subject: this is a test, please ignore ...................................................................... Patch Set 1: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom_gerrit/726/ : SUCCESS https://qa.coreboot.org/job/flashrom-customrules/830/ : SUCCESS -- To view, visit https://review.coreboot.org/22353 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: I9aa1dab92b6c69b27a2cb90e9f6154cb73214c26 Gerrit-Change-Number: 22353 Gerrit-PatchSet: 1 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 21:30:21 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: this is a test, please ignore
by David Hendricks (Code Review) 05 Nov '17

05 Nov '17

David Hendricks has uploaded this change for review. ( https://review.coreboot.org/22353 Change subject: this is a test, please ignore ...................................................................... this is a test, please ignore Change-Id: I9aa1dab92b6c69b27a2cb90e9f6154cb73214c26 Signed-off-by: David Hendricks <dhendricks(a)fb.com> --- A test.txt 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/53/22353/1 diff --git a/test.txt b/test.txt new file mode 100644 index 0000000..3b18e51 --- /dev/null +++ b/test.txt @@ -0,0 +1 @@ +hello world -- To view, visit https://review.coreboot.org/22353 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: newchange Gerrit-Change-Id: I9aa1dab92b6c69b27a2cb90e9f6154cb73214c26 Gerrit-Change-Number: 22353 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com>

1 0

Change in flashrom[1.0.x]: 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/22349 ) Change subject: spi25_statusreg: Return defined value on failed RDSR ...................................................................... Patch Set 1: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom_gerrit/725/ : SUCCESS https://qa.coreboot.org/job/flashrom-customrules/829/ : SUCCESS -- To view, visit https://review.coreboot.org/22349 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.0.x Gerrit-MessageType: comment Gerrit-Change-Id: I714b20001a5443bba665c2e0061ca14069777581 Gerrit-Change-Number: 22349 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 14:37:25 +0000 Gerrit-HasComments: No

1 0

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

05 Nov '17

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/22349 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> (cherry picked from commit 1f081530b60ee805532f106f59cc33973e160481) --- M spi25_statusreg.c 1 file changed, 4 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/49/22349/1 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/22349 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.0.x Gerrit-MessageType: newchange Gerrit-Change-Id: I714b20001a5443bba665c2e0061ca14069777581 Gerrit-Change-Number: 22349 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit November 2017