| Message ID | 1397044485-26483-1-git-send-email-gautam.vivek@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 -- 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
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 -- 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
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 -- 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
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.
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 -- 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
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.
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 -- 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
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.
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.
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;
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(+)