[PATCH 3.15 12/84] ahci: imx: manage only sata_ref_clk in imx_sata_enable[disable]

[Greg Kroah-Hartman]

3.15-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Shawn Guo <shawn.guo@xxxxxxxxxxxxx>

commit e6dd42a917e62d916c6e513dbf87a4dec8cf3a1c upstream.

Doing suspend/resume on imx6q and imx53 boards with no SATA disk
attached will trigger the following warning.

------------[ cut here ]------------
WARNING: CPU: 0 PID: 661 at drivers/ata/libahci.c:224 ahci_enable_ahci+0x74/0x8)
Modules linked in:
CPU: 0 PID: 661 Comm: sh Tainted: G        W     3.15.0-rc5-next-20140521-000027
Backtrace:
[<80011c90>] (dump_backtrace) from [<80011e2c>] (show_stack+0x18/0x1c)
 r6:803a22f4 r5:00000000 r4:00000000 r3:00000000
[<80011e14>] (show_stack) from [<80661e60>] (dump_stack+0x88/0xa4)
[<80661dd8>] (dump_stack) from [<80028fdc>] (warn_slowpath_common+0x70/0x94)
 r5:00000009 r4:00000000
[<80028f6c>] (warn_slowpath_common) from [<80029024>] (warn_slowpath_null+0x24/)
 r8:808f68c4 r7:00000000 r6:00000000 r5:00000000 r4:e0810004
[<80029000>] (warn_slowpath_null) from [<803a22f4>] (ahci_enable_ahci+0x74/0x80)
[<803a2280>] (ahci_enable_ahci) from [<803a2324>] (ahci_reset_controller+0x24/0)
 r8:ddcd9410 r7:80351178 r6:ddcd9444 r5:dde8b850 r4:e0810000 r3:ddf35e90
[<803a2300>] (ahci_reset_controller) from [<803a2c68>] (ahci_platform_resume_ho)
 r7:80351178 r6:ddcd9444 r5:dde8b850 r4:ddcd9410
[<803a2c30>] (ahci_platform_resume_host) from [<803a38f0>] (imx_ahci_resume+0x2)
 r5:00000000 r4:ddcd9410
[<803a38c4>] (imx_ahci_resume) from [<803511ac>] (platform_pm_resume+0x34/0x54)
....

The reason is that the SATA controller has no working clock at this
point, and thus ahci_enable_ahci() fails to enable the controller.  In
case that there is no SATA disk attached, the imx_sata_disable() gets
called in ahci_imx_error_handler(), and both sata_clk and sata_ref_clk
will be disabled there.  Because all the imx_sata_enable() calls
afterward will return immediately due to imxpriv->no_device check, the
SATA controller working clock sata_clk will never get any chance to be
enabled again.

This is a regression caused by commit 90870d79d4f2 (ahci-imx: Port to
library-ised ahci_platform).  Before the commit, only sata_ref_clk is
managed by the driver in enable/disable function.  But after the commit,
all the clocks are enabled/disabled in a row by ahci platform helpers
ahci_platform_enable[disable]_clks.  Since ahb_clk is a bus clock which
does not have gate at all, and i.MX low-power hardware module already
manages sata_clk across suspend/resume cycle, the only clock that needs
to be managed by software is sata_ref_clk.

So instead of using ahci_platform_enable[disable]_clks to manage all
the clocks in a row from imx_sata_enable[disable], we should manage
only sata_ref_clk in there.

Reported-by: Fabio Estevam <fabio.estevam@xxxxxxxxxxxxx>
Fixes: 90870d79d4f2 (ahci-imx: Port to library-ised ahci_platform)
Signed-off-by: Shawn Guo <shawn.guo@xxxxxxxxxxxxx>
Acked-by: Hans de Goede <hdegoede@xxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>

---
 drivers/ata/ahci_imx.c |   35 +++++++++++++++++++++++++++++++----
 1 file changed, 31 insertions(+), 4 deletions(-)

--- a/drivers/ata/ahci_imx.c
+++ b/drivers/ata/ahci_imx.c
@@ -58,6 +58,8 @@ enum ahci_imx_type {
 struct imx_ahci_priv {
 	struct platform_device *ahci_pdev;
 	enum ahci_imx_type type;
+	struct clk *sata_clk;
+	struct clk *sata_ref_clk;
 	struct clk *ahb_clk;
 	struct regmap *gpr;
 	bool no_device;
@@ -224,7 +226,7 @@ static int imx_sata_enable(struct ahci_h
 			return ret;
 	}
 
-	ret = ahci_platform_enable_clks(hpriv);
+	ret = clk_prepare_enable(imxpriv->sata_ref_clk);
 	if (ret < 0)
 		goto disable_regulator;
 
@@ -291,7 +293,7 @@ static void imx_sata_disable(struct ahci
 				   !IMX6Q_GPR13_SATA_MPLL_CLK_EN);
 	}
 
-	ahci_platform_disable_clks(hpriv);
+	clk_disable_unprepare(imxpriv->sata_ref_clk);
 
 	if (hpriv->target_pwr)
 		regulator_disable(hpriv->target_pwr);
@@ -385,6 +387,19 @@ static int imx_ahci_probe(struct platfor
 	imxpriv->no_device = false;
 	imxpriv->first_time = true;
 	imxpriv->type = (enum ahci_imx_type)of_id->data;
+
+	imxpriv->sata_clk = devm_clk_get(dev, "sata");
+	if (IS_ERR(imxpriv->sata_clk)) {
+		dev_err(dev, "can't get sata clock.\n");
+		return PTR_ERR(imxpriv->sata_clk);
+	}
+
+	imxpriv->sata_ref_clk = devm_clk_get(dev, "sata_ref");
+	if (IS_ERR(imxpriv->sata_ref_clk)) {
+		dev_err(dev, "can't get sata_ref clock.\n");
+		return PTR_ERR(imxpriv->sata_ref_clk);
+	}
+
 	imxpriv->ahb_clk = devm_clk_get(dev, "ahb");
 	if (IS_ERR(imxpriv->ahb_clk)) {
 		dev_err(dev, "can't get ahb clock.\n");
@@ -407,10 +422,14 @@ static int imx_ahci_probe(struct platfor
 
 	hpriv->plat_data = imxpriv;
 
-	ret = imx_sata_enable(hpriv);
+	ret = clk_prepare_enable(imxpriv->sata_clk);
 	if (ret)
 		return ret;
 
+	ret = imx_sata_enable(hpriv);
+	if (ret)
+		goto disable_clk;
+
 	/*
 	 * Configure the HWINIT bits of the HOST_CAP and HOST_PORTS_IMPL,
 	 * and IP vendor specific register IMX_TIMER1MS.
@@ -434,16 +453,24 @@ static int imx_ahci_probe(struct platfor
 
 	ret = ahci_platform_init_host(pdev, hpriv, &ahci_imx_port_info, 0, 0);
 	if (ret)
-		imx_sata_disable(hpriv);
+		goto disable_sata;
+
+	return 0;
 
+disable_sata:
+	imx_sata_disable(hpriv);
+disable_clk:
+	clk_disable_unprepare(imxpriv->sata_clk);
 	return ret;
 }
 
 static void ahci_imx_host_stop(struct ata_host *host)
 {
 	struct ahci_host_priv *hpriv = host->private_data;
+	struct imx_ahci_priv *imxpriv = hpriv->plat_data;
 
 	imx_sata_disable(hpriv);
+	clk_disable_unprepare(imxpriv->sata_clk);
 }
 
 #ifdef CONFIG_PM_SLEEP


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/