Hi Boris, On Sat, Feb 15, 2020 at 02:36:47PM +0900, Boris ARZUR wrote: > Hi Guenter, > > >> The first time around I was 0/ changing fonts 1/ trimming the dump message > >> in the kernel 2/ filming my screen. That's not practical at all... > Mmm, pstore does seem to work on my machine. CHROME_PSTORE is not available > to me because I'm not on x86, I just enabled the rest and nothing pops up > in /sys/fs/pstore... > > So I took pictures (OCR did not help): > - https://funyu.konbu.org/pQUF2P08etcpVxuKZ0A720%2b0/IMG_20200215_134343.jpg.webp > is a stack trace for your earlier patch "min_t", in > https://www.spinics.net/lists/linux-usb/msg191019.html ; > - https://funyu.konbu.org/pQUF2P08etcpVxuKZ0A720%2b0/IMG_20200215_135816.jpg.webp > is a stack trace for your later patch "container_of", in > https://www.spinics.net/lists/linux-usb/msg191074.html . > > Both patches crash (without even loading the machine), but "container_of" is > a bit more resilient. > Please find yet another patch attached. This one applies on top of the two patches I sent you yesterday. This patch still needs more testing, but it or something similar will be essential to fix the problem. Thanks, Guenter --- >From 2ac7dd226942c48532587f69e48491de940176e2 Mon Sep 17 00:00:00 2001 From: Guenter Roeck <linux@xxxxxxxxxxxx> Date: Thu, 20 Feb 2020 12:47:57 -0800 Subject: [PATCH] usb: dwc2: Abort transaction after errors if the receive buffer is full. In some situations, the following error messages are reported. dwc2 ff540000.usb: dwc2_hc_chhltd_intr_dma: Channel 1 - ChHltd set, but reason is unknown dwc2 ff540000.usb: hcint 0x00000002, intsts 0x04000021 This is sometimes followed by: dwc2 ff540000.usb: dwc2_update_urb_state_abn(): trimming xfer length and then: WARNING: CPU: 0 PID: 0 at kernel/v4.19/drivers/usb/dwc2/hcd.c:2913 dwc2_assign_and_init_hc+0x98c/0x990 The warning suggests an odd buffer address to be used for DMA. After the first messages, the receive buffer is full (urb->actual_length >= urb->length). However, the urb is still left in the queue. When it is selected again, the dwc2 hcd code translates this into a 1-block transfer; the length of that transfer depends on the endpoint's maximum block size. If and when that packet is received, it is placed at the end of the receive buffer (which was already full). This can have all kinds of adverse affects. To solve the problem, abort input transactions if there was an error and the receive buffer is full. We could possibly handle this situation as "transfer complete", but that seems risky since we don't really know why the transaction was aborted. Cc: Boris ARZUR <boris@xxxxxxxxx> Cc: Douglas Anderson <dianders@xxxxxxxxxxxx> Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx> --- drivers/usb/dwc2/hcd_intr.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/drivers/usb/dwc2/hcd_intr.c b/drivers/usb/dwc2/hcd_intr.c index a052d39b4375..1d99af809033 100644 --- a/drivers/usb/dwc2/hcd_intr.c +++ b/drivers/usb/dwc2/hcd_intr.c @@ -1167,8 +1167,11 @@ static void dwc2_hc_stall_intr(struct dwc2_hsotg *hsotg, * abnormal condition before the transfer completes. Modifies the * actual_length field of the URB to reflect the number of bytes that have * actually been transferred via the host channel. + * + * Return true if the receive buffer is full on an input transaction, + * false otherwise. */ -static void dwc2_update_urb_state_abn(struct dwc2_hsotg *hsotg, +static bool dwc2_update_urb_state_abn(struct dwc2_hsotg *hsotg, struct dwc2_host_chan *chan, int chnum, struct dwc2_hcd_urb *urb, struct dwc2_qtd *qtd, @@ -1199,6 +1202,8 @@ static void dwc2_update_urb_state_abn(struct dwc2_hsotg *hsotg, urb->actual_length); dev_vdbg(hsotg->dev, " urb->transfer_buffer_length %d\n", urb->length); + + return chan->ep_is_in && urb->actual_length >= urb->length; } /* @@ -1973,10 +1978,15 @@ static void dwc2_hc_chhltd_intr_dma(struct dwc2_hsotg *hsotg, "NYET/NAK/ACK/other in non-error case, 0x%08x\n", chan->hcint); error: - /* Failthrough: use 3-strikes rule */ - qtd->error_count++; - dwc2_update_urb_state_abn(hsotg, chan, chnum, qtd->urb, - qtd, DWC2_HC_XFER_XACT_ERR); + /* + * Failthrough: use 3-strikes rule. Abort input transactions + * after errors if the receive buffer is full. + */ + if (dwc2_update_urb_state_abn(hsotg, chan, chnum, qtd->urb, + qtd, DWC2_HC_XFER_XACT_ERR)) + qtd->error_count = 3; + else + qtd->error_count++; dwc2_hcd_save_data_toggle(hsotg, chan, chnum, qtd); dwc2_halt_channel(hsotg, chan, qtd, DWC2_HC_XFER_XACT_ERR); } -- 2.17.1
