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

Change in flashrom[master]: dummyflasher.c: Don't leak `emu_persistent_image`
by Anastasia Klimchuk (Code Review) 26 May '21

26 May '21

Attention is currently required from: Nico Huber, Angel Pons, Arthur Heymans. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54905 ) Change subject: dummyflasher.c: Don't leak `emu_persistent_image` ...................................................................... Patch Set 1: (1 comment) File dummyflasher.c: https://review.coreboot.org/c/flashrom/+/54905/comment/6113fea7_bcb0ad82 PS1, Line 1026: free(data->emu_persistent_image); > > Is it ok to free something which is !(not) ? […] Oh thank you, super useful! So the code like `if (pointer) free(pointer)` is not needed, just `free(pointer)` is sufficient? Specifically, when I write new code? -- To view, visit https://review.coreboot.org/c/flashrom/+/54905 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I76529973cefcc6a1472681e1f4da8239fcbf07a6 Gerrit-Change-Number: 54905 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> 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-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: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Wed, 26 May 2021 22:04:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No 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: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: dummyflasher.c: Don't leak `emu_persistent_image`
by Angel Pons (Code Review) 26 May '21

26 May '21

Attention is currently required from: Nico Huber, Arthur Heymans, Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54905 ) Change subject: dummyflasher.c: Don't leak `emu_persistent_image` ...................................................................... Patch Set 1: (1 comment) File dummyflasher.c: https://review.coreboot.org/c/flashrom/+/54905/comment/564cc727_142daf52 PS1, Line 1026: free(data->emu_persistent_image); > Is it ok to free something which is !(not) ? Do you mean if it's okay to pass a null pointer to `free()`? Quoting the C standard, 7.20.3.2/2 from ISO-IEC 9899 [1]: > If ptr is a null pointer, no action occurs. So yes, it's a perfectly fine thing to do. [1]: http://www.open-std.org/JTC1/SC22/wg14/www/docs/n1124.pdf -- To view, visit https://review.coreboot.org/c/flashrom/+/54905 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I76529973cefcc6a1472681e1f4da8239fcbf07a6 Gerrit-Change-Number: 54905 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 26 May 2021 10:00:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Add AmigaOS suppport to flashrom
by Angel Pons (Code Review) 26 May '21

26 May '21

Attention is currently required from: Paul Menzel, Stefan Reinauer. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54943 ) Change subject: Add AmigaOS suppport to flashrom ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/54943/comment/3145b028_1c020388 PS1, Line 8: Also, I'd greatly appreciate if you could give a body to this commit message. For example, mention that this change introduces the `IS_AMIGA` macro to handle AmigaOS-specific quirks, and describe how this was tested. -- To view, visit https://review.coreboot.org/c/flashrom/+/54943 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3e4a3885f88313aad019454fdbdf4be176b35330 Gerrit-Change-Number: 54943 Gerrit-PatchSet: 1 Gerrit-Owner: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Comment-Date: Wed, 26 May 2021 09:13:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Add support for TerribleFire TF530 SPI controller
by Angel Pons (Code Review) 26 May '21

26 May '21

Attention is currently required from: Stefan Reinauer. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54944 ) Change subject: Add support for TerribleFire TF530 SPI controller ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/54944/comment/4ab3b33b_914a7f37 PS1, Line 8: I'd greatly appreciate if you could elaborate a bit about this programmer. Where can one find this SPI controller? Is it part of a chipset? Was this code tested? -- To view, visit https://review.coreboot.org/c/flashrom/+/54944 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I109eec4dec89650bb80d3d1da905d33f65f60f5a Gerrit-Change-Number: 54944 Gerrit-PatchSet: 1 Gerrit-Owner: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Comment-Date: Wed, 26 May 2021 09:06:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Add support for TerribleFire TF530 SPI controller
by Angel Pons (Code Review) 26 May '21

26 May '21

Attention is currently required from: Stefan Reinauer. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54944 ) Change subject: Add support for TerribleFire TF530 SPI controller ...................................................................... Patch Set 1: Code-Review+1 (11 comments) File tf530_spi.c: https://review.coreboot.org/c/flashrom/+/54944/comment/8bf26232_729fe20c PS1, Line 17: * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA I don't think we need this paragraph https://review.coreboot.org/c/flashrom/+/54944/comment/f761131f_87369065 PS1, Line 20: #if CONFIG_TF530_SPI == 1 Is this really necessary? I know other files have these guards, but I don't think they're actually useful. https://review.coreboot.org/c/flashrom/+/54944/comment/6e02616e_fa9d92d9 PS1, Line 62: static struct ConfigDev *cd = NULL; There's an ongoing effort to remove global state from programmers. See changes to `digilent_spi.c` in CB:54012 and CB:54067 as an example of what should be changed here. https://review.coreboot.org/c/flashrom/+/54944/comment/3af232c9_df3b790d PS1, Line 64: #define TF53x_CTRL_CS0 1 : #define TF53x_CTRL_CS1 2 : #define TF53x_CTRL_BUSY 4 : #define TF53x_CTRL_CS2 8 Looks like these are bits within a register. Why not rewrite them as shifts? #define TF53x_CTRL_CS0 (1 << 0) #define TF53x_CTRL_CS1 (1 << 1) #define TF53x_CTRL_BUSY (1 << 2) #define TF53x_CTRL_CS2 (1 << 3) https://review.coreboot.org/c/flashrom/+/54944/comment/1b4e7db9_bf3e29a2 PS1, Line 71: uint8_t busy = 0; : : while (busy == 0) { : busy = port->ctrl & TF53x_CTRL_BUSY; : } nit: I'd remove the `busy` variable, the name is confusing (it's active-low): do {} while ((port->ctrl & TF53x_CTRL_BUSY) == 0); Same below. https://review.coreboot.org/c/flashrom/+/54944/comment/793764a9_450ff517 PS1, Line 106: return 0; Don't we need to release `struct ConfigDev *cd` or similar? https://review.coreboot.org/c/flashrom/+/54944/comment/57caca48_0d5a32ba PS1, Line 119: unsigned int Support for for-loop initial declarations was introduced with C99. Some old compilers that do not use C99 mode by default may complain about it. It's not a big deal in this particular case because this programmer is tied to a particular OS, but it's something to keep in mind. https://review.coreboot.org/c/flashrom/+/54944/comment/4a47284a_29851863 PS1, Line 125: uint8_t volatile nit: `volatile uint8_t` Also, would this work? (volatile uint8_t)spi_recv_byte(port); https://review.coreboot.org/c/flashrom/+/54944/comment/b9a142c6_71e09fa3 PS1, Line 149: : struct Library *ExpansionBase = NULL; : : if ((ExpansionBase = (struct Library *) : OpenLibrary((unsigned char *)"expansion.library", 0L)) == NULL) { Moving the assignment out of the if-block's condition should be easier to read: struct Library *ExpansionBase = (struct Library *) OpenLibrary((unsigned char *)"expansion.library", 0L); if ((ExpansionBase == NULL) { return 0; } Also, are the type casts really necessary? If not, please drop them. https://review.coreboot.org/c/flashrom/+/54944/comment/afa638da_69ab5f68 PS1, Line 154: 0 Why return 0? https://review.coreboot.org/c/flashrom/+/54944/comment/dda622a6_9d03416c PS1, Line 169: register_spi_master(&spi_master_tf53x, NULL); This function returns something. Why not propagate the return value? return register_spi_master(&spi_master_tf53x, NULL); -- To view, visit https://review.coreboot.org/c/flashrom/+/54944 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I109eec4dec89650bb80d3d1da905d33f65f60f5a Gerrit-Change-Number: 54944 Gerrit-PatchSet: 1 Gerrit-Owner: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Comment-Date: Wed, 26 May 2021 08:57:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Add AmigaOS suppport to flashrom
by Angel Pons (Code Review) 26 May '21

26 May '21

Attention is currently required from: Paul Menzel, Stefan Reinauer. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54943 ) Change subject: Add AmigaOS suppport to flashrom ...................................................................... Patch Set 1: Code-Review+1 (7 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/54943/comment/1541c8da_5b73dde0 PS1, Line 8: > How can this be tested? In QEMU? I'm not sure if it's doable (licensing issues, maybe?), but it would be nice to add a target to util/manibuilder to build-test for AmigaOS. File Makefile: https://review.coreboot.org/c/flashrom/+/54943/comment/40641b8f_96cba8e5 PS1, Line 141: Bus Pirate, Serprog and PonyProg are not supported under AmigaOS (missing serial support). I'd rather not list all the programmers in the comment. Instead, I'd say something like this: # AmigaOS is missing serial support, disable incompatible programmers https://review.coreboot.org/c/flashrom/+/54943/comment/0f1aae50_7b64caf7 PS1, Line 157: # Digilent SPI, Dediprog, Developerbox, USB-Blaster, PICkit2, CH341A and FT2232 are not supported under AmigaOS (missing USB support). Same story here: # AmigaOS is missing USB support, disable incompatible programmers https://review.coreboot.org/c/flashrom/+/54943/comment/b9460cae_60a1f251 PS1, Line 213: DOS AmigaOS File cli_classic.c: https://review.coreboot.org/c/flashrom/+/54943/comment/b2517d92_28b543ab PS1, Line 162: *argv[] I guess this type is what makes AmigaOS unhappy? File flash.h: https://review.coreboot.org/c/flashrom/+/54943/comment/51d11540_5010b230 PS1, Line 55: wrong Out of curiosity, what does `PRIxPTR` get defined as? https://review.coreboot.org/c/flashrom/+/54943/comment/9dd92f6b_c2996753 PS1, Line 56: __AMIGA__ Why not use `IS_AMIGA` here? -- To view, visit https://review.coreboot.org/c/flashrom/+/54943 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3e4a3885f88313aad019454fdbdf4be176b35330 Gerrit-Change-Number: 54943 Gerrit-PatchSet: 1 Gerrit-Owner: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Comment-Date: Wed, 26 May 2021 08:26:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: mstarddc_spi.c: Drop mstarddc_ prefix for spi data struct members
by Edward O'Callaghan (Code Review) 26 May '21

26 May '21

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/54034 ) Change subject: mstarddc_spi.c: Drop mstarddc_ prefix for spi data struct members ...................................................................... mstarddc_spi.c: Drop mstarddc_ prefix for spi data struct members The name of the struct already contains mstarddc_ prefix, so prefix doesn't need to be repeated in members names. BUG=b:185191942 TEST=builds Change-Id: Ic04b14ce502917ae3b959cf2acf23d58b8752d47 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/54034 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M mstarddc_spi.c 1 file changed, 16 insertions(+), 15 deletions(-) Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved diff --git a/mstarddc_spi.c b/mstarddc_spi.c index da9f152..0f638df 100644 --- a/mstarddc_spi.c +++ b/mstarddc_spi.c @@ -32,9 +32,9 @@ #include "spi.h" struct mstarddc_spi_data { - int mstarddc_fd; - int mstarddc_addr; - int mstarddc_doreset; + int fd; + int addr; + int doreset; }; // MSTAR DDC Commands @@ -49,9 +49,9 @@ struct mstarddc_spi_data *mstarddc_data = data; // Reset, disables ISP mode - if (mstarddc_data->mstarddc_doreset == 1) { + if (mstarddc_data->doreset == 1) { uint8_t cmd = MSTARDDC_SPI_RESET; - if (write(mstarddc_data->mstarddc_fd, &cmd, 1) < 0) { + if (write(mstarddc_data->fd, &cmd, 1) < 0) { msg_perr("Error sending reset command: errno %d.\n", errno); return -1; @@ -62,7 +62,7 @@ "or an error occurred.\n"); } - if (close(mstarddc_data->mstarddc_fd) < 0) { + if (close(mstarddc_data->fd) < 0) { msg_perr("Error closing device: errno %d.\n", errno); return -1; } @@ -89,7 +89,7 @@ if (!ret && writecnt) { cmd[0] = MSTARDDC_SPI_WRITE; memcpy(cmd + 1, writearr, writecnt); - if (write(mstarddc_data->mstarddc_fd, cmd, writecnt + 1) < 0) { + if (write(mstarddc_data->fd, cmd, writecnt + 1) < 0) { msg_perr("Error sending write command: errno %d.\n", errno); ret = -1; @@ -103,16 +103,16 @@ cmd[0] = MSTARDDC_SPI_READ; i2c_data.nmsgs = 2; i2c_data.msgs = msg; - i2c_data.msgs[0].addr = mstarddc_data->mstarddc_addr; + i2c_data.msgs[0].addr = mstarddc_data->addr; i2c_data.msgs[0].len = 1; i2c_data.msgs[0].flags = 0; i2c_data.msgs[0].buf = cmd; - i2c_data.msgs[1].addr = mstarddc_data->mstarddc_addr; + i2c_data.msgs[1].addr = mstarddc_data->addr; i2c_data.msgs[1].len = readcnt; i2c_data.msgs[1].flags = I2C_M_RD; i2c_data.msgs[1].buf = readarr; - if (ioctl(mstarddc_data->mstarddc_fd, I2C_RDWR, &i2c_data) < 0) { + if (ioctl(mstarddc_data->fd, I2C_RDWR, &i2c_data) < 0) { msg_perr("Error sending read command: errno %d.\n", errno); ret = -1; @@ -121,7 +121,7 @@ if (!ret && (writecnt || readcnt)) { cmd[0] = MSTARDDC_SPI_END; - if (write(mstarddc_data->mstarddc_fd, cmd, 1) < 0) { + if (write(mstarddc_data->fd, cmd, 1) < 0) { msg_perr("Error sending end command: errno %d.\n", errno); ret = -1; @@ -131,7 +131,7 @@ /* Do not reset if something went wrong, as it might prevent from * retrying flashing. */ if (ret != 0) - mstarddc_data->mstarddc_doreset = 0; + mstarddc_data->doreset = 0; if (cmd) free(cmd); @@ -235,9 +235,10 @@ ret = -1; goto out; } - mstarddc_data->mstarddc_fd = mstarddc_fd; - mstarddc_data->mstarddc_addr = mstarddc_addr; - mstarddc_data->mstarddc_doreset = mstarddc_doreset; + + mstarddc_data->fd = mstarddc_fd; + mstarddc_data->addr = mstarddc_addr; + mstarddc_data->doreset = mstarddc_doreset; // Register shutdown function register_shutdown(mstarddc_spi_shutdown, mstarddc_data); -- To view, visit https://review.coreboot.org/c/flashrom/+/54034 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic04b14ce502917ae3b959cf2acf23d58b8752d47 Gerrit-Change-Number: 54034 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: 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-MessageType: merged

1 0

Change in flashrom[master]: mstarddc_spi.c: Refactor singleton states into reentrant pattern
by Edward O'Callaghan (Code Review) 26 May '21

26 May '21

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/54033 ) Change subject: mstarddc_spi.c: Refactor singleton states into reentrant pattern ...................................................................... mstarddc_spi.c: Refactor singleton states into reentrant pattern Move global singleton states into a struct and store within the spi_master data field for the life-time of the driver. This is one of the steps on the way to move spi_master data memory management behind the initialisation API, for more context see other patches under the same topic "register_master_api". BUG=b:185191942 TEST=builds Change-Id: Ia0e5f6879bfbfac591a40119ee3e0942a5cbc4b9 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/54033 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M mstarddc_spi.c 1 file changed, 38 insertions(+), 14 deletions(-) Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved diff --git a/mstarddc_spi.c b/mstarddc_spi.c index 32b77cd..da9f152 100644 --- a/mstarddc_spi.c +++ b/mstarddc_spi.c @@ -31,9 +31,11 @@ #include "programmer.h" #include "spi.h" -static int mstarddc_fd; -static int mstarddc_addr; -static int mstarddc_doreset = 1; +struct mstarddc_spi_data { + int mstarddc_fd; + int mstarddc_addr; + int mstarddc_doreset; +}; // MSTAR DDC Commands #define MSTARDDC_SPI_WRITE 0x10 @@ -44,10 +46,12 @@ /* Returns 0 upon success, a negative number upon errors. */ static int mstarddc_spi_shutdown(void *data) { + struct mstarddc_spi_data *mstarddc_data = data; + // Reset, disables ISP mode - if (mstarddc_doreset == 1) { + if (mstarddc_data->mstarddc_doreset == 1) { uint8_t cmd = MSTARDDC_SPI_RESET; - if (write(mstarddc_fd, &cmd, 1) < 0) { + if (write(mstarddc_data->mstarddc_fd, &cmd, 1) < 0) { msg_perr("Error sending reset command: errno %d.\n", errno); return -1; @@ -58,10 +62,12 @@ "or an error occurred.\n"); } - if (close(mstarddc_fd) < 0) { + if (close(mstarddc_data->mstarddc_fd) < 0) { msg_perr("Error closing device: errno %d.\n", errno); return -1; } + + free(data); return 0; } @@ -72,6 +78,7 @@ const unsigned char *writearr, unsigned char *readarr) { + struct mstarddc_spi_data *mstarddc_data = flash->mst->spi.data; int ret = 0; uint8_t *cmd = malloc((writecnt + 1) * sizeof(uint8_t)); if (cmd == NULL) { @@ -82,7 +89,7 @@ if (!ret && writecnt) { cmd[0] = MSTARDDC_SPI_WRITE; memcpy(cmd + 1, writearr, writecnt); - if (write(mstarddc_fd, cmd, writecnt + 1) < 0) { + if (write(mstarddc_data->mstarddc_fd, cmd, writecnt + 1) < 0) { msg_perr("Error sending write command: errno %d.\n", errno); ret = -1; @@ -96,16 +103,16 @@ cmd[0] = MSTARDDC_SPI_READ; i2c_data.nmsgs = 2; i2c_data.msgs = msg; - i2c_data.msgs[0].addr = mstarddc_addr; + i2c_data.msgs[0].addr = mstarddc_data->mstarddc_addr; i2c_data.msgs[0].len = 1; i2c_data.msgs[0].flags = 0; i2c_data.msgs[0].buf = cmd; - i2c_data.msgs[1].addr = mstarddc_addr; + i2c_data.msgs[1].addr = mstarddc_data->mstarddc_addr; i2c_data.msgs[1].len = readcnt; i2c_data.msgs[1].flags = I2C_M_RD; i2c_data.msgs[1].buf = readarr; - if (ioctl(mstarddc_fd, I2C_RDWR, &i2c_data) < 0) { + if (ioctl(mstarddc_data->mstarddc_fd, I2C_RDWR, &i2c_data) < 0) { msg_perr("Error sending read command: errno %d.\n", errno); ret = -1; @@ -114,7 +121,7 @@ if (!ret && (writecnt || readcnt)) { cmd[0] = MSTARDDC_SPI_END; - if (write(mstarddc_fd, cmd, 1) < 0) { + if (write(mstarddc_data->mstarddc_fd, cmd, 1) < 0) { msg_perr("Error sending end command: errno %d.\n", errno); ret = -1; @@ -124,7 +131,7 @@ /* Do not reset if something went wrong, as it might prevent from * retrying flashing. */ if (ret != 0) - mstarddc_doreset = 0; + mstarddc_data->mstarddc_doreset = 0; if (cmd) free(cmd); @@ -146,6 +153,10 @@ int mstarddc_spi_init(void) { int ret = 0; + int mstarddc_fd = -1; + int mstarddc_addr; + int mstarddc_doreset = 1; + struct mstarddc_spi_data *mstarddc_data; // Get device, address from command-line char *i2c_device = extract_programmer_param("dev"); @@ -217,13 +228,26 @@ goto out; } } + + mstarddc_data = calloc(1, sizeof(*mstarddc_data)); + if (!mstarddc_data) { + msg_perr("Unable to allocate space for SPI master data\n"); + ret = -1; + goto out; + } + mstarddc_data->mstarddc_fd = mstarddc_fd; + mstarddc_data->mstarddc_addr = mstarddc_addr; + mstarddc_data->mstarddc_doreset = mstarddc_doreset; + // Register shutdown function - register_shutdown(mstarddc_spi_shutdown, NULL); + register_shutdown(mstarddc_spi_shutdown, mstarddc_data); // Register programmer - register_spi_master(&spi_master_mstarddc, NULL); + register_spi_master(&spi_master_mstarddc, mstarddc_data); out: free(i2c_device); + if (ret && (mstarddc_fd >= 0)) + close(mstarddc_fd); return ret; } -- To view, visit https://review.coreboot.org/c/flashrom/+/54033 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia0e5f6879bfbfac591a40119ee3e0942a5cbc4b9 Gerrit-Change-Number: 54033 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: 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-MessageType: merged

1 0

Change in flashrom[master]: Add support for TerribleFire TF530 SPI controller
by Paul Menzel (Code Review) 26 May '21

26 May '21

Attention is currently required from: Stefan Reinauer. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54944 ) Change subject: Add support for TerribleFire TF530 SPI controller ...................................................................... Patch Set 1: (1 comment) File tf530_spi.c: https://review.coreboot.org/c/flashrom/+/54944/comment/6a3ad7e9_2d7b77d4 PS1, Line 4: * Copyright (C) 2019 Steven J Leary This differs from the commit author data. -- To view, visit https://review.coreboot.org/c/flashrom/+/54944 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I109eec4dec89650bb80d3d1da905d33f65f60f5a Gerrit-Change-Number: 54944 Gerrit-PatchSet: 1 Gerrit-Owner: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Comment-Date: Wed, 26 May 2021 06:20:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Add AmigaOS suppport to flashrom
by Paul Menzel (Code Review) 26 May '21

26 May '21

Attention is currently required from: Stefan Reinauer. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54943 ) Change subject: Add AmigaOS suppport to flashrom ...................................................................... Patch Set 1: Code-Review+1 (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/54943/comment/8d4dbf0c_a782cf12 PS1, Line 8: How can this be tested? In QEMU? Patchset: PS1: Had to look twice to verify I am not misreading. Very nice! -- To view, visit https://review.coreboot.org/c/flashrom/+/54943 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3e4a3885f88313aad019454fdbdf4be176b35330 Gerrit-Change-Number: 54943 Gerrit-PatchSet: 1 Gerrit-Owner: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Comment-Date: Wed, 26 May 2021 06:19:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit May 2021