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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20160218210652.68ae464eed8ddbffd33e7a02@gmail.com>
Date:	Thu, 18 Feb 2016 21:06:52 +0300
From:	Antony Pavlov <antonynpavlov@...il.com>
To:	Alan Stern <stern@...land.harvard.edu>, Marek Vasut <marex@...x.de>
Cc:	linux-mips@...ux-mips.org, Wills Wang <wills.wang@...e.com>,
	Daniel Schwierzeck <daniel.schwierzeck@...il.com>,
	Alban Bedel <albeu@...e.fr>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	<linux-usb@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [RFC v5 07/15] usb: ehci: add vbus-gpio parameter

On Thu, 18 Feb 2016 11:12:43 -0500 (EST)
Alan Stern <stern@...land.harvard.edu> wrote:

> On Tue, 9 Feb 2016, 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@...il.com>
> > Cc: Alan Stern <stern@...land.harvard.edu>
> > Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> > Cc: linux-usb@...r.kernel.org
> > Cc: linux-kernel@...r.kernel.org
> > ---
> >  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@...00100 {
                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>;
                regulator-max-microvolt = <5000000>;
                gpio = <&gpio 8 GPIO_ACTIVE_HIGH>;
                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!

> > +}
> > +
> >  static int ehci_platform_probe(struct platform_device *dev)
> >  {
> >  	struct usb_hcd *hcd;
> > @@ -174,6 +194,8 @@ static int ehci_platform_probe(struct platform_device *dev)
> >  		return irq;
> >  	}
> >  
> > +	setup_vbus_gpio(&dev->dev);
> > +
> >  	hcd = usb_create_hcd(&ehci_platform_hc_driver, &dev->dev,
> >  			     dev_name(&dev->dev));
> >  	if (!hcd)
> > 
> 


-- 
-- 
Best regards,
  Antony Pavlov