2017-04-08 1:57 GMT+02:00 Thinh Nguyen <thinh.ngu...@synopsys.com>: > 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 <thi...@synopsys.com> > --- > 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
