Revert "usb: dwc3: pci: Use devm functions to get the phy GPIOs"

[Stephan Gerhold <stephan@gerhold.net>]

On Sun, Dec 09, 2018 at 04:44:16PM +0200, Andy Shevchenko wrote:
> On Thu, Dec 6, 2018 at 8:49 PM Stephan Gerhold <stephan@gerhold.net> wrote:
> >
> > Commit 211f658b7b40 ("usb: dwc3: pci: Use devm functions to get
> > the phy GPIOs") changed the code to claim the PHY GPIOs permanently
> > for Intel Baytrail devices.
> >
> > This causes issues when the actual PHY driver attempts to claim the
> > same GPIO descriptors. For example, tusb1210 now fails to probe with:
> >
> >   tusb1210: probe of dwc3.0.auto.ulpi failed with error -16 (EBUSY)
> >
> > dwc3-pci needs to turn on the PHY once before dwc3 is loaded, but
> > usually the PHY driver will then hold the GPIOs to turn off the
> > PHY when requested (e.g. during suspend).
> >
> > To fix the problem, this reverts the commit to restore the old
> > behavior to put the GPIOs immediately after usage.
> >
> > Link: https://www.spinics.net/lists/linux-usb/msg174681.html
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> > ---
> >  drivers/usb/dwc3/dwc3-pci.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
> > index 842795856bf4..fdc6e4e403e8 100644
> > --- a/drivers/usb/dwc3/dwc3-pci.c
> > +++ b/drivers/usb/dwc3/dwc3-pci.c
> > @@ -170,20 +170,20 @@ static int dwc3_pci_quirks(struct dwc3_pci *dwc)
> >                          * put the gpio descriptors again here because the phy driver
> >                          * might want to grab them, too.
> >                          */
> > -                       gpio = devm_gpiod_get_optional(&pdev->dev, "cs",
> > -                                                      GPIOD_OUT_LOW);
> > +                       gpio = gpiod_get_optional(&pdev->dev, "cs", GPIOD_OUT_LOW);
> >                         if (IS_ERR(gpio))
> >                                 return PTR_ERR(gpio);
> >
> >                         gpiod_set_value_cansleep(gpio, 1);
> > +                       gpiod_put(gpio);
> >
> > -                       gpio = devm_gpiod_get_optional(&pdev->dev, "reset",
> > -                                                      GPIOD_OUT_LOW);
> > +                       gpio = gpiod_get_optional(&pdev->dev, "reset", GPIOD_OUT_LOW);
> >                         if (IS_ERR(gpio))
> >                                 return PTR_ERR(gpio);
> >
> >                         if (gpio) {
> >                                 gpiod_set_value_cansleep(gpio, 1);
> 
> > +                               gpiod_put(gpio);
> >                                 usleep_range(10000, 11000);
> 
> If something happens to GPIO line in between of these lines, the sleep
> might become obsolete. Shouldn't gpiod_put() be placed after?

That's a good point, but I believe this would be more appropriately
fixed in a separate patch, since this is just an exact revert of
211f658b7b40 ("usb: dwc3: pci: Use devm functions to get the phy GPIOs")
(This is the way it was written when it was added to mainline 4 years 
ago...)

I can send a separate patch for this, or would you like to?

> 
> >                         }
> >                 }
> 
> 
> -- 
> With Best Regards,
> Andy Shevchenko