June 2018 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: Fix mingw detection on Windows 7 (NT-6.1)
by build bot (Jenkins) (Code Review) June 23, 2018

June 23, 2018

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/23865 ) Change subject: Fix mingw detection on Windows 7 (NT-6.1) ...................................................................... Patch Set 3: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/1342/ : SUCCESS https://qa.coreboot.org/job/flashrom_gerrit/1110/ : SUCCESS -- To view, visit https://review.coreboot.org/23865 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7f856dc4847c4ca9197b1935b7a9b9071b46c70a Gerrit-Change-Number: 23865 Gerrit-PatchSet: 3 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Miklós Márton <martonmiklosqdev(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-CC: Márton Miklós <martonmiklosqdev(a)gmail.com> Gerrit-Comment-Date: Sat, 23 Jun 2018 19:41:35 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in flashrom[master]: linux_mtd: Bail out early if sysfs node doesn't exist
by Nico Huber (Code Review) June 23, 2018

June 23, 2018

Nico Huber has posted comments on this change. ( https://review.coreboot.org/26500 ) Change subject: linux_mtd: Bail out early if sysfs node doesn't exist ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/26500 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8dc965eecc68cd305a989016869c688fe1a3921f Gerrit-Change-Number: 26500 Gerrit-PatchSet: 2 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sat, 23 Jun 2018 14:45:25 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in flashrom[master]: Fix mingw detection on Windows 7 (NT-6.1)
by Nico Huber (Code Review) June 23, 2018

June 23, 2018

Nico Huber has posted comments on this change. ( https://review.coreboot.org/23865 ) Change subject: Fix mingw detection on Windows 7 (NT-6.1) ...................................................................... Patch Set 2: Just realized that you didn't push the original patch to Gerrit yourself, https://coreboot.org/Git should give an overview. -- To view, visit https://review.coreboot.org/23865 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7f856dc4847c4ca9197b1935b7a9b9071b46c70a Gerrit-Change-Number: 23865 Gerrit-PatchSet: 2 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Miklós Márton <martonmiklosqdev(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-CC: Márton Miklós <martonmiklosqdev(a)gmail.com> Gerrit-Comment-Date: Sat, 23 Jun 2018 14:40:26 +0000 Gerrit-HasComments: No Gerrit-HasLabels: No

1 0

Change in flashrom[master]: Fix mingw detection on Windows 7 (NT-6.1)
by Nico Huber (Code Review) June 23, 2018

June 23, 2018

Nico Huber has posted comments on this change. ( https://review.coreboot.org/23865 ) Change subject: Fix mingw detection on Windows 7 (NT-6.1) ...................................................................... Patch Set 2: Code-Review+2 I've replaced it with the linked patch and took the liberty to fix the commit message, too (imperative present tense, max ~60 chars for the summary). Miklós, you can amend patches locally and push them again. As long as you keep the `Change-Id: ...` of the original push in the commit, Gerrit will add it as another `Patch Set` of the same change. -- To view, visit https://review.coreboot.org/23865 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7f856dc4847c4ca9197b1935b7a9b9071b46c70a Gerrit-Change-Number: 23865 Gerrit-PatchSet: 2 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Miklós Márton <martonmiklosqdev(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-CC: Márton Miklós <martonmiklosqdev(a)gmail.com> Gerrit-Comment-Date: Sat, 23 Jun 2018 14:37:46 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in flashrom[master]: Fix mingw detection on Windows 7 (NT-6.1)
by build bot (Jenkins) (Code Review) June 23, 2018

June 23, 2018

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/23865 ) Change subject: Fix mingw detection on Windows 7 (NT-6.1) ...................................................................... Patch Set 2: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/1341/ : SUCCESS https://qa.coreboot.org/job/flashrom_gerrit/1109/ : SUCCESS -- To view, visit https://review.coreboot.org/23865 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7f856dc4847c4ca9197b1935b7a9b9071b46c70a Gerrit-Change-Number: 23865 Gerrit-PatchSet: 2 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Miklós Márton <martonmiklosqdev(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-CC: Márton Miklós <martonmiklosqdev(a)gmail.com> Gerrit-Comment-Date: Sat, 23 Jun 2018 14:33:44 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in flashrom[master]: Fix mingw detection on Windows 7 (NT-6.1)
by Nico Huber (Code Review) June 23, 2018

June 23, 2018

Nico Huber has uploaded a new patch set (#2) to the change originally created by David Hendricks. ( https://review.coreboot.org/23865 ) Change subject: Fix mingw detection on Windows 7 (NT-6.1) ...................................................................... Fix mingw detection on Windows 7 (NT-6.1) Hopefully also for other non-XP Windows build environments. Change-Id: I7f856dc4847c4ca9197b1935b7a9b9071b46c70a Signed-off-by: Miklós Márton <martonmiklosqdev(a)gmail.com> --- M Makefile M util/ich_descriptors_tool/Makefile 2 files changed, 12 insertions(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/65/23865/2 -- To view, visit https://review.coreboot.org/23865 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7f856dc4847c4ca9197b1935b7a9b9071b46c70a Gerrit-Change-Number: 23865 Gerrit-PatchSet: 2 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Miklós Márton <martonmiklosqdev(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-CC: Márton Miklós <martonmiklosqdev(a)gmail.com>

1 0

Change in flashrom[master]: Add support for National Instruments USB-845x devices
by Nico Huber (Code Review) June 23, 2018

June 23, 2018

Nico Huber has posted comments on this change. ( https://review.coreboot.org/25683 ) Change subject: Add support for National Instruments USB-845x devices ...................................................................... Patch Set 2: (3 comments) https://review.coreboot.org/#/c/25683/2/Makefile File Makefile: https://review.coreboot.org/#/c/25683/2/Makefile@948 PS2, Line 948: LIBS += -lni845x > I have not find an explicit license for the NI 845x, but a txt file in the installation folder links […] I didn't mean to warn anybody of NI's wrath but ours. flashrom authors have rights too. If somebody sells a binary of flashrom along with or as his product, they have to adhere to the GPL. In this case this would mean to provide access to NI's source code under a GPL compatible license, which could cause them some trouble ;) However, building such binary for yourself and using it is absolutely fine. So it's not a big deal, I guess, as long as NI845X_SPI is disabled by default and nobody links to it by accident. https://review.coreboot.org/#/c/25683/2/ni845x_spi.c File ni845x_spi.c: PS2: > Camel cases removed, do you have an example of what parameters should I use with indent? Hmmm, looks like this was never formalized for flashrom. `indent -linux -l112` might do, not sure. Though, looking closer at the code, the indentation itself seems to be the only (obvious) problem, it should just be a tab. https://review.coreboot.org/#/c/25683/2/ni845x_spi.c@47 PS2, Line 47: uInt32 > The NI USB-854x API brings these types and I thought that using them for the library calls only woul […] Hard to argue about that. But uInt32 seems to translate rather safely to uint32_t. Your call. -- To view, visit https://review.coreboot.org/25683 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9477b6f0193bfdf20bbe63421a7fb97b597ec549 Gerrit-Change-Number: 25683 Gerrit-PatchSet: 2 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Miklós Márton <martonmiklosqdev(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-CC: Márton Miklós <martonmiklosqdev(a)gmail.com> Gerrit-Comment-Date: Sat, 23 Jun 2018 14:09:17 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in flashrom[master]: Add support for National Instruments USB-845x devices
by Márton Miklós (Code Review) June 23, 2018

June 23, 2018

Márton Miklós has posted comments on this change. ( https://review.coreboot.org/25683 ) Change subject: Add support for National Instruments USB-845x devices ...................................................................... Patch Set 2: (3 comments) https://review.coreboot.org/#/c/25683/2/Makefile File Makefile: https://review.coreboot.org/#/c/25683/2/Makefile@948 PS2, Line 948: LIBS += -lni845x > How is it licensed? If it's not GPLv2 compatible, should we print a […] I have not find an explicit license for the NI 845x, but a txt file in the installation folder links to the NI's general software license: "This product is licensed to you by National Instruments pursuant to the applicable National Instruments software license agreement." This can be found here: https://www.ni.com/pdf/legal/us/software_license_agreement.pdf I have not found any restrictions about the linking external software to their libraries. https://review.coreboot.org/#/c/25683/2/ni845x_spi.c File ni845x_spi.c: PS2: > Maybe run this file through `indent` first. And no camel-case, please. Camel cases removed, do you have an example of what parameters should I use with indent? https://review.coreboot.org/#/c/25683/2/ni845x_spi.c@47 PS2, Line 47: uInt32 > please use stdint. […] The NI USB-854x API brings these types and I thought that using them for the library calls only would be a good idea because this way we do not need to trust that NI won't change their types definition in the future (which is unlikely). -- To view, visit https://review.coreboot.org/25683 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9477b6f0193bfdf20bbe63421a7fb97b597ec549 Gerrit-Change-Number: 25683 Gerrit-PatchSet: 2 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Miklós Márton <martonmiklosqdev(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-CC: Márton Miklós <martonmiklosqdev(a)gmail.com> Gerrit-Comment-Date: Sat, 23 Jun 2018 13:39:28 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in flashrom[master]: digilent_spi: add a driver for the iCEblink40 development board
by Nico Huber (Code Review) June 23, 2018

June 23, 2018

Nico Huber has posted comments on this change. ( https://review.coreboot.org/23338 ) Change subject: digilent_spi: add a driver for the iCEblink40 development board ...................................................................... Patch Set 6: Code-Review+2 (3 comments) Sorry for the huge delay. Looks good. I'll give it a day or two in case you still want to change something, then merge it. https://review.coreboot.org/#/c/23338/4/digilent_spi.c File digilent_spi.c: https://review.coreboot.org/#/c/23338/4/digilent_spi.c@221 PS4, Line 221: > Added some more checking of the response. Thx https://review.coreboot.org/#/c/23338/4/digilent_spi.c@276 PS4, Line 276: ret = spi_start_io(read_follows, writecnt); : if (ret != 0) > Raised to 252, empirically determined as the actual limit of the payload the hardware can handle. […] The overhead is already reduced by setting it to 252. Now, I agree, changing the loop likely wouldn't bring a significant speed improve- ment (unless you use asychronous bulk transfers and that gets really complex, see dediprog for example). https://review.coreboot.org/#/c/23338/6/digilent_spi.c File digilent_spi.c: https://review.coreboot.org/#/c/23338/6/digilent_spi.c@318 PS6, Line 318: .max_data_read = 252, : .max_data_write = 252, Nit: 252 seems to be 256 - 4 (1B opcode, 3B address). I doubt that anyone would use this interface to program chips with 4B addresses, but if you want to prepare for that, I would reduce it to 251. -- To view, visit https://review.coreboot.org/23338 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7ffcd9a2db4395816f0e8b6ce6c3b0d8e930c9e6 Gerrit-Change-Number: 23338 Gerrit-PatchSet: 6 Gerrit-Owner: Lubomir Rintel <lkundrak(a)v3.sk> Gerrit-Reviewer: Lubomir Rintel <lkundrak(a)v3.sk> 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: Sat, 23 Jun 2018 13:11:11 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: Yes

1 0

Change in flashrom[master]: Add support for National Instruments USB-845x devices
by Nico Huber (Code Review) June 23, 2018

June 23, 2018

Nico Huber has posted comments on this change. ( https://review.coreboot.org/25683 ) Change subject: Add support for National Instruments USB-845x devices ...................................................................... Patch Set 2: (6 comments) https://review.coreboot.org/#/c/25683/2/Makefile File Makefile: https://review.coreboot.org/#/c/25683/2/Makefile@943 PS2, Line 943: NI845X_LIBS += -L'C:\Program Files (x86)\National Instruments\NI-845x\MS Visual C' : NI845X_LIBS += -L'C:\Program Files\National Instruments\NI-845x\MS Visual C' : NI845X_INCLUDES += -I'C:\Program Files (x86)\National Instruments\NI-845x\MS Visual C' : NI845X_INCLUDES += -I'C:\Program Files\National Instruments\NI-845x\MS Visual C' If there is no reasonable way to probe for this path, I would prefer to query the user for it. Asking the user to set a variable in case the test below fails would be one way. I would do that anyway, even if we have a default here because a windows version and localization specific path is just too fragile. https://review.coreboot.org/#/c/25683/2/Makefile@948 PS2, Line 948: LIBS += -lni845x How is it licensed? If it's not GPLv2 compatible, should we print a warning about shipping a binary linked against it? https://review.coreboot.org/#/c/25683/2/Makefile@1349 PS2, Line 1349: spurious space https://review.coreboot.org/#/c/25683/2/ni845x_spi.c File ni845x_spi.c: PS2: Maybe run this file through `indent` first. And no camel-case, please. https://review.coreboot.org/#/c/25683/2/ni845x_spi.c@8 PS2, Line 8: * the Free Software Foundation; version 2 of the License. Please do not restrict the code to version 2 unless you really want to. flashrom should be v2+, IMHO, the decision to make parts of it v2 only (to prevent a v3 fork) was rather stupid. https://review.coreboot.org/#/c/25683/2/ni845x_spi.c@47 PS2, Line 47: uInt32 please use stdint.h types where applicable -- To view, visit https://review.coreboot.org/25683 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9477b6f0193bfdf20bbe63421a7fb97b597ec549 Gerrit-Change-Number: 25683 Gerrit-PatchSet: 2 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Miklós Márton <martonmiklosqdev(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: Sat, 23 Jun 2018 12:24:22 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0