All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kishon Vijay Abraham I <kishon@ti.com>
To: <gregkh@linuxfoundation.org>
Cc: <kishon@ti.com>, <linux-kernel@vger.kernel.org>
Subject: [PATCH 1/4] phy: ti-pipe3: fix suspend
Date: Sat, 18 Jul 2015 11:27:46 +0530	[thread overview]
Message-ID: <1437199069-18643-2-git-send-email-kishon@ti.com> (raw)
In-Reply-To: <1437199069-18643-1-git-send-email-kishon@ti.com>

From: Roger Quadros <rogerq@ti.com>

Relying on PM-ops for shutting down PHY clocks was a
bad idea since the users (e.g. PCIe/SATA) might not
have been suspended by then.

The main culprit for not shutting down the clocks was
the stray pm_runtime_get() call in probe.

Fix the whole thing in the right way by getting rid
of that pm_runtime_get() call from probe and
removing all PM-ops. It is the sole responsibility
of the PHY user to properly turn OFF and de-initialize
the PHY as part of its suspend routine.

As PHY core serializes init/exit we don't need
to use a spinlock in this driver. So get rid of it.

Signed-off-by: Roger Quadros <rogerq@ti.com>
Signed-off-by: Sekhar Nori <nsekhar@ti.com>
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 drivers/phy/phy-ti-pipe3.c |  172 +++++++++++---------------------------------
 1 file changed, 41 insertions(+), 131 deletions(-)

diff --git a/drivers/phy/phy-ti-pipe3.c b/drivers/phy/phy-ti-pipe3.c
index 53f295c..3510b81 100644
--- a/drivers/phy/phy-ti-pipe3.c
+++ b/drivers/phy/phy-ti-pipe3.c
@@ -28,7 +28,6 @@
 #include <linux/delay.h>
 #include <linux/phy/omap_control_phy.h>
 #include <linux/of_platform.h>
-#include <linux/spinlock.h>
 
 #define	PLL_STATUS		0x00000004
 #define	PLL_GO			0x00000008
@@ -83,10 +82,6 @@ struct ti_pipe3 {
 	struct clk		*refclk;
 	struct clk		*div_clk;
 	struct pipe3_dpll_map	*dpll_map;
-	bool			enabled;
-	spinlock_t		lock;	/* serialize clock enable/disable */
-	/* the below flag is needed specifically for SATA */
-	bool			refclk_enabled;
 };
 
 static struct pipe3_dpll_map dpll_map_usb[] = {
@@ -137,6 +132,9 @@ static struct pipe3_dpll_params *ti_pipe3_get_dpll_params(struct ti_pipe3 *phy)
 	return NULL;
 }
 
+static int ti_pipe3_enable_clocks(struct ti_pipe3 *phy);
+static void ti_pipe3_disable_clocks(struct ti_pipe3 *phy);
+
 static int ti_pipe3_power_off(struct phy *x)
 {
 	struct ti_pipe3 *phy = phy_get_drvdata(x);
@@ -217,6 +215,7 @@ static int ti_pipe3_init(struct phy *x)
 	u32 val;
 	int ret = 0;
 
+	ti_pipe3_enable_clocks(phy);
 	/*
 	 * Set pcie_pcs register to 0x96 for proper functioning of phy
 	 * as recommended in AM572x TRM SPRUHZ6, section 18.5.2.2, table
@@ -250,33 +249,35 @@ static int ti_pipe3_exit(struct phy *x)
 	u32 val;
 	unsigned long timeout;
 
-	/* SATA DPLL can't be powered down due to Errata i783 and PCIe
-	 * does not have internal DPLL
-	 */
-	if (of_device_is_compatible(phy->dev->of_node, "ti,phy-pipe3-sata") ||
-	    of_device_is_compatible(phy->dev->of_node, "ti,phy-pipe3-pcie"))
+	/* SATA DPLL can't be powered down due to Errata i783 */
+	if (of_device_is_compatible(phy->dev->of_node, "ti,phy-pipe3-sata"))
 		return 0;
 
-	/* Put DPLL in IDLE mode */
-	val = ti_pipe3_readl(phy->pll_ctrl_base, PLL_CONFIGURATION2);
-	val |= PLL_IDLE;
-	ti_pipe3_writel(phy->pll_ctrl_base, PLL_CONFIGURATION2, val);
-
-	/* wait for LDO and Oscillator to power down */
-	timeout = jiffies + msecs_to_jiffies(PLL_IDLE_TIME);
-	do {
-		cpu_relax();
-		val = ti_pipe3_readl(phy->pll_ctrl_base, PLL_STATUS);
-		if ((val & PLL_TICOPWDN) && (val & PLL_LDOPWDN))
-			break;
-	} while (!time_after(jiffies, timeout));
+	/* PCIe doesn't have internal DPLL */
+	if (!of_device_is_compatible(phy->dev->of_node, "ti,phy-pipe3-pcie")) {
+		/* Put DPLL in IDLE mode */
+		val = ti_pipe3_readl(phy->pll_ctrl_base, PLL_CONFIGURATION2);
+		val |= PLL_IDLE;
+		ti_pipe3_writel(phy->pll_ctrl_base, PLL_CONFIGURATION2, val);
 
-	if (!(val & PLL_TICOPWDN) || !(val & PLL_LDOPWDN)) {
-		dev_err(phy->dev, "Failed to power down: PLL_STATUS 0x%x\n",
-			val);
-		return -EBUSY;
+		/* wait for LDO and Oscillator to power down */
+		timeout = jiffies + msecs_to_jiffies(PLL_IDLE_TIME);
+		do {
+			cpu_relax();
+			val = ti_pipe3_readl(phy->pll_ctrl_base, PLL_STATUS);
+			if ((val & PLL_TICOPWDN) && (val & PLL_LDOPWDN))
+				break;
+		} while (!time_after(jiffies, timeout));
+
+		if (!(val & PLL_TICOPWDN) || !(val & PLL_LDOPWDN)) {
+			dev_err(phy->dev, "Failed to power down: PLL_STATUS 0x%x\n",
+				val);
+			return -EBUSY;
+		}
 	}
 
+	ti_pipe3_disable_clocks(phy);
+
 	return 0;
 }
 static struct phy_ops ops = {
@@ -306,7 +307,6 @@ static int ti_pipe3_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	phy->dev		= &pdev->dev;
-	spin_lock_init(&phy->lock);
 
 	if (!of_device_is_compatible(node, "ti,phy-pipe3-pcie")) {
 		match = of_match_device(ti_pipe3_id_table, &pdev->dev);
@@ -402,6 +402,10 @@ static int ti_pipe3_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, phy);
 	pm_runtime_enable(phy->dev);
+	/* Prevent auto-disable of refclk for SATA PHY due to Errata i783 */
+	if (of_device_is_compatible(node, "ti,phy-pipe3-sata"))
+		if (!IS_ERR(phy->refclk))
+			clk_prepare_enable(phy->refclk);
 
 	generic_phy = devm_phy_create(phy->dev, NULL, &ops);
 	if (IS_ERR(generic_phy))
@@ -413,63 +417,33 @@ static int ti_pipe3_probe(struct platform_device *pdev)
 	if (IS_ERR(phy_provider))
 		return PTR_ERR(phy_provider);
 
-	pm_runtime_get(&pdev->dev);
-
 	return 0;
 }
 
 static int ti_pipe3_remove(struct platform_device *pdev)
 {
-	if (!pm_runtime_suspended(&pdev->dev))
-		pm_runtime_put(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 
 	return 0;
 }
 
-#ifdef CONFIG_PM
-static int ti_pipe3_enable_refclk(struct ti_pipe3 *phy)
+static int ti_pipe3_enable_clocks(struct ti_pipe3 *phy)
 {
-	if (!IS_ERR(phy->refclk) && !phy->refclk_enabled) {
-		int ret;
+	int ret = 0;
 
+	if (!IS_ERR(phy->refclk)) {
 		ret = clk_prepare_enable(phy->refclk);
 		if (ret) {
 			dev_err(phy->dev, "Failed to enable refclk %d\n", ret);
 			return ret;
 		}
-		phy->refclk_enabled = true;
 	}
 
-	return 0;
-}
-
-static void ti_pipe3_disable_refclk(struct ti_pipe3 *phy)
-{
-	if (!IS_ERR(phy->refclk))
-		clk_disable_unprepare(phy->refclk);
-
-	phy->refclk_enabled = false;
-}
-
-static int ti_pipe3_enable_clocks(struct ti_pipe3 *phy)
-{
-	int ret = 0;
-	unsigned long flags;
-
-	spin_lock_irqsave(&phy->lock, flags);
-	if (phy->enabled)
-		goto err1;
-
-	ret = ti_pipe3_enable_refclk(phy);
-	if (ret)
-		goto err1;
-
 	if (!IS_ERR(phy->wkupclk)) {
 		ret = clk_prepare_enable(phy->wkupclk);
 		if (ret) {
 			dev_err(phy->dev, "Failed to enable wkupclk %d\n", ret);
-			goto err2;
+			goto disable_refclk;
 		}
 	}
 
@@ -477,96 +451,33 @@ static int ti_pipe3_enable_clocks(struct ti_pipe3 *phy)
 		ret = clk_prepare_enable(phy->div_clk);
 		if (ret) {
 			dev_err(phy->dev, "Failed to enable div_clk %d\n", ret);
-			goto err3;
+			goto disable_wkupclk;
 		}
 	}
 
-	phy->enabled = true;
-	spin_unlock_irqrestore(&phy->lock, flags);
 	return 0;
 
-err3:
+disable_wkupclk:
 	if (!IS_ERR(phy->wkupclk))
 		clk_disable_unprepare(phy->wkupclk);
 
-err2:
+disable_refclk:
 	if (!IS_ERR(phy->refclk))
 		clk_disable_unprepare(phy->refclk);
 
-	ti_pipe3_disable_refclk(phy);
-err1:
-	spin_unlock_irqrestore(&phy->lock, flags);
 	return ret;
 }
 
 static void ti_pipe3_disable_clocks(struct ti_pipe3 *phy)
 {
-	unsigned long flags;
-
-	spin_lock_irqsave(&phy->lock, flags);
-	if (!phy->enabled) {
-		spin_unlock_irqrestore(&phy->lock, flags);
-		return;
-	}
-
 	if (!IS_ERR(phy->wkupclk))
 		clk_disable_unprepare(phy->wkupclk);
-	/* Don't disable refclk for SATA PHY due to Errata i783 */
-	if (!of_device_is_compatible(phy->dev->of_node, "ti,phy-pipe3-sata"))
-		ti_pipe3_disable_refclk(phy);
+	if (!IS_ERR(phy->refclk))
+		clk_disable_unprepare(phy->refclk);
 	if (!IS_ERR(phy->div_clk))
 		clk_disable_unprepare(phy->div_clk);
-	phy->enabled = false;
-	spin_unlock_irqrestore(&phy->lock, flags);
-}
-
-static int ti_pipe3_runtime_suspend(struct device *dev)
-{
-	struct ti_pipe3	*phy = dev_get_drvdata(dev);
-
-	ti_pipe3_disable_clocks(phy);
-	return 0;
 }
 
-static int ti_pipe3_runtime_resume(struct device *dev)
-{
-	struct ti_pipe3	*phy = dev_get_drvdata(dev);
-	int ret = 0;
-
-	ret = ti_pipe3_enable_clocks(phy);
-	return ret;
-}
-
-static int ti_pipe3_suspend(struct device *dev)
-{
-	struct ti_pipe3	*phy = dev_get_drvdata(dev);
-
-	ti_pipe3_disable_clocks(phy);
-	return 0;
-}
-
-static int ti_pipe3_resume(struct device *dev)
-{
-	struct ti_pipe3	*phy = dev_get_drvdata(dev);
-	int ret;
-
-	ret = ti_pipe3_enable_clocks(phy);
-	if (ret)
-		return ret;
-
-	pm_runtime_disable(dev);
-	pm_runtime_set_active(dev);
-	pm_runtime_enable(dev);
-	return 0;
-}
-#endif
-
-static const struct dev_pm_ops ti_pipe3_pm_ops = {
-	SET_RUNTIME_PM_OPS(ti_pipe3_runtime_suspend,
-			   ti_pipe3_runtime_resume, NULL)
-	SET_SYSTEM_SLEEP_PM_OPS(ti_pipe3_suspend, ti_pipe3_resume)
-};
-
 static const struct of_device_id ti_pipe3_id_table[] = {
 	{
 		.compatible = "ti,phy-usb3",
@@ -592,7 +503,6 @@ static struct platform_driver ti_pipe3_driver = {
 	.remove		= ti_pipe3_remove,
 	.driver		= {
 		.name	= "ti-pipe3",
-		.pm	= &ti_pipe3_pm_ops,
 		.of_match_table = ti_pipe3_id_table,
 	},
 };
-- 
1.7.9.5


  reply	other threads:[~2015-07-18  5:57 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-18  5:57 [GIT PULL 0/4] phy: for 4.2 -rc cycle Kishon Vijay Abraham I
2015-07-18  5:57 ` Kishon Vijay Abraham I [this message]
2015-07-18  5:57 ` [PATCH 2/4] phy/pxa: add HAS_IOMEM dependency Kishon Vijay Abraham I
2015-07-18  5:57 ` [PATCH 3/4] phy: berlin-usb: fix divider for BG2CD Kishon Vijay Abraham I
2015-07-18  5:57 ` [PATCH 4/4] phy: berlin-usb: fix divider for BG2 Kishon Vijay Abraham I
2015-07-22 21:23 ` [GIT PULL 0/4] phy: for 4.2 -rc cycle Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1437199069-18643-2-git-send-email-kishon@ti.com \
    --to=kishon@ti.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.