April 2021 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: lspcon_i2c_spi.c: Rename PAGE_SIZE macro
by Edward O'Callaghan (Code Review) 19 Apr '21

19 Apr '21

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/52469 ) Change subject: lspcon_i2c_spi.c: Rename PAGE_SIZE macro ...................................................................... lspcon_i2c_spi.c: Rename PAGE_SIZE macro This fixes building with musl libc on alpine:i386-v3.7. Change-Id: Ibd478f3fcd6b6803098035dbdc47f72f6ab3fa6b Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/52469 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Nico Huber <nico.h(a)gmx.de> --- M lspcon_i2c_spi.c 1 file changed, 8 insertions(+), 8 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved diff --git a/lspcon_i2c_spi.c b/lspcon_i2c_spi.c index 7b9f1c0..7599db6 100644 --- a/lspcon_i2c_spi.c +++ b/lspcon_i2c_spi.c @@ -27,7 +27,7 @@ #define REGISTER_ADDRESS (0x94 >> 1) #define PAGE_ADDRESS (0x9e >> 1) -#define PAGE_SIZE 256 +#define LSPCON_PAGE_SIZE 256 #define MAX_SPI_WAIT_RETRIES 1000 #define CLT2_SPI 0x82 @@ -330,7 +330,7 @@ static int lspcon_i2c_spi_map_page(int fd, unsigned int offset) { int ret = 0; - /* Page number byte, need to / PAGE_SIZE. */ + /* Page number byte, need to / LSPCON_PAGE_SIZE. */ ret |= lspcon_i2c_spi_write_register(fd, ROMADDR_BYTE1, (offset >> 8) & 0xff); ret |= lspcon_i2c_spi_write_register(fd, ROMADDR_BYTE2, (offset >> 16)); @@ -349,9 +349,9 @@ if (fd < 0) return SPI_GENERIC_ERROR; - for (i = 0; i < len; i += PAGE_SIZE) { + for (i = 0; i < len; i += LSPCON_PAGE_SIZE) { ret |= lspcon_i2c_spi_map_page(fd, start + i); - ret |= lspcon_i2c_spi_read_data(fd, PAGE_ADDRESS, buf + i, min(len - i, PAGE_SIZE)); + ret |= lspcon_i2c_spi_read_data(fd, PAGE_ADDRESS, buf + i, min(len - i, LSPCON_PAGE_SIZE)); } return ret; @@ -363,8 +363,8 @@ * Using static buffer with maximum possible size, * extra byte is needed for prefixing zero at index 0. */ - uint8_t write_buffer[PAGE_SIZE + 1] = { 0 }; - if (len > PAGE_SIZE) + uint8_t write_buffer[LSPCON_PAGE_SIZE + 1] = { 0 }; + if (len > LSPCON_PAGE_SIZE) return SPI_GENERIC_ERROR; /* First byte represents the writing offset and should always be zero. */ @@ -389,9 +389,9 @@ ret |= lspcon_i2c_spi_enable_hw_write(fd); ret |= lspcon_i2c_clt2_spi_reset(fd); - for (unsigned int i = 0; i < len; i += PAGE_SIZE) { + for (unsigned int i = 0; i < len; i += LSPCON_PAGE_SIZE) { ret |= lspcon_i2c_spi_map_page(fd, start + i); - ret |= lspcon_i2c_spi_write_page(fd, buf + i, min(len - i, PAGE_SIZE)); + ret |= lspcon_i2c_spi_write_page(fd, buf + i, min(len - i, LSPCON_PAGE_SIZE)); } ret |= lspcon_i2c_spi_enable_write_protection(fd); -- To view, visit https://review.coreboot.org/c/flashrom/+/52469 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ibd478f3fcd6b6803098035dbdc47f72f6ab3fa6b Gerrit-Change-Number: 52469 Gerrit-PatchSet: 2 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: linux_spi.c: Resolve comments left from previous patches
by Edward O'Callaghan (Code Review) 19 Apr '21

19 Apr '21

Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52492 ) Change subject: linux_spi.c: Resolve comments left from previous patches ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/52492 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I27a718b515fc474f63b3e61be58a6f9302527559 Gerrit-Change-Number: 52492 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-CC: Sam McNally <sammc(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 19 Apr 2021 05:35:16 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom.c: allow - as filename for stdin/stdout
by Edward O'Callaghan (Code Review) 19 Apr '21

19 Apr '21

Attention is currently required from: Nico Huber, Angel Pons, Daniel Campello. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52383 ) Change subject: flashrom.c: allow - as filename for stdin/stdout ...................................................................... Patch Set 11: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/52383/comment/894cab58_351bebce PS11, Line 9: Allows - as filename for -r/-w/-v options Do add a rational here, perhaps something like - "It is sometimes useful to script flashrom and allowing it to work with pipes allows for more flexibility in this specific use-case." I guess would be something reasonable to say? -- To view, visit https://review.coreboot.org/c/flashrom/+/52383 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I97889cfdf7ba9a257e182c4ee2b20075cfa58d4d Gerrit-Change-Number: 52383 Gerrit-PatchSet: 11 Gerrit-Owner: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Jack Rosenthal <jrosenth(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Daniel Campello <campello(a)chromium.org> Gerrit-Comment-Date: Mon, 19 Apr 2021 05:29:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: cli_classic.c: add -x option for do_extract()
by Edward O'Callaghan (Code Review) 19 Apr '21

19 Apr '21

Attention is currently required from: Sam McNally, Daniel Campello. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52450 ) Change subject: cli_classic.c: add -x option for do_extract() ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/52450 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8c69223fa92cf5b50abe070f1ab9f19d3b42f6ff Gerrit-Change-Number: 52450 Gerrit-PatchSet: 3 Gerrit-Owner: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Sam McNally <sammc(a)google.com> Gerrit-Attention: Daniel Campello <campello(a)chromium.org> Gerrit-Comment-Date: Mon, 19 Apr 2021 05:27:09 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ft2232_spi.c: Pack WREN and op in one ftdi_write_data() call
by Anastasia Klimchuk (Code Review) 19 Apr '21

19 Apr '21

Attention is currently required from: Simon Buhrow, Alan Green, Nico Huber, Edward O'Callaghan, Samir Ibradžić. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/40477 ) Change subject: ft2232_spi.c: Pack WREN and op in one ftdi_write_data() call ...................................................................... Patch Set 25: (1 comment) File ft2232_spi.c: https://review.coreboot.org/c/flashrom/+/40477/comment/a2228e8c_5f439590 PS25, Line 295: 9 > I see 6 potential paths, due to the 3 if's but the second may end […] oh yes... I made a mistake, I didn't notice continue statement. I agree, if de-assert CS# is always the last thing to do, it would be ideal to have it last in the code as well. -- To view, visit https://review.coreboot.org/c/flashrom/+/40477 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie4a07499ec5ef0af23818593f45dc427285a9e8a Gerrit-Change-Number: 40477 Gerrit-PatchSet: 25 Gerrit-Owner: Simon Buhrow Gerrit-Reviewer: Alan Green <avg(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Samir Ibradžić <sibradzic(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Simon Buhrow Gerrit-Attention: Alan Green <avg(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Samir Ibradžić <sibradzic(a)gmail.com> Gerrit-Comment-Date: Mon, 19 Apr 2021 03:26:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom.c: automatic generated programmer_enum.h
by Xiang Wang (Code Review) 19 Apr '21

19 Apr '21

Attention is currently required from: Sam McNally, Nico Huber, Stefan Reinauer. Xiang Wang has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49599 ) Change subject: flashrom.c: automatic generated programmer_enum.h ...................................................................... Patch Set 6: (1 comment) File Makefile: https://review.coreboot.org/c/flashrom/+/49599/comment/018cc561_7299734d PS4, Line 681: $(shell bash ./util/generator_programmer_enum.sh) > Sorry for the delay. I don't have much time and some effort the rework […] Thank you. I know these, but this modification will be inconsistent with meson. In fact, regenerating every time has little effect on compilation speed. -- 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: 6 Gerrit-Owner: Xiang Wang <wxjstz(a)126.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Idwer Vollering <vidwer(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Shawn C <citypw(a)hardenedlinux.org> Gerrit-Attention: Sam McNally <sammc(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Comment-Date: Mon, 19 Apr 2021 02:38:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Xiang Wang <wxjstz(a)126.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: linux_spi.c: Refactor singleton states into reentrant pattern
by Anastasia Klimchuk (Code Review) 19 Apr '21

19 Apr '21

Attention is currently required from: Nico Huber. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52285 ) Change subject: linux_spi.c: Refactor singleton states into reentrant pattern ...................................................................... Patch Set 3: (2 comments) Patchset: PS3: Thanks for comments! I have created another patch https://review.coreboot.org/c/flashrom/+/52492 to fix all of them (since this one is already merged and I can’t change it). I also “replied” to the comments in the new patch, if anymore changes needed I will do everything in 52492. File linux_spi.c: https://review.coreboot.org/c/flashrom/+/52285/comment/c99fd9f5_bda01253 PS3, Line 111: const > Why drop it? […] Yeah it's a step that I need to do on the way to new API, so it is not entirely reentrant at the moment - but will be! -- To view, visit https://review.coreboot.org/c/flashrom/+/52285 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I93408c2ca846fca6a1c7eda7180862c51bd48078 Gerrit-Change-Number: 52285 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> 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: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-CC: Sam McNally <sammc(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Mon, 19 Apr 2021 01:57:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: linux_spi.c: Separate shutdown from failed init cleanup
by Anastasia Klimchuk (Code Review) 19 Apr '21

19 Apr '21

Attention is currently required from: Nico Huber. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52284 ) Change subject: linux_spi.c: Separate shutdown from failed init cleanup ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: Thanks for comments! I have created another patch https://review.coreboot.org/c/flashrom/+/52492 to fix all of them (since this one is already merged and I can’t change it). I also “replied” to the comments in the new patch, if anymore changes needed I will do everything in 52492. -- To view, visit https://review.coreboot.org/c/flashrom/+/52284 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1c8da2878cd0e85a1e43ba9b4b8e6f3d9f38ae5c Gerrit-Change-Number: 52284 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> 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: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-CC: Sam McNally <sammc(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Mon, 19 Apr 2021 01:56:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: linux_spi.c: Extract get_max_kernel_buf_size() as a function
by Anastasia Klimchuk (Code Review) 19 Apr '21

19 Apr '21

Attention is currently required from: Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52283 ) Change subject: linux_spi.c: Extract get_max_kernel_buf_size() as a function ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: Thanks for comments! I have created another patch https://review.coreboot.org/c/flashrom/+/52492 to fix all of them (since this one is already merged and I can’t change it). I also “replied” to the comments in the new patch, if anymore changes needed I will do everything in 52492. -- To view, visit https://review.coreboot.org/c/flashrom/+/52283 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4b8c5775fb8f4b0dff702fcc0fb258221254c659 Gerrit-Change-Number: 52283 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> 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: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-CC: Sam McNally <sammc(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 19 Apr 2021 01:55:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: linux_spi.c: Resolve comments left from previous patches
by Anastasia Klimchuk (Code Review) 19 Apr '21

19 Apr '21

Attention is currently required from: Nico Huber, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52492 ) Change subject: linux_spi.c: Resolve comments left from previous patches ...................................................................... Patch Set 1: (4 comments) File linux_spi.c: https://review.coreboot.org/c/flashrom/+/52492/comment/ae8258a7_8122f9a5 PS1, Line 71: struct linux_spi_data *spi_data = data; Same thing may be in other shutdown functions, but since I will be going through all them soon, will try to fix where I can. https://review.coreboot.org/c/flashrom/+/52492/comment/564ada64_26c1f5d1 PS1, Line 176: int fd; First time this variable is used, it is immediately assigned as open(dev, ...) line 204, so default value is not needed. And since it is local, it will be created again every time init runs. https://review.coreboot.org/c/flashrom/+/52492/comment/98ef81cf_8d660829 PS1, Line 208: return 1; For this error path and all the above, close(fd) is not needed, because fd hasn't been opened yet. All the error paths below this one should goto init_err because it does cleanup i.e. close(fd). https://review.coreboot.org/c/flashrom/+/52492/comment/6b1706a3_159a37ef PS1, Line 252: return 1; Since SPI_GENERIC_ERROR is not used anymore, ret variable is not needed because init error always returns 1. -- To view, visit https://review.coreboot.org/c/flashrom/+/52492 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I27a718b515fc474f63b3e61be58a6f9302527559 Gerrit-Change-Number: 52492 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-CC: Sam McNally <sammc(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 19 Apr 2021 01:54:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit April 2021