net: ppp: Convert PPP driver to use normal UART APIs

This is needed so that we can support GSM 07.10 muxing protocol.

Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>

drivers/console/gsm_mux.h

@@ -5,8 +5,14 @@
  */
 
 #define DLCI_CONTROL 0
+
+#if IS_ENABLED(CONFIG_GSM_MUX)
 #define DLCI_AT      CONFIG_GSM_MUX_DLCI_AT
 #define DLCI_PPP     CONFIG_GSM_MUX_DLCI_PPP
+#else
+#define DLCI_AT      -1
+#define DLCI_PPP     -1
+#endif
 
 struct gsm_mux;
 struct gsm_dlci;

drivers/modem/Kconfig.gsm

@@ -32,6 +32,12 @@ endchoice
 config MODEM_GSM_UART_NAME
 	string "UART device name the modem is connected to"
 
+config MODEM_GSM_RX_STACK_SIZE
+	int "Size of the stack allocated for receiving data from modem"
+	default 512
+	help
+	  Sets the stack size which will be used by the GSM RX thread.
+
 config MODEM_GSM_INIT_PRIORITY
 	int "Init priority for the GSM modem driver"
 	default 42

drivers/modem/gsm_ppp.c

@@ -13,7 +13,6 @@ LOG_MODULE_REGISTER(modem_gsm, CONFIG_MODEM_LOG_LEVEL);
 #include <sys/util.h>
 #include <net/ppp.h>
 #include <drivers/uart.h>
-#include <drivers/console/uart_pipe.h>
 #include <drivers/console/uart_mux.h>
 
 #include "modem_context.h"
@@ -24,7 +23,7 @@ LOG_MODULE_REGISTER(modem_gsm, CONFIG_MODEM_LOG_LEVEL);
 #define GSM_CMD_READ_BUF       128
 #define GSM_CMD_AT_TIMEOUT     K_SECONDS(2)
 #define GSM_CMD_SETUP_TIMEOUT  K_SECONDS(6)
-#define GSM_RX_STACK_SIZE      1024
+#define GSM_RX_STACK_SIZE      CONFIG_MODEM_GSM_RX_STACK_SIZE
 #define GSM_RECV_MAX_BUF       30
 #define GSM_RECV_BUF_SIZE      128
 #define GSM_BUF_ALLOC_TIMEOUT  K_SECONDS(1)
@@ -56,8 +55,6 @@ static struct gsm_modem {
 
 	u8_t *ppp_recv_buf;
 	size_t ppp_recv_buf_len;
-	uart_pipe_recv_cb ppp_recv_cb;
-	struct k_sem ppp_send_sem;
 
 	enum setup_state state;
 	struct device *ppp_dev;
@@ -69,8 +66,6 @@ static struct gsm_modem {
 	bool setup_done : 1;
 } gsm;
 
-static size_t recv_buf_offset;
-
 NET_BUF_POOL_DEFINE(gsm_recv_pool, GSM_RECV_MAX_BUF, GSM_RECV_BUF_SIZE,
 		    0, NULL);
 K_THREAD_STACK_DEFINE(gsm_rx_stack, GSM_RX_STACK_SIZE);
@@ -79,47 +74,14 @@ struct k_thread gsm_rx_thread;
 
 static void gsm_rx(struct gsm_modem *gsm)
 {
-	int bytes, r;
-
 	LOG_DBG("starting");
 
 	while (true) {
 		k_sem_take(&gsm->gsm_data.rx_sem, K_FOREVER);
 
-		if (gsm->mux_enabled == false) {
+		/* The handler will listen AT channel */
-			if (gsm->setup_done == false) {
+		gsm->context.cmd_handler.process(&gsm->context.cmd_handler,
-				gsm->context.cmd_handler.process(
+						 &gsm->context.iface);
-						&gsm->context.cmd_handler,
-						&gsm->context.iface);
-				continue;
-			}
-
-			if (gsm->ppp_recv_cb == NULL ||
-			    gsm->ppp_recv_buf == NULL ||
-			    gsm->ppp_recv_buf_len == 0) {
-				return;
-			}
-
-			r = gsm->context.iface.read(
-					&gsm->context.iface,
-					&gsm->ppp_recv_buf[recv_buf_offset],
-					gsm->ppp_recv_buf_len -
-					recv_buf_offset,
-					&bytes);
-			if (r < 0 || bytes == 0) {
-				continue;
-			}
-
-			recv_buf_offset += bytes;
-
-			gsm->ppp_recv_buf = gsm->ppp_recv_cb(gsm->ppp_recv_buf,
-							     &recv_buf_offset);
-		} else if (IS_ENABLED(CONFIG_GSM_MUX) && gsm->mux_enabled) {
-			/* The handler will listen AT channel */
-			gsm->context.cmd_handler.process(
-						&gsm->context.cmd_handler,
-						&gsm->context.iface);
-		}
 	}
 }
 
@@ -282,20 +244,15 @@ static int gsm_setup_mccmno(struct gsm_modem *gsm)
 static void set_ppp_carrier_on(struct gsm_modem *gsm)
 {
 	struct device *ppp_dev = device_get_binding(CONFIG_NET_PPP_DRV_NAME);
-	struct net_if *iface;
+	const struct ppp_api *api =
+				(const struct ppp_api *)ppp_dev->driver_api;
 
 	if (!ppp_dev) {
 		LOG_ERR("Cannot find PPP %s!", "device");
 		return;
 	}
 
-	iface = net_if_lookup_by_dev(ppp_dev);
+	api->start(ppp_dev);
-	if (!iface) {
-		LOG_ERR("Cannot find PPP %s!", "network interface");
-		return;
-	}
-
-	net_ppp_carrier_on(iface);
 }
 
 static void gsm_finalize_connection(struct gsm_modem *gsm)
@@ -352,11 +309,12 @@ static void gsm_finalize_connection(struct gsm_modem *gsm)
 
 	gsm->setup_done = true;
 
-	/* FIXME: This will let PPP to start send data. We should actually
+	/* If we are not muxing, the modem interface and gsm_rx() thread is not
-	 * change this so that the PPP L2 is initialized after the GSM modem
+	 * needed as PPP will handle the incoming traffic internally.
-	 * is working and connection is created. TBDL.
 	 */
-	k_sem_give(&gsm->ppp_send_sem);
+	if (!IS_ENABLED(CONFIG_GSM_MUX)) {
+		k_thread_abort(&gsm_rx_thread);
+	}
 
 	set_ppp_carrier_on(gsm);
 }
@@ -576,8 +534,6 @@ static int gsm_init(struct device *device)
 
 	LOG_DBG("Generic GSM modem (%p)", gsm);
 
-	k_sem_init(&gsm->ppp_send_sem, 0, 1);
-
 	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];
@@ -630,6 +586,7 @@ static int gsm_init(struct device *device)
 			K_THREAD_STACK_SIZEOF(gsm_rx_stack),
 			(k_thread_entry_t) gsm_rx,
 			gsm, NULL, NULL, K_PRIO_COOP(7), 0, K_NO_WAIT);
+	k_thread_name_set(&gsm_rx_thread, "gsm_rx");
 
 	k_delayed_work_init(&gsm->gsm_configure_work, gsm_configure);
 
@@ -638,25 +595,5 @@ static int gsm_init(struct device *device)
 	return 0;
 }
 
-int uart_pipe_send(const u8_t *buf, int len)
-{
-	k_sem_take(&gsm.ppp_send_sem, K_FOREVER);
-
-	(void)gsm.context.iface.write(&gsm.context.iface, buf, len);
-
-	k_sem_give(&gsm.ppp_send_sem);
-
-	return 0;
-}
-
-
-/* Setup the connection to PPP. PPP driver will call this function. */
-void uart_pipe_register(u8_t *buf, size_t len, uart_pipe_recv_cb cb)
-{
-	gsm.ppp_recv_buf = buf;
-	gsm.ppp_recv_buf_len = len;
-	gsm.ppp_recv_cb = cb;
-}
-
 DEVICE_INIT(gsm_ppp, "modem_gsm", gsm_init, &gsm, NULL, POST_KERNEL,
 	    CONFIG_MODEM_GSM_INIT_PRIORITY);

drivers/net/Kconfig

@@ -8,24 +8,34 @@ menuconfig NET_PPP
 	bool "Point-to-point (PPP) UART based driver"
 	depends on NET_L2_PPP
 	depends on NET_NATIVE
-	select UART_PIPE if ! MODEM_GSM_PPP
+	select UART_MUX if GSM_MUX
-	select UART_INTERRUPT_DRIVEN
 
 if NET_PPP
 
+config NET_PPP_UART_NAME
+	string "UART device name the PPP is connected to"
+	depends on !MODEM_GSM_PPP
+
 config NET_PPP_DRV_NAME
 	string "PPP Driver name"
 	default "ppp"
 	help
 	  This option sets the driver name
 
-config NET_PPP_UART_PIPE_BUF_LEN
+config NET_PPP_UART_BUF_LEN
 	int "Buffer length when reading from UART"
 	default 8
 	help
-	  This options sets the size of the UART pipe buffer where data
+	  This options sets the size of the UART buffer where data
 	  is being read to.
 
+config NET_PPP_RINGBUF_SIZE
+	int "PPP ring buffer size"
+	default 256
+	help
+	  PPP ring buffer size when passing data from RX ISR to worker
+	  thread that will pass the data to IP stack.
+
 config NET_PPP_VERIFY_FCS
 	bool "Verify that received FCS is valid"
 	default y

drivers/net/ppp.c

@@ -27,13 +27,15 @@ LOG_MODULE_REGISTER(net_ppp, LOG_LEVEL);
 #include <net/net_pkt.h>
 #include <net/net_if.h>
 #include <net/net_core.h>
-#include <drivers/console/uart_pipe.h>
+#include <sys/ring_buffer.h>
 #include <sys/crc.h>
+#include <drivers/uart.h>
+#include <drivers/console/uart_mux.h>
 
 #include "../../subsys/net/ip/net_stats.h"
 #include "../../subsys/net/ip/net_private.h"
 
-#define UART_BUF_LEN CONFIG_NET_PPP_UART_PIPE_BUF_LEN
+#define UART_BUF_LEN CONFIG_NET_PPP_UART_BUF_LEN
 
 enum ppp_driver_state {
 	STATE_HDLC_FRAME_START,
@@ -42,6 +44,7 @@ enum ppp_driver_state {
 };
 
 struct ppp_driver_context {
+	struct device *dev;
 	struct net_if *iface;
 
 	/* This net_pkt contains pkt that is being read */
@@ -59,10 +62,19 @@ struct ppp_driver_context {
 	u8_t mac_addr[6];
 	struct net_linkaddr ll_addr;
 
+	/* Flag that tells whether this instance is initialized or not */
+	atomic_t modem_init_done;
+
+	/* Incoming data is routed via ring buffer */
+	struct ring_buf rx_ringbuf;
+	u8_t rx_buf[CONFIG_NET_PPP_RINGBUF_SIZE];
+
+	/* ISR function callback worker */
+	struct k_work cb_work;
+
 #if defined(CONFIG_NET_STATISTICS_PPP)
 	struct net_stats_ppp stats;
 #endif
-
 	enum ppp_driver_state state;
 
 #if defined(CONFIG_PPP_CLIENT_CLIENTSERVER)
@@ -176,7 +188,11 @@ static void ppp_change_state(struct ppp_driver_context *ctx,
 static int ppp_send_flush(struct ppp_driver_context *ppp, int off)
 {
 	if (!IS_ENABLED(CONFIG_NET_TEST)) {
-		uart_pipe_send(ppp->send_buf, off);
+		u8_t *buf = ppp->send_buf;
+
+		while (off--) {
+			uart_poll_out(ppp->dev, *buf++);
+		}
 	}
 
 	return 0;
@@ -401,6 +417,7 @@ static void ppp_process_msg(struct ppp_driver_context *ppp)
 	ppp->pkt = NULL;
 }
 
+#if defined(CONFIG_NET_TEST)
 static u8_t *ppp_recv_cb(u8_t *buf, size_t *off)
 {
 	struct ppp_driver_context *ppp =
@@ -435,7 +452,6 @@ static u8_t *ppp_recv_cb(u8_t *buf, size_t *off)
 	return buf;
 }
 
-#if defined(CONFIG_NET_TEST)
 void ppp_driver_feed_data(u8_t *data, int data_len)
 {
 	struct ppp_driver_context *ppp = &ppp_driver_context_data;
@@ -609,12 +625,57 @@ static int ppp_send(struct device *dev, struct net_pkt *pkt)
 	return 0;
 }
 
+#if !defined(CONFIG_NET_TEST)
+static void ppp_isr_cb_work(struct k_work *work)
+{
+	struct ppp_driver_context *ppp =
+		CONTAINER_OF(work, struct ppp_driver_context, cb_work);
+	u8_t *data;
+	size_t len, tmp;
+	int ret;
+
+	len = ring_buf_get_claim(&ppp->rx_ringbuf, &data,
+				 CONFIG_NET_PPP_RINGBUF_SIZE);
+	if (len == 0) {
+		LOG_DBG("Ringbuf %p is empty!", &ppp->rx_ringbuf);
+		return;
+	}
+
+	/* This will print too much data, enable only if really needed */
+	if (0) {
+		LOG_HEXDUMP_DBG(data, len, ppp->dev->config->name);
+	}
+
+	tmp = len;
+
+	do {
+		if (ppp_input_byte(ppp, *data++) == 0) {
+			/* Ignore empty or too short frames */
+			if (ppp->pkt && net_pkt_get_len(ppp->pkt) > 3) {
+				ppp_process_msg(ppp);
+				break;
+			}
+		}
+	} while (--tmp);
+
+	ret = ring_buf_get_finish(&ppp->rx_ringbuf, len);
+	if (ret < 0) {
+		LOG_DBG("Cannot flush ring buffer (%d)", ret);
+	}
+}
+#endif /* !CONFIG_NET_TEST */
+
 static int ppp_driver_init(struct device *dev)
 {
 	struct ppp_driver_context *ppp = dev->driver_data;
 
 	LOG_DBG("[%p] dev %p", ppp, dev);
 
+#if !defined(CONFIG_NET_TEST)
+	ring_buf_init(&ppp->rx_ringbuf, sizeof(ppp->rx_buf), ppp->rx_buf);
+	k_work_init(&ppp->cb_work, ppp_isr_cb_work);
+#endif
+
 	ppp->pkt = NULL;
 	ppp_change_state(ppp, STATE_HDLC_FRAME_START);
 #if defined(CONFIG_PPP_CLIENT_CLIENTSERVER)
@@ -674,13 +735,6 @@ use_random_mac:
 
 	memset(ppp->buf, 0, sizeof(ppp->buf));
 
-	/* We do not use uart_pipe for unit tests as the unit test has its
-	 * own handling of UART. See tests/net/ppp/driver for details.
-	 */
-	if (!IS_ENABLED(CONFIG_NET_TEST)) {
-		uart_pipe_register(ppp->buf, sizeof(ppp->buf), ppp_recv_cb);
-	}
-
 	/* If we have a GSM modem with PPP support, then do not start the
 	 * interface automatically but only after the modem is ready.
 	 */
@@ -698,10 +752,97 @@ static struct net_stats_ppp *ppp_get_stats(struct device *dev)
 }
 #endif
 
+#if !defined(CONFIG_NET_TEST)
+static void ppp_uart_flush(struct device *dev)
+{
+	u8_t c;
+
+	while (uart_fifo_read(dev, &c, 1) > 0) {
+		continue;
+	}
+}
+
+static void ppp_uart_isr(void *user_data)
+{
+	struct ppp_driver_context *context = user_data;
+	struct device *uart = context->dev;
+	int rx = 0, ret;
+
+	/* get all of the data off UART as fast as we can */
+	while (uart_irq_update(uart) && uart_irq_rx_ready(uart)) {
+		rx = uart_fifo_read(uart, context->buf, sizeof(context->buf));
+		if (rx <= 0) {
+			continue;
+		}
+
+		ret = ring_buf_put(&context->rx_ringbuf, context->buf, rx);
+		if (ret < rx) {
+			LOG_ERR("Rx buffer doesn't have enough space. "
+				"Bytes pending: %d, written: %d",
+				rx, ret);
+			break;
+		}
+
+		k_work_submit(&context->cb_work);
+	}
+}
+#endif /* !CONFIG_NET_TEST */
+
 static int ppp_start(struct device *dev)
 {
 	struct ppp_driver_context *context = dev->driver_data;
 
+	/* Init the PPP UART only once. This should only be done after
+	 * the GSM muxing is setup and enabled. GSM modem will call this
+	 * after everything is ready to be connected.
+	 */
+#if !defined(CONFIG_NET_TEST)
+	if (atomic_cas(&context->modem_init_done, false, true)) {
+		const char *dev_name = NULL;
+
+		/* Now try to figure out what device to open. If GSM muxing
+		 * is enabled, then use it. If not, then check if modem
+		 * configuration is enabled, and use that. If none are enabled,
+		 * then use our own config.
+		 */
+#if IS_ENABLED(CONFIG_GSM_MUX)
+		struct device *mux;
+
+		mux = uart_mux_find(CONFIG_GSM_MUX_DLCI_PPP);
+		if (mux == NULL) {
+			LOG_ERR("Cannot find GSM mux dev for DLCI %d",
+				CONFIG_GSM_MUX_DLCI_PPP);
+			return -ENOENT;
+		}
+
+		dev_name = mux->config->name;
+#elif IS_ENABLED(CONFIG_MODEM_GSM_PPP)
+		dev_name = CONFIG_MODEM_GSM_UART_NAME;
+#else
+		dev_name = CONFIG_NET_PPP_UART_NAME;
+#endif
+		if (dev_name == NULL || dev_name[0] == '\0') {
+			LOG_ERR("UART configuration is wrong!");
+			return -EINVAL;
+		}
+
+		LOG_DBG("Initializing PPP to use %s", dev_name);
+
+		context->dev = device_get_binding(dev_name);
+		if (!context->dev) {
+			LOG_ERR("Cannot find dev %s", dev_name);
+			return -ENODEV;
+		}
+
+		uart_irq_rx_disable(context->dev);
+		uart_irq_tx_disable(context->dev);
+		ppp_uart_flush(context->dev);
+		uart_irq_callback_user_data_set(context->dev, ppp_uart_isr,
+						context);
+		uart_irq_rx_enable(context->dev);
+	}
+#endif /* !CONFIG_NET_TEST */
+
 	net_ppp_carrier_on(context->iface);
 
 	return 0;