Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

mfd: qnap-mcu: Add proper error handling for command errors

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>
Link: https://patch.msgid.link/20251113165218.449616-5-heiko@sntech.de
Signed-off-by: Lee Jones <lee@kernel.org>

authored by

Heiko Stuebner and committed by
Lee Jones
56c1245d c3223f56

+65 -1
+65 -1
drivers/mfd/qnap-mcu.c
··· 19 19 /* The longest command found so far is 5 bytes long */ 20 20 #define QNAP_MCU_MAX_CMD_SIZE 5 21 21 #define QNAP_MCU_MAX_DATA_SIZE 36 22 + #define QNAP_MCU_ERROR_SIZE 2 22 23 #define QNAP_MCU_CHECKSUM_SIZE 1 23 24 24 25 #define QNAP_MCU_RX_BUFFER_SIZE \ ··· 104 103 return serdev_device_write(mcu->serdev, tx, length, HZ); 105 104 } 106 105 106 + static bool qnap_mcu_is_error_msg(size_t size) 107 + { 108 + return (size == QNAP_MCU_ERROR_SIZE + QNAP_MCU_CHECKSUM_SIZE); 109 + } 110 + 111 + static bool qnap_mcu_reply_is_generic_error(unsigned char *buf, size_t size) 112 + { 113 + if (!qnap_mcu_is_error_msg(size)) 114 + return false; 115 + 116 + if (buf[0] == '@' && buf[1] == '9') 117 + return true; 118 + 119 + return false; 120 + } 121 + 122 + static bool qnap_mcu_reply_is_checksum_error(unsigned char *buf, size_t size) 123 + { 124 + if (!qnap_mcu_is_error_msg(size)) 125 + return false; 126 + 127 + if (buf[0] == '@' && buf[1] == '8') 128 + return true; 129 + 130 + return false; 131 + } 132 + 133 + static bool qnap_mcu_reply_is_any_error(struct qnap_mcu *mcu, unsigned char *buf, size_t size) 134 + { 135 + if (qnap_mcu_reply_is_generic_error(buf, size)) { 136 + dev_err(&mcu->serdev->dev, "Controller sent generic error response\n"); 137 + return true; 138 + } 139 + 140 + if (qnap_mcu_reply_is_checksum_error(buf, size)) { 141 + dev_err(&mcu->serdev->dev, "Controller received invalid checksum for the command\n"); 142 + return true; 143 + } 144 + 145 + return false; 146 + } 147 + 107 148 static size_t qnap_mcu_receive_buf(struct serdev_device *serdev, const u8 *buf, size_t size) 108 149 { 109 150 struct device *dev = &serdev->dev; ··· 177 134 */ 178 135 return src - buf; 179 136 } 137 + } 138 + 139 + /* 140 + * We received everything the uart had to offer for now. 141 + * This could mean that either the uart will send more in a 2nd 142 + * receive run, or that the MCU cut the reply short because it 143 + * sent an error code instead of the expected reply. 144 + * 145 + * So check if the received data has the correct size for an error 146 + * reply and if it matches, is an actual error code. 147 + */ 148 + if (qnap_mcu_is_error_msg(reply->received) && 149 + qnap_mcu_verify_checksum(reply->data, reply->received) && 150 + qnap_mcu_reply_is_any_error(mcu, reply->data, reply->received)) { 151 + /* The reply was an error code, we're done */ 152 + reply->length = 0; 153 + 154 + complete(&reply->done); 180 155 } 181 156 182 157 /* ··· 243 182 } 244 183 245 184 if (!qnap_mcu_verify_checksum(rx, reply->received)) { 246 - dev_err(&mcu->serdev->dev, "Invalid Checksum received\n"); 185 + dev_err(&mcu->serdev->dev, "Invalid Checksum received from controller\n"); 247 186 return -EPROTO; 248 187 } 188 + 189 + if (qnap_mcu_reply_is_any_error(mcu, rx, reply->received)) 190 + return -EPROTO; 249 191 250 192 memcpy(reply_data, rx, reply_data_size); 251 193