[PATCH 0/2] usb: ehci/ohci-exynos: Move to generic phy framework

[PATCH 0/2] usb: ehci/ohci-exynos: Move to generic phy framework

[Vivek Gautam]

Added required support for getting phy-provider from generic phy framework

Based and tested on 'usb-next' branch of Greg's usb tree, with relevant
device tree patches (which i will sending soon).

Patch:
usb: ehci-exynos: Change to use phy provided by the generic phy framework
is the patch from Kamil's usb phy patch-series:
[PATCH v6 8/8] usb: ehci-exynos: Change to use phy provided by the generic phy framework
This is reworked for addressing review comments received in mailing list.

Patch:
usb: ohci-exynos: Add facility to use phy provided by the generic phy framework
is also reworked for addressing review comments.

Corresponding changelog for patches mentioned in each patch.

Kamil Debski (1):
  usb: ehci-exynos: Change to use phy provided by the generic phy
    framework

Vivek Gautam (1):
  usb: ohci-exynos: Add facility to use phy provided by the generic phy
    framework

 .../devicetree/bindings/usb/exynos-usb.txt         |   37 ++++++
 drivers/usb/host/ehci-exynos.c                     |  123 ++++++++++++++++++--
 drivers/usb/host/ohci-exynos.c                     |  123 ++++++++++++++++++--
 3 files changed, 263 insertions(+), 20 deletions(-)

-- 
1.7.10.4

[PATCH v2 1/2] usb: ohci-exynos: Add facility to use phy provided by the generic phy framework

[Vivek Gautam]

Add support to consume phy provided by Generic phy framework.
Keeping the support for older usb-phy intact right now, in order
to prevent any functionality break in absence of relevant
device tree side change for ohci-exynos.
Once we move to new phy in the device nodes for ohci, we can
remove the support for older phys.

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 v1:
 - Made two separate routines for exynos_ohci_phyg_on() and
   exynos_ohci_phyg_off().
 - Separated out the phy-get related code from probe() to separate function
   exynos_ehci_get_phy().
 - Using proper error labels in the code.

 .../devicetree/bindings/usb/exynos-usb.txt         |   19 +++
 drivers/usb/host/ohci-exynos.c                     |  123 ++++++++++++++++++--
 2 files changed, 132 insertions(+), 10 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/exynos-usb.txt b/Documentation/devicetree/bindings/usb/exynos-usb.txt
index d967ba1..03b7e43 100644
--- a/Documentation/devicetree/bindings/usb/exynos-usb.txt
+++ b/Documentation/devicetree/bindings/usb/exynos-usb.txt
@@ -38,6 +38,15 @@ Required properties:
  - interrupts: interrupt number to the cpu.
  - clocks: from common clock binding: handle to usb clock.
  - clock-names: from common clock binding: Shall be "usbhost".
+ - port: if in the SoC there are OHCI phys, they should be listed here.
+   One phy per port. Each port should have its 'reg' entry.
+	- reg: port number on OHCI controller, e.g
+	       On Exynos5250, port 0 is USB2.0 otg phy
+			      port 1 is HSIC phy0
+			      port 2 is HSIC phy1
+	- phys: from the *Generic PHY* bindings specifying phy used by port.
+	- phy-names: from the *Generic PHY* bindings specifying name of phy
+		     used by the port.
 
 Example:
 	usb@12120000 {
@@ -47,6 +56,16 @@ Example:
 
 		clocks = <&clock 285>;
 		clock-names = "usbhost";
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+		port@0 {
+		    reg = <0>;
+		    phys = <&usb2phy 1>;
+		    phy-names = "host";
+		    status = "disabled";
+		};
+
 	};
 
 DWC3
diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
index 68588d8..eac47cb 100644
--- a/drivers/usb/host/ohci-exynos.c
+++ b/drivers/usb/host/ohci-exynos.c
@@ -18,6 +18,7 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
+#include <linux/phy/phy.h>
 #include <linux/usb/phy.h>
 #include <linux/usb/samsung_usb_phy.h>
 #include <linux/usb.h>
@@ -33,12 +34,111 @@ static struct hc_driver __read_mostly exynos_ohci_hc_driver;
 
 #define to_exynos_ohci(hcd) (struct exynos_ohci_hcd *)(hcd_to_ohci(hcd)->priv)
 
+#define PHY_NUMBER 3
 struct exynos_ohci_hcd {
 	struct clk *clk;
 	struct usb_phy *phy;
 	struct usb_otg *otg;
+	struct phy *phy_g[PHY_NUMBER];
 };
 
+static int exynos_ohci_get_phy(struct platform_device *pdev,
+				struct exynos_ohci_hcd *exynos_ohci)
+{
+	struct device_node *child;
+	struct phy *phy;
+	int phy_number;
+	int ret = 0;
+
+	exynos_ohci->phy = devm_usb_get_phy(&pdev->dev, USB_PHY_TYPE_USB2);
+	if (IS_ERR(exynos_ohci->phy)) {
+		ret = PTR_ERR(exynos_ohci->phy);
+		/* This is the case when PHY config is disabled */
+		if (ret == -ENXIO || ret == -ENODEV) {
+			dev_dbg(&pdev->dev, "Failed to get usb2 phy\n");
+			exynos_ohci->phy = NULL;
+			ret = 0;
+		} else if (ret == -EPROBE_DEFER) {
+			goto fail_phy;
+		} else {
+			dev_err(&pdev->dev, "no usb2 phy configured\n");
+			goto fail_phy;
+		}
+	} else {
+		exynos_ohci->otg = exynos_ohci->phy->otg;
+	}
+
+	/* Getting generic phy:
+	 * We are keeping both types of phys as a part of transiting OHCI
+	 * to generic phy framework, so that in absence of supporting dts
+	 * changes the functionality doesn't break.
+	 * Once we move the ohci dt nodes to use new generic phys,
+	 * we can remove support for older PHY in this driver.
+	 */
+	for_each_available_child_of_node(pdev->dev.of_node, child) {
+		ret = of_property_read_u32(child, "reg", &phy_number);
+		if (ret) {
+			dev_err(&pdev->dev, "Failed to parse device tree\n");
+			of_node_put(child);
+			goto fail_phy;
+		}
+		if (phy_number >= PHY_NUMBER) {
+			dev_err(&pdev->dev, "Invalid number of PHYs\n");
+			of_node_put(child);
+			ret = -EINVAL;
+			goto fail_phy;
+		}
+		phy = devm_of_phy_get(&pdev->dev, child, 0);
+		of_node_put(child);
+		if (IS_ERR(phy)) {
+			ret = PTR_ERR(phy);
+			/* This is the case when PHY config is disabled */
+			if (ret == -ENOSYS || ret == -ENODEV) {
+				dev_dbg(&pdev->dev, "Failed to get usb2 phy\n");
+				phy = NULL;
+				ret = 0;
+			} else if (ret == -EPROBE_DEFER) {
+				goto fail_phy;
+			} else {
+				dev_err(&pdev->dev, "no usb2 phy configured\n");
+				goto fail_phy;
+			}
+		}
+		exynos_ohci->phy_g[phy_number] = phy;
+	}
+
+fail_phy:
+	return ret;
+}
+
+static int exynos_ohci_phyg_on(struct phy *phy[])
+{
+	int i;
+	int ret = 0;
+
+	for (i = 0; ret == 0 && i < PHY_NUMBER; i++)
+		if (phy[i])
+			ret = phy_power_on(phy[i]);
+	if (ret)
+		for (i--; i >= 0; i--)
+			if (phy[i])
+				phy_power_off(phy[i]);
+
+	return ret;
+}
+
+static int exynos_ohci_phyg_off(struct phy *phy[])
+{
+	int i;
+	int ret = 0;
+
+	for (i = 0; ret == 0 && i < PHY_NUMBER; i++)
+		if (phy[i])
+			ret = phy_power_off(phy[i]);
+
+	return ret;
+}
+
 static void exynos_ohci_phy_enable(struct platform_device *pdev)
 {
 	struct usb_hcd *hcd = platform_get_drvdata(pdev);
@@ -46,6 +146,11 @@ static void exynos_ohci_phy_enable(struct platform_device *pdev)
 
 	if (exynos_ohci->phy)
 		usb_phy_init(exynos_ohci->phy);
+
+	if (exynos_ohci->phy_g)
+		if (exynos_ohci_phyg_on(exynos_ohci->phy_g))
+			dev_warn(&pdev->dev, "Failed to enable phy\n");
+
 }
 
 static void exynos_ohci_phy_disable(struct platform_device *pdev)
@@ -55,6 +160,10 @@ static void exynos_ohci_phy_disable(struct platform_device *pdev)
 
 	if (exynos_ohci->phy)
 		usb_phy_shutdown(exynos_ohci->phy);
+
+	if (exynos_ohci->phy_g)
+		if (exynos_ohci_phyg_off(exynos_ohci->phy_g))
+			dev_warn(&pdev->dev, "Failed to enable phy\n");
 }
 
 static int exynos_ohci_probe(struct platform_device *pdev)
@@ -62,7 +171,6 @@ static int exynos_ohci_probe(struct platform_device *pdev)
 	struct exynos_ohci_hcd *exynos_ohci;
 	struct usb_hcd *hcd;
 	struct resource *res;
-	struct usb_phy *phy;
 	int irq;
 	int err;
 
@@ -88,15 +196,9 @@ static int exynos_ohci_probe(struct platform_device *pdev)
 					"samsung,exynos5440-ohci"))
 		goto skip_phy;
 
-	phy = devm_usb_get_phy(&pdev->dev, USB_PHY_TYPE_USB2);
-	if (IS_ERR(phy)) {
-		usb_put_hcd(hcd);
-		dev_warn(&pdev->dev, "no platform data or transceiver defined\n");
-		return -EPROBE_DEFER;
-	} else {
-		exynos_ohci->phy = phy;
-		exynos_ohci->otg = phy->otg;
-	}
+	err = exynos_ohci_get_phy(pdev, exynos_ohci);
+	if (err)
+		goto fail_clk;
 
 skip_phy:
 	exynos_ohci->clk = devm_clk_get(&pdev->dev, "usbhost");
@@ -151,6 +253,7 @@ skip_phy:
 
 fail_add_hcd:
 	exynos_ohci_phy_disable(pdev);
+	exynos_ohci_phyg_off(exynos_ohci->phy_g);
 fail_io:
 	clk_disable_unprepare(exynos_ohci->clk);
 fail_clk:
-- 
1.7.10.4

[PATCH v7 2/2] usb: ehci-exynos: Change to use phy provided by the generic phy framework

[Vivek Gautam]

From: Kamil Debski <k.debski@samsung.com>

Add the phy provider, supplied by new Exynos-usb2phy using
Generic phy framework.
Keeping the support for older USB phy intact right now, in order
to prevent any functionality break in absence of relevant
device tree side change for ehci-exynos.
Once we move to new phy in the device nodes for ohci, we can
remove the support for older phys.

Signed-off-by: Kamil Debski <k.debski@samsung.com>
[gautam.vivek@samsung.com: Addressed review comments from mailing list]
[gautam.vivek@samsung.com: Kept the code for old usb-phy, and just
added support for new exynos5-usb2phy in generic phy framework]
[gautam.vivek@samsung.com: Edited the commit message]
Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
---

Although the patch-series is named as 'v2' version, but
this patch is named as 'v7' since it is the next version of the patch
sent by Kamil:
[PATCH v6 8/8] usb: ehci-exynos: Change to use phy provided by the generic phy framework
https://lkml.org/lkml/2014/1/29/298

Changes from v6:
 - Added documentation for 'port' property including all its fields -
      reg, phys, phy-names.
 - Fixed looping in 'exynos_phys_on()' and renamed this function as
      exynos_ehci_phyg_on()
 - To avoid any regression because of movement from old usb-phy drivers
   to new generic phy framework, keeping the changes for old usb-phy driver
   intact, and just added support for phy provider from generic phy framework.
 - Separated out the phy-get related code from probe() to separate function
   exynos_ehci_get_phy().

 .../devicetree/bindings/usb/exynos-usb.txt         |   18 +++
 drivers/usb/host/ehci-exynos.c                     |  123 ++++++++++++++++++--
 2 files changed, 131 insertions(+), 10 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/exynos-usb.txt b/Documentation/devicetree/bindings/usb/exynos-usb.txt
index 03b7e43..4f368b0 100644
--- a/Documentation/devicetree/bindings/usb/exynos-usb.txt
+++ b/Documentation/devicetree/bindings/usb/exynos-usb.txt
@@ -12,6 +12,15 @@ Required properties:
  - interrupts: interrupt number to the cpu.
  - clocks: from common clock binding: handle to usb clock.
  - clock-names: from common clock binding: Shall be "usbhost".
+ - port: if in the SoC there are EHCI phys, they should be listed here.
+   One phy per port. Each port should have its 'reg' entry.
+	- reg: port number on EHCI controller, e.g
+	       On Exynos5250, port 0 is USB2.0 otg phy
+			      port 1 is HSIC phy0
+			      port 2 is HSIC phy1
+	- phys: from the *Generic PHY* bindings; specifying phy used by port.
+	- phy-names: from the *Generic PHY* bindings; specifying name of phy
+		     used by the port.
 
 Optional properties:
  - samsung,vbus-gpio:  if present, specifies the GPIO that
@@ -27,6 +36,15 @@ Example:
 
 		clocks = <&clock 285>;
 		clock-names = "usbhost";
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+		port@0 {
+		    reg = <0>;
+		    phys = <&usb2phy 1>;
+		    phy-names = "host";
+		    status = "disabled";
+		};
 	};
 
 OHCI
diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
index 7f425ac..fe0509b 100644
--- a/drivers/usb/host/ehci-exynos.c
+++ b/drivers/usb/host/ehci-exynos.c
@@ -19,6 +19,7 @@
 #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/usb/phy.h>
 #include <linux/usb/samsung_usb_phy.h>
@@ -42,14 +43,78 @@
 static const char hcd_name[] = "ehci-exynos";
 static struct hc_driver __read_mostly exynos_ehci_hc_driver;
 
+#define PHY_NUMBER 3
 struct exynos_ehci_hcd {
 	struct clk *clk;
 	struct usb_phy *phy;
 	struct usb_otg *otg;
+	struct phy *phy_g[PHY_NUMBER];
 };
 
 #define to_exynos_ehci(hcd) (struct exynos_ehci_hcd *)(hcd_to_ehci(hcd)->priv)
 
+static int exynos_ehci_get_phy(struct platform_device *pdev,
+				struct exynos_ehci_hcd *exynos_ehci)
+{
+	struct device_node *child;
+	struct phy *phy;
+	int phy_number;
+	int ret = 0;
+
+	exynos_ehci->phy = devm_usb_get_phy(&pdev->dev, USB_PHY_TYPE_USB2);
+	if (IS_ERR(exynos_ehci->phy)) {
+		ret = PTR_ERR(exynos_ehci->phy);
+		/* This is the case when PHY config is disabled */
+		if (ret == -ENXIO || ret == -ENODEV) {
+			dev_dbg(&pdev->dev, "Failed to get usb2 phy\n");
+			exynos_ehci->phy = NULL;
+			ret = 0;
+		} else if (ret == -EPROBE_DEFER) {
+			goto fail_phy;
+		} else {
+			dev_err(&pdev->dev, "no usb2 phy configured\n");
+			goto fail_phy;
+		}
+	} else {
+		exynos_ehci->otg = exynos_ehci->phy->otg;
+	}
+
+	for_each_available_child_of_node(pdev->dev.of_node, child) {
+		ret = of_property_read_u32(child, "reg", &phy_number);
+		if (ret) {
+			dev_err(&pdev->dev, "Failed to parse device tree\n");
+			of_node_put(child);
+			goto fail_phy;
+		}
+		if (phy_number >= PHY_NUMBER) {
+			dev_err(&pdev->dev, "Invalid number of PHYs\n");
+			of_node_put(child);
+			ret = -EINVAL;
+			goto fail_phy;
+		}
+		phy = devm_of_phy_get(&pdev->dev, child, 0);
+		of_node_put(child);
+		if (IS_ERR(phy)) {
+			ret = PTR_ERR(phy);
+			/* This is the case when PHY config is disabled */
+			if (ret == -ENOSYS || ret == -ENODEV) {
+				dev_dbg(&pdev->dev, "Failed to get usb2 phy\n");
+				phy = NULL;
+				ret = 0;
+			} else if (ret == -EPROBE_DEFER) {
+				goto fail_phy;
+			} else {
+				dev_err(&pdev->dev, "no usb2 phy configured\n");
+				goto fail_phy;
+			}
+		}
+		exynos_ehci->phy_g[phy_number] = phy;
+	}
+
+fail_phy:
+	return ret;
+}
+
 static void exynos_setup_vbus_gpio(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -69,13 +134,40 @@ static void exynos_setup_vbus_gpio(struct platform_device *pdev)
 		dev_err(dev, "can't request ehci vbus gpio %d", gpio);
 }
 
+static int exynos_ehci_phyg_on(struct phy *phy[])
+{
+	int i;
+	int ret = 0;
+
+	for (i = 0; ret == 0 && i < PHY_NUMBER; i++)
+		if (phy[i])
+			ret = phy_power_on(phy[i]);
+	if (ret)
+		for (i--; i > 0; i--)
+			if (phy[i])
+				phy_power_off(phy[i]);
+
+	return ret;
+}
+
+static int exynos_ehci_phyg_off(struct phy *phy[])
+{
+	int i;
+	int ret = 0;
+
+	for (i = 0; ret == 0 && i < PHY_NUMBER; i++)
+		if (phy[i])
+			ret = phy_power_off(phy[i]);
+
+	return ret;
+}
+
 static int exynos_ehci_probe(struct platform_device *pdev)
 {
 	struct exynos_ehci_hcd *exynos_ehci;
 	struct usb_hcd *hcd;
 	struct ehci_hcd *ehci;
 	struct resource *res;
-	struct usb_phy *phy;
 	int irq;
 	int err;
 
@@ -102,15 +194,9 @@ static int exynos_ehci_probe(struct platform_device *pdev)
 					"samsung,exynos5440-ehci"))
 		goto skip_phy;
 
-	phy = devm_usb_get_phy(&pdev->dev, USB_PHY_TYPE_USB2);
-	if (IS_ERR(phy)) {
-		usb_put_hcd(hcd);
-		dev_warn(&pdev->dev, "no platform data or transceiver defined\n");
-		return -EPROBE_DEFER;
-	} else {
-		exynos_ehci->phy = phy;
-		exynos_ehci->otg = phy->otg;
-	}
+	err = exynos_ehci_get_phy(pdev, exynos_ehci);
+	if (err)
+		goto fail_clk;
 
 skip_phy:
 
@@ -155,6 +241,14 @@ skip_phy:
 	if (exynos_ehci->phy)
 		usb_phy_init(exynos_ehci->phy);
 
+	if (exynos_ehci->phy_g) {
+		err = exynos_ehci_phyg_on(exynos_ehci->phy_g);
+		if (err) {
+			dev_err(&pdev->dev, "Failed to enable phys\n");
+			goto fail_io;
+		}
+	}
+
 	ehci = hcd_to_ehci(hcd);
 	ehci->caps = hcd->regs;
 
@@ -175,6 +269,7 @@ skip_phy:
 fail_add_hcd:
 	if (exynos_ehci->phy)
 		usb_phy_shutdown(exynos_ehci->phy);
+	exynos_ehci_phyg_off(exynos_ehci->phy_g);
 fail_io:
 	clk_disable_unprepare(exynos_ehci->clk);
 fail_clk:
@@ -195,6 +290,8 @@ static int exynos_ehci_remove(struct platform_device *pdev)
 	if (exynos_ehci->phy)
 		usb_phy_shutdown(exynos_ehci->phy);
 
+	exynos_ehci_phyg_off(exynos_ehci->phy_g);
+
 	clk_disable_unprepare(exynos_ehci->clk);
 
 	usb_put_hcd(hcd);
@@ -221,6 +318,9 @@ static int exynos_ehci_suspend(struct device *dev)
 	if (exynos_ehci->phy)
 		usb_phy_shutdown(exynos_ehci->phy);
 
+	if (exynos_ehci->phy_g)
+		exynos_ehci_phyg_off(exynos_ehci->phy_g);
+
 	clk_disable_unprepare(exynos_ehci->clk);
 
 	return rc;
@@ -239,6 +339,9 @@ static int exynos_ehci_resume(struct device *dev)
 	if (exynos_ehci->phy)
 		usb_phy_init(exynos_ehci->phy);
 
+	if (exynos_ehci->phy_g)
+		exynos_ehci_phyg_on(exynos_ehci->phy_g);
+
 	/* DMA burst Enable */
 	writel(EHCI_INSNREG00_ENABLE_DMA_BURST, EHCI_INSNREG00(hcd->regs));
 
-- 
1.7.10.4

Re: [PATCH v2 1/2] usb: ohci-exynos: Add facility to use phy provided by the generic phy framework

[Alan Stern]

On Fri, 25 Apr 2014, Vivek Gautam wrote:

> Add support to consume phy provided by Generic phy framework.
> Keeping the support for older usb-phy intact right now, in order
> to prevent any functionality break in absence of relevant
> device tree side change for ohci-exynos.
> Once we move to new phy in the device nodes for ohci, we can
> remove the support for older phys.
> 
> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
> Cc: Jingoo Han <jg1.han@samsung.com>
> Cc: Alan Stern <stern@rowland.harvard.edu>


> +static int exynos_ohci_phyg_on(struct phy *phy[])
> +{
> +	int i;
> +	int ret = 0;
> +
> +	for (i = 0; ret == 0 && i < PHY_NUMBER; i++)
> +		if (phy[i])
> +			ret = phy_power_on(phy[i]);
> +	if (ret)
> +		for (i--; i >= 0; i--)
> +			if (phy[i])
> +				phy_power_off(phy[i]);
> +
> +	return ret;
> +}
> +
> +static int exynos_ohci_phyg_off(struct phy *phy[])
> +{
> +	int i;
> +	int ret = 0;
> +
> +	for (i = 0; ret == 0 && i < PHY_NUMBER; i++)
> +		if (phy[i])
> +			ret = phy_power_off(phy[i]);
> +
> +	return ret;
> +}

You probably shouldn't break out of this loop if ret is nonzero; you
should continue to power off the remaining phys.

I'd be inclined to put these two routines directly into 
exynos_ohci_phy_enable() and exynos_ohci_phy_disable(), since they 
aren't used anywhere else.

> @@ -151,6 +253,7 @@ skip_phy:
>  
>  fail_add_hcd:
>  	exynos_ohci_phy_disable(pdev);
> +	exynos_ohci_phyg_off(exynos_ohci->phy_g);

Why did you add this line?  It doesn't do anything useful, because 
exynos_ohci_phy_disable() already calls exynos_ohci_phyg_off().

Alan Stern

Re: [PATCH v2 1/2] usb: ohci-exynos: Add facility to use phy provided by the generic phy framework

[Vivek Gautam]

Hi,


On Fri, Apr 25, 2014 at 8:06 PM, Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org> wrote:
> On Fri, 25 Apr 2014, Vivek Gautam wrote:
>
>> Add support to consume phy provided by Generic phy framework.
>> Keeping the support for older usb-phy intact right now, in order
>> to prevent any functionality break in absence of relevant
>> device tree side change for ohci-exynos.
>> Once we move to new phy in the device nodes for ohci, we can
>> remove the support for older phys.
>>
>> Signed-off-by: Vivek Gautam <gautam.vivek-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> Cc: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> Cc: Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org>
>
>
>> +static int exynos_ohci_phyg_on(struct phy *phy[])
>> +{
>> +     int i;
>> +     int ret = 0;
>> +
>> +     for (i = 0; ret == 0 && i < PHY_NUMBER; i++)
>> +             if (phy[i])
>> +                     ret = phy_power_on(phy[i]);
>> +     if (ret)
>> +             for (i--; i >= 0; i--)
>> +                     if (phy[i])
>> +                             phy_power_off(phy[i]);
>> +
>> +     return ret;
>> +}
>> +
>> +static int exynos_ohci_phyg_off(struct phy *phy[])
>> +{
>> +     int i;
>> +     int ret = 0;
>> +
>> +     for (i = 0; ret == 0 && i < PHY_NUMBER; i++)
>> +             if (phy[i])
>> +                     ret = phy_power_off(phy[i]);
>> +
>> +     return ret;
>> +}
>
> You probably shouldn't break out of this loop if ret is nonzero; you
> should continue to power off the remaining phys.

ok, will remove the 'ret' check in for loop.

>
> I'd be inclined to put these two routines directly into
> exynos_ohci_phy_enable() and exynos_ohci_phy_disable(), since they
> aren't used anywhere else.

Sure, will make these routines as a part of exynos_ohci_phy_enable()
and exynos_ohci_phy_disable().

>
>> @@ -151,6 +253,7 @@ skip_phy:
>>
>>  fail_add_hcd:
>>       exynos_ohci_phy_disable(pdev);
>> +     exynos_ohci_phyg_off(exynos_ohci->phy_g);
>
> Why did you add this line?  It doesn't do anything useful, because
> exynos_ohci_phy_disable() already calls exynos_ohci_phyg_off().

Ah ! my bad, will remove this.


-- 
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html