- flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: Add mediatek_i2c_spi interface
by Neill Corlett (Code Review) 01 Feb '22

01 Feb '22

Attention is currently required from: Nico Huber, Angel Pons. Neill Corlett has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61288 ) Change subject: Add mediatek_i2c_spi interface ...................................................................... Patch Set 3: (4 comments) Patchset: PS1: > Thank you very much! it helps a lot to know what we are dealing with here. […] Will address in a follow-up change as needed File mediatek_i2c_spi.c: https://review.coreboot.org/c/flashrom/+/61288/comment/337dcca9_c0fcce64 PS1, Line 242: GPIO line > At the very least, I'd add an option to enable WP# control. Will address in a follow-up change as needed https://review.coreboot.org/c/flashrom/+/61288/comment/ae9bb281_f759f7a9 PS1, Line 249: 0x426, 7 > Having seen these kinds of datasheets before I know what you mean Neill lol. […] Ack https://review.coreboot.org/c/flashrom/+/61288/comment/2896e5f3_ea71de24 PS1, Line 255: 0x428, 7 > What are these magic numbers? See line #249 -- To view, visit https://review.coreboot.org/c/flashrom/+/61288 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I24adb14e7b4f7160e1c3ff941774064d5a81e820 Gerrit-Change-Number: 61288 Gerrit-PatchSet: 3 Gerrit-Owner: Neill Corlett <corlett(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: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 31 Jan 2022 17:48:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Neill Corlett <corlett(a)google.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: libflashrom,writeprotect: add functions for reading/writing WP settings
by Nikolai Artemiev (Code Review) 31 Jan '22

31 Jan '22

Attention is currently required from: Nico Huber, Edward O'Callaghan, Anastasia Klimchuk, Sergii Dmytruk. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58479 ) Change subject: libflashrom,writeprotect: add functions for reading/writing WP settings ...................................................................... Patch Set 28: (1 comment) File writeprotect.c: https://review.coreboot.org/c/flashrom/+/58479/comment/0893c62a_71d034a8 PS22, Line 56: struct wp_chip_config *cfg > Done Undoing done -- To view, visit https://review.coreboot.org/c/flashrom/+/58479 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3ad25708c3321b8fb0216c3eaf6ffc07616537ad Gerrit-Change-Number: 58479 Gerrit-PatchSet: 28 Gerrit-Owner: Nikolai Artemiev <nartemiev(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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Mon, 31 Jan 2022 06:16:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: libflashrom,writeprotect: add functions for reading/writing WP settings
by Nikolai Artemiev (Code Review) 31 Jan '22

31 Jan '22

Attention is currently required from: Nico Huber, Edward O'Callaghan, Anastasia Klimchuk, Sergii Dmytruk. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58479 ) Change subject: libflashrom,writeprotect: add functions for reading/writing WP settings ...................................................................... Patch Set 28: (5 comments) Patchset: PS22: > Sorry for the silence. I think you made good progress on your own! The abstract […] That sounds good. I'm working on implementing it now. File libflashrom.h: https://review.coreboot.org/c/flashrom/+/58479/comment/29939841_b3c4fc52 PS27, Line 133: }; > Having a struct in the API means we can't ever change the struct without […] That sounds good to me, it's good to address ABI stability problems early on. I've been going through and updating the patches, one thing I'm not sure about is how the `flashrom_wp_get_ranges()` function (CB:58481) should be handled. It currently allocates an output buffer of `struct wp_range` elements, but that has the same ABI problem. We could have something like a ```flashrom_wp_get_ranges(size_t **starts, size_t **lengths, size_t *counts, flashrom_flashctx *)``` function, though it's a bit messy compared to just returning an array of structures. Alternatively we could have an opaque wp_range_collection or something like that and an accessor function: ``` get_wp_range_from_collection(struct wp_range_collection *, size_t index) ``` https://review.coreboot.org/c/flashrom/+/58479/comment/689828b6_cbf40989 PS27, Line 138: #define FLASHROM_WP_ERR_VERIFY_FAILED 4 > This could be an enum, maybe even without specific numbers? I'm open to using an enum, though we would probably want to explicitly assign values to ensure API stability right? File libflashrom.h: https://review.coreboot.org/c/flashrom/+/58479/comment/4df583fb_daaf2d28 PS22, Line 125: software > If it's software protection, why not name it like that? I've removed the comment now, mentioning software protection is confusing/inaccurate since software protection may not be active and can easily be disabled if it is. File writeprotect.c: https://review.coreboot.org/c/flashrom/+/58479/comment/233d2333_f8e81846 PS22, Line 56: struct wp_chip_config *cfg > Only realized this now: this requires the caller to know the size of […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/58479 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3ad25708c3321b8fb0216c3eaf6ffc07616537ad Gerrit-Change-Number: 58479 Gerrit-PatchSet: 28 Gerrit-Owner: Nikolai Artemiev <nartemiev(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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Mon, 31 Jan 2022 06:13:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Add mediatek_i2c_spi interface
by Nico Huber (Code Review) 27 Jan '22

27 Jan '22

Attention is currently required from: Angel Pons, Neill Corlett. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61288 ) Change subject: Add mediatek_i2c_spi interface ...................................................................... Patch Set 2: (1 comment) Patchset: PS1: > > Ah, hmmm, I'm not sure but I think there are SMBus controller drivers that could […] Thank you very much! it helps a lot to know what we are dealing with here. I think the cannonical flashrom way would be to add this as part of the `internal` programmer which handles on-board flashing. We have all the infrastructure there to detect a specific board, handle what is necessary to enable writes and eventually decide what driver will be used. Board detection and lifting write-protection is usually done in `board_enable.c`. From there one can call the initialization of a driver like `mstarddc_spi` (in this case with a hint if I2C or SMBus transfers should be used?). -- To view, visit https://review.coreboot.org/c/flashrom/+/61288 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I24adb14e7b4f7160e1c3ff941774064d5a81e820 Gerrit-Change-Number: 61288 Gerrit-PatchSet: 2 Gerrit-Owner: Neill Corlett <corlett(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: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Neill Corlett <corlett(a)google.com> Gerrit-Comment-Date: Thu, 27 Jan 2022 13:30:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Neill Corlett <corlett(a)google.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Add mediatek_i2c_spi interface
by Edward O'Callaghan (Code Review) 27 Jan '22

27 Jan '22

Attention is currently required from: Nico Huber, Angel Pons, Neill Corlett. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61288 ) Change subject: Add mediatek_i2c_spi interface ...................................................................... Patch Set 2: Code-Review+2 (2 comments) File mediatek_i2c_spi.c: https://review.coreboot.org/c/flashrom/+/61288/comment/a21a32e8_cbb2eac3 PS1, Line 96: args.data->block > Inclined to leave as-is unless there are strong objections. Ack. Angel is right, I like avoiding such things but Nico's point is fair style. https://review.coreboot.org/c/flashrom/+/61288/comment/8e985e3b_3f509e36 PS1, Line 249: 0x426, 7 > My understanding is that this is the usual, expected, line that's supposed to go to WP#. […] Having seen these kinds of datasheets before I know what you mean Neill lol. I think we may never know the precise meanings here. -- To view, visit https://review.coreboot.org/c/flashrom/+/61288 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I24adb14e7b4f7160e1c3ff941774064d5a81e820 Gerrit-Change-Number: 61288 Gerrit-PatchSet: 2 Gerrit-Owner: Neill Corlett <corlett(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: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Neill Corlett <corlett(a)google.com> Gerrit-Comment-Date: Thu, 27 Jan 2022 12:43:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Neill Corlett <corlett(a)google.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom.c: Move do_*() helpers into cli_classic.c
by Edward O'Callaghan (Code Review) 27 Jan '22

27 Jan '22

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/60070 ) Change subject: flashrom.c: Move do_*() helpers into cli_classic.c ...................................................................... flashrom.c: Move do_*() helpers into cli_classic.c These helpers are only used by the CLI logic and so we localise them here to move towards cli_classic being a pure libflashrom user. BUG=b:208132085 TEST=`make` Change-Id: If1112155e2421e0178fd73f847cbb80868387433 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/60070 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M cli_classic.c M flash.h M flashrom.c 3 files changed, 97 insertions(+), 101 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved diff --git a/cli_classic.c b/cli_classic.c index 3cd78d4..8c337eb 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -154,6 +154,103 @@ return 0; } +static int do_read(struct flashctx *const flash, const char *const filename) +{ + int ret; + + unsigned long size = flash->chip->total_size * 1024; + unsigned char *buf = calloc(size, sizeof(unsigned char)); + if (!buf) { + msg_gerr("Memory allocation failed!\n"); + return 1; + } + + ret = flashrom_image_read(flash, buf, size); + if (ret > 0) + goto free_out; + + if (write_buf_to_include_args(flash, buf)) { + ret = 1; + goto free_out; + } + if (filename) + ret = write_buf_to_file(buf, size, filename); + +free_out: + free(buf); + return ret; +} + +static int do_extract(struct flashctx *const flash) +{ + prepare_layout_for_extraction(flash); + return do_read(flash, NULL); +} + +static int do_write(struct flashctx *const flash, const char *const filename, const char *const referencefile) +{ + const size_t flash_size = flash->chip->total_size * 1024; + int ret = 1; + + uint8_t *const newcontents = malloc(flash_size); + uint8_t *const refcontents = referencefile ? malloc(flash_size) : NULL; + + if (!newcontents || (referencefile && !refcontents)) { + msg_gerr("Out of memory!\n"); + goto _free_ret; + } + + /* Read '-w' argument first... */ + if (read_buf_from_file(newcontents, flash_size, filename)) + goto _free_ret; + /* + * ... then update newcontents with contents from files provided to '-i' + * args if needed. + */ + if (read_buf_from_include_args(flash, newcontents)) + goto _free_ret; + + if (referencefile) { + if (read_buf_from_file(refcontents, flash_size, referencefile)) + goto _free_ret; + } + + ret = flashrom_image_write(flash, newcontents, flash_size, refcontents); + +_free_ret: + free(refcontents); + free(newcontents); + return ret; +} + +static int do_verify(struct flashctx *const flash, const char *const filename) +{ + const size_t flash_size = flash->chip->total_size * 1024; + int ret = 1; + + uint8_t *const newcontents = malloc(flash_size); + if (!newcontents) { + msg_gerr("Out of memory!\n"); + goto _free_ret; + } + + /* Read '-v' argument first... */ + if (read_buf_from_file(newcontents, flash_size, filename)) + goto _free_ret; + /* + * ... then update newcontents with contents from files provided to '-i' + * args if needed. + */ + if (read_buf_from_include_args(flash, newcontents)) + goto _free_ret; + + ret = flashrom_image_verify(flash, newcontents, flash_size); + +_free_ret: + free(newcontents); + return ret; +} + int main(int argc, char *argv[]) { const struct flashchip *chip = NULL; diff --git a/flash.h b/flash.h index 932419e..09c1c85 100644 --- a/flash.h +++ b/flash.h @@ -356,10 +356,6 @@ int write_buf_to_include_args(const struct flashctx *const flash, unsigned char *buf); int prepare_flash_access(struct flashctx *, bool read_it, bool write_it, bool erase_it, bool verify_it); void finalize_flash_access(struct flashctx *); -int do_read(struct flashctx *, const char *filename); -int do_extract(struct flashctx *); -int do_write(struct flashctx *, const char *const filename, const char *const referencefile); -int do_verify(struct flashctx *, const char *const filename); int register_chip_restore(chip_restore_fn_cb_t func, struct flashctx *flash, uint8_t status); /* Something happened that shouldn't happen, but we can go on. */ diff --git a/flashrom.c b/flashrom.c index 8287c31..ac61259 100644 --- a/flashrom.c +++ b/flashrom.c @@ -2189,100 +2189,3 @@ } /** @} */ /* end flashrom-ops */ - -int do_read(struct flashctx *const flash, const char *const filename) -{ - int ret; - - unsigned long size = flash->chip->total_size * 1024; - unsigned char *buf = calloc(size, sizeof(unsigned char)); - if (!buf) { - msg_gerr("Memory allocation failed!\n"); - return 1; - } - - ret = flashrom_image_read(flash, buf, size); - if (ret > 0) - goto free_out; - - if (write_buf_to_include_args(flash, buf)) { - ret = 1; - goto free_out; - } - if (filename) - ret = write_buf_to_file(buf, size, filename); - -free_out: - free(buf); - return ret; -} - -int do_extract(struct flashctx *const flash) -{ - prepare_layout_for_extraction(flash); - return do_read(flash, NULL); -} - -int do_write(struct flashctx *const flash, const char *const filename, const char *const referencefile) -{ - const size_t flash_size = flash->chip->total_size * 1024; - int ret = 1; - - uint8_t *const newcontents = malloc(flash_size); - uint8_t *const refcontents = referencefile ? malloc(flash_size) : NULL; - - if (!newcontents || (referencefile && !refcontents)) { - msg_gerr("Out of memory!\n"); - goto _free_ret; - } - - /* Read '-w' argument first... */ - if (read_buf_from_file(newcontents, flash_size, filename)) - goto _free_ret; - /* - * ... then update newcontents with contents from files provided to '-i' - * args if needed. - */ - if (read_buf_from_include_args(flash, newcontents)) - goto _free_ret; - - if (referencefile) { - if (read_buf_from_file(refcontents, flash_size, referencefile)) - goto _free_ret; - } - - ret = flashrom_image_write(flash, newcontents, flash_size, refcontents); - -_free_ret: - free(refcontents); - free(newcontents); - return ret; -} - -int do_verify(struct flashctx *const flash, const char *const filename) -{ - const size_t flash_size = flash->chip->total_size * 1024; - int ret = 1; - - uint8_t *const newcontents = malloc(flash_size); - if (!newcontents) { - msg_gerr("Out of memory!\n"); - goto _free_ret; - } - - /* Read '-v' argument first... */ - if (read_buf_from_file(newcontents, flash_size, filename)) - goto _free_ret; - /* - * ... then update newcontents with contents from files provided to '-i' - * args if needed. - */ - if (read_buf_from_include_args(flash, newcontents)) - goto _free_ret; - - ret = flashrom_image_verify(flash, newcontents, flash_size); - -_free_ret: - free(newcontents); - return ret; -} -- To view, visit https://review.coreboot.org/c/flashrom/+/60070 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If1112155e2421e0178fd73f847cbb80868387433 Gerrit-Change-Number: 60070 Gerrit-PatchSet: 6 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: flashrom.c: Make {read,write}_buf_from_include_args() public
by Edward O'Callaghan (Code Review) 27 Jan '22

27 Jan '22

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/60069 ) Change subject: flashrom.c: Make {read,write}_buf_from_include_args() public ...................................................................... flashrom.c: Make {read,write}_buf_from_include_args() public This allows for moving all the do_*() helper functions use for the cli from flashrom.c within static local functions in cli_classic.c BUG=b:208132085 TEST=`make` Change-Id: Ia0abec655a682ca449d0e8ba620886a2d616b86d Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/60069 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M flash.h M flashrom.c 2 files changed, 4 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved diff --git a/flash.h b/flash.h index 11e6389..932419e 100644 --- a/flash.h +++ b/flash.h @@ -351,7 +351,9 @@ void list_programmers_linebreak(int startcol, int cols, int paren); int selfcheck(void); int read_buf_from_file(unsigned char *buf, unsigned long size, const char *filename); +int read_buf_from_include_args(const struct flashctx *const flash, unsigned char *buf); int write_buf_to_file(const unsigned char *buf, unsigned long size, const char *filename); +int write_buf_to_include_args(const struct flashctx *const flash, unsigned char *buf); int prepare_flash_access(struct flashctx *, bool read_it, bool write_it, bool erase_it, bool verify_it); void finalize_flash_access(struct flashctx *); int do_read(struct flashctx *, const char *filename); diff --git a/flashrom.c b/flashrom.c index 257112f..8287c31 100644 --- a/flashrom.c +++ b/flashrom.c @@ -931,7 +931,7 @@ * @param buf Chip-sized buffer to write data to * @return 0 on success */ -static int read_buf_from_include_args(const struct flashctx *const flash, +int read_buf_from_include_args(const struct flashctx *const flash, unsigned char *buf) { const struct flashrom_layout *const layout = get_layout(flash); @@ -1028,7 +1028,7 @@ * @param buf Chip-sized buffer to read data from * @return 0 on success */ -static int write_buf_to_include_args(const struct flashctx *const flash, +int write_buf_to_include_args(const struct flashctx *const flash, unsigned char *buf) { const struct flashrom_layout *const layout = get_layout(flash); -- To view, visit https://review.coreboot.org/c/flashrom/+/60069 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia0abec655a682ca449d0e8ba620886a2d616b86d Gerrit-Change-Number: 60069 Gerrit-PatchSet: 6 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org> 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: ichspi: Split very long init function into two
by Nico Huber (Code Review) 27 Jan '22

27 Jan '22

Attention is currently required from: Felix Singer, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Nikolai Artemiev, Nicholas Chin. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58735 ) Change subject: ichspi: Split very long init function into two ...................................................................... Patch Set 7: (1 comment) Patchset: PS7: > Now that this is tested I've found a ThinkPad T60 in one of my […] My memory tries to tell me something... back in 2012 we were first trying coreboot on a spare T60 with broken display. This might be the very first computer I've ever used flashrom on *and* the first I've flashed coreboot to!!! ^^ -- To view, visit https://review.coreboot.org/c/flashrom/+/58735 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6789bc456a4878e6555831ae0b80ecbdbf62938b Gerrit-Change-Number: 58735 Gerrit-PatchSet: 7 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Comment-Date: Wed, 26 Jan 2022 23:43:33 +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]: ichspi: Split very long init function into two
by Nico Huber (Code Review) 27 Jan '22

27 Jan '22

Attention is currently required from: Felix Singer, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Nikolai Artemiev, Nicholas Chin. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58735 ) Change subject: ichspi: Split very long init function into two ...................................................................... Patch Set 7: Code-Review+1 (2 comments) Patchset: PS7: Now that this is tested I've found a ThinkPad T60 in one of my laptop piles /o\ didn't try to boot it up yet, and I have no idea whose it is oO File ichspi.c: https://review.coreboot.org/c/flashrom/+/58735/comment/fb5f3205_2049dfa2 PS7, Line 1771: ich_generation = ich_gen; : ich_spibar = spibar; These two are globals and also affect the ICH7 paths. I guess we missed them because they are not used for ICH7 in the original init function but only later during runtime. I've also checked all other writes to globals on the common paths of the original init function. There is only `swseq_data`, which is not used for ICH7 (although I noticed that it could be used to avoid code duplication in some cases, but that's for another day). -- To view, visit https://review.coreboot.org/c/flashrom/+/58735 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6789bc456a4878e6555831ae0b80ecbdbf62938b Gerrit-Change-Number: 58735 Gerrit-PatchSet: 7 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Comment-Date: Wed, 26 Jan 2022 23:36:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Add mediatek_i2c_spi interface
by Neill Corlett (Code Review) 27 Jan '22

27 Jan '22

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. Neill Corlett has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61288 ) Change subject: Add mediatek_i2c_spi interface ...................................................................... Patch Set 2: (3 comments) Patchset: PS1: > Ah, hmmm, I'm not sure but I think there are SMBus controller drivers that could > still advertise I2C_FUNC_I2C. Can you please confirm that this is not the case. > Or can you tell us what controller is used specifically? > > Also, is the chip aware if it is connected over SMBus or I2C, e.g. by a pin strap? I will research this and get back to you. If necessary, we can remove the SMBus code path from mainline, if that would simplify things. Only very early prototype hardware needs it. > Who is PanVision in this story? They are a vendor who has provided us with the scaler firmware running on the MST chip. > Also, where do you use these chips, integrated display or something external? > If you used SMBus, is it even on the mainboard? This is the scaler chip for the integrated display on the Google Meet Series One Desk 27. It's attached to I2C on the main x86_64 application CPU. File mediatek_i2c_spi.c: https://review.coreboot.org/c/flashrom/+/61288/comment/dbcfc9df_5a9edf76 PS1, Line 96: args.data->block > I would use `args.data->block + 1` when targeting an array and […] Inclined to leave as-is unless there are strong objections. https://review.coreboot.org/c/flashrom/+/61288/comment/1b0b2ff1_d36d7578 PS1, Line 249: 0x426, 7 > Hmmm, I see. […] My understanding is that this is the usual, expected, line that's supposed to go to WP#. In theory it could be wired up another way. -- To view, visit https://review.coreboot.org/c/flashrom/+/61288 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I24adb14e7b4f7160e1c3ff941774064d5a81e820 Gerrit-Change-Number: 61288 Gerrit-PatchSet: 2 Gerrit-Owner: Neill Corlett <corlett(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: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Wed, 26 Jan 2022 20:33:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Neill Corlett <corlett(a)google.com> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit