[PATCH v3 0/2] usb: host: ehci/ohci-exynos: phy cleanup

[PATCH v3 0/2] usb: host: ehci/ohci-exynos: phy cleanup

[Vivek Gautam]

Cleaning up the phy getting sequence in ehci-exynos and ohci-exynos
drivers.

Hi Alan, Jingoo,

I have not imported the Acked-by and Reviewed-by from you guys, from V2 version
of this patch series, since this version is now rebased on the already available
commit in usb-next - "usb: ehci/ohci-exynos: Fix PHY getting sequence".

Please feel free to comment on it and add your Acked-by's and Reviewed-by's.

Changes since v2:
 - Rebased on top of usb-next branch, which now has
   'usb: ehci/ohci-exynos: Fix PHY getting sequence' patch.

Changes since v1:
 - This patch was part of the series "[PATCH 0/7] usb-phy: samsung: Cleanup the unused drivers"
   https://lkml.org/lkml/2014/8/14/235. I have dropped the phy cleanup patches from that series-
   Patches 1-5.
 - Rebased this patch on top of 3.17-rc1.

Vivek Gautam (2):
  usb: host: ehci-exynos: Remove unnecessary usb-phy support
  usb: host: ohci-exynos: Remove unnecessary usb-phy support

 drivers/usb/host/ehci-exynos.c |   81 +++++++++++-------------------------
 drivers/usb/host/ohci-exynos.c |   89 +++++++++++-----------------------------
 2 files changed, 47 insertions(+), 123 deletions(-)

-- 
1.7.10.4

[PATCH v3 1/2] usb: host: ehci-exynos: Remove unnecessary usb-phy support

[Vivek Gautam]

Now that we have completely moved from older USB-PHY drivers
to newer GENERIC-PHY drivers for PHYs available with USB controllers
on Exynos series of SoCs, we can remove the support for the same
in our host drivers too.

Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
---
 drivers/usb/host/ehci-exynos.c |   81 ++++++++++++----------------------------
 1 file changed, 23 insertions(+), 58 deletions(-)

diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
index 2eed9a4..99c5f5f 100644
--- a/drivers/usb/host/ehci-exynos.c
+++ b/drivers/usb/host/ehci-exynos.c
@@ -21,11 +21,8 @@
 #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>
 #include <linux/usb.h>
 #include <linux/usb/hcd.h>
-#include <linux/usb/otg.h>
 
 #include "ehci.h"
 
@@ -47,9 +44,7 @@ static struct hc_driver __read_mostly exynos_ehci_hc_driver;
 
 struct exynos_ehci_hcd {
 	struct clk *clk;
-	struct usb_phy *phy;
-	struct usb_otg *otg;
-	struct phy *phy_g[PHY_NUMBER];
+	struct phy *phy[PHY_NUMBER];
 };
 
 #define to_exynos_ehci(hcd) (struct exynos_ehci_hcd *)(hcd_to_ehci(hcd)->priv)
@@ -59,49 +54,39 @@ static int exynos_ehci_get_phy(struct device *dev,
 {
 	struct device_node *child;
 	struct phy *phy;
-	int phy_number;
-	int ret = 0;
+	int phy_num;
+	int ret;
 
 	for_each_available_child_of_node(dev->of_node, child) {
-		ret = of_property_read_u32(child, "reg", &phy_number);
+		ret = of_property_read_u32(child, "reg", &phy_num);
 		if (ret) {
 			dev_err(dev, "Failed to parse device tree\n");
 			of_node_put(child);
 			return ret;
 		}
 
-		if (phy_number >= PHY_NUMBER) {
+		if (phy_num >= PHY_NUMBER) {
 			dev_err(dev, "Invalid number of PHYs\n");
 			of_node_put(child);
 			return -EINVAL;
 		}
 
-		phy = devm_of_phy_get(dev, child, NULL);
+		exynos_ehci->phy[phy_num] = devm_of_phy_get(dev, child, NULL);
+		phy = exynos_ehci->phy[phy_num];
 		of_node_put(child);
-		if (IS_ERR(phy))
-			/* Lets fallback to older USB-PHYs */
-			goto usb_phy_old;
-		exynos_ehci->phy_g[phy_number] = phy;
-		/* Make the older PHYs unavailable */
-		exynos_ehci->phy = ERR_PTR(-ENXIO);
-	}
-
-	return 0;
-
-usb_phy_old:
-	exynos_ehci->phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
-	if (IS_ERR(exynos_ehci->phy)) {
-		ret = PTR_ERR(exynos_ehci->phy);
-		if (ret != -ENXIO && ret != -ENODEV) {
-			dev_err(dev, "no usb2 phy configured\n");
-			return ret;
+		if (IS_ERR(phy)) {
+			ret = PTR_ERR(phy);
+			if (ret == -EPROBE_DEFER) {
+				return ret;
+			} else if (ret != -ENOSYS && ret != -ENODEV) {
+				dev_err(dev,
+					"Error retrieving usb2 phy: %d\n", ret);
+				return PTR_ERR(phy);
+			}
 		}
-		dev_dbg(dev, "Failed to get usb2 phy\n");
-	} else {
-		exynos_ehci->otg = exynos_ehci->phy->otg;
 	}
 
-	return ret;
+	return 0;
 }
 
 static int exynos_ehci_phy_enable(struct device *dev)
@@ -111,16 +96,13 @@ static int exynos_ehci_phy_enable(struct device *dev)
 	int i;
 	int ret = 0;
 
-	if (!IS_ERR(exynos_ehci->phy))
-		return usb_phy_init(exynos_ehci->phy);
-
 	for (i = 0; ret == 0 && i < PHY_NUMBER; i++)
-		if (!IS_ERR(exynos_ehci->phy_g[i]))
-			ret = phy_power_on(exynos_ehci->phy_g[i]);
+		if (!IS_ERR(exynos_ehci->phy[i]))
+			ret = phy_power_on(exynos_ehci->phy[i]);
 	if (ret)
 		for (i--; i >= 0; i--)
-			if (!IS_ERR(exynos_ehci->phy_g[i]))
-				phy_power_off(exynos_ehci->phy_g[i]);
+			if (!IS_ERR(exynos_ehci->phy[i]))
+				phy_power_off(exynos_ehci->phy[i]);
 
 	return ret;
 }
@@ -131,14 +113,9 @@ static void exynos_ehci_phy_disable(struct device *dev)
 	struct exynos_ehci_hcd *exynos_ehci = to_exynos_ehci(hcd);
 	int i;
 
-	if (!IS_ERR(exynos_ehci->phy)) {
-		usb_phy_shutdown(exynos_ehci->phy);
-		return;
-	}
-
 	for (i = 0; i < PHY_NUMBER; i++)
-		if (!IS_ERR(exynos_ehci->phy_g[i]))
-			phy_power_off(exynos_ehci->phy_g[i]);
+		if (!IS_ERR(exynos_ehci->phy[i]))
+			phy_power_off(exynos_ehci->phy[i]);
 }
 
 static void exynos_setup_vbus_gpio(struct device *dev)
@@ -231,9 +208,6 @@ skip_phy:
 		goto fail_io;
 	}
 
-	if (exynos_ehci->otg)
-		exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
-
 	err = exynos_ehci_phy_enable(&pdev->dev);
 	if (err) {
 		dev_err(&pdev->dev, "Failed to enable USB phy\n");
@@ -273,9 +247,6 @@ static int exynos_ehci_remove(struct platform_device *pdev)
 
 	usb_remove_hcd(hcd);
 
-	if (exynos_ehci->otg)
-		exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
-
 	exynos_ehci_phy_disable(&pdev->dev);
 
 	clk_disable_unprepare(exynos_ehci->clk);
@@ -298,9 +269,6 @@ static int exynos_ehci_suspend(struct device *dev)
 	if (rc)
 		return rc;
 
-	if (exynos_ehci->otg)
-		exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
-
 	exynos_ehci_phy_disable(dev);
 
 	clk_disable_unprepare(exynos_ehci->clk);
@@ -316,9 +284,6 @@ static int exynos_ehci_resume(struct device *dev)
 
 	clk_prepare_enable(exynos_ehci->clk);
 
-	if (exynos_ehci->otg)
-		exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
-
 	ret = exynos_ehci_phy_enable(dev);
 	if (ret) {
 		dev_err(dev, "Failed to enable USB phy\n");
-- 
1.7.10.4

[PATCH v3 2/2] usb: host: ohci-exynos: Remove unnecessary usb-phy support

[Vivek Gautam]

Now that we have completely moved from older USB-PHY drivers
to newer GENERIC-PHY drivers for PHYs available with USB controllers
on Exynos series of SoCs, we can remove the support for the same
in our host drivers too.

Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
---
 drivers/usb/host/ohci-exynos.c |   89 +++++++++++-----------------------------
 1 file changed, 24 insertions(+), 65 deletions(-)

diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
index 7c48e3f..992b28d 100644
--- a/drivers/usb/host/ohci-exynos.c
+++ b/drivers/usb/host/ohci-exynos.c
@@ -19,11 +19,8 @@
 #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>
 #include <linux/usb/hcd.h>
-#include <linux/usb/otg.h>
 
 #include "ohci.h"
 
@@ -38,9 +35,7 @@ static struct hc_driver __read_mostly exynos_ohci_hc_driver;
 
 struct exynos_ohci_hcd {
 	struct clk *clk;
-	struct usb_phy *phy;
-	struct usb_otg *otg;
-	struct phy *phy_g[PHY_NUMBER];
+	struct phy *phy[PHY_NUMBER];
 };
 
 static int exynos_ohci_get_phy(struct device *dev,
@@ -48,56 +43,40 @@ static int exynos_ohci_get_phy(struct device *dev,
 {
 	struct device_node *child;
 	struct phy *phy;
-	int phy_number;
-	int ret = 0;
+	int phy_num;
+	int ret;
 
-	/*
-	 * Getting generic phy:
-	 * We are keeping both types of phys as a part of transiting OHCI
-	 * to generic phy framework, so as to maintain backward compatibilty
-	 * with old DTB too.
-	 * We fallback to older USB-PHYs when we fail to get generic PHYs.
-	 */
+	/* Get the generic phys */
 	for_each_available_child_of_node(dev->of_node, child) {
-		ret = of_property_read_u32(child, "reg", &phy_number);
+		ret = of_property_read_u32(child, "reg", &phy_num);
 		if (ret) {
 			dev_err(dev, "Failed to parse device tree\n");
 			of_node_put(child);
 			return ret;
 		}
 
-		if (phy_number >= PHY_NUMBER) {
+		if (phy_num >= PHY_NUMBER) {
 			dev_err(dev, "Invalid number of PHYs\n");
 			of_node_put(child);
 			return -EINVAL;
 		}
 
-		phy = devm_of_phy_get(dev, child, NULL);
+		exynos_ohci->phy[phy_num] = devm_of_phy_get(dev, child, NULL);
+		phy = exynos_ohci->phy[phy_num];
 		of_node_put(child);
-		if (IS_ERR(phy))
-			/* Lets fallback to older USB-PHYs */
-			goto usb_phy_old;
-		exynos_ohci->phy_g[phy_number] = phy;
-		/* Make the older PHYs unavailable */
-		exynos_ohci->phy = ERR_PTR(-ENXIO);
-	}
-
-	return 0;
-
-usb_phy_old:
-	exynos_ohci->phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
-	if (IS_ERR(exynos_ohci->phy)) {
-		ret = PTR_ERR(exynos_ohci->phy);
-		if (ret != -ENXIO && ret != -ENODEV) {
-			dev_err(dev, "no usb2 phy configured\n");
-			return ret;
+		if (IS_ERR(phy)) {
+			ret = PTR_ERR(phy);
+			if (ret == -EPROBE_DEFER) {
+				return ret;
+			} else if (ret != -ENOSYS && ret != -ENODEV) {
+				dev_err(dev,
+					"Error retrieving usb2 phy: %d\n", ret);
+				return PTR_ERR(phy);
+			}
 		}
-		dev_dbg(dev, "Failed to get usb2 phy\n");
-	} else {
-		exynos_ohci->otg = exynos_ohci->phy->otg;
 	}
 
-	return ret;
+	return 0;
 }
 
 static int exynos_ohci_phy_enable(struct device *dev)
@@ -107,16 +86,13 @@ static int exynos_ohci_phy_enable(struct device *dev)
 	int i;
 	int ret = 0;
 
-	if (!IS_ERR(exynos_ohci->phy))
-		return usb_phy_init(exynos_ohci->phy);
-
 	for (i = 0; ret == 0 && i < PHY_NUMBER; i++)
-		if (!IS_ERR(exynos_ohci->phy_g[i]))
-			ret = phy_power_on(exynos_ohci->phy_g[i]);
+		if (!IS_ERR(exynos_ohci->phy[i]))
+			ret = phy_power_on(exynos_ohci->phy[i]);
 	if (ret)
 		for (i--; i >= 0; i--)
-			if (!IS_ERR(exynos_ohci->phy_g[i]))
-				phy_power_off(exynos_ohci->phy_g[i]);
+			if (!IS_ERR(exynos_ohci->phy[i]))
+				phy_power_off(exynos_ohci->phy[i]);
 
 	return ret;
 }
@@ -127,14 +103,9 @@ static void exynos_ohci_phy_disable(struct device *dev)
 	struct exynos_ohci_hcd *exynos_ohci = to_exynos_ohci(hcd);
 	int i;
 
-	if (!IS_ERR(exynos_ohci->phy)) {
-		usb_phy_shutdown(exynos_ohci->phy);
-		return;
-	}
-
 	for (i = 0; i < PHY_NUMBER; i++)
-		if (!IS_ERR(exynos_ohci->phy_g[i]))
-			phy_power_off(exynos_ohci->phy_g[i]);
+		if (!IS_ERR(exynos_ohci->phy[i]))
+			phy_power_off(exynos_ohci->phy[i]);
 }
 
 static int exynos_ohci_probe(struct platform_device *pdev)
@@ -206,9 +177,6 @@ skip_phy:
 		goto fail_io;
 	}
 
-	if (exynos_ohci->otg)
-		exynos_ohci->otg->set_host(exynos_ohci->otg, &hcd->self);
-
 	platform_set_drvdata(pdev, hcd);
 
 	err = exynos_ohci_phy_enable(&pdev->dev);
@@ -241,9 +209,6 @@ static int exynos_ohci_remove(struct platform_device *pdev)
 
 	usb_remove_hcd(hcd);
 
-	if (exynos_ohci->otg)
-		exynos_ohci->otg->set_host(exynos_ohci->otg, &hcd->self);
-
 	exynos_ohci_phy_disable(&pdev->dev);
 
 	clk_disable_unprepare(exynos_ohci->clk);
@@ -272,9 +237,6 @@ static int exynos_ohci_suspend(struct device *dev)
 	if (rc)
 		return rc;
 
-	if (exynos_ohci->otg)
-		exynos_ohci->otg->set_host(exynos_ohci->otg, &hcd->self);
-
 	exynos_ohci_phy_disable(dev);
 
 	clk_disable_unprepare(exynos_ohci->clk);
@@ -290,9 +252,6 @@ static int exynos_ohci_resume(struct device *dev)
 
 	clk_prepare_enable(exynos_ohci->clk);
 
-	if (exynos_ohci->otg)
-		exynos_ohci->otg->set_host(exynos_ohci->otg, &hcd->self);
-
 	ret = exynos_ohci_phy_enable(dev);
 	if (ret) {
 		dev_err(dev, "Failed to enable USB phy\n");
-- 
1.7.10.4

Re: [PATCH v3 1/2] usb: host: ehci-exynos: Remove unnecessary usb-phy support

[Jingoo Han]

On Wednesday, September 17, 2014 8:18 PM, Vivek Gautam wrote:
> 
> Now that we have completely moved from older USB-PHY drivers
> to newer GENERIC-PHY drivers for PHYs available with USB controllers
> on Exynos series of SoCs, we can remove the support for the same
> in our host drivers too.
> 
> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>

Acked-by: Jingoo Han <jg1.han@samsung.com>

Best regards,
Jingoo Han

> ---
>  drivers/usb/host/ehci-exynos.c |   81 ++++++++++++----------------------------
>  1 file changed, 23 insertions(+), 58 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
> index 2eed9a4..99c5f5f 100644
> --- a/drivers/usb/host/ehci-exynos.c
> +++ b/drivers/usb/host/ehci-exynos.c
> @@ -21,11 +21,8 @@
>  #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>
>  #include <linux/usb.h>
>  #include <linux/usb/hcd.h>
> -#include <linux/usb/otg.h>
> 
>  #include "ehci.h"
> 
> @@ -47,9 +44,7 @@ static struct hc_driver __read_mostly exynos_ehci_hc_driver;
> 
>  struct exynos_ehci_hcd {
>  	struct clk *clk;
> -	struct usb_phy *phy;
> -	struct usb_otg *otg;
> -	struct phy *phy_g[PHY_NUMBER];
> +	struct phy *phy[PHY_NUMBER];
>  };
> 
>  #define to_exynos_ehci(hcd) (struct exynos_ehci_hcd *)(hcd_to_ehci(hcd)->priv)
> @@ -59,49 +54,39 @@ static int exynos_ehci_get_phy(struct device *dev,
>  {
>  	struct device_node *child;
>  	struct phy *phy;
> -	int phy_number;
> -	int ret = 0;
> +	int phy_num;
> +	int ret;
> 
>  	for_each_available_child_of_node(dev->of_node, child) {
> -		ret = of_property_read_u32(child, "reg", &phy_number);
> +		ret = of_property_read_u32(child, "reg", &phy_num);
>  		if (ret) {
>  			dev_err(dev, "Failed to parse device tree\n");
>  			of_node_put(child);
>  			return ret;
>  		}
> 
> -		if (phy_number >= PHY_NUMBER) {
> +		if (phy_num >= PHY_NUMBER) {
>  			dev_err(dev, "Invalid number of PHYs\n");
>  			of_node_put(child);
>  			return -EINVAL;
>  		}
> 
> -		phy = devm_of_phy_get(dev, child, NULL);
> +		exynos_ehci->phy[phy_num] = devm_of_phy_get(dev, child, NULL);
> +		phy = exynos_ehci->phy[phy_num];
>  		of_node_put(child);
> -		if (IS_ERR(phy))
> -			/* Lets fallback to older USB-PHYs */
> -			goto usb_phy_old;
> -		exynos_ehci->phy_g[phy_number] = phy;
> -		/* Make the older PHYs unavailable */
> -		exynos_ehci->phy = ERR_PTR(-ENXIO);
> -	}
> -
> -	return 0;
> -
> -usb_phy_old:
> -	exynos_ehci->phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
> -	if (IS_ERR(exynos_ehci->phy)) {
> -		ret = PTR_ERR(exynos_ehci->phy);
> -		if (ret != -ENXIO && ret != -ENODEV) {
> -			dev_err(dev, "no usb2 phy configured\n");
> -			return ret;
> +		if (IS_ERR(phy)) {
> +			ret = PTR_ERR(phy);
> +			if (ret == -EPROBE_DEFER) {
> +				return ret;
> +			} else if (ret != -ENOSYS && ret != -ENODEV) {
> +				dev_err(dev,
> +					"Error retrieving usb2 phy: %d\n", ret);
> +				return PTR_ERR(phy);
> +			}
>  		}
> -		dev_dbg(dev, "Failed to get usb2 phy\n");
> -	} else {
> -		exynos_ehci->otg = exynos_ehci->phy->otg;
>  	}
> 
> -	return ret;
> +	return 0;
>  }
> 
>  static int exynos_ehci_phy_enable(struct device *dev)
> @@ -111,16 +96,13 @@ static int exynos_ehci_phy_enable(struct device *dev)
>  	int i;
>  	int ret = 0;
> 
> -	if (!IS_ERR(exynos_ehci->phy))
> -		return usb_phy_init(exynos_ehci->phy);
> -
>  	for (i = 0; ret == 0 && i < PHY_NUMBER; i++)
> -		if (!IS_ERR(exynos_ehci->phy_g[i]))
> -			ret = phy_power_on(exynos_ehci->phy_g[i]);
> +		if (!IS_ERR(exynos_ehci->phy[i]))
> +			ret = phy_power_on(exynos_ehci->phy[i]);
>  	if (ret)
>  		for (i--; i >= 0; i--)
> -			if (!IS_ERR(exynos_ehci->phy_g[i]))
> -				phy_power_off(exynos_ehci->phy_g[i]);
> +			if (!IS_ERR(exynos_ehci->phy[i]))
> +				phy_power_off(exynos_ehci->phy[i]);
> 
>  	return ret;
>  }
> @@ -131,14 +113,9 @@ static void exynos_ehci_phy_disable(struct device *dev)
>  	struct exynos_ehci_hcd *exynos_ehci = to_exynos_ehci(hcd);
>  	int i;
> 
> -	if (!IS_ERR(exynos_ehci->phy)) {
> -		usb_phy_shutdown(exynos_ehci->phy);
> -		return;
> -	}
> -
>  	for (i = 0; i < PHY_NUMBER; i++)
> -		if (!IS_ERR(exynos_ehci->phy_g[i]))
> -			phy_power_off(exynos_ehci->phy_g[i]);
> +		if (!IS_ERR(exynos_ehci->phy[i]))
> +			phy_power_off(exynos_ehci->phy[i]);
>  }
> 
>  static void exynos_setup_vbus_gpio(struct device *dev)
> @@ -231,9 +208,6 @@ skip_phy:
>  		goto fail_io;
>  	}
> 
> -	if (exynos_ehci->otg)
> -		exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
> -
>  	err = exynos_ehci_phy_enable(&pdev->dev);
>  	if (err) {
>  		dev_err(&pdev->dev, "Failed to enable USB phy\n");
> @@ -273,9 +247,6 @@ static int exynos_ehci_remove(struct platform_device *pdev)
> 
>  	usb_remove_hcd(hcd);
> 
> -	if (exynos_ehci->otg)
> -		exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
> -
>  	exynos_ehci_phy_disable(&pdev->dev);
> 
>  	clk_disable_unprepare(exynos_ehci->clk);
> @@ -298,9 +269,6 @@ static int exynos_ehci_suspend(struct device *dev)
>  	if (rc)
>  		return rc;
> 
> -	if (exynos_ehci->otg)
> -		exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
> -
>  	exynos_ehci_phy_disable(dev);
> 
>  	clk_disable_unprepare(exynos_ehci->clk);
> @@ -316,9 +284,6 @@ static int exynos_ehci_resume(struct device *dev)
> 
>  	clk_prepare_enable(exynos_ehci->clk);
> 
> -	if (exynos_ehci->otg)
> -		exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
> -
>  	ret = exynos_ehci_phy_enable(dev);
>  	if (ret) {
>  		dev_err(dev, "Failed to enable USB phy\n");
> --
> 1.7.10.4

Re: [PATCH v3 2/2] usb: host: ohci-exynos: Remove unnecessary usb-phy support

[Jingoo Han]

On Wednesday, September 17, 2014 8:18 PM, Vivek Gautam wrote:
> 
> Now that we have completely moved from older USB-PHY drivers
> to newer GENERIC-PHY drivers for PHYs available with USB controllers
> on Exynos series of SoCs, we can remove the support for the same
> in our host drivers too.
> 
> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>

Acked-by: Jingoo Han <jg1.han@samsung.com>

Best regards,
Jingoo Han

> ---
>  drivers/usb/host/ohci-exynos.c |   89 +++++++++++-----------------------------
>  1 file changed, 24 insertions(+), 65 deletions(-)
> 
> diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
> index 7c48e3f..992b28d 100644
> --- a/drivers/usb/host/ohci-exynos.c
> +++ b/drivers/usb/host/ohci-exynos.c
> @@ -19,11 +19,8 @@
>  #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>
>  #include <linux/usb/hcd.h>
> -#include <linux/usb/otg.h>
> 
>  #include "ohci.h"
> 
> @@ -38,9 +35,7 @@ static struct hc_driver __read_mostly exynos_ohci_hc_driver;
> 
>  struct exynos_ohci_hcd {
>  	struct clk *clk;
> -	struct usb_phy *phy;
> -	struct usb_otg *otg;
> -	struct phy *phy_g[PHY_NUMBER];
> +	struct phy *phy[PHY_NUMBER];
>  };
> 
>  static int exynos_ohci_get_phy(struct device *dev,
> @@ -48,56 +43,40 @@ static int exynos_ohci_get_phy(struct device *dev,
>  {
>  	struct device_node *child;
>  	struct phy *phy;
> -	int phy_number;
> -	int ret = 0;
> +	int phy_num;
> +	int ret;
> 
> -	/*
> -	 * Getting generic phy:
> -	 * We are keeping both types of phys as a part of transiting OHCI
> -	 * to generic phy framework, so as to maintain backward compatibilty
> -	 * with old DTB too.
> -	 * We fallback to older USB-PHYs when we fail to get generic PHYs.
> -	 */
> +	/* Get the generic phys */
>  	for_each_available_child_of_node(dev->of_node, child) {
> -		ret = of_property_read_u32(child, "reg", &phy_number);
> +		ret = of_property_read_u32(child, "reg", &phy_num);
>  		if (ret) {
>  			dev_err(dev, "Failed to parse device tree\n");
>  			of_node_put(child);
>  			return ret;
>  		}
> 
> -		if (phy_number >= PHY_NUMBER) {
> +		if (phy_num >= PHY_NUMBER) {
>  			dev_err(dev, "Invalid number of PHYs\n");
>  			of_node_put(child);
>  			return -EINVAL;
>  		}
> 
> -		phy = devm_of_phy_get(dev, child, NULL);
> +		exynos_ohci->phy[phy_num] = devm_of_phy_get(dev, child, NULL);
> +		phy = exynos_ohci->phy[phy_num];
>  		of_node_put(child);
> -		if (IS_ERR(phy))
> -			/* Lets fallback to older USB-PHYs */
> -			goto usb_phy_old;
> -		exynos_ohci->phy_g[phy_number] = phy;
> -		/* Make the older PHYs unavailable */
> -		exynos_ohci->phy = ERR_PTR(-ENXIO);
> -	}
> -
> -	return 0;
> -
> -usb_phy_old:
> -	exynos_ohci->phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
> -	if (IS_ERR(exynos_ohci->phy)) {
> -		ret = PTR_ERR(exynos_ohci->phy);
> -		if (ret != -ENXIO && ret != -ENODEV) {
> -			dev_err(dev, "no usb2 phy configured\n");
> -			return ret;
> +		if (IS_ERR(phy)) {
> +			ret = PTR_ERR(phy);
> +			if (ret == -EPROBE_DEFER) {
> +				return ret;
> +			} else if (ret != -ENOSYS && ret != -ENODEV) {
> +				dev_err(dev,
> +					"Error retrieving usb2 phy: %d\n", ret);
> +				return PTR_ERR(phy);
> +			}
>  		}
> -		dev_dbg(dev, "Failed to get usb2 phy\n");
> -	} else {
> -		exynos_ohci->otg = exynos_ohci->phy->otg;
>  	}
> 
> -	return ret;
> +	return 0;
>  }
> 
>  static int exynos_ohci_phy_enable(struct device *dev)
> @@ -107,16 +86,13 @@ static int exynos_ohci_phy_enable(struct device *dev)
>  	int i;
>  	int ret = 0;
> 
> -	if (!IS_ERR(exynos_ohci->phy))
> -		return usb_phy_init(exynos_ohci->phy);
> -
>  	for (i = 0; ret == 0 && i < PHY_NUMBER; i++)
> -		if (!IS_ERR(exynos_ohci->phy_g[i]))
> -			ret = phy_power_on(exynos_ohci->phy_g[i]);
> +		if (!IS_ERR(exynos_ohci->phy[i]))
> +			ret = phy_power_on(exynos_ohci->phy[i]);
>  	if (ret)
>  		for (i--; i >= 0; i--)
> -			if (!IS_ERR(exynos_ohci->phy_g[i]))
> -				phy_power_off(exynos_ohci->phy_g[i]);
> +			if (!IS_ERR(exynos_ohci->phy[i]))
> +				phy_power_off(exynos_ohci->phy[i]);
> 
>  	return ret;
>  }
> @@ -127,14 +103,9 @@ static void exynos_ohci_phy_disable(struct device *dev)
>  	struct exynos_ohci_hcd *exynos_ohci = to_exynos_ohci(hcd);
>  	int i;
> 
> -	if (!IS_ERR(exynos_ohci->phy)) {
> -		usb_phy_shutdown(exynos_ohci->phy);
> -		return;
> -	}
> -
>  	for (i = 0; i < PHY_NUMBER; i++)
> -		if (!IS_ERR(exynos_ohci->phy_g[i]))
> -			phy_power_off(exynos_ohci->phy_g[i]);
> +		if (!IS_ERR(exynos_ohci->phy[i]))
> +			phy_power_off(exynos_ohci->phy[i]);
>  }
> 
>  static int exynos_ohci_probe(struct platform_device *pdev)
> @@ -206,9 +177,6 @@ skip_phy:
>  		goto fail_io;
>  	}
> 
> -	if (exynos_ohci->otg)
> -		exynos_ohci->otg->set_host(exynos_ohci->otg, &hcd->self);
> -
>  	platform_set_drvdata(pdev, hcd);
> 
>  	err = exynos_ohci_phy_enable(&pdev->dev);
> @@ -241,9 +209,6 @@ static int exynos_ohci_remove(struct platform_device *pdev)
> 
>  	usb_remove_hcd(hcd);
> 
> -	if (exynos_ohci->otg)
> -		exynos_ohci->otg->set_host(exynos_ohci->otg, &hcd->self);
> -
>  	exynos_ohci_phy_disable(&pdev->dev);
> 
>  	clk_disable_unprepare(exynos_ohci->clk);
> @@ -272,9 +237,6 @@ static int exynos_ohci_suspend(struct device *dev)
>  	if (rc)
>  		return rc;
> 
> -	if (exynos_ohci->otg)
> -		exynos_ohci->otg->set_host(exynos_ohci->otg, &hcd->self);
> -
>  	exynos_ohci_phy_disable(dev);
> 
>  	clk_disable_unprepare(exynos_ohci->clk);
> @@ -290,9 +252,6 @@ static int exynos_ohci_resume(struct device *dev)
> 
>  	clk_prepare_enable(exynos_ohci->clk);
> 
> -	if (exynos_ohci->otg)
> -		exynos_ohci->otg->set_host(exynos_ohci->otg, &hcd->self);
> -
>  	ret = exynos_ohci_phy_enable(dev);
>  	if (ret) {
>  		dev_err(dev, "Failed to enable USB phy\n");
> --
> 1.7.10.4

Re: [PATCH v3 1/2] usb: host: ehci-exynos: Remove unnecessary usb-phy support

[Alan Stern]

On Wed, 17 Sep 2014, Vivek Gautam wrote:

> Now that we have completely moved from older USB-PHY drivers
> to newer GENERIC-PHY drivers for PHYs available with USB controllers
> on Exynos series of SoCs, we can remove the support for the same
> in our host drivers too.
> 
> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>

I don't see why you made your changes in this awkward way.  For 
instance...

> @@ -59,49 +54,39 @@ static int exynos_ehci_get_phy(struct device *dev,
>  {
>  	struct device_node *child;
>  	struct phy *phy;
> -	int phy_number;
> -	int ret = 0;
> +	int phy_num;

Why rename this variable?  Wasn't the original name good enough?

> +	int ret;
>  
>  	for_each_available_child_of_node(dev->of_node, child) {
> -		ret = of_property_read_u32(child, "reg", &phy_number);
> +		ret = of_property_read_u32(child, "reg", &phy_num);
>  		if (ret) {
>  			dev_err(dev, "Failed to parse device tree\n");
>  			of_node_put(child);
>  			return ret;
>  		}
>  
> -		if (phy_number >= PHY_NUMBER) {
> +		if (phy_num >= PHY_NUMBER) {
>  			dev_err(dev, "Invalid number of PHYs\n");
>  			of_node_put(child);
>  			return -EINVAL;
>  		}
>  
> -		phy = devm_of_phy_get(dev, child, NULL);
> +		exynos_ehci->phy[phy_num] = devm_of_phy_get(dev, child, NULL);
> +		phy = exynos_ehci->phy[phy_num];

Why make two changes, resulting in more code, when you could have made 
just one change?

		phy = devm_of_phy_get(dev, child, NULL);
+		exynos_ehci->phy[phy_num] = phy;

Also, the patch description should mention that you are adding support 
for EPROBE_DEFER.

Alan Stern

Re: [PATCH v3 1/2] usb: host: ehci-exynos: Remove unnecessary usb-phy support

[Vivek Gautam]

Hi Alan,


On Wed, Sep 17, 2014 at 8:27 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Wed, 17 Sep 2014, Vivek Gautam wrote:
>
>> Now that we have completely moved from older USB-PHY drivers
>> to newer GENERIC-PHY drivers for PHYs available with USB controllers
>> on Exynos series of SoCs, we can remove the support for the same
>> in our host drivers too.
>>
>> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
>
> I don't see why you made your changes in this awkward way.  For
> instance...
>
>> @@ -59,49 +54,39 @@ static int exynos_ehci_get_phy(struct device *dev,
>>  {
>>       struct device_node *child;
>>       struct phy *phy;
>> -     int phy_number;
>> -     int ret = 0;
>> +     int phy_num;
>
> Why rename this variable?  Wasn't the original name good enough?

fair enough, don't need to rename the variable.

>
>> +     int ret;
>>
>>       for_each_available_child_of_node(dev->of_node, child) {
>> -             ret = of_property_read_u32(child, "reg", &phy_number);
>> +             ret = of_property_read_u32(child, "reg", &phy_num);
>>               if (ret) {
>>                       dev_err(dev, "Failed to parse device tree\n");
>>                       of_node_put(child);
>>                       return ret;
>>               }
>>
>> -             if (phy_number >= PHY_NUMBER) {
>> +             if (phy_num >= PHY_NUMBER) {
>>                       dev_err(dev, "Invalid number of PHYs\n");
>>                       of_node_put(child);
>>                       return -EINVAL;
>>               }
>>
>> -             phy = devm_of_phy_get(dev, child, NULL);
>> +             exynos_ehci->phy[phy_num] = devm_of_phy_get(dev, child, NULL);
>> +             phy = exynos_ehci->phy[phy_num];
>
> Why make two changes, resulting in more code, when you could have made
> just one change?
>
>                 phy = devm_of_phy_get(dev, child, NULL);
> +               exynos_ehci->phy[phy_num] = phy;

Right. i don't know what state of mind i was in while making these changes.
i should have kept the changes to minimal.

>
> Also, the patch description should mention that you are adding support
> for EPROBE_DEFER.

Sure, will add that description.




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

[PATCH v4 1/2] usb: host: ehci-exynos: Remove unnecessary usb-phy support

[Vivek Gautam]

Now that we have completely moved from older USB-PHY drivers
to newer GENERIC-PHY drivers for PHYs available with USB controllers
on Exynos series of SoCs, we can remove the support for the same
in our host drivers too.

We also defer the probe for our host in case we end up getting
EPROBE_DEFER error when getting PHYs.

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

Changes since v3:
 - Addressed review comments by Alan:
   -- Skipped renaming 'phy_number' variable,
   -- resorted to just adding minimal change required for phy assignment.
   -- updated patch description to mention EPROBE_DEFER support.

 drivers/usb/host/ehci-exynos.c |   74 +++++++++++-----------------------------
 1 file changed, 20 insertions(+), 54 deletions(-)

diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
index 2eed9a4..f293453 100644
--- a/drivers/usb/host/ehci-exynos.c
+++ b/drivers/usb/host/ehci-exynos.c
@@ -21,11 +21,8 @@
 #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>
 #include <linux/usb.h>
 #include <linux/usb/hcd.h>
-#include <linux/usb/otg.h>
 
 #include "ehci.h"
 
@@ -47,9 +44,7 @@ static struct hc_driver __read_mostly exynos_ehci_hc_driver;
 
 struct exynos_ehci_hcd {
 	struct clk *clk;
-	struct usb_phy *phy;
-	struct usb_otg *otg;
-	struct phy *phy_g[PHY_NUMBER];
+	struct phy *phy[PHY_NUMBER];
 };
 
 #define to_exynos_ehci(hcd) (struct exynos_ehci_hcd *)(hcd_to_ehci(hcd)->priv)
@@ -60,8 +55,9 @@ static int exynos_ehci_get_phy(struct device *dev,
 	struct device_node *child;
 	struct phy *phy;
 	int phy_number;
-	int ret = 0;
+	int ret;
 
+	/* Get PHYs for the controller */
 	for_each_available_child_of_node(dev->of_node, child) {
 		ret = of_property_read_u32(child, "reg", &phy_number);
 		if (ret) {
@@ -77,31 +73,21 @@ static int exynos_ehci_get_phy(struct device *dev,
 		}
 
 		phy = devm_of_phy_get(dev, child, NULL);
+		exynos_ehci->phy[phy_number] = phy;
 		of_node_put(child);
-		if (IS_ERR(phy))
-			/* Lets fallback to older USB-PHYs */
-			goto usb_phy_old;
-		exynos_ehci->phy_g[phy_number] = phy;
-		/* Make the older PHYs unavailable */
-		exynos_ehci->phy = ERR_PTR(-ENXIO);
-	}
-
-	return 0;
-
-usb_phy_old:
-	exynos_ehci->phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
-	if (IS_ERR(exynos_ehci->phy)) {
-		ret = PTR_ERR(exynos_ehci->phy);
-		if (ret != -ENXIO && ret != -ENODEV) {
-			dev_err(dev, "no usb2 phy configured\n");
-			return ret;
+		if (IS_ERR(phy)) {
+			ret = PTR_ERR(phy);
+			if (ret == -EPROBE_DEFER) {
+				return ret;
+			} else if (ret != -ENOSYS && ret != -ENODEV) {
+				dev_err(dev,
+					"Error retrieving usb2 phy: %d\n", ret);
+				return PTR_ERR(phy);
+			}
 		}
-		dev_dbg(dev, "Failed to get usb2 phy\n");
-	} else {
-		exynos_ehci->otg = exynos_ehci->phy->otg;
 	}
 
-	return ret;
+	return 0;
 }
 
 static int exynos_ehci_phy_enable(struct device *dev)
@@ -111,16 +97,13 @@ static int exynos_ehci_phy_enable(struct device *dev)
 	int i;
 	int ret = 0;
 
-	if (!IS_ERR(exynos_ehci->phy))
-		return usb_phy_init(exynos_ehci->phy);
-
 	for (i = 0; ret == 0 && i < PHY_NUMBER; i++)
-		if (!IS_ERR(exynos_ehci->phy_g[i]))
-			ret = phy_power_on(exynos_ehci->phy_g[i]);
+		if (!IS_ERR(exynos_ehci->phy[i]))
+			ret = phy_power_on(exynos_ehci->phy[i]);
 	if (ret)
 		for (i--; i >= 0; i--)
-			if (!IS_ERR(exynos_ehci->phy_g[i]))
-				phy_power_off(exynos_ehci->phy_g[i]);
+			if (!IS_ERR(exynos_ehci->phy[i]))
+				phy_power_off(exynos_ehci->phy[i]);
 
 	return ret;
 }
@@ -131,14 +114,9 @@ static void exynos_ehci_phy_disable(struct device *dev)
 	struct exynos_ehci_hcd *exynos_ehci = to_exynos_ehci(hcd);
 	int i;
 
-	if (!IS_ERR(exynos_ehci->phy)) {
-		usb_phy_shutdown(exynos_ehci->phy);
-		return;
-	}
-
 	for (i = 0; i < PHY_NUMBER; i++)
-		if (!IS_ERR(exynos_ehci->phy_g[i]))
-			phy_power_off(exynos_ehci->phy_g[i]);
+		if (!IS_ERR(exynos_ehci->phy[i]))
+			phy_power_off(exynos_ehci->phy[i]);
 }
 
 static void exynos_setup_vbus_gpio(struct device *dev)
@@ -231,9 +209,6 @@ skip_phy:
 		goto fail_io;
 	}
 
-	if (exynos_ehci->otg)
-		exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
-
 	err = exynos_ehci_phy_enable(&pdev->dev);
 	if (err) {
 		dev_err(&pdev->dev, "Failed to enable USB phy\n");
@@ -273,9 +248,6 @@ static int exynos_ehci_remove(struct platform_device *pdev)
 
 	usb_remove_hcd(hcd);
 
-	if (exynos_ehci->otg)
-		exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
-
 	exynos_ehci_phy_disable(&pdev->dev);
 
 	clk_disable_unprepare(exynos_ehci->clk);
@@ -298,9 +270,6 @@ static int exynos_ehci_suspend(struct device *dev)
 	if (rc)
 		return rc;
 
-	if (exynos_ehci->otg)
-		exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
-
 	exynos_ehci_phy_disable(dev);
 
 	clk_disable_unprepare(exynos_ehci->clk);
@@ -316,9 +285,6 @@ static int exynos_ehci_resume(struct device *dev)
 
 	clk_prepare_enable(exynos_ehci->clk);
 
-	if (exynos_ehci->otg)
-		exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
-
 	ret = exynos_ehci_phy_enable(dev);
 	if (ret) {
 		dev_err(dev, "Failed to enable USB phy\n");
-- 
1.7.10.4

Re: [PATCH v4 1/2] usb: host: ehci-exynos: Remove unnecessary usb-phy support

[Alan Stern]

On Thu, 18 Sep 2014, Vivek Gautam wrote:

> Now that we have completely moved from older USB-PHY drivers
> to newer GENERIC-PHY drivers for PHYs available with USB controllers
> on Exynos series of SoCs, we can remove the support for the same
> in our host drivers too.
> 
> We also defer the probe for our host in case we end up getting
> EPROBE_DEFER error when getting PHYs.

Better now.  But I didn't notice this the first time:

> +		if (IS_ERR(phy)) {
> +			ret = PTR_ERR(phy);
> +			if (ret == -EPROBE_DEFER) {
> +				return ret;
> +			} else if (ret != -ENOSYS && ret != -ENODEV) {
> +				dev_err(dev,
> +					"Error retrieving usb2 phy: %d\n", ret);
> +				return PTR_ERR(phy);

It doesn't make any real difference, but wouldn't you prefer to say
"return ret" here?

With or without that change, for both these two patches:

Acked-by: Alan Stern <stern@rowland.harvard.edu>

Re: [PATCH v4 1/2] usb: host: ehci-exynos: Remove unnecessary usb-phy support

[Vivek Gautam]

On Thu, Sep 18, 2014 at 8:20 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Thu, 18 Sep 2014, Vivek Gautam wrote:
>
>> Now that we have completely moved from older USB-PHY drivers
>> to newer GENERIC-PHY drivers for PHYs available with USB controllers
>> on Exynos series of SoCs, we can remove the support for the same
>> in our host drivers too.
>>
>> We also defer the probe for our host in case we end up getting
>> EPROBE_DEFER error when getting PHYs.
>
> Better now.  But I didn't notice this the first time:
>
>> +             if (IS_ERR(phy)) {
>> +                     ret = PTR_ERR(phy);
>> +                     if (ret == -EPROBE_DEFER) {
>> +                             return ret;
>> +                     } else if (ret != -ENOSYS && ret != -ENODEV) {
>> +                             dev_err(dev,
>> +                                     "Error retrieving usb2 phy: %d\n", ret);
>> +                             return PTR_ERR(phy);
>
> It doesn't make any real difference, but wouldn't you prefer to say
> "return ret" here?

sure, will update this.

>
> With or without that change, for both these two patches:
>
> Acked-by: Alan Stern <stern@rowland.harvard.edu>
>
> --
> 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



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

[PATCH v5 1/2] usb: host: ehci-exynos: Remove unnecessary usb-phy support

[Vivek Gautam]

Now that we have completely moved from older USB-PHY drivers
to newer GENERIC-PHY drivers for PHYs available with USB controllers
on Exynos series of SoCs, we can remove the support for the same
in our host drivers too.

We also defer the probe for our host in case we end up getting
EPROBE_DEFER error when getting PHYs.

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

Changes since v4:
 - returning 'ret' instead of PTR_ERR(phy), since ret is nothing but that only.

Changes since v3:
 - Addressed review comments by Alan:
   -- Skipped renaming 'phy_number' variable,
   -- resorted to just adding minimal change required for phy assignment.
   -- updated patch description to mention EPROBE_DEFER support.

 drivers/usb/host/ehci-exynos.c |   74 +++++++++++-----------------------------
 1 file changed, 20 insertions(+), 54 deletions(-)

diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
index 2eed9a4..7189f2e 100644
--- a/drivers/usb/host/ehci-exynos.c
+++ b/drivers/usb/host/ehci-exynos.c
@@ -21,11 +21,8 @@
 #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>
 #include <linux/usb.h>
 #include <linux/usb/hcd.h>
-#include <linux/usb/otg.h>
 
 #include "ehci.h"
 
@@ -47,9 +44,7 @@ static struct hc_driver __read_mostly exynos_ehci_hc_driver;
 
 struct exynos_ehci_hcd {
 	struct clk *clk;
-	struct usb_phy *phy;
-	struct usb_otg *otg;
-	struct phy *phy_g[PHY_NUMBER];
+	struct phy *phy[PHY_NUMBER];
 };
 
 #define to_exynos_ehci(hcd) (struct exynos_ehci_hcd *)(hcd_to_ehci(hcd)->priv)
@@ -60,8 +55,9 @@ static int exynos_ehci_get_phy(struct device *dev,
 	struct device_node *child;
 	struct phy *phy;
 	int phy_number;
-	int ret = 0;
+	int ret;
 
+	/* Get PHYs for the controller */
 	for_each_available_child_of_node(dev->of_node, child) {
 		ret = of_property_read_u32(child, "reg", &phy_number);
 		if (ret) {
@@ -77,31 +73,21 @@ static int exynos_ehci_get_phy(struct device *dev,
 		}
 
 		phy = devm_of_phy_get(dev, child, NULL);
+		exynos_ehci->phy[phy_number] = phy;
 		of_node_put(child);
-		if (IS_ERR(phy))
-			/* Lets fallback to older USB-PHYs */
-			goto usb_phy_old;
-		exynos_ehci->phy_g[phy_number] = phy;
-		/* Make the older PHYs unavailable */
-		exynos_ehci->phy = ERR_PTR(-ENXIO);
-	}
-
-	return 0;
-
-usb_phy_old:
-	exynos_ehci->phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
-	if (IS_ERR(exynos_ehci->phy)) {
-		ret = PTR_ERR(exynos_ehci->phy);
-		if (ret != -ENXIO && ret != -ENODEV) {
-			dev_err(dev, "no usb2 phy configured\n");
-			return ret;
+		if (IS_ERR(phy)) {
+			ret = PTR_ERR(phy);
+			if (ret == -EPROBE_DEFER) {
+				return ret;
+			} else if (ret != -ENOSYS && ret != -ENODEV) {
+				dev_err(dev,
+					"Error retrieving usb2 phy: %d\n", ret);
+				return ret;
+			}
 		}
-		dev_dbg(dev, "Failed to get usb2 phy\n");
-	} else {
-		exynos_ehci->otg = exynos_ehci->phy->otg;
 	}
 
-	return ret;
+	return 0;
 }
 
 static int exynos_ehci_phy_enable(struct device *dev)
@@ -111,16 +97,13 @@ static int exynos_ehci_phy_enable(struct device *dev)
 	int i;
 	int ret = 0;
 
-	if (!IS_ERR(exynos_ehci->phy))
-		return usb_phy_init(exynos_ehci->phy);
-
 	for (i = 0; ret == 0 && i < PHY_NUMBER; i++)
-		if (!IS_ERR(exynos_ehci->phy_g[i]))
-			ret = phy_power_on(exynos_ehci->phy_g[i]);
+		if (!IS_ERR(exynos_ehci->phy[i]))
+			ret = phy_power_on(exynos_ehci->phy[i]);
 	if (ret)
 		for (i--; i >= 0; i--)
-			if (!IS_ERR(exynos_ehci->phy_g[i]))
-				phy_power_off(exynos_ehci->phy_g[i]);
+			if (!IS_ERR(exynos_ehci->phy[i]))
+				phy_power_off(exynos_ehci->phy[i]);
 
 	return ret;
 }
@@ -131,14 +114,9 @@ static void exynos_ehci_phy_disable(struct device *dev)
 	struct exynos_ehci_hcd *exynos_ehci = to_exynos_ehci(hcd);
 	int i;
 
-	if (!IS_ERR(exynos_ehci->phy)) {
-		usb_phy_shutdown(exynos_ehci->phy);
-		return;
-	}
-
 	for (i = 0; i < PHY_NUMBER; i++)
-		if (!IS_ERR(exynos_ehci->phy_g[i]))
-			phy_power_off(exynos_ehci->phy_g[i]);
+		if (!IS_ERR(exynos_ehci->phy[i]))
+			phy_power_off(exynos_ehci->phy[i]);
 }
 
 static void exynos_setup_vbus_gpio(struct device *dev)
@@ -231,9 +209,6 @@ skip_phy:
 		goto fail_io;
 	}
 
-	if (exynos_ehci->otg)
-		exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
-
 	err = exynos_ehci_phy_enable(&pdev->dev);
 	if (err) {
 		dev_err(&pdev->dev, "Failed to enable USB phy\n");
@@ -273,9 +248,6 @@ static int exynos_ehci_remove(struct platform_device *pdev)
 
 	usb_remove_hcd(hcd);
 
-	if (exynos_ehci->otg)
-		exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
-
 	exynos_ehci_phy_disable(&pdev->dev);
 
 	clk_disable_unprepare(exynos_ehci->clk);
@@ -298,9 +270,6 @@ static int exynos_ehci_suspend(struct device *dev)
 	if (rc)
 		return rc;
 
-	if (exynos_ehci->otg)
-		exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
-
 	exynos_ehci_phy_disable(dev);
 
 	clk_disable_unprepare(exynos_ehci->clk);
@@ -316,9 +285,6 @@ static int exynos_ehci_resume(struct device *dev)
 
 	clk_prepare_enable(exynos_ehci->clk);
 
-	if (exynos_ehci->otg)
-		exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
-
 	ret = exynos_ehci_phy_enable(dev);
 	if (ret) {
 		dev_err(dev, "Failed to enable USB phy\n");
-- 
1.7.10.4

Re: [PATCH v5 1/2] usb: host: ehci-exynos: Remove unnecessary usb-phy support

[Vivek Gautam]

Hi Greg,


On Mon, Sep 22, 2014 at 11:15 AM, Vivek Gautam <gautam.vivek@samsung.com> wrote:
> Now that we have completely moved from older USB-PHY drivers
> to newer GENERIC-PHY drivers for PHYs available with USB controllers
> on Exynos series of SoCs, we can remove the support for the same
> in our host drivers too.
>
> We also defer the probe for our host in case we end up getting
> EPROBE_DEFER error when getting PHYs.
>
> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
> Acked-by: Jingoo Han <jg1.han@samsung.com>
> Acked-by: Alan Stern <stern@rowland.harvard.edu>
> ---

Did this one got missed for usb-next ?
I can only see "usb: host: ohci-exynos: Remove unnecessary usb-phy support"
in the next branch.

Ignore me if you have already taken care of this, and plan to queue it up.

>
> Changes since v4:
>  - returning 'ret' instead of PTR_ERR(phy), since ret is nothing but that only.
>
> Changes since v3:
>  - Addressed review comments by Alan:
>    -- Skipped renaming 'phy_number' variable,
>    -- resorted to just adding minimal change required for phy assignment.
>    -- updated patch description to mention EPROBE_DEFER support.
>
>  drivers/usb/host/ehci-exynos.c |   74 +++++++++++-----------------------------
>  1 file changed, 20 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
> index 2eed9a4..7189f2e 100644
> --- a/drivers/usb/host/ehci-exynos.c
> +++ b/drivers/usb/host/ehci-exynos.c
> @@ -21,11 +21,8 @@
>  #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>
>  #include <linux/usb.h>
>  #include <linux/usb/hcd.h>
> -#include <linux/usb/otg.h>
>
>  #include "ehci.h"
>
> @@ -47,9 +44,7 @@ static struct hc_driver __read_mostly exynos_ehci_hc_driver;
>
>  struct exynos_ehci_hcd {
>         struct clk *clk;
> -       struct usb_phy *phy;
> -       struct usb_otg *otg;
> -       struct phy *phy_g[PHY_NUMBER];
> +       struct phy *phy[PHY_NUMBER];
>  };
>
>  #define to_exynos_ehci(hcd) (struct exynos_ehci_hcd *)(hcd_to_ehci(hcd)->priv)
> @@ -60,8 +55,9 @@ static int exynos_ehci_get_phy(struct device *dev,
>         struct device_node *child;
>         struct phy *phy;
>         int phy_number;
> -       int ret = 0;
> +       int ret;
>
> +       /* Get PHYs for the controller */
>         for_each_available_child_of_node(dev->of_node, child) {
>                 ret = of_property_read_u32(child, "reg", &phy_number);
>                 if (ret) {
> @@ -77,31 +73,21 @@ static int exynos_ehci_get_phy(struct device *dev,
>                 }
>
>                 phy = devm_of_phy_get(dev, child, NULL);
> +               exynos_ehci->phy[phy_number] = phy;
>                 of_node_put(child);
> -               if (IS_ERR(phy))
> -                       /* Lets fallback to older USB-PHYs */
> -                       goto usb_phy_old;
> -               exynos_ehci->phy_g[phy_number] = phy;
> -               /* Make the older PHYs unavailable */
> -               exynos_ehci->phy = ERR_PTR(-ENXIO);
> -       }
> -
> -       return 0;
> -
> -usb_phy_old:
> -       exynos_ehci->phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
> -       if (IS_ERR(exynos_ehci->phy)) {
> -               ret = PTR_ERR(exynos_ehci->phy);
> -               if (ret != -ENXIO && ret != -ENODEV) {
> -                       dev_err(dev, "no usb2 phy configured\n");
> -                       return ret;
> +               if (IS_ERR(phy)) {
> +                       ret = PTR_ERR(phy);
> +                       if (ret == -EPROBE_DEFER) {
> +                               return ret;
> +                       } else if (ret != -ENOSYS && ret != -ENODEV) {
> +                               dev_err(dev,
> +                                       "Error retrieving usb2 phy: %d\n", ret);
> +                               return ret;
> +                       }
>                 }
> -               dev_dbg(dev, "Failed to get usb2 phy\n");
> -       } else {
> -               exynos_ehci->otg = exynos_ehci->phy->otg;
>         }
>
> -       return ret;
> +       return 0;
>  }
>
>  static int exynos_ehci_phy_enable(struct device *dev)
> @@ -111,16 +97,13 @@ static int exynos_ehci_phy_enable(struct device *dev)
>         int i;
>         int ret = 0;
>
> -       if (!IS_ERR(exynos_ehci->phy))
> -               return usb_phy_init(exynos_ehci->phy);
> -
>         for (i = 0; ret == 0 && i < PHY_NUMBER; i++)
> -               if (!IS_ERR(exynos_ehci->phy_g[i]))
> -                       ret = phy_power_on(exynos_ehci->phy_g[i]);
> +               if (!IS_ERR(exynos_ehci->phy[i]))
> +                       ret = phy_power_on(exynos_ehci->phy[i]);
>         if (ret)
>                 for (i--; i >= 0; i--)
> -                       if (!IS_ERR(exynos_ehci->phy_g[i]))
> -                               phy_power_off(exynos_ehci->phy_g[i]);
> +                       if (!IS_ERR(exynos_ehci->phy[i]))
> +                               phy_power_off(exynos_ehci->phy[i]);
>
>         return ret;
>  }
> @@ -131,14 +114,9 @@ static void exynos_ehci_phy_disable(struct device *dev)
>         struct exynos_ehci_hcd *exynos_ehci = to_exynos_ehci(hcd);
>         int i;
>
> -       if (!IS_ERR(exynos_ehci->phy)) {
> -               usb_phy_shutdown(exynos_ehci->phy);
> -               return;
> -       }
> -
>         for (i = 0; i < PHY_NUMBER; i++)
> -               if (!IS_ERR(exynos_ehci->phy_g[i]))
> -                       phy_power_off(exynos_ehci->phy_g[i]);
> +               if (!IS_ERR(exynos_ehci->phy[i]))
> +                       phy_power_off(exynos_ehci->phy[i]);
>  }
>
>  static void exynos_setup_vbus_gpio(struct device *dev)
> @@ -231,9 +209,6 @@ skip_phy:
>                 goto fail_io;
>         }
>
> -       if (exynos_ehci->otg)
> -               exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
> -
>         err = exynos_ehci_phy_enable(&pdev->dev);
>         if (err) {
>                 dev_err(&pdev->dev, "Failed to enable USB phy\n");
> @@ -273,9 +248,6 @@ static int exynos_ehci_remove(struct platform_device *pdev)
>
>         usb_remove_hcd(hcd);
>
> -       if (exynos_ehci->otg)
> -               exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
> -
>         exynos_ehci_phy_disable(&pdev->dev);
>
>         clk_disable_unprepare(exynos_ehci->clk);
> @@ -298,9 +270,6 @@ static int exynos_ehci_suspend(struct device *dev)
>         if (rc)
>                 return rc;
>
> -       if (exynos_ehci->otg)
> -               exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
> -
>         exynos_ehci_phy_disable(dev);
>
>         clk_disable_unprepare(exynos_ehci->clk);
> @@ -316,9 +285,6 @@ static int exynos_ehci_resume(struct device *dev)
>
>         clk_prepare_enable(exynos_ehci->clk);
>
> -       if (exynos_ehci->otg)
> -               exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
> -
>         ret = exynos_ehci_phy_enable(dev);
>         if (ret) {
>                 dev_err(dev, "Failed to enable USB phy\n");
> --
> 1.7.10.4
>



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

Re: [PATCH v5 1/2] usb: host: ehci-exynos: Remove unnecessary usb-phy support

[Greg KH]

On Thu, Sep 25, 2014 at 10:50:22AM +0530, Vivek Gautam wrote:
> Hi Greg,
> 
> 
> On Mon, Sep 22, 2014 at 11:15 AM, Vivek Gautam <gautam.vivek@samsung.com> wrote:
> > Now that we have completely moved from older USB-PHY drivers
> > to newer GENERIC-PHY drivers for PHYs available with USB controllers
> > on Exynos series of SoCs, we can remove the support for the same
> > in our host drivers too.
> >
> > We also defer the probe for our host in case we end up getting
> > EPROBE_DEFER error when getting PHYs.
> >
> > Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
> > Acked-by: Jingoo Han <jg1.han@samsung.com>
> > Acked-by: Alan Stern <stern@rowland.harvard.edu>
> > ---
> 
> Did this one got missed for usb-next ?
> I can only see "usb: host: ohci-exynos: Remove unnecessary usb-phy support"
> in the next branch.
> 
> Ignore me if you have already taken care of this, and plan to queue it up.

If it's not in my tree already, please resend as I don't have this in my
queue anymore.

greg k-h

Re: [PATCH v5 1/2] usb: host: ehci-exynos: Remove unnecessary usb-phy support

[Vivek Gautam]

On Mon, Sep 29, 2014 at 7:21 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Thu, Sep 25, 2014 at 10:50:22AM +0530, Vivek Gautam wrote:
>> Hi Greg,
>>
>>
>> On Mon, Sep 22, 2014 at 11:15 AM, Vivek Gautam <gautam.vivek@samsung.com> wrote:
>> > Now that we have completely moved from older USB-PHY drivers
>> > to newer GENERIC-PHY drivers for PHYs available with USB controllers
>> > on Exynos series of SoCs, we can remove the support for the same
>> > in our host drivers too.
>> >
>> > We also defer the probe for our host in case we end up getting
>> > EPROBE_DEFER error when getting PHYs.
>> >
>> > Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
>> > Acked-by: Jingoo Han <jg1.han@samsung.com>
>> > Acked-by: Alan Stern <stern@rowland.harvard.edu>
>> > ---
>>
>> Did this one got missed for usb-next ?
>> I can only see "usb: host: ohci-exynos: Remove unnecessary usb-phy support"
>> in the next branch.
>>
>> Ignore me if you have already taken care of this, and plan to queue it up.
>
> If it's not in my tree already, please resend as I don't have this in my
> queue anymore.

yea, i don't see it in usb-next.
I will resend it then.

>
> greg k-h
> --
> 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



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

[PATCH v5 RESEND 1/2] usb: host: ehci-exynos: Remove unnecessary usb-phy support

[Vivek Gautam]

Now that we have completely moved from older USB-PHY drivers
to newer GENERIC-PHY drivers for PHYs available with USB controllers
on Exynos series of SoCs, we can remove the support for the same
in our host drivers too.

We also defer the probe for our host in case we end up getting
EPROBE_DEFER error when getting PHYs.

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

Changes since v4:
 - returning 'ret' instead of PTR_ERR(phy), since ret is nothing but that only.

Changes since v3:
 - Addressed review comments by Alan:
   -- Skipped renaming 'phy_number' variable,
   -- resorted to just adding minimal change required for phy assignment.
   -- updated patch description to mention EPROBE_DEFER support.

 drivers/usb/host/ehci-exynos.c |   74 +++++++++++-----------------------------
 1 file changed, 20 insertions(+), 54 deletions(-)

diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
index 2eed9a4..7189f2e 100644
--- a/drivers/usb/host/ehci-exynos.c
+++ b/drivers/usb/host/ehci-exynos.c
@@ -21,11 +21,8 @@
 #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>
 #include <linux/usb.h>
 #include <linux/usb/hcd.h>
-#include <linux/usb/otg.h>
 
 #include "ehci.h"
 
@@ -47,9 +44,7 @@ static struct hc_driver __read_mostly exynos_ehci_hc_driver;
 
 struct exynos_ehci_hcd {
 	struct clk *clk;
-	struct usb_phy *phy;
-	struct usb_otg *otg;
-	struct phy *phy_g[PHY_NUMBER];
+	struct phy *phy[PHY_NUMBER];
 };
 
 #define to_exynos_ehci(hcd) (struct exynos_ehci_hcd *)(hcd_to_ehci(hcd)->priv)
@@ -60,8 +55,9 @@ static int exynos_ehci_get_phy(struct device *dev,
 	struct device_node *child;
 	struct phy *phy;
 	int phy_number;
-	int ret = 0;
+	int ret;
 
+	/* Get PHYs for the controller */
 	for_each_available_child_of_node(dev->of_node, child) {
 		ret = of_property_read_u32(child, "reg", &phy_number);
 		if (ret) {
@@ -77,31 +73,21 @@ static int exynos_ehci_get_phy(struct device *dev,
 		}
 
 		phy = devm_of_phy_get(dev, child, NULL);
+		exynos_ehci->phy[phy_number] = phy;
 		of_node_put(child);
-		if (IS_ERR(phy))
-			/* Lets fallback to older USB-PHYs */
-			goto usb_phy_old;
-		exynos_ehci->phy_g[phy_number] = phy;
-		/* Make the older PHYs unavailable */
-		exynos_ehci->phy = ERR_PTR(-ENXIO);
-	}
-
-	return 0;
-
-usb_phy_old:
-	exynos_ehci->phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
-	if (IS_ERR(exynos_ehci->phy)) {
-		ret = PTR_ERR(exynos_ehci->phy);
-		if (ret != -ENXIO && ret != -ENODEV) {
-			dev_err(dev, "no usb2 phy configured\n");
-			return ret;
+		if (IS_ERR(phy)) {
+			ret = PTR_ERR(phy);
+			if (ret == -EPROBE_DEFER) {
+				return ret;
+			} else if (ret != -ENOSYS && ret != -ENODEV) {
+				dev_err(dev,
+					"Error retrieving usb2 phy: %d\n", ret);
+				return ret;
+			}
 		}
-		dev_dbg(dev, "Failed to get usb2 phy\n");
-	} else {
-		exynos_ehci->otg = exynos_ehci->phy->otg;
 	}
 
-	return ret;
+	return 0;
 }
 
 static int exynos_ehci_phy_enable(struct device *dev)
@@ -111,16 +97,13 @@ static int exynos_ehci_phy_enable(struct device *dev)
 	int i;
 	int ret = 0;
 
-	if (!IS_ERR(exynos_ehci->phy))
-		return usb_phy_init(exynos_ehci->phy);
-
 	for (i = 0; ret == 0 && i < PHY_NUMBER; i++)
-		if (!IS_ERR(exynos_ehci->phy_g[i]))
-			ret = phy_power_on(exynos_ehci->phy_g[i]);
+		if (!IS_ERR(exynos_ehci->phy[i]))
+			ret = phy_power_on(exynos_ehci->phy[i]);
 	if (ret)
 		for (i--; i >= 0; i--)
-			if (!IS_ERR(exynos_ehci->phy_g[i]))
-				phy_power_off(exynos_ehci->phy_g[i]);
+			if (!IS_ERR(exynos_ehci->phy[i]))
+				phy_power_off(exynos_ehci->phy[i]);
 
 	return ret;
 }
@@ -131,14 +114,9 @@ static void exynos_ehci_phy_disable(struct device *dev)
 	struct exynos_ehci_hcd *exynos_ehci = to_exynos_ehci(hcd);
 	int i;
 
-	if (!IS_ERR(exynos_ehci->phy)) {
-		usb_phy_shutdown(exynos_ehci->phy);
-		return;
-	}
-
 	for (i = 0; i < PHY_NUMBER; i++)
-		if (!IS_ERR(exynos_ehci->phy_g[i]))
-			phy_power_off(exynos_ehci->phy_g[i]);
+		if (!IS_ERR(exynos_ehci->phy[i]))
+			phy_power_off(exynos_ehci->phy[i]);
 }
 
 static void exynos_setup_vbus_gpio(struct device *dev)
@@ -231,9 +209,6 @@ skip_phy:
 		goto fail_io;
 	}
 
-	if (exynos_ehci->otg)
-		exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
-
 	err = exynos_ehci_phy_enable(&pdev->dev);
 	if (err) {
 		dev_err(&pdev->dev, "Failed to enable USB phy\n");
@@ -273,9 +248,6 @@ static int exynos_ehci_remove(struct platform_device *pdev)
 
 	usb_remove_hcd(hcd);
 
-	if (exynos_ehci->otg)
-		exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
-
 	exynos_ehci_phy_disable(&pdev->dev);
 
 	clk_disable_unprepare(exynos_ehci->clk);
@@ -298,9 +270,6 @@ static int exynos_ehci_suspend(struct device *dev)
 	if (rc)
 		return rc;
 
-	if (exynos_ehci->otg)
-		exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
-
 	exynos_ehci_phy_disable(dev);
 
 	clk_disable_unprepare(exynos_ehci->clk);
@@ -316,9 +285,6 @@ static int exynos_ehci_resume(struct device *dev)
 
 	clk_prepare_enable(exynos_ehci->clk);
 
-	if (exynos_ehci->otg)
-		exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
-
 	ret = exynos_ehci_phy_enable(dev);
 	if (ret) {
 		dev_err(dev, "Failed to enable USB phy\n");
-- 
1.7.10.4