michaelh/zephyr
1
0
Fork 0
Code Releases Activity

drivers: modem: cmd_handler: rework reading from interface

Browse source 
So far a dedicated buffer was used for data read from modem
interface. New net_bufs were allocated and filled later, which means
that data was lost when no more net_bufs were available in the pool.

Prevent data loss by allocating net_buf before attempting any read on
modem interface. Process incoming data in a loop as long as reading from
interface results in new data. Also remove dedicated buffer
(data->read_buf) and directly fill net_buf content instead. As a side
effect there are less memory copy operations and RAM usage is reduced.

Pre-allocated net_buf is now always appended to data->rx_buf. When there
was no (more) data read from interface to such net_buf, then this empty
net_buf will be on the end of data->rx_buf fragment list. Update
skipcrlf() and findcrlf() implementations to explicitly check for each
net_buf length, instead of blindly assuming them to have at least single
byte.

Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
This commit is contained in:
Marcin Niestroj 2020-10-26 18:46:21 +01:00 • committed by Maureen Helm
commit 318cbe649a
6 changed files with 48 additions and 46 deletions
Download patch file Download diff file Expand all files Collapse all files

3
drivers/modem/gsm_ppp.c
View file

@ -46,7 +46,6 @@ static struct gsm_modem {
struct modem_context context;
struct modem_cmd_handler_data cmd_handler_data;
uint8_t cmd_read_buf[GSM_CMD_READ_BUF];
uint8_t cmd_match_buf[GSM_CMD_READ_BUF];
struct k_sem sem_response;
@ -728,8 +727,6 @@ static int gsm_init(const struct device *device)
gsm->cmd_handler_data.cmds[CMD_RESP] = response_cmds;
gsm->cmd_handler_data.cmds_len[CMD_RESP] = ARRAY_SIZE(response_cmds);
gsm->cmd_handler_data.read_buf = &gsm->cmd_read_buf[0];
gsm->cmd_handler_data.read_buf_len = sizeof(gsm->cmd_read_buf);
gsm->cmd_handler_data.match_buf = &gsm->cmd_match_buf[0];
gsm->cmd_handler_data.match_buf_len = sizeof(gsm->cmd_match_buf);
gsm->cmd_handler_data.buf_pool = &gsm_recv_pool;

74
drivers/modem/modem_cmd_handler.c
View file

@ -36,7 +36,8 @@ static bool is_crlf(uint8_t c)
static void skipcrlf(struct modem_cmd_handler_data *data)
{
while (data->rx_buf && is_crlf(*data->rx_buf->data)) {
while (data->rx_buf && data->rx_buf->len &&
is_crlf(*data->rx_buf->data)) {
net_buf_pull_u8(data->rx_buf);
if (!data->rx_buf->len) {
data->rx_buf = net_buf_frag_del(NULL, data->rx_buf);
@ -50,7 +51,7 @@ static uint16_t findcrlf(struct modem_cmd_handler_data *data,
struct net_buf *buf = data->rx_buf;
uint16_t len = 0U, pos = 0U;
while (buf && !is_crlf(*(buf->data + pos))) {
while (buf && buf->len && !is_crlf(*(buf->data + pos))) {
if (pos + 1 >= buf->len) {
len += buf->len;
buf = buf->frags;
@ -60,7 +61,7 @@ static uint16_t findcrlf(struct modem_cmd_handler_data *data,
}
}
if (buf && is_crlf(*(buf->data + pos))) {
if (buf && buf->len && is_crlf(*(buf->data + pos))) {
len += pos;
*offset = pos;
*frag = buf;
@ -256,41 +257,51 @@ static struct modem_cmd *find_cmd_direct_match(
return NULL;
}
static void cmd_handler_process_iface_data(struct modem_cmd_handler_data *data,
static int cmd_handler_process_iface_data(struct modem_cmd_handler_data *data,
struct modem_iface *iface)
{
size_t rx_len, bytes_read = 0;
struct net_buf *last;
size_t bytes_read = 0;
int ret;
/* read all of the data from modem iface */
while (true) {
ret = iface->read(iface, data->read_buf,
data->read_buf_len, &bytes_read);
if (ret < 0 || bytes_read == 0) {
/* modem context buffer is empty */
break;
}
/* make sure we have storage */
if (!data->rx_buf) {
data->rx_buf = net_buf_alloc(data->buf_pool,
data->alloc_timeout);
if (!data->rx_buf) {
LOG_ERR("Can't allocate RX data! "
"Skipping data!");
break;
/* there is potentially more data waiting */
return -ENOMEM;
}
}
rx_len = net_buf_append_bytes(data->rx_buf, bytes_read,
data->read_buf,
data->alloc_timeout,
read_rx_allocator,
data->buf_pool);
if (rx_len < bytes_read) {
LOG_ERR("Data was lost! read %zu of %zu!",
rx_len, bytes_read);
last = net_buf_frag_last(data->rx_buf);
/* read all of the data from modem iface */
while (true) {
struct net_buf *frag = last;
size_t frag_room = net_buf_tailroom(frag);
if (!frag_room) {
frag = net_buf_alloc(data->buf_pool,
data->alloc_timeout);
if (!frag) {
/* there is potentially more data waiting */
return -ENOMEM;
}
net_buf_frag_insert(last, frag);
last = frag;
frag_room = net_buf_tailroom(frag);
}
ret = iface->read(iface, net_buf_tail(frag), frag_room,
&bytes_read);
if (ret < 0 || bytes_read == 0) {
/* modem context buffer is empty */
return 0;
}
net_buf_add(frag, bytes_read);
}
}
@ -303,9 +314,9 @@ static void cmd_handler_process_rx_buf(struct modem_cmd_handler_data *data)
uint16_t offset, len;
/* process all of the data in the net_buf */
while (data->rx_buf) {
while (data->rx_buf && data->rx_buf->len) {
skipcrlf(data);
if (!data->rx_buf) {
if (!data->rx_buf || !data->rx_buf->len) {
break;
}
@ -402,6 +413,7 @@ static void cmd_handler_process(struct modem_cmd_handler *cmd_handler,
struct modem_iface *iface)
{
struct modem_cmd_handler_data *data;
int err;
if (!cmd_handler || !cmd_handler->cmd_handler_data ||
!iface || !iface->read) {
@ -410,8 +422,10 @@ static void cmd_handler_process(struct modem_cmd_handler *cmd_handler,
data = (struct modem_cmd_handler_data *)(cmd_handler->cmd_handler_data);
cmd_handler_process_iface_data(data, iface);
do {
err = cmd_handler_process_iface_data(data, iface);
cmd_handler_process_rx_buf(data);
} while (err);
}
int modem_cmd_handler_get_error(struct modem_cmd_handler_data *data)
@ -635,7 +649,7 @@ int modem_cmd_handler_init(struct modem_cmd_handler *handler,
return -EINVAL;
}
if (!data->read_buf_len || !data->match_buf_len) {
if (!data->match_buf_len) {
return -EINVAL;
}

3
drivers/modem/modem_cmd_handler.h
View file

@ -80,9 +80,6 @@ struct modem_cmd_handler_data {
struct modem_cmd *cmds[CMD_MAX];
size_t cmds_len[CMD_MAX];
char *read_buf;
size_t read_buf_len;
char *match_buf;
size_t match_buf_len;

3
drivers/modem/ublox-sara-r4.c
View file

@ -136,7 +136,6 @@ struct modem_data {
/* modem cmds */
struct modem_cmd_handler_data cmd_handler_data;
uint8_t cmd_read_buf[MDM_RECV_BUF_SIZE];
uint8_t cmd_match_buf[MDM_RECV_BUF_SIZE + 1];
/* socket data */
@ -1741,8 +1740,6 @@ static int modem_init(const struct device *dev)
mdata.cmd_handler_data.cmds_len[CMD_RESP] = ARRAY_SIZE(response_cmds);
mdata.cmd_handler_data.cmds[CMD_UNSOL] = unsol_cmds;
mdata.cmd_handler_data.cmds_len[CMD_UNSOL] = ARRAY_SIZE(unsol_cmds);
mdata.cmd_handler_data.read_buf = &mdata.cmd_read_buf[0];
mdata.cmd_handler_data.read_buf_len = sizeof(mdata.cmd_read_buf);
mdata.cmd_handler_data.match_buf = &mdata.cmd_match_buf[0];
mdata.cmd_handler_data.match_buf_len = sizeof(mdata.cmd_match_buf);
mdata.cmd_handler_data.buf_pool = &mdm_recv_pool;

2
drivers/wifi/esp/esp.c
View file

@ -878,8 +878,6 @@ static int esp_init(const struct device *dev)
data->cmd_handler_data.cmds_len[CMD_RESP] = ARRAY_SIZE(response_cmds);
data->cmd_handler_data.cmds[CMD_UNSOL] = unsol_cmds;
data->cmd_handler_data.cmds_len[CMD_UNSOL] = ARRAY_SIZE(unsol_cmds);
data->cmd_handler_data.read_buf = &data->cmd_read_buf[0];
data->cmd_handler_data.read_buf_len = sizeof(data->cmd_read_buf);
data->cmd_handler_data.match_buf = &data->cmd_match_buf[0];
data->cmd_handler_data.match_buf_len = sizeof(data->cmd_match_buf);
data->cmd_handler_data.buf_pool = &mdm_recv_pool;

1
drivers/wifi/esp/esp.h
View file

@ -173,7 +173,6 @@ struct esp_data {
/* modem cmds */
struct modem_cmd_handler_data cmd_handler_data;
uint8_t cmd_read_buf[MDM_RECV_BUF_SIZE];
uint8_t cmd_match_buf[MDM_RECV_BUF_SIZE];
/* socket data */