From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752992AbbJEUCx (ORCPT ); Mon, 5 Oct 2015 16:02:53 -0400 Received: from comal.ext.ti.com ([198.47.26.152]:41525 "EHLO comal.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752919AbbJEUCv (ORCPT ); Mon, 5 Oct 2015 16:02:51 -0400 From: Felipe Balbi To: Gregory CLEMENT CC: Greg Kroah-Hartman , , , , Subject: Re: [PATCH] usb: musb: dsps: handle the otg_state_a_wait_vrise_timeout case In-Reply-To: <87twrfsbed.fsf@free-electrons.com> References: <1440087153-31084-1-git-send-email-gregory.clement@free-electrons.com> <55D719B1.8010700@free-electrons.com> <87twrfsbed.fsf@free-electrons.com> User-Agent: Notmuch/0.20.2 (http://notmuchmail.org) Emacs/24.5.1 (x86_64-pc-linux-gnu) Date: Mon, 5 Oct 2015 15:02:20 -0500 Message-ID: <87d1wtf5ir.fsf@saruman.tx.rr.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha1; protocol="application/pgp-signature" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Gregory CLEMENT writes: > Hi Felipe, >=20=20 > On ven., ao=C3=BBt 21 2015, Gregory CLEMENT wrote: >>> According to the OTG specification after a timeout of >>> OTG_TIME_A_WAIT_VRISE (the maximum value is 100ms) the driver must >>> move from the state a_wait_vrise to the state a_wait_bcon. However, >>> the dsps version of musb does not handle this case. >>>=20 >>> Without it, the driver could remain stuck in the a_wait_vrise. It can >>> be reproduce with the following steps: >>>=20 >>> 1) Boot a board with no USB adapter inserted >>> 2) Insert an empty OTG adapter >>> 3) Wait 2 seconds then remove the OTG adapter >>> 4) The unit is now stuck in host mode, the VBUS switch is latched on >>> and the ID pin is no longer polled. >>>=20 >>> The only way to exit this state was to insert a OTG adapter with an >>> USB device connected. Until this, the usb device mode was not >>> available. >>>=20 >>> It was tested on a AM35x based board. >>>=20 >>> CC: >>> Signed-off-by: Gregory CLEMENT >>> --- >>> drivers/usb/musb/musb_dsps.c | 14 +++++++++++++- >>> 1 file changed, 13 insertions(+), 1 deletion(-) >>>=20 >>> diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c >>> index 65d931a..2d22683 100644 >>> --- a/drivers/usb/musb/musb_dsps.c >>> +++ b/drivers/usb/musb/musb_dsps.c >>> @@ -145,6 +145,7 @@ struct dsps_glue { >>> struct timer_list timer; /* otg_workaround timer */ >>> unsigned long last_timer; /* last timer data for each instance */ >>> bool sw_babble_enabled; >>> + int otg_state_a_wait_vrise_timeout; >>>=20=20 >>> struct dsps_context context; >>> struct debugfs_regset32 regset; >>> @@ -268,9 +269,18 @@ static void otg_timer(unsigned long _musb) >>>=20=20 >>> spin_lock_irqsave(&musb->lock, flags); >>> switch (musb->xceiv->otg->state) { >>> + case OTG_STATE_A_WAIT_VRISE: >>> + if ((glue->otg_state_a_wait_vrise_timeout)) { >>> + musb->xceiv->otg->state =3D OTG_STATE_A_WAIT_BCON; >>> + musb->is_active =3D 0; >>> + } >>> + mod_timer(&glue->timer, jiffies + >>> + msecs_to_jiffies(OTG_TIME_A_WAIT_VRISE)); >> >> After more test on more USB drive, it seems that for some of them >> OTG_TIME_A_WAIT_VRISE is too short. 200ms seems better. It is >> disturbing because according to the OTG specification the maximum >> is 100ms, however I am not so surprised that USB device maker don't >> follow it. > > So after many tests on different devices, 200ms is enough for most of > them, but for one, 2000ms (2s) was needed! > > I see several option: > - adding a sysfs entry to setup the time > - adding a debugs entry entry > - adding configuration option in menuconfig > - using 2000ms and hopping it was enough for everyone > > My preference would go to the first option, what is your opinion? I'm not sure if either of them is good. man 2s is just too large. If the device isn't following the specification, I'm afraid that device needs to be fixed. I think the real issue here is the lack of a disconnect IRQ from AM335x devices. But here's something I've been meaning to test but never had time. If you look into AM335x address space, there's a bit in the wrapper which tells it to use the standard MUSB registers for IRQ, instead of the TI-specific thing. Can you see if we get a disconnect IRQ with that ? TRM is at [1], see page 2566. Basically, if you set bit 3 in register offset 0x1014, then it should use Mentor IRQ registers. If that works, quite a few problems will actually vanish :-p [1] http://www.ti.com/lit/ug/spruh73l/spruh73l.pdf =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJWEtdOAAoJEIaOsuA1yqRECTQQALJlgkwBNxvPcFUOzqcwZgM3 k0anZq/RY+zfZDJGPwFEBzhu+PSuTIhnxuomk70P0TuTbdgnKQtcQI4afgEzcONt L1r9rSJD6kDYfUyZZyPnZGTFq7cct9EKPJQLpzmDfjGlEhwUVfdpxMtjxa6YM1LM QQMZbgSi4YxpD6gSm0UasOFR2J/H3CtEZs/CG9zdzABZNFU7qwBc7ew8/9Q8wbQP 7S3K4ggPTg4uhj4an/jkhVTh9lFqn4Heu06SMbspb/13dzn4+YtYSBOEpkg6sNJx 3u3rgrjLl/Bhefy+S4BRJULGqHuTQJ1uDcZC6fYa8PnW5Gx3d54JCaB8w5NFEfjF XIxivEJ5kufdzJ/iY2OuhVk4gSAWZpOxZVDOhASeAg06uSOhts/yP+XJB1/llo4I HPeHn3cSNFQeTEJ8wfVDU61WjFe6VkvjVQ02dx4QjvVljYDlw3DrTay8W1kD+nIe P9Xb6B0CFQIhkKt3eEjiOWQ671oxpNgpznjClHrDAS9rVIVgNnnC3mEfuVcfJ0OL 5gdAaEXIctknxitt6iqxrL9Rm76xGEas6ykJnRhsZVpf+WubTuCObedeB6XUYrXW eWH9UvN9GC9bmLLWjqRpvgtC9mb5T66hgXCewQWrAdYhVGTLb35pX2GC4jjIWUxk hi3ogSjAVuzg0Myrpact =FTpb -----END PGP SIGNATURE----- --=-=-=--