Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 3 Apr 2018 11:29:10 +0200
From:   Maxime Ripard <maxime.ripard@bootlin.com>
To:     Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Cc:     linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Bin Liu <b-liu@ti.com>, Chen-Yu Tsai <wens@csie.org>,
        Paul Kocialkowski <contact@paulk.fr>
Subject: Re: [PATCH] usb: musb: Support gadget mode when the port is set to
 dual role
Message-ID: <20180403092910.wcgjn3r4job5y5pb@flea>
References: <20180328215213.29538-1-contact@paulk.fr>
 <20180329092326.dayuccomq5zrywqo@flea>
 <1522324644.1746.19.camel@bootlin.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="lilkrb6lwbhrider"
Content-Disposition: inline
In-Reply-To: <1522324644.1746.19.camel@bootlin.com>
User-Agent: NeoMutt/20180323
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


--lilkrb6lwbhrider
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hi,

On Thu, Mar 29, 2018 at 01:57:24PM +0200, Paul Kocialkowski wrote:
> On Thu, 2018-03-29 at 11:23 +0200, Maxime Ripard wrote:
> > On Wed, Mar 28, 2018 at 11:52:13PM +0200, Paul Kocialkowski wrote:
> > > This allows dual-role ports to be reported as having gadget mode by
> > > the
> > > musb_has_gadget helper. This is required to enable MUSB at all with
> > > MUSB
> > > glue layers that set the port mode to MUSB_PORT_MODE_DUAL_ROLE at
> > > init.
> > >=20
> > > Most notably, this allows calling musb_start when needed in the
> > > virtual
> > > MUSB root HUB, regardless of whether the current mode should be
> > > gadget
> > > or host.
> > >=20
> > > This fixes USB OTG on Allwinner devices that I could test it with,
> > > mainly A20 devices.
> > >=20
> > > Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> >=20
> > Surely there's more to it than that. The gadget mode of A20 boards
> > have been working in the past, including when compiling with mUSB
> > setup as dual role.
> >
> > Is this a regression since a particular commit? Or is there another,
> > deeper issue overlooked in the commit log?
>=20
> The root of the issue here is that musb_start is not called at any point
> without this patch. My understanding of the flow is the following: when
> the PHY detects that there was a VBUS/ID change, it will notify its
> listeners (mainly the musb sunxi glue layer). This will then schedule
> the driver's work (sunxi_musb_work), which does nothing since the
> SUNXI_MUSB_FL_ENABLED bit was never set. This bit is only set after
> calling sunxi_musb_enable, which is called from musb_platform_enable,
> that originates from musb_start.
>=20
> Currently I see two places where musb_start is called:
> * musb_virthub
> * musb_gadget
>=20
> In the latter case, it is in turn called from udc_start, which should
> probably (correct me if I'm wrong) happen later in the call chain than
> ID/VBUS change notification time.
>=20
> In the former case, musb_start is called in the root controller hub
> control, when setting the USB_PORT_FEAT_POWER feature. This looks
> perfectly legit and IMO this is where it should be initially calling
> musb_start in the dual role case. The kernel is indeed setting the
> feature, only that it fails to enable musb without this patch.
>=20
> First, I'd like to make sure that this understanding of the flow is
> correct as I may have missed something here. Does it make sense?
>=20
> Then, it seems that the offending commit is: be9d39881fc4f
> ("usb: musb: host: rely on port_mode to call musb_start()")
>=20
> That itself fixed: ae44df2e21b5
> ("usb: musb: call musb_start() only once in OTG mode")
>=20
> Still, this commit was authored in June 2015, so almost 3 years ago.
> In the meantime, the sunxi driver has received feature improvements, so
> it seems hard to believe that it was broken all this time...

I'm not that knowlegdeable about the musb driver, so I can't really
comment on whether what you're saying actually makes sense, but from
what you seem to say, the issue is just happening upon VBUS / ID
notification.

Have you tested without the role switching? For example, trying to
boot while acting as a gadget?

Maxime

--=20
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

--lilkrb6lwbhrider
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEE0VqZU19dR2zEVaqr0rTAlCFNr3QFAlrDSWUACgkQ0rTAlCFN
r3Q7gQ/7BJcMNegJMeP1qSFmHp2FLfHsrsmzQHx+1O4ilLEDIogfSCo1N1w+6lJY
B7soUSFhAVTgB3X8MulnfGJlB1EF7u/tnJz3/SkRB9lYAJlM/cOSsB2folREfHUC
Q5uPsATtFaI7Hbi98RwL0/bVaY+wyAwIOCKo7e5oVHchqJz4nDYHRUDOZw0A1Hdy
Y+Ph3PjtMhjel2rQT+9RLWu/CNR6yyfbvpEThtY9IzYBq+r1L+XBpOhC9W+CcZfw
bwjiHjoNmZUfn4mEqAaDLoX+BQkVlF9VGyY5L/8sJx4KC76TaSmNt2BIs92JiVZz
PAKcn24Ox3TKPc9Keh+HcfqG/S/ob3a3cQ8pR4F6fa3TRHumr8LsqElAfB5ZNmge
5RwSgkiIFTNFcYHO5Eb04ETkNw8m7JwucI9tlvu7AS0z5Rnbutmn89hRjDc/B2hZ
kb10Buayft1AqxF87FVSybY2jj7CipsKz5CksIJ62/Q2x9/MHHpkvRjmLMMxynlg
SwOsAvbcTsI4g+IN7md/m/ftm7GSiWdK7s7goiDYUz49FXPHFmh6WjqpcE9RrFHF
SSqVpO2IvIb5WRbX+8OjrhTV834OdoMCD3sCqVfmH+GFLnyxxs/Yp+PZgLXtrElQ
bHtVf1B6PMUJ/XDWJFzOUc8jzh/n8OVZNGa+vxGJ/bcKEuB9C+k=
=7LvQ
-----END PGP SIGNATURE-----

--lilkrb6lwbhrider--