January 2021 - flashrom-gerrit

Change in flashrom[master]: cbtable.c: don't assume high addresses can fully map 1 MiB
by Edward O'Callaghan (Code Review) 25 Jan '21

25 Jan '21

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/37240 ) Change subject: cbtable.c: don't assume high addresses can fully map 1 MiB ...................................................................... cbtable.c: don't assume high addresses can fully map 1 MiB Forward port the downstream `commit b17e9e41838`. When using a forwarding table entry for finding the coreboot table don't assume one has access to a full 1 MiB where the forwarding table entry points to. The reason is that the 1 MiB may cover address regions that have differing cacheability type. As such the kernel will complain and the mapping will fail. Instead, check the header first then map in the bytes that it indicates after sanity validation. That way there is no attempt at requesting an invalid mapping that spans different memory cacheability attributes. BUG=b:66681446 BRANCH=None TEST=Can successfully run 'flashrom -p host --wp-status' on kahlee without generating PAT errors. Original-Change-Id: Ic6c5832b069300cced66e11f4ca4a0bbc6e496de Original-Signed-off-by: Aaron Durbin <adurbin(a)chromium.org> Original-Reviewed-on: https://chromium-review.googlesource.com/685608 Original-Reviewed-by: Martin Roth <martinroth(a)chromium.org> Original-Reviewed-by: Justin TerAvest <teravest(a)chromium.org> Change-Id: I43705c19dd7c816098d03f528bde6f180c4c8f24 Signed-off-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M cbtable.c 1 file changed, 61 insertions(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/40/37240/1 diff --git a/cbtable.c b/cbtable.c index e566840..38a9f11 100644 --- a/cbtable.c +++ b/cbtable.c @@ -210,6 +210,66 @@ return NULL; } +static struct lb_header *find_lb_table_remap(unsigned long start_addr, + uint8_t **table_area) +{ + size_t offset; + unsigned long addr, end; + size_t mapping_size; + void *base; + + mapping_size = getpagesize(); + offset = start_addr % getpagesize(); + start_addr -= offset; + + base = physmap_ro("high tables", start_addr, mapping_size); + if (ERROR_PTR == base) { + msg_perr("Failed getting access to coreboot high tables.\n"); + return NULL; + } + + for (addr = offset, end = getpagesize(); addr < end; addr += 16) { + struct lb_record *recs; + struct lb_header *head; + + /* No more headers to check. */ + if (end - addr < sizeof(*head)) + return NULL; + + head = (struct lb_header *)(((char *)base) + addr); + + if (!lb_header_valid(head, addr)) + continue; + + if (mapping_size - addr < head->table_bytes + sizeof(*head)) { + size_t prev_mapping_size = mapping_size; + mapping_size = head->table_bytes + sizeof(*head); + mapping_size += addr; + mapping_size += getpagesize() - + (mapping_size % getpagesize()); + physunmap(base, prev_mapping_size); + base = physmap_ro("high tables", start_addr, + mapping_size); + if (ERROR_PTR == base) { + msg_perr("Failed getting access to coreboot high tables.\n"); + return NULL; + } + } + + head = (struct lb_header *)(((char *)base) + addr); + recs = + (struct lb_record *)(((char *)base) + addr + sizeof(*head)); + if (!lb_table_valid(head, recs)) + continue; + msg_pdbg("Found coreboot table at 0x%08lx.\n", addr); + *table_area = base; + return head; + } + + physunmap(base, mapping_size); + return NULL; +} + static void find_mainboard(struct lb_record *ptr, unsigned long addr) { struct lb_mainboard *rec; @@ -283,15 +343,8 @@ (((char *)lb_table) + lb_table->header_bytes); if (forward->tag == LB_TAG_FORWARD) { start = forward->forward; - start &= ~(getpagesize() - 1); physunmap_unaligned(table_area, BYTES_TO_MAP); - // FIXME: table_area is never unmapped below, nor is it unmapped above in the no-forward case - table_area = physmap_ro_unaligned("high tables", start, BYTES_TO_MAP); - if (ERROR_PTR == table_area) { - msg_perr("Failed getting access to coreboot high tables.\n"); - return -1; - } - lb_table = find_lb_table(table_area, 0x00000, 0x1000); + lb_table = find_lb_table_remap(start, &table_area); } } -- To view, visit https://review.coreboot.org/c/flashrom/+/37240 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I43705c19dd7c816098d03f528bde6f180c4c8f24 Gerrit-Change-Number: 37240 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

5 13

Change in flashrom[master]: flashrom.c: automatic generated programmer_enum.h
by Paul Menzel (Code Review) 25 Jan '21

25 Jan '21

Attention is currently required from: Xiang Wang, Stefan Reinauer. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49599 ) Change subject: flashrom.c: automatic generated programmer_enum.h ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/49599/comment/b541b397_96da1b50 PS3, Line 7: flashrom.c: automatic generated programmer_enum.h Generate programmer_enum.h automatically -- To view, visit https://review.coreboot.org/c/flashrom/+/49599 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3f4370ccae2b64da3c4178243b192700d3d205d2 Gerrit-Change-Number: 49599 Gerrit-PatchSet: 3 Gerrit-Owner: Xiang Wang <merle(a)hardenedlinux.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Idwer Vollering <vidwer(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: Shawn C <citypw(a)hardenedlinux.org> Gerrit-Attention: Xiang Wang <merle(a)hardenedlinux.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Comment-Date: Mon, 25 Jan 2021 11:26:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom.c: automatic generated programmer_enum.h
by Idwer Vollering (Code Review) 25 Jan '21

25 Jan '21

Attention is currently required from: Xiang Wang, Stefan Reinauer. Idwer Vollering has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49599 ) Change subject: flashrom.c: automatic generated programmer_enum.h ...................................................................... Patch Set 3: (3 comments) File Makefile: https://review.coreboot.org/c/flashrom/+/49599/comment/40049a83_73f3337d PS3, Line 681: $(shell bash ./util/generator_programmer_enum.sh) Calling bash directly is unnecessary when changing the permissions of util/generator_programmer_enum.sh to 0755. File util/generator_programmer_enum.sh: https://review.coreboot.org/c/flashrom/+/49599/comment/673b0b15_6ec0ac85 PS3, Line 1: #! /bin/bash Please locate the bash binary using env: '/usr/bin/env bash' https://review.coreboot.org/c/coreboot/+/48904 https://review.coreboot.org/c/flashrom/+/49599/comment/632fd344_e9d825fd PS3, Line 22: generator Typo: 'this file is automatically generated by ...' -- To view, visit https://review.coreboot.org/c/flashrom/+/49599 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3f4370ccae2b64da3c4178243b192700d3d205d2 Gerrit-Change-Number: 49599 Gerrit-PatchSet: 3 Gerrit-Owner: Xiang Wang <merle(a)hardenedlinux.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Idwer Vollering <vidwer(a)gmail.com> Gerrit-CC: Shawn C <citypw(a)hardenedlinux.org> Gerrit-Attention: Xiang Wang <merle(a)hardenedlinux.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Comment-Date: Mon, 25 Jan 2021 10:09:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: it87spi.c: Simplify param free like cros
by Edward O'Callaghan (Code Review) 24 Jan '21

24 Jan '21

Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49823 ) Change subject: it87spi.c: Simplify param free like cros ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > This undoes what CB:46551 fixed. Thank you for the catch, patch and history Angel! -- To view, visit https://review.coreboot.org/c/flashrom/+/49823 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia17dfcaf758f729ae25b35d807bfbe5dc3dc5ff6 Gerrit-Change-Number: 49823 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sun, 24 Jan 2021 23:05:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: it87spi.c: Simplify param free like cros
by Edward O'Callaghan (Code Review) 24 Jan '21

24 Jan '21

Edward O'Callaghan has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/49823 ) Change subject: it87spi.c: Simplify param free like cros ...................................................................... Abandoned note to self, try to remember cros patches are mostly likely garbage -- To view, visit https://review.coreboot.org/c/flashrom/+/49823 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia17dfcaf758f729ae25b35d807bfbe5dc3dc5ff6 Gerrit-Change-Number: 49823 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: abandon

1 0

Change in flashrom[master]: helpers.c: Fix undefined behavior in strndup()
by Edward O'Callaghan (Code Review) 24 Jan '21

24 Jan '21

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/49741 ) Change subject: helpers.c: Fix undefined behavior in strndup() ...................................................................... helpers.c: Fix undefined behavior in strndup() Using strlen() or strdup() inside strndup() is problematic: if the input string is not null-terminated, these functions can read past the end of the buffer, which triggers undefined behavior. Rewrite the function to never read past the provided `maxlen` bound. Change-Id: Id34127024085879228626fbad59af03268ec5255 Signed-off-by: Xiang Wang <merle(a)hardenedliux.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/49741 Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M helpers.c 1 file changed, 9 insertions(+), 8 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved diff --git a/helpers.c b/helpers.c index c83cd2c..289848d 100644 --- a/helpers.c +++ b/helpers.c @@ -106,15 +106,16 @@ /* strndup is a POSIX function not present in MinGW */ char *strndup(const char *src, size_t maxlen) { - if (strlen(src) > maxlen) { - char *retbuf; - if ((retbuf = malloc(1 + maxlen)) != NULL) { - memcpy(retbuf, src, maxlen); - retbuf[maxlen] = '\0'; - } - return retbuf; + char *retbuf; + size_t len; + for (len = 0; len < maxlen; len++) + if (src[len] == '\0') + break; + if ((retbuf = malloc(1 + len)) != NULL) { + memcpy(retbuf, src, len); + retbuf[len] = '\0'; } - return strdup(src); + return retbuf; } #endif -- To view, visit https://review.coreboot.org/c/flashrom/+/49741 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id34127024085879228626fbad59af03268ec5255 Gerrit-Change-Number: 49741 Gerrit-PatchSet: 3 Gerrit-Owner: Xiang Wang <merle(a)hardenedlinux.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: Shawn C <citypw(a)hardenedlinux.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: ft2232_spi.c: Rename cs_bits to pinlvl
by Edward O'Callaghan (Code Review) 24 Jan '21

24 Jan '21

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/49780 ) Change subject: ft2232_spi.c: Rename cs_bits to pinlvl ...................................................................... ft2232_spi.c: Rename cs_bits to pinlvl Renames the variable cs_bits to pinlvl, to more accurately reflect its role. pinlvl works in conjunction with pindir to specify GPIO pin state. Signed-off-by: Alan Green <avg(a)google.com> Change-Id: I53d1ccae8fa870f037b03762bec79fee1b7bad21 Reviewed-on: https://review.coreboot.org/c/flashrom/+/49780 Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M ft2232_spi.c 1 file changed, 12 insertions(+), 12 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved diff --git a/ft2232_spi.c b/ft2232_spi.c index 0d287df..9060973 100644 --- a/ft2232_spi.c +++ b/ft2232_spi.c @@ -84,7 +84,7 @@ #define BITMODE_BITBANG_NORMAL 1 #define BITMODE_BITBANG_SPI 2 -/* The variables cs_bits and pindir store the values for the "set data bits low byte" MPSSE command that +/* The variables pinlvl and pindir store the values for the "set data bits low byte" MPSSE command that * sets the initial state and the direction of the I/O pins. The pin offsets are as follows: * TCK/SK is bit 0. * TDI/DO is bit 1. @@ -102,7 +102,7 @@ * value: 0x08 CS=high, DI=low, DO=low, SK=low * dir: 0x0b CS=output, DI=input, DO=output, SK=output */ -static uint8_t cs_bits = 0x08; +static uint8_t pinlvl = 0x08; static uint8_t pindir = 0x0b; static struct ftdi_context ftdic_context; @@ -246,7 +246,7 @@ /* JTAGkey(2) needs to enable its output via Bit4 / GPIOL0 * value: 0x18 OE=high, CS=high, DI=low, DO=low, SK=low * dir: 0x1b OE=output, CS=output, DI=input, DO=output, SK=output */ - cs_bits = 0x18; + pinlvl = 0x18; pindir = 0x1b; } else if (!strcasecmp(arg, "picotap")) { ft2232_vid = GOEPEL_VID; @@ -264,7 +264,7 @@ /* In its default configuration it is a jtagkey clone */ ft2232_type = FTDI_FT2232H_PID; channel_count = 2; - cs_bits = 0x18; + pinlvl = 0x18; pindir = 0x1b; } else if (!strcasecmp(arg, "openmoko")) { ft2232_vid = FIC_VID; @@ -277,7 +277,7 @@ /* arm-usb-ocd(-h) has an output buffer that needs to be enabled by pulling ADBUS4 low. * value: 0x08 #OE=low, CS=high, DI=low, DO=low, SK=low * dir: 0x1b #OE=output, CS=output, DI=input, DO=output, SK=output */ - cs_bits = 0x08; + pinlvl = 0x08; pindir = 0x1b; } else if (!strcasecmp(arg, "arm-usb-tiny")) { ft2232_vid = OLIMEX_VID; @@ -288,7 +288,7 @@ ft2232_type = OLIMEX_ARM_OCD_H_PID; channel_count = 2; /* See arm-usb-ocd */ - cs_bits = 0x08; + pinlvl = 0x08; pindir = 0x1b; } else if (!strcasecmp(arg, "arm-usb-tiny-h")) { ft2232_vid = OLIMEX_VID; @@ -381,7 +381,7 @@ return -2; } else { unsigned int pin = temp + 4; - cs_bits |= 1 << pin; + pinlvl |= 1 << pin; pindir |= 1 << pin; } } @@ -398,11 +398,11 @@ unsigned int pin = i + 4; switch (toupper(arg[i])) { case 'H': - cs_bits |= 1 << pin; + pinlvl |= 1 << pin; pindir |= 1 << pin; break; case 'L': - cs_bits &= ~(1 << pin); + pinlvl &= ~(1 << pin); pindir |= 1 << pin; break; case 'Z': @@ -512,7 +512,7 @@ msg_pdbg("Set data bits\n"); buf[0] = SET_BITS_LOW; - buf[1] = cs_bits; + buf[1] = pinlvl; buf[2] = pindir; if (send_buf(ftdic, buf, 3)) { ret = -8; @@ -569,7 +569,7 @@ */ msg_pspew("Assert CS#\n"); buf[i++] = SET_BITS_LOW; - buf[i++] = ~ 0x08 & cs_bits; /* assert CS (3rd) bit only */ + buf[i++] = ~ 0x08 & pinlvl; /* assert CS (3rd) bit only */ buf[i++] = pindir; if (writecnt) { @@ -610,7 +610,7 @@ msg_pspew("De-assert CS#\n"); buf[i++] = SET_BITS_LOW; - buf[i++] = cs_bits; + buf[i++] = pinlvl; buf[i++] = pindir; ret = send_buf(ftdic, buf, i); failed |= ret; -- To view, visit https://review.coreboot.org/c/flashrom/+/49780 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I53d1ccae8fa870f037b03762bec79fee1b7bad21 Gerrit-Change-Number: 49780 Gerrit-PatchSet: 5 Gerrit-Owner: Alan Green <avg(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: ft2232_spi.c: Generalize GPIOL pin control
by Edward O'Callaghan (Code Review) 24 Jan '21

24 Jan '21

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/49637 ) Change subject: ft2232_spi.c: Generalize GPIOL pin control ...................................................................... ft2232_spi.c: Generalize GPIOL pin control Adds a new arg "gpiol" to allow the four FT2232 GPIOL pins to be set to any combination of high, low or high-impedance. The existing arg "csgpiol", is intended to function as an additional "cs" signal, allowing pins to be set high but not low. This patch preserves the csgpiol arg for backward compatibility. In the event that both arguments are specified, gpiol is used. Signed-off-by: Alan Green <avg(a)google.com> Change-Id: I1f2b3b968577e62e3c5b11bcdf4afe2de6eb84ab Reviewed-on: https://review.coreboot.org/c/flashrom/+/49637 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Xiang Wang <merle(a)hardenedlinux.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M ft2232_spi.c 1 file changed, 43 insertions(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Xiang Wang: Looks good to me, approved Angel Pons: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved diff --git a/ft2232_spi.c b/ft2232_spi.c index c963a45..0d287df 100644 --- a/ft2232_spi.c +++ b/ft2232_spi.c @@ -367,7 +367,7 @@ } free(arg); - /* Allows setting multiple GPIOL states, for example: csgpiol=012 */ + /* Allows setting multiple GPIOL pins to high, for example: csgpiol=012 */ arg = extract_programmer_param("csgpiol"); if (arg) { unsigned int ngpios = strlen(arg); @@ -388,6 +388,48 @@ } free(arg); + /* Allows setting GPIOL pins high, low or input (high-z) */ + arg = extract_programmer_param("gpiol"); + if (arg) { + int ok = 0; + if (strlen(arg) == 4) { + ok = 1; + for (int i = 0; i < 4; i++) { + unsigned int pin = i + 4; + switch (toupper(arg[i])) { + case 'H': + cs_bits |= 1 << pin; + pindir |= 1 << pin; + break; + case 'L': + cs_bits &= ~(1 << pin); + pindir |= 1 << pin; + break; + case 'Z': + pindir &= ~(1 << pin); + break; + case 'X': + break; + default: + ok = 0; + } + } + } + if (!ok) { + msg_perr("Error: Invalid GPIOLs specified: \"%s\".\n" + "Valid values are 4 character strings of H, L, Z and X.\n" + " H - Set GPIOL output high\n" + " L - Set GPIOL output low\n" + " Z - Set GPIOL as input (high impedance)\n" + " X - Leave as programmer default\n" + "Example: gpiol=LZXH drives GPIOL 0 low, and GPIOL 3 high, sets GPIOL 1\n" + "to an input and leaves GPIOL 2 set according to the programmer type.\n", arg); + free(arg); + return -2; + } + } + free(arg); + msg_pdbg("Using device type %s %s ", get_ft2232_vendorname(ft2232_vid, ft2232_type), get_ft2232_devicename(ft2232_vid, ft2232_type)); -- To view, visit https://review.coreboot.org/c/flashrom/+/49637 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1f2b3b968577e62e3c5b11bcdf4afe2de6eb84ab Gerrit-Change-Number: 49637 Gerrit-PatchSet: 11 Gerrit-Owner: Alan Green <avg(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Xiang Wang <merle(a)hardenedlinux.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: ft2232_spi.c: release I/Os on shutdown
by Edward O'Callaghan (Code Review) 24 Jan '21

24 Jan '21

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/49632 ) Change subject: ft2232_spi.c: release I/Os on shutdown ...................................................................... ft2232_spi.c: release I/Os on shutdown Reset FTDI I/O pins to high-Z (input) when shutting down. This allows other devices to use the SPI bus without having to disconnect the programmer. This change will introduce a backward incompatibility in the case where a user is relying on the state of FTDI outputs post-programming (eg. to disallow another device from driving CS low). However, there are likely more cases where releasing the SPI bus is the correct thing to do. Signed-off-by: Alan Green <avg(a)google.com> Change-Id: I9fae55e532595752983f55fac2298f81699dbe5b Reviewed-on: https://review.coreboot.org/c/flashrom/+/49632 Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M ft2232_spi.c 1 file changed, 25 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, but someone else must approve Edward O'Callaghan: Looks good to me, approved diff --git a/ft2232_spi.c b/ft2232_spi.c index 65ff449..c963a45 100644 --- a/ft2232_spi.c +++ b/ft2232_spi.c @@ -159,6 +159,30 @@ return 0; } +static int ft2232_shutdown(void *data) +{ + int f; + struct ftdi_context *ftdic = (struct ftdi_context *) data; + unsigned char buf[3]; + + msg_pdbg("Releasing I/Os\n"); + buf[0] = SET_BITS_LOW; + buf[1] = 0; /* Output byte ignored */ + buf[2] = 0; /* Pin direction: all inputs */ + if (send_buf(ftdic, buf, 3)) { + msg_perr("Unable to set pins back inputs: (%s)\n", + ftdi_get_error_string(ftdic)); + } + + if ((f = ftdi_usb_close(ftdic)) < 0) { + msg_perr("Unable to close FTDI device: %d (%s)\n", f, + ftdi_get_error_string(ftdic)); + return f; + } + + return 0; +} + static int ft2232_spi_send_command(const struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, @@ -453,6 +477,7 @@ goto ftdi_err; } + register_shutdown(ft2232_shutdown, ftdic); register_spi_master(&spi_master_ft2232); return 0; -- To view, visit https://review.coreboot.org/c/flashrom/+/49632 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9fae55e532595752983f55fac2298f81699dbe5b Gerrit-Change-Number: 49632 Gerrit-PatchSet: 9 Gerrit-Owner: Alan Green <avg(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: helpers.c: Fix undefined behavior in strndup()
by Xiang Wang (Code Review) 24 Jan '21

24 Jan '21

Attention is currently required from: Stefan Reinauer. Xiang Wang has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49741 ) Change subject: helpers.c: Fix undefined behavior in strndup() ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/49741/comment/d9fae29e_25c98017 PS1, Line 7: helpers.c: optimize strndup > About adding a unit-test for this function: Maybe, but it is only used for MinGW. […] I don't know how to write effective test code -- To view, visit https://review.coreboot.org/c/flashrom/+/49741 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id34127024085879228626fbad59af03268ec5255 Gerrit-Change-Number: 49741 Gerrit-PatchSet: 2 Gerrit-Owner: Xiang Wang <merle(a)hardenedlinux.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: Shawn C <citypw(a)hardenedlinux.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Comment-Date: Sun, 24 Jan 2021 06:20:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Xiang Wang <merle(a)hardenedlinux.org> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit January 2021