On 12:35 Wed 22 Nov , Minas Harutyunyan wrote:
> Hi Oliver,
>
> On 11/15/23 18:45, Oliver Neukum wrote:
> > dwc2_hc_n_intr() writes back INTMASK as read but evaluates it
> > with intmask applied. In stress testing this causes spurious
> > interrupts like this:
> >
> > [Mon Aug 14 10:51:07 2023] dwc2 3f980000.usb: dwc2_hc_chhltd_intr_dma: Channel 7 - ChHltd set, but reason is unknown
> > [Mon Aug 14 10:51:07 2023] dwc2 3f980000.usb: hcint 0x00000002, intsts 0x04600001
> > [Mon Aug 14 10:51:08 2023] dwc2 3f980000.usb: dwc2_hc_chhltd_intr_dma: Channel 0 - ChHltd set, but reason is unknown
> > [Mon Aug 14 10:51:08 2023] dwc2 3f980000.usb: hcint 0x00000002, intsts 0x04600001
> > [Mon Aug 14 10:51:08 2023] dwc2 3f980000.usb: dwc2_hc_chhltd_intr_dma: Channel 4 - ChHltd set, but reason is unknown
> > [Mon Aug 14 10:51:08 2023] dwc2 3f980000.usb: hcint 0x00000002, intsts 0x04600001
> > [Mon Aug 14 10:51:08 2023] dwc2 3f980000.usb: dwc2_update_urb_state_abn(): trimming xfer length
> >
> > Applying INTMASK prevents this. The issue exists in all versions of the
> > driver.
>
> I'm Ok with this patch, just need some elaboration.
> So, channel halted interrupt asserted due to some other interrupt (AHB
> Error, Excessive transaction errors, Babble, Stall) which was masked.
> Can you check which of masked interrupts is cause of channel halt interrupt?
>
> Thanks,
> Minas
>
Hi Minas,
here's the report from dmesg:
[209384.238724] hcint 0x00000012, hcintmsk 0x00000006, hcint&hcintmsk 0x00000002
[209384.246725] hcint 0x00000012, hcintmsk 0x00000006, hcint&hcintmsk 0x00000002
[209384.247634] hcint 0x00000003, hcintmsk 0x00000006, hcint&hcintmsk 0x00000002
[209384.254722] hcint 0x00000012, hcintmsk 0x00000006, hcint&hcintmsk 0x00000002
[209384.262725] hcint 0x00000012, hcintmsk 0x00000006, hcint&hcintmsk 0x00000002
[209384.270724] hcint 0x00000012, hcintmsk 0x00000006, hcint&hcintmsk 0x00000002
[209384.278336] hcint 0x00000092, hcintmsk 0x00000006, hcint&hcintmsk 0x00000002
[209384.278384] hcint 0x00000010, hcintmsk 0x00000436, hcint&hcintmsk 0x00000010
[209384.278720] hcint 0x00000012, hcintmsk 0x00000006, hcint&hcintmsk 0x00000002
[209384.286723] hcint 0x00000012, hcintmsk 0x00000006, hcint&hcintmsk 0x00000002
[209384.288486] hcint 0x00000021, hcintmsk 0x00000426, hcint&hcintmsk 0x00000020
[209384.288511] hcint 0x00000002, hcintmsk 0x00000406, hcint&hcintmsk 0x00000002
[209384.288528] dwc2 3f980000.usb: dwc2_hc_chhltd_intr_dma: Channel 1 - ChHltd set, but reason is unknown
[209384.288604] hcint 0x00000010, hcintmsk 0x00000436, hcint&hcintmsk 0x00000010
[209384.294720] hcint 0x00000012, hcintmsk 0x00000006, hcint&hcintmsk 0x00000002
[209384.302734] hcint 0x00000012, hcintmsk 0x00000006, hcint&hcintmsk 0x00000002
[209384.310724] hcint 0x00000012, hcintmsk 0x00000006, hcint&hcintmsk 0x00000002
[209384.318721] hcint 0x00000012, hcintmsk 0x00000006, hcint&hcintmsk 0x00000002
[209384.321722] hcint 0x00000012, hcintmsk 0x00000006, hcint&hcintmsk 0x00000002
[209384.326729] hcint 0x00000012, hcintmsk 0x00000006, hcint&hcintmsk 0x00000002
Many thanks,
Andrea
> >
> > Signed-off-by: Oliver Neukum <oneukum@xxxxxxxx>
> > Tested-by: Ivan Ivanov <ivan.ivanov@xxxxxxxx>
> > Tested-by: Andrea della Porta <andrea.porta@xxxxxxxx>
> > ---
> > drivers/usb/dwc2/hcd_intr.c | 15 +++++++--------
> > 1 file changed, 7 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/usb/dwc2/hcd_intr.c b/drivers/usb/dwc2/hcd_intr.c
> > index 0144ca8350c3..5c7538d498dd 100644
> > --- a/drivers/usb/dwc2/hcd_intr.c
> > +++ b/drivers/usb/dwc2/hcd_intr.c
> > @@ -2015,15 +2015,17 @@ static void dwc2_hc_n_intr(struct dwc2_hsotg *hsotg, int chnum)
> > {
> > struct dwc2_qtd *qtd;
> > struct dwc2_host_chan *chan;
> > - u32 hcint, hcintmsk;
> > + u32 hcint, hcintraw, hcintmsk;
> >
> > chan = hsotg->hc_ptr_array[chnum];
> >
> > - hcint = dwc2_readl(hsotg, HCINT(chnum));
> > + hcintraw = dwc2_readl(hsotg, HCINT(chnum));
> > hcintmsk = dwc2_readl(hsotg, HCINTMSK(chnum));
> > + hcint = hcintraw & hcintmsk;
> > + dwc2_writel(hsotg, hcint, HCINT(chnum));
> > +
> > if (!chan) {
> > dev_err(hsotg->dev, "## hc_ptr_array for channel is NULL ##\n");
> > - dwc2_writel(hsotg, hcint, HCINT(chnum));
> > return;
> > }
> >
> > @@ -2032,11 +2034,9 @@ static void dwc2_hc_n_intr(struct dwc2_hsotg *hsotg, int chnum)
> > chnum);
> > dev_vdbg(hsotg->dev,
> > " hcint 0x%08x, hcintmsk 0x%08x, hcint&hcintmsk 0x%08x\n",
> > - hcint, hcintmsk, hcint & hcintmsk);
> > + hcintraw, hcintmsk, hcint);
> > }
> >
> > - dwc2_writel(hsotg, hcint, HCINT(chnum));
> > -
> > /*
> > * If we got an interrupt after someone called
> > * dwc2_hcd_endpoint_disable() we don't want to crash below
> > @@ -2046,8 +2046,7 @@ static void dwc2_hc_n_intr(struct dwc2_hsotg *hsotg, int chnum)
> > return;
> > }
> >
> > - chan->hcint = hcint;
> > - hcint &= hcintmsk;
> > + chan->hcint = hcintraw;
> >
> > /*
> > * If the channel was halted due to a dequeue, the qtd list might
