touchpad_st: refactor st_tp_read_all_events()

st_tp_read_all_events() will always return "number of events available
in rx_buf.events[]".  If there are any error, recommended action will be
stored in "tp_control".

BRANCH=nocturne
BUG=b:117203130
TEST=manual on device
Signed-off-by: Wei-Han Chen <stimim@chromium.org>

Change-Id: I021b0b1baf19bee8ebe0f60dd264f3a52a656532
Reviewed-on: https://chromium-review.googlesource.com/1270401
Commit-Ready: Nicolas Boichat <drinkcat@chromium.org>
Tested-by: Wei-Han Chen <stimim@chromium.org>
Reviewed-by: Nicolas Boichat <drinkcat@chromium.org>

1 files changed, 57 insertions, 55 deletions

diff --git a/driver/touchpad_st.c b/driver/touchpad_st.c
index 65545418d7..903fee843a 100644
--- a/driver/touchpad_st.c
+++ b/driver/touchpad_st.c
@@ -40,7 +40,7 @@ BUILD_ASSERT(BYTES_PER_PIXEL == 1);
 
 /* Function prototypes */
 static int st_tp_panel_init(int full);
-static int st_tp_read_all_events(int suppress_error);
+static int st_tp_read_all_events(int show_error);
 static int st_tp_read_host_buffer_header(void);
 static int st_tp_send_ack(void);
 static int st_tp_start_scan(void);
@@ -270,9 +270,9 @@ static int st_tp_write_hid_report(void)
 	domeswitch_changed = ((old_system_state ^ system_state) &
 			      SYSTEM_STATE_DOME_SWITCH_LEVEL);
 
-	num_events = st_tp_read_all_events(0);
-	if (num_events < 0)
-		return -num_events;
+	num_events = st_tp_read_all_events(1);
+	if (tp_control)
+		return 1;
 
 	memset(&report, 0, sizeof(report));
 	report.id = REPORT_ID_TOUCHPAD;
@@ -611,7 +611,10 @@ static void dump_memory(void)
 	msleep(8);
 }
 
-static int st_tp_handle_error(uint8_t error_type)
+/*
+ * Set `tp_control` if there are any actions should be taken.
+ */
+static void st_tp_handle_error(uint8_t error_type)
 {
 	tp_control |= TP_CONTROL_SHALL_DUMP_ERROR;
 
@@ -624,7 +627,7 @@ static int st_tp_handle_error(uint8_t error_type)
 	    error_type == 0xF3 ||
 	    (error_type >= 0x47 && error_type <= 0x4E)) {
 		tp_control |= TP_CONTROL_SHALL_RESET;
-		return 1;
+		return;
 	}
 
 	/*
@@ -632,8 +635,9 @@ static int st_tp_handle_error(uint8_t error_type)
 	 */
 	if ((error_type >= 0x20 && error_type <= 0x25) ||
 	    (error_type >= 0x2E && error_type <= 0x46)) {
+		CPRINTS("tp shall halt");
 		tp_control |= TP_CONTROL_SHALL_HALT;
-		return 1;
+		return;
 	}
 
 	/*
@@ -641,7 +645,7 @@ static int st_tp_handle_error(uint8_t error_type)
 	 */
 	if (error_type >= 0x28 && error_type <= 0x2A) {
 		tp_control |= TP_CONTROL_SHALL_INIT;
-		return 1;
+		return;
 	}
 
 	/*
@@ -655,13 +659,8 @@ static int st_tp_handle_error(uint8_t error_type)
 		} else {
 			tp_control |= TP_CONTROL_SHALL_HALT;
 		}
-		return 1;
+		return;
 	}
-
-	/*
-	 * Otherwise, just ignore it.
-	 */
-	return 0;
 }
 
 /*
@@ -672,8 +671,7 @@ static int st_tp_handle_error(uint8_t error_type)
  * @return 0 for minor errors, 1 for major errors (should not handle non-error
  *   events).
  */
-static int st_tp_handle_error_report(struct st_tp_event_t *e,
-				     int suppress_error)
+static void st_tp_handle_error_report(struct st_tp_event_t *e)
 {
 	uint8_t error_type = e->report.report_type;
 
@@ -681,10 +679,7 @@ static int st_tp_handle_error_report(struct st_tp_event_t *e,
 		((e->report.info[0] << 0) | (e->report.info[1] << 8) |
 		 (e->report.info[2] << 16) | (e->report.info[3] << 24)));
 
-	if (suppress_error)
-		return 0;
-
-	return st_tp_handle_error(error_type);
+	st_tp_handle_error(error_type);
 }
 
 static void st_tp_handle_status_report(struct st_tp_event_t *e)
@@ -730,21 +725,21 @@ static void st_tp_handle_status_report(struct st_tp_event_t *e)
 /*
  * Read all events, and handle errors.
  *
- * @param suppress_error: succeed even if error events present.
+ * When there are error events, suggested action will be saved in `tp_control`.
  *
- * @return number of events available on success, or negative error code on
- *         failure.
+ * @param show_error: weather EC should read and dump error or not.
+ *   ***If this is true, rx_buf.events[] will be cleared.***
+ *
+ * @return number of events available
  */
-static int st_tp_read_all_events(int suppress_error)
+static int st_tp_read_all_events(int show_error)
 {
 	uint8_t cmd = ST_TP_CMD_READ_ALL_EVENTS;
 	int rx_len = sizeof(rx_buf.events) + ST_TP_DUMMY_BYTE;
-	int ret;
 	int i;
 
-	ret = spi_transaction(SPI, &cmd, 1, (uint8_t *)&rx_buf, rx_len);
-	if (ret)
-		return -ret;
+	if (spi_transaction(SPI, &cmd, 1, (uint8_t *)&rx_buf, rx_len))
+		return 0;
 
 	for (i = 0; i < ARRAY_SIZE(rx_buf.events); i++) {
 		struct st_tp_event_t *e = &rx_buf.events[i];
@@ -752,25 +747,25 @@ static int st_tp_read_all_events(int suppress_error)
 		if (e->magic != ST_TP_EVENT_MAGIC)
 			break;
 
-		if (e->evt_id == ST_TP_EVENT_ID_ERROR_REPORT)
-			ret |= st_tp_handle_error_report(e, suppress_error);
-
-		if (e->evt_id == ST_TP_EVENT_ID_STATUS_REPORT)
+		switch (e->evt_id) {
+		case ST_TP_EVENT_ID_ERROR_REPORT:
+			st_tp_handle_error_report(e);
+			break;
+		case ST_TP_EVENT_ID_STATUS_REPORT:
 			st_tp_handle_status_report(e);
-	}
-
-	if (!suppress_error) {
-		if (tp_control & TP_CONTROL_SHALL_DUMP_ERROR) {
-			enable_deep_sleep(0);
-			dump_error();
-			dump_memory();
-			enable_deep_sleep(1);
-			tp_control &= ~TP_CONTROL_SHALL_DUMP_ERROR;
+			break;
 		}
+	}
 
-		if (ret)
-			return -ret;
+	if (show_error && (tp_control & TP_CONTROL_SHALL_DUMP_ERROR)) {
+		enable_deep_sleep(0);
+		dump_error();
+		dump_memory();
+		enable_deep_sleep(1);
+		/* rx_buf.events[] is invalid now */
+		i = 0;
 	}
+	tp_control &= ~TP_CONTROL_SHALL_DUMP_ERROR;
 	return i;
 }
 
@@ -785,9 +780,16 @@ static int st_tp_reset(void)
 	board_touchpad_reset();
 
 	while (retry--) {
-		num_events = st_tp_read_all_events(1);
-		if (num_events < 0)
-			return -num_events;
+		num_events = st_tp_read_all_events(0);
+
+		/*
+		 * We are not doing full panel initialization, and error code
+		 * suggest us to reset or halt.
+		 */
+		if (!(tp_control & (TP_CONTROL_INIT | TP_CONTROL_INIT_FULL)) &&
+		    (tp_control & (TP_CONTROL_SHALL_HALT |
+				   TP_CONTROL_SHALL_RESET)))
+			break;
 
 		for (i = 0; i < num_events; i++) {
 			struct st_tp_event_t *e = &rx_buf.events[i];
@@ -808,7 +810,7 @@ static int st_tp_reset(void)
 /* Initialize the controller ICs after reset */
 static void st_tp_init(void)
 {
-	tp_control = TP_CONTROL_RESETTING;
+	tp_control = 0;
 
 	if (st_tp_reset())
 		return;
@@ -1041,9 +1043,7 @@ static int st_tp_write_flash(int offset, int size, const uint8_t *data)
 static int st_tp_check_command_echo(const uint8_t *cmd, const size_t len)
 {
 	int num_events, i;
-	num_events = st_tp_read_all_events(1);
-	if (num_events < 0)
-		return -num_events;
+	num_events = st_tp_read_all_events(0);
 
 	for (i = 0; i < num_events; i++) {
 		struct st_tp_event_t *e = &rx_buf.events[i];
@@ -1053,7 +1053,7 @@ static int st_tp_check_command_echo(const uint8_t *cmd, const size_t len)
 		    memcmp(e->report.info, cmd, MIN(4, len)) == 0)
 			return EC_SUCCESS;
 	}
-	return -EC_ERROR_BUSY;
+	return EC_ERROR_BUSY;
 }
 
 static uint8_t get_cx_version(uint8_t tp_version)
@@ -1120,7 +1120,7 @@ static int st_tp_panel_init(int full)
 			tp_control &= ~(TP_CONTROL_INIT | TP_CONTROL_INIT_FULL);
 			st_tp_init();
 			return EC_SUCCESS;
-		} else if (ret == -EC_ERROR_BUSY) {
+		} else if (ret == EC_ERROR_BUSY) {
 			CPRINTS("Panel initialization on going...");
 		} else {
 			CPRINTS("Panel initialization failed: %x", -ret);
@@ -1227,7 +1227,7 @@ int touchpad_debug(const uint8_t *param, unsigned int param_size,
 		CPRINTS("header: %.*h", *data_size, buf);
 		return EC_SUCCESS;
 	case ST_TP_DEBUG_CMD_READ_EVENTS:
-		num_events = st_tp_read_all_events(1);
+		num_events = st_tp_read_all_events(0);
 		if (num_events) {
 			int i;
 
@@ -1517,9 +1517,11 @@ static int st_tp_read_frame(void)
 	uint8_t *rx_buf = usb_packet[spi_buffer_index & 1].frame;
 #endif
 
-	ret = st_tp_read_all_events(0);
-	if (ret < 0)
+	st_tp_read_all_events(1);
+	if (tp_control) {
+		ret = EC_ERROR_UNKNOWN;
 		goto failed;
+	}
 
 	if (heat_map_addr < 0)
 		goto failed;