Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752786AbdIRILr (ORCPT ); Mon, 18 Sep 2017 04:11:47 -0400 Received: from mga02.intel.com ([134.134.136.20]:40933 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752156AbdIRILo (ORCPT ); Mon, 18 Sep 2017 04:11:44 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.42,411,1500966000"; d="asc'?scan'208";a="150325207" 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: <20170916042404.i2snvrpjigipdwhj@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> <871snipyrw.fsf@linux.intel.com> <20170915090059.otw4sbwefm6nsxxg@gpanizzo.laptop.office.chorus> <20170916042404.i2snvrpjigipdwhj@gpanizzo.laptop.office.chorus> Date: Mon, 18 Sep 2017 11:11:16 +0300 Message-ID: <874ls0o28r.fsf@linux.intel.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6785 Lines: 188 --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Hi, gustavo panizzo writes: >>>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_dev= ice >>>>>>>>>>>*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= , in case >>>>>>>>>>>the >>>>>>>>>>>+ * probe is deferred, we don't end up getting error in r= equest >>>>>>>>>>>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 N= IC 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 U= SB >>>> 3.0 hub is not able to detect after warm boot. >>>> Original patch by Brian Kim, updated and submitted upstream by gusta= vo >>>> 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 *pd= ev) >>>> 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, in ca= se 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 c6be5a0e3cebc145127d46a58350e05d2bcf632= 3 from linux-next >>>>Can you _please_ merge it? >>> >>>why are you upset? You didn't do the changes I requested until now. It's >> >>I'm not upset >> >>>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. >> >>the commit i sent was tested, and is still running, on top linux-next >>964bcc1b4f57028d56dace7d9bc5924f2eb43f36, which gives an uname -r >>4.13.0-rc1-next-20170717+ >> >> >>> >>>Test this against a vanilla tree (v4.13 was tagged days ago) and give me >>>logs showing the problem without your commit. > > the patch does not work on top of 4.13.0 :( > > could you draft a patch i can test? why me? I don't have this platform. I have no idea what you need. You (or the patch author) should be the one fixing comments to the patch. Good luck =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEElLzh7wn96CXwjh2IzL64meEamQYFAlm/f6QACgkQzL64meEa mQZc9w//UwQTUwpwG5lg6auvsiAGNMz03lzDFWh9PRO/p5v96Belxxo7o+0YaZNh PW5Z4WoFhvMk1Jzd1oLz3cunRCkenPNa90xYZq3GHadrWMkSboo5tKi7PlanSKUG KNWtREgZbZP6fNSxHUdnMuU+mM9587RWEgHW9nYTX4Ctkpm0wtEx7TpsG5vmKssP 8brilOEmnJ8GJjR4f0XHLuqCKVZDlBg/0We5DXui5Z6ngaZeV4cy4boCT5Ke648u 9CdUiJPWIQm4BWLWMVnziyh2ePXNl9Hl/ZnYxu0jjhZ9GYFWoMQfHKHgluAIRoOp RRjvueWTixuVBQUo/R+kllzS53Db20FMV11A5oYbUSlvwca7TZIoH2DXbNKVK4vg 07/YnSJBvG87FyOyFRaMOf9oDICeVUeI7O7StclxszMWnetu5lu/V8FRYASJad+s zY1nEnaOu6uBDZEf5zpHNcZl647xOXEDirpASg7tF/YYPpWbLrMSRAzFsTCicVsj ZZ2+Z9weK6OZw06AKAZ68kmec7Rwx3Pk1+ysQ58Uz79HYhGhqmHuQ4veRxiBdNXv b1IfDmMRTPVmqmcfHMxFxpCiYNM47bLn2eGP/DPeP556G4EprjGVzLwBsniQImi7 nfsQa+gwWrZXZJqGZ0uCHK5emNOTwEDLkJpDEC985tGe+ll1mWY= =/c+6 -----END PGP SIGNATURE----- --=-=-=--