diff --git a/drivers/misc/rm-pogo/pogo.h b/drivers/misc/rm-pogo/pogo.h index f855f42a726a..d30a74a98d98 100644 --- a/drivers/misc/rm-pogo/pogo.h +++ b/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]; diff --git a/drivers/misc/rm-pogo/pogo_attribute_storage.h b/drivers/misc/rm-pogo/pogo_attribute_storage.h index 0c92206ac764..80aab8eb79af 100644 --- a/drivers/misc/rm-pogo/pogo_attribute_storage.h +++ b/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) diff --git a/drivers/misc/rm-pogo/pogo_fsm.c b/drivers/misc/rm-pogo/pogo_fsm.c index 0e538aad8ada..6d51da331e25 100644 --- a/drivers/misc/rm-pogo/pogo_fsm.c +++ b/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); diff --git a/drivers/misc/rm-pogo/pogo_main.c b/drivers/misc/rm-pogo/pogo_main.c index 7d137952dc88..a36403829915 100644 --- a/drivers/misc/rm-pogo/pogo_main.c +++ b/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);