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

[gustavo panizzo]

Hi

On Mon, Jul 24, 2017 at 12:53:27PM +0300, Felipe Balbi wrote:
> Hi,
>
> gustavo panizzo <gfa@xxxxxxxxxxxx> writes:
> > > > On Wed, Jul 12, 2017 at 02:08:04PM +0800, Baolin Wang wrote:
> > > > > Hi,
> > > > >
> > > > > On 12 July 2017 at 11:52, gustavo panizzo <gfa@xxxxxxxxxxxx> 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 <brian.kim@xxxxxxxxxxxxxx>
> > > > > > Signed-off-by: gustavo panizzo <gfa@xxxxxxxxxxxx>
> > > > > > ---
> > > > > >  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 <gfa@xxxxxxxxxxxx>
> > 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