Re: [PATCH] usb: dwc3: Fix the USB 3.0 hub detection bug after warm boot

From: Felipe Balbi
Date: Mon Sep 18 2017 - 04:11:54 EST



Hi,

gustavo panizzo <gfa@xxxxxxxxxxxx> writes:
>>>gustavo panizzo <gfa@xxxxxxxxxxxx> 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 = 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: Brian Kim <brian.kim@xxxxxxxxxxxxxx>
>>>>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 <brian.kim@xxxxxxxxxxxxxx>
>>>> Signed-off-by: gustavo panizzo <gfa@xxxxxxxxxxxx>
>>>>
>>>>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 = 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);
>>>>+ 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 = platform_get_drvdata(pdev);
>>>>@@ -1488,6 +1514,7 @@ MODULE_DEVICE_TABLE(acpi, dwc3_acpi_match);
>>>>static struct platform_driver dwc3_driver = {
>>>> .probe = dwc3_probe,
>>>> .remove = dwc3_remove,
>>>>+ .shutdown = dwc3_shutdown,
>>>> .driver = {
>>>> .name = "dwc3",
>>>> .of_match_table = 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
>>
>>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

--
balbi

Attachment: signature.asc
Description: PGP signature