Modify stm32mon to better suite autotest use

This change introduces the following modifications:

When used outside chroot on Goobuntu the serial interface fails to initialize
if parity enable bit is set (for a reason not clearly understood). On top of
that the tcsetattr() return value of zero is not a guarantee of success. To be
sure that the settings came through one is supposed to read back the driver
config and compare it with the desired config.

To add insult to injury, gPrecise driver rejects attempts to enable parity.
Parity setting is not essential in many cases, this is why we check the actual
config and if the only missing setting is parity we print a warning message
and continue.

In case an operation fails, the exit value should reflect that (so
that the autotest suite using the utility could report failure).

Often when the programming attempt is undertaken soon after reset,
this utility gets overwhelmed with the console output generated by the
EC on resets. Consume the output before proceeding.

Instead of printing a long set of dots (one per written/read block),
print a spinning wheel instead.

BRANCH=None
BUG=chrome-os-partner:15610
TEST=manual

   . used the utility to program Snow EC both inside and outside
     chroot, it succeeded.

    Observed the failing attempt to set parity when running outside
    chroot.

    Observed spinning characters instead of stream of dots.

Change-Id: Id25595d35a2a3ca578639cebd508f599e618787c
Signed-off-by: Vadim Bendebury <vbendeb@chromium.org>
Reviewed-on: https://gerrit.chromium.org/gerrit/42310
Reviewed-by: Vincent Palatin <vpalatin@chromium.org>

1 files changed, 66 insertions, 17 deletions

diff --git a/util/stm32mon.c b/util/stm32mon.c
index e6c76f9a62..e8dd908f23 100644
--- a/util/stm32mon.c
+++ b/util/stm32mon.c
@@ -83,13 +83,14 @@ typedef struct {
 } payload_t;
 
 static int has_exterase;
+static void discard_input(int);
 
 #define MIN(a, b) ((a) < (b) ? (a) : (b))
 
 int open_serial(const char *port)
 {
 	int fd, res;
-	struct termios cfg;
+	struct termios cfg, cfg_copy;
 
 	fd = open(port, O_RDWR | O_NOCTTY);
 	if (fd == -1) {
@@ -106,18 +107,46 @@ int open_serial(const char *port)
 	}
 	cfmakeraw(&cfg);
 	cfsetspeed(&cfg, baudrate);
-	/* serial mode is 8e1 */
+	/* serial mode should be 8e1 */
 	cfg.c_cflag |= PARENB;
 	/* 200 ms timeout */
 	cfg.c_cc[VTIME] = 2;
 	cfg.c_cc[VMIN] = 0;
-	res = tcsetattr(fd, TCSANOW, &cfg);
-	if (res == -1) {
-		perror("Cannot set tty attributes");
+	memcpy(&cfg_copy, &cfg, sizeof(cfg_copy));
+
+	/*
+	 * tcsetattr() returns success if any of the modifications succeed, so
+	 * its return value of zero is not an indication of success, one needs
+	 * to check the result explicitely.
+	 */
+	tcsetattr(fd, TCSANOW, &cfg);
+	if (tcgetattr(fd, &cfg)) {
+		perror("Failed to re-read tty attributes");
 		close(fd);
 		return -1;
 	}
 
+	if (memcmp(&cfg, &cfg_copy, sizeof(cfg))) {
+		/*
+		 * On some systems the setting which does not come through is
+		 * the parity. We can try continuing without it when using
+		 * certain interfaces, let's try.
+		 */
+		cfg_copy.c_cflag &= ~PARENB;
+		if (memcmp(&cfg, &cfg_copy, sizeof(cfg))) {
+			/*
+			 * Something other than parity failed to get set, this
+			 * is an error.
+			 */
+			perror("Cannot set tty attributes");
+			close(fd);
+			return -1;
+		} else {
+			fprintf(stderr, "Failed to enable parity\n");
+		}
+	}
+
+	discard_input(fd); /* in case were were invoked soon after reset */
 	return fd;
 }
 
@@ -303,7 +332,6 @@ int init_monitor(int fd)
 		}
 		if (res < 0 && res != -ETIMEDOUT)
 			return -1;
-		printf(".");
 		fflush(stdout);
 	}
 	printf("Done.\n");
@@ -341,6 +369,14 @@ int command_get_commands(int fd)
 	return -1;
 }
 
+static int windex;
+static const char wheel[] = {'|', '/', '-', '\\' };
+static void draw_spinner(void)
+{
+	printf("%c%c", 8, wheel[windex++]);
+	windex %= sizeof(wheel);
+}
+
 int command_read_mem(int fd, uint32_t address, uint32_t size, uint8_t *buffer)
 {
 	int res;
@@ -356,7 +392,7 @@ int command_read_mem(int fd, uint32_t address, uint32_t size, uint8_t *buffer)
 		cnt = (remaining > PAGE_SIZE) ? PAGE_SIZE - 1 : remaining - 1;
 		addr_be = htonl(address);
 
-		printf(".");
+		draw_spinner();
 		fflush(stdout);
 		res = send_command(fd, CMD_READMEM, loads, 2, buffer, cnt + 1);
 		if (res < 0)
@@ -389,7 +425,7 @@ int command_write_mem(int fd, uint32_t address, uint32_t size, uint8_t *buffer)
 		loads[1].size = cnt + 1;
 		memcpy(outbuf + 1, buffer, cnt);
 
-		printf(".");
+		draw_spinner();
 		fflush(stdout);
 		res = send_command(fd, CMD_WRITEMEM, loads, 2, NULL, 0);
 		if (res < 0)
@@ -531,6 +567,7 @@ int command_go(int fd, uint32_t address)
 	return 0;
 }
 
+/* Return zero on success, a negative error value on failures. */
 int read_flash(int fd, struct stm32_def *chip, const char *filename,
 	       uint32_t offset, uint32_t size)
 {
@@ -553,7 +590,7 @@ int read_flash(int fd, struct stm32_def *chip, const char *filename,
 	if (!size)
 		size = chip->flash_size;
 	offset += chip->flash_start;
-	printf("Reading %d bytes at 0x%08x ", size, offset);
+	printf("Reading %d bytes at 0x%08x  ", size, offset);
 	res = command_read_mem(fd, offset, size, buffer);
 	if (res > 0) {
 		if (fwrite(buffer, res, 1, hnd) != 1)
@@ -563,9 +600,10 @@ int read_flash(int fd, struct stm32_def *chip, const char *filename,
 
 	fclose(hnd);
 	free(buffer);
-	return res;
+	return (res < 0) ? res : 0;
 }
 
+/* Return zero on success, a negative error value on failures. */
 int write_flash(int fd, struct stm32_def *chip, const char *filename,
 		uint32_t offset)
 {
@@ -587,19 +625,23 @@ int write_flash(int fd, struct stm32_def *chip, const char *filename,
 	}
 	if ((res = fread(buffer, 1, size, hnd)) <= 0) {
 		fprintf(stderr, "Cannot read %s\n", filename);
+		free(buffer);
 		return -EIO;
 	}
 	fclose(hnd);
 
 	offset += chip->flash_start;
-	printf("Writing %d bytes at 0x%08x ", res, offset);
+	printf("Writing %d bytes at 0x%08x  ", res, offset);
 	written = command_write_mem(fd, offset, res, buffer);
-	if (written != res)
+	if (written != res) {
 		fprintf(stderr, "Error writing to flash\n");
+		free(buffer);
+		return -EIO;
+	}
 	printf("Done.\n");
 
 	free(buffer);
-	return written;
+	return 0;
 }
 
 static const struct option longopts[] = {
@@ -739,11 +781,18 @@ int main(int argc, char **argv)
 		}
 	}
 
-	if (input_filename)
-		read_flash(ser, chip, input_filename, 0, chip->flash_size);
+	if (input_filename) {
+		ret = read_flash(ser, chip, input_filename,
+				 0, chip->flash_size);
+		if (ret)
+			goto terminate;
+	}
 
-	if (output_filename)
-		write_flash(ser, chip, output_filename, 0);
+	if (output_filename) {
+		ret = write_flash(ser, chip, output_filename, 0);
+		if (ret)
+			goto terminate;
+	}
 
 	/* Run the program from flash */
 	if (flags & FLAG_GO)