hash_tester should read the correct number of response bytes

Both versions of hash_tester optimistically assumed a READ_OK or
WRITE_OK response, and tried to read the expected number of bytes.
But ERROR and UNKNOWN responses are shorter, which could lead to an
overrun of available data from i2c. With commit a768072, this is no
longer fatal (i2c will zero-pad the response), but it's good form to do
the right thing when possible.

1 files changed, 64 insertions, 25 deletions

diff --git a/src/sw/hash_tester.c b/src/sw/hash_tester.c
index d02db41..f202a03 100644
--- a/src/sw/hash_tester.c
+++ b/src/sw/hash_tester.c
@@ -63,15 +63,16 @@
 #define EOC       0xaa
 #define READ_CMD  0x10
 #define WRITE_CMD 0x11
+#define RESET_CMD 0x01
 
 /* response codes */
 #define SOR       0xaa
 #define EOR       0x55
-#define UNKNOWN   0xfe
-#define ERROR     0xfd
 #define READ_OK   0x7f
 #define WRITE_OK  0x7e
 #define RESET_OK  0x7d
+#define UNKNOWN   0xfe
+#define ERROR     0xfd
 
 /* addresses and codes common to all hash cores */
 #define ADDR_NAME0              0x00
@@ -253,26 +254,16 @@ int i2c_write(uint8_t *buf, int len)
     return 0;
 }
 
-int i2c_read(uint8_t *buf, int len)
+int i2c_read(uint8_t *b)
 {
-    int i;
-
-    if (debug)
-	printf("read  [");
-
-    for (i = 0; i < len; ++i) {
-	if (read(i2cfd, &buf[i], 1) != 1) {
-	    fprintf(stderr, "i2c read failed on byte %d: ", i);
-	    perror("");
-	    return 1;
-	}
-	if (debug)
-	    printf(" %02x", buf[i]);
+    /* read() on the i2c device only returns one byte at a time,
+     * and tc_get_resp() needs to parse the response one byte at a time
+     */
+    if (read(i2cfd, b, 1) != 1) {
+	perror("i2c read failed");
+	return 1;
     }
 
-    if (debug)
-	printf(" ]\n");
-
     return 0;
 }
 
@@ -307,12 +298,58 @@ int tc_send_read_cmd(uint8_t addr0, uint8_t addr1)
     return i2c_write(buf, sizeof(buf));
 }
 
-int tc_get_resp(uint8_t *expected, int len)
+int tc_get_resp(uint8_t *buf, int len)
+{
+    int i;
+
+    for (i = 0; i < len; ++i) {
+	if (i2c_read(&buf[i]) != 0)
+	    return 1;
+	if ((i == 0) && (buf[i] != SOR)) {
+	    /* we've gotten out of sync, and there's probably nothing we can do */
+	    fprintf(stderr, "response byte 0: expected 0x%02x (SOR), got 0x%02x\n",
+		    SOR, buf[0]);
+	    return 1;
+	}
+	else if (i == 1) {	/* response code */
+	    switch (buf[i]) {
+	    case READ_OK:
+		len = 9;
+		break;
+	    case WRITE_OK:
+		len = 5;
+		break;
+	    case RESET_OK:
+		len = 3;
+		break;
+	    case ERROR:
+	    case UNKNOWN:
+		len = 4;
+		break;
+	    default:
+		/* we've gotten out of sync, and there's probably nothing we can do */
+		fprintf(stderr, "unknown response code 0x%02x\n", buf[i]);
+		return 1;
+	    }
+	}
+    }
+
+    if (debug) {
+	printf("read  [");
+	for (i = 0; i < len; ++i)
+	    printf(" %02x", buf[i]);
+	printf(" ]\n");
+    }
+
+    return 0;
+}
+
+int tc_get_expected(uint8_t *expected, int len)
 {
     uint8_t buf[9];
     int i;
 
-    if (i2c_read(buf, len) != 0)
+    if (tc_get_resp(buf, sizeof(buf)) != 0)
 	return 1;
 
     for (i = 0; i < len; ++i) {
@@ -336,7 +373,7 @@ int tc_get_write_resp(uint8_t addr0, uint8_t addr1)
     expected[3] = addr1;
     expected[4] = EOR;
 
-    return tc_get_resp(expected, sizeof(expected));
+    return tc_get_expected(expected, sizeof(expected));
 }
 
 int tc_get_read_resp(uint8_t addr0, uint8_t addr1, uint32_t data)
@@ -353,7 +390,7 @@ int tc_get_read_resp(uint8_t addr0, uint8_t addr1, uint32_t data)
     expected[7] = data & 0xff;
     expected[8] = EOR;
 
-    return tc_get_resp(expected, sizeof(expected));
+    return tc_get_expected(expected, sizeof(expected));
 }
 
 int tc_write(uint8_t addr0, uint8_t addr1, uint32_t data)
@@ -385,7 +422,9 @@ int tc_wait(uint8_t addr0, uint8_t status)
     do {
 	if (tc_send_read_cmd(addr0, ADDR_STATUS) != 0)
 	    return 1;
-	if (i2c_read(buf, 9) != 0)
+	if (tc_get_resp(buf, 9) != 0)
+	    return 1;
+	if (buf[1] != READ_OK)
 	    return 1;
     } while ((buf[7] & status) != status);
 
@@ -869,7 +908,7 @@ int main(int argc, char *argv[])
     int addr = I2C_addr;
     int i, j, opt;
 
-    while ((opt = getopt(argc, argv, "di:a:")) != -1) {
+    while ((opt = getopt(argc, argv, "h?di:a:")) != -1) {
 	switch (opt) {
 	case 'h':
 	case '?':