You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by ag...@apache.org on 2020/09/01 19:49:11 UTC

[incubator-nuttx] branch master updated (b5d3ba6 -> a5a3e54)

This is an automated email from the ASF dual-hosted git repository.

aguettouche pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git.


    from b5d3ba6  arch/xtensa/src/esp32/esp32_gpio.c: Enable input mode only when configuring an input.
     new e521c22  Kinetis USBHSHOST improvement. Avoid race conditions during freeing of queue head structures by using Async Advance Doorbell.
     new a5a3e54  Kinetis USBHSHOST: Changed Async Await to linked list, restored two accidently deleted lines.

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 arch/arm/src/kinetis/Kconfig             |  35 +++++++++
 arch/arm/src/kinetis/kinetis_usbhshost.c | 130 +++++++++++++++++++------------
 2 files changed, 114 insertions(+), 51 deletions(-)


[incubator-nuttx] 02/02: Kinetis USBHSHOST: Changed Async Await to linked list, restored two accidently deleted lines.

Posted by ag...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

aguettouche pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git

commit a5a3e54be0b7c4d45104826ace897a26fe3efa9b
Author: Johannes Schock <jo...@nivus.com>
AuthorDate: Sun Aug 23 10:50:05 2020 +0200

    Kinetis USBHSHOST: Changed Async Await to linked list, restored two accidently deleted lines.
---
 arch/arm/src/kinetis/kinetis_usbhshost.c | 78 +++++++++++++++++++-------------
 1 file changed, 47 insertions(+), 31 deletions(-)

diff --git a/arch/arm/src/kinetis/kinetis_usbhshost.c b/arch/arm/src/kinetis/kinetis_usbhshost.c
index 421f0f0..a73f21e 100644
--- a/arch/arm/src/kinetis/kinetis_usbhshost.c
+++ b/arch/arm/src/kinetis/kinetis_usbhshost.c
@@ -215,8 +215,8 @@ struct kinetis_qh_s
 
   struct kinetis_epinfo_s *epinfo; /* Endpoint used for the transfer */
   uint32_t fqp;                    /* First qTD in the list (physical address) */
-  bool     aawait;                 /* Waiting for async_advance doorbell interrupt */
-  uint8_t  pad[7];                 /* Padding to assure 32-byte alignment */
+  uint8_t  pad[4];                 /* Padding to assure 32-byte alignment */
+  struct kinetis_qh_s *flink;      /* Link for async await and free list */
 };
 
 /* Internal representation of the EHCI Queue Element Transfer Descriptor (qTD) */
@@ -230,13 +230,13 @@ struct kinetis_qtd_s
   /* Internal fields used by the EHCI driver */
 };
 
-/* The following is used to manage lists of free QHs and qTDs */
+/* The following is used to manage lists of free qTDs */
 
 struct kinetis_list_s
 {
   struct kinetis_list_s *flink;    /* Link to next entry in the list
-                                  * Variable length entry data follows
-                                  */
+                                    * Variable length entry data follows
+                                    */
 };
 
 /* List traversal call-out functions */
@@ -303,7 +303,8 @@ struct kinetis_ehci_s
   sem_t pscsem;                 /* Semaphore to wait for port status change events */
 
   struct kinetis_epinfo_s ep0;    /* Endpoint 0 */
-  struct kinetis_list_s *qhfree;  /* List of free Queue Head (QH) structures */
+  struct kinetis_qh_s *qhaawait;  /* List of waiting Queue Head (QH) structures */
+  struct kinetis_qh_s *qhfree;    /* List of free Queue Head (QH) structures */
   struct kinetis_list_s *qtdfree; /* List of free Queue Element Transfer Descriptor (qTD) */
   struct work_s work;             /* Supports interrupt bottom half */
 
@@ -1136,10 +1137,10 @@ static struct kinetis_qh_s *kinetis_qh_alloc(void)
 
   /* Remove the QH structure from the freelist */
 
-  qh = (struct kinetis_qh_s *)g_ehci.qhfree;
+  qh = g_ehci.qhfree;
   if (qh)
     {
-      g_ehci.qhfree = ((struct kinetis_list_s *)qh)->flink;
+      g_ehci.qhfree = qh->flink;
       memset(qh, 0, sizeof(struct kinetis_qh_s));
     }
 
@@ -1147,6 +1148,29 @@ static struct kinetis_qh_s *kinetis_qh_alloc(void)
 }
 
 /************************************************************************************
+ * Name: kinetis_qh_aawait
+ *
+ * Description:
+ *   Let a Queue Head (QH) structure wait for free by adding it to the aawait list
+ *
+ * Assumption:  Caller holds the exclsem
+ *
+ ************************************************************************************/
+
+static void kinetis_qh_aawait(struct kinetis_qh_s *qh)
+{
+  uint32_t regval;
+
+  /* Put the QH structure to the aawait list */
+
+  qh->flink  = g_ehci.qhaawait;
+  g_ehci.qhaawait = qh;
+
+  regval = kinetis_getreg(&HCOR->usbcmd);
+  kinetis_putreg(regval | EHCI_USBCMD_IAADB, &HCOR->usbcmd);
+}
+
+/************************************************************************************
  * Name: kinetis_qh_free
  *
  * Description:
@@ -1158,12 +1182,10 @@ static struct kinetis_qh_s *kinetis_qh_alloc(void)
 
 static void kinetis_qh_free(struct kinetis_qh_s *qh)
 {
-  struct kinetis_list_s *entry = (struct kinetis_list_s *)qh;
-
   /* Put the QH structure back into the free list */
 
-  entry->flink  = g_ehci.qhfree;
-  g_ehci.qhfree = entry;
+  qh->flink  = g_ehci.qhfree;
+  g_ehci.qhfree = qh;
 }
 
 /************************************************************************************
@@ -2816,7 +2838,6 @@ static int kinetis_qh_ioccheck(struct kinetis_qh_s *qh, uint32_t **bp, void *arg
   struct kinetis_epinfo_s *epinfo;
   uint32_t token;
   int ret;
-  uint32_t regval;
 
   DEBUGASSERT(qh && bp);
 
@@ -2951,10 +2972,7 @@ static int kinetis_qh_ioccheck(struct kinetis_qh_s *qh, uint32_t **bp, void *arg
 
       /* Then start async advance doorbell process */
 
-      qh->aawait = true;
-
-      regval = kinetis_getreg(&HCOR->usbcmd);
-      kinetis_putreg(regval | EHCI_USBCMD_IAADB, &HCOR->usbcmd);
+      kinetis_qh_aawait(qh);
     }
   else
     {
@@ -3043,6 +3061,12 @@ static int kinetis_qh_cancel(struct kinetis_qh_s *qh, uint32_t **bp, void *arg)
       return OK;
     }
 
+  /* Disable both the asynchronous and period schedules */
+
+  regval = kinetis_getreg(&HCOR->usbcmd);
+  kinetis_putreg(regval & ~(EHCI_USBCMD_ASEN | EHCI_USBCMD_PSEN),
+               &HCOR->usbcmd);
+
   /* Remove the QH from the list
    *
    * NOTE that we don't check if the qTD is active nor do we check if there
@@ -3070,10 +3094,7 @@ static int kinetis_qh_cancel(struct kinetis_qh_s *qh, uint32_t **bp, void *arg)
 
   /* Then start async advance doorbell process */
 
-  qh->aawait = true;
-
-  regval = kinetis_getreg(&HCOR->usbcmd);
-  kinetis_putreg(regval | EHCI_USBCMD_IAADB, &HCOR->usbcmd);
+  kinetis_qh_aawait(qh);
 
   /* Return 1 to stop the traverse without an error. */
 
@@ -3339,18 +3360,14 @@ static inline void kinetis_syserr_bottomhalf(void)
 
 static inline void kinetis_async_advance_bottomhalf(void)
 {
-  int i;
+  struct kinetis_qh_s *qh;
   usbhost_vtrace1(EHCI_VTRACE1_AAINTR, 0);
 
-  for (i = 0; i < CONFIG_KINETIS_EHCI_NQHS; i++)
+  while (g_ehci.qhaawait != NULL)
     {
-      /* Put the QH structure in the free list if tagged */
-
-      if (g_qhpool[i].aawait)
-        {
-          g_qhpool[i].aawait = false;
-          kinetis_qh_free(&g_qhpool[i]);
-        }
+      qh = g_ehci.qhaawait;
+      g_ehci.qhaawait = qh->flink;
+      kinetis_qh_free(qh);
     }
 }
 
@@ -3387,7 +3404,6 @@ static void kinetis_ehci_bottomhalf(FAR void *arg)
 
   if ((pending & EHCI_INT_AAINT) != 0)
     {
-      uerr("Async Advance\n");
       kinetis_async_advance_bottomhalf();
       kinetis_putreg(EHCI_INT_AAINT, &HCOR->usbsts);
     }


[incubator-nuttx] 01/02: Kinetis USBHSHOST improvement. Avoid race conditions during freeing of queue head structures by using Async Advance Doorbell.

Posted by ag...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

aguettouche pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git

commit e521c224c12f355ff757b8483da8ca116f2fa175
Author: Johannes Schock <jo...@nivus.com>
AuthorDate: Sat Aug 22 15:25:37 2020 +0200

    Kinetis USBHSHOST improvement.
    Avoid race conditions during freeing of queue head structures by using Async Advance Doorbell.
---
 arch/arm/src/kinetis/Kconfig             |  35 ++++++++++
 arch/arm/src/kinetis/kinetis_usbhshost.c | 106 +++++++++++++++++--------------
 2 files changed, 94 insertions(+), 47 deletions(-)

diff --git a/arch/arm/src/kinetis/Kconfig b/arch/arm/src/kinetis/Kconfig
index c5a51c4..c648bb6 100644
--- a/arch/arm/src/kinetis/Kconfig
+++ b/arch/arm/src/kinetis/Kconfig
@@ -1114,6 +1114,41 @@ config KINETIS_SD4BIT_FREQ
 endif
 endmenu # Kinetis SDHC Configuration
 
+if KINETIS_USBHS && USBHOST
+
+menu "USB host controller driver (HCD) options"
+
+config KINETIS_EHCI_NQHS
+	int "Number of Queue Head (QH) structures"
+	default 4
+	---help---
+		Configurable number of Queue Head (QH) structures.
+
+config KINETIS_EHCI_NQTDS
+	int "Number of Queue Element Transfer Descriptor (qTDs)"
+	default 6
+	---help---
+		Configurable number of Queue Element Transfer Descriptor (qTDs).
+
+config KINETIS_EHCI_BUFSIZE
+	int "Size of one request/descriptor buffer"
+	default 128
+	---help---
+		The size of one request/descriptor buffer in bytes.  The TD buffe
+		size must be an even number of 32-bit words and must be large enough
+		to hangle the largest transfer via a SETUP request.
+
+config KINETIS_EHCI_PREALLOCATE
+	bool "Preallocate descriptor pool"
+	default y
+	---help---
+		Select this option to pre-allocate EHCI queue and descriptor
+		structure pools in .bss.  Otherwise, these pools will be
+		dynamically allocated using kmm_memalign().
+
+endmenu # USB host controller driver (HCD) options
+endif # KINETIS_USBHS && USBHOST
+
 #
 # MCU serial peripheral driver?
 #
diff --git a/arch/arm/src/kinetis/kinetis_usbhshost.c b/arch/arm/src/kinetis/kinetis_usbhshost.c
index bfdba30..421f0f0 100644
--- a/arch/arm/src/kinetis/kinetis_usbhshost.c
+++ b/arch/arm/src/kinetis/kinetis_usbhshost.c
@@ -87,20 +87,18 @@
 #  error Hi-priority work queue support is required (CONFIG_SCHED_HPWORK)
 #endif
 
-/* Configurable number of Queue Head (QH) structures.  The default is one per
- * Root hub port plus one for EP0.
- */
+/* Configurable number of Queue Head (QH) structures.  The default is 4. */
 
 #ifndef CONFIG_KINETIS_EHCI_NQHS
-#  define CONFIG_KINETIS_EHCI_NQHS (KINETIS_EHCI_NRHPORT + 1)
+#  define CONFIG_KINETIS_EHCI_NQHS (4)
 #endif
 
 /* Configurable number of Queue Element Transfer Descriptor (qTDs).  The
- * default is one per root hub plus three from EP0.
+ * default is 6
  */
 
 #ifndef CONFIG_KINETIS_EHCI_NQTDS
-#  define CONFIG_KINETIS_EHCI_NQTDS (KINETIS_EHCI_NRHPORT + 3)
+#  define CONFIG_KINETIS_EHCI_NQTDS (6)
 #endif
 
 /* Buffers must be aligned to the cache line size */
@@ -217,7 +215,8 @@ struct kinetis_qh_s
 
   struct kinetis_epinfo_s *epinfo; /* Endpoint used for the transfer */
   uint32_t fqp;                    /* First qTD in the list (physical address) */
-  uint8_t pad[8];                  /* Padding to assure 32-byte alignment */
+  bool     aawait;                 /* Waiting for async_advance doorbell interrupt */
+  uint8_t  pad[7];                 /* Padding to assure 32-byte alignment */
 };
 
 /* Internal representation of the EHCI Queue Element Transfer Descriptor (qTD) */
@@ -587,8 +586,8 @@ static int kinetis_reset(void);
  * Private Data
  ************************************************************************************/
 
-/* In this driver implementation, support is provided for only a single a
- * single USB device.  All status information can be simply retained in a
+/* In this driver implementation, support is provided for only a single
+ * USB device.  All status information can be simply retained in a
  * single global instance.
  */
 
@@ -2817,6 +2816,7 @@ static int kinetis_qh_ioccheck(struct kinetis_qh_s *qh, uint32_t **bp, void *arg
   struct kinetis_epinfo_s *epinfo;
   uint32_t token;
   int ret;
+  uint32_t regval;
 
   DEBUGASSERT(qh && bp);
 
@@ -2949,9 +2949,12 @@ static int kinetis_qh_ioccheck(struct kinetis_qh_s *qh, uint32_t **bp, void *arg
         }
 #endif
 
-      /* Then release this QH by returning it to the free list */
+      /* Then start async advance doorbell process */
 
-      kinetis_qh_free(qh);
+      qh->aawait = true;
+
+      regval = kinetis_getreg(&HCOR->usbcmd);
+      kinetis_putreg(regval | EHCI_USBCMD_IAADB, &HCOR->usbcmd);
     }
   else
     {
@@ -3040,12 +3043,6 @@ static int kinetis_qh_cancel(struct kinetis_qh_s *qh, uint32_t **bp, void *arg)
       return OK;
     }
 
-  /* Disable both the asynchronous and period schedules */
-
-  regval = kinetis_getreg(&HCOR->usbcmd);
-  kinetis_putreg(regval & ~(EHCI_USBCMD_ASEN | EHCI_USBCMD_PSEN),
-               &HCOR->usbcmd);
-
   /* Remove the QH from the list
    *
    * NOTE that we don't check if the qTD is active nor do we check if there
@@ -3071,11 +3068,15 @@ static int kinetis_qh_cancel(struct kinetis_qh_s *qh, uint32_t **bp, void *arg)
       usbhost_trace1(EHCI_TRACE1_QTDFOREACH_FAILED, -ret);
     }
 
-  /* Then release this QH by returning it to the free list.  Return 1
-   * to stop the traverse without an error.
-   */
+  /* Then start async advance doorbell process */
+
+  qh->aawait = true;
+
+  regval = kinetis_getreg(&HCOR->usbcmd);
+  kinetis_putreg(regval | EHCI_USBCMD_IAADB, &HCOR->usbcmd);
+
+  /* Return 1 to stop the traverse without an error. */
 
-  kinetis_qh_free(qh);
   return 1;
 }
 #endif /* CONFIG_USBHOST_ASYNCH */
@@ -3338,9 +3339,19 @@ static inline void kinetis_syserr_bottomhalf(void)
 
 static inline void kinetis_async_advance_bottomhalf(void)
 {
+  int i;
   usbhost_vtrace1(EHCI_VTRACE1_AAINTR, 0);
 
-  /* REVISIT: Could remove all tagged QH entries here */
+  for (i = 0; i < CONFIG_KINETIS_EHCI_NQHS; i++)
+    {
+      /* Put the QH structure in the free list if tagged */
+
+      if (g_qhpool[i].aawait)
+        {
+          g_qhpool[i].aawait = false;
+          kinetis_qh_free(&g_qhpool[i]);
+        }
+    }
 }
 
 /************************************************************************************
@@ -3363,7 +3374,25 @@ static void kinetis_ehci_bottomhalf(FAR void *arg)
   kinetis_takesem_noncancelable(&g_ehci.exclsem);
 
   /* Handle all unmasked interrupt sources
-   * USB Interrupt (USBINT)
+   * Interrupt on Async Advance
+   *
+   *  "System software can force the host controller to issue an interrupt
+   *   the next time the host controller advances the asynchronous schedule
+   *   by writing a one to the Interrupt on Async Advance Doorbell bit in
+   *   the USBCMD register. This status bit indicates the assertion of that
+   *   interrupt source."
+   *
+   *   Must be first because later more QH can become unlinked.
+   */
+
+  if ((pending & EHCI_INT_AAINT) != 0)
+    {
+      uerr("Async Advance\n");
+      kinetis_async_advance_bottomhalf();
+      kinetis_putreg(EHCI_INT_AAINT, &HCOR->usbsts);
+    }
+
+  /* USB Interrupt (USBINT)
    *
    *  "The Host Controller sets this bit to 1 on the completion of a USB
    *   transaction, which results in the retirement of a Transfer Descriptor
@@ -3396,6 +3425,7 @@ static void kinetis_ehci_bottomhalf(FAR void *arg)
         }
 
       kinetis_ioc_bottomhalf();
+      kinetis_putreg(EHCI_INT_USBINT | EHCI_INT_USBERRINT, &HCOR->usbsts);
     }
 
   /* Port Change Detect
@@ -3418,6 +3448,7 @@ static void kinetis_ehci_bottomhalf(FAR void *arg)
   if ((pending & EHCI_INT_PORTSC) != 0)
     {
       kinetis_portsc_bottomhalf();
+      kinetis_putreg(EHCI_INT_PORTSC, &HCOR->usbsts);
     }
 
   /* Frame List Rollover
@@ -3436,6 +3467,7 @@ static void kinetis_ehci_bottomhalf(FAR void *arg)
   if ((pending & EHCI_INT_FLROLL) != 0)
     {
       kinetis_flroll_bottomhalf();
+      kinetis_putreg(EHCI_INT_FLROLL, &HCOR->usbsts);
     }
 #endif
 
@@ -3452,21 +3484,7 @@ static void kinetis_ehci_bottomhalf(FAR void *arg)
     {
       uerr("Syserror\n");
       kinetis_syserr_bottomhalf();
-    }
-
-  /* Interrupt on Async Advance
-   *
-   *  "System software can force the host controller to issue an interrupt
-   *   the next time the host controller advances the asynchronous schedule
-   *   by writing a one to the Interrupt on Async Advance Doorbell bit in
-   *   the USBCMD register. This status bit indicates the assertion of that
-   *   interrupt source."
-   */
-
-  if ((pending & EHCI_INT_AAINT) != 0)
-    {
-      uerr("Async Advance\n");
-      kinetis_async_advance_bottomhalf();
+      kinetis_putreg(EHCI_INT_SYSERROR, &HCOR->usbsts);
     }
 
   /* We are done with the EHCI structures */
@@ -3523,16 +3541,10 @@ static int kinetis_ehci_interrupt(int irq, FAR void *context, FAR void *arg)
                             (FAR void *)pending, 0));
 
       /* Disable further EHCI interrupts so that we do not overrun the work
-       * queue.
+       * queue. We acknowledge the interrupts after servicing.
        */
 
       kinetis_putreg(0, &HCOR->usbintr);
-
-      /* Clear all pending status bits by writing the value of the pending
-       * interrupt bits back to the status register.
-       */
-
-      kinetis_putreg(usbsts & EHCI_INT_ALLINTS, &HCOR->usbsts);
     }
 
   return OK;
@@ -4986,7 +4998,7 @@ static int kinetis_reset(void)
 
   do
     {
-      /* Wait five microsecondw and update the timeout counter */
+      /* Wait five microseconds and update the timeout counter */
 
       up_udelay(5);
       timeout += 5;
@@ -5287,7 +5299,7 @@ FAR struct usbhost_connection_s *kinetis_ehci_initialize(int controller)
    */
 
   memset(&g_asynchead, 0, sizeof(struct kinetis_qh_s));
-  physaddr = kinetis_physramaddr((uintptr_t) & g_asynchead);
+  physaddr = kinetis_physramaddr((uintptr_t)&g_asynchead);
   g_asynchead.hw.hlp = kinetis_swap32(physaddr | QH_HLP_TYP_QH);
   g_asynchead.hw.epchar = kinetis_swap32(QH_EPCHAR_H | QH_EPCHAR_EPS_FULL);
   g_asynchead.hw.overlay.nqp = kinetis_swap32(QH_NQP_T);