Hi, Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> writes: > 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 interrupts are masked, why would top half get invoked again? Is this, perhaps, related to DWC3 3.00a which has the "Interrupt line doesn't lower when masked" problem? We've added a lot of code to workaround that problem and, apparently, it wasn't enough. In any case, there's no way top half would be invoked again in a properly working DWC3. > 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 it's not about SW and HW being in sync, it's about the HW being quirky, right? > 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. so you don't really know what's going on :-) Why send a patch if you don't understand the problem fully? > 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. this new event can come, and it'll remain in the event buffer. We have masked the IRQ line and this should be enough to make sure IRQ isn't generated again. Why is the IRQ raising again? Which kernel version are you testing? Which DWC3 revision? Do you have interrupt moderation? > 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); NAK, it's clear that you don't understand the problem you're dealing with. Please give further explanations of what's going on. -- balbi
Attachment:
signature.asc
Description: PGP signature
