[PATCH 0/6] omap: phy: dra7-evm PHY fixes for 3.16

[PATCH 0/6] omap: phy: dra7-evm PHY fixes for 3.16

[Roger Quadros]

Hi,

On DRA7-evm, the VDDA_1V8_PHY supply must be always-on for proper functioning
of the PHYs on the SoC.

The 3.3V USB supply (ldousb_reg) can be turned OFF when the High Speed USB PHYs
are not in use. We add regulator support in the PHY framework core to manage
the PHY's regulator during PHY power on/off.

Finally we add the 3.3V regulator information to the USB2 PHYs.

This series should get the USB and SATA working again on 3.16 on DRA7-evm.

cheers,
-roger

---
Roger Quadros (6):
  ARM: dts: dra7-evm: Make VDDA_1V8_PHY supply always on
  phy: core: Fix error path in phy_create()
  phy: core: Support regulator supply for PHY power
  phy: core: Add phy-supply to DT binding documentation
  phy: omap-usb2: Balance pm_runtime_enable() on probe failure
  ARM: dts: dra7-evm: Add regulator information to USB2 PHYs

 .../devicetree/bindings/phy/phy-bindings.txt       |  4 +++
 arch/arm/boot/dts/dra7-evm.dts                     |  9 +++++
 drivers/phy/phy-core.c                             | 39 ++++++++++++++++++++--
 drivers/phy/phy-omap-usb2.c                        |  8 +++--
 include/linux/phy/phy.h                            |  2 ++
 5 files changed, 57 insertions(+), 5 deletions(-)

-- 
1.8.3.2

[PATCH 0/6] omap: phy: dra7-evm PHY fixes for 3.16

[Roger Quadros]

Hi,

On DRA7-evm, the VDDA_1V8_PHY supply must be always-on for proper functioning
of the PHYs on the SoC.

The 3.3V USB supply (ldousb_reg) can be turned OFF when the High Speed USB PHYs
are not in use. We add regulator support in the PHY framework core to manage
the PHY's regulator during PHY power on/off.

Finally we add the 3.3V regulator information to the USB2 PHYs.

This series should get the USB and SATA working again on 3.16 on DRA7-evm.

cheers,
-roger

---
Roger Quadros (6):
  ARM: dts: dra7-evm: Make VDDA_1V8_PHY supply always on
  phy: core: Fix error path in phy_create()
  phy: core: Support regulator supply for PHY power
  phy: core: Add phy-supply to DT binding documentation
  phy: omap-usb2: Balance pm_runtime_enable() on probe failure
  ARM: dts: dra7-evm: Add regulator information to USB2 PHYs

 .../devicetree/bindings/phy/phy-bindings.txt       |  4 +++
 arch/arm/boot/dts/dra7-evm.dts                     |  9 +++++
 drivers/phy/phy-core.c                             | 39 ++++++++++++++++++++--
 drivers/phy/phy-omap-usb2.c                        |  8 +++--
 include/linux/phy/phy.h                            |  2 ++
 5 files changed, 57 insertions(+), 5 deletions(-)

-- 
1.8.3.2

[PATCH 1/6] ARM: dts: dra7-evm: Make VDDA_1V8_PHY supply always on

[Roger Quadros]

After clarification from the hardware team it was found that
this 1.8V PHY supply can't be switched OFF when SoC is Active.
It can only be switched off when in PORz (power on reset).

This patch is required for proper functionality of USB, SATA
and PCIe on DRA7-evm.

CC: Rajendra Nayak <rnayak@ti.com>
CC: Tero Kristo <t-kristo@ti.com>
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 arch/arm/boot/dts/dra7-evm.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/dra7-evm.dts b/arch/arm/boot/dts/dra7-evm.dts
index 4adc280..8308954 100644
--- a/arch/arm/boot/dts/dra7-evm.dts
+++ b/arch/arm/boot/dts/dra7-evm.dts
@@ -240,6 +240,7 @@
 					regulator-name = "ldo3";
 					regulator-min-microvolt = <1800000>;
 					regulator-max-microvolt = <1800000>;
+					regulator-always-on;
 					regulator-boot-on;
 				};
 
-- 
1.8.3.2

[PATCH 1/6] ARM: dts: dra7-evm: Make VDDA_1V8_PHY supply always on

[Roger Quadros]

After clarification from the hardware team it was found that
this 1.8V PHY supply can't be switched OFF when SoC is Active.
It can only be switched off when in PORz (power on reset).

This patch is required for proper functionality of USB, SATA
and PCIe on DRA7-evm.

CC: Rajendra Nayak <rnayak@ti.com>
CC: Tero Kristo <t-kristo@ti.com>
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 arch/arm/boot/dts/dra7-evm.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/dra7-evm.dts b/arch/arm/boot/dts/dra7-evm.dts
index 4adc280..8308954 100644
--- a/arch/arm/boot/dts/dra7-evm.dts
+++ b/arch/arm/boot/dts/dra7-evm.dts
@@ -240,6 +240,7 @@
 					regulator-name = "ldo3";
 					regulator-min-microvolt = <1800000>;
 					regulator-max-microvolt = <1800000>;
+					regulator-always-on;
 					regulator-boot-on;
 				};
 
-- 
1.8.3.2

[PATCH 2/6] phy: core: Fix error path in phy_create()

[Roger Quadros]

Prevent resources from being freed twice in case device_add() call
fails within phy_create(). Also use ida_simple_remove() instead of
ida_remove() as we had used ida_simple_get() to allocate the ida.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/phy/phy-core.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index c64a2f3..49c4465 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -614,8 +614,9 @@ struct phy *phy_create(struct device *dev, const struct phy_ops *ops,
 	return phy;
 
 put_dev:
-	put_device(&phy->dev);
-	ida_remove(&phy_ida, phy->id);
+	put_device(&phy->dev);  /* calls phy_release() which frees resources */
+	return ERR_PTR(ret);
+
 free_phy:
 	kfree(phy);
 	return ERR_PTR(ret);
@@ -799,7 +800,7 @@ static void phy_release(struct device *dev)
 
 	phy = to_phy(dev);
 	dev_vdbg(dev, "releasing '%s'\n", dev_name(dev));
-	ida_remove(&phy_ida, phy->id);
+	ida_simple_remove(&phy_ida, phy->id);
 	kfree(phy);
 }
 
-- 
1.8.3.2

[PATCH 2/6] phy: core: Fix error path in phy_create()

[Roger Quadros]

Prevent resources from being freed twice in case device_add() call
fails within phy_create(). Also use ida_simple_remove() instead of
ida_remove() as we had used ida_simple_get() to allocate the ida.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/phy/phy-core.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index c64a2f3..49c4465 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -614,8 +614,9 @@ struct phy *phy_create(struct device *dev, const struct phy_ops *ops,
 	return phy;
 
 put_dev:
-	put_device(&phy->dev);
-	ida_remove(&phy_ida, phy->id);
+	put_device(&phy->dev);  /* calls phy_release() which frees resources */
+	return ERR_PTR(ret);
+
 free_phy:
 	kfree(phy);
 	return ERR_PTR(ret);
@@ -799,7 +800,7 @@ static void phy_release(struct device *dev)
 
 	phy = to_phy(dev);
 	dev_vdbg(dev, "releasing '%s'\n", dev_name(dev));
-	ida_remove(&phy_ida, phy->id);
+	ida_simple_remove(&phy_ida, phy->id);
 	kfree(phy);
 }
 
-- 
1.8.3.2

[PATCH 3/6] phy: core: Support regulator supply for PHY power

[Roger Quadros]

Some PHYs can be powered by an external power regulator.
e.g. USB_HS PHY on DRA7 SoC. Make the PHY core support a
power regulator.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/phy/phy-core.c  | 32 ++++++++++++++++++++++++++++++++
 include/linux/phy/phy.h |  2 ++
 2 files changed, 34 insertions(+)

diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 49c4465..d817107 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -21,6 +21,7 @@
 #include <linux/phy/phy.h>
 #include <linux/idr.h>
 #include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
 
 static struct class *phy_class;
 static DEFINE_MUTEX(phy_provider_mutex);
@@ -226,6 +227,12 @@ int phy_power_on(struct phy *phy)
 	if (!phy)
 		return 0;
 
+	if (phy->pwr) {
+		ret = regulator_enable(phy->pwr);
+		if (ret)
+			return ret;
+	}
+
 	ret = phy_pm_runtime_get_sync(phy);
 	if (ret < 0 && ret != -ENOTSUPP)
 		return ret;
@@ -247,6 +254,8 @@ int phy_power_on(struct phy *phy)
 out:
 	mutex_unlock(&phy->mutex);
 	phy_pm_runtime_put_sync(phy);
+	if (phy->pwr)
+		regulator_disable(phy->pwr);
 
 	return ret;
 }
@@ -272,6 +281,9 @@ int phy_power_off(struct phy *phy)
 	mutex_unlock(&phy->mutex);
 	phy_pm_runtime_put(phy);
 
+	if (phy->pwr)
+		regulator_disable(phy->pwr);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(phy_power_off);
@@ -588,6 +600,16 @@ struct phy *phy_create(struct device *dev, const struct phy_ops *ops,
 		goto free_phy;
 	}
 
+	/* phy-supply */
+	phy->pwr = regulator_get_optional(dev, "phy");
+	if (IS_ERR(phy->pwr)) {
+		if (PTR_ERR(phy->pwr) == -EPROBE_DEFER) {
+			ret = -EPROBE_DEFER;
+			goto free_ida;
+		}
+		phy->pwr = NULL;
+	}
+
 	device_initialize(&phy->dev);
 	mutex_init(&phy->mutex);
 
@@ -617,6 +639,9 @@ put_dev:
 	put_device(&phy->dev);  /* calls phy_release() which frees resources */
 	return ERR_PTR(ret);
 
+free_ida:
+	ida_simple_remove(&phy_ida, phy->id);
+
 free_phy:
 	kfree(phy);
 	return ERR_PTR(ret);
@@ -664,6 +689,10 @@ EXPORT_SYMBOL_GPL(devm_phy_create);
 void phy_destroy(struct phy *phy)
 {
 	pm_runtime_disable(&phy->dev);
+
+	if (phy->pwr)
+		regulator_put(phy->pwr);
+
 	device_unregister(&phy->dev);
 }
 EXPORT_SYMBOL_GPL(phy_destroy);
@@ -800,6 +829,9 @@ static void phy_release(struct device *dev)
 
 	phy = to_phy(dev);
 	dev_vdbg(dev, "releasing '%s'\n", dev_name(dev));
+	if (phy->pwr)
+		regulator_put(phy->pwr);
+
 	ida_simple_remove(&phy_ida, phy->id);
 	kfree(phy);
 }
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 2760744..9a86945 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -18,6 +18,7 @@
 #include <linux/of.h>
 #include <linux/device.h>
 #include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
 
 struct phy;
 
@@ -65,6 +66,7 @@ struct phy {
 	int			init_count;
 	int			power_count;
 	struct phy_attrs	attrs;
+	struct regulator	*pwr;
 };
 
 /**
-- 
1.8.3.2

[PATCH 3/6] phy: core: Support regulator supply for PHY power

[Roger Quadros]

Some PHYs can be powered by an external power regulator.
e.g. USB_HS PHY on DRA7 SoC. Make the PHY core support a
power regulator.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/phy/phy-core.c  | 32 ++++++++++++++++++++++++++++++++
 include/linux/phy/phy.h |  2 ++
 2 files changed, 34 insertions(+)

diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 49c4465..d817107 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -21,6 +21,7 @@
 #include <linux/phy/phy.h>
 #include <linux/idr.h>
 #include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
 
 static struct class *phy_class;
 static DEFINE_MUTEX(phy_provider_mutex);
@@ -226,6 +227,12 @@ int phy_power_on(struct phy *phy)
 	if (!phy)
 		return 0;
 
+	if (phy->pwr) {
+		ret = regulator_enable(phy->pwr);
+		if (ret)
+			return ret;
+	}
+
 	ret = phy_pm_runtime_get_sync(phy);
 	if (ret < 0 && ret != -ENOTSUPP)
 		return ret;
@@ -247,6 +254,8 @@ int phy_power_on(struct phy *phy)
 out:
 	mutex_unlock(&phy->mutex);
 	phy_pm_runtime_put_sync(phy);
+	if (phy->pwr)
+		regulator_disable(phy->pwr);
 
 	return ret;
 }
@@ -272,6 +281,9 @@ int phy_power_off(struct phy *phy)
 	mutex_unlock(&phy->mutex);
 	phy_pm_runtime_put(phy);
 
+	if (phy->pwr)
+		regulator_disable(phy->pwr);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(phy_power_off);
@@ -588,6 +600,16 @@ struct phy *phy_create(struct device *dev, const struct phy_ops *ops,
 		goto free_phy;
 	}
 
+	/* phy-supply */
+	phy->pwr = regulator_get_optional(dev, "phy");
+	if (IS_ERR(phy->pwr)) {
+		if (PTR_ERR(phy->pwr) == -EPROBE_DEFER) {
+			ret = -EPROBE_DEFER;
+			goto free_ida;
+		}
+		phy->pwr = NULL;
+	}
+
 	device_initialize(&phy->dev);
 	mutex_init(&phy->mutex);
 
@@ -617,6 +639,9 @@ put_dev:
 	put_device(&phy->dev);  /* calls phy_release() which frees resources */
 	return ERR_PTR(ret);
 
+free_ida:
+	ida_simple_remove(&phy_ida, phy->id);
+
 free_phy:
 	kfree(phy);
 	return ERR_PTR(ret);
@@ -664,6 +689,10 @@ EXPORT_SYMBOL_GPL(devm_phy_create);
 void phy_destroy(struct phy *phy)
 {
 	pm_runtime_disable(&phy->dev);
+
+	if (phy->pwr)
+		regulator_put(phy->pwr);
+
 	device_unregister(&phy->dev);
 }
 EXPORT_SYMBOL_GPL(phy_destroy);
@@ -800,6 +829,9 @@ static void phy_release(struct device *dev)
 
 	phy = to_phy(dev);
 	dev_vdbg(dev, "releasing '%s'\n", dev_name(dev));
+	if (phy->pwr)
+		regulator_put(phy->pwr);
+
 	ida_simple_remove(&phy_ida, phy->id);
 	kfree(phy);
 }
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 2760744..9a86945 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -18,6 +18,7 @@
 #include <linux/of.h>
 #include <linux/device.h>
 #include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
 
 struct phy;
 
@@ -65,6 +66,7 @@ struct phy {
 	int			init_count;
 	int			power_count;
 	struct phy_attrs	attrs;
+	struct regulator	*pwr;
 };
 
 /**
-- 
1.8.3.2

[PATCH 4/6] phy: core: Add phy-supply to DT binding documentation

[Roger Quadros]

phy-supply is a phandle to the regulator that provides power to the
PHY. This regulator is managed during the PHY power on/off sequence
by the phy core driver.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 Documentation/devicetree/bindings/phy/phy-bindings.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/phy-bindings.txt b/Documentation/devicetree/bindings/phy/phy-bindings.txt
index 8ae844f..2aa1840 100644
--- a/Documentation/devicetree/bindings/phy/phy-bindings.txt
+++ b/Documentation/devicetree/bindings/phy/phy-bindings.txt
@@ -10,6 +10,10 @@ Required Properties:
 		provider can use the values in cells to find the appropriate
 		PHY.
 
+Optional Properties:
+phy-supply:	Phandle to a regulator that provides power to the PHY. This
+		regulator will be managed during the PHY power on/off sequence.
+
 For example:
 
 phys: phy {
-- 
1.8.3.2

[PATCH 4/6] phy: core: Add phy-supply to DT binding documentation

[Roger Quadros]

phy-supply is a phandle to the regulator that provides power to the
PHY. This regulator is managed during the PHY power on/off sequence
by the phy core driver.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 Documentation/devicetree/bindings/phy/phy-bindings.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/phy-bindings.txt b/Documentation/devicetree/bindings/phy/phy-bindings.txt
index 8ae844f..2aa1840 100644
--- a/Documentation/devicetree/bindings/phy/phy-bindings.txt
+++ b/Documentation/devicetree/bindings/phy/phy-bindings.txt
@@ -10,6 +10,10 @@ Required Properties:
 		provider can use the values in cells to find the appropriate
 		PHY.
 
+Optional Properties:
+phy-supply:	Phandle to a regulator that provides power to the PHY. This
+		regulator will be managed during the PHY power on/off sequence.
+
 For example:
 
 phys: phy {
-- 
1.8.3.2

[PATCH 5/6] phy: omap-usb2: Balance pm_runtime_enable() on probe failure

[Roger Quadros]

If probe fails then we need to call pm_runtime_disable() to balance
out the previous pm_runtime_enable() call. Else it will cause
unbalanced pm_runtime_enable() call in the succeding probe call.

This anomaly was observed when the call to devm_phy_create() failed
with -EPROBE_DEFER.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/phy/phy-omap-usb2.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/phy-omap-usb2.c b/drivers/phy/phy-omap-usb2.c
index 7007c11..c6f9809 100644
--- a/drivers/phy/phy-omap-usb2.c
+++ b/drivers/phy/phy-omap-usb2.c
@@ -265,15 +265,19 @@ static int omap_usb2_probe(struct platform_device *pdev)
 	pm_runtime_enable(phy->dev);
 
 	generic_phy = devm_phy_create(phy->dev, &ops, NULL);
-	if (IS_ERR(generic_phy))
+	if (IS_ERR(generic_phy)) {
+		pm_runtime_disable(phy->dev);
 		return PTR_ERR(generic_phy);
+	}
 
 	phy_set_drvdata(generic_phy, phy);
 
 	phy_provider = devm_of_phy_provider_register(phy->dev,
 			of_phy_simple_xlate);
-	if (IS_ERR(phy_provider))
+	if (IS_ERR(phy_provider)) {
+		pm_runtime_disable(phy->dev);
 		return PTR_ERR(phy_provider);
+	}
 
 	phy->wkupclk = devm_clk_get(phy->dev, "wkupclk");
 	if (IS_ERR(phy->wkupclk)) {
-- 
1.8.3.2

[PATCH 5/6] phy: omap-usb2: Balance pm_runtime_enable() on probe failure

[Roger Quadros]

If probe fails then we need to call pm_runtime_disable() to balance
out the previous pm_runtime_enable() call. Else it will cause
unbalanced pm_runtime_enable() call in the succeding probe call.

This anomaly was observed when the call to devm_phy_create() failed
with -EPROBE_DEFER.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/phy/phy-omap-usb2.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/phy-omap-usb2.c b/drivers/phy/phy-omap-usb2.c
index 7007c11..c6f9809 100644
--- a/drivers/phy/phy-omap-usb2.c
+++ b/drivers/phy/phy-omap-usb2.c
@@ -265,15 +265,19 @@ static int omap_usb2_probe(struct platform_device *pdev)
 	pm_runtime_enable(phy->dev);
 
 	generic_phy = devm_phy_create(phy->dev, &ops, NULL);
-	if (IS_ERR(generic_phy))
+	if (IS_ERR(generic_phy)) {
+		pm_runtime_disable(phy->dev);
 		return PTR_ERR(generic_phy);
+	}
 
 	phy_set_drvdata(generic_phy, phy);
 
 	phy_provider = devm_of_phy_provider_register(phy->dev,
 			of_phy_simple_xlate);
-	if (IS_ERR(phy_provider))
+	if (IS_ERR(phy_provider)) {
+		pm_runtime_disable(phy->dev);
 		return PTR_ERR(phy_provider);
+	}
 
 	phy->wkupclk = devm_clk_get(phy->dev, "wkupclk");
 	if (IS_ERR(phy->wkupclk)) {
-- 
1.8.3.2

[PATCH 6/6] ARM: dts: dra7-evm: Add regulator information to USB2 PHYs

[Roger Quadros]

The ldousb_reg regulator provides power to the USB1 and USB2
High Speed PHYs.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 arch/arm/boot/dts/dra7-evm.dts | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/dra7-evm.dts b/arch/arm/boot/dts/dra7-evm.dts
index 8308954..50f8022 100644
--- a/arch/arm/boot/dts/dra7-evm.dts
+++ b/arch/arm/boot/dts/dra7-evm.dts
@@ -496,3 +496,11 @@
 		};
 	};
 };
+
+&usb2_phy1 {
+	phy-supply = <&ldousb_reg>;
+};
+
+&usb2_phy2 {
+	phy-supply = <&ldousb_reg>;
+};
-- 
1.8.3.2

[PATCH 6/6] ARM: dts: dra7-evm: Add regulator information to USB2 PHYs

[Roger Quadros]

The ldousb_reg regulator provides power to the USB1 and USB2
High Speed PHYs.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 arch/arm/boot/dts/dra7-evm.dts | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/dra7-evm.dts b/arch/arm/boot/dts/dra7-evm.dts
index 8308954..50f8022 100644
--- a/arch/arm/boot/dts/dra7-evm.dts
+++ b/arch/arm/boot/dts/dra7-evm.dts
@@ -496,3 +496,11 @@
 		};
 	};
 };
+
+&usb2_phy1 {
+	phy-supply = <&ldousb_reg>;
+};
+
+&usb2_phy2 {
+	phy-supply = <&ldousb_reg>;
+};
-- 
1.8.3.2

Re: [PATCH 3/6] phy: core: Support regulator supply for PHY power

[Sergei Shtylyov]

Hello.

On 07/02/2014 04:03 PM, Roger Quadros wrote:

> Some PHYs can be powered by an external power regulator.
> e.g. USB_HS PHY on DRA7 SoC. Make the PHY core support a
> power regulator.

> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
>   drivers/phy/phy-core.c  | 32 ++++++++++++++++++++++++++++++++
>   include/linux/phy/phy.h |  2 ++
>   2 files changed, 34 insertions(+)

> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index 49c4465..d817107 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
[...]
> @@ -664,6 +689,10 @@ EXPORT_SYMBOL_GPL(devm_phy_create);
>   void phy_destroy(struct phy *phy)
>   {
>   	pm_runtime_disable(&phy->dev);
> +
> +	if (phy->pwr)
> +		regulator_put(phy->pwr);

    regulator_put() already handles NULL pointer.

> +
>   	device_unregister(&phy->dev);
>   }
>   EXPORT_SYMBOL_GPL(phy_destroy);
> @@ -800,6 +829,9 @@ static void phy_release(struct device *dev)
>
>   	phy = to_phy(dev);
>   	dev_vdbg(dev, "releasing '%s'\n", dev_name(dev));
> +	if (phy->pwr)
> +		regulator_put(phy->pwr);

    Same comment here.

> +
>   	ida_simple_remove(&phy_ida, phy->id);
>   	kfree(phy);
>   }

WBR, Sergei

Re: [PATCH 1/6] ARM: dts: dra7-evm: Make VDDA_1V8_PHY supply always on

[Nishanth Menon]

On 07/02/2014 07:03 AM, Roger Quadros wrote:
> After clarification from the hardware team it was found that
> this 1.8V PHY supply can't be switched OFF when SoC is Active.
> It can only be switched off when in PORz (power on reset).

I dont think folks know the reasoning why hardware team decided that
the voltage rail cannot be switched off -> I suggest adding the
following information as well.

Since isolation is not present in the design for these PHY IPs to
allow for the PHY rail to be switched off, there is a very high risk
of IP reliability and additional leakage paths which can result in
additional power consumption.

Only scenario where this rail can be switched off is part of Power on
reset sequencing, but it needs to be kept always-on during operation.

> 
> This patch is required for proper functionality of USB, SATA
> and PCIe on DRA7-evm.
> 
> CC: Rajendra Nayak <rnayak@ti.com>
> CC: Tero Kristo <t-kristo@ti.com>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
>  arch/arm/boot/dts/dra7-evm.dts | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/boot/dts/dra7-evm.dts b/arch/arm/boot/dts/dra7-evm.dts
> index 4adc280..8308954 100644
> --- a/arch/arm/boot/dts/dra7-evm.dts
> +++ b/arch/arm/boot/dts/dra7-evm.dts
> @@ -240,6 +240,7 @@
>  					regulator-name = "ldo3";
>  					regulator-min-microvolt = <1800000>;
>  					regulator-max-microvolt = <1800000>;
> +					regulator-always-on;
>  					regulator-boot-on;
>  				};
>  
> 


-- 
Regards,
Nishanth Menon

Re: [PATCH 1/6] ARM: dts: dra7-evm: Make VDDA_1V8_PHY supply always on

[Nishanth Menon]

On 07/02/2014 07:03 AM, Roger Quadros wrote:
> After clarification from the hardware team it was found that
> this 1.8V PHY supply can't be switched OFF when SoC is Active.
> It can only be switched off when in PORz (power on reset).

I dont think folks know the reasoning why hardware team decided that
the voltage rail cannot be switched off -> I suggest adding the
following information as well.

Since isolation is not present in the design for these PHY IPs to
allow for the PHY rail to be switched off, there is a very high risk
of IP reliability and additional leakage paths which can result in
additional power consumption.

Only scenario where this rail can be switched off is part of Power on
reset sequencing, but it needs to be kept always-on during operation.

> 
> This patch is required for proper functionality of USB, SATA
> and PCIe on DRA7-evm.
> 
> CC: Rajendra Nayak <rnayak@ti.com>
> CC: Tero Kristo <t-kristo@ti.com>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
>  arch/arm/boot/dts/dra7-evm.dts | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/boot/dts/dra7-evm.dts b/arch/arm/boot/dts/dra7-evm.dts
> index 4adc280..8308954 100644
> --- a/arch/arm/boot/dts/dra7-evm.dts
> +++ b/arch/arm/boot/dts/dra7-evm.dts
> @@ -240,6 +240,7 @@
>  					regulator-name = "ldo3";
>  					regulator-min-microvolt = <1800000>;
>  					regulator-max-microvolt = <1800000>;
> +					regulator-always-on;
>  					regulator-boot-on;
>  				};
>  
> 


-- 
Regards,
Nishanth Menon

Re: [PATCH 1/6] ARM: dts: dra7-evm: Make VDDA_1V8_PHY supply always on

[Roger Quadros]

On 07/02/2014 04:05 PM, Nishanth Menon wrote:
> On 07/02/2014 07:03 AM, Roger Quadros wrote:
>> After clarification from the hardware team it was found that
>> this 1.8V PHY supply can't be switched OFF when SoC is Active.
>> It can only be switched off when in PORz (power on reset).
> 
> I dont think folks know the reasoning why hardware team decided that
> the voltage rail cannot be switched off -> I suggest adding the
> following information as well.
> 
> Since isolation is not present in the design for these PHY IPs to
> allow for the PHY rail to be switched off, there is a very high risk
> of IP reliability and additional leakage paths which can result in
> additional power consumption.
> 
> Only scenario where this rail can be switched off is part of Power on
> reset sequencing, but it needs to be kept always-on during operation.

This sort of information should be in the TRM instead, but doesn't harm to be in the commit message so I'll do that.
Thanks.

cheers,
-roger
> 
>>
>> This patch is required for proper functionality of USB, SATA
>> and PCIe on DRA7-evm.
>>
>> CC: Rajendra Nayak <rnayak@ti.com>
>> CC: Tero Kristo <t-kristo@ti.com>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>>  arch/arm/boot/dts/dra7-evm.dts | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm/boot/dts/dra7-evm.dts b/arch/arm/boot/dts/dra7-evm.dts
>> index 4adc280..8308954 100644
>> --- a/arch/arm/boot/dts/dra7-evm.dts
>> +++ b/arch/arm/boot/dts/dra7-evm.dts
>> @@ -240,6 +240,7 @@
>>  					regulator-name = "ldo3";
>>  					regulator-min-microvolt = <1800000>;
>>  					regulator-max-microvolt = <1800000>;
>> +					regulator-always-on;
>>  					regulator-boot-on;
>>  				};
>>  
>>
> 
> 

Re: [PATCH 1/6] ARM: dts: dra7-evm: Make VDDA_1V8_PHY supply always on

[Roger Quadros]

On 07/02/2014 04:05 PM, Nishanth Menon wrote:
> On 07/02/2014 07:03 AM, Roger Quadros wrote:
>> After clarification from the hardware team it was found that
>> this 1.8V PHY supply can't be switched OFF when SoC is Active.
>> It can only be switched off when in PORz (power on reset).
> 
> I dont think folks know the reasoning why hardware team decided that
> the voltage rail cannot be switched off -> I suggest adding the
> following information as well.
> 
> Since isolation is not present in the design for these PHY IPs to
> allow for the PHY rail to be switched off, there is a very high risk
> of IP reliability and additional leakage paths which can result in
> additional power consumption.
> 
> Only scenario where this rail can be switched off is part of Power on
> reset sequencing, but it needs to be kept always-on during operation.

This sort of information should be in the TRM instead, but doesn't harm to be in the commit message so I'll do that.
Thanks.

cheers,
-roger
> 
>>
>> This patch is required for proper functionality of USB, SATA
>> and PCIe on DRA7-evm.
>>
>> CC: Rajendra Nayak <rnayak@ti.com>
>> CC: Tero Kristo <t-kristo@ti.com>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>>  arch/arm/boot/dts/dra7-evm.dts | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm/boot/dts/dra7-evm.dts b/arch/arm/boot/dts/dra7-evm.dts
>> index 4adc280..8308954 100644
>> --- a/arch/arm/boot/dts/dra7-evm.dts
>> +++ b/arch/arm/boot/dts/dra7-evm.dts
>> @@ -240,6 +240,7 @@
>>  					regulator-name = "ldo3";
>>  					regulator-min-microvolt = <1800000>;
>>  					regulator-max-microvolt = <1800000>;
>> +					regulator-always-on;
>>  					regulator-boot-on;
>>  				};
>>  
>>
> 
> 

Re: [PATCH 3/6] phy: core: Support regulator supply for PHY power

[Roger Quadros]

On 07/02/2014 03:32 PM, Sergei Shtylyov wrote:
> Hello.
> 
> On 07/02/2014 04:03 PM, Roger Quadros wrote:
> 
>> Some PHYs can be powered by an external power regulator.
>> e.g. USB_HS PHY on DRA7 SoC. Make the PHY core support a
>> power regulator.
> 
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>>   drivers/phy/phy-core.c  | 32 ++++++++++++++++++++++++++++++++
>>   include/linux/phy/phy.h |  2 ++
>>   2 files changed, 34 insertions(+)
> 
>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
>> index 49c4465..d817107 100644
>> --- a/drivers/phy/phy-core.c
>> +++ b/drivers/phy/phy-core.c
> [...]
>> @@ -664,6 +689,10 @@ EXPORT_SYMBOL_GPL(devm_phy_create);
>>   void phy_destroy(struct phy *phy)
>>   {
>>       pm_runtime_disable(&phy->dev);
>> +
>> +    if (phy->pwr)
>> +        regulator_put(phy->pwr);
> 
>    regulator_put() already handles NULL pointer.

Good to know that. I'll remove the 'if' then.

cheers,
-roger

> 
>> +
>>       device_unregister(&phy->dev);
>>   }
>>   EXPORT_SYMBOL_GPL(phy_destroy);
>> @@ -800,6 +829,9 @@ static void phy_release(struct device *dev)
>>
>>       phy = to_phy(dev);
>>       dev_vdbg(dev, "releasing '%s'\n", dev_name(dev));
>> +    if (phy->pwr)
>> +        regulator_put(phy->pwr);
> 
>    Same comment here.
> 
>> +
>>       ida_simple_remove(&phy_ida, phy->id);
>>       kfree(phy);
>>   }
> 
> WBR, Sergei
> 

Re: [PATCH 3/6] phy: core: Support regulator supply for PHY power

[Roger Quadros]

On 07/02/2014 03:32 PM, Sergei Shtylyov wrote:
> Hello.
> 
> On 07/02/2014 04:03 PM, Roger Quadros wrote:
> 
>> Some PHYs can be powered by an external power regulator.
>> e.g. USB_HS PHY on DRA7 SoC. Make the PHY core support a
>> power regulator.
> 
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>>   drivers/phy/phy-core.c  | 32 ++++++++++++++++++++++++++++++++
>>   include/linux/phy/phy.h |  2 ++
>>   2 files changed, 34 insertions(+)
> 
>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
>> index 49c4465..d817107 100644
>> --- a/drivers/phy/phy-core.c
>> +++ b/drivers/phy/phy-core.c
> [...]
>> @@ -664,6 +689,10 @@ EXPORT_SYMBOL_GPL(devm_phy_create);
>>   void phy_destroy(struct phy *phy)
>>   {
>>       pm_runtime_disable(&phy->dev);
>> +
>> +    if (phy->pwr)
>> +        regulator_put(phy->pwr);
> 
>    regulator_put() already handles NULL pointer.

Good to know that. I'll remove the 'if' then.

cheers,
-roger

> 
>> +
>>       device_unregister(&phy->dev);
>>   }
>>   EXPORT_SYMBOL_GPL(phy_destroy);
>> @@ -800,6 +829,9 @@ static void phy_release(struct device *dev)
>>
>>       phy = to_phy(dev);
>>       dev_vdbg(dev, "releasing '%s'\n", dev_name(dev));
>> +    if (phy->pwr)
>> +        regulator_put(phy->pwr);
> 
>    Same comment here.
> 
>> +
>>       ida_simple_remove(&phy_ida, phy->id);
>>       kfree(phy);
>>   }
> 
> WBR, Sergei
> 

[PATCH v2 1/6] ARM: dts: dra7-evm: Make VDDA_1V8_PHY supply always on

[Roger Quadros]

From: Roger Quadros <rogerq@ti.com>

After clarification from the hardware team it was found that
this 1.8V PHY supply can't be switched OFF when SoC is Active.

Since the PHY IPs don't contain isolation logic built in the design to
allow the power rail to be switched off, there is a very high risk
of IP reliability and additional leakage paths which can result in
additional power consumption.

The only scenario where this rail can be switched off is part of Power on
reset sequencing, but it needs to be kept always-on during operation.

This patch is required for proper functionality of USB, SATA
and PCIe on DRA7-evm.

CC: Rajendra Nayak <rnayak@ti.com>
CC: Tero Kristo <t-kristo@ti.com>
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 arch/arm/boot/dts/dra7-evm.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/dra7-evm.dts b/arch/arm/boot/dts/dra7-evm.dts
index 4adc280..8308954 100644
--- a/arch/arm/boot/dts/dra7-evm.dts
+++ b/arch/arm/boot/dts/dra7-evm.dts
@@ -240,6 +240,7 @@
 					regulator-name = "ldo3";
 					regulator-min-microvolt = <1800000>;
 					regulator-max-microvolt = <1800000>;
+					regulator-always-on;
 					regulator-boot-on;
 				};
 
-- 
1.8.3.2

[PATCH v2 1/6] ARM: dts: dra7-evm: Make VDDA_1V8_PHY supply always on

[Roger Quadros]

From: Roger Quadros <rogerq@ti.com>

After clarification from the hardware team it was found that
this 1.8V PHY supply can't be switched OFF when SoC is Active.

Since the PHY IPs don't contain isolation logic built in the design to
allow the power rail to be switched off, there is a very high risk
of IP reliability and additional leakage paths which can result in
additional power consumption.

The only scenario where this rail can be switched off is part of Power on
reset sequencing, but it needs to be kept always-on during operation.

This patch is required for proper functionality of USB, SATA
and PCIe on DRA7-evm.

CC: Rajendra Nayak <rnayak@ti.com>
CC: Tero Kristo <t-kristo@ti.com>
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 arch/arm/boot/dts/dra7-evm.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/dra7-evm.dts b/arch/arm/boot/dts/dra7-evm.dts
index 4adc280..8308954 100644
--- a/arch/arm/boot/dts/dra7-evm.dts
+++ b/arch/arm/boot/dts/dra7-evm.dts
@@ -240,6 +240,7 @@
 					regulator-name = "ldo3";
 					regulator-min-microvolt = <1800000>;
 					regulator-max-microvolt = <1800000>;
+					regulator-always-on;
 					regulator-boot-on;
 				};
 
-- 
1.8.3.2

[PATCH v2 3/6] phy: core: Support regulator supply for PHY power

[Roger Quadros]

From: Roger Quadros <rogerq@ti.com>

Some PHYs can be powered by an external power regulator.
e.g. USB_HS PHY on DRA7 SoC. Make the PHY core support a
power regulator.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/phy/phy-core.c  | 27 +++++++++++++++++++++++++++
 include/linux/phy/phy.h |  2 ++
 2 files changed, 29 insertions(+)

diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 49c4465..2add59c 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -21,6 +21,7 @@
 #include <linux/phy/phy.h>
 #include <linux/idr.h>
 #include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
 
 static struct class *phy_class;
 static DEFINE_MUTEX(phy_provider_mutex);
@@ -226,6 +227,12 @@ int phy_power_on(struct phy *phy)
 	if (!phy)
 		return 0;
 
+	if (phy->pwr) {
+		ret = regulator_enable(phy->pwr);
+		if (ret)
+			return ret;
+	}
+
 	ret = phy_pm_runtime_get_sync(phy);
 	if (ret < 0 && ret != -ENOTSUPP)
 		return ret;
@@ -247,6 +254,8 @@ int phy_power_on(struct phy *phy)
 out:
 	mutex_unlock(&phy->mutex);
 	phy_pm_runtime_put_sync(phy);
+	if (phy->pwr)
+		regulator_disable(phy->pwr);
 
 	return ret;
 }
@@ -272,6 +281,9 @@ int phy_power_off(struct phy *phy)
 	mutex_unlock(&phy->mutex);
 	phy_pm_runtime_put(phy);
 
+	if (phy->pwr)
+		regulator_disable(phy->pwr);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(phy_power_off);
@@ -588,6 +600,16 @@ struct phy *phy_create(struct device *dev, const struct phy_ops *ops,
 		goto free_phy;
 	}
 
+	/* phy-supply */
+	phy->pwr = regulator_get_optional(dev, "phy");
+	if (IS_ERR(phy->pwr)) {
+		if (PTR_ERR(phy->pwr) == -EPROBE_DEFER) {
+			ret = -EPROBE_DEFER;
+			goto free_ida;
+		}
+		phy->pwr = NULL;
+	}
+
 	device_initialize(&phy->dev);
 	mutex_init(&phy->mutex);
 
@@ -617,6 +639,9 @@ put_dev:
 	put_device(&phy->dev);  /* calls phy_release() which frees resources */
 	return ERR_PTR(ret);
 
+free_ida:
+	ida_simple_remove(&phy_ida, phy->id);
+
 free_phy:
 	kfree(phy);
 	return ERR_PTR(ret);
@@ -664,6 +689,7 @@ EXPORT_SYMBOL_GPL(devm_phy_create);
 void phy_destroy(struct phy *phy)
 {
 	pm_runtime_disable(&phy->dev);
+	regulator_put(phy->pwr);
 	device_unregister(&phy->dev);
 }
 EXPORT_SYMBOL_GPL(phy_destroy);
@@ -800,6 +826,7 @@ static void phy_release(struct device *dev)
 
 	phy = to_phy(dev);
 	dev_vdbg(dev, "releasing '%s'\n", dev_name(dev));
+	regulator_put(phy->pwr);
 	ida_simple_remove(&phy_ida, phy->id);
 	kfree(phy);
 }
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 2760744..9a86945 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -18,6 +18,7 @@
 #include <linux/of.h>
 #include <linux/device.h>
 #include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
 
 struct phy;
 
@@ -65,6 +66,7 @@ struct phy {
 	int			init_count;
 	int			power_count;
 	struct phy_attrs	attrs;
+	struct regulator	*pwr;
 };
 
 /**
-- 
1.8.3.2

[PATCH v2 3/6] phy: core: Support regulator supply for PHY power

[Roger Quadros]

From: Roger Quadros <rogerq@ti.com>

Some PHYs can be powered by an external power regulator.
e.g. USB_HS PHY on DRA7 SoC. Make the PHY core support a
power regulator.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/phy/phy-core.c  | 27 +++++++++++++++++++++++++++
 include/linux/phy/phy.h |  2 ++
 2 files changed, 29 insertions(+)

diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 49c4465..2add59c 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -21,6 +21,7 @@
 #include <linux/phy/phy.h>
 #include <linux/idr.h>
 #include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
 
 static struct class *phy_class;
 static DEFINE_MUTEX(phy_provider_mutex);
@@ -226,6 +227,12 @@ int phy_power_on(struct phy *phy)
 	if (!phy)
 		return 0;
 
+	if (phy->pwr) {
+		ret = regulator_enable(phy->pwr);
+		if (ret)
+			return ret;
+	}
+
 	ret = phy_pm_runtime_get_sync(phy);
 	if (ret < 0 && ret != -ENOTSUPP)
 		return ret;
@@ -247,6 +254,8 @@ int phy_power_on(struct phy *phy)
 out:
 	mutex_unlock(&phy->mutex);
 	phy_pm_runtime_put_sync(phy);
+	if (phy->pwr)
+		regulator_disable(phy->pwr);
 
 	return ret;
 }
@@ -272,6 +281,9 @@ int phy_power_off(struct phy *phy)
 	mutex_unlock(&phy->mutex);
 	phy_pm_runtime_put(phy);
 
+	if (phy->pwr)
+		regulator_disable(phy->pwr);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(phy_power_off);
@@ -588,6 +600,16 @@ struct phy *phy_create(struct device *dev, const struct phy_ops *ops,
 		goto free_phy;
 	}
 
+	/* phy-supply */
+	phy->pwr = regulator_get_optional(dev, "phy");
+	if (IS_ERR(phy->pwr)) {
+		if (PTR_ERR(phy->pwr) == -EPROBE_DEFER) {
+			ret = -EPROBE_DEFER;
+			goto free_ida;
+		}
+		phy->pwr = NULL;
+	}
+
 	device_initialize(&phy->dev);
 	mutex_init(&phy->mutex);
 
@@ -617,6 +639,9 @@ put_dev:
 	put_device(&phy->dev);  /* calls phy_release() which frees resources */
 	return ERR_PTR(ret);
 
+free_ida:
+	ida_simple_remove(&phy_ida, phy->id);
+
 free_phy:
 	kfree(phy);
 	return ERR_PTR(ret);
@@ -664,6 +689,7 @@ EXPORT_SYMBOL_GPL(devm_phy_create);
 void phy_destroy(struct phy *phy)
 {
 	pm_runtime_disable(&phy->dev);
+	regulator_put(phy->pwr);
 	device_unregister(&phy->dev);
 }
 EXPORT_SYMBOL_GPL(phy_destroy);
@@ -800,6 +826,7 @@ static void phy_release(struct device *dev)
 
 	phy = to_phy(dev);
 	dev_vdbg(dev, "releasing '%s'\n", dev_name(dev));
+	regulator_put(phy->pwr);
 	ida_simple_remove(&phy_ida, phy->id);
 	kfree(phy);
 }
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 2760744..9a86945 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -18,6 +18,7 @@
 #include <linux/of.h>
 #include <linux/device.h>
 #include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
 
 struct phy;
 
@@ -65,6 +66,7 @@ struct phy {
 	int			init_count;
 	int			power_count;
 	struct phy_attrs	attrs;
+	struct regulator	*pwr;
 };
 
 /**
-- 
1.8.3.2

Re: [PATCH 5/6] phy: omap-usb2: Balance pm_runtime_enable() on probe failure

[Kishon Vijay Abraham I]

Hi Roger,

On Wednesday 02 July 2014 05:33 PM, Roger Quadros wrote:
> If probe fails then we need to call pm_runtime_disable() to balance
> out the previous pm_runtime_enable() call. Else it will cause
> unbalanced pm_runtime_enable() call in the succeding probe call.
> 
> This anomaly was observed when the call to devm_phy_create() failed
> with -EPROBE_DEFER.
> 
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
>  drivers/phy/phy-omap-usb2.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/phy/phy-omap-usb2.c b/drivers/phy/phy-omap-usb2.c
> index 7007c11..c6f9809 100644
> --- a/drivers/phy/phy-omap-usb2.c
> +++ b/drivers/phy/phy-omap-usb2.c
> @@ -265,15 +265,19 @@ static int omap_usb2_probe(struct platform_device *pdev)
>  	pm_runtime_enable(phy->dev);
>  
>  	generic_phy = devm_phy_create(phy->dev, &ops, NULL);
> -	if (IS_ERR(generic_phy))
> +	if (IS_ERR(generic_phy)) {
> +		pm_runtime_disable(phy->dev);
>  		return PTR_ERR(generic_phy);
> +	}

Maybe we can move pm_runtime_enable to just before phy_provider_register?
>  
>  	phy_set_drvdata(generic_phy, phy);
>  
>  	phy_provider = devm_of_phy_provider_register(phy->dev,
>  			of_phy_simple_xlate);
> -	if (IS_ERR(phy_provider))
> +	if (IS_ERR(phy_provider)) {
> +		pm_runtime_disable(phy->dev);
>  		return PTR_ERR(phy_provider);
> +	}

Noticed pm_runtime_disable was missing in omap_usb2_remove too. Would be good
to fix it here.

Cheers
Kishon

Re: [PATCH 5/6] phy: omap-usb2: Balance pm_runtime_enable() on probe failure

[Kishon Vijay Abraham I]

Hi Roger,

On Wednesday 02 July 2014 05:33 PM, Roger Quadros wrote:
> If probe fails then we need to call pm_runtime_disable() to balance
> out the previous pm_runtime_enable() call. Else it will cause
> unbalanced pm_runtime_enable() call in the succeding probe call.
> 
> This anomaly was observed when the call to devm_phy_create() failed
> with -EPROBE_DEFER.
> 
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
>  drivers/phy/phy-omap-usb2.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/phy/phy-omap-usb2.c b/drivers/phy/phy-omap-usb2.c
> index 7007c11..c6f9809 100644
> --- a/drivers/phy/phy-omap-usb2.c
> +++ b/drivers/phy/phy-omap-usb2.c
> @@ -265,15 +265,19 @@ static int omap_usb2_probe(struct platform_device *pdev)
>  	pm_runtime_enable(phy->dev);
>  
>  	generic_phy = devm_phy_create(phy->dev, &ops, NULL);
> -	if (IS_ERR(generic_phy))
> +	if (IS_ERR(generic_phy)) {
> +		pm_runtime_disable(phy->dev);
>  		return PTR_ERR(generic_phy);
> +	}

Maybe we can move pm_runtime_enable to just before phy_provider_register?
>  
>  	phy_set_drvdata(generic_phy, phy);
>  
>  	phy_provider = devm_of_phy_provider_register(phy->dev,
>  			of_phy_simple_xlate);
> -	if (IS_ERR(phy_provider))
> +	if (IS_ERR(phy_provider)) {
> +		pm_runtime_disable(phy->dev);
>  		return PTR_ERR(phy_provider);
> +	}

Noticed pm_runtime_disable was missing in omap_usb2_remove too. Would be good
to fix it here.

Cheers
Kishon

Re: [PATCH 5/6] phy: omap-usb2: Balance pm_runtime_enable() on probe failure

[Roger Quadros]

On 07/03/2014 04:31 PM, Kishon Vijay Abraham I wrote:
> Hi Roger,
> 
> On Wednesday 02 July 2014 05:33 PM, Roger Quadros wrote:
>> If probe fails then we need to call pm_runtime_disable() to balance
>> out the previous pm_runtime_enable() call. Else it will cause
>> unbalanced pm_runtime_enable() call in the succeding probe call.
>>
>> This anomaly was observed when the call to devm_phy_create() failed
>> with -EPROBE_DEFER.
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>>  drivers/phy/phy-omap-usb2.c | 8 ++++++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/phy/phy-omap-usb2.c b/drivers/phy/phy-omap-usb2.c
>> index 7007c11..c6f9809 100644
>> --- a/drivers/phy/phy-omap-usb2.c
>> +++ b/drivers/phy/phy-omap-usb2.c
>> @@ -265,15 +265,19 @@ static int omap_usb2_probe(struct platform_device *pdev)
>>  	pm_runtime_enable(phy->dev);
>>  
>>  	generic_phy = devm_phy_create(phy->dev, &ops, NULL);
>> -	if (IS_ERR(generic_phy))
>> +	if (IS_ERR(generic_phy)) {
>> +		pm_runtime_disable(phy->dev);
>>  		return PTR_ERR(generic_phy);
>> +	}
> 
> Maybe we can move pm_runtime_enable to just before phy_provider_register?

OK. will try that.

>>  
>>  	phy_set_drvdata(generic_phy, phy);
>>  
>>  	phy_provider = devm_of_phy_provider_register(phy->dev,
>>  			of_phy_simple_xlate);
>> -	if (IS_ERR(phy_provider))
>> +	if (IS_ERR(phy_provider)) {
>> +		pm_runtime_disable(phy->dev);
>>  		return PTR_ERR(phy_provider);
>> +	}
> 
> Noticed pm_runtime_disable was missing in omap_usb2_remove too. Would be good
> to fix it here.
> 

Fine, I'll add it there.

cheers,
-roger

Re: [PATCH 5/6] phy: omap-usb2: Balance pm_runtime_enable() on probe failure

[Roger Quadros]

On 07/03/2014 04:31 PM, Kishon Vijay Abraham I wrote:
> Hi Roger,
> 
> On Wednesday 02 July 2014 05:33 PM, Roger Quadros wrote:
>> If probe fails then we need to call pm_runtime_disable() to balance
>> out the previous pm_runtime_enable() call. Else it will cause
>> unbalanced pm_runtime_enable() call in the succeding probe call.
>>
>> This anomaly was observed when the call to devm_phy_create() failed
>> with -EPROBE_DEFER.
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>>  drivers/phy/phy-omap-usb2.c | 8 ++++++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/phy/phy-omap-usb2.c b/drivers/phy/phy-omap-usb2.c
>> index 7007c11..c6f9809 100644
>> --- a/drivers/phy/phy-omap-usb2.c
>> +++ b/drivers/phy/phy-omap-usb2.c
>> @@ -265,15 +265,19 @@ static int omap_usb2_probe(struct platform_device *pdev)
>>  	pm_runtime_enable(phy->dev);
>>  
>>  	generic_phy = devm_phy_create(phy->dev, &ops, NULL);
>> -	if (IS_ERR(generic_phy))
>> +	if (IS_ERR(generic_phy)) {
>> +		pm_runtime_disable(phy->dev);
>>  		return PTR_ERR(generic_phy);
>> +	}
> 
> Maybe we can move pm_runtime_enable to just before phy_provider_register?

OK. will try that.

>>  
>>  	phy_set_drvdata(generic_phy, phy);
>>  
>>  	phy_provider = devm_of_phy_provider_register(phy->dev,
>>  			of_phy_simple_xlate);
>> -	if (IS_ERR(phy_provider))
>> +	if (IS_ERR(phy_provider)) {
>> +		pm_runtime_disable(phy->dev);
>>  		return PTR_ERR(phy_provider);
>> +	}
> 
> Noticed pm_runtime_disable was missing in omap_usb2_remove too. Would be good
> to fix it here.
> 

Fine, I'll add it there.

cheers,
-roger