2014-04-09 11:55:04

by Vivek Gautam

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

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

Signed-off-by: Vivek Gautam <[email protected]>
---

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


2014-04-09 12:11:42

by Sylwester Nawrocki

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

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 <[email protected]>
[...]
> + /* 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

2014-04-09 12:24:42

by Vivek Gautam

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

Hi Sylwester,


On Wed, Apr 9, 2014 at 5:41 PM, Sylwester Nawrocki
<[email protected]> 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 <[email protected]>
> [...]
>> + /* 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2014-04-09 12:38:17

by Tomasz Figa

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

Hi,

On 09.04.2014 14:24, Vivek Gautam wrote:
> Hi Sylwester,
>
>
> On Wed, Apr 9, 2014 at 5:41 PM, Sylwester Nawrocki
> <[email protected]> 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

2014-04-09 13:09:07

by Sylwester Nawrocki

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

On 09/04/14 14:24, Vivek Gautam wrote:
> On Wed, Apr 9, 2014 at 5:41 PM, Sylwester Nawrocki
> <[email protected]> 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

2014-04-10 09:09:53

by Kishon Vijay Abraham I

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

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 <[email protected]>
> ---
>
> 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

2014-04-10 10:32:58

by Vivek Gautam

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

Hi Kishon,


On Thu, Apr 10, 2014 at 2:39 PM, Kishon Vijay Abraham I <[email protected]> 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 <[email protected]>
>> ---
>>
>> 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

2014-04-10 11:07:09

by Vivek Gautam

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

Hi,


On Wed, Apr 9, 2014 at 6:08 PM, Tomasz Figa <[email protected]> wrote:
> Hi,
>
>
> On 09.04.2014 14:24, Vivek Gautam wrote:
>>
>> Hi Sylwester,
>>
>>
>> On Wed, Apr 9, 2014 at 5:41 PM, Sylwester Nawrocki
>> <[email protected]> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2014-04-12 03:39:58

by Felipe Balbi

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

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 <[email protected]>
> ---
>
> 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


Attachments:
(No filename) (1.41 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments

2014-04-14 08:12:34

by Vivek Gautam

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

Hi Felipe,


On Sat, Apr 12, 2014 at 9:07 AM, Felipe Balbi <[email protected]> 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 <[email protected]>
>> ---
>>
>> 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