Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753096AbdG2XzV (ORCPT ); Sat, 29 Jul 2017 19:55:21 -0400 Received: from tk.zumbi.com.ar ([106.185.28.14]:60950 "EHLO tk.zumbi.com.ar" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751884AbdG2XzT (ORCPT ); Sat, 29 Jul 2017 19:55:19 -0400 Date: Sun, 30 Jul 2017 07:53:03 +0800 From: gustavo panizzo To: Felipe Balbi Cc: 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 Message-ID: <20170729235303.p4b7ga7ubplreof4@zumbi.com.ar> References: <20170712035238.27554-1-gfa@zumbi.com.ar> <20170712232026.77hznul754yiovfm@zumbi.com.ar> <20170721140544.j4obgspw7mcyikw5@zumbi.com.ar> <87h8y240vc.fsf@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: <87h8y240vc.fsf@linux.intel.com> X-PGP-Key: https://blog.zumbi.com.ar/posts/my-public-gpg-key/44BB1BA79F6C6333.asc User-Agent: NeoMutt/20170306 (1.8.0) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4255 Lines: 129 Hi On Mon, Jul 24, 2017 at 12:53:27PM +0300, Felipe Balbi wrote: > >Hi, > >gustavo panizzo writes: >>>> On Wed, Jul 12, 2017 at 02:08:04PM +0800, Baolin Wang wrote: >>>>> >>>>> Hi, >>>>> >>>>> On 12 July 2017 at 11:52, gustavo panizzo wrote: >>>>>> >>>>>> 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 >>>>>> --- >>>>>> 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 = platform_get_drvdata(pdev); >>>>>> + struct resource *res = 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 request >>>>>> the >>>>>> + * memory region the next time probe is called. >>>>>> + */ >>>>>> + res->start -= 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: gustavo panizzo >> 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. > >if the original patch is from Brian, then 'Author' should be >Brian. Please fix it. sure, i just didn't want to attribute something he didn't write > >I also don't get why all of a sudden we need to implement >->shutdown(). Why is it that we never needed it before and we need it now? Commit c499ff71ff2a281366c6ec7a904c547d806cbcd1 broke USB3 for the Odroid XU4, this 2 patches fix the USB3 https://github.com/hardkernel/linux/commit/74b9605e5587b30912d6b6093e9d7fb06d043c33 https://github.com/hardkernel/linux/commit/2166ffd004e04a61887eb2a39f8639dc12140c58 Hardkernel didn't upstreamed them, after investigation in Debian #843448 the 2 patches were identified. The first patch is being upstreamed by Jochen Sprickerhof http://marc.info/?l=linux-usb&m=149945465112440&w=2 I'm taking care of the second patch > >Care to bisect to find the first commit which started causing this >problem? c499ff71ff2a281366c6ec7a904c547d806cbcd1 > >-- >balbi -- IRC: gfa GPG: 0X44BB1BA79F6C6333