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
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
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
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
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
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>
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
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
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
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