From 9061809e2d759be98ce0a2a0d6eac1c6b1e77aed Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Fri, 15 May 2015 07:31:13 -0600 Subject: [PATCH] OHCI drivers: Try disabling bulk list when cancelling bulk transfers --- arch/arm/src/lpc17xx/lpc17_usbhost.c | 153 +++++++++++++++++++-------- arch/arm/src/sama5/sam_ohci.c | 64 ++++++++--- 2 files changed, 159 insertions(+), 58 deletions(-) diff --git a/arch/arm/src/lpc17xx/lpc17_usbhost.c b/arch/arm/src/lpc17xx/lpc17_usbhost.c index ea183f1ee3..fe21420221 100644 --- a/arch/arm/src/lpc17xx/lpc17_usbhost.c +++ b/arch/arm/src/lpc17xx/lpc17_usbhost.c @@ -192,7 +192,7 @@ struct lpc17_xfrinfo_s uint8_t *buffer; /* Transfer buffer start */ uint16_t buflen; /* Buffer length */ uint16_t xfrd; /* Number of bytes transferred */ - + #ifdef CONFIG_USBHOST_ASYNCH #if LPC17_IOBUFFERS > 0 /* Remember the allocated DMA buffer address so that it can be freed when @@ -858,22 +858,30 @@ static void lpc17_free_xfrinfo(struct lpc17_xfrinfo_s *xfrinfo) static inline int lpc17_addctrled(struct lpc17_usbhost_s *priv, struct lpc17_ed_s *ed) { + irqstate_t flags; uint32_t regval; + /* Disable control list processing while we modify the list */ + + flags = irqsave(); + regval = lpc17_getreg(LPC17_USBHOST_CTRL); + regval &= ~OHCI_CTRL_CLE; + lpc17_putreg(regval, LPC17_USBHOST_CTRL); + /* Add the new bulk ED to the head of the bulk list */ ed->hw.nexted = lpc17_getreg(LPC17_USBHOST_CTRLHEADED); lpc17_putreg((uint32_t)ed, LPC17_USBHOST_CTRLHEADED); - /* ControlListEnable. This bit is set to enable the processing of the - * Control list. Note: once enabled, it remains enabled and we may even - * complete list processing before we get the bit set. We really - * should never modify the control list while CLE is set. - */ + /* Re-enable control list processing. */ + + lpc17_putreg(0, LPC17_USBHOST_CTRLED); regval = lpc17_getreg(LPC17_USBHOST_CTRL); regval |= OHCI_CTRL_CLE; lpc17_putreg(regval, LPC17_USBHOST_CTRL); + + irqrestore(flags); return OK; } @@ -891,11 +899,17 @@ static inline int lpc17_remctrled(struct lpc17_usbhost_s *priv, struct lpc17_ed_s *curr; struct lpc17_ed_s *prev; struct lpc17_ed_s *head; + irqstate_t flags; uint32_t regval; - /* Find the ED in the control list. NOTE: We really should never be mucking - * with the control list while CLE is set. - */ + /* Disable control list processing while we modify the list */ + + flags = irqsave(); + regval = lpc17_getreg(LPC17_USBHOST_CTRL); + regval &= ~OHCI_CTRL_CLE; + lpc17_putreg(regval, LPC17_USBHOST_CTRL); + + /* Find the ED in the control list. */ head = (struct lpc17_ed_s *)lpc17_getreg(LPC17_USBHOST_CTRLHEADED); for (prev = NULL, curr = head; @@ -918,17 +932,6 @@ static inline int lpc17_remctrled(struct lpc17_usbhost_s *priv, head = (struct lpc17_ed_s *)ed->hw.nexted; lpc17_putreg((uint32_t)head, LPC17_USBHOST_CTRLHEADED); - - /* If the control list is now empty, then disable it. - * This should never happen! - */ - - if (head == NULL) - { - regval = lpc17_getreg(LPC17_USBHOST_CTRL); - regval &= ~OHCI_CTRL_CLE; - lpc17_putreg(regval, LPC17_USBHOST_CTRL); - } } else { @@ -946,6 +949,21 @@ static inline int lpc17_remctrled(struct lpc17_usbhost_s *priv, ed->hw.nexted = 0; } + /* Re-enable control list processing if the control list is still non-empty + * after removing the ED node. + */ + + lpc17_putreg(0, LPC17_USBHOST_CTRLED); + if (lpc17_getreg(LPC17_USBHOST_CTRLHEADED) != 0) + { + /* If the control list is now empty, then disable it */ + + regval = lpc17_getreg(LPC17_USBHOST_CTRL); + regval &= ~OHCI_CTRL_CLE; + lpc17_putreg(regval, LPC17_USBHOST_CTRL); + } + + irqrestore(flags); return OK; } @@ -961,21 +979,30 @@ static inline int lpc17_addbulked(struct lpc17_usbhost_s *priv, struct lpc17_ed_s *ed) { #ifndef CONFIG_USBHOST_BULK_DISABLE + irqstate_t flags; uint32_t regval; + /* Disable bulk list processing while we modify the list */ + + flags = irqsave(); + regval = lpc17_getreg(LPC17_USBHOST_CTRL); + regval &= ~OHCI_CTRL_BLE; + lpc17_putreg(regval, LPC17_USBHOST_CTRL); + /* Add the new bulk ED to the head of the bulk list */ ed->hw.nexted = lpc17_getreg(LPC17_USBHOST_BULKHEADED); lpc17_putreg((uint32_t)ed, LPC17_USBHOST_BULKHEADED); - /* BulkListEnable. This bit is set to enable the processing of the - * Bulk list. Note: once enabled, it remains. We really should - * never modify the bulk list while BLE is set. - */ + /* Re-enable bulk list processing. */ + + lpc17_putreg(0, LPC17_USBHOST_BULKED); regval = lpc17_getreg(LPC17_USBHOST_CTRL); regval |= OHCI_CTRL_BLE; lpc17_putreg(regval, LPC17_USBHOST_CTRL); + + irqrestore(flags); return OK; #else return -ENOSYS; @@ -997,18 +1024,24 @@ static inline int lpc17_rembulked(struct lpc17_usbhost_s *priv, struct lpc17_ed_s *curr; struct lpc17_ed_s *prev; struct lpc17_ed_s *head; + irqstate_t flags; uint32_t regval; - /* Find the ED in the bulk list. NOTE: We really should never be mucking - * with the bulk list while BLE is set. - */ + /* Disable bulk list processing while we modify the list */ + + flags = irqsave(); + regval = lpc17_getreg(LPC17_USBHOST_CTRL); + regval &= ~OHCI_CTRL_BLE; + lpc17_putreg(regval, LPC17_USBHOST_CTRL); + + /* Find the ED in the bulk list. */ head = (struct lpc17_ed_s *)lpc17_getreg(LPC17_USBHOST_BULKHEADED); for (prev = NULL, curr = head; curr && curr != ed; prev = curr, curr = (struct lpc17_ed_s *)curr->hw.nexted); - /* Hmmm.. It would be a bug if we do not find the ED in the bulk list. */ + /* It would be a bug if we do not find the ED in the bulk list. */ DEBUGASSERT(curr != NULL); @@ -1024,15 +1057,6 @@ static inline int lpc17_rembulked(struct lpc17_usbhost_s *priv, head = (struct lpc17_ed_s *)ed->hw.nexted; lpc17_putreg((uint32_t)head, LPC17_USBHOST_BULKHEADED); - - /* If the bulk list is now empty, then disable it */ - - if (head == NULL); - { - regval = lpc17_getreg(LPC17_USBHOST_CTRL); - regval &= ~OHCI_CTRL_BLE; - lpc17_putreg(regval, LPC17_USBHOST_CTRL); - } } else { @@ -1044,6 +1068,21 @@ static inline int lpc17_rembulked(struct lpc17_usbhost_s *priv, } } + /* Re-enable bulk list processing if the bulk list is still non-empty + * after removing the ED node. + */ + + lpc17_putreg(0, LPC17_USBHOST_BULKED); + if (lpc17_getreg(LPC17_USBHOST_BULKHEADED) != 0) + { + /* If the bulk list is now empty, then disable it */ + + regval = lpc17_getreg(LPC17_USBHOST_CTRL); + regval |= OHCI_CTRL_BLE; + lpc17_putreg(regval, LPC17_USBHOST_CTRL); + } + + irqrestore(flags); return OK; #else return -ENOSYS; @@ -3246,6 +3285,7 @@ static int lpc17_cancel(FAR struct usbhost_driver_s *drvr, usbhost_ep_t ep) struct lpc17_gtd_s *td; struct lpc17_gtd_s *next; struct lpc17_xfrinfo_s *xfrinfo; + uint32_t ctrl; irqstate_t flags; DEBUGASSERT(priv != NULL && ed != NULL); @@ -3269,15 +3309,42 @@ static int lpc17_cancel(FAR struct usbhost_driver_s *drvr, usbhost_ep_t ep) if (xfrinfo->wdhwait) #endif { - /* We really need some kind of atomic test and set to do this right */ + /* Control endpoints should not come through this path and + * isochronous endpoints are not yet implemented. So we only have + * to distinguish bulk and interrupt endpoints. + */ - td = (struct lpc17_gtd_s *)(ed->hw.headp & ED_HEADP_ADDR_MASK); - ed->hw.headp = LPC17_TDTAIL_ADDR; - ed->xfrinfo = NULL; + if (ed->xfrtype == USB_EP_ATTR_XFER_BULK) + { + /* Disable bulk list processing while we modify the list */ - /* Free all transfer descriptors that were connected to the ED */ + ctrl = lpc17_getreg(LPC17_USBHOST_CTRL); + lpc17_putreg(ctrl & ~OHCI_CTRL_BLE, LPC17_USBHOST_CTRL); + + /* Remove the TDs attached to the ED, keeping the ED in the list */ + + td = (struct lpc17_gtd_s *)(ed->hw.headp & ED_HEADP_ADDR_MASK); + ed->hw.headp = LPC17_TDTAIL_ADDR; + ed->xfrinfo = NULL; + + /* Re-enable bulk list processing, if it was enabled before */ + + lpc17_putreg(0, LPC17_USBHOST_BULKED); + lpc17_putreg(ctrl, LPC17_USBHOST_CTRL); + } + else + { + /* Remove the TDs attached to the ED, keeping the Ed in the list */ + + td = (struct lpc17_gtd_s *)(ed->hw.headp & ED_HEADP_ADDR_MASK); + ed->hw.headp = LPC17_TDTAIL_ADDR; + ed->xfrinfo = NULL; + } + + /* Free all transfer descriptors that were connected to the ED. In + * some race conditions with the hardware, this might be none. + */ - DEBUGASSERT(td != (struct lpc17_gtd_s *)LPC17_TDTAIL_ADDR); while (td != (struct lpc17_gtd_s *)LPC17_TDTAIL_ADDR) { next = (struct lpc17_gtd_s *)td->hw.nexttd; diff --git a/arch/arm/src/sama5/sam_ohci.c b/arch/arm/src/sama5/sam_ohci.c index 73846d1cb8..684230faed 100644 --- a/arch/arm/src/sama5/sam_ohci.c +++ b/arch/arm/src/sama5/sam_ohci.c @@ -856,6 +856,8 @@ static int sam_addctrled(struct sam_ed_s *ed) /* Re-enable control list processing. */ + sam_putreg(0, SAM_USBHOST_CTRLED); + regval = sam_getreg(SAM_USBHOST_CTRL); regval |= OHCI_CTRL_CLE; sam_putreg(regval, SAM_USBHOST_CTRL); @@ -887,9 +889,7 @@ static inline int sam_remctrled(struct sam_ed_s *ed) regval &= ~OHCI_CTRL_CLE; sam_putreg(regval, SAM_USBHOST_CTRL); - /* Find the ED in the control list. NOTE: We really should never be mucking - * with the control list while BLE is set. - */ + /* Find the ED in the control list. */ physed = sam_getreg(SAM_USBHOST_CTRLHEADED); for (curr = (struct sam_ed_s *)sam_virtramaddr(physed), prev = NULL; @@ -928,6 +928,7 @@ static inline int sam_remctrled(struct sam_ed_s *ed) * after removing the ED node. */ + sam_putreg(0, SAM_USBHOST_CTRLED); if (sam_getreg(SAM_USBHOST_CTRLHEADED) != 0) { /* If the control list is now empty, then disable it */ @@ -973,6 +974,8 @@ static inline int sam_addbulked(struct sam_ed_s *ed) /* Re-enable bulk list processing. */ + sam_putreg(0, SAM_USBHOST_BULKED); + regval = sam_getreg(SAM_USBHOST_CTRL); regval |= OHCI_CTRL_BLE; sam_putreg(regval, SAM_USBHOST_CTRL); @@ -1008,9 +1011,7 @@ static inline int sam_rembulked(struct sam_ed_s *ed) regval &= ~OHCI_CTRL_BLE; sam_putreg(regval, SAM_USBHOST_CTRL); - /* Find the ED in the bulk list. NOTE: We really should never be mucking - * with the bulk list while BLE is set. - */ + /* Find the ED in the bulk list. */ physed = sam_getreg(SAM_USBHOST_BULKHEADED); for (curr = (struct sam_ed_s *)sam_virtramaddr(physed), prev = NULL; @@ -1049,6 +1050,7 @@ static inline int sam_rembulked(struct sam_ed_s *ed) * after removing the ED node. */ + sam_getreg(0, SAM_USBHOST_BULKED); if (sam_getreg(SAM_USBHOST_BULKHEADED) != 0) { /* If the bulk list is now empty, then disable it */ @@ -3646,20 +3648,52 @@ static int sam_cancel(struct usbhost_driver_s *drvr, usbhost_ep_t ep) if (eplist->wdhwait) #endif { - /* We really need some kind of atomic test and set to do this right */ + /* Control endpoints should not come through this path and + * isochronous endpoints are not yet implemented. So we only have + * to distinguish bulk and interrupt endpoints. + */ - paddr = ed->hw.headp & ED_HEADP_ADDR_MASK; - td = (struct sam_gtd_s *)sam_virtramaddr(paddr); + if (ed->xfrtype == USB_EP_ATTR_XFER_BULK) + { + /* Disable bulk list processing while we modify the list */ - paddr = sam_physramaddr((uintptr_t)eplist->tail); - ed->hw.headp = paddr; + ctrl = sam_getreg(SAM_USBHOST_CTRL); + sam_putreg(ctrl & ~OHCI_CTRL_BLE, SAM_USBHOST_CTRL); - arch_clean_dcache((uintptr_t)ed, - (uintptr_t)ed + sizeof(struct ohci_ed_s)); + /* Remove the TDs attached to the ED, keeping the ED in the list */ - /* Free all transfer descriptors that were connected to the ED */ + paddr = ed->hw.headp & ED_HEADP_ADDR_MASK; + td = (struct sam_gtd_s *)sam_virtramaddr(paddr); + + paddr = sam_physramaddr((uintptr_t)eplist->tail); + ed->hw.headp = paddr; + + arch_clean_dcache((uintptr_t)ed, + (uintptr_t)ed + sizeof(struct ohci_ed_s)); + + /* Re-enable bulk list processing, if it was enabled before */ + + sam_putreg(0, SAM_USBHOST_BULKED); + sam_putreg(ctrl, SAM_USBHOST_CTRL); + } + else + { + /* Remove the TDs attached to the ED, keeping the Ed in the list */ + + paddr = ed->hw.headp & ED_HEADP_ADDR_MASK; + td = (struct sam_gtd_s *)sam_virtramaddr(paddr); + + paddr = sam_physramaddr((uintptr_t)eplist->tail); + ed->hw.headp = paddr; + + arch_clean_dcache((uintptr_t)ed, + (uintptr_t)ed + sizeof(struct ohci_ed_s)); + } + + /* Free all transfer descriptors that were connected to the ED. In some + * race conditions with the hardware, this might be none. + */ - DEBUGASSERT(td != (struct sam_gtd_s *)eplist->tail); while (td != (struct sam_gtd_s *)eplist->tail) { paddr = (uintptr_t)td->hw.nexttd;