Merge pull request #342 from reMarkable/rm1xx_5.4.70_v1.3.x

merge in release branch rm1xx 5.4.70 v1.3.x

drivers/misc/rm-pogo/pogo.h

@@ -208,7 +208,8 @@ struct rm_pogo_data {
 
 	int					mode_requested;
 	int					uart_rx_mode;
-	bool					serdev_ready;
+	atomic_t				serdev_ready;
+	bool					serdev_open;
 	bool					tx_ack_timeout;
 	bool					tx_ack_required;
 	bool					mcu_authenticated;
@@ -233,10 +234,11 @@ struct rm_pogo_data {
 	struct timer_list			alive_timer;
 	unsigned int				kb_row_shift;
 
-	u8				user_command;
-	u8				*user_command_data;
-	int				user_command_data_len;
-	u8				user_command_response[ONE_WIRE_MCU_MSG_SIZE];
+	u8					user_command;
+	bool					user_command_sent;
+	u8					*user_command_data;
+	int					user_command_data_len;
+	u8					user_command_response[ONE_WIRE_MCU_MSG_SIZE];
 
 	struct pinctrl*				one_wire_pinctrl;
 	struct pinctrl_state*			one_wire_pinctrl_states[POGO_ONEWIRE_STATE_NR];

drivers/misc/rm-pogo/pogo_attribute_storage.h

@@ -25,6 +25,7 @@
 			pdata->user_command_data = (u8 *)devm_kmalloc(         \
 				pdata->dev, DATA_SIZE, GFP_KERNEL);            \
 		pdata->user_command = COMMAND;                                 \
+		pdata->user_command_sent = false;                              \
 		pdata->user_command_data_len = DATA_SIZE;                      \
 	} while (0)
 

drivers/misc/rm-pogo/pogo_fsm.c

@@ -195,19 +195,18 @@ static bool fsm_entry_idle(struct rm_pogo_data *pdata)
 	pdata->mfg_log = 0;
 	pdata->mcu_alive_interval = 0;
 	pdata->user_command = POGO_CMD_NONE;
+	pdata->user_command_sent = false;
 	if (pdata->user_command_data) {
 		devm_kfree(pdata->dev, pdata->user_command_data);
 	}
 	pdata->user_command_data = NULL;
 	pdata->user_command_data_len = 0;
-	
+
 	memset(pdata->onewire_rx_buf, 0, ONE_WIRE_MAX_TX_MSG_SIZE);
 	pdata->onewire_rx_buf_len = 0;
 	pdata->pogo_connected = false;
 
-	if (pdata->serdev_ready)
-		pogo_serdev_close(pdata);
-	pdata->serdev_ready = false;
+	atomic_set(&pdata->serdev_ready, 0);
 
 	pdata->tx_ack_timeout = false;
 	pdata->tx_ack_required = false;
@@ -249,13 +248,17 @@ static bool fsm_entry_enumerate(struct rm_pogo_data *pdata)
 			"Unable to turn on OTG power, check connections\n");
 		return false;
 	}
-
+	if (pdata->serdev_open) {
+		pogo_serdev_close(pdata);
+		pdata->serdev_open = false;
+	}
 	/* Sleep to allow VBUS to be turned on */
 	usleep_range(20000, 21000);
 
-	if (!pogo_serdev_open(pdata))
-		pdata->serdev_ready = true;
-	else {
+	if (!pogo_serdev_open(pdata)) {
+		pdata->serdev_open = true;
+		atomic_set(&pdata->serdev_ready, 1);
+	} else {
 		return false;
 	}
 
@@ -708,9 +711,7 @@ static void fsm_routine_start_app(struct rm_pogo_data *pdata)
 
 static void clean_up_uart_keyboard(struct rm_pogo_data *pdata)
 {
-	if (pdata->serdev_ready)
-		pogo_serdev_close(pdata);
-	pdata->serdev_ready = false;
+	atomic_set(&pdata->serdev_ready, 0);
 
 	if (pdata->kb_dev == NULL)
 		return;
@@ -769,7 +770,7 @@ fsm_routine_uart_keyboard_handle_message(struct rm_pogo_data *pdata,
 		mod_timer(&pdata->alive_timer,
 			  jiffies + msecs_to_jiffies(pdata->alive_timeout));
 
-		if (pdata->user_command != POGO_CMD_NONE) {
+		if (pdata->user_command != POGO_CMD_NONE && !pdata->user_command_sent) {
 			memset(pdata->user_command_response, 0x00,
 			       sizeof(pdata->user_command_response));
 			/* This will work also if there is no data to send.
@@ -788,10 +789,11 @@ fsm_routine_uart_keyboard_handle_message(struct rm_pogo_data *pdata,
 					"failed sending user cmd 0x%x\n",
 					pdata->user_command);
 			}
+			pdata->user_command_sent = true;
 		}
 		mod_timer(&pdata->alive_timer,
 			  jiffies + msecs_to_jiffies(pdata->alive_timeout));
-		break;
+		return;
 	case POGO_CMD_ATTRIBUTE_READ:
 		len = packet_payload_len(msg);
 		update_pdata_from_attribute_read_response(pdata, &msg[3], len);
@@ -825,7 +827,7 @@ fsm_routine_uart_keyboard_handle_message(struct rm_pogo_data *pdata,
 	 * KB_REPORT_KEY or KB_REPORT_ALIVE as these commands are not allowed
 	 * to be sent as user commands
 	 */
-	if (pdata->user_command != POGO_CMD_NONE) {
+	if (pdata->user_command != POGO_CMD_NONE && pdata->user_command_sent) {
 		if (msg[2] == pdata->user_command) {
 			dev_dbg(pdata->dev, "user command reply 0x%x\n",
 				msg[2]);
@@ -849,10 +851,12 @@ fsm_routine_uart_keyboard_handle_message(struct rm_pogo_data *pdata,
 				msg[2], pdata->user_command);
 		}
 		pdata->user_command = POGO_CMD_NONE;
+		pdata->user_command_sent = false;
 		if (pdata->user_command_data) {
 			devm_kfree(pdata->dev, pdata->user_command_data);
 			pdata->user_command_data = NULL;
 		}
+		return;
 	}
 }
 
@@ -872,7 +876,7 @@ static void fsm_routine_uart_keyboard(struct rm_pogo_data *pdata)
 		fsm_routine_uart_keyboard_handle_message(pdata, msg, count,
 							 len);
 
-		/* Do we have more data in the RX buffer? 
+		/* Do we have more data in the RX buffer?
 		 * If not, that's fine, the macro will return from this
 		 * function. */
 		GET_AND_VERIFY_MSG(msg, ONE_WIRE_MCU_MSG_SIZE, , false);

drivers/misc/rm-pogo/pogo_main.c

@@ -298,16 +298,12 @@ static int pogo_onewire_receive_buf(struct serdev_device *serdev,
 				const unsigned char *buf, const size_t count)
 {
 	struct rm_pogo_data *data = serdev_device_get_drvdata(serdev);
-	int i, ret = 0;
+	int ret = 0;
 	int len = 0;
 	int bytes_processed = 0;
 
 	const bool tx_ack_required_backup = data->tx_ack_required;
 
-	// FIXME Remove, only for debugging
-	unsigned char *str_buf =
-		kzalloc((ONE_WIRE_MAX_TX_MSG_SIZE * 2) + 1, GFP_KERNEL);
-
 	/* The tx ack timeout work might run here. To avoid the work triggering
 	 * a timeout just as we might be about to/just as we have resent a
 	 * message we temporarily set the tx_ack_required flag to false which
@@ -320,25 +316,31 @@ static int pogo_onewire_receive_buf(struct serdev_device *serdev,
 	cancel_delayed_work_sync(&data->uart_tx_ack_watchdog_work_queue);
 	data->tx_ack_required = tx_ack_required_backup;
 
-	if (!mutex_trylock(&data->lock)) {
-		/* When FSM fall back to idle, serdev_device_close() is called.
-		 * At the same time TTY core may call pogo_onewire_receive_buf()
-		 * to notify pogo driver to handle just received RX data.
-		 * serdev_device_close() want to hold tty lock with pogo lock held,
-		 * while pogo_onewire_receive_buf() want to hold pogo lock
-		 * with tty lock held.
-		 */
-		dev_warn(data->dev, "%s: mutex_trylock failed\n", __func__);
-		return 0;
+	print_hex_dump_debug("pogo raw data:", DUMP_PREFIX_NONE, count, 1,
+			     buf, count, false);
+
+
+	/* We are closing serdev on connect (if open), not on disconnect.
+	 * There is a small possibility for MCU to send a message if it is
+	 * physically disconnected and reconnected within a very short time. This is
+	 * because there might just enough charge to keep the MCU running.
+	 * At the same time the driver has initialized re-connection and tries to
+	 * close serdev. In that situation the pogo lock is taken by fsm_entry_enumerate()
+	 * while the tty lock is taken by pogo_onewire_receive_buf(). Then both
+	 * functions try to take the other lock resulting in deadlock.
+	 * At the same time serdev_ready should be 0. It is safe to check it
+	 * without taking the lock and exit allowing fsm_entry_enumerate() to
+	 * close and open serdev without deadlocking.
+	 */
+
+	if (atomic_read(&data->serdev_ready) == 0) {
+		dev_warn(data->dev, "%s: serdev is not ready, dropping data\n", __func__);
+		return count;
 	}
 
-	dev_dbg(data->dev, "%s: %d bytes data received.\n", __func__, count);
+	mutex_lock(&data->lock);
 
-	for (i = 0; i < count; ++i) {
-		snprintf(str_buf + (i * 2), 3, "%02hhx", buf[i]);
-	}
-	dev_dbg(data->dev, "%s: complete recv msg: %s\n", __func__, str_buf);
-	kfree(str_buf);
+	dev_dbg(data->dev, "%s: %d bytes data received.\n", __func__, count);
 
 	/* For debugging from user-space */
 	if (count <= ONE_WIRE_MAX_TX_MSG_SIZE) {
@@ -544,7 +546,8 @@ static int rm_pogo_probe(struct serdev_device *serdev)
 
 	pdata->dev = dev;
 	pdata->serdev = serdev;
-	pdata->serdev_ready = false;
+	atomic_set(&pdata->serdev_ready, 0);
+	pdata->serdev_open = false;
 	mutex_init(&pdata->lock);
 
 	serdev_device_set_drvdata(serdev, pdata);