michaelh/zephyr
1
0
Fork 0
Code Releases Activity

drivers: modem: gsm_ppp: lock modem when required

Browse source 
If other threads are accessing the modem concurrently during gsm_ppp
initialization, they can easily corrupt UART comms. Normally this is not
noticable since those services are usually started only after modem
setup has been completed. But with the new gsm_ppp stop/start
functionality, the need of synchronizing concurrent access in a
structured way has arisen.

This commit ensures modem_cmd_handler is locked when concurrent access
to the modem UART would interfere with the driver, or otherwise cause
problems. Since the semaphore is not available during this period, all
essential calls to modem_cmd_send has been replaced with the non-locking
equivalents.

Signed-off-by: Benjamin Lindqvist <benjamin.lindqvist@endian.se>
This commit is contained in:
Benjamin Lindqvist 2020-10-09 13:08:28 +02:00 • committed by Jukka Rissanen
commit e7df25c848
1 changed files with 53 additions and 46 deletions
Download patch file Download diff file Expand all files Collapse all files

99
drivers/modem/gsm_ppp.c
View file

@ -289,21 +289,21 @@ static int gsm_setup_mccmno(struct gsm_modem *gsm)
if (CONFIG_MODEM_GSM_MANUAL_MCCMNO[0]) { if (CONFIG_MODEM_GSM_MANUAL_MCCMNO[0]) {
/* use manual MCC/MNO entry */ /* use manual MCC/MNO entry */
ret = modem_cmd_send(&gsm->context.iface, ret = modem_cmd_send_nolock(&gsm->context.iface,
&gsm->context.cmd_handler, &gsm->context.cmd_handler,
NULL, 0, NULL, 0,
"AT+COPS=1,2,\"" "AT+COPS=1,2,\""
CONFIG_MODEM_GSM_MANUAL_MCCMNO CONFIG_MODEM_GSM_MANUAL_MCCMNO
"\"", "\"",
&gsm->sem_response, &gsm->sem_response,
GSM_CMD_AT_TIMEOUT); GSM_CMD_AT_TIMEOUT);
} else { } else {
/* register operator automatically */ /* register operator automatically */
ret = modem_cmd_send(&gsm->context.iface, ret = modem_cmd_send_nolock(&gsm->context.iface,
&gsm->context.cmd_handler, &gsm->context.cmd_handler,
NULL, 0, "AT+COPS=0,0", NULL, 0, "AT+COPS=0,0",
&gsm->sem_response, &gsm->sem_response,
GSM_CMD_AT_TIMEOUT); GSM_CMD_AT_TIMEOUT);
} }
if (ret < 0) { if (ret < 0) {
@ -353,12 +353,12 @@ static void gsm_finalize_connection(struct gsm_modem *gsm)
int ret; int ret;
if (IS_ENABLED(CONFIG_GSM_MUX) && gsm->mux_enabled) { if (IS_ENABLED(CONFIG_GSM_MUX) && gsm->mux_enabled) {
ret = modem_cmd_send(&gsm->context.iface, ret = modem_cmd_send_nolock(&gsm->context.iface,
&gsm->context.cmd_handler, &gsm->context.cmd_handler,
&response_cmds[0], &response_cmds[0],
ARRAY_SIZE(response_cmds), ARRAY_SIZE(response_cmds),
"AT", &gsm->sem_response, "AT", &gsm->sem_response,
GSM_CMD_AT_TIMEOUT); GSM_CMD_AT_TIMEOUT);
if (ret < 0) { if (ret < 0) {
LOG_ERR("modem setup returned %d, %s", LOG_ERR("modem setup returned %d, %s",
ret, "retrying..."); ret, "retrying...");
@ -370,12 +370,12 @@ static void gsm_finalize_connection(struct gsm_modem *gsm)
(void)gsm_setup_mccmno(gsm); (void)gsm_setup_mccmno(gsm);
ret = modem_cmd_handler_setup_cmds(&gsm->context.iface, ret = modem_cmd_handler_setup_cmds_nolock(&gsm->context.iface,
&gsm->context.cmd_handler, &gsm->context.cmd_handler,
setup_cmds, setup_cmds,
ARRAY_SIZE(setup_cmds), ARRAY_SIZE(setup_cmds),
&gsm->sem_response, &gsm->sem_response,
GSM_CMD_SETUP_TIMEOUT); GSM_CMD_SETUP_TIMEOUT);
if (ret < 0) { if (ret < 0) {
LOG_DBG("modem setup returned %d, %s", LOG_DBG("modem setup returned %d, %s",
ret, "retrying..."); ret, "retrying...");
@ -401,12 +401,12 @@ static void gsm_finalize_connection(struct gsm_modem *gsm)
LOG_DBG("modem setup returned %d, %s", ret, "enable PPP"); LOG_DBG("modem setup returned %d, %s", ret, "enable PPP");
ret = modem_cmd_handler_setup_cmds(&gsm->context.iface, ret = modem_cmd_handler_setup_cmds_nolock(&gsm->context.iface,
&gsm->context.cmd_handler, &gsm->context.cmd_handler,
connect_cmds, connect_cmds,
ARRAY_SIZE(connect_cmds), ARRAY_SIZE(connect_cmds),
&gsm->sem_response, &gsm->sem_response,
GSM_CMD_SETUP_TIMEOUT); GSM_CMD_SETUP_TIMEOUT);
if (ret < 0) { if (ret < 0) {
LOG_DBG("modem setup returned %d, %s", LOG_DBG("modem setup returned %d, %s",
ret, "retrying..."); ret, "retrying...");
@ -427,20 +427,22 @@ static void gsm_finalize_connection(struct gsm_modem *gsm)
LOG_DBG("iface %suart error %d", "AT ", ret); LOG_DBG("iface %suart error %d", "AT ", ret);
} else { } else {
/* Do a test and try to send AT command to modem */ /* Do a test and try to send AT command to modem */
ret = modem_cmd_send(&gsm->context.iface, ret = modem_cmd_send_nolock(
&gsm->context.cmd_handler, &gsm->context.iface,
&response_cmds[0], &gsm->context.cmd_handler,
ARRAY_SIZE(response_cmds), &response_cmds[0],
"AT", &gsm->sem_response, ARRAY_SIZE(response_cmds),
GSM_CMD_AT_TIMEOUT); "AT", &gsm->sem_response,
GSM_CMD_AT_TIMEOUT);
if (ret < 0) { if (ret < 0) {
LOG_DBG("modem setup returned %d, %s", LOG_WRN("modem setup returned %d, %s",
ret, "AT cmds failed"); ret, "AT cmds failed");
} else { } else {
LOG_INF("AT channel %d connected to %s", LOG_INF("AT channel %d connected to %s",
DLCI_AT, gsm->at_dev->name); DLCI_AT, gsm->at_dev->name);
} }
} }
modem_cmd_handler_tx_unlock(&gsm->context.cmd_handler);
} }
} }
@ -450,7 +452,7 @@ static int mux_enable(struct gsm_modem *gsm)
/* Turn on muxing */ /* Turn on muxing */
if (IS_ENABLED(CONFIG_MODEM_GSM_SIMCOM)) { if (IS_ENABLED(CONFIG_MODEM_GSM_SIMCOM)) {
ret = modem_cmd_send( ret = modem_cmd_send_nolock(
&gsm->context.iface, &gsm->context.iface,
&gsm->context.cmd_handler, &gsm->context.cmd_handler,
&response_cmds[0], &response_cmds[0],
@ -473,7 +475,7 @@ static int mux_enable(struct gsm_modem *gsm)
GSM_CMD_AT_TIMEOUT); GSM_CMD_AT_TIMEOUT);
} else { } else {
/* Generic GSM modem */ /* Generic GSM modem */
ret = modem_cmd_send(&gsm->context.iface, ret = modem_cmd_send_nolock(&gsm->context.iface,
&gsm->context.cmd_handler, &gsm->context.cmd_handler,
&response_cmds[0], &response_cmds[0],
ARRAY_SIZE(response_cmds), ARRAY_SIZE(response_cmds),
@ -632,12 +634,12 @@ static void gsm_configure(struct k_work *work)
LOG_DBG("Starting modem %p configuration", gsm); LOG_DBG("Starting modem %p configuration", gsm);
ret = modem_cmd_send(&gsm->context.iface, ret = modem_cmd_send_nolock(&gsm->context.iface,
&gsm->context.cmd_handler, &gsm->context.cmd_handler,
&response_cmds[0], &response_cmds[0],
ARRAY_SIZE(response_cmds), ARRAY_SIZE(response_cmds),
"AT", &gsm->sem_response, "AT", &gsm->sem_response,
GSM_CMD_AT_TIMEOUT); GSM_CMD_AT_TIMEOUT);
if (ret < 0) { if (ret < 0) {
LOG_DBG("modem not ready %d", ret); LOG_DBG("modem not ready %d", ret);
@ -710,6 +712,11 @@ void gsm_ppp_stop(const struct device *device)
uart_mux_disable(gsm->ppp_dev); uart_mux_disable(gsm->ppp_dev);
} }
} }
if (modem_cmd_handler_tx_lock(&gsm->context.cmd_handler,
K_SECONDS(10))) {
LOG_WRN("Failed locking modem cmds!");
}
} }
static int gsm_init(const struct device *device) static int gsm_init(const struct device *device)