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.
>
> Signed-off-by: Oliver Neukum <oneukum@xxxxxxxx>
> Tested-by: Ivan Ivanov <ivan.ivanov@xxxxxxxx>
> Tested-by: Andrea della Porta <andrea.porta@xxxxxxxx>
Acked-by: Minas Harutyunyan <hminas@xxxxxxxxxxxx>
> ---
> 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
