Author: stefanct Date: Sat Aug 24 04:10:18 2013 New Revision: 1719 URL: http://flashrom.org/trac/flashrom/changeset/1719
Log: Get rid of sp_die().
- Add return values to sp_flush_stream(), sp_pass_writen(), sp_execute_opbuf(), sp_execute_opbuf_noflush(), sp_check_opbuf_usage(), sp_do_read_n(). - Use those return values to propagate errors instead of exiting. In some places this has to wait for core API changes (error handling for chip_readb, chip_readn, chip_write) hence comments are added instead.
Signed-off-by: Stefan Tauner stefan.tauner@student.tuwien.ac.at Acked-by: Stefan Tauner stefan.tauner@student.tuwien.ac.at
Modified: trunk/programmer.h trunk/serial.c trunk/serprog.c
Modified: trunk/programmer.h ============================================================================== --- trunk/programmer.h Sat Aug 24 01:29:23 2013 (r1718) +++ trunk/programmer.h Sat Aug 24 04:10:18 2013 (r1719) @@ -661,7 +661,6 @@ void sp_flush_incoming(void); fdtype sp_openserport(char *dev, unsigned int baud); int serialport_config(fdtype fd, unsigned int baud); -void __attribute__((noreturn)) sp_die(char *msg); extern fdtype sp_fd; /* expose serialport_shutdown as it's currently used by buspirate */ int serialport_shutdown(void *data);
Modified: trunk/serial.c ============================================================================== --- trunk/serial.c Sat Aug 24 01:29:23 2013 (r1718) +++ trunk/serial.c Sat Aug 24 04:10:18 2013 (r1719) @@ -41,12 +41,6 @@
fdtype sp_fd = SER_INV_FD;
-void __attribute__((noreturn)) sp_die(char *msg) -{ - perror(msg); - exit(1); -} - #ifdef _WIN32 struct baudentry { DWORD flag;
Modified: trunk/serprog.c ============================================================================== --- trunk/serprog.c Sat Aug 24 01:29:23 2013 (r1718) +++ trunk/serprog.c Sat Aug 24 04:10:18 2013 (r1719) @@ -237,43 +237,58 @@ return 0; }
-static void sp_flush_stream(void) +static int sp_flush_stream(void) { if (sp_streamed_transmit_ops) do { unsigned char c; if (serialport_read(&c, 1) != 0) { - sp_die("Error: cannot read from device (flushing stream)"); + msg_perr("Error: cannot read from device (flushing stream)"); + return 1; } if (c == S_NAK) { msg_perr("Error: NAK to a stream buffer operation\n"); - exit(1); + return 1; } if (c != S_ACK) { msg_perr("Error: Invalid reply 0x%02X from device\n", c); - exit(1); + return 1; } } while (--sp_streamed_transmit_ops); sp_streamed_transmit_ops = 0; sp_streamed_transmit_bytes = 0; + return 0; }
-static int sp_stream_buffer_op(uint8_t cmd, uint32_t parmlen, uint8_t * parms) +static int sp_stream_buffer_op(uint8_t cmd, uint32_t parmlen, uint8_t *parms) { uint8_t *sp; if (sp_automatic_cmdcheck(cmd)) return 1; + sp = malloc(1 + parmlen); - if (!sp) sp_die("Error: cannot malloc command buffer"); + if (!sp) { + msg_perr("Error: cannot malloc command buffer\n"); + return 1; + } sp[0] = cmd; memcpy(&(sp[1]), parms, parmlen); - if (sp_streamed_transmit_bytes >= (1 + parmlen + sp_device_serbuf_size)) - sp_flush_stream(); - if (serialport_write(sp, 1 + parmlen) != 0) - sp_die("Error: cannot write command"); - free(sp); + + if (sp_streamed_transmit_bytes >= (1 + parmlen + sp_device_serbuf_size)) { + if (sp_flush_stream() != 0) { + free(sp); + return 1; + } + } + if (serialport_write(sp, 1 + parmlen) != 0) { + msg_perr("Error: cannot write command\n"); + free(sp); + return 1; + } sp_streamed_transmit_ops += 1; sp_streamed_transmit_bytes += 1 + parmlen; + + free(sp); return 0; }
@@ -656,16 +671,16 @@ return 0; }
-/* Move an in flashrom buffer existing write-n operation to * - * the on-device operation buffer. */ -static void sp_pass_writen(void) +/* Move an in flashrom buffer existing write-n operation to the on-device operation buffer. */ +static int sp_pass_writen(void) { unsigned char header[7]; - msg_pspew(MSGHEADER "Passing write-n bytes=%d addr=0x%x\n", - sp_write_n_bytes, sp_write_n_addr); - if (sp_streamed_transmit_bytes >= - (7 + sp_write_n_bytes + sp_device_serbuf_size)) - sp_flush_stream(); + msg_pspew(MSGHEADER "Passing write-n bytes=%d addr=0x%x\n", sp_write_n_bytes, sp_write_n_addr); + if (sp_streamed_transmit_bytes >= (7 + sp_write_n_bytes + sp_device_serbuf_size)) { + if (sp_flush_stream() != 0) { + return 1; + } + } /* In case it's just a single byte send it as a single write. */ if (sp_write_n_bytes == 1) { sp_write_n_bytes = 0; @@ -673,9 +688,10 @@ header[1] = (sp_write_n_addr >> 8) & 0xFF; header[2] = (sp_write_n_addr >> 16) & 0xFF; header[3] = sp_write_n_buf[0]; - sp_stream_buffer_op(S_CMD_O_WRITEB, 4, header); + if (sp_stream_buffer_op(S_CMD_O_WRITEB, 4, header) != 0) + return 1; sp_opbuf_usage += 5; - return; + return 0; } header[0] = S_CMD_O_WRITEN; header[1] = (sp_write_n_bytes >> 0) & 0xFF; @@ -684,39 +700,55 @@ header[4] = (sp_write_n_addr >> 0) & 0xFF; header[5] = (sp_write_n_addr >> 8) & 0xFF; header[6] = (sp_write_n_addr >> 16) & 0xFF; - if (serialport_write(header, 7) != 0) - sp_die("Error: cannot write write-n command\n"); - if (serialport_write(sp_write_n_buf, sp_write_n_bytes) != 0) - sp_die("Error: cannot write write-n data"); + if (serialport_write(header, 7) != 0) { + msg_perr(MSGHEADER "Error: cannot write write-n command\n"); + return 1; + } + if (serialport_write(sp_write_n_buf, sp_write_n_bytes) != 0) { + msg_perr(MSGHEADER "Error: cannot write write-n data"); + return 1; + } sp_streamed_transmit_bytes += 7 + sp_write_n_bytes; sp_streamed_transmit_ops += 1; sp_opbuf_usage += 7 + sp_write_n_bytes; sp_write_n_bytes = 0; sp_prev_was_write = 0; + return 0; }
-static void sp_execute_opbuf_noflush(void) +static int sp_execute_opbuf_noflush(void) { - if ((sp_max_write_n) && (sp_write_n_bytes)) - sp_pass_writen(); - sp_stream_buffer_op(S_CMD_O_EXEC, 0, NULL); - msg_pspew(MSGHEADER "Executed operation buffer of %d bytes\n", - sp_opbuf_usage); + if ((sp_max_write_n) && (sp_write_n_bytes)) { + if (sp_pass_writen() != 0) { + msg_perr("Error: could not transfer write buffer\n"); + return 1; + } + } + if (sp_stream_buffer_op(S_CMD_O_EXEC, 0, NULL) != 0) { + msg_perr("Error: could not execute command buffer\n"); + return 1; + } + msg_pspew(MSGHEADER "Executed operation buffer of %d bytes\n", sp_opbuf_usage); sp_opbuf_usage = 0; sp_prev_was_write = 0; - return; + return 0; }
-static void sp_execute_opbuf(void) +static int sp_execute_opbuf(void) { - sp_execute_opbuf_noflush(); - sp_flush_stream(); + if (sp_execute_opbuf_noflush() != 0) + return 1; + if (sp_flush_stream() != 0) + return 1; + + return 0; }
static int serprog_shutdown(void *data) { if ((sp_opbuf_usage) || (sp_max_write_n && sp_write_n_bytes)) - sp_execute_opbuf(); + if (sp_execute_opbuf() != 0) + msg_pwarn("Could not flush command buffer.\n"); if (sp_check_commandavail(S_CMD_S_PIN_STATE)) { uint8_t dis = 0; if (sp_docommand(S_CMD_S_PIN_STATE, 1, &dis, 0, NULL) == 0) @@ -731,14 +763,15 @@ return 0; }
-static void sp_check_opbuf_usage(int bytes_to_be_added) +static int sp_check_opbuf_usage(int bytes_to_be_added) { if (sp_device_opbuf_size <= (sp_opbuf_usage + bytes_to_be_added)) { - sp_execute_opbuf(); - /* If this happens in the mid of an page load the page load * - * will probably fail. */ - msg_pdbg(MSGHEADER "Warning: executed operation buffer due to size reasons\n"); + /* If this happens in the middle of a page load the page load will probably fail. */ + msg_pwarn(MSGHEADER "Warning: executed operation buffer due to size reasons\n"); + if (sp_execute_opbuf() != 0) + return 1; } + return 0; }
static void serprog_chip_writeb(const struct flashctx *flash, uint8_t val, @@ -768,7 +801,7 @@ writeb_parm[1] = (addr >> 8) & 0xFF; writeb_parm[2] = (addr >> 16) & 0xFF; writeb_parm[3] = val; - sp_stream_buffer_op(S_CMD_O_WRITEB, 4, writeb_parm); + sp_stream_buffer_op(S_CMD_O_WRITEB, 4, writeb_parm); // FIXME: return error sp_opbuf_usage += 5; } } @@ -785,16 +818,16 @@ buf[0] = ((addr >> 0) & 0xFF); buf[1] = ((addr >> 8) & 0xFF); buf[2] = ((addr >> 16) & 0xFF); - sp_stream_buffer_op(S_CMD_R_BYTE, 3, buf); - sp_flush_stream(); + sp_stream_buffer_op(S_CMD_R_BYTE, 3, buf); // FIXME: return error + sp_flush_stream(); // FIXME: return error if (serialport_read(&c, 1) != 0) - sp_die("readb byteread"); + msg_perr(MSGHEADER "readb byteread"); // FIXME: return error msg_pspew("%s addr=0x%" PRIxPTR " returning 0x%02X\n", __func__, addr, c); return c; }
/* Local version that really does the job, doesn't care of max_read_n. */ -static void sp_do_read_n(uint8_t * buf, const chipaddr addr, size_t len) +static int sp_do_read_n(uint8_t * buf, const chipaddr addr, size_t len) { unsigned char sbuf[6]; msg_pspew("%s: addr=0x%" PRIxPTR " len=%zu\n", __func__, addr, len); @@ -808,10 +841,13 @@ sbuf[4] = ((len >> 8) & 0xFF); sbuf[5] = ((len >> 16) & 0xFF); sp_stream_buffer_op(S_CMD_R_NBYTES, 6, sbuf); - sp_flush_stream(); - if (serialport_read(buf, len) != 0) - sp_die("Error: cannot read read-n data"); - return; + if (sp_flush_stream() != 0) + return 1; + if (serialport_read(buf, len) != 0) { + msg_perr(MSGHEADER "Error: cannot read read-n data"); + return 1; + } + return 0; }
/* The externally called version that makes sure that max_read_n is obeyed. */ @@ -821,12 +857,12 @@ size_t lenm = len; chipaddr addrm = addr; while ((sp_max_read_n != 0) && (lenm > sp_max_read_n)) { - sp_do_read_n(&(buf[addrm-addr]), addrm, sp_max_read_n); + sp_do_read_n(&(buf[addrm-addr]), addrm, sp_max_read_n); // FIXME: return error addrm += sp_max_read_n; lenm -= sp_max_read_n; } if (lenm) - sp_do_read_n(&(buf[addrm-addr]), addrm, lenm); + sp_do_read_n(&(buf[addrm-addr]), addrm, lenm); // FIXME: return error }
void serprog_delay(int usecs) @@ -858,11 +894,18 @@ unsigned char *parmbuf; int ret; msg_pspew("%s, writecnt=%i, readcnt=%i\n", __func__, writecnt, readcnt); - if ((sp_opbuf_usage) || (sp_max_write_n && sp_write_n_bytes)) - sp_execute_opbuf(); + if ((sp_opbuf_usage) || (sp_max_write_n && sp_write_n_bytes)) { + if (sp_execute_opbuf() != 0) { + msg_perr("Error: could not execute command buffer before sending SPI commands.\n"); + return 1; + } + } + parmbuf = malloc(writecnt + 6); - if (!parmbuf) - sp_die("Error: cannot malloc SPI send param buffer"); + if (!parmbuf) { + msg_perr("Error: could not allocate SPI send param buffer.\n"); + return 1; + } parmbuf[0] = (writecnt >> 0) & 0xFF; parmbuf[1] = (writecnt >> 8) & 0xFF; parmbuf[2] = (writecnt >> 16) & 0xFF;