All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christian Eggers <ceggers@arri.de>
To: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Andrey Smirnov <andrew.smirnov@gmail.com>,
	barebox@lists.infradead.org,
	Primoz Fiser <primoz.fiser@norik.com>
Subject: Re: Some USB memory sticks not (fully) recognized
Date: Tue, 11 Aug 2020 08:10:17 +0200	[thread overview]
Message-ID: <1867530.d7CUNdKKll@n95hx1g2> (raw)
In-Reply-To: <20200810192643.GB31536@pengutronix.de>

Hi Sascha,

On Monday, 10 August 2020, 21:26:43 CEST, Sascha Hauer wrote:
> Hi Christian,
> 
> On Mon, Aug 10, 2020 at 01:52:01PM +0200, Christian Eggers wrote:
> > On Thursday, 6 August 2020, 16:29:13 CEST, Christian Eggers wrote:
> > > I got a bug report that some newer USB memory sticks do not work with
> > > barebox. While with barebox-2020.01 these devices are not recognized at
> > > all, in barebox-2020.07 I get at least some warnings:
> > I checked again with an older release of barebox (2019-06). With this
> > version, the USB drive is detected:
> > [...]
> > Also mounting the drive works fine. So I guess that this problem may have
> > been introduced here:
> > 
> > b1d9837182 ("usb: Change power-on / scanning timeout handling")
> 
> Before guessing further could you verify that exactly this commit breaks
> your setup? There are a few more changes in the area that could be the
> culprit.
sorry, that was nonprofessional. In the meantime, I got some new findings:

- v2019.06 is the last release were the device is detected

- reverting b1d9837182 ("usb: Change power-on / scanning timeout handling")
doesn't change anything (so it's not related to this problem)

- the problem was definitely introduced in
6044d6c08e ("usb: host: ehci: Use to USBSTS to wait for transfer completion")

Debugging with and without 6044d6c08e showed that waiting for USBSTS:INT
is not sufficient as QT_TOKEN_STATUS_ACTIVE is still set after this. Turning
the dev_dbg() into an dev_err() clearly shows this:

> usb: USB: scanning bus for devices...
> usb1: Bus 001 Device 001: ID 0000:0000 EHCI Host Controller
> usb1-0: Bus 001 Device 002: ID 0424:4916 USB4916
> ERROR: imx-usb 2184200.usb@2184200.of: dev=3, usbsts=0x40081, p[1]=0x18001205, p[2]=0x0
> ERROR: imx-usb 2184200.usb@2184200.of: dev=3, usbsts=0x40081, p[1]=0x18001205, p[2]=0x0
> ERROR: usb1-0-1: unable to get descriptor, error 80000000
> ERROR: imx-usb 2184200.usb@2184200.of: dev=3, usbsts=0x40081, p[1]=0x18001205, p[2]=0x0
> ERROR: imx-usb 2184200.usb@2184200.of: dev=3, usbsts=0x40081, p[1]=0x18001205, p[2]=0x0
> ERROR: imx-usb 2184200.usb@2184200.of: dev=3, usbsts=0x40081, p[1]=0x18001205, p[2]=0x0
> ERROR: imx-usb 2184200.usb@2184200.of: dev=3, usbsts=0x40081, p[1]=0x18001205, p[2]=0x0
> ERROR: imx-usb 2184200.usb@2184200.of: dev=3, usbsts=0x40081, p[1]=0x18001205, p[2]=0x0
> ERROR: imx-usb 2184200.usb@2184200.of: dev=3, usbsts=0x40081, p[1]=0x18001205, p[2]=0x0
> ERROR: imx-usb 2184200.usb@2184200.of: dev=3, usbsts=0x40081, p[1]=0x18001205, p[2]=0x0
> ERROR: imx-usb 2184200.usb@2184200.of: dev=3, usbsts=0x40081, p[1]=0x18001205, p[2]=0x0
> ERROR: imx-usb 2184200.usb@2184200.of: dev=3, usbsts=0x40081, p[1]=0x18001205, p[2]=0x0
> ERROR: imx-usb 2184200.usb@2184200.of: dev=3, usbsts=0x40081, p[1]=0x18001205, p[2]=0x0
> ERROR: imx-usb 2184200.usb@2184200.of: dev=3, usbsts=0x40081, p[1]=0x18001205, p[2]=0x0
> ERROR: imx-usb 2184200.usb@2184200.of: dev=3, usbsts=0x40081, p[1]=0x18001205, p[2]=0x0
> usb1-0-1: Bus 001 Device 003: ID 090c:3267 
> usb1-0-6: Bus 001 Device 004: ID 0424:494a USB2 Controller Hub
> usb: 4 USB Device(s) found

It seems that I can take multiple USBINTs until QT_TOKEN_STATUS_ACTIVE is
cleared. The following snippet works fine for me:

	volatile struct qTD *vtd;
	...
	vtd = td;
	do {
		ret = handshake(&ehci->hcor->or_usbsts, STS_USBINT, STS_USBINT,
				timeout_ms * 1000);
		if (ret < 0) {
			dev_err(ehci->dev, "handshake failed: %d\n", ret);
			ehci_enable_async_schedule(ehci, false);
			ehci_writel(&qh->qt_token, 0);
			return -ETIMEDOUT;
		}
		token = hc32_to_cpu(vtd->qt_token);
	} while (token & QT_TOKEN_STATUS_ACTIVE);

There is probably no benefit left compared to the version prior 6044d6c08e, so
if there is not better way to do this, I would propose to revert 6044d6c08e.

regards
Christian




_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

  reply	other threads:[~2020-08-11  6:10 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-06 14:29 Some USB memory sticks not (fully) recognized Christian Eggers
2020-08-10 11:52 ` Christian Eggers
2020-08-10 19:26   ` Sascha Hauer
2020-08-11  6:10     ` Christian Eggers [this message]
2020-08-11  6:33       ` Sascha Hauer
2020-08-12  8:35         ` [PATCH] Revert "usb: host: ehci: Use to USBSTS to wait for transfer completion" Christian Eggers
2020-08-14 13:21           ` Sascha Hauer

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=1867530.d7CUNdKKll@n95hx1g2 \
    --to=ceggers@arri.de \
    --cc=andrew.smirnov@gmail.com \
    --cc=barebox@lists.infradead.org \
    --cc=primoz.fiser@norik.com \
    --cc=s.hauer@pengutronix.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.