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

[Marek Vasut]

On 02/18/2016 05:12 PM, Alan Stern 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@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?

The idea is I believe that if there is no vbus gpio specified, the port
might just not have vbus control, so the probe can continue. But this
patch is irrelevant anyway, since Alexey will switch to CI HDRC driver
and use standard regulator, as it should be done.