[PATCH 4/4] mfd: qnap-mcu: Add proper error handling for command errors

[Heiko Stuebner <heiko@sntech.de>]

Further investigation revealed that the MCU in QNAP devices may return
two error states. One "@8" for a checksum error in the submitted command
and one "@9" for any generic (and sadly unspecified) error.

These error codes with 2 data character can of course also be shorter
then the expected reply length for the submitted command, so we'll
need to check the received data for error codes and exit the receive
portion early in that case.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/mfd/qnap-mcu.c | 44 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/qnap-mcu.c b/drivers/mfd/qnap-mcu.c
index cd836bdd44a8..fb78609a8433 100644
--- a/drivers/mfd/qnap-mcu.c
+++ b/drivers/mfd/qnap-mcu.c
@@ -19,6 +19,7 @@
 /* The longest command found so far is 5 bytes long */
 #define QNAP_MCU_MAX_CMD_SIZE		5
 #define QNAP_MCU_MAX_DATA_SIZE		36
+#define QNAP_MCU_ERROR_SIZE		2
 #define QNAP_MCU_CHECKSUM_SIZE		1
 
 #define QNAP_MCU_RX_BUFFER_SIZE		\
@@ -103,6 +104,24 @@ static int qnap_mcu_write(struct qnap_mcu *mcu, const u8 *data, u8 data_size)
 	return serdev_device_write(mcu->serdev, tx, length, HZ);
 }
 
+static bool qnap_mcu_reply_is_generic_error(unsigned char *buf, size_t size)
+{
+	return (size == QNAP_MCU_ERROR_SIZE + QNAP_MCU_CHECKSUM_SIZE &&
+		buf[0] == '@' && buf[1] == '9');
+}
+
+static bool qnap_mcu_reply_is_checksum_error(unsigned char *buf, size_t size)
+{
+	return (size == QNAP_MCU_ERROR_SIZE + QNAP_MCU_CHECKSUM_SIZE &&
+		buf[0] == '@' && buf[1] == '8');
+}
+
+static bool qnap_mcu_reply_is_any_error(unsigned char *buf, size_t size)
+{
+	return (size == QNAP_MCU_ERROR_SIZE + QNAP_MCU_CHECKSUM_SIZE &&
+		buf[0] == '@' && (buf[1] == '8' || buf[1] == '9'));
+}
+
 static size_t qnap_mcu_receive_buf(struct serdev_device *serdev, const u8 *buf, size_t size)
 {
 	struct device *dev = &serdev->dev;
@@ -136,6 +155,19 @@ static size_t qnap_mcu_receive_buf(struct serdev_device *serdev, const u8 *buf,
 		}
 	}
 
+	/*
+	 * We received everything the uart had to offer for now.
+	 * Check for a possible error reply in the received data.
+	 */
+	if (reply->received == QNAP_MCU_ERROR_SIZE + QNAP_MCU_CHECKSUM_SIZE &&
+	    qnap_mcu_verify_checksum(reply->data, reply->received) &&
+	    qnap_mcu_reply_is_any_error(reply->data, reply->received)) {
+		/* The reply was an error code, we're done */
+		reply->length = 0;
+
+		complete(&reply->done);
+	}
+
 	/*
 	 * The only way to get out of the above loop and end up here
 	 * is through consuming all of the supplied data, so here we
@@ -182,10 +214,20 @@ int qnap_mcu_exec(struct qnap_mcu *mcu,
 	}
 
 	if (!qnap_mcu_verify_checksum(rx, reply->received)) {
-		dev_err(&mcu->serdev->dev, "Invalid Checksum received\n");
+		dev_err(&mcu->serdev->dev, "Invalid Checksum received from controller\n");
+		return -EPROTO;
+	}
+
+	if (qnap_mcu_reply_is_checksum_error(rx, reply->received)) {
+		dev_err(&mcu->serdev->dev, "Controller received invalid Checksum\n");
 		return -EPROTO;
 	}
 
+	if (qnap_mcu_reply_is_generic_error(rx, reply->received)) {
+		dev_err(&mcu->serdev->dev, "Generic error received from controller\n");
+		return -EIO;
+	}
+
 	memcpy(reply_data, rx, reply_data_size);
 
 	return 0;
-- 
2.47.2