[PATCH 0/5] usb: dwc3/phy-exynos5-usbdrd: Extend support to Exynos7

[PATCH 0/5] usb: dwc3/phy-exynos5-usbdrd: Extend support to Exynos7

[Vivek Gautam]

Adding required support for clocks and additional VBUS regulators
to enable USB 3.0 support on Exynos7 SoC.

This series depends for ACRH_EXYNOS7 support on following series:
[PATCH 00/14] Support 64bit Cortex A57 based Exynos7 SoC
https://www.mail-archive.com/devicetree@vger.kernel.org/msg39392.html

The series is based on usb-next branch.

Vivek Gautam (5):
  usb: dwc3: exynos: Add support for SCLK present on Exynos7
  phy: exynos5-usbdrd: Add pipe-clk and utmi-clk support
  phy: exynos5-usbdrd: Add facility for VBUS-BOOST-5V supply
  usb: dwc3: Adding Kconfig dependency for Exynos7
  phy: exynos5-usbdrd: Adding Kconfig dependency for Exynos7

 .../devicetree/bindings/phy/samsung-phy.txt        |    4 ++
 drivers/phy/Kconfig                                |    2 +-
 drivers/phy/phy-exynos5-usbdrd.c                   |   51 +++++++++++++++++++-
 drivers/usb/dwc3/Kconfig                           |    2 +-
 drivers/usb/dwc3/dwc3-exynos.c                     |   16 ++++++
 5 files changed, 71 insertions(+), 4 deletions(-)

-- 
1.7.10.4

[PATCH 1/5] usb: dwc3: exynos: Add support for SCLK present on Exynos7

[Vivek Gautam]

Exynos7 also has a separate special gate clock going to the IP
apart from the usual AHB clock. So add support for the same.

Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
---
 drivers/usb/dwc3/dwc3-exynos.c |   16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
index f9fb8ad..bab6395 100644
--- a/drivers/usb/dwc3/dwc3-exynos.c
+++ b/drivers/usb/dwc3/dwc3-exynos.c
@@ -35,6 +35,7 @@ struct dwc3_exynos {
 	struct device		*dev;
 
 	struct clk		*clk;
+	struct clk		*sclk;
 	struct regulator	*vdd33;
 	struct regulator	*vdd10;
 };
@@ -141,10 +142,17 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
+	/* Exynos7 has a special gate clock going to this IP */
+	exynos->sclk = devm_clk_get(dev, "usbdrd30_sclk");
+	if (IS_ERR(exynos->sclk))
+		dev_warn(dev, "couldn't get sclk\n");
+
 	exynos->dev	= dev;
 	exynos->clk	= clk;
 
 	clk_prepare_enable(exynos->clk);
+	if (!IS_ERR(exynos->sclk))
+		clk_prepare_enable(exynos->sclk);
 
 	exynos->vdd33 = devm_regulator_get(dev, "vdd33");
 	if (IS_ERR(exynos->vdd33)) {
@@ -187,6 +195,8 @@ err4:
 err3:
 	regulator_disable(exynos->vdd33);
 err2:
+	if (!IS_ERR(exynos->sclk))
+		clk_disable_unprepare(exynos->sclk);
 	clk_disable_unprepare(clk);
 	return ret;
 }
@@ -199,6 +209,8 @@ static int dwc3_exynos_remove(struct platform_device *pdev)
 	platform_device_unregister(exynos->usb2_phy);
 	platform_device_unregister(exynos->usb3_phy);
 
+	if (!IS_ERR(exynos->sclk))
+		clk_disable_unprepare(exynos->sclk);
 	clk_disable_unprepare(exynos->clk);
 
 	regulator_disable(exynos->vdd33);
@@ -220,6 +232,8 @@ static int dwc3_exynos_suspend(struct device *dev)
 {
 	struct dwc3_exynos *exynos = dev_get_drvdata(dev);
 
+	if (!IS_ERR(exynos->sclk))
+		clk_disable(exynos->sclk);
 	clk_disable(exynos->clk);
 
 	regulator_disable(exynos->vdd33);
@@ -245,6 +259,8 @@ static int dwc3_exynos_resume(struct device *dev)
 	}
 
 	clk_enable(exynos->clk);
+	if (!IS_ERR(exynos->sclk))
+		clk_enable(exynos->sclk);
 
 	/* runtime set active to reflect active state. */
 	pm_runtime_disable(dev);
-- 
1.7.10.4

[PATCH 2/5] phy: exynos5-usbdrd: Add pipe-clk and utmi-clk support

[Vivek Gautam]

Exynos7 SoC has now separate gate control for 125MHz pipe3 phy
clock, as well as 60MHz utmi phy clock.
So get the same and control in the phy-exynos5-usbdrd driver.

Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
---
 .../devicetree/bindings/phy/samsung-phy.txt        |    4 ++++
 drivers/phy/phy-exynos5-usbdrd.c                   |   24 ++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt
index 7a6feea..b64d616 100644
--- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
+++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
@@ -135,6 +135,10 @@ Required properties:
 	       PHY operations, associated by phy name. It is used to
 	       determine bit values for clock settings register.
 	       For Exynos5420 this is given as 'sclk_usbphy30' in CMU.
+	- optional clocks: Next gen Exynos SoCs have following additional
+			   gate clocks available:
+			   - phy_pipe: for PIPE3 phy
+			   - phy_utmi: for UTMI+ phy
 - samsung,pmu-syscon: phandle for PMU system controller interface, used to
 		      control pmu registers for power isolation.
 - #phy-cells : from the generic PHY bindings, must be 1;
diff --git a/drivers/phy/phy-exynos5-usbdrd.c b/drivers/phy/phy-exynos5-usbdrd.c
index b05302b..685c108 100644
--- a/drivers/phy/phy-exynos5-usbdrd.c
+++ b/drivers/phy/phy-exynos5-usbdrd.c
@@ -148,6 +148,8 @@ struct exynos5_usbdrd_phy_drvdata {
  * @dev: pointer to device instance of this platform device
  * @reg_phy: usb phy controller register memory base
  * @clk: phy clock for register access
+ * @pipeclk: clock for pipe3 phy
+ * @utmiclk: clock for utmi+ phy
  * @drv_data: pointer to SoC level driver data structure
  * @phys[]: array for 'EXYNOS5_DRDPHYS_NUM' number of PHY
  *	    instances each with its 'phy' and 'phy_cfg'.
@@ -161,6 +163,8 @@ struct exynos5_usbdrd_phy {
 	struct device *dev;
 	void __iomem *reg_phy;
 	struct clk *clk;
+	struct clk *pipeclk;
+	struct clk *utmiclk;
 	const struct exynos5_usbdrd_phy_drvdata *drv_data;
 	struct phy_usb_instance {
 		struct phy *phy;
@@ -446,6 +450,10 @@ static int exynos5_usbdrd_phy_power_on(struct phy *phy)
 
 	dev_dbg(phy_drd->dev, "Request to power_on usbdrd_phy phy\n");
 
+	if (!IS_ERR(phy_drd->utmiclk))
+		clk_prepare_enable(phy_drd->utmiclk);
+	if (!IS_ERR(phy_drd->pipeclk))
+		clk_prepare_enable(phy_drd->pipeclk);
 	clk_prepare_enable(phy_drd->ref_clk);
 
 	/* Enable VBUS supply */
@@ -464,6 +472,10 @@ static int exynos5_usbdrd_phy_power_on(struct phy *phy)
 
 fail_vbus:
 	clk_disable_unprepare(phy_drd->ref_clk);
+	if (!IS_ERR(phy_drd->pipeclk))
+		clk_disable_unprepare(phy_drd->pipeclk);
+	if (!IS_ERR(phy_drd->utmiclk))
+		clk_disable_unprepare(phy_drd->utmiclk);
 
 	return ret;
 }
@@ -483,6 +495,10 @@ static int exynos5_usbdrd_phy_power_off(struct phy *phy)
 		regulator_disable(phy_drd->vbus);
 
 	clk_disable_unprepare(phy_drd->ref_clk);
+	if (!IS_ERR(phy_drd->pipeclk))
+		clk_disable_unprepare(phy_drd->pipeclk);
+	if (!IS_ERR(phy_drd->utmiclk))
+		clk_disable_unprepare(phy_drd->utmiclk);
 
 	return 0;
 }
@@ -581,6 +597,14 @@ static int exynos5_usbdrd_phy_probe(struct platform_device *pdev)
 		return PTR_ERR(phy_drd->clk);
 	}
 
+	phy_drd->pipeclk = devm_clk_get(dev, "phy_pipe");
+	if (IS_ERR(phy_drd->pipeclk))
+		dev_warn(dev, "Failed to get pipe3 phy operational clock\n");
+
+	phy_drd->utmiclk = devm_clk_get(dev, "phy_utmi");
+	if (IS_ERR(phy_drd->utmiclk))
+		dev_warn(dev, "Failed to get utmi phy operational clock\n");
+
 	phy_drd->ref_clk = devm_clk_get(dev, "ref");
 	if (IS_ERR(phy_drd->ref_clk)) {
 		dev_err(dev, "Failed to get reference clock of usbdrd phy\n");
-- 
1.7.10.4

[PATCH 3/5] phy: exynos5-usbdrd: Add facility for VBUS-BOOST-5V supply

[Vivek Gautam]

Some Exynos SoCs have a separate regulator controlling a
Boost 5V supply which goes as input for VBUS regulator.
So adding a control for the same in driver, to enable
vbus supply on the port.

Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
---
 drivers/phy/phy-exynos5-usbdrd.c |   27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/phy-exynos5-usbdrd.c b/drivers/phy/phy-exynos5-usbdrd.c
index 685c108..a6d0cb7 100644
--- a/drivers/phy/phy-exynos5-usbdrd.c
+++ b/drivers/phy/phy-exynos5-usbdrd.c
@@ -176,6 +176,7 @@ struct exynos5_usbdrd_phy {
 	u32 extrefclk;
 	struct clk *ref_clk;
 	struct regulator *vbus;
+	struct regulator *vbus_boost;
 };
 
 static inline
@@ -457,11 +458,19 @@ static int exynos5_usbdrd_phy_power_on(struct phy *phy)
 	clk_prepare_enable(phy_drd->ref_clk);
 
 	/* Enable VBUS supply */
+	if (phy_drd->vbus_boost) {
+		ret = regulator_enable(phy_drd->vbus_boost);
+		if (ret) {
+			dev_err(phy_drd->dev,
+				"Failed to enable VBUS boost supply\n");
+			goto fail_vbus;
+		}
+	}
 	if (phy_drd->vbus) {
 		ret = regulator_enable(phy_drd->vbus);
 		if (ret) {
 			dev_err(phy_drd->dev, "Failed to enable VBUS supply\n");
-			goto fail_vbus;
+			goto fail_vbus_boost;
 		}
 	}
 
@@ -470,6 +479,9 @@ static int exynos5_usbdrd_phy_power_on(struct phy *phy)
 
 	return 0;
 
+fail_vbus_boost:
+	if (phy_drd->vbus_boost)
+		regulator_disable(phy_drd->vbus_boost);
 fail_vbus:
 	clk_disable_unprepare(phy_drd->ref_clk);
 	if (!IS_ERR(phy_drd->pipeclk))
@@ -493,6 +505,8 @@ static int exynos5_usbdrd_phy_power_off(struct phy *phy)
 	/* Disable VBUS supply */
 	if (phy_drd->vbus)
 		regulator_disable(phy_drd->vbus);
+	if (phy_drd->vbus_boost)
+		regulator_disable(phy_drd->vbus_boost);
 
 	clk_disable_unprepare(phy_drd->ref_clk);
 	if (!IS_ERR(phy_drd->pipeclk))
@@ -645,7 +659,7 @@ static int exynos5_usbdrd_phy_probe(struct platform_device *pdev)
 		break;
 	}
 
-	/* Get Vbus regulator */
+	/* Get Vbus regulators */
 	phy_drd->vbus = devm_regulator_get(dev, "vbus");
 	if (IS_ERR(phy_drd->vbus)) {
 		ret = PTR_ERR(phy_drd->vbus);
@@ -655,6 +669,15 @@ static int exynos5_usbdrd_phy_probe(struct platform_device *pdev)
 		dev_warn(dev, "Failed to get VBUS supply regulator\n");
 		phy_drd->vbus = NULL;
 	}
+	phy_drd->vbus_boost = devm_regulator_get(dev, "vbus-boost");
+	if (IS_ERR(phy_drd->vbus_boost)) {
+		ret = PTR_ERR(phy_drd->vbus_boost);
+		if (ret == -EPROBE_DEFER)
+			return ret;
+
+		dev_warn(dev, "Failed to get VBUS boost supply regulator\n");
+		phy_drd->vbus_boost = NULL;
+	}
 
 	dev_vdbg(dev, "Creating usbdrd_phy phy\n");
 
-- 
1.7.10.4

[PATCH 4/5] usb: dwc3: Adding Kconfig dependency for Exynos7

[Vivek Gautam]

The Exynos-DWC3 USB 3.0 DRD controller is also present on
Exynos7 platform, so adding the dependency on ARCH_EXYNOS7
for this driver.

Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
---
 drivers/usb/dwc3/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 785510a..e235894 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -55,7 +55,7 @@ config USB_DWC3_OMAP
 
 config USB_DWC3_EXYNOS
 	tristate "Samsung Exynos Platform"
-	depends on ARCH_EXYNOS || COMPILE_TEST
+	depends on (ARCH_EXYNOS || ARCH_EXYNOS7) || COMPILE_TEST
 	default USB_DWC3
 	help
 	  Recent Exynos5 SoCs ship with one DesignWare Core USB3 IP inside,
-- 
1.7.10.4

[PATCH 5/5] phy: exynos5-usbdrd: Adding Kconfig dependency for Exynos7

[Vivek Gautam]

This USB 3.0 PHY controller is also present on Exynos7
platform, so adding the dependency on ARCH_EXYNOS7 for this driver.

Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
---
 drivers/phy/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 0dd7427..c3bc380 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -186,7 +186,7 @@ config PHY_EXYNOS5250_USB2
 
 config PHY_EXYNOS5_USBDRD
 	tristate "Exynos5 SoC series USB DRD PHY driver"
-	depends on ARCH_EXYNOS5 && OF
+	depends on (ARCH_EXYNOS5 || ARCH_EXYNOS7) && OF
 	depends on HAS_IOMEM
 	depends on USB_DWC3_EXYNOS
 	select GENERIC_PHY
-- 
1.7.10.4

Re: [PATCH 5/5] phy: exynos5-usbdrd: Adding Kconfig dependency for Exynos7

[Daniele Forsi]

2014-08-28 10:02 GMT+02:00 Vivek Gautam:

> This USB 3.0 PHY controller is also present on Exynos7
> platform, so adding the dependency on ARCH_EXYNOS7 for this driver.

> +++ b/drivers/phy/Kconfig
> @@ -186,7 +186,7 @@ config PHY_EXYNOS5250_USB2
>
>  config PHY_EXYNOS5_USBDRD
>         tristate "Exynos5 SoC series USB DRD PHY driver"
> -       depends on ARCH_EXYNOS5 && OF
> +       depends on (ARCH_EXYNOS5 || ARCH_EXYNOS7) && OF

shouldn't that prompt and its help text be updated to mention also Exynos7?

-- 
Daniele Forsi

Re: [PATCH 1/5] usb: dwc3: exynos: Add support for SCLK present on Exynos7

[Mark Rutland]

On Thu, Aug 28, 2014 at 09:01:56AM +0100, Vivek Gautam wrote:
> Exynos7 also has a separate special gate clock going to the IP
> apart from the usual AHB clock. So add support for the same.
> 
> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
> ---
>  drivers/usb/dwc3/dwc3-exynos.c |   16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
> index f9fb8ad..bab6395 100644
> --- a/drivers/usb/dwc3/dwc3-exynos.c
> +++ b/drivers/usb/dwc3/dwc3-exynos.c
> @@ -35,6 +35,7 @@ struct dwc3_exynos {
>  	struct device		*dev;
>  
>  	struct clk		*clk;
> +	struct clk		*sclk;
>  	struct regulator	*vdd33;
>  	struct regulator	*vdd10;
>  };
> @@ -141,10 +142,17 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
>  		return -EINVAL;
>  	}
>  
> +	/* Exynos7 has a special gate clock going to this IP */
> +	exynos->sclk = devm_clk_get(dev, "usbdrd30_sclk");
> +	if (IS_ERR(exynos->sclk))
> +		dev_warn(dev, "couldn't get sclk\n");

Doesn't this introduce a pointless warning for Exynos SoCs other than
Exynos7?

> +
>  	exynos->dev	= dev;
>  	exynos->clk	= clk;
>  
>  	clk_prepare_enable(exynos->clk);
> +	if (!IS_ERR(exynos->sclk))
> +		clk_prepare_enable(exynos->sclk);

If you replaced the returned err value with NULL you could avoid these
IS_ERR cases.

Mark.

>  
>  	exynos->vdd33 = devm_regulator_get(dev, "vdd33");
>  	if (IS_ERR(exynos->vdd33)) {
> @@ -187,6 +195,8 @@ err4:
>  err3:
>  	regulator_disable(exynos->vdd33);
>  err2:
> +	if (!IS_ERR(exynos->sclk))
> +		clk_disable_unprepare(exynos->sclk);
>  	clk_disable_unprepare(clk);
>  	return ret;
>  }
> @@ -199,6 +209,8 @@ static int dwc3_exynos_remove(struct platform_device *pdev)
>  	platform_device_unregister(exynos->usb2_phy);
>  	platform_device_unregister(exynos->usb3_phy);
>  
> +	if (!IS_ERR(exynos->sclk))
> +		clk_disable_unprepare(exynos->sclk);
>  	clk_disable_unprepare(exynos->clk);
>  
>  	regulator_disable(exynos->vdd33);
> @@ -220,6 +232,8 @@ static int dwc3_exynos_suspend(struct device *dev)
>  {
>  	struct dwc3_exynos *exynos = dev_get_drvdata(dev);
>  
> +	if (!IS_ERR(exynos->sclk))
> +		clk_disable(exynos->sclk);
>  	clk_disable(exynos->clk);
>  
>  	regulator_disable(exynos->vdd33);
> @@ -245,6 +259,8 @@ static int dwc3_exynos_resume(struct device *dev)
>  	}
>  
>  	clk_enable(exynos->clk);
> +	if (!IS_ERR(exynos->sclk))
> +		clk_enable(exynos->sclk);
>  
>  	/* runtime set active to reflect active state. */
>  	pm_runtime_disable(dev);
> -- 
> 1.7.10.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 2/5] phy: exynos5-usbdrd: Add pipe-clk and utmi-clk support

[Mark Rutland]

On Thu, Aug 28, 2014 at 09:01:57AM +0100, Vivek Gautam wrote:
> Exynos7 SoC has now separate gate control for 125MHz pipe3 phy
> clock, as well as 60MHz utmi phy clock.
> So get the same and control in the phy-exynos5-usbdrd driver.
> 
> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
> ---
>  .../devicetree/bindings/phy/samsung-phy.txt        |    4 ++++
>  drivers/phy/phy-exynos5-usbdrd.c                   |   24 ++++++++++++++++++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt
> index 7a6feea..b64d616 100644
> --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
> +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
> @@ -135,6 +135,10 @@ Required properties:
>  	       PHY operations, associated by phy name. It is used to
>  	       determine bit values for clock settings register.
>  	       For Exynos5420 this is given as 'sclk_usbphy30' in CMU.
> +	- optional clocks: Next gen Exynos SoCs have following additional

It's not going to be 'Next gen' for long...

> +			   gate clocks available:
> +			   - phy_pipe: for PIPE3 phy
> +			   - phy_utmi: for UTMI+ phy
>  - samsung,pmu-syscon: phandle for PMU system controller interface, used to
>  		      control pmu registers for power isolation.
>  - #phy-cells : from the generic PHY bindings, must be 1;
> diff --git a/drivers/phy/phy-exynos5-usbdrd.c b/drivers/phy/phy-exynos5-usbdrd.c
> index b05302b..685c108 100644
> --- a/drivers/phy/phy-exynos5-usbdrd.c
> +++ b/drivers/phy/phy-exynos5-usbdrd.c
> @@ -148,6 +148,8 @@ struct exynos5_usbdrd_phy_drvdata {
>   * @dev: pointer to device instance of this platform device
>   * @reg_phy: usb phy controller register memory base
>   * @clk: phy clock for register access
> + * @pipeclk: clock for pipe3 phy
> + * @utmiclk: clock for utmi+ phy
>   * @drv_data: pointer to SoC level driver data structure
>   * @phys[]: array for 'EXYNOS5_DRDPHYS_NUM' number of PHY
>   *	    instances each with its 'phy' and 'phy_cfg'.
> @@ -161,6 +163,8 @@ struct exynos5_usbdrd_phy {
>  	struct device *dev;
>  	void __iomem *reg_phy;
>  	struct clk *clk;
> +	struct clk *pipeclk;
> +	struct clk *utmiclk;
>  	const struct exynos5_usbdrd_phy_drvdata *drv_data;
>  	struct phy_usb_instance {
>  		struct phy *phy;
> @@ -446,6 +450,10 @@ static int exynos5_usbdrd_phy_power_on(struct phy *phy)
>  
>  	dev_dbg(phy_drd->dev, "Request to power_on usbdrd_phy phy\n");
>  
> +	if (!IS_ERR(phy_drd->utmiclk))
> +		clk_prepare_enable(phy_drd->utmiclk);
> +	if (!IS_ERR(phy_drd->pipeclk))
> +		clk_prepare_enable(phy_drd->pipeclk);
>  	clk_prepare_enable(phy_drd->ref_clk);
>  
>  	/* Enable VBUS supply */
> @@ -464,6 +472,10 @@ static int exynos5_usbdrd_phy_power_on(struct phy *phy)
>  
>  fail_vbus:
>  	clk_disable_unprepare(phy_drd->ref_clk);
> +	if (!IS_ERR(phy_drd->pipeclk))
> +		clk_disable_unprepare(phy_drd->pipeclk);
> +	if (!IS_ERR(phy_drd->utmiclk))
> +		clk_disable_unprepare(phy_drd->utmiclk);
>  
>  	return ret;
>  }
> @@ -483,6 +495,10 @@ static int exynos5_usbdrd_phy_power_off(struct phy *phy)
>  		regulator_disable(phy_drd->vbus);
>  
>  	clk_disable_unprepare(phy_drd->ref_clk);
> +	if (!IS_ERR(phy_drd->pipeclk))
> +		clk_disable_unprepare(phy_drd->pipeclk);
> +	if (!IS_ERR(phy_drd->utmiclk))
> +		clk_disable_unprepare(phy_drd->utmiclk);
>  
>  	return 0;
>  }
> @@ -581,6 +597,14 @@ static int exynos5_usbdrd_phy_probe(struct platform_device *pdev)
>  		return PTR_ERR(phy_drd->clk);
>  	}
>  
> +	phy_drd->pipeclk = devm_clk_get(dev, "phy_pipe");
> +	if (IS_ERR(phy_drd->pipeclk))
> +		dev_warn(dev, "Failed to get pipe3 phy operational clock\n");
> +
> +	phy_drd->utmiclk = devm_clk_get(dev, "phy_utmi");
> +	if (IS_ERR(phy_drd->utmiclk))
> +		dev_warn(dev, "Failed to get utmi phy operational clock\n");
> +

Pointless warnings for !Exynos7?

Would it not be better to set these to NULL and not litter the code with
IS_ERR checks?

Mark.

Re: [PATCH 3/5] phy: exynos5-usbdrd: Add facility for VBUS-BOOST-5V supply

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 777 bytes --]

hi,

On Thu, Aug 28, 2014 at 01:31:58PM +0530, Vivek Gautam wrote:
> @@ -457,11 +458,19 @@ static int exynos5_usbdrd_phy_power_on(struct phy *phy)
>  	clk_prepare_enable(phy_drd->ref_clk);
>  
>  	/* Enable VBUS supply */
> +	if (phy_drd->vbus_boost) {
> +		ret = regulator_enable(phy_drd->vbus_boost);
> +		if (ret) {
> +			dev_err(phy_drd->dev,
> +				"Failed to enable VBUS boost supply\n");
> +			goto fail_vbus;
> +		}
> +	}

really this is nitpicking, but can you add a blank line here just make
my inner child happy ? :-)

> @@ -470,6 +479,9 @@ static int exynos5_usbdrd_phy_power_on(struct phy *phy)
>  
>  	return 0;
>  
> +fail_vbus_boost:
> +	if (phy_drd->vbus_boost)
> +		regulator_disable(phy_drd->vbus_boost);

same here

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 4/5] usb: dwc3: Adding Kconfig dependency for Exynos7

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 838 bytes --]

On Thu, Aug 28, 2014 at 01:31:59PM +0530, Vivek Gautam wrote:
> The Exynos-DWC3 USB 3.0 DRD controller is also present on
> Exynos7 platform, so adding the dependency on ARCH_EXYNOS7
> for this driver.
> 
> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
> ---
>  drivers/usb/dwc3/Kconfig |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> index 785510a..e235894 100644
> --- a/drivers/usb/dwc3/Kconfig
> +++ b/drivers/usb/dwc3/Kconfig
> @@ -55,7 +55,7 @@ config USB_DWC3_OMAP
>  
>  config USB_DWC3_EXYNOS
>  	tristate "Samsung Exynos Platform"
> -	depends on ARCH_EXYNOS || COMPILE_TEST
> +	depends on (ARCH_EXYNOS || ARCH_EXYNOS7) || COMPILE_TEST

wait when building for ARCH_EXYNOS7 you don't get ARCH_EXYNOS ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 3/5] phy: exynos5-usbdrd: Add facility for VBUS-BOOST-5V supply

[Vivek Gautam]

Hi Felipe,


On Fri, Aug 29, 2014 at 12:46 AM, Felipe Balbi <balbi@ti.com> wrote:
> hi,
>
> On Thu, Aug 28, 2014 at 01:31:58PM +0530, Vivek Gautam wrote:
>> @@ -457,11 +458,19 @@ static int exynos5_usbdrd_phy_power_on(struct phy *phy)
>>       clk_prepare_enable(phy_drd->ref_clk);
>>
>>       /* Enable VBUS supply */
>> +     if (phy_drd->vbus_boost) {
>> +             ret = regulator_enable(phy_drd->vbus_boost);
>> +             if (ret) {
>> +                     dev_err(phy_drd->dev,
>> +                             "Failed to enable VBUS boost supply\n");
>> +                     goto fail_vbus;
>> +             }
>> +     }
>
> really this is nitpicking, but can you add a blank line here just make
> my inner child happy ? :-)

Sure will add an extra line here and similar instances of this change.

>
>> @@ -470,6 +479,9 @@ static int exynos5_usbdrd_phy_power_on(struct phy *phy)
>>
>>       return 0;
>>
>> +fail_vbus_boost:
>> +     if (phy_drd->vbus_boost)
>> +             regulator_disable(phy_drd->vbus_boost);
>
> same here
>
> --
> balbi



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

Re: [PATCH 5/5] phy: exynos5-usbdrd: Adding Kconfig dependency for Exynos7

[Vivek Gautam]

On Thu, Aug 28, 2014 at 8:36 PM, Daniele Forsi <dforsi@gmail.com> wrote:
> 2014-08-28 10:02 GMT+02:00 Vivek Gautam:
>
>> This USB 3.0 PHY controller is also present on Exynos7
>> platform, so adding the dependency on ARCH_EXYNOS7 for this driver.
>
>> +++ b/drivers/phy/Kconfig
>> @@ -186,7 +186,7 @@ config PHY_EXYNOS5250_USB2
>>
>>  config PHY_EXYNOS5_USBDRD
>>         tristate "Exynos5 SoC series USB DRD PHY driver"
>> -       depends on ARCH_EXYNOS5 && OF
>> +       depends on (ARCH_EXYNOS5 || ARCH_EXYNOS7) && OF
>
> shouldn't that prompt and its help text be updated to mention also Exynos7?

Right, even that has to be updated accordingly. Will update the same in next
version of the patch. Thanks for pointing this.



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

Re: [PATCH 1/5] usb: dwc3: exynos: Add support for SCLK present on Exynos7

[Vivek Gautam]

Hi,


On Fri, Aug 29, 2014 at 12:18 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> On Thu, Aug 28, 2014 at 09:01:56AM +0100, Vivek Gautam wrote:
>> Exynos7 also has a separate special gate clock going to the IP
>> apart from the usual AHB clock. So add support for the same.
>>
>> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
>> ---
>>  drivers/usb/dwc3/dwc3-exynos.c |   16 ++++++++++++++++
>>  1 file changed, 16 insertions(+)
>>
>> diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
>> index f9fb8ad..bab6395 100644
>> --- a/drivers/usb/dwc3/dwc3-exynos.c
>> +++ b/drivers/usb/dwc3/dwc3-exynos.c
>> @@ -35,6 +35,7 @@ struct dwc3_exynos {
>>       struct device           *dev;
>>
>>       struct clk              *clk;
>> +     struct clk              *sclk;
>>       struct regulator        *vdd33;
>>       struct regulator        *vdd10;
>>  };
>> @@ -141,10 +142,17 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
>>               return -EINVAL;
>>       }
>>
>> +     /* Exynos7 has a special gate clock going to this IP */
>> +     exynos->sclk = devm_clk_get(dev, "usbdrd30_sclk");
>> +     if (IS_ERR(exynos->sclk))
>> +             dev_warn(dev, "couldn't get sclk\n");
>
> Doesn't this introduce a pointless warning for Exynos SoCs other than
> Exynos7?

True, it will introduce an unnecessary warning for non-Exynos7 systems.
I initially thought of introducing a compatible check for Exynos7-dwc3, but that
way we may end up adding such checks for future SoCs which have similar
controller but have some clock difference or some other small change, no ?

>
>> +
>>       exynos->dev     = dev;
>>       exynos->clk     = clk;
>>
>>       clk_prepare_enable(exynos->clk);
>> +     if (!IS_ERR(exynos->sclk))
>> +             clk_prepare_enable(exynos->sclk);
>
> If you replaced the returned err value with NULL you could avoid these
> IS_ERR cases.

Right, point taken.

[snip]



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

Re: [PATCH 1/5] usb: dwc3: exynos: Add support for SCLK present on Exynos7

[Mark Rutland]

On Tue, Sep 02, 2014 at 11:39:08AM +0100, Vivek Gautam wrote:
> Hi,
> 
> 
> On Fri, Aug 29, 2014 at 12:18 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Thu, Aug 28, 2014 at 09:01:56AM +0100, Vivek Gautam wrote:
> >> Exynos7 also has a separate special gate clock going to the IP
> >> apart from the usual AHB clock. So add support for the same.
> >>
> >> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
> >> ---
> >>  drivers/usb/dwc3/dwc3-exynos.c |   16 ++++++++++++++++
> >>  1 file changed, 16 insertions(+)
> >>
> >> diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
> >> index f9fb8ad..bab6395 100644
> >> --- a/drivers/usb/dwc3/dwc3-exynos.c
> >> +++ b/drivers/usb/dwc3/dwc3-exynos.c
> >> @@ -35,6 +35,7 @@ struct dwc3_exynos {
> >>       struct device           *dev;
> >>
> >>       struct clk              *clk;
> >> +     struct clk              *sclk;
> >>       struct regulator        *vdd33;
> >>       struct regulator        *vdd10;
> >>  };
> >> @@ -141,10 +142,17 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
> >>               return -EINVAL;
> >>       }
> >>
> >> +     /* Exynos7 has a special gate clock going to this IP */
> >> +     exynos->sclk = devm_clk_get(dev, "usbdrd30_sclk");
> >> +     if (IS_ERR(exynos->sclk))
> >> +             dev_warn(dev, "couldn't get sclk\n");
> >
> > Doesn't this introduce a pointless warning for Exynos SoCs other than
> > Exynos7?
> 
> True, it will introduce an unnecessary warning for non-Exynos7 systems.
> I initially thought of introducing a compatible check for Exynos7-dwc3, but that
> way we may end up adding such checks for future SoCs which have similar
> controller but have some clock difference or some other small change, no ?

Perhaps. I don't know what your future hardware will look like.

Is the usbdrd30_sclk input unique to Exynos7, or was it previously there
but just without an input?

If the latter you could just reduce this to:

dev_info(dev, "no sclk specified");

Mark.

Re: [PATCH 1/5] usb: dwc3: exynos: Add support for SCLK present on Exynos7

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1863 bytes --]

On Tue, Sep 02, 2014 at 04:09:08PM +0530, Vivek Gautam wrote:
> Hi,
> 
> 
> On Fri, Aug 29, 2014 at 12:18 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Thu, Aug 28, 2014 at 09:01:56AM +0100, Vivek Gautam wrote:
> >> Exynos7 also has a separate special gate clock going to the IP
> >> apart from the usual AHB clock. So add support for the same.
> >>
> >> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
> >> ---
> >>  drivers/usb/dwc3/dwc3-exynos.c |   16 ++++++++++++++++
> >>  1 file changed, 16 insertions(+)
> >>
> >> diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
> >> index f9fb8ad..bab6395 100644
> >> --- a/drivers/usb/dwc3/dwc3-exynos.c
> >> +++ b/drivers/usb/dwc3/dwc3-exynos.c
> >> @@ -35,6 +35,7 @@ struct dwc3_exynos {
> >>       struct device           *dev;
> >>
> >>       struct clk              *clk;
> >> +     struct clk              *sclk;
> >>       struct regulator        *vdd33;
> >>       struct regulator        *vdd10;
> >>  };
> >> @@ -141,10 +142,17 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
> >>               return -EINVAL;
> >>       }
> >>
> >> +     /* Exynos7 has a special gate clock going to this IP */
> >> +     exynos->sclk = devm_clk_get(dev, "usbdrd30_sclk");
> >> +     if (IS_ERR(exynos->sclk))
> >> +             dev_warn(dev, "couldn't get sclk\n");
> >
> > Doesn't this introduce a pointless warning for Exynos SoCs other than
> > Exynos7?
> 
> True, it will introduce an unnecessary warning for non-Exynos7 systems.
> I initially thought of introducing a compatible check for Exynos7-dwc3, but that
> way we may end up adding such checks for future SoCs which have similar
> controller but have some clock difference or some other small change, no ?

maybe dev_dbg() is what you want ? :-)

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 5/5] phy: exynos5-usbdrd: Adding Kconfig dependency for Exynos7

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1034 bytes --]

On Mon, Sep 01, 2014 at 01:30:21PM +0530, Vivek Gautam wrote:
> On Thu, Aug 28, 2014 at 8:36 PM, Daniele Forsi <dforsi@gmail.com> wrote:
> > 2014-08-28 10:02 GMT+02:00 Vivek Gautam:
> >
> >> This USB 3.0 PHY controller is also present on Exynos7
> >> platform, so adding the dependency on ARCH_EXYNOS7 for this driver.
> >
> >> +++ b/drivers/phy/Kconfig
> >> @@ -186,7 +186,7 @@ config PHY_EXYNOS5250_USB2
> >>
> >>  config PHY_EXYNOS5_USBDRD
> >>         tristate "Exynos5 SoC series USB DRD PHY driver"
> >> -       depends on ARCH_EXYNOS5 && OF
> >> +       depends on (ARCH_EXYNOS5 || ARCH_EXYNOS7) && OF
> >
> > shouldn't that prompt and its help text be updated to mention also Exynos7?
> 
> Right, even that has to be updated accordingly. Will update the same in next
> version of the patch. Thanks for pointing this.

I would rather change that to ARCH_EXYNOS, unless Kishon doesn't like
that idea. The thing is that this will likely need to be patches for
exynos8, 9, 10, 11...

cheers

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 4/5] usb: dwc3: Adding Kconfig dependency for Exynos7

[Vivek Gautam]

On Fri, Aug 29, 2014 at 12:58 AM, Felipe Balbi <balbi@ti.com> wrote:
> On Thu, Aug 28, 2014 at 01:31:59PM +0530, Vivek Gautam wrote:
>> The Exynos-DWC3 USB 3.0 DRD controller is also present on
>> Exynos7 platform, so adding the dependency on ARCH_EXYNOS7
>> for this driver.
>>
>> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
>> ---
>>  drivers/usb/dwc3/Kconfig |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
>> index 785510a..e235894 100644
>> --- a/drivers/usb/dwc3/Kconfig
>> +++ b/drivers/usb/dwc3/Kconfig
>> @@ -55,7 +55,7 @@ config USB_DWC3_OMAP
>>
>>  config USB_DWC3_EXYNOS
>>       tristate "Samsung Exynos Platform"
>> -     depends on ARCH_EXYNOS || COMPILE_TEST
>> +     depends on (ARCH_EXYNOS || ARCH_EXYNOS7) || COMPILE_TEST
>
> wait when building for ARCH_EXYNOS7 you don't get ARCH_EXYNOS ?

Right, we do get it now in V2 patch series for Exynos7 [1],
but it wasn't available in the first series [2].
Will drop this patch now.

[1] http://www.spinics.net/lists/linux-samsung-soc/msg36378.html
[2] http://www.spinics.net/lists/arm-kernel/msg357382.html



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

Re: [PATCH 5/5] phy: exynos5-usbdrd: Adding Kconfig dependency for Exynos7

[Vivek Gautam]

On Tue, Sep 2, 2014 at 8:07 PM, Felipe Balbi <balbi@ti.com> wrote:
> On Mon, Sep 01, 2014 at 01:30:21PM +0530, Vivek Gautam wrote:
>> On Thu, Aug 28, 2014 at 8:36 PM, Daniele Forsi <dforsi@gmail.com> wrote:
>> > 2014-08-28 10:02 GMT+02:00 Vivek Gautam:
>> >
>> >> This USB 3.0 PHY controller is also present on Exynos7
>> >> platform, so adding the dependency on ARCH_EXYNOS7 for this driver.
>> >
>> >> +++ b/drivers/phy/Kconfig
>> >> @@ -186,7 +186,7 @@ config PHY_EXYNOS5250_USB2
>> >>
>> >>  config PHY_EXYNOS5_USBDRD
>> >>         tristate "Exynos5 SoC series USB DRD PHY driver"
>> >> -       depends on ARCH_EXYNOS5 && OF
>> >> +       depends on (ARCH_EXYNOS5 || ARCH_EXYNOS7) && OF
>> >
>> > shouldn't that prompt and its help text be updated to mention also Exynos7?
>>
>> Right, even that has to be updated accordingly. Will update the same in next
>> version of the patch. Thanks for pointing this.
>
> I would rather change that to ARCH_EXYNOS, unless Kishon doesn't like
> that idea. The thing is that this will likely need to be patches for
> exynos8, 9, 10, 11...

Yes, after we have the 2nd version of Exynos7 support patches, it makes
more sense to keep dependency on ARCH_EXYNOS.




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

Re: [PATCH 5/5] phy: exynos5-usbdrd: Adding Kconfig dependency for Exynos7

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1432 bytes --]

On Wed, Sep 03, 2014 at 09:32:14AM +0530, Vivek Gautam wrote:
> On Tue, Sep 2, 2014 at 8:07 PM, Felipe Balbi <balbi@ti.com> wrote:
> > On Mon, Sep 01, 2014 at 01:30:21PM +0530, Vivek Gautam wrote:
> >> On Thu, Aug 28, 2014 at 8:36 PM, Daniele Forsi <dforsi@gmail.com> wrote:
> >> > 2014-08-28 10:02 GMT+02:00 Vivek Gautam:
> >> >
> >> >> This USB 3.0 PHY controller is also present on Exynos7
> >> >> platform, so adding the dependency on ARCH_EXYNOS7 for this driver.
> >> >
> >> >> +++ b/drivers/phy/Kconfig
> >> >> @@ -186,7 +186,7 @@ config PHY_EXYNOS5250_USB2
> >> >>
> >> >>  config PHY_EXYNOS5_USBDRD
> >> >>         tristate "Exynos5 SoC series USB DRD PHY driver"
> >> >> -       depends on ARCH_EXYNOS5 && OF
> >> >> +       depends on (ARCH_EXYNOS5 || ARCH_EXYNOS7) && OF
> >> >
> >> > shouldn't that prompt and its help text be updated to mention also Exynos7?
> >>
> >> Right, even that has to be updated accordingly. Will update the same in next
> >> version of the patch. Thanks for pointing this.
> >
> > I would rather change that to ARCH_EXYNOS, unless Kishon doesn't like
> > that idea. The thing is that this will likely need to be patches for
> > exynos8, 9, 10, 11...
> 
> Yes, after we have the 2nd version of Exynos7 support patches, it makes
> more sense to keep dependency on ARCH_EXYNOS.

thank you, that'll help new silicon wakeup; one less thing to patch :-p

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]