January 2021 - flashrom-gerrit

Change in flashrom[master]: bitbang-spi.c: support clock polarity and phase
by Angel Pons (Code Review) 10 Jan '21

10 Jan '21

Attention is currently required from: Xiang Wang, Stefan Reinauer, Edward O'Callaghan. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49255 ) Change subject: bitbang-spi.c: support clock polarity and phase ...................................................................... Patch Set 14: (1 comment) File bitbang_spi.c: https://review.coreboot.org/c/flashrom/+/49255/comment/e74f6b8f_f3924cb4 PS14, Line 210: ERROR_FLASHROM_BUG Definitely not a flashrom bug. -- To view, visit https://review.coreboot.org/c/flashrom/+/49255 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I04c1dfe132d756119229b27c3cd611d1be1abc8d Gerrit-Change-Number: 49255 Gerrit-PatchSet: 14 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: Shawn C <citypw(a)hardenedlinux.org> Gerrit-Attention: Xiang Wang <merle(a)hardenedlinux.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Sun, 10 Jan 2021 12:36:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: bitbang-spi.c: support clock polarity and phase
by Angel Pons (Code Review) 10 Jan '21

10 Jan '21

Attention is currently required from: Xiang Wang, Stefan Reinauer, Edward O'Callaghan. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49255 ) Change subject: bitbang-spi.c: support clock polarity and phase ...................................................................... Patch Set 14: (1 comment) File bitbang_spi.c: https://review.coreboot.org/c/flashrom/+/49255/comment/42d4cfd9_d7528830 PS14, Line 53: master->set_sck(val); For sanity's sake, only use `cpol` here. -- To view, visit https://review.coreboot.org/c/flashrom/+/49255 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I04c1dfe132d756119229b27c3cd611d1be1abc8d Gerrit-Change-Number: 49255 Gerrit-PatchSet: 14 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: Shawn C <citypw(a)hardenedlinux.org> Gerrit-Attention: Xiang Wang <merle(a)hardenedlinux.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Sun, 10 Jan 2021 11:28:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: bitbang-spi.c: support clock polarity and phase
by Angel Pons (Code Review) 10 Jan '21

10 Jan '21

Attention is currently required from: Xiang Wang, Stefan Reinauer, Edward O'Callaghan. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49255 ) Change subject: bitbang-spi.c: support clock polarity and phase ...................................................................... Patch Set 14: (2 comments) File bitbang_spi.c: https://review.coreboot.org/c/flashrom/+/49255/comment/bc7652ca_f21a1c43 PS14, Line 74: bitbang_spi_set_sck_set_mosi Where did this go? https://review.coreboot.org/c/flashrom/+/49255/comment/5bd932a7_6aca3ab4 PS14, Line 68: bitbang_spi_set_sck_set_mosi If you're reordering the operations inside this function, I would also rename everything accordingly. Preferably, in a separate patch. -- To view, visit https://review.coreboot.org/c/flashrom/+/49255 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I04c1dfe132d756119229b27c3cd611d1be1abc8d Gerrit-Change-Number: 49255 Gerrit-PatchSet: 14 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: Shawn C <citypw(a)hardenedlinux.org> Gerrit-Attention: Xiang Wang <merle(a)hardenedlinux.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Sun, 10 Jan 2021 11:16:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: bitbang_spi.c: fix bitbang_spi_send_command to make the delay symmetric
by Angel Pons (Code Review) 10 Jan '21

10 Jan '21

Attention is currently required from: Xiang Wang, Stefan Reinauer. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49262 ) Change subject: bitbang_spi.c: fix bitbang_spi_send_command to make the delay symmetric ...................................................................... Patch Set 9: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/49262/comment/861b0cbe_b1cf5269 PS9, Line 7: fix What is being fixed? In other words, how did you test this? Please mention it in the commit message. -- To view, visit https://review.coreboot.org/c/flashrom/+/49262 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Icf8b65aed20244ccffc231da2c599c6ca1796104 Gerrit-Change-Number: 49262 Gerrit-PatchSet: 9 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: 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: Sun, 10 Jan 2021 11:10:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: sysfsgpio.c implement spi interface via linux sysfs
by Angel Pons (Code Review) 10 Jan '21

10 Jan '21

Attention is currently required from: Xiang Wang, Stefan Reinauer. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49254 ) Change subject: sysfsgpio.c implement spi interface via linux sysfs ...................................................................... Patch Set 4: (11 comments) File Makefile: https://review.coreboot.org/c/flashrom/+/49254/comment/28cdba27_32c512a3 PS4, Line 800: : CONFIG_SYSFSGPIO ?= no nit: move this above CONFIG_PRINT_WIKI File meson.build: https://review.coreboot.org/c/flashrom/+/49254/comment/24cdf167_a44fa065 PS4, Line 71: config_sysfsgpio We don't use this anywhere. https://review.coreboot.org/c/flashrom/+/49254/comment/e0a67676_7ec55fa4 PS4, Line 313: if host_machine.system() == 'linux' Maybe it should be used here. Also, this depends on `config_bitbang_spi`. I have no idea how to tell meson about this dependency. File sysfsgpio.c: https://review.coreboot.org/c/flashrom/+/49254/comment/23e9d3ae_915ea889 PS1, Line 80: if (ret == (int)strlen(str)) > I tried it, and the compilation still reports an error Meh, silly compilers https://review.coreboot.org/c/flashrom/+/49254/comment/15d66c8f_ebc8f6b2 PS1, Line 209: /* parameter format: pins=cs_pin:sck_pin:mosi_pin:miso_pin */ : char *pins = extract_programmer_param("pins"); : int pins_inited = 0; : do { : struct pin_desc *pins_tab[] = { : &pin_cs, &pin_sck, &pin_mosi, &pin_miso : }; : if (!(pins && strlen(pins))) : break; : char *token = strtok(pins, ":"); : for (unsigned i = 0; i < ARRAY_SIZE(pins_tab); i++) { : long v; : if (!token) : break; : if (atoi_s(token, 1, &v)) : break; : pins_tab[i]->pin = v; : if (export_sysfsgpio(pins_tab[i])) : break; : token = strtok(NULL, ":"); : pins_inited = (i + 1 == ARRAY_SIZE(pins_tab)); : } : } while (0); : if (pins) : free(pins); : if (!pins_inited) : return 1; > Done Yeah, it would be great to have all functions be pure, but the ones in `bitbang_spi_master` are an exception 😞 File sysfsgpio.c: https://review.coreboot.org/c/flashrom/+/49254/comment/d2567f53_2763e03a PS4, Line 83: if (ret == (int)strlen(str)) Ah, I just thought of another way to avoid this cast: /* Use an int to avoid signed-unsigned comparisons */ const int len = strlen(str); int ret = write(fd, str, len); close(fd); if (ret == len) return 0; This saves having to call `strlen` twice. This could have problems with extremely long strings, but it doesn't really matter since it's a static function with controlled inputs. https://review.coreboot.org/c/flashrom/+/49254/comment/70ff1ab3_9f71c10e PS4, Line 93: snprintf(s, sizeof(s), "/sys/class/gpio/gpio%s/", desc->pin); Hmmm... I think you should be able to define the common prefix in a macro and do the following: #define SYSFS_GPIO "/sys/class/gpio/" ... snprintf(s, sizeof(s), SYSFS_GPIO "gpio%s/", desc->pin); https://review.coreboot.org/c/flashrom/+/49254/comment/fb442c6f_8fb65560 PS4, Line 99: snprintf(s, sizeof(s), "/sys/class/gpio/gpio%s/direction", desc->pin); : desc->fd_direction = open(s, O_WRONLY); : if (desc->fd_direction < 0) { : msg_perr("Error: failed to open %s\n", s); : return -1; : } : : snprintf(s, sizeof(s), "/sys/class/gpio/gpio%s/value", desc->pin); : desc->fd_value = open(s, O_RDONLY); : if (desc->fd_value < 0) { : msg_perr("Error: failed to open %s\n", s); : return -1; : } Here, you could do the following instead: const size_t n = strlen(s); if (n + 10 >= sizeof(s)) return -1; /* Or use malloc() to allocate a large enough buffer */ strcpy(&s[n], "direction"); desc->fd_direction = open(s, O_WRONLY); if (desc->fd_direction < 0) { msg_perr("Error: failed to open %s\n", s); return -1; } strcpy(&s[n], "value"); desc->fd_value = open(s, O_RDONLY); if (desc->fd_value < 0) { msg_perr("Error: failed to open %s\n", s); return -1; } Yes, with `strcpy()`. As long as you know (i.e. include code to test!) that there's enough space in the destination buffer, it is safe to use. People think `strncpy()` is safer because it takes a length parameter, but it's just another place for a mistake to hide. `strncpy()` does not produce a null-terminated string when there's no terminating null byte in the first `n` bytes of `src`, and this is the case when `n = strlen(src)`. https://review.coreboot.org/c/flashrom/+/49254/comment/42c0d2f6_3e3d59cf PS4, Line 135: desc->fd_direction = -1; Also invalidate the pin string: strncpy(desc->pin, "", sizeof(desc->pin)); Here, `strncpy()` makes sense because we want to fill the entire buffer with null bytes. https://review.coreboot.org/c/flashrom/+/49254/comment/af5f2c62_0de1f616 PS4, Line 179: .get_miso = sysfsgpio_get_miso, nit: I would align the equals signs to the same column: .set_cs = sysfsgpio_set_cs, .set_sck = sysfsgpio_set_sck, .set_mosi = sysfsgpio_set_mosi, .get_miso = sysfsgpio_get_miso, https://review.coreboot.org/c/flashrom/+/49254/comment/5b909ee4_5c50f309 PS4, Line 204: static int parse_pins(struct sysfsgpio_pins *pins) I had eight comments on this function, but it was very hard to understand what I wanted you to do. Instead, here's how I would rewrite this function: static int parse_pins(struct sysfsgpio_pins *pins) { char *const arg = extract_programmer_param("pins"); if (!arg) return 1; struct pin_desc *const pins_tab[] = { &pins->cs, &pins->sck, &pins->mosi, &pins->miso, }; char *token = strtok(arg, ":"); unsigned int i; for (i = 0; token && i < ARRAY_SIZE(pins_tab); i++) { char *suffix; const long v = strtol(token, &suffix, 10); if (errno != 0 || token == suffix) break; if (v < 0 || v > 9999999) break; snprintf(pins_tab[i]->pin, sizeof(pins_tab[i]->pin), "%ld", v); if (export_sysfsgpio(pins_tab[i])) break; token = strtok(NULL, ":"); } free(arg); return i != ARRAY_SIZE(pins_tab); } -- To view, visit https://review.coreboot.org/c/flashrom/+/49254 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1bc47ef8011bba560326b501a80869340bb9f733 Gerrit-Change-Number: 49254 Gerrit-PatchSet: 4 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: Angel Pons <th3fanbus(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: Sun, 10 Jan 2021 11:08:03 +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

Change in flashrom[master]: bitbang-spi.c: support clock polarity and phase
by Xiang Wang (Code Review) 10 Jan '21

10 Jan '21

Attention is currently required from: Stefan Reinauer, Edward O'Callaghan, Angel Pons. Xiang Wang has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49255 ) Change subject: bitbang-spi.c: support clock polarity and phase ...................................................................... Patch Set 14: (2 comments) File bitbang_spi.c: https://review.coreboot.org/c/flashrom/+/49255/comment/10b9876e_6a00c602 PS12, Line 208: : if (parse_spi_mode(&cpol, &cpha)) : return ERROR_FLASHROM_BUG; > Where are they initialised to 0? […] Global static variables are initialized to 0 by default File bitbang_spi.c: https://review.coreboot.org/c/flashrom/+/49255/comment/a5d19e97_9c22baaa PS14, Line 41: : static int cpol; /* Clock Polarity */ : static int cpha; /* Clock Phase */ > sounds like you want, […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/49255 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I04c1dfe132d756119229b27c3cd611d1be1abc8d Gerrit-Change-Number: 49255 Gerrit-PatchSet: 14 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: Shawn C <citypw(a)hardenedlinux.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sun, 10 Jan 2021 08:01:50 +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> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: bitbang-spi.c: support clock polarity and phase
by Edward O'Callaghan (Code Review) 10 Jan '21

10 Jan '21

Attention is currently required from: Xiang Wang, Stefan Reinauer, Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49255 ) Change subject: bitbang-spi.c: support clock polarity and phase ...................................................................... Patch Set 14: (2 comments) File bitbang_spi.c: https://review.coreboot.org/c/flashrom/+/49255/comment/5eda1d7a_c06cf981 PS12, Line 208: : if (parse_spi_mode(&cpol, &cpha)) : return ERROR_FLASHROM_BUG; > If the parameter mode is not set, parse_spi_mode returns 0. […] Where are they initialised to 0? Also you may as well update the man page in this commit then for completeness. File bitbang_spi.c: https://review.coreboot.org/c/flashrom/+/49255/comment/e5c5dd6e_877c30ae PS14, Line 41: : static int cpol; /* Clock Polarity */ : static int cpha; /* Clock Phase */ sounds like you want, ``` static int cpol = 0; /* Clock Polarity */ static int cpha = 0; /* Clock Phase */ ``` -- To view, visit https://review.coreboot.org/c/flashrom/+/49255 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I04c1dfe132d756119229b27c3cd611d1be1abc8d Gerrit-Change-Number: 49255 Gerrit-PatchSet: 14 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: Shawn C <citypw(a)hardenedlinux.org> Gerrit-Attention: Xiang Wang <merle(a)hardenedlinux.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sun, 10 Jan 2021 07:54:56 +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> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: bitbang_spi.c: fix bitbang_spi_send_command to make the delay symmetric
by Xiang Wang (Code Review) 10 Jan '21

10 Jan '21

Attention is currently required from: Xiang Wang, Stefan Reinauer, Angel Pons. Hello build bot (Jenkins), Stefan Reinauer, Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/49262 to look at the new patch set (#9). Change subject: bitbang_spi.c: fix bitbang_spi_send_command to make the delay symmetric ...................................................................... bitbang_spi.c: fix bitbang_spi_send_command to make the delay symmetric This patch is used to make the following delays as equal as possible: delay from getting the bus to enabling the chip delay from disabling the chip to releasing the bus delay from enabling the chip to the start of communication delay from ending communication to the disabling of the chip Change-Id: Icf8b65aed20244ccffc231da2c599c6ca1796104 Signed-off-by: Xiang Wang <merle(a)hardenedlinux.org> --- M bitbang_spi.c 1 file changed, 3 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/62/49262/9 -- To view, visit https://review.coreboot.org/c/flashrom/+/49262 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Icf8b65aed20244ccffc231da2c599c6ca1796104 Gerrit-Change-Number: 49262 Gerrit-PatchSet: 9 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: Shawn C <citypw(a)hardenedlinux.org> Gerrit-Attention: Xiang Wang <merle(a)hardenedlinux.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: bitbang-spi.c: support clock polarity and phase
by Xiang Wang (Code Review) 10 Jan '21

10 Jan '21

Attention is currently required from: Stefan Reinauer, Edward O'Callaghan, Angel Pons. Hello build bot (Jenkins), Stefan Reinauer, Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/49255 to look at the new patch set (#14). Change subject: bitbang-spi.c: support clock polarity and phase ...................................................................... bitbang-spi.c: support clock polarity and phase SPI has four modes, which are determined by Clock polarity and Clock phase. See https://en.wikipedia.org/wiki/Serial_Peripheral_Interface#Clock_polarity_an… for details. The previous code only supports one mode and may not be able to handle some special spi flash. This patch uses spi's mode0 by default to be consistent with the previous code. TEST=build and run flashrom with sysfsgpio on raspberrypi to read W25Q128.V Change-Id: I04c1dfe132d756119229b27c3cd611d1be1abc8d Signed-off-by: Xiang Wang <merle(a)hardenedlinux.org> --- M bitbang_spi.c 1 file changed, 70 insertions(+), 16 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/55/49255/14 -- To view, visit https://review.coreboot.org/c/flashrom/+/49255 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I04c1dfe132d756119229b27c3cd611d1be1abc8d Gerrit-Change-Number: 49255 Gerrit-PatchSet: 14 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: Shawn C <citypw(a)hardenedlinux.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: bitbang-spi.c: support clock polarity and phase
by Xiang Wang (Code Review) 10 Jan '21

10 Jan '21

Attention is currently required from: Stefan Reinauer, Edward O'Callaghan, Angel Pons. Xiang Wang has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49255 ) Change subject: bitbang-spi.c: support clock polarity and phase ...................................................................... Patch Set 13: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/49255/comment/cbaa3c9b_184bb89f PS12, Line 12: . > Also state the previous mode here that is currently assumed. Done File bitbang_spi.c: https://review.coreboot.org/c/flashrom/+/49255/comment/0699be98_26d81ce2 PS12, Line 208: : if (parse_spi_mode(&cpol, &cpha)) : return ERROR_FLASHROM_BUG; > Hmm, the only thing is this could be a little too harsh for this patch in that, could we just assume […] If the parameter mode is not set, parse_spi_mode returns 0. cpol/cpha will default to 0 and use spi's mode0. Only when the user specifies the mode parameter and the value of mode is not 0-3 will an error occur -- To view, visit https://review.coreboot.org/c/flashrom/+/49255 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I04c1dfe132d756119229b27c3cd611d1be1abc8d Gerrit-Change-Number: 49255 Gerrit-PatchSet: 13 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: Shawn C <citypw(a)hardenedlinux.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sun, 10 Jan 2021 07:43:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit January 2021