Jonathan A. Kollasch has posted comments on this change. ( https://review.coreboot.org/23693 )
Change subject: improve termios settings on some OSes
......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/23693/1/serial.c
File serial.c:
https://review.coreboot.org/#/c/23693/1/serial.c@216
PS1, Line 216: wanted.c_cflag &= ~(PARENB | CSTOPB | CSIZE | CRTSCTS);
: wanted.c_cflag |= (CS8 | CLOCAL | CREAD);
: wanted.c_lflag &= ~(ICANON | ECHO | ECHOE | ECHOK | ECHONL | ISIG | IEXTEN);
: wanted.c_lflag &= ~(ECHOCTL | ECHOKE);
: wanted.c_iflag &= ~(IXON | IXOFF | IXANY | ICRNL | IGNCR | INLCR);
: wanted.c_iflag &= ~(PARMRK);
: wanted.c_oflag &= ~OPOST;
It might be wise to start with cfmakeraw(3). Although it does not appear to be standardized by Open Group, I see cfmakeraw(3) available on my GNU/Linux installations.
https://nxr.netbsd.org/xref/src/lib/libc/termios/cfmakeraw.c#51
But, if you've covered all the bits that NetBSD's cfmakeraw(3) does, I have no objection.
--
To view, visit https://review.coreboot.org/23693
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: I5da80f541e02c3b8e676e47a45388bfb115b4762
Gerrit-Change-Number: 23693
Gerrit-PatchSet: 1
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Jonathan A. Kollasch <jakllsch(a)kollasch.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Sun, 11 Feb 2018 14:13:06 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
Nico Huber has posted comments on this change. ( https://review.coreboot.org/22971 )
Change subject: git-hooks: Fix install script for various git versions
......................................................................
Patch Set 2:
Beside the problems with the install script, I run into more trouble
lately while I moved patches from patchwork to gerrit. The whole
approach with the in-tree wrapper.sh seems broken: You must never
check a commit out before wrapper.sh was introduced. Otherwise not
even your <hook>.local files are run. So, the lesson I've learned,
the hooks must not rely on the state of the tree.
One way to mitigate that would be to copy wrapper.sh along with the
flashrom hooks into .git/hooks/. We could still try to keep them
up to date there (based on timestamps? not sure how much shell foo
is necessary).
One problem will haunt us forever, though. Every time you run make
for a commit between the introduction of wrapper.sh and a possible
fix, it will overwrite existing hooks and <hook>.local files (un-
conditionally). So we have to find a new name for <hook>.local and
should probably keep wrapper.sh around forever to help until ano-
ther run of (the updated) install script fixed things again.
An alternative: Drop all this shit. Use something less error-prone
like `make gitconfig` in coreboot. Downside: After a `make` with a
wrapper.sh revision broke things, you'd have to repair it manually,
hopefully just run `make gitconfig` again. Upside: We don't look
like fools.
--
To view, visit https://review.coreboot.org/22971
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: I7ee8d4d54db48f7207fe8abf895c7fbba7685ad2
Gerrit-Change-Number: 22971
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: Stefan Tauner <stefan.tauner(a)gmx.at>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Sun, 11 Feb 2018 13:38:09 +0000
Gerrit-HasComments: No
Gerrit-HasLabels: No
Nico Huber has posted comments on this change. ( https://review.coreboot.org/23203 )
Change subject: [WIP]Add support to get layout from a binary fmap file
......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/#/c/23203/5/fmap.h
File fmap.h:
https://review.coreboot.org/#/c/23203/5/fmap.h@27
PS5, Line 27: struct fmap_area areas[];
You could just give it a number, e.g. MAX_ROMLAYOUT. Dynamic
allocation would be cleaner, IMHO (or just inline the decoding
into the processing or the other way around).
https://review.coreboot.org/#/c/23203/5/fmap.c
File fmap.c:
https://review.coreboot.org/#/c/23203/5/fmap.c@63
PS5, Line 63: fmap->ver_major = dump[i++];
overflow check?
Though, I think it would be much easier to check only two times.
One time for `sizeof(struct fmap)` and one time for `nareas`.
--
To view, visit https://review.coreboot.org/23203
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: I0e7fad38ed79a84d41358e1f175c36d255786c12
Gerrit-Change-Number: 23203
Gerrit-PatchSet: 5
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
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, 10 Feb 2018 14:33:27 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
Nico Huber has posted comments on this change. ( https://review.coreboot.org/23678 )
Change subject: [RFC]Don't make layoutfile and --ifd mutually exclusive
......................................................................
Patch Set 5: Code-Review+1
I'm generally ok with this. But the whole layout stuff should be less
error-prone (e.g. sanity check for conflicting region names), before
we do it.
Also, looking at the diff-stat, I know this is supposed to save some
ugly code in the next commit, but it seems to move the problems else-
where. Generally I think, we should focus on a clean refactoring in-
stead of wasting time writing and bikeshedding shortcuts.
--
To view, visit https://review.coreboot.org/23678
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: I8602196e56e8903a4a22f8070a97f39628e0cc13
Gerrit-Change-Number: 23678
Gerrit-PatchSet: 5
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Sat, 10 Feb 2018 14:18:31 +0000
Gerrit-HasComments: No
Gerrit-HasLabels: Yes
Nico Huber has posted comments on this change. ( https://review.coreboot.org/23678 )
Change subject: [RFC]Don't make layoutfile and --ifd mutually exclusive
......................................................................
Patch Set 5:
(1 comment)
>> Hmmm, I'll sleep over it. Don't like the possibility to specify
>> a conflicting layout file by accident without noticing.
>
> This is already a problem if 2 homonymous but different entries
> exist in a layout file...
Wow, that's not sanity checked? where is the bug tracker?
https://review.coreboot.org/#/c/23678/1/libflashrom.c
File libflashrom.c:
https://review.coreboot.org/#/c/23678/1/libflashrom.c@377
PS1, Line 377:
> Using an append function like in 23353 could do it?
Again, only if you know that it goes to the global layout (which
should be made non-global sooner or later...).
--
To view, visit https://review.coreboot.org/23678
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: I8602196e56e8903a4a22f8070a97f39628e0cc13
Gerrit-Change-Number: 23678
Gerrit-PatchSet: 5
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Sat, 10 Feb 2018 14:11:26 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
Nico Huber has posted comments on this change. ( https://review.coreboot.org/23263 )
Change subject: Add support for reading the current flash contents from a file
......................................................................
Patch Set 12:
After all the rebasing, it would be nice if somebody could check this
commit out and retest the EDI driver.
--
To view, visit https://review.coreboot.org/23263
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: Idf153b6955f37779ae9bfb228a434ed10c304947
Gerrit-Change-Number: 23263
Gerrit-PatchSet: 12
Gerrit-Owner: Mike Banon <mikebdp2(a)gmail.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Mike Banon <mikebdp2(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Kocialkowski <contact(a)paulk.fr>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Sat, 10 Feb 2018 10:53:57 +0000
Gerrit-HasComments: No
Gerrit-HasLabels: No