Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754898AbdIGKwK (ORCPT ); Thu, 7 Sep 2017 06:52:10 -0400 Received: from mga06.intel.com ([134.134.136.31]:21867 "EHLO mga06.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754780AbdIGKwI (ORCPT ); Thu, 7 Sep 2017 06:52:08 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.42,357,1500966000"; d="asc'?scan'208";a="1011945854" From: Felipe Balbi To: gustavo panizzo , Baolin Wang , Greg KH , USB , LKML , stable@vger.kernel.org, Brian Kim Subject: Re: [PATCH] usb: dwc3: Fix the USB 3.0 hub detection bug after warm boot In-Reply-To: <20170907065123.pkan4cqvynpqpsp2@gpanizzo.laptop.office.chorus> References: <20170712035238.27554-1-gfa@zumbi.com.ar> <20170712232026.77hznul754yiovfm@zumbi.com.ar> <20170721140544.j4obgspw7mcyikw5@zumbi.com.ar> <87h8y240vc.fsf@linux.intel.com> <20170729235303.p4b7ga7ubplreof4@zumbi.com.ar> <20170907065123.pkan4cqvynpqpsp2@gpanizzo.laptop.office.chorus> Date: Thu, 07 Sep 2017 13:51:31 +0300 Message-ID: <871snipyrw.fsf@linux.intel.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6326 Lines: 172 --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Hi, gustavo panizzo writes: >>>>>>>>--- >>>>>>>> drivers/usb/dwc3/core.c | 33 +++++++++++++++++++++++++++++++++ >>>>>>>> 1 file changed, 33 insertions(+) >>>>>>>> >>>>>>>>diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >>>>>>>>index 326b302fc440..f92dfe213d89 100644 >>>>>>>>--- a/drivers/usb/dwc3/core.c >>>>>>>>+++ b/drivers/usb/dwc3/core.c >>>>>>>>@@ -1259,6 +1259,38 @@ static int dwc3_probe(struct platform_device >>>>>>>>*pdev) >>>>>>>> return ret; >>>>>>>> } >>>>>>>> >>>>>>>>+static void dwc3_shutdown(struct platform_device *pdev) >>>>>>>>+{ >>>>>>>>+ struct dwc3 *dwc =3D platform_get_drvdata(pdev); >>>>>>>>+ struct resource *res =3D platform_get_resource(pdev, >>>>>>>>IORESOURCE_MEM, 0); >>>>>>>>+ >>>>>>>>+ pm_runtime_get_sync(&pdev->dev); >>>>>>>>+ /* >>>>>>>>+ * restore res->start back to its original value so that, i= n case >>>>>>>>the >>>>>>>>+ * probe is deferred, we don't end up getting error in requ= est >>>>>>>>the >>>>>>>>+ * memory region the next time probe is called. >>>>>>>>+ */ >>>>>>>>+ res->start -=3D DWC3_GLOBALS_REGS_START; >>>>>>>>+ >>>>>>>>+ dwc3_debugfs_exit(dwc); >>>>>>>>+ dwc3_core_exit_mode(dwc); >>>>>>>>+ dwc3_event_buffers_cleanup(dwc); >>>>>> >>>>>> >>>>>>What about dwc3_event_buffers_cleanup? should I remove it from >>>>>>dwc3_shutdown()? >>>>>>It is already in dwc3_core_exit() >>>>> >>>>>I think so. We should avoid duplicate code. >>>>> >>>>>>>>+ dwc3_free_event_buffers(dwc); >>>>>>>>+ >>>>>>>>+ usb_phy_set_suspend(dwc->usb2_phy, 1); >>>>>>>>+ usb_phy_set_suspend(dwc->usb3_phy, 1); >>>>>>>>+ >>>>>>>>+ phy_power_off(dwc->usb2_generic_phy); >>>>>>>>+ phy_power_off(dwc->usb3_generic_phy); >>>>>>> >>>>>>> >>>>>>>We've done these in dwc3_core_exit(). >>>> >>>>This is the patch after testing on a Odroid XU4, on top of linux-next >>>>964bcc1b4f57028d56dace7d9bc5924f2eb43f36 which translates to 4.13.0-rc1= -next-20170717+ >>>>I tested this patch for a week without problems with heavy USB and NIC = usage. >>>>Please consider merging it > > Author: Brian Kim > Date: Wed Jul 12 11:26:55 2017 +0800 > > usb: dwc3: Fix the USB 3.0 hub detection bug after warm boot > The dwc3 could not release resources when the module is built-in > because this module does not have shutdown method. This causes the USB > 3.0 hub is not able to detect after warm boot. > Original patch by Brian Kim, updated and submitted upstream by gustavo > panizzo. > Also see https://bugs.debian.org/843448 > Signed-off-by: Brian Kim > Signed-off-by: gustavo panizzo > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > index 326b302fc440..09de37d47ee7 100644 > --- a/drivers/usb/dwc3/core.c > +++ b/drivers/usb/dwc3/core.c > @@ -1259,6 +1259,32 @@ static int dwc3_probe(struct platform_device *pdev) > return ret; > } > > +static void dwc3_shutdown(struct platform_device *pdev) > +{ > + struct dwc3 *dwc =3D platform_get_drvdata(pdev); > + struct resource *res =3D platform_get_resource(pdev, IORESOURCE_M= EM, 0); > + > + pm_runtime_get_sync(&pdev->dev); > + /* > + * restore res->start back to its original value so that, in case= the > + * probe is deferred, we don't end up getting error in request the > + * memory region the next time probe is called. > + */ > + res->start -=3D DWC3_GLOBALS_REGS_START; > + > + dwc3_debugfs_exit(dwc); > + dwc3_core_exit_mode(dwc); > + dwc3_event_buffers_cleanup(dwc); > + dwc3_free_event_buffers(dwc); > + > + dwc3_core_exit(dwc); > + dwc3_ulpi_exit(dwc); > + > + pm_runtime_put_sync(&pdev->dev); > + pm_runtime_allow(&pdev->dev); > + pm_runtime_disable(&pdev->dev); > +} > + > static int dwc3_remove(struct platform_device *pdev) > { > struct dwc3 *dwc =3D platform_get_drvdata(pdev); > @@ -1488,6 +1514,7 @@ MODULE_DEVICE_TABLE(acpi, dwc3_acpi_match); > static struct platform_driver dwc3_driver =3D { > .probe =3D dwc3_probe, > .remove =3D dwc3_remove, > + .shutdown =3D dwc3_shutdown, > .driver =3D { > .name =3D "dwc3", > .of_match_table =3D of_match_ptr(of_dwc3_match), > > Patch applies cleanly on top of c6be5a0e3cebc145127d46a58350e05d2bcf6323 = from linux-next > Can you _please_ merge it? why are you upset? You didn't do the changes I requested until now. It's too late for v4.14 merge window and you didn't even send this as a proper patch. I also have no evidence that you've been testing mainline kernel, the commits you pointed me to are against a v4.9 vendor kernel. Test this against a vanilla tree (v4.13 was tagged days ago) and give me logs showing the problem without your commit. Also, the commit you pointed me to couldn't be the culprit: we didn't have a ->shutdown() before and that commit didn't change that. All that commit did was extract some code to helper functions which would be reused. On top of all that, Brian's commit merely duplicates the ->remove() method as ->shutdown(), we don't want duplicated code, sorry. =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEElLzh7wn96CXwjh2IzL64meEamQYFAlmxJLMACgkQzL64meEa mQaF4g/8D1emVUrDURGwSCgQwqEWG10+CQHQFViKRtH+UQUv/PSv6bc8eBcX7D98 zLH55bDGwOn+xONhL3OdFbaE1sVsBq0h+iDusOEQBHaQfD20V/N6U96850g3a06n GDYMNq6vzn34tX9oP5YqDZ0d1QXttNn4U0UySTfmOwjxPGJkaEW7vusbaM6azi/1 GPrPeBFU8c3CfDH/G5p2xfP4nPbn4Mp5olYhPvWa6EASbhRh3YYDYb62uTKdxQJP 1Vbtba7BRw222/jnKcdO9VoelVHj69XhKNzq7q0EUsJNfolDI/9+P+RnopuOHO6t KqKZrrn+0r8kjJ1HVwOEj+ECiukZqukLhXpYZM6yalDYV/n+G7KAqqj/jTaO78AC RHDTbuGc9us688fvzWKiix8F96oTEDWABjFQI61fMf6GDIv4iefLldk2dMo7pEhh zapFrWUA2iC/5V9SyUxwaDNSTq5Sq59KP3p1viRaiWjnSeQ7x76FaayiJdOEOLL4 ILHRBBgBYYqltIt4cLjTv6MksWNzgRrhwxBiaBpqsX1u3nngz/C9z0pPkzwYA5cB m1dg82S4WknF7zYfz1HQdGpJQg88dv9QiNRPlnJcJWOCboma0+YowUi3+9FoQDB1 H3KC3biTTmWW8gcLDn4ZPvEwNDtK3KYH6ch0zUGMAM59EFQgF3o= =SgXq -----END PGP SIGNATURE----- --=-=-=--