[PATCH 0/7] Exynos4412-based Trats2 USB gadget (DWC2) fixes

[PATCH 0/7] Exynos4412-based Trats2 USB gadget (DWC2) fixes

[Marek Szyprowski]

Dear All,

Since v3.19 s3c-hsotg (DWC2) USB controller stopped working on
Exynos4412-based Trats2 platform. However on Odroid-U3 (which is also
Exynos4412-based) it worked fine all the time. After long investigation
it turned out that this was caused by 2 independent issues.

First issue was caused by patch 7eec1266751bd3a25e35ce88686634c768fedc24
("ARM: dts: Add Maxim 77693 PMIC to exynos4412-trats2") added support
for Maxim 77693 regulators, but without defining consumers for them.
This causes regulator core to disable them. However SAFEOUT1 regulator
provides power supply to VBUS signal, which also power some USB phy
related parts of Exynos SoC core. This has been fixed by patches 1-3,
which adds support for VBUS regulator, defines them in DTS for all
Exynos platforms and changes probe sequence of the drivers to ensure no
deferred probe occurs (USB gadget subsystem doesn't support deferred
probe yet).

Second issue is related to DWC2 driver rewrite and host/gadget/dual-role
integration code. DWC2 module on some platforms needs three additional
hardware resources: phy, clock and power supply. All of them must be
enabled/activated to properly initialize and operate. This was initially
handled in s3c-hsotg driver, which has been converted to 'gadget' part
of dwc2 driver. Unfortunately, not all of this code got moved to common
platform code, what resulted in accessing DWC2 registers without
enabling power supply regulators and clock. This caused initialization
failure on Exynos4412-based Trats2. Odroid U3 board worked fine, because
power supplies used by DWC2 module are marked there as 'always-on',
because they are also used by other modules (USB hub) and clock was
shared with USB2 PHY, so it was already enabled. This initialization
issue has been fixed by the last patch. While preparing it I've also
fixed a few minor issues found in nearby code (patches 4-6).

I would like to get those patches merged separately to respective
subsystem trees:
patch 1: phy subsystem tree
patch 2: Samsung Exynos tree
patch 3: regulator subsystem tree
patch 4-7: USB gadget subsystem tree

Patches have been prepared on top of linux-next from 20150821.

Best regards
Marek Szyprowski
Samsung R&D Institute Poland


Patch summary:

Marek Szyprowski (7):
  phy: exynos-usb2: add vbus regulator support
  arm: dts: exynos: add vbus regulator to USB2 phy nodes
  regulators: max77693: register driver earlier to avoid deferred probe
  usb: dwc2: remove double call to s3c_hsotg_of_probe
  usb: dwc2: remove non-functional clock gating
  usb: dwc2: fix unbalanced phy control
  usb: dwc2: refactor common low-level hw code to platform.c

 .../devicetree/bindings/phy/samsung-phy.txt        |   3 +
 arch/arm/boot/dts/exynos3250-monk.dts              |   1 +
 arch/arm/boot/dts/exynos3250-rinato.dts            |   1 +
 arch/arm/boot/dts/exynos4210-trats.dts             |   2 +-
 arch/arm/boot/dts/exynos4210-universal_c210.dts    |   2 +-
 arch/arm/boot/dts/exynos4412-trats2.dts            |   1 +
 drivers/phy/phy-samsung-usb2.c                     |  25 ++-
 drivers/phy/phy-samsung-usb2.h                     |   2 +
 drivers/regulator/max77693.c                       |  12 +-
 drivers/usb/dwc2/core.h                            |   4 +-
 drivers/usb/dwc2/gadget.c                          | 196 +++----------------
 drivers/usb/dwc2/platform.c                        | 209 +++++++++++++++++----
 12 files changed, 239 insertions(+), 219 deletions(-)

-- 
1.9.2

[PATCH 1/7] phy: exynos-usb2: add vbus regulator support

[Marek Szyprowski]

Exynos USB2 PHY has separate power supply, which is usually provided by
VBUS regulator. This patch adds support for it. VBUS regulator is
optional, to keep compatibility with boards, which have VBUS provided
from some always-on power source.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 .../devicetree/bindings/phy/samsung-phy.txt        |  3 +++
 drivers/phy/phy-samsung-usb2.c                     | 25 ++++++++++++++++++++--
 drivers/phy/phy-samsung-usb2.h                     |  2 ++
 3 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt
index 60c6f2a633e0..0289d3b07853 100644
--- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
+++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
@@ -44,6 +44,9 @@ Required properties:
 	- the "ref" clock is used to get the rate of the clock provided to the
 	  PHY module
 
+Optional properties:
+- vbus-supply: power-supply phandle for vbus power source
+
 The first phandle argument in the PHY specifier identifies the PHY, its
 meaning is compatible dependent. For the currently supported SoCs (Exynos 4210
 and Exynos 4212) it is as follows:
diff --git a/drivers/phy/phy-samsung-usb2.c b/drivers/phy/phy-samsung-usb2.c
index f278a9c547e1..1d22d93b552d 100644
--- a/drivers/phy/phy-samsung-usb2.c
+++ b/drivers/phy/phy-samsung-usb2.c
@@ -27,6 +27,13 @@ static int samsung_usb2_phy_power_on(struct phy *phy)
 
 	dev_dbg(drv->dev, "Request to power_on \"%s\" usb phy\n",
 		inst->cfg->label);
+
+	if (drv->vbus) {
+		ret = regulator_enable(drv->vbus);
+		if (ret)
+			goto err_regulator;
+	}
+
 	ret = clk_prepare_enable(drv->clk);
 	if (ret)
 		goto err_main_clk;
@@ -48,6 +55,9 @@ err_power_on:
 err_instance_clk:
 	clk_disable_unprepare(drv->clk);
 err_main_clk:
+	if (drv->vbus)
+		regulator_disable(drv->vbus);
+err_regulator:
 	return ret;
 }
 
@@ -55,7 +65,7 @@ static int samsung_usb2_phy_power_off(struct phy *phy)
 {
 	struct samsung_usb2_phy_instance *inst = phy_get_drvdata(phy);
 	struct samsung_usb2_phy_driver *drv = inst->drv;
-	int ret;
+	int ret = 0;
 
 	dev_dbg(drv->dev, "Request to power_off \"%s\" usb phy\n",
 		inst->cfg->label);
@@ -68,7 +78,10 @@ static int samsung_usb2_phy_power_off(struct phy *phy)
 	}
 	clk_disable_unprepare(drv->ref_clk);
 	clk_disable_unprepare(drv->clk);
-	return 0;
+	if (drv->vbus)
+		ret = regulator_disable(drv->vbus);
+
+	return ret;
 }
 
 static const struct phy_ops samsung_usb2_phy_ops = {
@@ -203,6 +216,14 @@ static int samsung_usb2_phy_probe(struct platform_device *pdev)
 			return ret;
 	}
 
+	drv->vbus = devm_regulator_get(dev, "vbus");
+	if (IS_ERR(drv->vbus)) {
+		ret = PTR_ERR(drv->vbus);
+		if (ret == -EPROBE_DEFER)
+			return ret;
+		drv->vbus = NULL;
+	}
+
 	for (i = 0; i < drv->cfg->num_phys; i++) {
 		char *label = drv->cfg->phys[i].label;
 		struct samsung_usb2_phy_instance *p = &drv->instances[i];
diff --git a/drivers/phy/phy-samsung-usb2.h b/drivers/phy/phy-samsung-usb2.h
index 44bead9b8f34..6563e7ca0ac4 100644
--- a/drivers/phy/phy-samsung-usb2.h
+++ b/drivers/phy/phy-samsung-usb2.h
@@ -17,6 +17,7 @@
 #include <linux/device.h>
 #include <linux/regmap.h>
 #include <linux/spinlock.h>
+#include <linux/regulator/consumer.h>
 
 #define KHZ 1000
 #define MHZ (KHZ * KHZ)
@@ -37,6 +38,7 @@ struct samsung_usb2_phy_driver {
 	const struct samsung_usb2_phy_config *cfg;
 	struct clk *clk;
 	struct clk *ref_clk;
+	struct regulator *vbus;
 	unsigned long ref_rate;
 	u32 ref_reg_val;
 	struct device *dev;
-- 
1.9.2

Re: [PATCH 1/7] phy: exynos-usb2: add vbus regulator support

[Kishon Vijay Abraham I]

Hi,

On Friday 21 August 2015 06:08 PM, Marek Szyprowski wrote:
> Exynos USB2 PHY has separate power supply, which is usually provided by
> VBUS regulator. This patch adds support for it. VBUS regulator is
> optional, to keep compatibility with boards, which have VBUS provided
> from some always-on power source.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  .../devicetree/bindings/phy/samsung-phy.txt        |  3 +++
>  drivers/phy/phy-samsung-usb2.c                     | 25 ++++++++++++++++++++--
>  drivers/phy/phy-samsung-usb2.h                     |  2 ++
>  3 files changed, 28 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt
> index 60c6f2a633e0..0289d3b07853 100644
> --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
> +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
> @@ -44,6 +44,9 @@ Required properties:
>  	- the "ref" clock is used to get the rate of the clock provided to the
>  	  PHY module
>  
> +Optional properties:
> +- vbus-supply: power-supply phandle for vbus power source

how about using phy-supply?

Thanks
Kishon

> +
>  The first phandle argument in the PHY specifier identifies the PHY, its
>  meaning is compatible dependent. For the currently supported SoCs (Exynos 4210
>  and Exynos 4212) it is as follows:
> diff --git a/drivers/phy/phy-samsung-usb2.c b/drivers/phy/phy-samsung-usb2.c
> index f278a9c547e1..1d22d93b552d 100644
> --- a/drivers/phy/phy-samsung-usb2.c
> +++ b/drivers/phy/phy-samsung-usb2.c
> @@ -27,6 +27,13 @@ static int samsung_usb2_phy_power_on(struct phy *phy)
>  
>  	dev_dbg(drv->dev, "Request to power_on \"%s\" usb phy\n",
>  		inst->cfg->label);
> +
> +	if (drv->vbus) {
> +		ret = regulator_enable(drv->vbus);
> +		if (ret)
> +			goto err_regulator;
> +	}
> +
>  	ret = clk_prepare_enable(drv->clk);
>  	if (ret)
>  		goto err_main_clk;
> @@ -48,6 +55,9 @@ err_power_on:
>  err_instance_clk:
>  	clk_disable_unprepare(drv->clk);
>  err_main_clk:
> +	if (drv->vbus)
> +		regulator_disable(drv->vbus);
> +err_regulator:
>  	return ret;
>  }
>  
> @@ -55,7 +65,7 @@ static int samsung_usb2_phy_power_off(struct phy *phy)
>  {
>  	struct samsung_usb2_phy_instance *inst = phy_get_drvdata(phy);
>  	struct samsung_usb2_phy_driver *drv = inst->drv;
> -	int ret;
> +	int ret = 0;
>  
>  	dev_dbg(drv->dev, "Request to power_off \"%s\" usb phy\n",
>  		inst->cfg->label);
> @@ -68,7 +78,10 @@ static int samsung_usb2_phy_power_off(struct phy *phy)
>  	}
>  	clk_disable_unprepare(drv->ref_clk);
>  	clk_disable_unprepare(drv->clk);
> -	return 0;
> +	if (drv->vbus)
> +		ret = regulator_disable(drv->vbus);
> +
> +	return ret;
>  }
>  
>  static const struct phy_ops samsung_usb2_phy_ops = {
> @@ -203,6 +216,14 @@ static int samsung_usb2_phy_probe(struct platform_device *pdev)
>  			return ret;
>  	}
>  
> +	drv->vbus = devm_regulator_get(dev, "vbus");
> +	if (IS_ERR(drv->vbus)) {
> +		ret = PTR_ERR(drv->vbus);
> +		if (ret == -EPROBE_DEFER)
> +			return ret;
> +		drv->vbus = NULL;
> +	}
> +
>  	for (i = 0; i < drv->cfg->num_phys; i++) {
>  		char *label = drv->cfg->phys[i].label;
>  		struct samsung_usb2_phy_instance *p = &drv->instances[i];
> diff --git a/drivers/phy/phy-samsung-usb2.h b/drivers/phy/phy-samsung-usb2.h
> index 44bead9b8f34..6563e7ca0ac4 100644
> --- a/drivers/phy/phy-samsung-usb2.h
> +++ b/drivers/phy/phy-samsung-usb2.h
> @@ -17,6 +17,7 @@
>  #include <linux/device.h>
>  #include <linux/regmap.h>
>  #include <linux/spinlock.h>
> +#include <linux/regulator/consumer.h>
>  
>  #define KHZ 1000
>  #define MHZ (KHZ * KHZ)
> @@ -37,6 +38,7 @@ struct samsung_usb2_phy_driver {
>  	const struct samsung_usb2_phy_config *cfg;
>  	struct clk *clk;
>  	struct clk *ref_clk;
> +	struct regulator *vbus;
>  	unsigned long ref_rate;
>  	u32 ref_reg_val;
>  	struct device *dev;
> 

Re: [PATCH 1/7] phy: exynos-usb2: add vbus regulator support

[Marek Szyprowski]

Hello,

On 2015-08-21 14:44, Kishon Vijay Abraham I wrote:
> On Friday 21 August 2015 06:08 PM, Marek Szyprowski wrote:
>> Exynos USB2 PHY has separate power supply, which is usually provided by
>> VBUS regulator. This patch adds support for it. VBUS regulator is
>> optional, to keep compatibility with boards, which have VBUS provided
>> from some always-on power source.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>>   .../devicetree/bindings/phy/samsung-phy.txt        |  3 +++
>>   drivers/phy/phy-samsung-usb2.c                     | 25 ++++++++++++++++++++--
>>   drivers/phy/phy-samsung-usb2.h                     |  2 ++
>>   3 files changed, 28 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt
>> index 60c6f2a633e0..0289d3b07853 100644
>> --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
>> +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
>> @@ -44,6 +44,9 @@ Required properties:
>>   	- the "ref" clock is used to get the rate of the clock provided to the
>>   	  PHY module
>>   
>> +Optional properties:
>> +- vbus-supply: power-supply phandle for vbus power source
> how about using phy-supply?

I wanted to make it a bit more descriptive (vbus-supply is rather self 
explaining name)
and keep it in line with Exynos usb3-drd phy, which already supports 
vbus-supply.
If you think that this is a bad idea, a will use phy-supply then.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH 1/7] phy: exynos-usb2: add vbus regulator support

[Krzysztof Kozlowski]

On 25.08.2015 14:47, Marek Szyprowski wrote:
> Hello,
> 
> On 2015-08-21 14:44, Kishon Vijay Abraham I wrote:
>> On Friday 21 August 2015 06:08 PM, Marek Szyprowski wrote:
>>> Exynos USB2 PHY has separate power supply, which is usually provided by
>>> VBUS regulator. This patch adds support for it. VBUS regulator is
>>> optional, to keep compatibility with boards, which have VBUS provided
>>> from some always-on power source.
>>>
>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>> ---
>>>   .../devicetree/bindings/phy/samsung-phy.txt        |  3 +++
>>>   drivers/phy/phy-samsung-usb2.c                     | 25
>>> ++++++++++++++++++++--
>>>   drivers/phy/phy-samsung-usb2.h                     |  2 ++
>>>   3 files changed, 28 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt
>>> b/Documentation/devicetree/bindings/phy/samsung-phy.txt
>>> index 60c6f2a633e0..0289d3b07853 100644
>>> --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
>>> +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
>>> @@ -44,6 +44,9 @@ Required properties:
>>>       - the "ref" clock is used to get the rate of the clock provided
>>> to the
>>>         PHY module
>>>   +Optional properties:
>>> +- vbus-supply: power-supply phandle for vbus power source
>> how about using phy-supply?
> 
> I wanted to make it a bit more descriptive (vbus-supply is rather self
> explaining name)
> and keep it in line with Exynos usb3-drd phy, which already supports
> vbus-supply.
> If you think that this is a bad idea, a will use phy-supply then.

This is actually supply for VBUS, not for the phy. Using phy-supply
would work fine and reduce the size of code... but would be rather a
hacky-use of phy and it could be misleading.

I don't have strong feeling about this, both ideas have its advantages.
If I had to choose than I would like to use vbus-supply because of its
correctness with real-world (this is a VBUS after all).

Best regards,
Krzysztof

Re: [PATCH 1/7] phy: exynos-usb2: add vbus regulator support

[Kishon Vijay Abraham I]

Hi,

On Tuesday 25 August 2015 11:42 AM, Krzysztof Kozlowski wrote:
> On 25.08.2015 14:47, Marek Szyprowski wrote:
>> Hello,
>>
>> On 2015-08-21 14:44, Kishon Vijay Abraham I wrote:
>>> On Friday 21 August 2015 06:08 PM, Marek Szyprowski wrote:
>>>> Exynos USB2 PHY has separate power supply, which is usually provided by
>>>> VBUS regulator. This patch adds support for it. VBUS regulator is
>>>> optional, to keep compatibility with boards, which have VBUS provided
>>>> from some always-on power source.
>>>>
>>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>>> ---
>>>>   .../devicetree/bindings/phy/samsung-phy.txt        |  3 +++
>>>>   drivers/phy/phy-samsung-usb2.c                     | 25
>>>> ++++++++++++++++++++--
>>>>   drivers/phy/phy-samsung-usb2.h                     |  2 ++
>>>>   3 files changed, 28 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt
>>>> b/Documentation/devicetree/bindings/phy/samsung-phy.txt
>>>> index 60c6f2a633e0..0289d3b07853 100644
>>>> --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
>>>> +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
>>>> @@ -44,6 +44,9 @@ Required properties:
>>>>       - the "ref" clock is used to get the rate of the clock provided
>>>> to the
>>>>         PHY module
>>>>   +Optional properties:
>>>> +- vbus-supply: power-supply phandle for vbus power source
>>> how about using phy-supply?
>>
>> I wanted to make it a bit more descriptive (vbus-supply is rather self
>> explaining name)
>> and keep it in line with Exynos usb3-drd phy, which already supports
>> vbus-supply.
>> If you think that this is a bad idea, a will use phy-supply then.
> 
> This is actually supply for VBUS, not for the phy. Using phy-supply
> would work fine and reduce the size of code... but would be rather a
> hacky-use of phy and it could be misleading.
> 
> I don't have strong feeling about this, both ideas have its advantages.
> If I had to choose than I would like to use vbus-supply because of its
> correctness with real-world (this is a VBUS after all).

Alright.. lets use vbus-supply then.

Thanks
Kishon

Re: [PATCH 1/7] phy: exynos-usb2: add vbus regulator support

[Marek Szyprowski]

Hello,

On 2015-08-27 11:54, Kishon Vijay Abraham I wrote:
> On Tuesday 25 August 2015 11:42 AM, Krzysztof Kozlowski wrote:
>> On 25.08.2015 14:47, Marek Szyprowski wrote:
>>> Hello,
>>>
>>> On 2015-08-21 14:44, Kishon Vijay Abraham I wrote:
>>>> On Friday 21 August 2015 06:08 PM, Marek Szyprowski wrote:
>>>>> Exynos USB2 PHY has separate power supply, which is usually provided by
>>>>> VBUS regulator. This patch adds support for it. VBUS regulator is
>>>>> optional, to keep compatibility with boards, which have VBUS provided
>>>>> from some always-on power source.
>>>>>
>>>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>>>> ---
>>>>>    .../devicetree/bindings/phy/samsung-phy.txt        |  3 +++
>>>>>    drivers/phy/phy-samsung-usb2.c                     | 25
>>>>> ++++++++++++++++++++--
>>>>>    drivers/phy/phy-samsung-usb2.h                     |  2 ++
>>>>>    3 files changed, 28 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt
>>>>> b/Documentation/devicetree/bindings/phy/samsung-phy.txt
>>>>> index 60c6f2a633e0..0289d3b07853 100644
>>>>> --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
>>>>> +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
>>>>> @@ -44,6 +44,9 @@ Required properties:
>>>>>        - the "ref" clock is used to get the rate of the clock provided
>>>>> to the
>>>>>          PHY module
>>>>>    +Optional properties:
>>>>> +- vbus-supply: power-supply phandle for vbus power source
>>>> how about using phy-supply?
>>> I wanted to make it a bit more descriptive (vbus-supply is rather self
>>> explaining name)
>>> and keep it in line with Exynos usb3-drd phy, which already supports
>>> vbus-supply.
>>> If you think that this is a bad idea, a will use phy-supply then.
>> This is actually supply for VBUS, not for the phy. Using phy-supply
>> would work fine and reduce the size of code... but would be rather a
>> hacky-use of phy and it could be misleading.
>>
>> I don't have strong feeling about this, both ideas have its advantages.
>> If I had to choose than I would like to use vbus-supply because of its
>> correctness with real-world (this is a VBUS after all).
> Alright.. lets use vbus-supply then.

Could you take this patch to -next then? I would like to have all needed 
pieces
merged to v4.4 to finally get USB gadget stuff working again on Trats2 
boards.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

[PATCH 2/7] arm: dts: exynos: add vbus regulator to USB2 phy nodes

[Marek Szyprowski]

Exynos USB2 PHY driver now supports VBUS regulator, so add it to all
boards which have it available. This also fixes commit
7eec1266751bd3a25e35ce88686634c768fedc24 ("ARM: dts: Add Maxim 77693
PMIC to exynos4412-trats2"), which added new regulators to Trats2 board,
but without linking them to the consumers.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/arm/boot/dts/exynos3250-monk.dts           | 1 +
 arch/arm/boot/dts/exynos3250-rinato.dts         | 1 +
 arch/arm/boot/dts/exynos4210-trats.dts          | 2 +-
 arch/arm/boot/dts/exynos4210-universal_c210.dts | 2 +-
 arch/arm/boot/dts/exynos4412-trats2.dts         | 1 +
 5 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/exynos3250-monk.dts b/arch/arm/boot/dts/exynos3250-monk.dts
index 540a0adf2be6..35b39d2255d3 100644
--- a/arch/arm/boot/dts/exynos3250-monk.dts
+++ b/arch/arm/boot/dts/exynos3250-monk.dts
@@ -161,6 +161,7 @@
 };
 
 &exynos_usbphy {
+	vbus-supply = <&safeout_reg>;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/exynos3250-rinato.dts b/arch/arm/boot/dts/exynos3250-rinato.dts
index 41a5fafb9aa9..23623cd3ebd9 100644
--- a/arch/arm/boot/dts/exynos3250-rinato.dts
+++ b/arch/arm/boot/dts/exynos3250-rinato.dts
@@ -153,6 +153,7 @@
 
 &exynos_usbphy {
 	status = "okay";
+	vbus-supply = <&safeout_reg>;
 };
 
 &hsotg {
diff --git a/arch/arm/boot/dts/exynos4210-trats.dts b/arch/arm/boot/dts/exynos4210-trats.dts
index ba34886f8b65..01d38f2145b9 100644
--- a/arch/arm/boot/dts/exynos4210-trats.dts
+++ b/arch/arm/boot/dts/exynos4210-trats.dts
@@ -251,6 +251,7 @@
 
 &exynos_usbphy {
 	status = "okay";
+	vbus-supply = <&safe1_sreg>;
 };
 
 &fimd {
@@ -448,7 +449,6 @@
 
 			safe1_sreg: ESAFEOUT1 {
 			     regulator-name = "SAFEOUT1";
-			     regulator-always-on;
 			};
 
 			safe2_sreg: ESAFEOUT2 {
diff --git a/arch/arm/boot/dts/exynos4210-universal_c210.dts b/arch/arm/boot/dts/exynos4210-universal_c210.dts
index eb379526e234..2c04297825fe 100644
--- a/arch/arm/boot/dts/exynos4210-universal_c210.dts
+++ b/arch/arm/boot/dts/exynos4210-universal_c210.dts
@@ -248,6 +248,7 @@
 
 &exynos_usbphy {
 	status = "okay";
+	vbus-supply = <&safeout1_reg>;
 };
 
 &fimd {
@@ -486,7 +487,6 @@
 
 			safeout1_reg: ESAFEOUT1 {
 				regulator-name = "SAFEOUT1";
-				regulator-always-on;
 			};
 
 			safeout2_reg: ESAFEOUT2 {
diff --git a/arch/arm/boot/dts/exynos4412-trats2.dts b/arch/arm/boot/dts/exynos4412-trats2.dts
index 2a1ebb76ebe0..50a5e8a85283 100644
--- a/arch/arm/boot/dts/exynos4412-trats2.dts
+++ b/arch/arm/boot/dts/exynos4412-trats2.dts
@@ -391,6 +391,7 @@
 };
 
 &exynos_usbphy {
+	vbus-supply = <&esafeout1_reg>;
 	status = "okay";
 };
 
-- 
1.9.2

Re: [PATCH 2/7] arm: dts: exynos: add vbus regulator to USB2 phy nodes

[Krzysztof Kozlowski]

W dniu 21.08.2015 o 21:38, Marek Szyprowski pisze:
> Exynos USB2 PHY driver now supports VBUS regulator, so add it to all
> boards which have it available. This also fixes commit
> 7eec1266751bd3a25e35ce88686634c768fedc24 ("ARM: dts: Add Maxim 77693
> PMIC to exynos4412-trats2"), which added new regulators to Trats2 board,
> but without linking them to the consumers.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>

Hi,

Thanks for taking care of it! Nice analysis.

1. If there would be a resend please change the "arm" to "ARM" in
subject. If not, maybe Kukjin can fix it when applying.

2. How about backporting 1st and 2nd patch to stable kernels? At least
for Trats2? Mentioned commit 7eec126675 introduces a bug (a feature
stops working) which would be nice to fix for stable releases as well.
We could also backport simpler patch, adding only "regulator-always-on".

3. What about safeout regulator #2? On Trats2 it goes to USB_VBUS of
modem. Shouldn't it be enabled?

I'll test the patchset later at work on my boards.

As for the code:
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Best regards,
Krzysztof


> ---
>  arch/arm/boot/dts/exynos3250-monk.dts           | 1 +
>  arch/arm/boot/dts/exynos3250-rinato.dts         | 1 +
>  arch/arm/boot/dts/exynos4210-trats.dts          | 2 +-
>  arch/arm/boot/dts/exynos4210-universal_c210.dts | 2 +-
>  arch/arm/boot/dts/exynos4412-trats2.dts         | 1 +
>  5 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/exynos3250-monk.dts b/arch/arm/boot/dts/exynos3250-monk.dts
> index 540a0adf2be6..35b39d2255d3 100644
> --- a/arch/arm/boot/dts/exynos3250-monk.dts
> +++ b/arch/arm/boot/dts/exynos3250-monk.dts
> @@ -161,6 +161,7 @@
>  };
>  
>  &exynos_usbphy {
> +	vbus-supply = <&safeout_reg>;
>  	status = "okay";
>  };
>  
> diff --git a/arch/arm/boot/dts/exynos3250-rinato.dts b/arch/arm/boot/dts/exynos3250-rinato.dts
> index 41a5fafb9aa9..23623cd3ebd9 100644
> --- a/arch/arm/boot/dts/exynos3250-rinato.dts
> +++ b/arch/arm/boot/dts/exynos3250-rinato.dts
> @@ -153,6 +153,7 @@
>  
>  &exynos_usbphy {
>  	status = "okay";
> +	vbus-supply = <&safeout_reg>;
>  };
>  
>  &hsotg {
> diff --git a/arch/arm/boot/dts/exynos4210-trats.dts b/arch/arm/boot/dts/exynos4210-trats.dts
> index ba34886f8b65..01d38f2145b9 100644
> --- a/arch/arm/boot/dts/exynos4210-trats.dts
> +++ b/arch/arm/boot/dts/exynos4210-trats.dts
> @@ -251,6 +251,7 @@
>  
>  &exynos_usbphy {
>  	status = "okay";
> +	vbus-supply = <&safe1_sreg>;
>  };
>  
>  &fimd {
> @@ -448,7 +449,6 @@
>  
>  			safe1_sreg: ESAFEOUT1 {
>  			     regulator-name = "SAFEOUT1";
> -			     regulator-always-on;
>  			};
>  
>  			safe2_sreg: ESAFEOUT2 {
> diff --git a/arch/arm/boot/dts/exynos4210-universal_c210.dts b/arch/arm/boot/dts/exynos4210-universal_c210.dts
> index eb379526e234..2c04297825fe 100644
> --- a/arch/arm/boot/dts/exynos4210-universal_c210.dts
> +++ b/arch/arm/boot/dts/exynos4210-universal_c210.dts
> @@ -248,6 +248,7 @@
>  
>  &exynos_usbphy {
>  	status = "okay";
> +	vbus-supply = <&safeout1_reg>;
>  };
>  
>  &fimd {
> @@ -486,7 +487,6 @@
>  
>  			safeout1_reg: ESAFEOUT1 {
>  				regulator-name = "SAFEOUT1";
> -				regulator-always-on;
>  			};
>  
>  			safeout2_reg: ESAFEOUT2 {
> diff --git a/arch/arm/boot/dts/exynos4412-trats2.dts b/arch/arm/boot/dts/exynos4412-trats2.dts
> index 2a1ebb76ebe0..50a5e8a85283 100644
> --- a/arch/arm/boot/dts/exynos4412-trats2.dts
> +++ b/arch/arm/boot/dts/exynos4412-trats2.dts
> @@ -391,6 +391,7 @@
>  };
>  
>  &exynos_usbphy {
> +	vbus-supply = <&esafeout1_reg>;
>  	status = "okay";
>  };
>  
> 

[PATCH 3/7] regulators: max77693: register driver earlier to avoid deferred probe

[Marek Szyprowski]

MAX77693 based regulators are used by USB gadget subsystem, which
doesn't support deferred probe, so the driver should be registered
before USB gadget drivers get probed.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/regulator/max77693.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/max77693.c b/drivers/regulator/max77693.c
index 788379b87962..de730fd3f8a5 100644
--- a/drivers/regulator/max77693.c
+++ b/drivers/regulator/max77693.c
@@ -300,7 +300,17 @@ static struct platform_driver max77693_pmic_driver = {
 	.id_table = max77693_pmic_id,
 };
 
-module_platform_driver(max77693_pmic_driver);
+static int __init max77693_pmic_init(void)
+{
+	return platform_driver_register(&max77693_pmic_driver);
+}
+subsys_initcall(max77693_pmic_init);
+
+static void __exit max77693_pmic_cleanup(void)
+{
+	platform_driver_unregister(&max77693_pmic_driver);
+}
+module_exit(max77693_pmic_cleanup);
 
 MODULE_DESCRIPTION("MAXIM 77693/77843 regulator driver");
 MODULE_AUTHOR("Jonghwa Lee <jonghwa3.lee@samsung.com>");
-- 
1.9.2

Re: [PATCH 3/7] regulators: max77693: register driver earlier to avoid deferred probe

[Krzysztof Kozlowski]

W dniu 21.08.2015 o 21:38, Marek Szyprowski pisze:
> MAX77693 based regulators are used by USB gadget subsystem, which
> doesn't support deferred probe, so the driver should be registered
> before USB gadget drivers get probed.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/regulator/max77693.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 

Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Best regards,
Krzysztof

[PATCH 4/7] usb: dwc2: remove double call to s3c_hsotg_of_probe

[Marek Szyprowski]

This patch removes doubled call to s3c_hsotg_of_probe() function.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/usb/dwc2/gadget.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 3ee5b4c77a1f..47960787f572 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3488,8 +3488,6 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
 	/* Set default UTMI width */
 	hsotg->phyif = GUSBCFG_PHYIF16;
 
-	s3c_hsotg_of_probe(hsotg);
-
 	/* Initialize to legacy fifo configuration values */
 	hsotg->g_rx_fifo_sz = 2048;
 	hsotg->g_np_g_tx_fifo_sz = 1024;
-- 
1.9.2

[PATCH 5/7] usb: dwc2: remove non-functional clock gating

[Marek Szyprowski]

During typical gadget operation, dwc2 clock was enabled 3 times: from
dwc2_gadget_init(), s3c_hsotg_udc_start() and s3c_hsotg_pullup(), and then
disabled in s3c_hsotg_pullup(), s3c_hsotg_udc_stop() and
s3c_hsotg_remove(). This really makes no sense, so leave clock control
code only in dwc2_gadget_init/remove functions.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/usb/dwc2/gadget.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 47960787f572..bccb60fcdd70 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3068,8 +3068,6 @@ static int s3c_hsotg_udc_start(struct usb_gadget *gadget,
 	hsotg->gadget.dev.of_node = hsotg->dev->of_node;
 	hsotg->gadget.speed = USB_SPEED_UNKNOWN;
 
-	clk_enable(hsotg->clk);
-
 	ret = regulator_bulk_enable(ARRAY_SIZE(hsotg->supplies),
 				    hsotg->supplies);
 	if (ret) {
@@ -3139,8 +3137,6 @@ static int s3c_hsotg_udc_stop(struct usb_gadget *gadget)
 
 	regulator_bulk_disable(ARRAY_SIZE(hsotg->supplies), hsotg->supplies);
 
-	clk_disable(hsotg->clk);
-
 	mutex_unlock(&hsotg->init_mutex);
 
 	return 0;
@@ -3174,7 +3170,6 @@ static int s3c_hsotg_pullup(struct usb_gadget *gadget, int is_on)
 	mutex_lock(&hsotg->init_mutex);
 	spin_lock_irqsave(&hsotg->lock, flags);
 	if (is_on) {
-		clk_enable(hsotg->clk);
 		hsotg->enabled = 1;
 		s3c_hsotg_core_init_disconnected(hsotg, false);
 		s3c_hsotg_core_connect(hsotg);
@@ -3182,7 +3177,6 @@ static int s3c_hsotg_pullup(struct usb_gadget *gadget, int is_on)
 		s3c_hsotg_core_disconnect(hsotg);
 		s3c_hsotg_disconnect(hsotg);
 		hsotg->enabled = 0;
-		clk_disable(hsotg->clk);
 	}
 
 	hsotg->gadget.speed = USB_SPEED_UNKNOWN;
-- 
1.9.2

[PATCH 6/7] usb: dwc2: fix unbalanced phy control

[Marek Szyprowski]

Even when DWC2 is in (internal) suspended state, it should disable PHY
in suspend and then enable it in resume. This patch fixes unbalanced PHY
control sequence.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/usb/dwc2/platform.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index 90935304185a..dad15ad1ecb4 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -290,8 +290,6 @@ static int __maybe_unused dwc2_suspend(struct device *dev)
 	if (dwc2_is_device_mode(dwc2)) {
 		ret = s3c_hsotg_suspend(dwc2);
 	} else {
-		if (dwc2->lx_state == DWC2_L0)
-			return 0;
 		phy_exit(dwc2->phy);
 		phy_power_off(dwc2->phy);
 
-- 
1.9.2

[PATCH 7/7] usb: dwc2: refactor common low-level hw code to platform.c

[Marek Szyprowski]

DWC2 module on some platforms needs three additional hardware
resources: phy controller, clock and power supply. All of them must be
enabled/activated to properly initialize and operate. This was initially
handled in s3c-hsotg driver, which has been converted to 'gadget' part
of dwc2 driver. Unfortunately, not all of this code got moved to common
platform code, what resulted in accessing DWC2 registers without
enabling low-level hardware resources. This fails for example on Exynos
SoCs. This patch moves all the code for managing those resources to
common platform.c file and provides convenient wrappers for controlling
them.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/usb/dwc2/core.h     |   4 +-
 drivers/usb/dwc2/gadget.c   | 188 +++++-----------------------------------
 drivers/usb/dwc2/platform.c | 207 ++++++++++++++++++++++++++++++++++++--------
 3 files changed, 195 insertions(+), 204 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 0ed87620941b..ec820bdf98c0 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -690,6 +690,7 @@ struct dwc2_hsotg {
 	enum usb_dr_mode dr_mode;
 	unsigned int hcd_enabled:1;
 	unsigned int gadget_enabled:1;
+	unsigned int ll_hw_enabled:1;
 
 	struct phy *phy;
 	struct usb_phy *uphy;
@@ -1088,7 +1089,8 @@ extern void dwc2_set_all_params(struct dwc2_core_params *params, int value);
 
 extern int dwc2_get_hwparams(struct dwc2_hsotg *hsotg);
 
-
+extern int dwc2_lowlevel_hw_enable(struct dwc2_hsotg *hsotg);
+extern int dwc2_lowlevel_hw_disable(struct dwc2_hsotg *hsotg);
 
 /*
  * Dump core registers and SPRAM
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index bccb60fcdd70..98f139be8c7f 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -25,15 +25,11 @@
 #include <linux/delay.h>
 #include <linux/io.h>
 #include <linux/slab.h>
-#include <linux/clk.h>
-#include <linux/regulator/consumer.h>
 #include <linux/of_platform.h>
-#include <linux/phy/phy.h>
 
 #include <linux/usb/ch9.h>
 #include <linux/usb/gadget.h>
 #include <linux/usb/phy.h>
-#include <linux/platform_data/s3c-hsotg.h>
 
 #include "core.h"
 #include "hw.h"
@@ -2944,50 +2940,6 @@ static struct usb_ep_ops s3c_hsotg_ep_ops = {
 };
 
 /**
- * s3c_hsotg_phy_enable - enable platform phy dev
- * @hsotg: The driver state
- *
- * A wrapper for platform code responsible for controlling
- * low-level USB code
- */
-static void s3c_hsotg_phy_enable(struct dwc2_hsotg *hsotg)
-{
-	struct platform_device *pdev = to_platform_device(hsotg->dev);
-
-	dev_dbg(hsotg->dev, "pdev 0x%p\n", pdev);
-
-	if (hsotg->uphy)
-		usb_phy_init(hsotg->uphy);
-	else if (hsotg->plat && hsotg->plat->phy_init)
-		hsotg->plat->phy_init(pdev, hsotg->plat->phy_type);
-	else {
-		phy_init(hsotg->phy);
-		phy_power_on(hsotg->phy);
-	}
-}
-
-/**
- * s3c_hsotg_phy_disable - disable platform phy dev
- * @hsotg: The driver state
- *
- * A wrapper for platform code responsible for controlling
- * low-level USB code
- */
-static void s3c_hsotg_phy_disable(struct dwc2_hsotg *hsotg)
-{
-	struct platform_device *pdev = to_platform_device(hsotg->dev);
-
-	if (hsotg->uphy)
-		usb_phy_shutdown(hsotg->uphy);
-	else if (hsotg->plat && hsotg->plat->phy_exit)
-		hsotg->plat->phy_exit(pdev, hsotg->plat->phy_type);
-	else {
-		phy_power_off(hsotg->phy);
-		phy_exit(hsotg->phy);
-	}
-}
-
-/**
  * s3c_hsotg_init - initalize the usb core
  * @hsotg: The driver state
  */
@@ -3068,14 +3020,13 @@ static int s3c_hsotg_udc_start(struct usb_gadget *gadget,
 	hsotg->gadget.dev.of_node = hsotg->dev->of_node;
 	hsotg->gadget.speed = USB_SPEED_UNKNOWN;
 
-	ret = regulator_bulk_enable(ARRAY_SIZE(hsotg->supplies),
-				    hsotg->supplies);
-	if (ret) {
-		dev_err(hsotg->dev, "failed to enable supplies: %d\n", ret);
-		goto err;
+	if (hsotg->dr_mode == USB_DR_MODE_PERIPHERAL) {
+		ret = dwc2_lowlevel_hw_enable(hsotg);
+		if (ret)
+			goto err;
+		hsotg->ll_hw_enabled = true;
 	}
 
-	s3c_hsotg_phy_enable(hsotg);
 	if (!IS_ERR_OR_NULL(hsotg->uphy))
 		otg_set_peripheral(hsotg->uphy->otg, &hsotg->gadget);
 
@@ -3133,9 +3084,11 @@ static int s3c_hsotg_udc_stop(struct usb_gadget *gadget)
 
 	if (!IS_ERR_OR_NULL(hsotg->uphy))
 		otg_set_peripheral(hsotg->uphy->otg, NULL);
-	s3c_hsotg_phy_disable(hsotg);
 
-	regulator_bulk_disable(ARRAY_SIZE(hsotg->supplies), hsotg->supplies);
+	if (hsotg->dr_mode == USB_DR_MODE_PERIPHERAL) {
+		dwc2_lowlevel_hw_disable(hsotg);
+		hsotg->ll_hw_enabled = false;
+	}
 
 	mutex_unlock(&hsotg->init_mutex);
 
@@ -3473,7 +3426,6 @@ static inline void s3c_hsotg_of_probe(struct dwc2_hsotg *hsotg) { }
 int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
 {
 	struct device *dev = hsotg->dev;
-	struct s3c_hsotg_plat *plat = dev->platform_data;
 	int epnum;
 	int ret;
 	int i;
@@ -3495,19 +3447,8 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
 	for (i = 0; i < MAX_EPS_CHANNELS; i++)
 		dev_dbg(dev, "Periodic TXFIFO%2d size: %d\n", i,
 						hsotg->g_tx_fifo_sz[i]);
-	/*
-	 * If platform probe couldn't find a generic PHY or an old style
-	 * USB PHY, fall back to pdata
-	 */
-	if (IS_ERR_OR_NULL(hsotg->phy) && IS_ERR_OR_NULL(hsotg->uphy)) {
-		plat = dev_get_platdata(dev);
-		if (!plat) {
-			dev_err(dev,
-			"no platform data or transceiver defined\n");
-			return -EPROBE_DEFER;
-		}
-		hsotg->plat = plat;
-	} else if (hsotg->phy) {
+
+	if (hsotg->phy) {
 		/*
 		 * If using the generic PHY framework, check if the PHY bus
 		 * width is 8-bit and set the phyif appropriately.
@@ -3516,50 +3457,12 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
 			hsotg->phyif = GUSBCFG_PHYIF8;
 	}
 
-	hsotg->clk = devm_clk_get(dev, "otg");
-	if (IS_ERR(hsotg->clk)) {
-		hsotg->clk = NULL;
-		dev_dbg(dev, "cannot get otg clock\n");
-	}
-
 	hsotg->gadget.max_speed = USB_SPEED_HIGH;
 	hsotg->gadget.ops = &s3c_hsotg_gadget_ops;
 	hsotg->gadget.name = dev_name(dev);
 	if (hsotg->dr_mode == USB_DR_MODE_OTG)
 		hsotg->gadget.is_otg = 1;
 
-	/* reset the system */
-
-	ret = clk_prepare_enable(hsotg->clk);
-	if (ret) {
-		dev_err(dev, "failed to enable otg clk\n");
-		goto err_clk;
-	}
-
-
-	/* regulators */
-
-	for (i = 0; i < ARRAY_SIZE(hsotg->supplies); i++)
-		hsotg->supplies[i].supply = s3c_hsotg_supply_names[i];
-
-	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(hsotg->supplies),
-				 hsotg->supplies);
-	if (ret) {
-		dev_err(dev, "failed to request supplies: %d\n", ret);
-		goto err_clk;
-	}
-
-	ret = regulator_bulk_enable(ARRAY_SIZE(hsotg->supplies),
-				    hsotg->supplies);
-
-	if (ret) {
-		dev_err(dev, "failed to enable supplies: %d\n", ret);
-		goto err_clk;
-	}
-
-	/* usb phy enable */
-	s3c_hsotg_phy_enable(hsotg);
-
 	/*
 	 * Force Device mode before initialization.
 	 * This allows correctly configuring fifo for device mode.
@@ -3577,7 +3480,7 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
 	ret = s3c_hsotg_hw_cfg(hsotg);
 	if (ret) {
 		dev_err(hsotg->dev, "Hardware configuration failed: %d\n", ret);
-		goto err_clk;
+		return ret;
 	}
 
 	s3c_hsotg_init(hsotg);
@@ -3589,35 +3492,28 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
 			DWC2_CTRL_BUFF_SIZE, GFP_KERNEL);
 	if (!hsotg->ctrl_buff) {
 		dev_err(dev, "failed to allocate ctrl request buff\n");
-		ret = -ENOMEM;
-		goto err_supplies;
+		return -ENOMEM;
 	}
 
 	hsotg->ep0_buff = devm_kzalloc(hsotg->dev,
 			DWC2_CTRL_BUFF_SIZE, GFP_KERNEL);
 	if (!hsotg->ep0_buff) {
 		dev_err(dev, "failed to allocate ctrl reply buff\n");
-		ret = -ENOMEM;
-		goto err_supplies;
+		return -ENOMEM;
 	}
 
 	ret = devm_request_irq(hsotg->dev, irq, s3c_hsotg_irq, IRQF_SHARED,
 				dev_name(hsotg->dev), hsotg);
 	if (ret < 0) {
-		s3c_hsotg_phy_disable(hsotg);
-		clk_disable_unprepare(hsotg->clk);
-		regulator_bulk_disable(ARRAY_SIZE(hsotg->supplies),
-				       hsotg->supplies);
 		dev_err(dev, "cannot claim IRQ for gadget\n");
-		goto err_supplies;
+		return ret;
 	}
 
 	/* hsotg->num_of_eps holds number of EPs other than ep0 */
 
 	if (hsotg->num_of_eps == 0) {
 		dev_err(dev, "wrong number of EPs (zero)\n");
-		ret = -EINVAL;
-		goto err_supplies;
+		return -EINVAL;
 	}
 
 	/* setup endpoint information */
@@ -3631,8 +3527,7 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
 						     GFP_KERNEL);
 	if (!hsotg->ctrl_req) {
 		dev_err(dev, "failed to allocate ctrl req\n");
-		ret = -ENOMEM;
-		goto err_supplies;
+		return -ENOMEM;
 	}
 
 	/* initialise the endpoints now the core has been initialised */
@@ -3645,30 +3540,13 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
 								epnum, 0);
 	}
 
-	/* disable power and clock */
-	s3c_hsotg_phy_disable(hsotg);
-
-	ret = regulator_bulk_disable(ARRAY_SIZE(hsotg->supplies),
-				    hsotg->supplies);
-	if (ret) {
-		dev_err(dev, "failed to disable supplies: %d\n", ret);
-		goto err_supplies;
-	}
-
 	ret = usb_add_gadget_udc(dev, &hsotg->gadget);
 	if (ret)
-		goto err_supplies;
+		return ret;
 
 	s3c_hsotg_dump(hsotg);
 
 	return 0;
-
-err_supplies:
-	s3c_hsotg_phy_disable(hsotg);
-err_clk:
-	clk_disable_unprepare(hsotg->clk);
-
-	return ret;
 }
 
 /**
@@ -3678,7 +3556,6 @@ err_clk:
 int s3c_hsotg_remove(struct dwc2_hsotg *hsotg)
 {
 	usb_del_gadget_udc(&hsotg->gadget);
-	clk_disable_unprepare(hsotg->clk);
 
 	return 0;
 }
@@ -3686,12 +3563,9 @@ int s3c_hsotg_remove(struct dwc2_hsotg *hsotg)
 int s3c_hsotg_suspend(struct dwc2_hsotg *hsotg)
 {
 	unsigned long flags;
-	int ret = 0;
 
 	if (hsotg->lx_state != DWC2_L0)
-		return ret;
-
-	mutex_lock(&hsotg->init_mutex);
+		return 0;
 
 	if (hsotg->driver) {
 		int ep;
@@ -3706,52 +3580,34 @@ int s3c_hsotg_suspend(struct dwc2_hsotg *hsotg)
 		hsotg->gadget.speed = USB_SPEED_UNKNOWN;
 		spin_unlock_irqrestore(&hsotg->lock, flags);
 
-		s3c_hsotg_phy_disable(hsotg);
-
 		for (ep = 0; ep < hsotg->num_of_eps; ep++) {
 			if (hsotg->eps_in[ep])
 				s3c_hsotg_ep_disable(&hsotg->eps_in[ep]->ep);
 			if (hsotg->eps_out[ep])
 				s3c_hsotg_ep_disable(&hsotg->eps_out[ep]->ep);
 		}
-
-		ret = regulator_bulk_disable(ARRAY_SIZE(hsotg->supplies),
-					     hsotg->supplies);
-		clk_disable(hsotg->clk);
 	}
 
-	mutex_unlock(&hsotg->init_mutex);
-
-	return ret;
+	return 0;
 }
 
 int s3c_hsotg_resume(struct dwc2_hsotg *hsotg)
 {
 	unsigned long flags;
-	int ret = 0;
 
 	if (hsotg->lx_state == DWC2_L2)
-		return ret;
-
-	mutex_lock(&hsotg->init_mutex);
+		return 0;
 
 	if (hsotg->driver) {
 		dev_info(hsotg->dev, "resuming usb gadget %s\n",
 			 hsotg->driver->driver.name);
 
-		clk_enable(hsotg->clk);
-		ret = regulator_bulk_enable(ARRAY_SIZE(hsotg->supplies),
-					    hsotg->supplies);
-
-		s3c_hsotg_phy_enable(hsotg);
-
 		spin_lock_irqsave(&hsotg->lock, flags);
 		s3c_hsotg_core_init_disconnected(hsotg, false);
 		if (hsotg->enabled)
 			s3c_hsotg_core_connect(hsotg);
 		spin_unlock_irqrestore(&hsotg->lock, flags);
 	}
-	mutex_unlock(&hsotg->init_mutex);
 
-	return ret;
+	return 0;
 }
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index dad15ad1ecb4..2e55481e030c 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -37,11 +37,14 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/slab.h>
+#include <linux/clk.h>
 #include <linux/device.h>
 #include <linux/dma-mapping.h>
 #include <linux/of_device.h>
 #include <linux/mutex.h>
 #include <linux/platform_device.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_data/s3c-hsotg.h>
 
 #include <linux/usb/of.h>
 
@@ -112,6 +115,114 @@ static const struct dwc2_core_params params_rk3066 = {
 };
 
 /**
+ * dwc2_lowlevel_hw_enable - enable platform lowlevel hw resources
+ * @hsotg: The driver state
+ *
+ * A wrapper for platform code responsible for controlling
+ * low-level USB platform resources (phy, clock, regulators)
+ */
+int dwc2_lowlevel_hw_enable(struct dwc2_hsotg *hsotg)
+{
+	struct platform_device *pdev = to_platform_device(hsotg->dev);
+	int ret;
+
+	ret = regulator_bulk_enable(ARRAY_SIZE(hsotg->supplies),
+				    hsotg->supplies);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(hsotg->clk);
+	if (ret)
+		return ret;
+
+	if (hsotg->uphy)
+		ret = usb_phy_init(hsotg->uphy);
+	else if (hsotg->plat && hsotg->plat->phy_init)
+		ret = hsotg->plat->phy_init(pdev, hsotg->plat->phy_type);
+	else {
+		ret = phy_power_on(hsotg->phy);
+		if (ret == 0)
+			ret = phy_init(hsotg->phy);
+	}
+	return ret;
+}
+
+/**
+ * dwc2_lowlevel_hw_enable - disable platform lowlevel hw resources
+ * @hsotg: The driver state
+ *
+ * A wrapper for platform code responsible for controlling
+ * low-level USB platform resources (phy, clock, regulators)
+ */
+int dwc2_lowlevel_hw_disable(struct dwc2_hsotg *hsotg)
+{
+	struct platform_device *pdev = to_platform_device(hsotg->dev);
+	int ret = 0;
+
+	if (hsotg->uphy)
+		usb_phy_shutdown(hsotg->uphy);
+	else if (hsotg->plat && hsotg->plat->phy_exit)
+		ret = hsotg->plat->phy_exit(pdev, hsotg->plat->phy_type);
+	else {
+		ret = phy_exit(hsotg->phy);
+		if (ret == 0)
+			ret = phy_power_off(hsotg->phy);
+	}
+	if (ret)
+		return ret;
+
+	clk_disable_unprepare(hsotg->clk);
+
+	ret = regulator_bulk_disable(ARRAY_SIZE(hsotg->supplies),
+				     hsotg->supplies);
+
+	return ret;
+}
+
+static int dwc2_lowlevel_hw_init(struct dwc2_hsotg *hsotg)
+{
+	int i, ret;
+	/*
+	 * Attempt to find a generic PHY, then look for an old style
+	 * USB PHY and then fall back to pdata
+	 */
+	hsotg->phy = devm_phy_get(hsotg->dev, "usb2-phy");
+	if (IS_ERR(hsotg->phy)) {
+		hsotg->phy = NULL;
+		hsotg->uphy = devm_usb_get_phy(hsotg->dev, USB_PHY_TYPE_USB2);
+		if (IS_ERR(hsotg->uphy))
+			hsotg->uphy = NULL;
+		else
+			hsotg->plat = dev_get_platdata(hsotg->dev);
+	}
+
+	if (!hsotg->phy && !hsotg->uphy && !hsotg->plat) {
+		dev_err(hsotg->dev, "no platform data or transceiver defined\n");
+		return -EPROBE_DEFER;
+	}
+
+	/* Clock */
+	hsotg->clk = devm_clk_get(hsotg->dev, "otg");
+	if (IS_ERR(hsotg->clk)) {
+		hsotg->clk = NULL;
+		dev_dbg(hsotg->dev, "cannot get otg clock\n");
+	}
+
+	/* Regulators */
+	for (i = 0; i < ARRAY_SIZE(hsotg->supplies); i++)
+		hsotg->supplies[i].supply = s3c_hsotg_supply_names[i];
+
+	ret = devm_regulator_bulk_get(hsotg->dev, ARRAY_SIZE(hsotg->supplies),
+				      hsotg->supplies);
+	if (ret) {
+		dev_err(hsotg->dev, "failed to request supplies: %d\n", ret);
+		return ret;
+	}
+	return 0;
+}
+
+
+/**
  * dwc2_driver_remove() - Called when the DWC_otg core is unregistered with the
  * DWC_otg driver
  *
@@ -126,12 +237,19 @@ static int dwc2_driver_remove(struct platform_device *dev)
 {
 	struct dwc2_hsotg *hsotg = platform_get_drvdata(dev);
 
+	mutex_lock(&hsotg->init_mutex);
+
 	dwc2_debugfs_exit(hsotg);
 	if (hsotg->hcd_enabled)
 		dwc2_hcd_remove(hsotg);
 	if (hsotg->gadget_enabled)
 		s3c_hsotg_remove(hsotg);
 
+	if (hsotg->ll_hw_enabled)
+		dwc2_lowlevel_hw_disable(hsotg);
+
+	mutex_unlock(&hsotg->init_mutex);
+
 	return 0;
 }
 
@@ -163,8 +281,6 @@ static int dwc2_driver_probe(struct platform_device *dev)
 	struct dwc2_core_params defparams;
 	struct dwc2_hsotg *hsotg;
 	struct resource *res;
-	struct phy *phy;
-	struct usb_phy *uphy;
 	int retval;
 	int irq;
 
@@ -222,32 +338,13 @@ static int dwc2_driver_probe(struct platform_device *dev)
 
 	hsotg->dr_mode = of_usb_get_dr_mode(dev->dev.of_node);
 
-	/*
-	 * Attempt to find a generic PHY, then look for an old style
-	 * USB PHY
-	 */
-	phy = devm_phy_get(&dev->dev, "usb2-phy");
-	if (IS_ERR(phy)) {
-		hsotg->phy = NULL;
-		uphy = devm_usb_get_phy(&dev->dev, USB_PHY_TYPE_USB2);
-		if (IS_ERR(uphy))
-			hsotg->uphy = NULL;
-		else
-			hsotg->uphy = uphy;
-	} else {
-		hsotg->phy = phy;
-		phy_power_on(hsotg->phy);
-		phy_init(hsotg->phy);
-	}
+	retval = dwc2_lowlevel_hw_init(hsotg);
+	if (retval)
+		return retval;
 
 	spin_lock_init(&hsotg->lock);
 	mutex_init(&hsotg->init_mutex);
 
-	/* Detect config values from hardware */
-	retval = dwc2_get_hwparams(hsotg);
-	if (retval)
-		return retval;
-
 	hsotg->core_params = devm_kzalloc(&dev->dev,
 				sizeof(*hsotg->core_params), GFP_KERNEL);
 	if (!hsotg->core_params)
@@ -255,13 +352,25 @@ static int dwc2_driver_probe(struct platform_device *dev)
 
 	dwc2_set_all_params(hsotg->core_params, -1);
 
+	retval = dwc2_lowlevel_hw_enable(hsotg);
+	if (retval)
+		return retval;
+	hsotg->ll_hw_enabled = true;
+
+	/* Detect config values from hardware */
+	retval = dwc2_get_hwparams(hsotg);
+	if (retval)
+		goto error;
+
 	/* Validate parameter values */
 	dwc2_set_parameters(hsotg, params);
 
+	mutex_lock(&hsotg->init_mutex);
+
 	if (hsotg->dr_mode != USB_DR_MODE_HOST) {
 		retval = dwc2_gadget_init(hsotg, irq);
 		if (retval)
-			return retval;
+			goto err_unlock;
 		hsotg->gadget_enabled = 1;
 	}
 
@@ -270,7 +379,7 @@ static int dwc2_driver_probe(struct platform_device *dev)
 		if (retval) {
 			if (hsotg->gadget_enabled)
 				s3c_hsotg_remove(hsotg);
-			return retval;
+			goto err_unlock;
 		}
 		hsotg->hcd_enabled = 1;
 	}
@@ -279,6 +388,21 @@ static int dwc2_driver_probe(struct platform_device *dev)
 
 	dwc2_debugfs_init(hsotg);
 
+	/* Gadget code manages lowlevel hw on its own */
+	if (hsotg->dr_mode == USB_DR_MODE_PERIPHERAL) {
+		dwc2_lowlevel_hw_disable(hsotg);
+		hsotg->ll_hw_enabled = false;
+	}
+
+	mutex_unlock(&hsotg->init_mutex);
+
+	return 0;
+
+err_unlock:
+	mutex_unlock(&hsotg->init_mutex);
+error:
+	dwc2_lowlevel_hw_disable(hsotg);
+	hsotg->ll_hw_enabled = false;
 	return retval;
 }
 
@@ -287,13 +411,16 @@ static int __maybe_unused dwc2_suspend(struct device *dev)
 	struct dwc2_hsotg *dwc2 = dev_get_drvdata(dev);
 	int ret = 0;
 
-	if (dwc2_is_device_mode(dwc2)) {
-		ret = s3c_hsotg_suspend(dwc2);
-	} else {
-		phy_exit(dwc2->phy);
-		phy_power_off(dwc2->phy);
+	mutex_lock(&dwc2->init_mutex);
+
+	if (dwc2_is_device_mode(dwc2))
+		s3c_hsotg_suspend(dwc2);
+
+	if (dwc2->ll_hw_enabled)
+		ret = dwc2_lowlevel_hw_disable(dwc2);
+
+	mutex_unlock(&dwc2->init_mutex);
 
-	}
 	return ret;
 }
 
@@ -302,13 +429,19 @@ static int __maybe_unused dwc2_resume(struct device *dev)
 	struct dwc2_hsotg *dwc2 = dev_get_drvdata(dev);
 	int ret = 0;
 
-	if (dwc2_is_device_mode(dwc2)) {
-		ret = s3c_hsotg_resume(dwc2);
-	} else {
-		phy_power_on(dwc2->phy);
-		phy_init(dwc2->phy);
+	mutex_lock(&dwc2->init_mutex);
 
+	if (dwc2->ll_hw_enabled) {
+		ret = dwc2_lowlevel_hw_enable(dwc2);
+		if (ret)
+			goto err;
 	}
+
+	if (dwc2_is_device_mode(dwc2))
+		ret = s3c_hsotg_resume(dwc2);
+err:
+	mutex_unlock(&dwc2->init_mutex);
+
 	return ret;
 }
 
-- 
1.9.2

Re: [PATCH 7/7] usb: dwc2: refactor common low-level hw code to platform.c

[John Youn]

On 8/21/2015 5:39 AM, Marek Szyprowski wrote:
> DWC2 module on some platforms needs three additional hardware
> resources: phy controller, clock and power supply. All of them must be
> enabled/activated to properly initialize and operate. This was initially
> handled in s3c-hsotg driver, which has been converted to 'gadget' part
> of dwc2 driver. Unfortunately, not all of this code got moved to common
> platform code, what resulted in accessing DWC2 registers without
> enabling low-level hardware resources. This fails for example on Exynos
> SoCs. This patch moves all the code for managing those resources to
> common platform.c file and provides convenient wrappers for controlling
> them.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/usb/dwc2/core.h     |   4 +-
>  drivers/usb/dwc2/gadget.c   | 188 +++++-----------------------------------
>  drivers/usb/dwc2/platform.c | 207 ++++++++++++++++++++++++++++++++++++--------
>  3 files changed, 195 insertions(+), 204 deletions(-)
> 
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index 0ed87620941b..ec820bdf98c0 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -690,6 +690,7 @@ struct dwc2_hsotg {
>  	enum usb_dr_mode dr_mode;
>  	unsigned int hcd_enabled:1;
>  	unsigned int gadget_enabled:1;
> +	unsigned int ll_hw_enabled:1;

It seems like this flag is used to restore the state of the
lowlevel_hw on suspend/resume?

I think it is clearer to put the setting/clearing of the flag
inside the ll_hw_enable/disable and then check and adjust on
suspend/resume. That way you don't have to manage this flag
everywhere you call enable/disable().

John

[PATCH v2 7/7] usb: dwc2: refactor common low-level hw code to platform.c

[Marek Szyprowski]

DWC2 module on some platforms needs three additional hardware
resources: phy controller, clock and power supply. All of them must be
enabled/activated to properly initialize and operate. This was initially
handled in s3c-hsotg driver, which has been converted to 'gadget' part
of dwc2 driver. Unfortunately, not all of this code got moved to common
platform code, what resulted in accessing DWC2 registers without
enabling low-level hardware resources. This fails for example on Exynos
SoCs. This patch moves all the code for managing those resources to
common platform.c file and provides convenient wrappers for controlling
them.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
Changelog:
v2:
- moved setting of ll_hw_enabled flag to enable/disable functions, as
  suggested by John Youn
- moved setting of phy width to dwc2_lowlevel_init function
---
 drivers/usb/dwc2/core.h     |   4 +-
 drivers/usb/dwc2/gadget.c   | 193 ++++--------------------------------
 drivers/usb/dwc2/platform.c | 234 +++++++++++++++++++++++++++++++++++++-------
 3 files changed, 217 insertions(+), 214 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 0ed87620941b..ec820bdf98c0 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -690,6 +690,7 @@ struct dwc2_hsotg {
 	enum usb_dr_mode dr_mode;
 	unsigned int hcd_enabled:1;
 	unsigned int gadget_enabled:1;
+	unsigned int ll_hw_enabled:1;
 
 	struct phy *phy;
 	struct usb_phy *uphy;
@@ -1088,7 +1089,8 @@ extern void dwc2_set_all_params(struct dwc2_core_params *params, int value);
 
 extern int dwc2_get_hwparams(struct dwc2_hsotg *hsotg);
 
-
+extern int dwc2_lowlevel_hw_enable(struct dwc2_hsotg *hsotg);
+extern int dwc2_lowlevel_hw_disable(struct dwc2_hsotg *hsotg);
 
 /*
  * Dump core registers and SPRAM
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index bccb60fcdd70..3685aae39885 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -25,15 +25,11 @@
 #include <linux/delay.h>
 #include <linux/io.h>
 #include <linux/slab.h>
-#include <linux/clk.h>
-#include <linux/regulator/consumer.h>
 #include <linux/of_platform.h>
-#include <linux/phy/phy.h>
 
 #include <linux/usb/ch9.h>
 #include <linux/usb/gadget.h>
 #include <linux/usb/phy.h>
-#include <linux/platform_data/s3c-hsotg.h>
 
 #include "core.h"
 #include "hw.h"
@@ -2944,50 +2940,6 @@ static struct usb_ep_ops s3c_hsotg_ep_ops = {
 };
 
 /**
- * s3c_hsotg_phy_enable - enable platform phy dev
- * @hsotg: The driver state
- *
- * A wrapper for platform code responsible for controlling
- * low-level USB code
- */
-static void s3c_hsotg_phy_enable(struct dwc2_hsotg *hsotg)
-{
-	struct platform_device *pdev = to_platform_device(hsotg->dev);
-
-	dev_dbg(hsotg->dev, "pdev 0x%p\n", pdev);
-
-	if (hsotg->uphy)
-		usb_phy_init(hsotg->uphy);
-	else if (hsotg->plat && hsotg->plat->phy_init)
-		hsotg->plat->phy_init(pdev, hsotg->plat->phy_type);
-	else {
-		phy_init(hsotg->phy);
-		phy_power_on(hsotg->phy);
-	}
-}
-
-/**
- * s3c_hsotg_phy_disable - disable platform phy dev
- * @hsotg: The driver state
- *
- * A wrapper for platform code responsible for controlling
- * low-level USB code
- */
-static void s3c_hsotg_phy_disable(struct dwc2_hsotg *hsotg)
-{
-	struct platform_device *pdev = to_platform_device(hsotg->dev);
-
-	if (hsotg->uphy)
-		usb_phy_shutdown(hsotg->uphy);
-	else if (hsotg->plat && hsotg->plat->phy_exit)
-		hsotg->plat->phy_exit(pdev, hsotg->plat->phy_type);
-	else {
-		phy_power_off(hsotg->phy);
-		phy_exit(hsotg->phy);
-	}
-}
-
-/**
  * s3c_hsotg_init - initalize the usb core
  * @hsotg: The driver state
  */
@@ -3068,14 +3020,12 @@ static int s3c_hsotg_udc_start(struct usb_gadget *gadget,
 	hsotg->gadget.dev.of_node = hsotg->dev->of_node;
 	hsotg->gadget.speed = USB_SPEED_UNKNOWN;
 
-	ret = regulator_bulk_enable(ARRAY_SIZE(hsotg->supplies),
-				    hsotg->supplies);
-	if (ret) {
-		dev_err(hsotg->dev, "failed to enable supplies: %d\n", ret);
-		goto err;
+	if (hsotg->dr_mode == USB_DR_MODE_PERIPHERAL) {
+		ret = dwc2_lowlevel_hw_enable(hsotg);
+		if (ret)
+			goto err;
 	}
 
-	s3c_hsotg_phy_enable(hsotg);
 	if (!IS_ERR_OR_NULL(hsotg->uphy))
 		otg_set_peripheral(hsotg->uphy->otg, &hsotg->gadget);
 
@@ -3133,9 +3083,9 @@ static int s3c_hsotg_udc_stop(struct usb_gadget *gadget)
 
 	if (!IS_ERR_OR_NULL(hsotg->uphy))
 		otg_set_peripheral(hsotg->uphy->otg, NULL);
-	s3c_hsotg_phy_disable(hsotg);
 
-	regulator_bulk_disable(ARRAY_SIZE(hsotg->supplies), hsotg->supplies);
+	if (hsotg->dr_mode == USB_DR_MODE_PERIPHERAL)
+		dwc2_lowlevel_hw_disable(hsotg);
 
 	mutex_unlock(&hsotg->init_mutex);
 
@@ -3473,15 +3423,11 @@ static inline void s3c_hsotg_of_probe(struct dwc2_hsotg *hsotg) { }
 int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
 {
 	struct device *dev = hsotg->dev;
-	struct s3c_hsotg_plat *plat = dev->platform_data;
 	int epnum;
 	int ret;
 	int i;
 	u32 p_tx_fifo[] = DWC2_G_P_LEGACY_TX_FIFO_SIZE;
 
-	/* Set default UTMI width */
-	hsotg->phyif = GUSBCFG_PHYIF16;
-
 	/* Initialize to legacy fifo configuration values */
 	hsotg->g_rx_fifo_sz = 2048;
 	hsotg->g_np_g_tx_fifo_sz = 1024;
@@ -3495,32 +3441,6 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
 	for (i = 0; i < MAX_EPS_CHANNELS; i++)
 		dev_dbg(dev, "Periodic TXFIFO%2d size: %d\n", i,
 						hsotg->g_tx_fifo_sz[i]);
-	/*
-	 * If platform probe couldn't find a generic PHY or an old style
-	 * USB PHY, fall back to pdata
-	 */
-	if (IS_ERR_OR_NULL(hsotg->phy) && IS_ERR_OR_NULL(hsotg->uphy)) {
-		plat = dev_get_platdata(dev);
-		if (!plat) {
-			dev_err(dev,
-			"no platform data or transceiver defined\n");
-			return -EPROBE_DEFER;
-		}
-		hsotg->plat = plat;
-	} else if (hsotg->phy) {
-		/*
-		 * If using the generic PHY framework, check if the PHY bus
-		 * width is 8-bit and set the phyif appropriately.
-		 */
-		if (phy_get_bus_width(hsotg->phy) == 8)
-			hsotg->phyif = GUSBCFG_PHYIF8;
-	}
-
-	hsotg->clk = devm_clk_get(dev, "otg");
-	if (IS_ERR(hsotg->clk)) {
-		hsotg->clk = NULL;
-		dev_dbg(dev, "cannot get otg clock\n");
-	}
 
 	hsotg->gadget.max_speed = USB_SPEED_HIGH;
 	hsotg->gadget.ops = &s3c_hsotg_gadget_ops;
@@ -3528,38 +3448,6 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
 	if (hsotg->dr_mode == USB_DR_MODE_OTG)
 		hsotg->gadget.is_otg = 1;
 
-	/* reset the system */
-
-	ret = clk_prepare_enable(hsotg->clk);
-	if (ret) {
-		dev_err(dev, "failed to enable otg clk\n");
-		goto err_clk;
-	}
-
-
-	/* regulators */
-
-	for (i = 0; i < ARRAY_SIZE(hsotg->supplies); i++)
-		hsotg->supplies[i].supply = s3c_hsotg_supply_names[i];
-
-	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(hsotg->supplies),
-				 hsotg->supplies);
-	if (ret) {
-		dev_err(dev, "failed to request supplies: %d\n", ret);
-		goto err_clk;
-	}
-
-	ret = regulator_bulk_enable(ARRAY_SIZE(hsotg->supplies),
-				    hsotg->supplies);
-
-	if (ret) {
-		dev_err(dev, "failed to enable supplies: %d\n", ret);
-		goto err_clk;
-	}
-
-	/* usb phy enable */
-	s3c_hsotg_phy_enable(hsotg);
-
 	/*
 	 * Force Device mode before initialization.
 	 * This allows correctly configuring fifo for device mode.
@@ -3577,7 +3465,7 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
 	ret = s3c_hsotg_hw_cfg(hsotg);
 	if (ret) {
 		dev_err(hsotg->dev, "Hardware configuration failed: %d\n", ret);
-		goto err_clk;
+		return ret;
 	}
 
 	s3c_hsotg_init(hsotg);
@@ -3589,35 +3477,28 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
 			DWC2_CTRL_BUFF_SIZE, GFP_KERNEL);
 	if (!hsotg->ctrl_buff) {
 		dev_err(dev, "failed to allocate ctrl request buff\n");
-		ret = -ENOMEM;
-		goto err_supplies;
+		return -ENOMEM;
 	}
 
 	hsotg->ep0_buff = devm_kzalloc(hsotg->dev,
 			DWC2_CTRL_BUFF_SIZE, GFP_KERNEL);
 	if (!hsotg->ep0_buff) {
 		dev_err(dev, "failed to allocate ctrl reply buff\n");
-		ret = -ENOMEM;
-		goto err_supplies;
+		return -ENOMEM;
 	}
 
 	ret = devm_request_irq(hsotg->dev, irq, s3c_hsotg_irq, IRQF_SHARED,
 				dev_name(hsotg->dev), hsotg);
 	if (ret < 0) {
-		s3c_hsotg_phy_disable(hsotg);
-		clk_disable_unprepare(hsotg->clk);
-		regulator_bulk_disable(ARRAY_SIZE(hsotg->supplies),
-				       hsotg->supplies);
 		dev_err(dev, "cannot claim IRQ for gadget\n");
-		goto err_supplies;
+		return ret;
 	}
 
 	/* hsotg->num_of_eps holds number of EPs other than ep0 */
 
 	if (hsotg->num_of_eps == 0) {
 		dev_err(dev, "wrong number of EPs (zero)\n");
-		ret = -EINVAL;
-		goto err_supplies;
+		return -EINVAL;
 	}
 
 	/* setup endpoint information */
@@ -3631,8 +3512,7 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
 						     GFP_KERNEL);
 	if (!hsotg->ctrl_req) {
 		dev_err(dev, "failed to allocate ctrl req\n");
-		ret = -ENOMEM;
-		goto err_supplies;
+		return -ENOMEM;
 	}
 
 	/* initialise the endpoints now the core has been initialised */
@@ -3645,30 +3525,13 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
 								epnum, 0);
 	}
 
-	/* disable power and clock */
-	s3c_hsotg_phy_disable(hsotg);
-
-	ret = regulator_bulk_disable(ARRAY_SIZE(hsotg->supplies),
-				    hsotg->supplies);
-	if (ret) {
-		dev_err(dev, "failed to disable supplies: %d\n", ret);
-		goto err_supplies;
-	}
-
 	ret = usb_add_gadget_udc(dev, &hsotg->gadget);
 	if (ret)
-		goto err_supplies;
+		return ret;
 
 	s3c_hsotg_dump(hsotg);
 
 	return 0;
-
-err_supplies:
-	s3c_hsotg_phy_disable(hsotg);
-err_clk:
-	clk_disable_unprepare(hsotg->clk);
-
-	return ret;
 }
 
 /**
@@ -3678,7 +3541,6 @@ err_clk:
 int s3c_hsotg_remove(struct dwc2_hsotg *hsotg)
 {
 	usb_del_gadget_udc(&hsotg->gadget);
-	clk_disable_unprepare(hsotg->clk);
 
 	return 0;
 }
@@ -3686,12 +3548,9 @@ int s3c_hsotg_remove(struct dwc2_hsotg *hsotg)
 int s3c_hsotg_suspend(struct dwc2_hsotg *hsotg)
 {
 	unsigned long flags;
-	int ret = 0;
 
 	if (hsotg->lx_state != DWC2_L0)
-		return ret;
-
-	mutex_lock(&hsotg->init_mutex);
+		return 0;
 
 	if (hsotg->driver) {
 		int ep;
@@ -3706,52 +3565,34 @@ int s3c_hsotg_suspend(struct dwc2_hsotg *hsotg)
 		hsotg->gadget.speed = USB_SPEED_UNKNOWN;
 		spin_unlock_irqrestore(&hsotg->lock, flags);
 
-		s3c_hsotg_phy_disable(hsotg);
-
 		for (ep = 0; ep < hsotg->num_of_eps; ep++) {
 			if (hsotg->eps_in[ep])
 				s3c_hsotg_ep_disable(&hsotg->eps_in[ep]->ep);
 			if (hsotg->eps_out[ep])
 				s3c_hsotg_ep_disable(&hsotg->eps_out[ep]->ep);
 		}
-
-		ret = regulator_bulk_disable(ARRAY_SIZE(hsotg->supplies),
-					     hsotg->supplies);
-		clk_disable(hsotg->clk);
 	}
 
-	mutex_unlock(&hsotg->init_mutex);
-
-	return ret;
+	return 0;
 }
 
 int s3c_hsotg_resume(struct dwc2_hsotg *hsotg)
 {
 	unsigned long flags;
-	int ret = 0;
 
 	if (hsotg->lx_state == DWC2_L2)
-		return ret;
-
-	mutex_lock(&hsotg->init_mutex);
+		return 0;
 
 	if (hsotg->driver) {
 		dev_info(hsotg->dev, "resuming usb gadget %s\n",
 			 hsotg->driver->driver.name);
 
-		clk_enable(hsotg->clk);
-		ret = regulator_bulk_enable(ARRAY_SIZE(hsotg->supplies),
-					    hsotg->supplies);
-
-		s3c_hsotg_phy_enable(hsotg);
-
 		spin_lock_irqsave(&hsotg->lock, flags);
 		s3c_hsotg_core_init_disconnected(hsotg, false);
 		if (hsotg->enabled)
 			s3c_hsotg_core_connect(hsotg);
 		spin_unlock_irqrestore(&hsotg->lock, flags);
 	}
-	mutex_unlock(&hsotg->init_mutex);
 
-	return ret;
+	return 0;
 }
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index dad15ad1ecb4..159c3ccd2b3f 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -37,11 +37,14 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/slab.h>
+#include <linux/clk.h>
 #include <linux/device.h>
 #include <linux/dma-mapping.h>
 #include <linux/of_device.h>
 #include <linux/mutex.h>
 #include <linux/platform_device.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_data/s3c-hsotg.h>
 
 #include <linux/usb/of.h>
 
@@ -111,6 +114,145 @@ static const struct dwc2_core_params params_rk3066 = {
 	.hibernation			= -1,
 };
 
+static int __dwc2_lowlevel_hw_enable(struct dwc2_hsotg *hsotg)
+{
+	struct platform_device *pdev = to_platform_device(hsotg->dev);
+	int ret;
+
+	ret = regulator_bulk_enable(ARRAY_SIZE(hsotg->supplies),
+				    hsotg->supplies);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(hsotg->clk);
+	if (ret)
+		return ret;
+
+	if (hsotg->uphy)
+		ret = usb_phy_init(hsotg->uphy);
+	else if (hsotg->plat && hsotg->plat->phy_init)
+		ret = hsotg->plat->phy_init(pdev, hsotg->plat->phy_type);
+	else {
+		ret = phy_power_on(hsotg->phy);
+		if (ret == 0)
+			ret = phy_init(hsotg->phy);
+	}
+
+	return ret;
+}
+
+/**
+ * dwc2_lowlevel_hw_enable - enable platform lowlevel hw resources
+ * @hsotg: The driver state
+ *
+ * A wrapper for platform code responsible for controlling
+ * low-level USB platform resources (phy, clock, regulators)
+ */
+int dwc2_lowlevel_hw_enable(struct dwc2_hsotg *hsotg)
+{
+	int ret = __dwc2_lowlevel_hw_enable(hsotg);
+
+	if (ret == 0)
+		hsotg->ll_hw_enabled = true;
+	return ret;
+}
+
+static int __dwc2_lowlevel_hw_disable(struct dwc2_hsotg *hsotg)
+{
+	struct platform_device *pdev = to_platform_device(hsotg->dev);
+	int ret = 0;
+
+	if (hsotg->uphy)
+		usb_phy_shutdown(hsotg->uphy);
+	else if (hsotg->plat && hsotg->plat->phy_exit)
+		ret = hsotg->plat->phy_exit(pdev, hsotg->plat->phy_type);
+	else {
+		ret = phy_exit(hsotg->phy);
+		if (ret == 0)
+			ret = phy_power_off(hsotg->phy);
+	}
+	if (ret)
+		return ret;
+
+	clk_disable_unprepare(hsotg->clk);
+
+	ret = regulator_bulk_disable(ARRAY_SIZE(hsotg->supplies),
+				     hsotg->supplies);
+
+	return ret;
+}
+
+/**
+ * dwc2_lowlevel_hw_disable - disable platform lowlevel hw resources
+ * @hsotg: The driver state
+ *
+ * A wrapper for platform code responsible for controlling
+ * low-level USB platform resources (phy, clock, regulators)
+ */
+int dwc2_lowlevel_hw_disable(struct dwc2_hsotg *hsotg)
+{
+	int ret = __dwc2_lowlevel_hw_disable(hsotg);
+
+	if (ret == 0)
+		hsotg->ll_hw_enabled = false;
+	return ret;
+}
+
+static int dwc2_lowlevel_hw_init(struct dwc2_hsotg *hsotg)
+{
+	int i, ret;
+
+	/* Set default UTMI width */
+	hsotg->phyif = GUSBCFG_PHYIF16;
+
+	/*
+	 * Attempt to find a generic PHY, then look for an old style
+	 * USB PHY and then fall back to pdata
+	 */
+	hsotg->phy = devm_phy_get(hsotg->dev, "usb2-phy");
+	if (IS_ERR(hsotg->phy)) {
+		hsotg->phy = NULL;
+		hsotg->uphy = devm_usb_get_phy(hsotg->dev, USB_PHY_TYPE_USB2);
+		if (IS_ERR(hsotg->uphy))
+			hsotg->uphy = NULL;
+		else
+			hsotg->plat = dev_get_platdata(hsotg->dev);
+	}
+
+	if (hsotg->phy) {
+		/*
+		 * If using the generic PHY framework, check if the PHY bus
+		 * width is 8-bit and set the phyif appropriately.
+		 */
+		if (phy_get_bus_width(hsotg->phy) == 8)
+			hsotg->phyif = GUSBCFG_PHYIF8;
+	}
+
+	if (!hsotg->phy && !hsotg->uphy && !hsotg->plat) {
+		dev_err(hsotg->dev, "no platform data or transceiver defined\n");
+		return -EPROBE_DEFER;
+	}
+
+	/* Clock */
+	hsotg->clk = devm_clk_get(hsotg->dev, "otg");
+	if (IS_ERR(hsotg->clk)) {
+		hsotg->clk = NULL;
+		dev_dbg(hsotg->dev, "cannot get otg clock\n");
+	}
+
+	/* Regulators */
+	for (i = 0; i < ARRAY_SIZE(hsotg->supplies); i++)
+		hsotg->supplies[i].supply = s3c_hsotg_supply_names[i];
+
+	ret = devm_regulator_bulk_get(hsotg->dev, ARRAY_SIZE(hsotg->supplies),
+				      hsotg->supplies);
+	if (ret) {
+		dev_err(hsotg->dev, "failed to request supplies: %d\n", ret);
+		return ret;
+	}
+	return 0;
+}
+
 /**
  * dwc2_driver_remove() - Called when the DWC_otg core is unregistered with the
  * DWC_otg driver
@@ -126,12 +268,19 @@ static int dwc2_driver_remove(struct platform_device *dev)
 {
 	struct dwc2_hsotg *hsotg = platform_get_drvdata(dev);
 
+	mutex_lock(&hsotg->init_mutex);
+
 	dwc2_debugfs_exit(hsotg);
 	if (hsotg->hcd_enabled)
 		dwc2_hcd_remove(hsotg);
 	if (hsotg->gadget_enabled)
 		s3c_hsotg_remove(hsotg);
 
+	if (hsotg->ll_hw_enabled)
+		dwc2_lowlevel_hw_disable(hsotg);
+
+	mutex_unlock(&hsotg->init_mutex);
+
 	return 0;
 }
 
@@ -163,8 +312,6 @@ static int dwc2_driver_probe(struct platform_device *dev)
 	struct dwc2_core_params defparams;
 	struct dwc2_hsotg *hsotg;
 	struct resource *res;
-	struct phy *phy;
-	struct usb_phy *uphy;
 	int retval;
 	int irq;
 
@@ -222,32 +369,13 @@ static int dwc2_driver_probe(struct platform_device *dev)
 
 	hsotg->dr_mode = of_usb_get_dr_mode(dev->dev.of_node);
 
-	/*
-	 * Attempt to find a generic PHY, then look for an old style
-	 * USB PHY
-	 */
-	phy = devm_phy_get(&dev->dev, "usb2-phy");
-	if (IS_ERR(phy)) {
-		hsotg->phy = NULL;
-		uphy = devm_usb_get_phy(&dev->dev, USB_PHY_TYPE_USB2);
-		if (IS_ERR(uphy))
-			hsotg->uphy = NULL;
-		else
-			hsotg->uphy = uphy;
-	} else {
-		hsotg->phy = phy;
-		phy_power_on(hsotg->phy);
-		phy_init(hsotg->phy);
-	}
+	retval = dwc2_lowlevel_hw_init(hsotg);
+	if (retval)
+		return retval;
 
 	spin_lock_init(&hsotg->lock);
 	mutex_init(&hsotg->init_mutex);
 
-	/* Detect config values from hardware */
-	retval = dwc2_get_hwparams(hsotg);
-	if (retval)
-		return retval;
-
 	hsotg->core_params = devm_kzalloc(&dev->dev,
 				sizeof(*hsotg->core_params), GFP_KERNEL);
 	if (!hsotg->core_params)
@@ -255,13 +383,24 @@ static int dwc2_driver_probe(struct platform_device *dev)
 
 	dwc2_set_all_params(hsotg->core_params, -1);
 
+	retval = dwc2_lowlevel_hw_enable(hsotg);
+	if (retval)
+		return retval;
+
+	/* Detect config values from hardware */
+	retval = dwc2_get_hwparams(hsotg);
+	if (retval)
+		goto error;
+
 	/* Validate parameter values */
 	dwc2_set_parameters(hsotg, params);
 
+	mutex_lock(&hsotg->init_mutex);
+
 	if (hsotg->dr_mode != USB_DR_MODE_HOST) {
 		retval = dwc2_gadget_init(hsotg, irq);
 		if (retval)
-			return retval;
+			goto err_unlock;
 		hsotg->gadget_enabled = 1;
 	}
 
@@ -270,7 +409,7 @@ static int dwc2_driver_probe(struct platform_device *dev)
 		if (retval) {
 			if (hsotg->gadget_enabled)
 				s3c_hsotg_remove(hsotg);
-			return retval;
+			goto err_unlock;
 		}
 		hsotg->hcd_enabled = 1;
 	}
@@ -279,6 +418,18 @@ static int dwc2_driver_probe(struct platform_device *dev)
 
 	dwc2_debugfs_init(hsotg);
 
+	/* Gadget code manages lowlevel hw on its own */
+	if (hsotg->dr_mode == USB_DR_MODE_PERIPHERAL)
+		dwc2_lowlevel_hw_disable(hsotg);
+
+	mutex_unlock(&hsotg->init_mutex);
+
+	return 0;
+
+err_unlock:
+	mutex_unlock(&hsotg->init_mutex);
+error:
+	dwc2_lowlevel_hw_disable(hsotg);
 	return retval;
 }
 
@@ -287,13 +438,16 @@ static int __maybe_unused dwc2_suspend(struct device *dev)
 	struct dwc2_hsotg *dwc2 = dev_get_drvdata(dev);
 	int ret = 0;
 
-	if (dwc2_is_device_mode(dwc2)) {
-		ret = s3c_hsotg_suspend(dwc2);
-	} else {
-		phy_exit(dwc2->phy);
-		phy_power_off(dwc2->phy);
+	mutex_lock(&dwc2->init_mutex);
+
+	if (dwc2_is_device_mode(dwc2))
+		s3c_hsotg_suspend(dwc2);
+
+	if (dwc2->ll_hw_enabled)
+		ret = __dwc2_lowlevel_hw_disable(dwc2);
+
+	mutex_unlock(&dwc2->init_mutex);
 
-	}
 	return ret;
 }
 
@@ -302,13 +456,19 @@ static int __maybe_unused dwc2_resume(struct device *dev)
 	struct dwc2_hsotg *dwc2 = dev_get_drvdata(dev);
 	int ret = 0;
 
-	if (dwc2_is_device_mode(dwc2)) {
-		ret = s3c_hsotg_resume(dwc2);
-	} else {
-		phy_power_on(dwc2->phy);
-		phy_init(dwc2->phy);
+	mutex_lock(&dwc2->init_mutex);
 
+	if (dwc2->ll_hw_enabled) {
+		ret = __dwc2_lowlevel_hw_enable(dwc2);
+		if (ret)
+			goto err;
 	}
+
+	if (dwc2_is_device_mode(dwc2))
+		ret = s3c_hsotg_resume(dwc2);
+err:
+	mutex_unlock(&dwc2->init_mutex);
+
 	return ret;
 }
 
-- 
1.9.2

Re: [PATCH 0/7] Exynos4412-based Trats2 USB gadget (DWC2) fixes

[Krzysztof Kozlowski]

On 21.08.2015 21:38, Marek Szyprowski wrote:
> Dear All,
> 
> Since v3.19 s3c-hsotg (DWC2) USB controller stopped working on
> Exynos4412-based Trats2 platform. However on Odroid-U3 (which is also
> Exynos4412-based) it worked fine all the time. After long investigation
> it turned out that this was caused by 2 independent issues.
> 
> First issue was caused by patch 7eec1266751bd3a25e35ce88686634c768fedc24
> ("ARM: dts: Add Maxim 77693 PMIC to exynos4412-trats2") added support
> for Maxim 77693 regulators, but without defining consumers for them.
> This causes regulator core to disable them. However SAFEOUT1 regulator
> provides power supply to VBUS signal, which also power some USB phy
> related parts of Exynos SoC core. This has been fixed by patches 1-3,
> which adds support for VBUS regulator, defines them in DTS for all
> Exynos platforms and changes probe sequence of the drivers to ensure no
> deferred probe occurs (USB gadget subsystem doesn't support deferred
> probe yet).
> 
> Second issue is related to DWC2 driver rewrite and host/gadget/dual-role
> integration code. DWC2 module on some platforms needs three additional
> hardware resources: phy, clock and power supply. All of them must be
> enabled/activated to properly initialize and operate. This was initially
> handled in s3c-hsotg driver, which has been converted to 'gadget' part
> of dwc2 driver. Unfortunately, not all of this code got moved to common
> platform code, what resulted in accessing DWC2 registers without
> enabling power supply regulators and clock. This caused initialization
> failure on Exynos4412-based Trats2. Odroid U3 board worked fine, because
> power supplies used by DWC2 module are marked there as 'always-on',
> because they are also used by other modules (USB hub) and clock was
> shared with USB2 PHY, so it was already enabled. This initialization
> issue has been fixed by the last patch. While preparing it I've also
> fixed a few minor issues found in nearby code (patches 4-6).
> 
> I would like to get those patches merged separately to respective
> subsystem trees:
> patch 1: phy subsystem tree
> patch 2: Samsung Exynos tree
> patch 3: regulator subsystem tree
> patch 4-7: USB gadget subsystem tree
> 
> Patches have been prepared on top of linux-next from 20150821.

Entire patchset tested on Trats2 board:
Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Best regards,
Krzysztof