Re: [RFC v5 07/15] usb: ehci: add vbus-gpio parameter

[Sergei Shtylyov]

On 02/18/2016 09:06 PM, Antony Pavlov wrote:

> > > This patch retrieves and configures the vbus control gpio via
> > > the device tree.
> > >
> > > This patch is based on a ehci-s5p.c commit fd81d59c90d38661
> > > ("USB: ehci-s5p: Add vbus setup function to the s5p ehci glue layer").
> > >
> > > Signed-off-by: Antony Pavlov <antonynpavlov@xxxxxxxxx>
> > > Cc: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
> > > Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> > > Cc: linux-usb@xxxxxxxxxxxxxxx
> > > Cc: linux-kernel@xxxxxxxxxxxxxxx
> > > ---
> > >   drivers/usb/host/ehci-platform.c | 22 ++++++++++++++++++++++
> > >   1 file changed, 22 insertions(+)
> > >
> > > diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c
> > > index bd7082f2..0d95ced 100644
> > > --- a/drivers/usb/host/ehci-platform.c
> > > +++ b/drivers/usb/host/ehci-platform.c
> > > @@ -28,6 +28,7 @@
> > >   #include <linux/io.h>
> > >   #include <linux/module.h>
> > >   #include <linux/of.h>
> > > +#include <linux/of_gpio.h>
> > >   #include <linux/phy/phy.h>
> > >   #include <linux/platform_device.h>
> > >   #include <linux/reset.h>
> > > @@ -142,6 +143,25 @@ static struct usb_ehci_pdata ehci_platform_defaults = {
> > >   	.power_off =		ehci_platform_power_off,
> > >   };
> > >
> > > +static void setup_vbus_gpio(struct device *dev)
> > > +{
> > > +	int err;
> > > +	int gpio;
> > > +
> > > +	if (!dev->of_node)
> > > +		return;
> > > +
> > > +	gpio = of_get_named_gpio(dev->of_node, "vbus-gpio", 0);
> > > +	if (!gpio_is_valid(gpio))
> > > +		return;
> > > +
> > > +	err = devm_gpio_request_one(dev, gpio,
> > > +				GPIOF_OUT_INIT_HIGH | GPIOF_EXPORT_DIR_FIXED,
> > > +				"ehci_vbus_gpio");
> > > +	if (err)
> > > +		dev_err(dev, "can't request ehci vbus gpio %d", gpio);
> >
> >
> > I don't understand this.  If you get an error here, what's the point of
> > allowing the probe to continue?  Shouldn't you return an error code so
> > the probe will fail?
>
> Please ignore the 'usb: ehci: add vbus-gpio parameter' patch!
>
> In the new AR9331 patchseries I use chipidea USB driver (thanks to Marek for the suggestion)
> in the AR9331 dtsi-file:
>
>          usb: usb@1b000100 {
>                  compatible = "chipidea,usb2";
>                  reg = <0x1b000000 0x200>;
>
>                  interrupt-parent = <&cpuintc>;
>                  interrupts = <3>;
>                  resets = <&rst 5>;
>
>                  phy-names = "usb-phy";
>                  phys = <&usb_phy>;
>
>                  status = "disabled";
>          };
>
>
> so I use regulator in the TL-MR3020 board dts file:
>
>          reg_usb_vbus: reg_usb_vbus {
>                  compatible = "regulator-fixed";
>                  regulator-name = "usb_vbus";
>                  regulator-min-microvolt = <5000000>;

   Not 0?

>                  regulator-max-microvolt = <5000000>;
>                  gpio = <&gpio 8 GPIO_ACTIVE_HIGH>;

   Where's the switch if both voltages are equal?

>                  enable-active-high;
>          };
>
> &usb {
>          dr_mode = "host";
>          vbus-supply = <&reg_usb_vbus>;
>          status = "okay";
> };
>
> As a result there is no need in adding vbus-gpio parameter to ehci anymore!
[...]

MBR, Sergei