October 2017 - flashrom-gerrit

Change in flashrom[staging]: Use bzip2 when making tarball
by David Hendricks (Code Review) 20 Oct '17

20 Oct '17

David Hendricks has uploaded this change for review. ( https://review.coreboot.org/22116 Change subject: Use bzip2 when making tarball ...................................................................... Use bzip2 when making tarball Tarballs on download.flashrom.org are generally packaged using bzip2, so we may as well be internally consistent. Change-Id: Ib9fb1ea6d5994cd0285ce8db9675640fae992773 Signed-off-by: David Hendricks <dhendricks(a)fb.com> --- M Makefile 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/16/22116/1 diff --git a/Makefile b/Makefile index 52f6c98..5bd7158 100644 --- a/Makefile +++ b/Makefile @@ -1408,11 +1408,11 @@ @echo "Exported $(EXPORTDIR)/flashrom-$(RELEASENAME)/" tarball: _export - @tar -cz --format=ustar -f "$(EXPORTDIR)/flashrom-$(RELEASENAME).tar.gz" -C $(EXPORTDIR)/ \ + @tar -cj --format=ustar -f "$(EXPORTDIR)/flashrom-$(RELEASENAME).tar.bz2" -C $(EXPORTDIR)/ \ $(TAROPTIONS) "flashrom-$(RELEASENAME)/" # Delete the exported directory again because it is most likely what's expected by the user. @rm -rf "$(EXPORTDIR)/flashrom-$(RELEASENAME)" - @echo Created "$(EXPORTDIR)/flashrom-$(RELEASENAME).tar.gz" + @echo Created "$(EXPORTDIR)/flashrom-$(RELEASENAME).tar.bz2" libpayload: clean make CC="CC=i386-elf-gcc lpgcc" AR=i386-elf-ar RANLIB=i386-elf-ranlib -- To view, visit https://review.coreboot.org/22116 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: newchange Gerrit-Change-Id: Ib9fb1ea6d5994cd0285ce8db9675640fae992773 Gerrit-Change-Number: 22116 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com>

1 0

Change in flashrom[staging]: Fix serprog on FreeBSD
by build bot (Jenkins) (Code Review) 19 Oct '17

19 Oct '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/21919 ) Change subject: Fix serprog on FreeBSD ...................................................................... Patch Set 3: Build Successful https://qa.coreboot.org/job/flashrom-customrules/797/ : SUCCESS -- To view, visit https://review.coreboot.org/21919 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: I7aa6a283d523c544d9b8923cd4c622bf08c0fb3f Gerrit-Change-Number: 21919 Gerrit-PatchSet: 3 Gerrit-Owner: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Urja Rannikko <urjaman(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 19 Oct 2017 15:17:16 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: Fix serprog on FreeBSD
by Nico Huber (Code Review) 19 Oct '17

19 Oct '17

Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/21919 ) Change subject: Fix serprog on FreeBSD ...................................................................... Fix serprog on FreeBSD Using serprog on FreeBSD to read an SPI flash (MX25L6406) via an Arduino Nano V3 with flashrom hangs after 5 seconds while reading. The problem is that kernel method "ttydisc_rint" ignores some bytes. It happens due to enabled IEXTEN local flag of termios. TTY cuts a few bytes, Arduino reads 11264 bytes, but flashrom gets only 11244 bytes and waits for the remaining 20 bytes. The fix is simple: turn off the IEXTEN local flag. Tested on Arduino Nano V3 + FreeBSD 12-CURRENT. Change-Id: I7aa6a283d523c544d9b8923cd4c622bf08c0fb3f Signed-off-by: Michael Zhilin <mizhka(a)gmail.com> Reviewed-on: https://review.coreboot.org/21919 Reviewed-by: Nico Huber <nico.h(a)gmx.de> Reviewed-by: Urja Rannikko <urjaman(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M serial.c 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Nico Huber: Looks good to me, approved Urja Rannikko: Looks good to me, approved build bot (Jenkins): Verified diff --git a/serial.c b/serial.c index 608464b..aa75708 100644 --- a/serial.c +++ b/serial.c @@ -215,7 +215,7 @@ } wanted.c_cflag &= ~(PARENB | CSTOPB | CSIZE | CRTSCTS); wanted.c_cflag |= (CS8 | CLOCAL | CREAD); - wanted.c_lflag &= ~(ICANON | ECHO | ECHOE | ISIG); + wanted.c_lflag &= ~(ICANON | ECHO | ECHOE | ISIG | IEXTEN); wanted.c_iflag &= ~(IXON | IXOFF | IXANY | ICRNL | IGNCR | INLCR); wanted.c_oflag &= ~OPOST; if (tcsetattr(fd, TCSANOW, &wanted) != 0) { -- To view, visit https://review.coreboot.org/21919 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: merged Gerrit-Change-Id: I7aa6a283d523c544d9b8923cd4c622bf08c0fb3f Gerrit-Change-Number: 21919 Gerrit-PatchSet: 3 Gerrit-Owner: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Urja Rannikko <urjaman(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[staging]: Fix serprog on FreeBSD
by build bot (Jenkins) (Code Review) 19 Oct '17

19 Oct '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/21919 ) Change subject: Fix serprog on FreeBSD ...................................................................... Patch Set 2: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom_gerrit/707/ : SUCCESS https://qa.coreboot.org/job/flashrom-customrules/796/ : SUCCESS -- To view, visit https://review.coreboot.org/21919 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: I7aa6a283d523c544d9b8923cd4c622bf08c0fb3f Gerrit-Change-Number: 21919 Gerrit-PatchSet: 2 Gerrit-Owner: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Urja Rannikko <urjaman(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 19 Oct 2017 15:15:36 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: serial: Support custom baud rates on linux
by build bot (Jenkins) (Code Review) 19 Oct '17

19 Oct '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/20224 ) Change subject: serial: Support custom baud rates on linux ...................................................................... Patch Set 6: Build Successful https://qa.coreboot.org/job/flashrom-customrules/795/ : SUCCESS -- To view, visit https://review.coreboot.org/20224 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: I2a20f9182cb85e7bce5d6654a2caf20e6202b195 Gerrit-Change-Number: 20224 Gerrit-PatchSet: 6 Gerrit-Owner: Urja Rannikko <urjaman(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: Thu, 19 Oct 2017 15:14:49 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: serial: Support custom baud rates on linux
by Nico Huber (Code Review) 19 Oct '17

19 Oct '17

Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/20224 ) Change subject: serial: Support custom baud rates on linux ...................................................................... serial: Support custom baud rates on linux The function to do this is contained in custom_baud.c because of broken include stuff. Change-Id: I2a20f9182cb85e7bce5d6654a2caf20e6202b195 Signed-off-by: Urja Rannikko <urjaman(a)gmail.com> Reviewed-on: https://review.coreboot.org/20224 Reviewed-by: Nico Huber <nico.h(a)gmx.de> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M Makefile A custom_baud.c A custom_baud.h M serial.c 4 files changed, 136 insertions(+), 11 deletions(-) Approvals: Nico Huber: Looks good to me, approved build bot (Jenkins): Verified diff --git a/Makefile b/Makefile index 5573127..52f6c98 100644 --- a/Makefile +++ b/Makefile @@ -921,7 +921,7 @@ endif ifneq ($(NEED_SERIAL), ) -LIB_OBJS += serial.o +LIB_OBJS += serial.o custom_baud.o endif ifneq ($(NEED_POSIX_SOCKETS), ) diff --git a/custom_baud.c b/custom_baud.c new file mode 100644 index 0000000..0caca80 --- /dev/null +++ b/custom_baud.c @@ -0,0 +1,78 @@ +/* + * This file is part of the flashrom project. + * + * Copyright (C) 2017 Urja Rannikko <urjaman(a)gmail.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include "platform.h" +#include "custom_baud.h" + +#if IS_LINUX +#include <sys/ioctl.h> +#include <fcntl.h> +#include <asm-generic/termbits.h> +#include <asm-generic/ioctls.h> + +/* + * This include hell above is why this is in a separate source file. See eg. + * https://www.downtowndougbrown.com/2013/11/linux-custom-serial-baud-rates/ + * https://stackoverflow.com/questions/12646324/how-to-set-a-custom-baud-rate-… + * https://github.com/jbkim/Linux-custom-baud-rate + * for more info. + */ + +int set_custom_baudrate(int fd, unsigned int baud) +{ + struct termios2 tio; + if (ioctl(fd, TCGETS2, &tio)) { + return -1; + } + tio.c_cflag &= ~CBAUD; + tio.c_cflag |= BOTHER; + tio.c_ispeed = baud; + tio.c_ospeed = baud; + return ioctl(fd, TCSETS2, &tio); +} + +int use_custom_baud(unsigned int baud, const struct baudentry *baudtable) +{ + int i; + for (i = 0; baudtable[i].baud; i++) { + if (baudtable[i].baud == baud) + return 0; + + if (baudtable[i].baud > baud) + return 1; + } + return 1; +} + +#else +#include <errno.h> + +/* Stub, should not get called. */ +int set_custom_baudrate(int fd, unsigned int baud) +{ + errno = ENOSYS; /* Hoping "Function not supported" will make you look here. */ + return -1; +} + +int use_custom_baud(unsigned int baud, const struct baudentry *baudtable) +{ + return 0; +} +#endif diff --git a/custom_baud.h b/custom_baud.h new file mode 100644 index 0000000..ae286f5 --- /dev/null +++ b/custom_baud.h @@ -0,0 +1,35 @@ +/* + * This file is part of the flashrom project. + * + * Copyright (C) 2017 Urja Rannikko <urjaman(a)gmail.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#ifndef __CUSTOM_BAUD_H__ +#define __CUSTOM_BAUD_H__ 1 + +struct baudentry { + int flag; + unsigned int baud; +}; + +int set_custom_baudrate(int fd, unsigned int baud); + +/* Returns 1 if non-exact rate would be used, and setting a custom rate is supported. + The baudtable must be in ascending order and terminated with a 0-baud entry. */ +int use_custom_baud(unsigned int baud, const struct baudentry *baudtable); + +#endif diff --git a/serial.c b/serial.c index a64a51d..608464b 100644 --- a/serial.c +++ b/serial.c @@ -40,6 +40,7 @@ #endif #include "flash.h" #include "programmer.h" +#include "custom_baud.h" fdtype sp_fd = SER_INV_FD; @@ -49,18 +50,14 @@ * The code below creates a mapping in sp_baudtable between these macros and the numerical baud rates to deal * with numerical user input. * - * On Linux there is a non-standard way to use arbitrary baud rates that flashrom does not support (yet), cf. - * http://www.downtowndougbrown.com/2013/11/linux-custom-serial-baud-rates/ + * On Linux there is a non-standard way to use arbitrary baud rates that we use if there is no + * matching standard rate, see custom_baud.c * * On Windows there exist similar macros (starting with CBR_ instead of B) but they are only defined for * backwards compatibility and the API supports arbitrary baud rates in the same manner as the macros, see * http://msdn.microsoft.com/en-us/library/windows/desktop/aa363214(v=vs.85).a… */ #if !IS_WINDOWS -struct baudentry { - int flag; - unsigned int baud; -}; #define BAUDENTRY(baud) { B##baud, baud }, static const struct baudentry sp_baudtable[] = { @@ -195,10 +192,25 @@ } wanted = observed; if (baud >= 0) { - const struct baudentry *entry = round_baud(baud); - if (cfsetispeed(&wanted, entry->flag) != 0 || cfsetospeed(&wanted, entry->flag) != 0) { - msg_perr_strerror("Could not set serial baud rate: "); - return 1; + if (use_custom_baud(baud, sp_baudtable)) { + if (set_custom_baudrate(fd, baud)) { + msg_perr_strerror("Could not set custom baudrate: "); + return 1; + } + /* We want whatever the termios looks like now, so the rest of the + setup doesnt mess up the custom rate. */ + if (tcgetattr(fd, &wanted) != 0) { + /* This should pretty much never happen (see above), but.. */ + msg_perr_strerror("Could not fetch serial port configuration: "); + return 1; + } + msg_pdbg("Using custom baud rate.\n"); + } else { + const struct baudentry *entry = round_baud(baud); + if (cfsetispeed(&wanted, entry->flag) != 0 || cfsetospeed(&wanted, entry->flag) != 0) { + msg_perr_strerror("Could not set serial baud rate: "); + return 1; + } } } wanted.c_cflag &= ~(PARENB | CSTOPB | CSIZE | CRTSCTS); -- To view, visit https://review.coreboot.org/20224 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: merged Gerrit-Change-Id: I2a20f9182cb85e7bce5d6654a2caf20e6202b195 Gerrit-Change-Number: 20224 Gerrit-PatchSet: 6 Gerrit-Owner: Urja Rannikko <urjaman(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>

1 0

Change in flashrom[staging]: spi25: Enable direct 4BA read and write using feature bits
by Nico Huber (Code Review) 17 Oct '17

17 Oct '17

Nico Huber has posted comments on this change. ( https://review.coreboot.org/22021 ) Change subject: spi25: Enable direct 4BA read and write using feature bits ...................................................................... Patch Set 1: (2 comments) https://review.coreboot.org/#/c/22021/1/flash.h File flash.h: https://review.coreboot.org/#/c/22021/1/flash.h@124 PS1, Line 124: #define FEATURE_4BA_READ (1 << 12) /* Native 4BA read instruction (0x13) is supported */ https://review.coreboot.org/#/c/22021/1/flash.h@125 PS1, Line 125: #define FEATURE_4BA_WRITE (1 << 13) /* Native 4BA byte program (0x12) is supported */ -- To view, visit https://review.coreboot.org/22021 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: I2f6817ca198bf923671a7aa67e956e5477d71848 Gerrit-Change-Number: 22021 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 17 Oct 2017 18:21:53 +0000 Gerrit-HasComments: Yes

1 0

Change in flashrom[staging]: spi25: Add native 4BA support
by Nico Huber (Code Review) 17 Oct '17

17 Oct '17

Nico Huber has posted comments on this change. ( https://review.coreboot.org/22020 ) Change subject: spi25: Add native 4BA support ...................................................................... Patch Set 1: (2 comments) > The code here looks fine, I just want to check my understanding of > how you're deciding whether to use 4BA native instructions or > extended address mode. As you're probably already aware, some chips > support one but not the other, so we need to be a little careful. I target to have all information (excluding erase instructions) about supported instructions in the feature bits. > From what I've seen, it's safer to use native 4BA instructions when > they're available since they should work no matter what mode the > chip is in. This is exactly what (I think) I'm doing in the next patch. e.g. `FEATURE_4BA_READ` means the native 4BA read instruction is sup- ported and if it's set, we just use that. It only works, though, if `FEATURE_4BA_SUPPORT` is set too and `FEATURE_4BA_EXT_ADDR` is not. I have to admit I'm a bit clueless about the latter, e.g. are there chips that support an extended address register and native 4BA instructions? If yes, or if we just want to pre- pare for it, spi_prepare_address() will need a parameter to override the default choice, I guess. > > Also, we should consider how we might override the default behavior > in case the programmer can only operate in a particular mode (I'm > thinking Intel chipsets and Dediprog programmers). My preferred solution is to have more feature bits. One for each sup- ported instruction if need be. The programmer driver can choose for itself then what to do (or to bail out). https://review.coreboot.org/#/c/22020/1/flash.h File flash.h: https://review.coreboot.org/#/c/22020/1/flash.h@123 PS1, Line 123: #define FEATURE_4BA_EXT_ADDR (1 << 11) I shall comment the feature bits in my next iteration. This one: /* Regular 3-byte operations can be used by writing the most signifcant address byte into an extended address register. */ https://review.coreboot.org/#/c/22020/1/spi25.c File spi25.c: https://review.coreboot.org/#/c/22020/1/spi25.c@363 PS1, Line 363: flash->address_high_byte = addr_high; > The follow-up patch seems to use native 4BA instructions for read and write On certain feature-bit combinations this can indeed be a problem. -- To view, visit https://review.coreboot.org/22020 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: I644600beaab9a571b97b67f7516abe571d3460c1 Gerrit-Change-Number: 22020 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 17 Oct 2017 18:17:13 +0000 Gerrit-HasComments: Yes

1 0

Change in flashrom[staging]: spi25: Use common code for nbyte read/write and erase
by Nico Huber (Code Review) 17 Oct '17

17 Oct '17

Nico Huber has posted comments on this change. ( https://review.coreboot.org/22019 ) Change subject: spi25: Use common code for nbyte read/write and erase ...................................................................... Patch Set 1: (3 comments) https://review.coreboot.org/#/c/22019/1/spi25.c File spi25.c: https://review.coreboot.org/#/c/22019/1/spi25.c@326 PS1, Line 326: spi_write_status > spi_poll_status? Yeah, that is what it does. Though, with all the FIXMEs fixed, it should be more like check_write_status()... Don't have a strong preference (but it should keep `write` in its name). spi_poll_wip() maybe? https://review.coreboot.org/#/c/22019/1/spi25.c@377 PS1, Line 377: JEDEC_WREN > Hmmm, what do we do about chips that only use EWSR? I know this patch is on I'm not an expert, but thought EWSR is actually only used before status register updates. https://review.coreboot.org/#/c/22019/1/spi25.c@383 PS1, Line 383: > Check if out_len < 1? Hmmm, no as it's also used for block erasers. I guess the function name needs to reflect that better, or the signature needs to be commented. -- To view, visit https://review.coreboot.org/22019 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: Ibc1ae48acbfbd427a30bcd64bdc080dc3dc20503 Gerrit-Change-Number: 22019 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 17 Oct 2017 17:38:57 +0000 Gerrit-HasComments: Yes

1 0

Change in flashrom[staging]: spi25: Introduce spi_simple_write_cmd()
by Nico Huber (Code Review) 17 Oct '17

17 Oct '17

Nico Huber has posted comments on this change. ( https://review.coreboot.org/22018 ) Change subject: spi25: Introduce spi_simple_write_cmd() ...................................................................... Patch Set 1: (1 comment) https://review.coreboot.org/#/c/22018/1/spi25.c File spi25.c: https://review.coreboot.org/#/c/22018/1/spi25.c@326 PS1, Line 326: spi_simple_write_cmd > Is there anything other than chip erase commands that this applies to? I th Yes, I've repurposed this later for 4BA-enable functions that need a WREN. With that said, we could also name the functions after WREN, e.g. static int spi_simple_wren_cmd(...); and static int spi_wren_cmd(...); instead of `spi_write_cmd()` (that is added later) as it's also used for erasures. Or if we want to emphasize the command parts even more: static int spi_wren_simple_cmd(...); static int spi_wren_address_cmd(...); My head is full of ideas, I just don't have a strong preference. Another thought: Would you prefer both functions to be commented, i.e. a doxygen style interface documentation? I'm often unsure if it's necessary as the code is already very specific about what is done ;) and having it commented means we have to take special care to keep the comment in sync with the implementation. -- To view, visit https://review.coreboot.org/22018 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: Ib244356fa471e15863b52e6037899d19113cb4a9 Gerrit-Change-Number: 22018 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 17 Oct 2017 17:27:17 +0000 Gerrit-HasComments: Yes

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit October 2017