From 0883ea1af0898a2b0bfd507a55ccb106a7b13db4 Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Thu, 14 May 2015 09:58:21 -0600 Subject: [PATCH] USB host drivers: Cannot take semaphores in cancel() method --- arch/arm/src/efm32/efm32_usbhost.c | 18 ++++++++---------- arch/arm/src/lpc31xx/lpc31_ehci.c | 3 +++ arch/arm/src/sama5/sam_ehci.c | 3 +++ arch/arm/src/stm32/stm32_otgfshost.c | 18 ++++++++---------- arch/arm/src/stm32/stm32_otghshost.c | 18 ++++++++---------- 5 files changed, 30 insertions(+), 30 deletions(-) diff --git a/arch/arm/src/efm32/efm32_usbhost.c b/arch/arm/src/efm32/efm32_usbhost.c index 16b65f3096..97b75f75d5 100644 --- a/arch/arm/src/efm32/efm32_usbhost.c +++ b/arch/arm/src/efm32/efm32_usbhost.c @@ -2560,7 +2560,7 @@ static inline void efm32_gint_hcinisr(FAR struct efm32_usbhost_s *priv, if (chan->chreason == CHREASON_XFRC) { - /* Set the request done reult */ + /* Set the request done result */ chan->result = OK; } @@ -2629,7 +2629,7 @@ static inline void efm32_gint_hcinisr(FAR struct efm32_usbhost_s *priv, /* For a BULK transfer, the hardware is capable of retrying * automatically on a NAK. However, this is not always * what we need to do. So we always halt the transfer and - * return control to high level logic in the even of a NAK. + * return control to high level logic in the event of a NAK. */ #if 1 @@ -2642,7 +2642,9 @@ static inline void efm32_gint_hcinisr(FAR struct efm32_usbhost_s *priv, efm32_chan_halt(priv, chidx, CHREASON_NAK); } - /* Re-activate CTRL and BULK channels */ + /* Re-activate CTRL and BULK channels. + * REVISIT: This can cause a lot of interrupts! + */ else if (chan->eptype == EFM32_USB_EPTYPE_CTRL || chan->eptype == EFM32_USB_EPTYPE_BULK) @@ -3520,7 +3522,7 @@ static int efm32_gint_isr(int irq, FAR void *context) pending = efm32_getreg(EFM32_USB_GINTSTS); pending &= efm32_getreg(EFM32_USB_GINTMSK); - /* Return from the interrupt when there are no furhter pending + /* Return from the interrupt when there are no further pending * interrupts. */ @@ -3531,7 +3533,6 @@ static int efm32_gint_isr(int irq, FAR void *context) /* Otherwise, process each pending, unmasked GINT interrupts */ - /* Handle the start of frame interrupt */ #ifdef CONFIG_EFM32_OTGFS_SOFINTR @@ -4705,12 +4706,10 @@ static int efm32_cancel(FAR struct usbhost_driver_s *drvr, usbhost_ep_t ep) DEBUGASSERT(priv && chidx < EFM32_MAX_TX_FIFOS); chan = &priv->chan[chidx]; - /* We must have exclusive access to the USB host hardware and state structures. - * And when we have that, we need to disable interrupts to avoid race conditions - * with the asynchronous completion of the transfer being cancelled. + /* We need to disable interrupts to avoid race conditions with the asynchronous + * completion of the transfer being cancelled. */ - efm32_takesem(&priv->exclsem); flags = irqsave(); /* Halt the channel */ @@ -4760,7 +4759,6 @@ static int efm32_cancel(FAR struct usbhost_driver_s *drvr, usbhost_ep_t ep) #endif irqrestore(flags); - efm32_givesem(&priv->exclsem); return OK; } diff --git a/arch/arm/src/lpc31xx/lpc31_ehci.c b/arch/arm/src/lpc31xx/lpc31_ehci.c index b92f80a4fa..332d51cac0 100644 --- a/arch/arm/src/lpc31xx/lpc31_ehci.c +++ b/arch/arm/src/lpc31xx/lpc31_ehci.c @@ -4508,6 +4508,9 @@ static int lpc31_cancel(FAR struct usbhost_driver_s *drvr, usbhost_ep_t ep) /* We must have exclusive access to the EHCI hardware and data structures. This * will prevent servicing any transfer completion events while we perform the * the cancellation, but will not prevent DMA-related race conditions. + * + * REVISIT: This won't work. This function must be callable from the interrupt + * level. */ lpc31_takesem(&g_ehci.exclsem); diff --git a/arch/arm/src/sama5/sam_ehci.c b/arch/arm/src/sama5/sam_ehci.c index e1acab370e..46dff9ca1b 100644 --- a/arch/arm/src/sama5/sam_ehci.c +++ b/arch/arm/src/sama5/sam_ehci.c @@ -4335,6 +4335,9 @@ static int sam_cancel(FAR struct usbhost_driver_s *drvr, usbhost_ep_t ep) /* We must have exclusive access to the EHCI hardware and data structures. This * will prevent servicing any transfer completion events while we perform the * the cancellation, but will not prevent DMA-related race conditions. + * + * REVISIT: This won't work. This function must be callable from the interrupt + * level. */ sam_takesem(&g_ehci.exclsem); diff --git a/arch/arm/src/stm32/stm32_otgfshost.c b/arch/arm/src/stm32/stm32_otgfshost.c index e60a1c70ce..56e09db17a 100644 --- a/arch/arm/src/stm32/stm32_otgfshost.c +++ b/arch/arm/src/stm32/stm32_otgfshost.c @@ -2482,7 +2482,7 @@ static inline void stm32_gint_hcinisr(FAR struct stm32_usbhost_s *priv, if (chan->chreason == CHREASON_XFRC) { - /* Set the request done reult */ + /* Set the request done result */ chan->result = OK; } @@ -2551,7 +2551,7 @@ static inline void stm32_gint_hcinisr(FAR struct stm32_usbhost_s *priv, /* For a BULK transfer, the hardware is capable of retrying * automatically on a NAK. However, this is not always * what we need to do. So we always halt the transfer and - * return control to high level logic in the even of a NAK. + * return control to high level logic in the event of a NAK. */ #if 1 @@ -2564,7 +2564,9 @@ static inline void stm32_gint_hcinisr(FAR struct stm32_usbhost_s *priv, stm32_chan_halt(priv, chidx, CHREASON_NAK); } - /* Re-activate CTRL and BULK channels */ + /* Re-activate CTRL and BULK channels. + * REVISIT: This can cause a lot of interrupts! + */ else if (chan->eptype == OTGFS_EPTYPE_CTRL || chan->eptype == OTGFS_EPTYPE_BULK) @@ -3455,7 +3457,7 @@ static int stm32_gint_isr(int irq, FAR void *context) pending = stm32_getreg(STM32_OTGFS_GINTSTS); pending &= stm32_getreg(STM32_OTGFS_GINTMSK); - /* Return from the interrupt when there are no furhter pending + /* Return from the interrupt when there are no further pending * interrupts. */ @@ -3466,7 +3468,6 @@ static int stm32_gint_isr(int irq, FAR void *context) /* Otherwise, process each pending, unmasked GINT interrupts */ - /* Handle the start of frame interrupt */ #ifdef CONFIG_STM32_OTGFS_SOFINTR @@ -4637,12 +4638,10 @@ static int stm32_cancel(FAR struct usbhost_driver_s *drvr, usbhost_ep_t ep) DEBUGASSERT(priv && chidx < STM32_MAX_TX_FIFOS); chan = &priv->chan[chidx]; - /* We must have exclusive access to the USB host hardware and state structures. - * And when we have that, we need to disable interrupts to avoid race conditions - * with the asynchronous completion of the transfer being cancelled. + /* We need to disable interrupts to avoid race conditions with the asynchronous + * completion of the transfer being cancelled. */ - stm32_takesem(&priv->exclsem); flags = irqsave(); /* Halt the channel */ @@ -4692,7 +4691,6 @@ static int stm32_cancel(FAR struct usbhost_driver_s *drvr, usbhost_ep_t ep) #endif irqrestore(flags); - stm32_givesem(&priv->exclsem); return OK; } diff --git a/arch/arm/src/stm32/stm32_otghshost.c b/arch/arm/src/stm32/stm32_otghshost.c index edcda2d64b..9e9cc0e978 100644 --- a/arch/arm/src/stm32/stm32_otghshost.c +++ b/arch/arm/src/stm32/stm32_otghshost.c @@ -2482,7 +2482,7 @@ static inline void stm32_gint_hcinisr(FAR struct stm32_usbhost_s *priv, if (chan->chreason == CHREASON_XFRC) { - /* Set the request done reult */ + /* Set the request done result */ chan->result = OK; } @@ -2551,7 +2551,7 @@ static inline void stm32_gint_hcinisr(FAR struct stm32_usbhost_s *priv, /* For a BULK transfer, the hardware is capable of retrying * automatically on a NAK. However, this is not always * what we need to do. So we always halt the transfer and - * return control to high level logic in the even of a NAK. + * return control to high level logic in the event of a NAK. */ #if 1 @@ -2564,7 +2564,9 @@ static inline void stm32_gint_hcinisr(FAR struct stm32_usbhost_s *priv, stm32_chan_halt(priv, chidx, CHREASON_NAK); } - /* Re-activate CTRL and BULK channels */ + /* Re-activate CTRL and BULK channels. + * REVISIT: This can cause a lot of interrupts! + */ else if (chan->eptype == OTGHS_EPTYPE_CTRL || chan->eptype == OTGHS_EPTYPE_BULK) @@ -3455,7 +3457,7 @@ static int stm32_gint_isr(int irq, FAR void *context) pending = stm32_getreg(STM32_OTGHS_GINTSTS); pending &= stm32_getreg(STM32_OTGHS_GINTMSK); - /* Return from the interrupt when there are no furhter pending + /* Return from the interrupt when there are no further pending * interrupts. */ @@ -3466,7 +3468,6 @@ static int stm32_gint_isr(int irq, FAR void *context) /* Otherwise, process each pending, unmasked GINT interrupts */ - /* Handle the start of frame interrupt */ #ifdef CONFIG_STM32_OTGHS_SOFINTR @@ -4637,12 +4638,10 @@ static int stm32_cancel(FAR struct usbhost_driver_s *drvr, usbhost_ep_t ep) DEBUGASSERT(priv && chidx < STM32_MAX_TX_FIFOS); chan = &priv->chan[chidx]; - /* We must have exclusive access to the USB host hardware and state structures. - * And when we have that, we need to disable interrupts to avoid race conditions - * with the asynchronous completion of the transfer being cancelled. + /* We need to disable interrupts to avoid race conditions with the asynchronous + * completion of the transfer being cancelled. */ - stm32_takesem(&priv->exclsem); flags = irqsave(); /* Halt the channel */ @@ -4692,7 +4691,6 @@ static int stm32_cancel(FAR struct usbhost_driver_s *drvr, usbhost_ep_t ep) #endif irqrestore(flags); - stm32_givesem(&priv->exclsem); return OK; }