From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: usb: dwc3: of-simple: reset host controller at suspend/resume From: Felipe Balbi Message-Id: <87d0vnslwh.fsf@linux.intel.com> Date: Mon, 16 Jul 2018 10:02:54 +0300 To: Enric Balletbo Serra , ezequiel@collabora.com Cc: Enric Balletbo i Serra , linux-kernel , Brian Norris , kernel@collabora.com, Heiko =?utf-8?Q?St=C3=BCbner?= , Greg Kroah-Hartman , linux-usb@vger.kernel.org, "devicetree@vger.kernel.org" List-ID: SGksCgpFbnJpYyBCYWxsZXRibyBTZXJyYSA8ZWJhbGxldGJvQGdtYWlsLmNvbT4gd3JpdGVzOgo+ IEhpLAo+IE1pc3NhdGdlIGRlIEV6ZXF1aWVsIEdhcmNpYSA8ZXplcXVpZWxAY29sbGFib3JhLmNv bT4gZGVsIGRpYSBkdC4sIDEwCj4gZGUganVsLiAyMDE4IGEgbGVzIDA6MTA6Cj4+Cj4+IFNpZ2gs IG5vdyBhY3R1YWxseSBDYyBkZXZpY2V0cmVlIGZvbGtzCj4+Cj4+IE9uIE1vbiwgMjAxOC0wNy0w OSBhdCAxOTowNCAtMDMwMCwgRXplcXVpZWwgR2FyY2lhIHdyb3RlOgo+PiA+IChDYyBkZXZpY2V0 cmVlIGZvbGtzKQo+PiA+Cj4+ID4gSGkgRW5yaWMsCj4+ID4KPj4gPiBUaGFua3MgZm9yIHRoZSBw YXRjaC4KPj4gPgo+PiA+IE9uIE1vbiwgMjAxOC0wNy0wOSBhdCAxNzowOCArMDIwMCwgRW5yaWMg QmFsbGV0Ym8gaSBTZXJyYSB3cm90ZToKPj4gPiA+IElmIHdlIHBvd2VyIG9mZiB0aGUgU29DIGxv Z2ljIHJhaWwgaW4gUzMsIHdlIGNhbiBmaW5kIHRoYXQgdGhlCj4+ID4gPiBUeXBlLUMKPj4gPiA+ IFBIWSBjYW4ndCBpbml0aWFsaXplIGNvcnJlY3RseSBhZnRlciBzeXN0ZW0gcmVzdW1lLiBXZSBu ZWVkIHRvCj4+ID4gPiB0b2dnbGUKPj4gPiA+IHRoZSBVU0IzLU9URyByZXNldCBiZWZvcmUgdHJ5 aW5nIHRvIGluaXRpYWxpemUgdGhlIFBIWSwgb3IgZWxzZSBpdAo+PiA+ID4gdGltZXMgb3V0Lgo+ PiA+ID4KPj4gPiA+ICAgICBwaHkgcGh5LWZmODAwMDAwLnBoeS45OiBwaHkgcG93ZXJvbiBmYWls ZWQgLS0+IC0xMTAKPj4gPiA+ICAgICBkd2MzIGZlOTAwMDAwLmR3YzM6IGZhaWxlZCB0byBpbml0 aWFsaXplIGNvcmUKPj4gPiA+ICAgICBkd2MzOiBwcm9iZSBvZiBmZTkwMDAwMC5kd2MzIGZhaWxl ZCB3aXRoIGVycm9yIC0xMTAKPj4gPiA+Cj4+ID4gPiBOb3RlIHRoYXQgdGhlIFJLMzM5OSBUUk0g c3VnZ2VzdHMgdGhhdCB3ZSBzaG91bGQga2VlcCB0aGUgd2hvbGUKPj4gPiA+IHVzYjMKPj4gPiA+ IGNvbnRyb2xsZXIgaW4gcmVzZXQgZm9yIHRoZSBkdXJhdGlvbiBvZiB0aGUgVHlwZS1DIFBIWQo+ PiA+ID4gaW5pdGlhbGl6YXRpb24uCj4+ID4gPiBIb3dldmVyLCBpdCdzIGhhcmQgdG8gYXNzZXJ0 IHRoZSByZXNldCBpbiB0aGUgY3VycmVudCBmcmFtZXdvcmsgb2YKPj4gPiA+IHJlc2V0LiBXZSdy ZSBzdGlsbCBza2VwdGljYWwgYWJvdXQgdGhhdCwgYW5kIHdlIGhhdmVuJ3QgeWV0IGZvdW5kIGEK Pj4gPiA+IGNhc2Ugd2hlcmUgdGhpcyBzZWVtcyB0byBoYXZlIG1hdHRlcmVkLiBUaGlzIGFwcHJv YWNoIGlzIG11Y2gKPj4gPiA+IGVhc2llciwKPj4gPiA+IGl0Cj4+ID4gPiBzaW1wbHkgaG9sZHMg dGhlIFVTQjMtT1RHIHJlc2V0IHdoaWxlIGRldmljZSBpcyBzdXBlbmRlZC4KPj4gPiA+Cj4+ID4g PiBUaGUgZHdjMyBjb3JlIGlzIGdvaW5nIHRvIHJlaW5pdGlhbGl6ZSB0aGUgY29udHJvbGxlciBh dAo+PiA+ID4gc3VzcGVuZC9yZXN1bWUKPj4gPiA+IGFueXdheSAoaW5jbHVkaW5nIGEgInNvZnQg cmVzZXQiKSwgc28gaXQgc2hvdWxkIGJlIHNhZmUgdG8gZG8gdGhpcy4KPj4gPiA+Cj4+ID4gPiBT aWduZWQtb2ZmLWJ5OiBFbnJpYyBCYWxsZXRibyBpIFNlcnJhIDxlbnJpYy5iYWxsZXRib0Bjb2xs YWJvcmEuY29tCj4+ID4gPiA+Cj4+ID4gPiAtLS0KPj4gPiA+IERlYXIgYWxsLAo+PiA+ID4KPj4g PiA+IE5vdyB0aGF0IHRoZSB1c2IzLXBoeSBvdGcgcG9ydCBzdXBwb3J0IGZvciByazMzOTkgaGFz IGJlZW4gbWVyZ2VkCj4+ID4gPiBbMV0KPj4gPiA+IHdlCj4+ID4gPiBmb3VuZCB0aGF0IHN1c3Bl bmQvcmVzdW1lIGlzIGJyb2tlbi4gVGhlIHByb2JsZW0gaXMgd2VsbCBrbm93biBmb3IKPj4gPiA+ IENocm9tZU9TIGtlcm5lbHMsIHRoZXkgc29sdmVkIGl0IGluIGEgc2ltaWxhciB3YXkgYWRkaW5n IGEgcmVzZXQKPj4gPiA+IHB1bHNlIG9uCj4+ID4gPiByZXN1bWUgaW4gdGhlIHNwZWNpZmljIHVz YiBnbHVlIGxheWVyIChkd2MzLXJvY2tjaGlwKS4gSW4gbWFpbmxpbmUsCj4+ID4gPiB0aG91Z2gs IHdlIHVzZSB0aGUgZHdjMy1vZi1zaW1wbGUgZ2x1ZSBsYXllciBpbnN0ZWFkIG9mIGEgc3BlY2lm aWMKPj4gPiA+IGxheWVyCj4+ID4gPiBmb3Igcm9ja2NoaXAuIFRoZSBwYXRjaCBpcyBiYXNlZCBv biB0aGUgQnJpYW4gTm9ycmlzIHdvcmsgYnV0Cj4+ID4gPiBzbGlnaHRseQo+PiA+ID4gZGlmZmVy ZW50LCBpdCBob2xkcyB0aGUgcmVzZXQgd2hpbGUgZGV2aWNlIGlzIHN1c3BlbmRlZC4gSXQgd2Fz Cj4+ID4gPiB0ZXN0ZWQKPj4gPiA+IG9uIGEgU2Ftc3VuZyBDaHJvbWVib29rIFBsdXMgd2l0aCB1 c2JjIGRvY2tpbmcgc3RhdGlvbiBhdHRhY2hlZCBieQo+PiA+ID4gZG9pbmcKPj4gPiA+IGRpZmZl cmVudCBzdXNwZW5kL3Jlc3VtZSBjeWNsZXMgYW5kIGNoZWNraW5nIG5vIHVzYiBkZXZpY2VzIGhh cwo+PiA+ID4gYmVlbgo+PiA+ID4gbG9zdC4KPj4gPiA+Cj4+ID4gPiBJIGFtIG5vdCBzdXJlIHRo aXMgaXMgdGhlIGJldHRlciB3YXkgdG8gc29sdmUgdGhpcyBidXQgSSBkaWQgbm90Cj4+ID4gPiBm aW5kCj4+ID4gPiBhbnkgb3RoZXIgd2F5LCBhbmQsIGFzIEkgYW0gbm90IHN1cmUgdGhpcyBjYW4g YmUgZ2VuZXJpYywgdGhlIHJlc2V0Cj4+ID4gPiBpcyBvbmx5Cj4+ID4gPiBkb25lIG9uIHJvY2tj aGlwIHBsYXRmb3Jtcy4KPj4gPiA+Cj4+ID4KPj4gPiBJIGRvbid0IHJlYWxseSB1bmRlcnN0YW5k IHdoeSB0aGVyZSBhcmUgcGVyLXBsYXRmb3JtIGhhY2tzIGluIGdsdWUKPj4gPiBkcml2ZXJzLCBp bnN0ZWFkIG9mIGhhdmluZyBwZXItcGxhdGZvcm0gZ2x1ZSBkcml2ZXJzLCBvciBzb21lIG90aGVy Cj4+ID4gcGx1Z2dhYmxlIGhvb2tzLgo+PiA+Cj4+ID4gPiBCZXN0IHJlZ2FyZHMsCj4+ID4gPiAg RW5yaWMKPj4gPiA+Cj4+ID4gPiBbMV0gYmZkY2ExNzM2ZWE3NjM0NTA3MWJiYzU2MDdkMTg5Mjhl NTQ5MDlhYyAoJ2FybTY0OiBkdHM6Cj4+ID4gPiByb2NrY2hpcDoKPj4gPiA+IGFkZAo+PiA+ID4g ICAgIHVzYjMtcGh5IG90Zy1wb3J0IHN1cHBvcnQgZm9yIHJrMzM5OScpCj4+ID4gPgo+PiA+ID4g IGRyaXZlcnMvdXNiL2R3YzMvZHdjMy1vZi1zaW1wbGUuYyB8IDIxICsrKysrKysrKysrKysrKysr KysrKwo+PiA+ID4gIDEgZmlsZSBjaGFuZ2VkLCAyMSBpbnNlcnRpb25zKCspCj4+ID4gPgo+PiA+ ID4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvdXNiL2R3YzMvZHdjMy1vZi1zaW1wbGUuYwo+PiA+ID4g Yi9kcml2ZXJzL3VzYi9kd2MzL2R3YzMtb2Ytc2ltcGxlLmMKPj4gPiA+IGluZGV4IGRiZWZmNWU2 YWQxNC4uMWQxZWNlOTllZDk0IDEwMDY0NAo+PiA+ID4gLS0tIGEvZHJpdmVycy91c2IvZHdjMy9k d2MzLW9mLXNpbXBsZS5jCj4+ID4gPiArKysgYi9kcml2ZXJzL3VzYi9kd2MzL2R3YzMtb2Ytc2lt cGxlLmMKPj4gPiA+IEBAIC0yMDEsOSArMjAxLDMwIEBAIHN0YXRpYyBpbnQKPj4gPiA+IGR3YzNf b2Zfc2ltcGxlX3J1bnRpbWVfcmVzdW1lKHN0cnVjdAo+PiA+ID4gZGV2aWNlICpkZXYpCj4+ID4g Pgo+PiA+ID4gICAgIHJldHVybiAwOwo+PiA+ID4gIH0KPj4gPiA+ICsKPj4gPiA+ICtzdGF0aWMg aW50IGR3YzNfb2Zfc2ltcGxlX3N1c3BlbmQoc3RydWN0IGRldmljZSAqZGV2KQo+PiA+ID4gK3sK Pj4gPiA+ICsgICBzdHJ1Y3QgZHdjM19vZl9zaW1wbGUgKnNpbXBsZSA9IGRldl9nZXRfZHJ2ZGF0 YShkZXYpOwo+PiA+ID4gKwo+PiA+ID4gKyAgIGlmIChvZl9kZXZpY2VfaXNfY29tcGF0aWJsZShk ZXYtPm9mX25vZGUsCj4+ID4gPiAicm9ja2NoaXAscmszMzk5LQo+PiA+ID4gZHdjMyIpKQo+PiA+ ID4KPj4gPgo+PiA+IEluc3RlYWQgb2YgY2FsbGluZyBvZl9kZXZpY2VfaXNfY29tcGF0aWJsZSBp biBlYWNoIHN1c3BlbmQvcmVzdW1lLAo+PiA+IHdoaWNoIHNlZW1zIHF1aXRlIGV4cGVuc2l2ZSwg aG93IGFib3V0IGhhdmluZyBhIHBlci1kZXZpY2UgYm9vbGVhbgo+PiA+ICduZWVkc19yZXNldCcg b3Igc29tZXRoaW5nIGxpa2UgdGhhdD8KPj4gPgo+IFllcCwgbm90IHN1cmUgaG93IG11Y2gsIGJ1 dCBwcm9iYWJseSB1c2UgYSBib29sZWFuIHdpbGwgYmUgZmFzdGVyLiBJCj4gYW0gYWxzbyB3b25k ZXJpbmcgaWYgd2UgY2FuIHJlbW92ZSB0aGUgb2ZfZGV2aWNlX2lzX2NvbXBhdGlibGUgY2FsbAo+ IGFuZCBkbyB0aGUgcmVzZXQgb24gYWxsIHBsYXRmb3Jtcy4gVGhpcyB3aWxsIG5lZWQgbG90cyBv ZiB0ZXN0cyBvbgo+IGRpZmZlcmVudCBwbGF0Zm9ybXMsIG9mIGNvdXJzZS4gaSdkIGxpa2UgdG8g aGVhciBtYWludGFpbmVycyBmZWVkYmFjawo+IGhlcmUuCgppZGVhbGx5LCB0aGUgcmVzZXQgd291 bGQgYmUgdW5jb25kaXRpb25hbCwgYnV0IGluIHByYWN0aWNlLCBwcm9iYWJseQppdCdzIGhhbmRs ZWQgZGlmZmVyZW50bHkgYnkgZGlmZmVyZW50IGltcGxlbWVudGF0aW9ucy4KCkknZCBiZSBmaW5l IHdpdGggYSBzaW5nbGUgb2ZfZGV2aWNlX2lzX2NvbXBhdGlibGUoKSBjYWxsIGR1cmluZyBwcm9i ZSB0bwpzZXQgYSBkcml2ZXIgZmxhZy4gQW5vdGhlciBwb3NzaWJpbGl0eSB3b3VsZCBiZSB0byB1 c2UgdGhlIGRhdGEgZmllbGQgb2YKb2ZfZGV2aWNlX2lkIHRvIHBhc3MgdGhhdCBmbGFnLgo= From mboxrd@z Thu Jan 1 00:00:00 1970 From: Felipe Balbi Subject: Re: [PATCH] usb: dwc3: of-simple: reset host controller at suspend/resume Date: Mon, 16 Jul 2018 10:02:54 +0300 Message-ID: <87d0vnslwh.fsf@linux.intel.com> References: <20180709150844.32505-1-enric.balletbo@collabora.com> <536642654ca55f08877497c86a5cb106d5458384.camel@collabora.com> Mime-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Enric Balletbo Serra , ezequiel@collabora.com Cc: Enric Balletbo i Serra , linux-kernel , Brian Norris , kernel@collabora.com, Heiko =?utf-8?Q?St=C3=BCbner?= , Greg Kroah-Hartman , linux-usb@vger.kernel.org, "devicetree@vger.kernel.org" List-Id: devicetree@vger.kernel.org --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Hi, Enric Balletbo Serra writes: > Hi, > Missatge de Ezequiel Garcia del dia dt., 10 > de jul. 2018 a les 0:10: >> >> Sigh, now actually Cc devicetree folks >> >> On Mon, 2018-07-09 at 19:04 -0300, Ezequiel Garcia wrote: >> > (Cc devicetree folks) >> > >> > Hi Enric, >> > >> > Thanks for the patch. >> > >> > On Mon, 2018-07-09 at 17:08 +0200, Enric Balletbo i Serra wrote: >> > > If we power off the SoC logic rail in S3, we can find that the >> > > Type-C >> > > PHY can't initialize correctly after system resume. We need to >> > > toggle >> > > the USB3-OTG reset before trying to initialize the PHY, or else it >> > > times out. >> > > >> > > phy phy-ff800000.phy.9: phy poweron failed --> -110 >> > > dwc3 fe900000.dwc3: failed to initialize core >> > > dwc3: probe of fe900000.dwc3 failed with error -110 >> > > >> > > Note that the RK3399 TRM suggests that we should keep the whole >> > > usb3 >> > > controller in reset for the duration of the Type-C PHY >> > > initialization. >> > > However, it's hard to assert the reset in the current framework of >> > > reset. We're still skeptical about that, and we haven't yet found a >> > > case where this seems to have mattered. This approach is much >> > > easier, >> > > it >> > > simply holds the USB3-OTG reset while device is supended. >> > > >> > > The dwc3 core is going to reinitialize the controller at >> > > suspend/resume >> > > anyway (including a "soft reset"), so it should be safe to do this. >> > > >> > > Signed-off-by: Enric Balletbo i Serra > > > > >> > > --- >> > > Dear all, >> > > >> > > Now that the usb3-phy otg port support for rk3399 has been merged >> > > [1] >> > > we >> > > found that suspend/resume is broken. The problem is well known for >> > > ChromeOS kernels, they solved it in a similar way adding a reset >> > > pulse on >> > > resume in the specific usb glue layer (dwc3-rockchip). In mainline, >> > > though, we use the dwc3-of-simple glue layer instead of a specific >> > > layer >> > > for rockchip. The patch is based on the Brian Norris work but >> > > slightly >> > > different, it holds the reset while device is suspended. It was >> > > tested >> > > on a Samsung Chromebook Plus with usbc docking station attached by >> > > doing >> > > different suspend/resume cycles and checking no usb devices has >> > > been >> > > lost. >> > > >> > > I am not sure this is the better way to solve this but I did not >> > > find >> > > any other way, and, as I am not sure this can be generic, the reset >> > > is only >> > > done on rockchip platforms. >> > > >> > >> > I don't really understand why there are per-platform hacks in glue >> > drivers, instead of having per-platform glue drivers, or some other >> > pluggable hooks. >> > >> > > Best regards, >> > > Enric >> > > >> > > [1] bfdca1736ea76345071bbc5607d18928e54909ac ('arm64: dts: >> > > rockchip: >> > > add >> > > usb3-phy otg-port support for rk3399') >> > > >> > > drivers/usb/dwc3/dwc3-of-simple.c | 21 +++++++++++++++++++++ >> > > 1 file changed, 21 insertions(+) >> > > >> > > diff --git a/drivers/usb/dwc3/dwc3-of-simple.c >> > > b/drivers/usb/dwc3/dwc3-of-simple.c >> > > index dbeff5e6ad14..1d1ece99ed94 100644 >> > > --- a/drivers/usb/dwc3/dwc3-of-simple.c >> > > +++ b/drivers/usb/dwc3/dwc3-of-simple.c >> > > @@ -201,9 +201,30 @@ static int >> > > dwc3_of_simple_runtime_resume(struct >> > > device *dev) >> > > >> > > return 0; >> > > } >> > > + >> > > +static int dwc3_of_simple_suspend(struct device *dev) >> > > +{ >> > > + struct dwc3_of_simple *simple =3D dev_get_drvdata(dev); >> > > + >> > > + if (of_device_is_compatible(dev->of_node, >> > > "rockchip,rk3399- >> > > dwc3")) >> > > >> > >> > Instead of calling of_device_is_compatible in each suspend/resume, >> > which seems quite expensive, how about having a per-device boolean >> > 'needs_reset' or something like that? >> > > Yep, not sure how much, but probably use a boolean will be faster. I > am also wondering if we can remove the of_device_is_compatible call > and do the reset on all platforms. This will need lots of tests on > different platforms, of course. i'd like to hear maintainers feedback > here. ideally, the reset would be unconditional, but in practice, probably it's handled differently by different implementations. I'd be fine with a single of_device_is_compatible() call during probe to set a driver flag. Another possibility would be to use the data field of of_device_id to pass that flag. =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEElLzh7wn96CXwjh2IzL64meEamQYFAltMQx4ACgkQzL64meEa mQZNoQ/9HqRisCYieeLPQQqghb8/BHZweLozYv+K7D39p5n5+FNFv72lW3NAZWji sfhmXYOnKlM6HI30b8NC0zb7SioefRhjTxP2sIUg4mYIWnyStHH4Ot46pdsQ4SAm 7Fd+YalmYh4xaUcJkSFbhY3FyGd1DCTzSti03ndG2vrG5Og9iotb6Ono3eBUIBdu cQKRaSyWwneHtrQEyvAQtX1eqyJHsFdt5VTt6ZfhZ6+BF1x4TedUgqGwz60aiyIO w1JfOlARvsK1h4/eBQMmDmEj6QcCwhj6urNKImP557sDeJVTng4zg3iKgmUtFsX5 A4bLG6ZE0Lb73q6MRBZVKi5Lsc6mqIKxe0Q5rron/WljLjB/sqA3hC54fg4xcyY3 YypEDs5/FdY4MdIrprFDSAAaijjBv7LFNk39l5MgvF8Nn/pvbFwsmfpZt6MlwQG9 MVGLMiZRwSB+Z8+IP4WL6Vg3h+VnDqL0MkuGgZPyBqFpjwOLUfHpuEY4pDemeqso fHxSLISpW6zwYbnnXY5Chf7H/v/1E/AhXNZUUYHNVWegolzvCeIPi5ZWGKPFgB1x juCc9NkKhAGxmgixLbbpkCK/wxr+lDHAHXa8msPX00t7j1YfBbcFuUv7zrDbZBP/ JutuYphkY0qrsUuDBE5KWi7UUWYSj6OyKBVEjkdsYfrYWjre9ls= =EHHp -----END PGP SIGNATURE----- --=-=-=--