linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/2] usb: ehci-exynos: Make provision for vdd regulators
@ 2014-06-06 14:20 Vivek Gautam
  2014-06-06 14:20 ` [PATCH v2 2/2] usb: ohci-exynos: " Vivek Gautam
  2014-06-11 15:39 ` [PATCH v2 1/2] usb: ehci-exynos: " Alan Stern
  0 siblings, 2 replies; 8+ messages in thread
From: Vivek Gautam @ 2014-06-06 14:20 UTC (permalink / raw)
  To: linux-usb, linux-samsung-soc, gregkh
  Cc: linux-kernel, linux-arm-kernel, stern, kgene.kim, Vivek Gautam,
	Jingoo Han

Facilitate getting required 3.3V and 1.0V VDD supply for
EHCI controller on Exynos.

With patches for regulators' nodes merged in 3.15:
c8c253f ARM: dts: Add regulator entries to smdk5420
275dcd2 ARM: dts: add max77686 pmic node for smdk5250,

certain perripherals will now need to ensure that,
they request VDD regulators in their drivers, and enable
them so as to make them working.

Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
Cc: Jingoo Han <jg1.han@samsung.com>
---

Changes from v1:
 - Addressed review comments for making the regulators optional,
   and thus refactored the checks for the obtained regulators.

 drivers/usb/host/ehci-exynos.c |   59 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 58 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
index d1c7621..b788ddd 100644
--- a/drivers/usb/host/ehci-exynos.c
+++ b/drivers/usb/host/ehci-exynos.c
@@ -21,6 +21,7 @@
 #include <linux/of_gpio.h>
 #include <linux/phy/phy.h>
 #include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
 #include <linux/usb/phy.h>
 #include <linux/usb/samsung_usb_phy.h>
 #include <linux/usb.h>
@@ -50,6 +51,8 @@ struct exynos_ehci_hcd {
 	struct usb_phy *phy;
 	struct usb_otg *otg;
 	struct phy *phy_g[PHY_NUMBER];
+	struct regulator *vdd33;
+	struct regulator *vdd10;
 };
 
 #define to_exynos_ehci(hcd) (struct exynos_ehci_hcd *)(hcd_to_ehci(hcd)->priv)
@@ -193,7 +196,31 @@ static int exynos_ehci_probe(struct platform_device *pdev)
 
 	err = exynos_ehci_get_phy(&pdev->dev, exynos_ehci);
 	if (err)
-		goto fail_clk;
+		goto fail_regulator1;
+
+	exynos_ehci->vdd33 = devm_regulator_get(&pdev->dev, "vdd33");
+	if (!IS_ERR(exynos_ehci->vdd33)) {
+		err = regulator_enable(exynos_ehci->vdd33);
+		if (err) {
+			dev_err(&pdev->dev,
+				"Failed to enable 3.3V Vdd supply\n");
+			goto fail_regulator1;
+		}
+	} else {
+		dev_warn(&pdev->dev, "Regulator 3.3V Vdd supply not found\n");
+	}
+
+	exynos_ehci->vdd10 = devm_regulator_get(&pdev->dev, "vdd10");
+	if (!IS_ERR(exynos_ehci->vdd10)) {
+		err = regulator_enable(exynos_ehci->vdd10);
+		if (err) {
+			dev_err(&pdev->dev,
+				"Failed to enable 1.0V Vdd supply\n");
+			goto fail_regulator2;
+		}
+	} else {
+		dev_warn(&pdev->dev, "Regulator 1.0V Vdd supply not found\n");
+	}
 
 skip_phy:
 
@@ -262,6 +289,10 @@ fail_add_hcd:
 fail_io:
 	clk_disable_unprepare(exynos_ehci->clk);
 fail_clk:
+	regulator_disable(exynos_ehci->vdd10);
+fail_regulator2:
+	regulator_disable(exynos_ehci->vdd33);
+fail_regulator1:
 	usb_put_hcd(hcd);
 	return err;
 }
@@ -280,6 +311,11 @@ static int exynos_ehci_remove(struct platform_device *pdev)
 
 	clk_disable_unprepare(exynos_ehci->clk);
 
+	if (!IS_ERR(exynos_ehci->vdd33))
+		regulator_disable(exynos_ehci->vdd33);
+	if (!IS_ERR(exynos_ehci->vdd10))
+		regulator_disable(exynos_ehci->vdd10);
+
 	usb_put_hcd(hcd);
 
 	return 0;
@@ -305,6 +341,11 @@ static int exynos_ehci_suspend(struct device *dev)
 
 	clk_disable_unprepare(exynos_ehci->clk);
 
+	if (!IS_ERR(exynos_ehci->vdd33))
+		regulator_disable(exynos_ehci->vdd33);
+	if (!IS_ERR(exynos_ehci->vdd10))
+		regulator_disable(exynos_ehci->vdd10);
+
 	return rc;
 }
 
@@ -314,6 +355,22 @@ static int exynos_ehci_resume(struct device *dev)
 	struct exynos_ehci_hcd *exynos_ehci = to_exynos_ehci(hcd);
 	int ret;
 
+	if (!IS_ERR(exynos_ehci->vdd33)) {
+		ret = regulator_enable(exynos_ehci->vdd33);
+		if (ret) {
+			dev_err(dev, "Failed to enable 3.3V Vdd supply\n");
+			return ret;
+		}
+	}
+
+	if (!IS_ERR(exynos_ehci->vdd10)) {
+		ret = regulator_enable(exynos_ehci->vdd10);
+		if (ret) {
+			dev_err(dev, "Failed to enable 1.0V Vdd supply\n");
+			return ret;
+		}
+	}
+
 	clk_prepare_enable(exynos_ehci->clk);
 
 	if (exynos_ehci->otg)
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 2/2] usb: ohci-exynos: Make provision for vdd regulators
  2014-06-06 14:20 [PATCH v2 1/2] usb: ehci-exynos: Make provision for vdd regulators Vivek Gautam
@ 2014-06-06 14:20 ` Vivek Gautam
  2014-06-25 12:43   ` [PATCH v3 " Vivek Gautam
  2014-06-11 15:39 ` [PATCH v2 1/2] usb: ehci-exynos: " Alan Stern
  1 sibling, 1 reply; 8+ messages in thread
From: Vivek Gautam @ 2014-06-06 14:20 UTC (permalink / raw)
  To: linux-usb, linux-samsung-soc, gregkh
  Cc: linux-kernel, linux-arm-kernel, stern, kgene.kim, Vivek Gautam,
	Jingoo Han

Facilitate getting required 3.3V and 1.0V VDD supply for
OHCI controller on Exynos.

With patches for regulators' nodes merged in 3.15:
c8c253f ARM: dts: Add regulator entries to smdk5420
275dcd2 ARM: dts: add max77686 pmic node for smdk5250,

certain perripherals will now need to ensure that,
they request VDD regulators in their drivers, and enable
them so as to make them working.

Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
Cc: Jingoo Han <jg1.han@samsung.com>
---

Changes from v1:
 - Addressed review comments for making the regulators optional,
   and thus refactored the checks for the obtained regulators.

 drivers/usb/host/ohci-exynos.c |   59 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 58 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
index 060a6a4..01f7c43 100644
--- a/drivers/usb/host/ohci-exynos.c
+++ b/drivers/usb/host/ohci-exynos.c
@@ -19,6 +19,7 @@
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/phy/phy.h>
+#include <linux/regulator/consumer.h>
 #include <linux/usb/phy.h>
 #include <linux/usb/samsung_usb_phy.h>
 #include <linux/usb.h>
@@ -41,6 +42,8 @@ struct exynos_ohci_hcd {
 	struct usb_phy *phy;
 	struct usb_otg *otg;
 	struct phy *phy_g[PHY_NUMBER];
+	struct regulator *vdd33;
+	struct regulator *vdd10;
 };
 
 static int exynos_ohci_get_phy(struct device *dev,
@@ -172,7 +175,31 @@ static int exynos_ohci_probe(struct platform_device *pdev)
 
 	err = exynos_ohci_get_phy(&pdev->dev, exynos_ohci);
 	if (err)
-		goto fail_clk;
+		goto fail_regulator1;
+
+	exynos_ohci->vdd33 = devm_regulator_get(&pdev->dev, "vdd33");
+	if (!IS_ERR(exynos_ohci->vdd33)) {
+		err = regulator_enable(exynos_ohci->vdd33);
+		if (err) {
+			dev_err(&pdev->dev,
+				"Failed to enable 3.3V Vdd supply\n");
+			goto fail_regulator1;
+		}
+	} else {
+		dev_warn(&pdev->dev, "Regulator 3.3V Vdd supply not found\n");
+	}
+
+	exynos_ohci->vdd10 = devm_regulator_get(&pdev->dev, "vdd10");
+	if (!IS_ERR(exynos_ohci->vdd10)) {
+		err = regulator_enable(exynos_ohci->vdd10);
+		if (err) {
+			dev_err(&pdev->dev,
+				"Failed to enable 1.0V Vdd supply\n");
+			goto fail_regulator2;
+		}
+	} else {
+		dev_warn(&pdev->dev, "Regulator 1.0V Vdd supply not found\n");
+	}
 
 skip_phy:
 	exynos_ohci->clk = devm_clk_get(&pdev->dev, "usbhost");
@@ -233,6 +260,10 @@ fail_add_hcd:
 fail_io:
 	clk_disable_unprepare(exynos_ohci->clk);
 fail_clk:
+	regulator_disable(exynos_ohci->vdd10);
+fail_regulator2:
+	regulator_disable(exynos_ohci->vdd33);
+fail_regulator1:
 	usb_put_hcd(hcd);
 	return err;
 }
@@ -251,6 +282,11 @@ static int exynos_ohci_remove(struct platform_device *pdev)
 
 	clk_disable_unprepare(exynos_ohci->clk);
 
+	if (!IS_ERR(exynos_ohci->vdd33))
+		regulator_disable(exynos_ohci->vdd33);
+	if (!IS_ERR(exynos_ohci->vdd10))
+		regulator_disable(exynos_ohci->vdd10);
+
 	usb_put_hcd(hcd);
 
 	return 0;
@@ -282,6 +318,11 @@ static int exynos_ohci_suspend(struct device *dev)
 
 	clk_disable_unprepare(exynos_ohci->clk);
 
+	if (!IS_ERR(exynos_ohci->vdd33))
+		regulator_disable(exynos_ohci->vdd33);
+	if (!IS_ERR(exynos_ohci->vdd10))
+		regulator_disable(exynos_ohci->vdd10);
+
 	return 0;
 }
 
@@ -291,6 +332,22 @@ static int exynos_ohci_resume(struct device *dev)
 	struct exynos_ohci_hcd *exynos_ohci	= to_exynos_ohci(hcd);
 	int ret;
 
+	if (!IS_ERR(exynos_ohci->vdd33)) {
+		ret = regulator_enable(exynos_ohci->vdd33);
+		if (ret) {
+			dev_err(dev, "Failed to enable 3.3V Vdd supply\n");
+			return ret;
+		}
+	}
+
+	if (!IS_ERR(exynos_ohci->vdd10)) {
+		ret = regulator_enable(exynos_ohci->vdd10);
+		if (ret) {
+			dev_err(dev, "Failed to enable 1.0V Vdd supply\n");
+			return ret;
+		}
+	}
+
 	clk_prepare_enable(exynos_ohci->clk);
 
 	if (exynos_ohci->otg)
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] usb: ehci-exynos: Make provision for vdd regulators
  2014-06-06 14:20 [PATCH v2 1/2] usb: ehci-exynos: Make provision for vdd regulators Vivek Gautam
  2014-06-06 14:20 ` [PATCH v2 2/2] usb: ohci-exynos: " Vivek Gautam
@ 2014-06-11 15:39 ` Alan Stern
  2014-06-12  4:21   ` Jingoo Han
  2014-06-13 11:33   ` Vivek Gautam
  1 sibling, 2 replies; 8+ messages in thread
From: Alan Stern @ 2014-06-11 15:39 UTC (permalink / raw)
  To: Vivek Gautam
  Cc: linux-usb, linux-samsung-soc, gregkh, linux-kernel,
	linux-arm-kernel, kgene.kim, Jingoo Han

On Fri, 6 Jun 2014, Vivek Gautam wrote:

> Facilitate getting required 3.3V and 1.0V VDD supply for
> EHCI controller on Exynos.
> 
> With patches for regulators' nodes merged in 3.15:
> c8c253f ARM: dts: Add regulator entries to smdk5420
> 275dcd2 ARM: dts: add max77686 pmic node for smdk5250,
> 
> certain perripherals will now need to ensure that,
> they request VDD regulators in their drivers, and enable
> them so as to make them working.

"Certain peripherals"?  Don't you mean "certain controllers"?

Does this mean some controllers don't need to use the VDD regulators?

> @@ -193,7 +196,31 @@ static int exynos_ehci_probe(struct platform_device *pdev)
>  
>  	err = exynos_ehci_get_phy(&pdev->dev, exynos_ehci);
>  	if (err)
> -		goto fail_clk;
> +		goto fail_regulator1;
> +
> +	exynos_ehci->vdd33 = devm_regulator_get(&pdev->dev, "vdd33");
> +	if (!IS_ERR(exynos_ehci->vdd33)) {
> +		err = regulator_enable(exynos_ehci->vdd33);
> +		if (err) {
> +			dev_err(&pdev->dev,
> +				"Failed to enable 3.3V Vdd supply\n");
> +			goto fail_regulator1;
> +		}
> +	} else {
> +		dev_warn(&pdev->dev, "Regulator 3.3V Vdd supply not found\n");
> +	}

What if this is one of the controllers that don't need to use a VDD 
regulator?  Do you really want to print out a warning in that case?  
Should you call devm_regulator_get_optional() instead?

Alan Stern


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] usb: ehci-exynos: Make provision for vdd regulators
  2014-06-11 15:39 ` [PATCH v2 1/2] usb: ehci-exynos: " Alan Stern
@ 2014-06-12  4:21   ` Jingoo Han
  2014-06-13 11:33   ` Vivek Gautam
  1 sibling, 0 replies; 8+ messages in thread
From: Jingoo Han @ 2014-06-12  4:21 UTC (permalink / raw)
  To: 'Alan Stern', 'Vivek Gautam'
  Cc: linux-usb, linux-samsung-soc, gregkh, linux-kernel,
	linux-arm-kernel, kgene.kim, 'Jingoo Han'

On Thursday, June 12, 2014 12:39 AM, Alan Stern wrote:
> On Fri, 6 Jun 2014, Vivek Gautam wrote:
> 
> > Facilitate getting required 3.3V and 1.0V VDD supply for
> > EHCI controller on Exynos.
> >
> > With patches for regulators' nodes merged in 3.15:
> > c8c253f ARM: dts: Add regulator entries to smdk5420
> > 275dcd2 ARM: dts: add max77686 pmic node for smdk5250,
> >
> > certain perripherals will now need to ensure that,
> > they request VDD regulators in their drivers, and enable
> > them so as to make them working.
> 
> "Certain peripherals"?  Don't you mean "certain controllers"?
> 
> Does this mean some controllers don't need to use the VDD regulators?
> 
> > @@ -193,7 +196,31 @@ static int exynos_ehci_probe(struct platform_device *pdev)
> >
> >  	err = exynos_ehci_get_phy(&pdev->dev, exynos_ehci);
> >  	if (err)
> > -		goto fail_clk;
> > +		goto fail_regulator1;
> > +
> > +	exynos_ehci->vdd33 = devm_regulator_get(&pdev->dev, "vdd33");
> > +	if (!IS_ERR(exynos_ehci->vdd33)) {
> > +		err = regulator_enable(exynos_ehci->vdd33);
> > +		if (err) {
> > +			dev_err(&pdev->dev,
> > +				"Failed to enable 3.3V Vdd supply\n");
> > +			goto fail_regulator1;
> > +		}
> > +	} else {
> > +		dev_warn(&pdev->dev, "Regulator 3.3V Vdd supply not found\n");
> > +	}
> 
> What if this is one of the controllers that don't need to use a VDD
> regulator?  Do you really want to print out a warning in that case?
> Should you call devm_regulator_get_optional() instead?

I agree with Alan's suggestion. This warning message is not
proper, when USB controllers that don't need a VDD regulator
are used. The devm_regulator_get_optional() looks better.

Best regards,
Jingoo Han



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] usb: ehci-exynos: Make provision for vdd regulators
  2014-06-11 15:39 ` [PATCH v2 1/2] usb: ehci-exynos: " Alan Stern
  2014-06-12  4:21   ` Jingoo Han
@ 2014-06-13 11:33   ` Vivek Gautam
  2014-06-25 12:42     ` [PATCH v3 " Vivek Gautam
  1 sibling, 1 reply; 8+ messages in thread
From: Vivek Gautam @ 2014-06-13 11:33 UTC (permalink / raw)
  To: Alan Stern
  Cc: Linux USB Mailing List, linux-samsung-soc, Greg KH, linux-kernel,
	linux-arm-kernel, Kukjin Kim, Jingoo Han

Hi,


On Wed, Jun 11, 2014 at 9:09 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Fri, 6 Jun 2014, Vivek Gautam wrote:
>
>> Facilitate getting required 3.3V and 1.0V VDD supply for
>> EHCI controller on Exynos.
>>
>> With patches for regulators' nodes merged in 3.15:
>> c8c253f ARM: dts: Add regulator entries to smdk5420
>> 275dcd2 ARM: dts: add max77686 pmic node for smdk5250,
>>
>> certain perripherals will now need to ensure that,
>> they request VDD regulators in their drivers, and enable
>> them so as to make them working.
>
> "Certain peripherals"?  Don't you mean "certain controllers"?

Right, 'certain controllers'.

>
> Does this mean some controllers don't need to use the VDD regulators?

Actually until the two patches got merged, the USB controllers were
depending on bootloader
for the VDD supply, wherein it was enabled, which ofcourse was bad.
And by 'certain' i meant that above mentioned dt patches enable only the minimum
number of regulators for the system, however leaves other for the
drivers to enable.
Anyways, i will re-do this commit message.

>
>> @@ -193,7 +196,31 @@ static int exynos_ehci_probe(struct platform_device *pdev)
>>
>>       err = exynos_ehci_get_phy(&pdev->dev, exynos_ehci);
>>       if (err)
>> -             goto fail_clk;
>> +             goto fail_regulator1;
>> +
>> +     exynos_ehci->vdd33 = devm_regulator_get(&pdev->dev, "vdd33");
>> +     if (!IS_ERR(exynos_ehci->vdd33)) {
>> +             err = regulator_enable(exynos_ehci->vdd33);
>> +             if (err) {
>> +                     dev_err(&pdev->dev,
>> +                             "Failed to enable 3.3V Vdd supply\n");
>> +                     goto fail_regulator1;
>> +             }
>> +     } else {
>> +             dev_warn(&pdev->dev, "Regulator 3.3V Vdd supply not found\n");
>> +     }
>
> What if this is one of the controllers that don't need to use a VDD
> regulator?  Do you really want to print out a warning in that case?
> Should you call devm_regulator_get_optional() instead?

Right, better to use devm_regulator_get_optional(). Thanks for
pointing this out.




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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3 1/2] usb: ehci-exynos: Make provision for vdd regulators
  2014-06-13 11:33   ` Vivek Gautam
@ 2014-06-25 12:42     ` Vivek Gautam
  2015-05-31  6:34       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 8+ messages in thread
From: Vivek Gautam @ 2014-06-25 12:42 UTC (permalink / raw)
  To: linux-usb, linux-samsung-soc
  Cc: devicetree, linux-doc, linux-arm-kernel, linux-kernel, gregkh,
	Vivek Gautam, Jingoo Han, Alan Stern

Facilitate getting required 3.3V and 1.0V VDD supply for
EHCI controller on Exynos.

With the patches for regulators' nodes merged in 3.15:
c8c253f ARM: dts: Add regulator entries to smdk5420
275dcd2 ARM: dts: add max77686 pmic node for smdk5250,
the exynos systems turn on only minimal number of regulators.

Until now, the VDD regulator supplies were either turned on
by the bootloader, or the regulators were enabled by default
in the kernel, so that the controller drivers did not need to
care about turning on these regulators on their own.
This was rather bad about these controller drivers.
So ensuring now that the controller driver requests the necessary
VDD regulators (if available, unless there are direct VDD rails),
and enable them so as to make them working.

Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Alan Stern <stern@rowland.harvard.edu>
---

Changes from v2:
 - replaced devm_regulator_get() with devm_regulator_get_optional().
 - Added Documentation for the vdd supplies for the controller.
 - Re-did the commit message.

 .../devicetree/bindings/usb/exynos-usb.txt         |    2 +
 drivers/usb/host/ehci-exynos.c                     |   55 +++++++++++++++++++-
 2 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/usb/exynos-usb.txt b/Documentation/devicetree/bindings/usb/exynos-usb.txt
index a3b5990..b04b015 100644
--- a/Documentation/devicetree/bindings/usb/exynos-usb.txt
+++ b/Documentation/devicetree/bindings/usb/exynos-usb.txt
@@ -23,6 +23,8 @@ Required properties:
 Optional properties:
  - samsung,vbus-gpio:  if present, specifies the GPIO that
    needs to be pulled up for the bus to be powered.
+ - vdd33-supply: handle to 3.3V Vdd supply regulator for the controller.
+ - vdd10-supply: handle to 1.0V Vdd supply regulator for the controller.
 
 Example:
 
diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
index d1c7621..b920642 100644
--- a/drivers/usb/host/ehci-exynos.c
+++ b/drivers/usb/host/ehci-exynos.c
@@ -21,6 +21,7 @@
 #include <linux/of_gpio.h>
 #include <linux/phy/phy.h>
 #include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
 #include <linux/usb/phy.h>
 #include <linux/usb/samsung_usb_phy.h>
 #include <linux/usb.h>
@@ -50,6 +51,8 @@ struct exynos_ehci_hcd {
 	struct usb_phy *phy;
 	struct usb_otg *otg;
 	struct phy *phy_g[PHY_NUMBER];
+	struct regulator *vdd33;
+	struct regulator *vdd10;
 };
 
 #define to_exynos_ehci(hcd) (struct exynos_ehci_hcd *)(hcd_to_ehci(hcd)->priv)
@@ -193,7 +196,27 @@ static int exynos_ehci_probe(struct platform_device *pdev)
 
 	err = exynos_ehci_get_phy(&pdev->dev, exynos_ehci);
 	if (err)
-		goto fail_clk;
+		goto fail_regulator1;
+
+	exynos_ehci->vdd33 = devm_regulator_get_optional(&pdev->dev, "vdd33");
+	if (!IS_ERR(exynos_ehci->vdd33)) {
+		err = regulator_enable(exynos_ehci->vdd33);
+		if (err) {
+			dev_err(&pdev->dev,
+				"Failed to enable 3.3V Vdd supply\n");
+			goto fail_regulator1;
+		}
+	}
+
+	exynos_ehci->vdd10 = devm_regulator_get_optional(&pdev->dev, "vdd10");
+	if (!IS_ERR(exynos_ehci->vdd10)) {
+		err = regulator_enable(exynos_ehci->vdd10);
+		if (err) {
+			dev_err(&pdev->dev,
+				"Failed to enable 1.0V Vdd supply\n");
+			goto fail_regulator2;
+		}
+	}
 
 skip_phy:
 
@@ -262,6 +285,10 @@ fail_add_hcd:
 fail_io:
 	clk_disable_unprepare(exynos_ehci->clk);
 fail_clk:
+	regulator_disable(exynos_ehci->vdd10);
+fail_regulator2:
+	regulator_disable(exynos_ehci->vdd33);
+fail_regulator1:
 	usb_put_hcd(hcd);
 	return err;
 }
@@ -280,6 +307,11 @@ static int exynos_ehci_remove(struct platform_device *pdev)
 
 	clk_disable_unprepare(exynos_ehci->clk);
 
+	if (!IS_ERR(exynos_ehci->vdd33))
+		regulator_disable(exynos_ehci->vdd33);
+	if (!IS_ERR(exynos_ehci->vdd10))
+		regulator_disable(exynos_ehci->vdd10);
+
 	usb_put_hcd(hcd);
 
 	return 0;
@@ -305,6 +337,11 @@ static int exynos_ehci_suspend(struct device *dev)
 
 	clk_disable_unprepare(exynos_ehci->clk);
 
+	if (!IS_ERR(exynos_ehci->vdd33))
+		regulator_disable(exynos_ehci->vdd33);
+	if (!IS_ERR(exynos_ehci->vdd10))
+		regulator_disable(exynos_ehci->vdd10);
+
 	return rc;
 }
 
@@ -314,6 +351,22 @@ static int exynos_ehci_resume(struct device *dev)
 	struct exynos_ehci_hcd *exynos_ehci = to_exynos_ehci(hcd);
 	int ret;
 
+	if (!IS_ERR(exynos_ehci->vdd33)) {
+		ret = regulator_enable(exynos_ehci->vdd33);
+		if (ret) {
+			dev_err(dev, "Failed to enable 3.3V Vdd supply\n");
+			return ret;
+		}
+	}
+
+	if (!IS_ERR(exynos_ehci->vdd10)) {
+		ret = regulator_enable(exynos_ehci->vdd10);
+		if (ret) {
+			dev_err(dev, "Failed to enable 1.0V Vdd supply\n");
+			return ret;
+		}
+	}
+
 	clk_prepare_enable(exynos_ehci->clk);
 
 	if (exynos_ehci->otg)
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3 2/2] usb: ohci-exynos: Make provision for vdd regulators
  2014-06-06 14:20 ` [PATCH v2 2/2] usb: ohci-exynos: " Vivek Gautam
@ 2014-06-25 12:43   ` Vivek Gautam
  0 siblings, 0 replies; 8+ messages in thread
From: Vivek Gautam @ 2014-06-25 12:43 UTC (permalink / raw)
  To: linux-usb, linux-samsung-soc
  Cc: devicetree, linux-doc, linux-arm-kernel, linux-kernel, gregkh,
	Vivek Gautam, Jingoo Han, Alan Stern

Facilitate getting required 3.3V and 1.0V VDD supply for
OHCI controller on Exynos.

With patches for regulators' nodes merged in 3.15:
c8c253f ARM: dts: Add regulator entries to smdk5420
275dcd2 ARM: dts: add max77686 pmic node for smdk5250,
the exynos systems turn on only minimal number of regulators.

Until now, the VDD regulator supplies were either turned on
by the bootloader, or the regulators were enabled by default
in the kernel, so that the controller drivers did not need to
care about turning on these regulators on their own.
This was rather bad about these controller drivers.
So ensuring now that the controller driver requests the necessary
VDD regulators (if available, unless there are direct VDD rails),
and enable them so as to make them working.

Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Alan Stern <stern@rowland.harvard.edu>
---

Changes from v2:
 - replaced devm_regulator_get() with devm_regulator_get_optional().
 - Added Documentation for the vdd supplies for the controller.
 - Re-did the commit message.

 .../devicetree/bindings/usb/exynos-usb.txt         |    4 ++
 drivers/usb/host/ohci-exynos.c                     |   55 +++++++++++++++++++-
 2 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/usb/exynos-usb.txt b/Documentation/devicetree/bindings/usb/exynos-usb.txt
index b04b015..3991c50 100644
--- a/Documentation/devicetree/bindings/usb/exynos-usb.txt
+++ b/Documentation/devicetree/bindings/usb/exynos-usb.txt
@@ -63,6 +63,10 @@ Required properties:
 			      port 2 is HSIC phy1
 	- phys: from the *Generic PHY* bindings, specifying phy used by port.
 
+Optional properties:
+ - vdd33-supply: handle to 3.3V Vdd supply regulator for the controller.
+ - vdd10-supply: handle to 1.0V Vdd supply regulator for the controller.
+
 Example:
 	usb@12120000 {
 		compatible = "samsung,exynos4210-ohci";
diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
index 060a6a4..57c807c 100644
--- a/drivers/usb/host/ohci-exynos.c
+++ b/drivers/usb/host/ohci-exynos.c
@@ -19,6 +19,7 @@
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/phy/phy.h>
+#include <linux/regulator/consumer.h>
 #include <linux/usb/phy.h>
 #include <linux/usb/samsung_usb_phy.h>
 #include <linux/usb.h>
@@ -41,6 +42,8 @@ struct exynos_ohci_hcd {
 	struct usb_phy *phy;
 	struct usb_otg *otg;
 	struct phy *phy_g[PHY_NUMBER];
+	struct regulator *vdd33;
+	struct regulator *vdd10;
 };
 
 static int exynos_ohci_get_phy(struct device *dev,
@@ -172,7 +175,27 @@ static int exynos_ohci_probe(struct platform_device *pdev)
 
 	err = exynos_ohci_get_phy(&pdev->dev, exynos_ohci);
 	if (err)
-		goto fail_clk;
+		goto fail_regulator1;
+
+	exynos_ohci->vdd33 = devm_regulator_get_optional(&pdev->dev, "vdd33");
+	if (!IS_ERR(exynos_ohci->vdd33)) {
+		err = regulator_enable(exynos_ohci->vdd33);
+		if (err) {
+			dev_err(&pdev->dev,
+				"Failed to enable 3.3V Vdd supply\n");
+			goto fail_regulator1;
+		}
+	}
+
+	exynos_ohci->vdd10 = devm_regulator_get_optional(&pdev->dev, "vdd10");
+	if (!IS_ERR(exynos_ohci->vdd10)) {
+		err = regulator_enable(exynos_ohci->vdd10);
+		if (err) {
+			dev_err(&pdev->dev,
+				"Failed to enable 1.0V Vdd supply\n");
+			goto fail_regulator2;
+		}
+	}
 
 skip_phy:
 	exynos_ohci->clk = devm_clk_get(&pdev->dev, "usbhost");
@@ -233,6 +256,10 @@ fail_add_hcd:
 fail_io:
 	clk_disable_unprepare(exynos_ohci->clk);
 fail_clk:
+	regulator_disable(exynos_ohci->vdd10);
+fail_regulator2:
+	regulator_disable(exynos_ohci->vdd33);
+fail_regulator1:
 	usb_put_hcd(hcd);
 	return err;
 }
@@ -251,6 +278,11 @@ static int exynos_ohci_remove(struct platform_device *pdev)
 
 	clk_disable_unprepare(exynos_ohci->clk);
 
+	if (!IS_ERR(exynos_ohci->vdd33))
+		regulator_disable(exynos_ohci->vdd33);
+	if (!IS_ERR(exynos_ohci->vdd10))
+		regulator_disable(exynos_ohci->vdd10);
+
 	usb_put_hcd(hcd);
 
 	return 0;
@@ -282,6 +314,11 @@ static int exynos_ohci_suspend(struct device *dev)
 
 	clk_disable_unprepare(exynos_ohci->clk);
 
+	if (!IS_ERR(exynos_ohci->vdd33))
+		regulator_disable(exynos_ohci->vdd33);
+	if (!IS_ERR(exynos_ohci->vdd10))
+		regulator_disable(exynos_ohci->vdd10);
+
 	return 0;
 }
 
@@ -291,6 +328,22 @@ static int exynos_ohci_resume(struct device *dev)
 	struct exynos_ohci_hcd *exynos_ohci	= to_exynos_ohci(hcd);
 	int ret;
 
+	if (!IS_ERR(exynos_ohci->vdd33)) {
+		ret = regulator_enable(exynos_ohci->vdd33);
+		if (ret) {
+			dev_err(dev, "Failed to enable 3.3V Vdd supply\n");
+			return ret;
+		}
+	}
+
+	if (!IS_ERR(exynos_ohci->vdd10)) {
+		ret = regulator_enable(exynos_ohci->vdd10);
+		if (ret) {
+			dev_err(dev, "Failed to enable 1.0V Vdd supply\n");
+			return ret;
+		}
+	}
+
 	clk_prepare_enable(exynos_ohci->clk);
 
 	if (exynos_ohci->otg)
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/2] usb: ehci-exynos: Make provision for vdd regulators
  2014-06-25 12:42     ` [PATCH v3 " Vivek Gautam
@ 2015-05-31  6:34       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 8+ messages in thread
From: Krzysztof Kozlowski @ 2015-05-31  6:34 UTC (permalink / raw)
  To: Vivek Gautam
  Cc: linux-usb, linux-samsung-soc, devicetree, linux-doc,
	linux-arm-kernel, linux-kernel, gregkh, Jingoo Han, Alan Stern

2014-06-25 21:42 GMT+09:00 Vivek Gautam <gautam.vivek@samsung.com>:
> Facilitate getting required 3.3V and 1.0V VDD supply for
> EHCI controller on Exynos.
>
> With the patches for regulators' nodes merged in 3.15:
> c8c253f ARM: dts: Add regulator entries to smdk5420
> 275dcd2 ARM: dts: add max77686 pmic node for smdk5250,
> the exynos systems turn on only minimal number of regulators.
>
> Until now, the VDD regulator supplies were either turned on
> by the bootloader, or the regulators were enabled by default
> in the kernel, so that the controller drivers did not need to
> care about turning on these regulators on their own.
> This was rather bad about these controller drivers.
> So ensuring now that the controller driver requests the necessary
> VDD regulators (if available, unless there are direct VDD rails),
> and enable them so as to make them working.
>
> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
> Cc: Jingoo Han <jg1.han@samsung.com>
> Cc: Alan Stern <stern@rowland.harvard.edu>
> ---
>
> Changes from v2:
>  - replaced devm_regulator_get() with devm_regulator_get_optional().
>  - Added Documentation for the vdd supplies for the controller.
>  - Re-did the commit message.

The patch looks useful but it seems it was forgotten. Do you have any
plans to resend it?

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2015-05-31  6:34 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-06 14:20 [PATCH v2 1/2] usb: ehci-exynos: Make provision for vdd regulators Vivek Gautam
2014-06-06 14:20 ` [PATCH v2 2/2] usb: ohci-exynos: " Vivek Gautam
2014-06-25 12:43   ` [PATCH v3 " Vivek Gautam
2014-06-11 15:39 ` [PATCH v2 1/2] usb: ehci-exynos: " Alan Stern
2014-06-12  4:21   ` Jingoo Han
2014-06-13 11:33   ` Vivek Gautam
2014-06-25 12:42     ` [PATCH v3 " Vivek Gautam
2015-05-31  6:34       ` Krzysztof Kozlowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).