From: Andrea della Porta <aporta@suse.de>
To: Minas Harutyunyan <Minas.Harutyunyan@synopsys.com>
Cc: Oliver Neukum <oneukum@suse.com>,
"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
Ivan Ivanov <ivan.ivanov@suse.com>,
Andrea della Porta <andrea.porta@suse.com>
Subject: Re: [PATCH] USB: dwc2: write HCINT with INTMASK applied
Date: Tue, 28 Nov 2023 10:00:10 +0100 [thread overview]
Message-ID: <ZWWsGknhNuVggNNa@apocalypse> (raw)
In-Reply-To: <f293d306-54fb-ecb5-4515-70a6c1faf1b1@synopsys.com>
Hi Minas,
On 10:46 Mon 27 Nov , Minas Harutyunyan wrote:
> Hi Andrea,
>
> On 11/27/23 13:04, Andrea della Porta wrote:
> > 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
> According your patch terminology above 'hcint' is a 'hcintraw' and here
> asserted only channel halted interrupt. So, channel halted without any
> reason. Not clear how your patch fix this case?
>
> Couple of questions related to your tests:
> 1. Which DMA mode used here - buffer or descriptor DMA?
> 2. Which type of transfers testing? Or you can add to this print HCCHARx
> also.
> 3. Which version of core you use (GSNPSID)?
>
> Thanks,
> Minas
>
The augmented log is here below (hcchar[chnum] and others added):
[330438.623017] --Host Channel Interrupt--, Channel 4
[330438.623029] hcint 0x00000012, hcintmsk 0x00000006, hcint&hcintmsk 0x00000002
[330438.623039] hcchar[4] = 0x015c9810, chan->ep_type=3
[330438.626183] --Host Channel Interrupt--, Channel 5
[330438.626198] hcint 0x00000003, hcintmsk 0x00000006, hcint&hcintmsk 0x00000002
[330438.626208] hcchar[5] = 0x01d83200, chan->ep_type=2
[330438.627659] --Host Channel Interrupt--, Channel 3
[330438.627674] hcint 0x00000021, hcintmsk 0x00000426, hcint&hcintmsk 0x00000020
[330438.627686] hcchar[3] = 0x01d8d200, chan->ep_type=2
[330438.627703] --Host Channel Interrupt--, Channel 3
[330438.627711] hcint 0x00000002, hcintmsk 0x00000406, hcint&hcintmsk 0x00000002
[330438.627721] hcchar[3] = 0x01d8d200, chan->ep_type=2
[330438.627740] dwc2 3f980000.usb: dwc2_hc_chhltd_intr_dma: Channel 3 - ChHltd set, but reason is unknown
[330438.627758] dwc2 3f980000.usb: hcint 0x00000002, intsts 0x04600001
[330438.627798] --Host Channel Interrupt--, Channel 0
[330438.627807] hcint 0x00000010, hcintmsk 0x00000436, hcint&hcintmsk 0x00000010
[330438.627818] hcchar[0] = 0x81d8d200, chan->ep_type=2
[330438.631017] --Host Channel Interrupt--, Channel 1
[330438.631025] hcint 0x00000012, hcintmsk 0x00000006, hcint&hcintmsk 0x00000002
[330438.631035] hcchar[1] = 0x015c9810, chan->ep_type=3
[330438.639019] --Host Channel Interrupt--, Channel 7
[330438.639041] hcint 0x00000012, hcintmsk 0x00000006, hcint&hcintmsk 0x00000002
[330438.639053] hcchar[7] = 0x015c9810, chan->ep_type=3
[330438.647019] --Host Channel Interrupt--, Channel 6
[330438.647040] hcint 0x00000012, hcintmsk 0x00000006, hcint&hcintmsk 0x00000002
[330438.647051] hcchar[6] = 0x015c9810, chan->ep_type=3
[330438.649015] --Host Channel Interrupt--, Channel 4
[330438.649020] hcint 0x00000012, hcintmsk 0x00000006, hcint&hcintmsk 0x00000002
we're using buffer DMA mode (dma_desc_enable=0), we are atrss testing via ping flooding
through an LTE modem attached to USB which should mostly do bulk dataflow (confirmed
by ep_type==2). From regdump, GSNPSID = 0x4f54280a.
Many thanks,
Andrea
> >>>
> >>> Signed-off-by: Oliver Neukum <oneukum@suse.com>
> >>> Tested-by: Ivan Ivanov <ivan.ivanov@suse.com>
> >>> Tested-by: Andrea della Porta <andrea.porta@suse.com>
> >>> ---
> >>> 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
next prev parent reply other threads:[~2023-11-28 9:00 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-15 14:45 [PATCH] USB: dwc2: write HCINT with INTMASK applied Oliver Neukum
2023-11-22 12:35 ` Minas Harutyunyan
2023-11-27 9:04 ` Andrea della Porta
2023-11-27 10:46 ` Minas Harutyunyan
2023-11-28 9:00 ` Andrea della Porta [this message]
2023-11-28 11:48 ` Minas Harutyunyan
2023-11-28 14:43 ` Ivan Ivanov
2023-12-01 10:26 ` Minas Harutyunyan
2023-12-03 6:52 ` Andrea della Porta
2023-12-14 12:23 ` Minas Harutyunyan
2023-12-18 14:36 ` Andrea della Porta
2023-12-19 10:19 ` Minas Harutyunyan
2023-12-19 10:18 ` Minas Harutyunyan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZWWsGknhNuVggNNa@apocalypse \
--to=aporta@suse.de \
--cc=Minas.Harutyunyan@synopsys.com \
--cc=andrea.porta@suse.com \
--cc=gregkh@linuxfoundation.org \
--cc=ivan.ivanov@suse.com \
--cc=linux-usb@vger.kernel.org \
--cc=oneukum@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox