[PATCH] phy: exynos5-usbdrd: Add facility to toggle vbus gpio on/off

[PATCH] phy: exynos5-usbdrd: Add facility to toggle vbus gpio on/off

[Vivek Gautam]

Adding support to enable/disable VBUS hooked to a gpio
to enable vbus supply on the port.

Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
---

Based on 'phy-exynos5-usbdrd' patches:
[PATCH V4 0/5] Add Exynos5 USB 3.0 phy driver based on generic PHY framework
http://www.spinics.net/lists/linux-usb/msg105507.html

 drivers/phy/phy-exynos5-usbdrd.c |   18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/phy/phy-exynos5-usbdrd.c b/drivers/phy/phy-exynos5-usbdrd.c
index ff54a7c..5ca7485 100644
--- a/drivers/phy/phy-exynos5-usbdrd.c
+++ b/drivers/phy/phy-exynos5-usbdrd.c
@@ -18,6 +18,7 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
+#include <linux/of_gpio.h>
 #include <linux/phy/phy.h>
 #include <linux/platform_device.h>
 #include <linux/mutex.h>
@@ -176,6 +177,7 @@ struct exynos5_usbdrd_phy {
 	struct clk *ref_clk;
 	unsigned long ref_rate;
 	unsigned int refclk_reg;
+	int gpio;
 };
 
 #define to_usbdrd_phy(inst) \
@@ -460,6 +462,9 @@ static int exynos5_usbdrd_phy_power_on(struct phy *phy)
 	if (!IS_ERR(phy_drd->usb30_sclk))
 		clk_prepare_enable(phy_drd->usb30_sclk);
 
+	/* Toggle VBUS gpio - on */
+	gpio_set_value(phy_drd->gpio, 1);
+
 	/* Power-on PHY*/
 	inst->phy_cfg->phy_isol(inst, 0);
 
@@ -476,6 +481,9 @@ static int exynos5_usbdrd_phy_power_off(struct phy *phy)
 	/* Power-off the PHY */
 	inst->phy_cfg->phy_isol(inst, 1);
 
+	/* Toggle VBUS gpio - off */
+	gpio_set_value(phy_drd->gpio, 0);
+
 	if (!IS_ERR(phy_drd->usb30_sclk))
 		clk_disable_unprepare(phy_drd->usb30_sclk);
 
@@ -585,6 +593,16 @@ static int exynos5_usbdrd_phy_probe(struct platform_device *pdev)
 
 	phy_drd->drv_data = drv_data;
 
+	/* Get required GPIO for vbus */
+	phy_drd->gpio = of_get_named_gpio(dev->of_node,
+					  "samsung,vbus-gpio", 0);
+	if (!gpio_is_valid(phy_drd->gpio))
+		dev_dbg(dev, "no usbdrd-phy vbus gpio defined\n");
+
+	if (devm_gpio_request(dev, phy_drd->gpio, "phydrd_vbus_gpio"))
+		dev_dbg(dev, "can't request usbdrd-phy vbus gpio %d\n",
+			phy_drd->gpio);
+
 	if (of_property_read_u32(node, "samsung,pmu-offset", &pmu_offset)) {
 		dev_err(dev, "Missing pmu-offset for phy isolation\n");
 		return -EINVAL;
-- 
1.7.10.4

Re: [PATCH] phy: exynos5-usbdrd: Add facility to toggle vbus gpio on/off

[Sylwester Nawrocki]

Hi Vivek,

On 09/04/14 13:54, Vivek Gautam wrote:
> Adding support to enable/disable VBUS hooked to a gpio
> to enable vbus supply on the port.

Does the GPIO control a fixed voltage regulator ? If so, shouldn't
it be modelled by the regulator API instead ?

> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
[...]
> +	/* Get required GPIO for vbus */
> +	phy_drd->gpio = of_get_named_gpio(dev->of_node,
> +					  "samsung,vbus-gpio", 0);
> +	if (!gpio_is_valid(phy_drd->gpio))
> +		dev_dbg(dev, "no usbdrd-phy vbus gpio defined\n");
> +
> +	if (devm_gpio_request(dev, phy_drd->gpio, "phydrd_vbus_gpio"))
> +		dev_dbg(dev, "can't request usbdrd-phy vbus gpio %d\n",
> +			phy_drd->gpio);

--
Regards,
Sylwester

Re: [PATCH] phy: exynos5-usbdrd: Add facility to toggle vbus gpio on/off

[Vivek Gautam]

Hi Sylwester,


On Wed, Apr 9, 2014 at 5:41 PM, Sylwester Nawrocki
<s.nawrocki@samsung.com> wrote:
> Hi Vivek,
>
> On 09/04/14 13:54, Vivek Gautam wrote:
>> Adding support to enable/disable VBUS hooked to a gpio
>> to enable vbus supply on the port.
>
> Does the GPIO control a fixed voltage regulator ? If so, shouldn't
> it be modelled by the regulator API instead ?

No, this GPIO controls a 'current limiting power distribution switch',
which gives the output vbus to usb controller.
Should i model this as a fixed regulator ?

>
>> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
> [...]
>> +     /* Get required GPIO for vbus */
>> +     phy_drd->gpio = of_get_named_gpio(dev->of_node,
>> +                                       "samsung,vbus-gpio", 0);
>> +     if (!gpio_is_valid(phy_drd->gpio))
>> +             dev_dbg(dev, "no usbdrd-phy vbus gpio defined\n");
>> +
>> +     if (devm_gpio_request(dev, phy_drd->gpio, "phydrd_vbus_gpio"))
>> +             dev_dbg(dev, "can't request usbdrd-phy vbus gpio %d\n",
>> +                     phy_drd->gpio);
>
> --
> Regards,
> Sylwester
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] phy: exynos5-usbdrd: Add facility to toggle vbus gpio on/off

[Tomasz Figa]

Hi,

On 09.04.2014 14:24, Vivek Gautam wrote:
> Hi Sylwester,
>
>
> On Wed, Apr 9, 2014 at 5:41 PM, Sylwester Nawrocki
> <s.nawrocki@samsung.com> wrote:
>> Hi Vivek,
>>
>> On 09/04/14 13:54, Vivek Gautam wrote:
>>> Adding support to enable/disable VBUS hooked to a gpio
>>> to enable vbus supply on the port.
>>
>> Does the GPIO control a fixed voltage regulator ? If so, shouldn't
>> it be modelled by the regulator API instead ?
>
> No, this GPIO controls a 'current limiting power distribution switch',
> which gives the output vbus to usb controller.
> Should i model this as a fixed regulator ?

If I understand this correctly, this is just a switch that lets you 
control whether vbus is provided to the USB connector or not. If so, 
this doesn't look like an Exynos-specific thing at all and should rather 
be modeled on higher level.

Best regards,
Tomasz

Re: [PATCH] phy: exynos5-usbdrd: Add facility to toggle vbus gpio on/off

[Vivek Gautam]

Hi,


On Wed, Apr 9, 2014 at 6:08 PM, Tomasz Figa <t.figa@samsung.com> wrote:
> Hi,
>
>
> On 09.04.2014 14:24, Vivek Gautam wrote:
>>
>> Hi Sylwester,
>>
>>
>> On Wed, Apr 9, 2014 at 5:41 PM, Sylwester Nawrocki
>> <s.nawrocki@samsung.com> wrote:
>>>
>>> Hi Vivek,
>>>
>>> On 09/04/14 13:54, Vivek Gautam wrote:
>>>>
>>>> Adding support to enable/disable VBUS hooked to a gpio
>>>> to enable vbus supply on the port.
>>>
>>>
>>> Does the GPIO control a fixed voltage regulator ? If so, shouldn't
>>> it be modelled by the regulator API instead ?
>>
>>
>> No, this GPIO controls a 'current limiting power distribution switch',
>> which gives the output vbus to usb controller.
>> Should i model this as a fixed regulator ?
>
>
> If I understand this correctly, this is just a switch that lets you control
> whether vbus is provided to the USB connector or not. If so, this doesn't
> look like an Exynos-specific thing at all and should rather be modeled on
> higher level.

You are right. The Vbus to the USB connector is given through this a
switch which current limiting capabilities.
This switch just provides the 5V at its input to its output based on
the state of Enable pin
(which is the GPIO we are talking in this patch).

So can you please suggest on how this should be handled ?

>
> Best regards,
> Tomasz
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
>
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] phy: exynos5-usbdrd: Add facility to toggle vbus gpio on/off

[Sylwester Nawrocki]

On 09/04/14 14:24, Vivek Gautam wrote:
> On Wed, Apr 9, 2014 at 5:41 PM, Sylwester Nawrocki
> <s.nawrocki@samsung.com> wrote:
>> > On 09/04/14 13:54, Vivek Gautam wrote:
>>> >> Adding support to enable/disable VBUS hooked to a gpio
>>> >> to enable vbus supply on the port.
>> >
>> > Does the GPIO control a fixed voltage regulator ? If so, shouldn't
>> > it be modelled by the regulator API instead ?
>
> No, this GPIO controls a 'current limiting power distribution switch',
> which gives the output vbus to usb controller.
> Should i model this as a fixed regulator ?

OK, it's just a power switch then. I suspect using the regulator API
would be more universal, as such a GPIO is somewhat a board design
detail. I'm not going to object to your patch, just might be better
to use the gpiod API instead.

-- 
Regards,
Sylwester

Re: [PATCH] phy: exynos5-usbdrd: Add facility to toggle vbus gpio on/off

[Kishon Vijay Abraham I]

Hi.

On Wednesday 09 April 2014 05:24 PM, Vivek Gautam wrote:
> Adding support to enable/disable VBUS hooked to a gpio
> to enable vbus supply on the port.
> 
> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
> ---
> 
> Based on 'phy-exynos5-usbdrd' patches:
> [PATCH V4 0/5] Add Exynos5 USB 3.0 phy driver based on generic PHY framework
> http://www.spinics.net/lists/linux-usb/msg105507.html
> 
>  drivers/phy/phy-exynos5-usbdrd.c |   18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/phy/phy-exynos5-usbdrd.c b/drivers/phy/phy-exynos5-usbdrd.c
> index ff54a7c..5ca7485 100644
> --- a/drivers/phy/phy-exynos5-usbdrd.c
> +++ b/drivers/phy/phy-exynos5-usbdrd.c
> @@ -18,6 +18,7 @@
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_address.h>
> +#include <linux/of_gpio.h>
>  #include <linux/phy/phy.h>
>  #include <linux/platform_device.h>
>  #include <linux/mutex.h>
> @@ -176,6 +177,7 @@ struct exynos5_usbdrd_phy {
>  	struct clk *ref_clk;
>  	unsigned long ref_rate;
>  	unsigned int refclk_reg;
> +	int gpio;
>  };
>  
>  #define to_usbdrd_phy(inst) \
> @@ -460,6 +462,9 @@ static int exynos5_usbdrd_phy_power_on(struct phy *phy)
>  	if (!IS_ERR(phy_drd->usb30_sclk))
>  		clk_prepare_enable(phy_drd->usb30_sclk);
>  
> +	/* Toggle VBUS gpio - on */
> +	gpio_set_value(phy_drd->gpio, 1);
> +
>  	/* Power-on PHY*/
>  	inst->phy_cfg->phy_isol(inst, 0);
>  
> @@ -476,6 +481,9 @@ static int exynos5_usbdrd_phy_power_off(struct phy *phy)
>  	/* Power-off the PHY */
>  	inst->phy_cfg->phy_isol(inst, 1);
>  
> +	/* Toggle VBUS gpio - off */
> +	gpio_set_value(phy_drd->gpio, 0);
> +
>  	if (!IS_ERR(phy_drd->usb30_sclk))
>  		clk_disable_unprepare(phy_drd->usb30_sclk);
>  
> @@ -585,6 +593,16 @@ static int exynos5_usbdrd_phy_probe(struct platform_device *pdev)
>  
>  	phy_drd->drv_data = drv_data;
>  
> +	/* Get required GPIO for vbus */
> +	phy_drd->gpio = of_get_named_gpio(dev->of_node,
> +					  "samsung,vbus-gpio", 0);

Is this dt property documented somewhere?
> +	if (!gpio_is_valid(phy_drd->gpio))
> +		dev_dbg(dev, "no usbdrd-phy vbus gpio defined\n");

No return here? Can the PHY be functional even without the VBUS?

Thanks
Kishon

Re: [PATCH] phy: exynos5-usbdrd: Add facility to toggle vbus gpio on/off

[Vivek Gautam]

Hi Kishon,


On Thu, Apr 10, 2014 at 2:39 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote:
> Hi.
>
> On Wednesday 09 April 2014 05:24 PM, Vivek Gautam wrote:
>> Adding support to enable/disable VBUS hooked to a gpio
>> to enable vbus supply on the port.
>>
>> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
>> ---
>>
>> Based on 'phy-exynos5-usbdrd' patches:
>> [PATCH V4 0/5] Add Exynos5 USB 3.0 phy driver based on generic PHY framework
>> http://www.spinics.net/lists/linux-usb/msg105507.html
>>
>>  drivers/phy/phy-exynos5-usbdrd.c |   18 ++++++++++++++++++
>>  1 file changed, 18 insertions(+)
>>
>> diff --git a/drivers/phy/phy-exynos5-usbdrd.c b/drivers/phy/phy-exynos5-usbdrd.c
>> index ff54a7c..5ca7485 100644
>> --- a/drivers/phy/phy-exynos5-usbdrd.c
>> +++ b/drivers/phy/phy-exynos5-usbdrd.c
>> @@ -18,6 +18,7 @@
>>  #include <linux/module.h>
>>  #include <linux/of.h>
>>  #include <linux/of_address.h>
>> +#include <linux/of_gpio.h>
>>  #include <linux/phy/phy.h>
>>  #include <linux/platform_device.h>
>>  #include <linux/mutex.h>
>> @@ -176,6 +177,7 @@ struct exynos5_usbdrd_phy {
>>       struct clk *ref_clk;
>>       unsigned long ref_rate;
>>       unsigned int refclk_reg;
>> +     int gpio;
>>  };
>>
>>  #define to_usbdrd_phy(inst) \
>> @@ -460,6 +462,9 @@ static int exynos5_usbdrd_phy_power_on(struct phy *phy)
>>       if (!IS_ERR(phy_drd->usb30_sclk))
>>               clk_prepare_enable(phy_drd->usb30_sclk);
>>
>> +     /* Toggle VBUS gpio - on */
>> +     gpio_set_value(phy_drd->gpio, 1);
>> +
>>       /* Power-on PHY*/
>>       inst->phy_cfg->phy_isol(inst, 0);
>>
>> @@ -476,6 +481,9 @@ static int exynos5_usbdrd_phy_power_off(struct phy *phy)
>>       /* Power-off the PHY */
>>       inst->phy_cfg->phy_isol(inst, 1);
>>
>> +     /* Toggle VBUS gpio - off */
>> +     gpio_set_value(phy_drd->gpio, 0);
>> +
>>       if (!IS_ERR(phy_drd->usb30_sclk))
>>               clk_disable_unprepare(phy_drd->usb30_sclk);
>>
>> @@ -585,6 +593,16 @@ static int exynos5_usbdrd_phy_probe(struct platform_device *pdev)
>>
>>       phy_drd->drv_data = drv_data;
>>
>> +     /* Get required GPIO for vbus */
>> +     phy_drd->gpio = of_get_named_gpio(dev->of_node,
>> +                                       "samsung,vbus-gpio", 0);
>
> Is this dt property documented somewhere?
>> +     if (!gpio_is_valid(phy_drd->gpio))
>> +             dev_dbg(dev, "no usbdrd-phy vbus gpio defined\n");
>
> No return here? Can the PHY be functional even without the VBUS?

On few boards, the switch IC's enable may no be controlled by a GPIO,
rather enabled by default.
So in those cases we may not want to stop PHY probe, when the VBUS
GPIO is not present.


-- 
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India

Re: [PATCH] phy: exynos5-usbdrd: Add facility to toggle vbus gpio on/off

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1494 bytes --]

On Wed, Apr 09, 2014 at 05:24:45PM +0530, Vivek Gautam wrote:
> Adding support to enable/disable VBUS hooked to a gpio
> to enable vbus supply on the port.
> 
> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
> ---
> 
> Based on 'phy-exynos5-usbdrd' patches:
> [PATCH V4 0/5] Add Exynos5 USB 3.0 phy driver based on generic PHY framework
> http://www.spinics.net/lists/linux-usb/msg105507.html
> 
>  drivers/phy/phy-exynos5-usbdrd.c |   18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/phy/phy-exynos5-usbdrd.c b/drivers/phy/phy-exynos5-usbdrd.c
> index ff54a7c..5ca7485 100644
> --- a/drivers/phy/phy-exynos5-usbdrd.c
> +++ b/drivers/phy/phy-exynos5-usbdrd.c
> @@ -18,6 +18,7 @@
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_address.h>
> +#include <linux/of_gpio.h>
>  #include <linux/phy/phy.h>
>  #include <linux/platform_device.h>
>  #include <linux/mutex.h>
> @@ -176,6 +177,7 @@ struct exynos5_usbdrd_phy {
>  	struct clk *ref_clk;
>  	unsigned long ref_rate;
>  	unsigned int refclk_reg;
> +	int gpio;
>  };
>  
>  #define to_usbdrd_phy(inst) \
> @@ -460,6 +462,9 @@ static int exynos5_usbdrd_phy_power_on(struct phy *phy)
>  	if (!IS_ERR(phy_drd->usb30_sclk))
>  		clk_prepare_enable(phy_drd->usb30_sclk);
>  
> +	/* Toggle VBUS gpio - on */
> +	gpio_set_value(phy_drd->gpio, 1);

It seems like you'd be better off using a regulator_enable() call for
this.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] phy: exynos5-usbdrd: Add facility to toggle vbus gpio on/off

[Vivek Gautam]

Hi Felipe,


On Sat, Apr 12, 2014 at 9:07 AM, Felipe Balbi <balbi@ti.com> wrote:
> On Wed, Apr 09, 2014 at 05:24:45PM +0530, Vivek Gautam wrote:
>> Adding support to enable/disable VBUS hooked to a gpio
>> to enable vbus supply on the port.
>>
>> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
>> ---
>>
>> Based on 'phy-exynos5-usbdrd' patches:
>> [PATCH V4 0/5] Add Exynos5 USB 3.0 phy driver based on generic PHY framework
>> http://www.spinics.net/lists/linux-usb/msg105507.html
>>
>>  drivers/phy/phy-exynos5-usbdrd.c |   18 ++++++++++++++++++
>>  1 file changed, 18 insertions(+)
>>
>> diff --git a/drivers/phy/phy-exynos5-usbdrd.c b/drivers/phy/phy-exynos5-usbdrd.c
>> index ff54a7c..5ca7485 100644
>> --- a/drivers/phy/phy-exynos5-usbdrd.c
>> +++ b/drivers/phy/phy-exynos5-usbdrd.c
>> @@ -18,6 +18,7 @@
>>  #include <linux/module.h>
>>  #include <linux/of.h>
>>  #include <linux/of_address.h>
>> +#include <linux/of_gpio.h>
>>  #include <linux/phy/phy.h>
>>  #include <linux/platform_device.h>
>>  #include <linux/mutex.h>
>> @@ -176,6 +177,7 @@ struct exynos5_usbdrd_phy {
>>       struct clk *ref_clk;
>>       unsigned long ref_rate;
>>       unsigned int refclk_reg;
>> +     int gpio;
>>  };
>>
>>  #define to_usbdrd_phy(inst) \
>> @@ -460,6 +462,9 @@ static int exynos5_usbdrd_phy_power_on(struct phy *phy)
>>       if (!IS_ERR(phy_drd->usb30_sclk))
>>               clk_prepare_enable(phy_drd->usb30_sclk);
>>
>> +     /* Toggle VBUS gpio - on */
>> +     gpio_set_value(phy_drd->gpio, 1);
>
> It seems like you'd be better off using a regulator_enable() call for
> this.

Sure, i will use a regulator for this vbus settings.


-- 
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India