PPP, PPPoS, replaced static sio_write() calls to a user defined callback

The overall lwIP design on data flows (netif,udp,tcp) is to use a user
defined callback to get data from stack and a static function to send
data to stack, which makes perfect sense. The SIO port was an exception,
the PPP stack never really used the SIO port by only using the
sio_send() function (and the ignominious sio_read_abort() function a
while back).

The way the SIO port is currently designed adds a tight coupling between
the lwIP port and the user code if the user need to do specific user
code if the current uart used is the PPPoS uart, which is not nice,
especially because all the lwIP stack is quite clean at this subject.

While we are at stabilizing the PPP API, change this behavior before
it's too late by replacing the static sio_write() calls to a user
defined callback.

doc/ppp.txt

@@ -183,17 +183,30 @@ static void status_cb(ppp_pcb *pcb, int err_code, void *ctx) {
 
 #include "netif/ppp/pppos.h"
 
+/*
+ * PPPoS serial output callback
+ *
+ * ppp_pcb, PPP control block
+ * data, buffer to write to serial port
+ * len, length of the data buffer
+ * ctx, optional user-provided callback context pointer
+ *
+ * Return value: len if write succeed
+ */
+static u32_t output_cb(ppp_pcb *pcb, u8_t *data, u32_t len, void *ctx) {
+  return uart_write(UART, data, len);
+}
+
 /*
  * Create a new PPPoS interface
  *
  * ppp_netif, netif to use for this PPP link, i.e. PPP IP interface
- * ppp_sio, handler to your SIO port (see include/lwip/sio.h)
+ * output_cb, PPPoS serial output callback
  * status_cb, PPP status callback, called on PPP status change (up, down, …)
  * ctx_cb, optional user-provided callback context pointer
  */
 ppp = pppos_create(&ppp_netif,
-       ppp_sio,
+       output_cb, status_cb, ctx_cb);
-       status_cb, ctx_cb);
 
 
 /*
@@ -304,7 +317,6 @@ ppp_free(ppp);
 3 PPPoS input path (raw API, IRQ safe API, TCPIP API)
 =====================================================
 
-PPPoS requires a serial I/O SIO port (see include/lwip/sio.h).
 Received data on serial port should be sent to lwIP using the pppos_input()
 function or the pppos_input_sys() function.
 
@@ -405,3 +417,5 @@ from previous lwIP version is pretty easy:
   unconditionally registering them, which is probably the wanted behavior in
   almost all cases. If you need it conditional contact us and we will made it
   conditional.
+
+* PPPoS now requires a serial output callback

src/api/pppapi.c

@@ -127,7 +127,7 @@ pppapi_set_notify_phase_callback(ppp_pcb *pcb, ppp_notify_phase_cb_fn notify_pha
 static void
 pppapi_do_pppos_create(struct pppapi_msg_msg *msg)
 {
-  msg->ppp = pppos_create(msg->msg.serialcreate.pppif, msg->msg.serialcreate.fd,
+  msg->ppp = pppos_create(msg->msg.serialcreate.pppif, msg->msg.serialcreate.output_cb,
     msg->msg.serialcreate.link_status_cb, msg->msg.serialcreate.ctx_cb);
   TCPIP_PPPAPI_ACK(msg);
 }
@@ -137,13 +137,13 @@ pppapi_do_pppos_create(struct pppapi_msg_msg *msg)
  * tcpip_thread context.
  */
 ppp_pcb*
-pppapi_pppos_create(struct netif *pppif, sio_fd_t fd, ppp_link_status_cb_fn link_status_cb,
+pppapi_pppos_create(struct netif *pppif, pppos_output_cb_fn output_cb,
-                          void *ctx_cb)
+               ppp_link_status_cb_fn link_status_cb, void *ctx_cb)
 {
   struct pppapi_msg msg;
   msg.function = pppapi_do_pppos_create;
   msg.msg.msg.serialcreate.pppif = pppif;
-  msg.msg.msg.serialcreate.fd = fd;
+  msg.msg.msg.serialcreate.output_cb = output_cb;
   msg.msg.msg.serialcreate.link_status_cb = link_status_cb;
   msg.msg.msg.serialcreate.ctx_cb = ctx_cb;
   TCPIP_PPPAPI(&msg);

src/include/lwip/pppapi.h

@@ -35,6 +35,9 @@
 #include "lwip/sys.h"
 #include "lwip/netif.h"
 #include "netif/ppp/ppp.h"
+#if PPPOS_SUPPORT
+#include "netif/ppp/pppos.h"
+#endif /* PPPOS_SUPPORT */
 
 #ifdef __cplusplus
 extern "C" {
@@ -60,7 +63,7 @@ struct pppapi_msg_msg {
 #if PPPOS_SUPPORT
     struct {
       struct netif *pppif;
-      sio_fd_t fd;
+      pppos_output_cb_fn output_cb;
       ppp_link_status_cb_fn link_status_cb;
       void *ctx_cb;
     } serialcreate;
@@ -119,7 +122,7 @@ void pppapi_set_auth(ppp_pcb *pcb, u8_t authtype, const char *user, const char *
 void pppapi_set_notify_phase_callback(ppp_pcb *pcb, ppp_notify_phase_cb_fn notify_phase_cb);
 #endif /* PPP_NOTIFY_PHASE */
 #if PPPOS_SUPPORT
-ppp_pcb *pppapi_pppos_create(struct netif *pppif, sio_fd_t fd, ppp_link_status_cb_fn link_status_cb, void *ctx_cb);
+ppp_pcb *pppapi_pppos_create(struct netif *pppif, pppos_output_cb_fn output_cb, ppp_link_status_cb_fn link_status_cb, void *ctx_cb);
 #endif /* PPPOS_SUPPORT */
 #if PPPOE_SUPPORT
 ppp_pcb *pppapi_pppoe_create(struct netif *pppif, struct netif *ethif, const char *service_name,

src/include/netif/ppp/pppos.h

@@ -55,6 +55,9 @@ enum {
   PDDATA       /* Process data byte. */
 };
 
+/* PPPoS serial output callback function prototype */
+typedef u32_t (*pppos_output_cb_fn)(ppp_pcb *pcb, u8_t *data, u32_t len, void *ctx);
+
 /*
  * Extended asyncmap - allows any character to be escaped.
  */
@@ -67,7 +70,7 @@ typedef struct pppos_pcb_s pppos_pcb;
 struct pppos_pcb_s {
   /* -- below are data that will NOT be cleared between two sessions */
   ppp_pcb *ppp;                    /* PPP PCB */
-  sio_fd_t fd;                     /* File device ID of port. */
+  pppos_output_cb_fn output_cb;    /* PPP serial output callback */
 
   /* -- below are data that will be cleared between two sessions
    *
@@ -92,7 +95,7 @@ struct pppos_pcb_s {
 };
 
 /* Create a new PPPoS session. */
-ppp_pcb *pppos_create(struct netif *pppif, sio_fd_t fd,
+ppp_pcb *pppos_create(struct netif *pppif, pppos_output_cb_fn output_cb,
        ppp_link_status_cb_fn link_status_cb, void *ctx_cb);
 
 #if !NO_SYS && !PPP_INPROC_IRQ_SAFE

src/netif/ppp/pppos.c

@@ -44,7 +44,6 @@
 #include "lwip/snmp_mib2.h"
 #include "lwip/tcpip.h"
 #include "lwip/api.h"
-#include "lwip/sio.h"
 #include "lwip/ip4.h" /* for ip4_input() */
 
 #include "netif/ppp/ppp_impl.h"
@@ -170,7 +169,7 @@ ppp_get_fcs(u8_t byte)
  *
  * Return 0 on success, an error code on failure.
  */
-ppp_pcb *pppos_create(struct netif *pppif, sio_fd_t fd,
+ppp_pcb *pppos_create(struct netif *pppif, pppos_output_cb_fn output_cb,
        ppp_link_status_cb_fn link_status_cb, void *ctx_cb)
 {
   pppos_pcb *pppos;
@@ -188,7 +187,7 @@ ppp_pcb *pppos_create(struct netif *pppif, sio_fd_t fd,
   }
 
   pppos->ppp = ppp;
-  pppos->fd = fd;
+  pppos->output_cb = output_cb;
   ppp_link_set_callbacks(ppp, &pppos_callbacks, pppos);
   return ppp;
 }
@@ -786,21 +785,10 @@ pppos_recv_config(ppp_pcb *ppp, void *ctx, u32_t accm, int pcomp, int accomp)
 static err_t
 pppos_ioctl(ppp_pcb *pcb, void *ctx, int cmd, void *arg)
 {
-  pppos_pcb *pppos = (pppos_pcb *)ctx;
   LWIP_UNUSED_ARG(pcb);
-
+  LWIP_UNUSED_ARG(ctx);
-  switch(cmd) {
+  LWIP_UNUSED_ARG(cmd);
-    case PPPCTLG_FD:            /* Get the fd associated with the ppp */
+  LWIP_UNUSED_ARG(arg);
-      if (!arg) {
-        goto fail;
-      }
-      *(sio_fd_t *)arg = pppos->fd;
-      return ERR_OK;
-
-    default: ;
-  }
-
-fail:
   return ERR_VAL;
 }
 
@@ -858,7 +846,7 @@ pppos_output_append(pppos_pcb *pppos, err_t err, struct pbuf *nb, u8_t c, u8_t a
    * Sure we don't quite fill the buffer if the character doesn't
    * get escaped but is one character worth complicating this? */
   if ((PBUF_POOL_BUFSIZE - nb->len) < 2) {
-    u32_t l = sio_write(pppos->fd, (u8_t*)nb->payload, nb->len);
+    u32_t l = pppos->output_cb(pppos->ppp, (u8_t*)nb->payload, nb->len, pppos->ppp->ctx_cb);
     if (l != nb->len) {
       return ERR_IF;
     }
@@ -884,9 +872,7 @@ pppos_output_append(pppos_pcb *pppos, err_t err, struct pbuf *nb, u8_t c, u8_t a
 static err_t
 pppos_output_last(pppos_pcb *pppos, err_t err, struct pbuf *nb, u16_t *fcs)
 {
-#if MIB2_STATS
   ppp_pcb *ppp = pppos->ppp;
-#endif /* MIB2_STATS */
 
   /* Add FCS and trailing flag. */
   err = pppos_output_append(pppos, err,  nb, ~(*fcs) & 0xFF, 1, NULL);
@@ -899,7 +885,7 @@ pppos_output_last(pppos_pcb *pppos, err_t err, struct pbuf *nb, u16_t *fcs)
 
   /* Send remaining buffer if not empty */
   if (nb->len > 0) {
-    u32_t l = sio_write(pppos->fd, (u8_t*)nb->payload, nb->len);
+    u32_t l = pppos->output_cb(ppp, (u8_t*)nb->payload, nb->len, ppp->ctx_cb);
     if (l != nb->len) {
       err = ERR_IF;
       goto failed;