dlizewski commented on code in PR #18815:
URL: https://github.com/apache/nuttx/pull/18815#discussion_r3156710244


##########
drivers/usbhost/usbhost_cdcecm.c:
##########
@@ -0,0 +1,2160 @@
+/****************************************************************************
+ * drivers/usbhost/usbhost_cdcecm.c
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <semaphore.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+#include <poll.h>
+#include <fcntl.h>
+
+#include <arpa/inet.h>
+
+#include <nuttx/arch.h>
+#include <nuttx/irq.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/kthread.h>
+#include <nuttx/mutex.h>
+#include <nuttx/fs/fs.h>
+#include <nuttx/wqueue.h>
+#include <nuttx/signal.h>
+#include <nuttx/net/netdev.h>
+
+#include <nuttx/usb/cdc.h>
+#include <nuttx/usb/usb.h>
+#include <nuttx/usb/usbhost.h>
+
+#ifdef CONFIG_NET_PKT
+  #include <nuttx/net/pkt.h>
+#endif
+
+#ifdef CONFIG_USBHOST_CDCECM
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Default max segment size if not specified by device */
+
+#define CDCECM_DEFAULT_MAXSEG   1514
+
+/* ECM control requests */
+
+#define ECM_SET_PACKET_FILTER_REQUEST 0x43
+
+/* ECM packet filter bits */
+
+#define ECM_PACKET_TYPE_PROMISCUOUS    (1 << 0)
+#define ECM_PACKET_TYPE_ALL_MULTICAST  (1 << 1)
+#define ECM_PACKET_TYPE_DIRECTED       (1 << 2)
+#define ECM_PACKET_TYPE_BROADCAST      (1 << 3)
+#define ECM_PACKET_TYPE_MULTICAST      (1 << 4)
+
+/* Default packet filter: unicast + broadcast */
+
+#define ECM_DEFAULT_PACKET_FILTER      (ECM_PACKET_TYPE_DIRECTED | \
+                                        ECM_PACKET_TYPE_BROADCAST)
+
+/* Configuration ************************************************************/
+
+#ifndef CONFIG_SCHED_WORKQUEUE
+#  warning "Worker thread support is required (CONFIG_SCHED_WORKQUEUE)"
+#endif
+
+#ifndef CONFIG_USBHOST_ASYNCH
+#  warning Asynchronous transfer support is required (CONFIG_USBHOST_ASYNCH)
+#endif
+
+#ifdef CONFIG_USBHOST_CDCECM_NTDELAY
+#  define USBHOST_CDCECM_NTDELAY MSEC2TICK(CONFIG_USBHOST_CDCECM_NTDELAY)
+#else
+#  define USBHOST_CDCECM_NTDELAY MSEC2TICK(200)
+#endif
+
+#ifndef CONFIG_USBHOST_CDCECM_RXBUFSIZE
+  #define CONFIG_USBHOST_CDCECM_RXBUFSIZE 2048
+#endif
+
+#ifndef CONFIG_USBHOST_CDCECM_TXBUFSIZE
+  #define CONFIG_USBHOST_CDCECM_TXBUFSIZE 2048
+#endif
+
+/* Used in usbhost_cfgdesc() */
+
+#define USBHOST_CTRLIFFOUND 0x01
+#define USBHOST_DATAIFFOUND 0x02
+#define USBHOST_INTRIFFOUND 0x04
+#define USBHOST_BINFOUND    0x08
+#define USBHOST_BOUTFOUND   0x10
+#define USBHOST_ECMFOUND    0x20
+#define USBHOST_ALLFOUND    0x3f
+
+#define USBHOST_MAX_CREFS   0x7fff
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+struct usb_csifdesc_s
+{
+  uint8_t len;
+  uint8_t type;
+  uint8_t subtype;
+};
+
+/* This structure contains the internal, private state of the USB host class
+ * driver.
+ */
+
+struct usbhost_cdcecm_s
+{
+  /* This is the externally visible portion of the state */
+
+  struct usbhost_class_s  usbclass;
+
+  /* The remainder of the fields are provided to the class driver */
+
+  volatile bool           disconnected; /* TRUE: Device has been disconnected 
*/
+  uint16_t                ctrlif;       /* Control interface number */
+  uint16_t                dataif;       /* Data interface number */
+  int16_t                 crefs;        /* Reference count on the driver 
instance */
+  mutex_t                 lock;         /* Used to maintain mutual exclusive 
access */
+  struct work_s           ntwork;       /* Notification work */
+  struct work_s           bulk_rxwork;
+  struct work_s           txpollwork;
+  struct work_s           destroywork;
+  int16_t                 nnbytes;      /* Number of bytes received in 
notification */
+  int16_t                 bulkinbytes;
+  uint16_t                maxintsize;   /* Maximum size of interrupt IN packet 
*/
+  FAR uint8_t            *ctrlreq;      /* Allocated ctrl request structure */
+  FAR uint8_t            *notification; /* Allocated RX buffer for async 
notifications */
+  FAR uint8_t            *bulkinbuf;    /* Allocated RX buffer for bulk IN */
+  FAR uint8_t            *bulkoutbuf;   /* Allocated TX buffer for bulk OUT */
+  usbhost_ep_t            intin;        /* Interrupt endpoint */
+  usbhost_ep_t            bulkin;       /* Bulk IN endpoint */
+  usbhost_ep_t            bulkout;      /* Bulk OUT endpoint */
+  uint16_t                bulkmxpacket; /* Max packet size for Bulk OUT 
endpoint */
+
+  /* Device info from ECM descriptor */
+
+  uint16_t                maxsegment;    /* Max Ethernet frame size */
+  uint8_t                 macstridx;     /* MAC address string index */
+  uint8_t                 macaddr[6];    /* Device MAC address */
+
+  /* Network device members */
+
+  bool                    registered;   /* true if the device was registered */
+  bool                    bifup;        /* true:ifup false:ifdown */
+  struct net_driver_s     netdev;       /* Interface understood by the network 
*/
+};
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+/* Memory allocation services */
+
+static inline FAR struct usbhost_cdcecm_s *usbhost_allocclass(void);
+static inline void usbhost_freeclass(FAR struct usbhost_cdcecm_s *usbclass);
+
+/* Worker thread actions */
+
+static void usbhost_notification_work(FAR void *arg);
+static void usbhost_notification_callback(FAR void *arg, ssize_t nbytes);
+static void usbhost_bulkin_work(FAR void *arg);
+
+static void usbhost_destroy(FAR void *arg);
+
+/* Helpers for usbhost_connect() */
+
+static int usbhost_cfgdesc(FAR struct usbhost_cdcecm_s *priv,
+                           FAR const uint8_t *configdesc, int desclen);
+static inline int usbhost_devinit(FAR struct usbhost_cdcecm_s *priv);
+
+/* (Little Endian) Data helpers */
+
+static inline uint16_t usbhost_getle16(const uint8_t *val);
+static inline void usbhost_putle16(uint8_t *dest, uint16_t val);
+static inline uint32_t usbhost_getle32(const uint8_t *val);
+
+/* Buffer memory management */
+
+static int usbhost_alloc_buffers(FAR struct usbhost_cdcecm_s *priv);
+static void usbhost_free_buffers(FAR struct usbhost_cdcecm_s *priv);
+
+/* struct usbhost_registry_s methods */
+
+static struct usbhost_class_s
+              *usbhost_create(FAR struct usbhost_hubport_s *hport,
+                              FAR const struct usbhost_id_s *id);
+
+/* struct usbhost_class_s methods */
+
+static int usbhost_connect(FAR struct usbhost_class_s *usbclass,
+                           FAR const uint8_t *configdesc, int desclen);
+static int usbhost_disconnected(FAR struct usbhost_class_s *usbclass);
+
+/* NuttX network callback functions */
+
+static int cdcecm_ifup(struct net_driver_s *dev);
+static int cdcecm_ifdown(struct net_driver_s *dev);
+static int cdcecm_txavail(struct net_driver_s *dev);
+
+/* Network support functions */
+
+static void cdcecm_receive(struct usbhost_cdcecm_s *priv);
+
+static int cdcecm_txpoll(struct net_driver_s *dev);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/* This structure provides the registry entry ID information that will  be
+ * used to associate the USB class driver to a connected USB device.
+ */
+
+static const struct usbhost_id_s g_id[] =
+{
+  {
+    USB_CLASS_CDC,          /* base - CDC class */
+    CDC_SUBCLASS_NONE,      /* subclass */
+    CDC_PROTO_NONE,         /* proto    */
+    0,                      /* vid      */
+    0                       /* pid      */
+  },
+  {
+    USB_CLASS_CDC,          /* base - CDC class */
+    CDC_SUBCLASS_ECM,       /* subclass - Ethernet Control Model */
+    CDC_PROTO_NONE,         /* proto - No class specific protocol */
+    0,                      /* vid - Any vendor */
+    0                       /* pid - Any product */
+  }
+};
+
+/* This is the USB host storage class's registry entry */
+
+static struct usbhost_registry_s g_cdcecm =
+{
+  NULL,                           /* flink */
+  usbhost_create,                 /* create */
+  sizeof(g_id) / sizeof(g_id[0]), /* nids */
+  g_id                            /* id[] */
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(CONFIG_DEBUG_USB) && defined(USBHOST_CDCECM_PACKET_PRINT)
+static void cdcecm_print_packet(const char * dir, const uint8_t * buf,

Review Comment:
   I didnt see those functions. You are right, much better to use 
uinfodumpbuffer



##########
drivers/usbhost/usbhost_cdcecm.c:
##########
@@ -0,0 +1,2160 @@
+/****************************************************************************
+ * drivers/usbhost/usbhost_cdcecm.c
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <semaphore.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+#include <poll.h>
+#include <fcntl.h>
+
+#include <arpa/inet.h>
+
+#include <nuttx/arch.h>
+#include <nuttx/irq.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/kthread.h>
+#include <nuttx/mutex.h>
+#include <nuttx/fs/fs.h>
+#include <nuttx/wqueue.h>
+#include <nuttx/signal.h>
+#include <nuttx/net/netdev.h>
+
+#include <nuttx/usb/cdc.h>
+#include <nuttx/usb/usb.h>
+#include <nuttx/usb/usbhost.h>
+
+#ifdef CONFIG_NET_PKT
+  #include <nuttx/net/pkt.h>
+#endif
+
+#ifdef CONFIG_USBHOST_CDCECM
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Default max segment size if not specified by device */
+
+#define CDCECM_DEFAULT_MAXSEG   1514
+
+/* ECM control requests */
+
+#define ECM_SET_PACKET_FILTER_REQUEST 0x43
+
+/* ECM packet filter bits */
+
+#define ECM_PACKET_TYPE_PROMISCUOUS    (1 << 0)
+#define ECM_PACKET_TYPE_ALL_MULTICAST  (1 << 1)
+#define ECM_PACKET_TYPE_DIRECTED       (1 << 2)
+#define ECM_PACKET_TYPE_BROADCAST      (1 << 3)
+#define ECM_PACKET_TYPE_MULTICAST      (1 << 4)
+
+/* Default packet filter: unicast + broadcast */
+
+#define ECM_DEFAULT_PACKET_FILTER      (ECM_PACKET_TYPE_DIRECTED | \
+                                        ECM_PACKET_TYPE_BROADCAST)
+
+/* Configuration ************************************************************/
+
+#ifndef CONFIG_SCHED_WORKQUEUE
+#  warning "Worker thread support is required (CONFIG_SCHED_WORKQUEUE)"
+#endif
+
+#ifndef CONFIG_USBHOST_ASYNCH
+#  warning Asynchronous transfer support is required (CONFIG_USBHOST_ASYNCH)
+#endif
+
+#ifdef CONFIG_USBHOST_CDCECM_NTDELAY
+#  define USBHOST_CDCECM_NTDELAY MSEC2TICK(CONFIG_USBHOST_CDCECM_NTDELAY)
+#else
+#  define USBHOST_CDCECM_NTDELAY MSEC2TICK(200)
+#endif
+
+#ifndef CONFIG_USBHOST_CDCECM_RXBUFSIZE
+  #define CONFIG_USBHOST_CDCECM_RXBUFSIZE 2048
+#endif
+
+#ifndef CONFIG_USBHOST_CDCECM_TXBUFSIZE
+  #define CONFIG_USBHOST_CDCECM_TXBUFSIZE 2048
+#endif
+
+/* Used in usbhost_cfgdesc() */
+
+#define USBHOST_CTRLIFFOUND 0x01
+#define USBHOST_DATAIFFOUND 0x02
+#define USBHOST_INTRIFFOUND 0x04
+#define USBHOST_BINFOUND    0x08
+#define USBHOST_BOUTFOUND   0x10
+#define USBHOST_ECMFOUND    0x20
+#define USBHOST_ALLFOUND    0x3f
+
+#define USBHOST_MAX_CREFS   0x7fff
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+struct usb_csifdesc_s
+{
+  uint8_t len;
+  uint8_t type;
+  uint8_t subtype;
+};
+
+/* This structure contains the internal, private state of the USB host class
+ * driver.
+ */
+
+struct usbhost_cdcecm_s
+{
+  /* This is the externally visible portion of the state */
+
+  struct usbhost_class_s  usbclass;
+
+  /* The remainder of the fields are provided to the class driver */
+
+  volatile bool           disconnected; /* TRUE: Device has been disconnected 
*/
+  uint16_t                ctrlif;       /* Control interface number */
+  uint16_t                dataif;       /* Data interface number */
+  int16_t                 crefs;        /* Reference count on the driver 
instance */
+  mutex_t                 lock;         /* Used to maintain mutual exclusive 
access */
+  struct work_s           ntwork;       /* Notification work */
+  struct work_s           bulk_rxwork;
+  struct work_s           txpollwork;
+  struct work_s           destroywork;
+  int16_t                 nnbytes;      /* Number of bytes received in 
notification */
+  int16_t                 bulkinbytes;
+  uint16_t                maxintsize;   /* Maximum size of interrupt IN packet 
*/
+  FAR uint8_t            *ctrlreq;      /* Allocated ctrl request structure */
+  FAR uint8_t            *notification; /* Allocated RX buffer for async 
notifications */
+  FAR uint8_t            *bulkinbuf;    /* Allocated RX buffer for bulk IN */
+  FAR uint8_t            *bulkoutbuf;   /* Allocated TX buffer for bulk OUT */
+  usbhost_ep_t            intin;        /* Interrupt endpoint */
+  usbhost_ep_t            bulkin;       /* Bulk IN endpoint */
+  usbhost_ep_t            bulkout;      /* Bulk OUT endpoint */
+  uint16_t                bulkmxpacket; /* Max packet size for Bulk OUT 
endpoint */
+
+  /* Device info from ECM descriptor */
+
+  uint16_t                maxsegment;    /* Max Ethernet frame size */
+  uint8_t                 macstridx;     /* MAC address string index */
+  uint8_t                 macaddr[6];    /* Device MAC address */
+
+  /* Network device members */
+
+  bool                    registered;   /* true if the device was registered */
+  bool                    bifup;        /* true:ifup false:ifdown */
+  struct net_driver_s     netdev;       /* Interface understood by the network 
*/
+};
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+/* Memory allocation services */
+
+static inline FAR struct usbhost_cdcecm_s *usbhost_allocclass(void);
+static inline void usbhost_freeclass(FAR struct usbhost_cdcecm_s *usbclass);
+
+/* Worker thread actions */
+
+static void usbhost_notification_work(FAR void *arg);
+static void usbhost_notification_callback(FAR void *arg, ssize_t nbytes);
+static void usbhost_bulkin_work(FAR void *arg);
+
+static void usbhost_destroy(FAR void *arg);
+
+/* Helpers for usbhost_connect() */
+
+static int usbhost_cfgdesc(FAR struct usbhost_cdcecm_s *priv,
+                           FAR const uint8_t *configdesc, int desclen);
+static inline int usbhost_devinit(FAR struct usbhost_cdcecm_s *priv);
+
+/* (Little Endian) Data helpers */
+
+static inline uint16_t usbhost_getle16(const uint8_t *val);
+static inline void usbhost_putle16(uint8_t *dest, uint16_t val);
+static inline uint32_t usbhost_getle32(const uint8_t *val);
+
+/* Buffer memory management */
+
+static int usbhost_alloc_buffers(FAR struct usbhost_cdcecm_s *priv);
+static void usbhost_free_buffers(FAR struct usbhost_cdcecm_s *priv);
+
+/* struct usbhost_registry_s methods */
+
+static struct usbhost_class_s
+              *usbhost_create(FAR struct usbhost_hubport_s *hport,
+                              FAR const struct usbhost_id_s *id);
+
+/* struct usbhost_class_s methods */
+
+static int usbhost_connect(FAR struct usbhost_class_s *usbclass,
+                           FAR const uint8_t *configdesc, int desclen);
+static int usbhost_disconnected(FAR struct usbhost_class_s *usbclass);
+
+/* NuttX network callback functions */
+
+static int cdcecm_ifup(struct net_driver_s *dev);
+static int cdcecm_ifdown(struct net_driver_s *dev);
+static int cdcecm_txavail(struct net_driver_s *dev);
+
+/* Network support functions */
+
+static void cdcecm_receive(struct usbhost_cdcecm_s *priv);
+
+static int cdcecm_txpoll(struct net_driver_s *dev);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/* This structure provides the registry entry ID information that will  be
+ * used to associate the USB class driver to a connected USB device.
+ */
+
+static const struct usbhost_id_s g_id[] =
+{
+  {
+    USB_CLASS_CDC,          /* base - CDC class */
+    CDC_SUBCLASS_NONE,      /* subclass */
+    CDC_PROTO_NONE,         /* proto    */
+    0,                      /* vid      */
+    0                       /* pid      */
+  },
+  {
+    USB_CLASS_CDC,          /* base - CDC class */
+    CDC_SUBCLASS_ECM,       /* subclass - Ethernet Control Model */
+    CDC_PROTO_NONE,         /* proto - No class specific protocol */
+    0,                      /* vid - Any vendor */
+    0                       /* pid - Any product */
+  }
+};
+
+/* This is the USB host storage class's registry entry */
+
+static struct usbhost_registry_s g_cdcecm =
+{
+  NULL,                           /* flink */
+  usbhost_create,                 /* create */
+  sizeof(g_id) / sizeof(g_id[0]), /* nids */
+  g_id                            /* id[] */
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(CONFIG_DEBUG_USB) && defined(USBHOST_CDCECM_PACKET_PRINT)
+static void cdcecm_print_packet(const char * dir, const uint8_t * buf,
+                                size_t len)
+{
+  char * str = kmm_malloc(len * 2 + 1);
+  if (str == NULL)
+    {
+      uerr("Failed to dump ECM %s packet\n", dir);
+    }
+  else
+    {
+      for (size_t i = 0; i < len; ++i)
+        {
+          sprintf(&str[i * 2], "%02X", buf[i]);
+        }
+
+      str[len * 2] = '\0';
+      uinfo("ECM %s packet: %s\n", dir, str);
+      kmm_free(str);
+    }
+}
+#else
+#  define cdcecm_print_packet(dir, buf, len) {}
+#endif
+
+static int usbhost_ctrl_cmd(FAR struct usbhost_cdcecm_s *priv,
+                            uint8_t type, uint8_t req, uint16_t value,
+                            uint16_t index, uint8_t *payload, uint16_t len)
+{
+  FAR struct usbhost_hubport_s *hport;
+  struct usb_ctrlreq_s *ctrlreq;
+  int ret;
+
+  hport = priv->usbclass.hport;
+
+  ctrlreq       = (struct usb_ctrlreq_s *)priv->ctrlreq;
+  ctrlreq->type = type;
+  ctrlreq->req  = req;
+
+  usbhost_putle16(ctrlreq->value, value);
+  usbhost_putle16(ctrlreq->index, index);
+  usbhost_putle16(ctrlreq->len,   len);
+
+  if (type & USB_REQ_DIR_IN)
+    {
+      ret = DRVR_CTRLIN(hport->drvr, hport->ep0, ctrlreq, payload);
+    }
+  else
+    {
+      ret = DRVR_CTRLOUT(hport->drvr, hport->ep0, ctrlreq, payload);
+    }
+
+  return ret;
+}
+
+/****************************************************************************
+ * Name: usbhost_allocclass
+ *
+ * Description:
+ *   This is really part of the logic that implements the create() method
+ *   of struct usbhost_registry_s.  This function allocates memory for one
+ *   new class instance.
+ *
+ * Input Parameters:
+ *   None
+ *
+ * Returned Value:
+ *   On success, this function will return a non-NULL instance of struct
+ *   usbhost_class_s.  NULL is returned on failure; this function will
+ *   will fail only if there are insufficient resources to create another
+ *   USB host class instance.
+ *
+ ****************************************************************************/
+
+static inline FAR struct usbhost_cdcecm_s *usbhost_allocclass(void)
+{
+  FAR struct usbhost_cdcecm_s *priv;
+
+  DEBUGASSERT(!up_interrupt_context());
+  priv = (FAR struct usbhost_cdcecm_s *)kmm_malloc(
+                                         sizeof(struct usbhost_cdcecm_s));
+  uinfo("Allocated: %p\n", priv);
+  return priv;
+}
+
+/****************************************************************************
+ * Name: usbhost_freeclass
+ *
+ * Description:
+ *   Free a class instance previously allocated by usbhost_allocclass().
+ *
+ * Input Parameters:
+ *   usbclass - A reference to the class instance to be freed.
+ *
+ * Returned Value:
+ *   None
+ *
+ ****************************************************************************/
+
+static inline void usbhost_freeclass(FAR struct usbhost_cdcecm_s *usbclass)
+{
+  DEBUGASSERT(usbclass != NULL);
+
+  /* Free the class instance (perhaps calling sched_kmm_free() in case we are
+   * executing from an interrupt handler.
+   */
+
+  uinfo("Freeing: %p\n", usbclass);
+  kmm_free(usbclass);
+}
+
+static void usbhost_bulkin_callback(FAR void *arg, ssize_t nbytes)
+{
+  struct usbhost_cdcecm_s *priv = (struct usbhost_cdcecm_s *)arg;
+  uint32_t delay = 0;
+
+  DEBUGASSERT(priv);
+
+  /* We can't lock the mutex from an interrupt context */
+
+  if (priv->disconnected)
+    {
+      return;
+    }
+
+  priv->bulkinbytes = (int16_t)nbytes;
+
+  if (nbytes < 0)
+    {
+      if (nbytes != -EAGAIN)
+        {
+          uerr("ERROR: Transfer failed: %d\n", nbytes);
+        }
+
+      delay = MSEC2TICK(30);

Review Comment:
   This is following a pattern was taken from other drivers like the ACM and 
MBIM.
   
   The idea is that normally we should not trigger a delay if we get data, but 
if for example the device we are talking to gets into a bad state or there are 
uncontrolled bus errors, then without a delay this might turn into a tight poll 
loop of error -> callback -> work -> error



##########
drivers/usbhost/usbhost_cdcecm.c:
##########
@@ -0,0 +1,2160 @@
+/****************************************************************************
+ * drivers/usbhost/usbhost_cdcecm.c
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <semaphore.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+#include <poll.h>
+#include <fcntl.h>
+
+#include <arpa/inet.h>
+
+#include <nuttx/arch.h>
+#include <nuttx/irq.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/kthread.h>
+#include <nuttx/mutex.h>
+#include <nuttx/fs/fs.h>
+#include <nuttx/wqueue.h>
+#include <nuttx/signal.h>
+#include <nuttx/net/netdev.h>
+
+#include <nuttx/usb/cdc.h>
+#include <nuttx/usb/usb.h>
+#include <nuttx/usb/usbhost.h>
+
+#ifdef CONFIG_NET_PKT
+  #include <nuttx/net/pkt.h>
+#endif
+
+#ifdef CONFIG_USBHOST_CDCECM
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Default max segment size if not specified by device */
+
+#define CDCECM_DEFAULT_MAXSEG   1514
+
+/* ECM control requests */
+
+#define ECM_SET_PACKET_FILTER_REQUEST 0x43
+
+/* ECM packet filter bits */
+
+#define ECM_PACKET_TYPE_PROMISCUOUS    (1 << 0)
+#define ECM_PACKET_TYPE_ALL_MULTICAST  (1 << 1)
+#define ECM_PACKET_TYPE_DIRECTED       (1 << 2)
+#define ECM_PACKET_TYPE_BROADCAST      (1 << 3)
+#define ECM_PACKET_TYPE_MULTICAST      (1 << 4)
+
+/* Default packet filter: unicast + broadcast */
+
+#define ECM_DEFAULT_PACKET_FILTER      (ECM_PACKET_TYPE_DIRECTED | \
+                                        ECM_PACKET_TYPE_BROADCAST)
+
+/* Configuration ************************************************************/
+
+#ifndef CONFIG_SCHED_WORKQUEUE
+#  warning "Worker thread support is required (CONFIG_SCHED_WORKQUEUE)"
+#endif
+
+#ifndef CONFIG_USBHOST_ASYNCH
+#  warning Asynchronous transfer support is required (CONFIG_USBHOST_ASYNCH)
+#endif
+
+#ifdef CONFIG_USBHOST_CDCECM_NTDELAY
+#  define USBHOST_CDCECM_NTDELAY MSEC2TICK(CONFIG_USBHOST_CDCECM_NTDELAY)
+#else
+#  define USBHOST_CDCECM_NTDELAY MSEC2TICK(200)
+#endif
+
+#ifndef CONFIG_USBHOST_CDCECM_RXBUFSIZE
+  #define CONFIG_USBHOST_CDCECM_RXBUFSIZE 2048
+#endif
+
+#ifndef CONFIG_USBHOST_CDCECM_TXBUFSIZE
+  #define CONFIG_USBHOST_CDCECM_TXBUFSIZE 2048
+#endif
+
+/* Used in usbhost_cfgdesc() */
+
+#define USBHOST_CTRLIFFOUND 0x01
+#define USBHOST_DATAIFFOUND 0x02
+#define USBHOST_INTRIFFOUND 0x04
+#define USBHOST_BINFOUND    0x08
+#define USBHOST_BOUTFOUND   0x10
+#define USBHOST_ECMFOUND    0x20
+#define USBHOST_ALLFOUND    0x3f
+
+#define USBHOST_MAX_CREFS   0x7fff
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+struct usb_csifdesc_s
+{
+  uint8_t len;
+  uint8_t type;
+  uint8_t subtype;
+};
+
+/* This structure contains the internal, private state of the USB host class
+ * driver.
+ */
+
+struct usbhost_cdcecm_s
+{
+  /* This is the externally visible portion of the state */
+
+  struct usbhost_class_s  usbclass;
+
+  /* The remainder of the fields are provided to the class driver */
+
+  volatile bool           disconnected; /* TRUE: Device has been disconnected 
*/
+  uint16_t                ctrlif;       /* Control interface number */
+  uint16_t                dataif;       /* Data interface number */
+  int16_t                 crefs;        /* Reference count on the driver 
instance */
+  mutex_t                 lock;         /* Used to maintain mutual exclusive 
access */
+  struct work_s           ntwork;       /* Notification work */
+  struct work_s           bulk_rxwork;
+  struct work_s           txpollwork;
+  struct work_s           destroywork;
+  int16_t                 nnbytes;      /* Number of bytes received in 
notification */
+  int16_t                 bulkinbytes;
+  uint16_t                maxintsize;   /* Maximum size of interrupt IN packet 
*/
+  FAR uint8_t            *ctrlreq;      /* Allocated ctrl request structure */
+  FAR uint8_t            *notification; /* Allocated RX buffer for async 
notifications */
+  FAR uint8_t            *bulkinbuf;    /* Allocated RX buffer for bulk IN */
+  FAR uint8_t            *bulkoutbuf;   /* Allocated TX buffer for bulk OUT */
+  usbhost_ep_t            intin;        /* Interrupt endpoint */
+  usbhost_ep_t            bulkin;       /* Bulk IN endpoint */
+  usbhost_ep_t            bulkout;      /* Bulk OUT endpoint */
+  uint16_t                bulkmxpacket; /* Max packet size for Bulk OUT 
endpoint */
+
+  /* Device info from ECM descriptor */
+
+  uint16_t                maxsegment;    /* Max Ethernet frame size */
+  uint8_t                 macstridx;     /* MAC address string index */
+  uint8_t                 macaddr[6];    /* Device MAC address */
+
+  /* Network device members */
+
+  bool                    registered;   /* true if the device was registered */
+  bool                    bifup;        /* true:ifup false:ifdown */
+  struct net_driver_s     netdev;       /* Interface understood by the network 
*/

Review Comment:
   I based this off of nuttx_latest/nuttx/drivers/usbhost/usbhost_cdcmbim.c so 
maybe I inherited some bad patterns.
   
   Can you please elaborate a little more on what you mean by that?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to