Re: [PATCH 1/3] usb: dwc3: gadget: Prevent losing events in event cache

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



2017-04-08 1:57 GMT+02:00 Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx>:
> The dwc3 driver can overwite its previous events if its top half IRQ
> handler gets invoked again before processing the events in the cache. We
> see this as a hang in the file transfer and the host will attempt to
> reset the device. This shouldn't be possible in the dwc3 implementation
> if the HW and the SW are in sync. However, through testing and reading
> the pcie trace, the top half IRQ handler occasionally still gets invoked
> one more time after HW interrupt deassertion. We suspect that there is a
> small detection delay in the SW.
>
> Top half IRQ handler deasserts the interrupt line after it gets the
> event count by writing DWC3_GEVNTSIZ_INTMASK to DWC3_GEVNTSIZ. There's a
> small window for a new event coming in between reading the event count
> and the interrupt deassertion. More generally, we will see 0 event
> count, which should not affect anything.
>
> To avoid this issue, we ensure that the events in the cache are
> processed before checking for the new ones. The bottom half IRQ will
> unmask the DWC3_GEVNTSIZ_INTMASK once it finishes processing the events.
> Top half IRQ handler needs to check whether the DWC3_GEVNTSIZ_INTMASK is
> still set before storing the new events. It also should return
> IRQ_HANDLED if the mask is still set since IRQ handler was invoked for
> our usb interrupt.
>
> Signed-off-by: Thinh Nguyen <thinhn@xxxxxxxxxxxx>
> ---
>  drivers/usb/dwc3/gadget.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 4dc80729ae11..93d98fb7215e 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -3045,6 +3045,10 @@ static irqreturn_t dwc3_check_event_buf(struct dwc3_event_buffer *evt)
>                 return IRQ_HANDLED;
>         }
>
> +       reg = dwc3_readl(dwc->regs, DWC3_GEVNTSIZ(0));
> +       if (reg & DWC3_GEVNTSIZ_INTMASK)
> +               return IRQ_HANDLED;
> +
>         count = dwc3_readl(dwc->regs, DWC3_GEVNTCOUNT(0));
>         count &= DWC3_GEVNTCOUNT_MASK;
>         if (!count)
> @@ -3054,7 +3058,6 @@ static irqreturn_t dwc3_check_event_buf(struct dwc3_event_buffer *evt)
>         evt->flags |= DWC3_EVENT_PENDING;
>
>         /* Mask interrupt */
> -       reg = dwc3_readl(dwc->regs, DWC3_GEVNTSIZ(0));
>         reg |= DWC3_GEVNTSIZ_INTMASK;
>         dwc3_writel(dwc->regs, DWC3_GEVNTSIZ(0), reg);
>
> --
> 2.11.0
>
This seems like a workaround, while when we mask interrupts we should
not get IRQ before BH will done. Current driver implementation base on
this.
We are using 2.60a and didn't see problem you describe. Is it specyfic
for some HW revision?

I can imagine we can have the "last" interrupt and you will return
IRQ_HANDLED. Will HW issue this interrupt once again? If not will we
loose this IRQ (eg. disconnect event)?

BTW, other option here could be using (like Baolin propose some time
ago) dwc->lock in top half while this is held for BH.
Question how long spin_lock() will be held in top half...
I am not sure what option is the best.

BR
Janusz
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux